Add GL_CHROMIUM_screen_space_antialiasing to support alternative AA

Add GL_CHROMIUM_screen_space_antialiasing to support alternative AA

MSAA is very expensive on some platforms. We introduce extention
GL_CHROMIUM_screen_space_antialiasing to support other antialiasing
approach. It will perform antialiasing to the color attachments of
the currently bound draw framebuffer. As an example, we implement it
using GL_INTEL_framebuffer_CMAA which results in similar quality of
MSAA at better performance.

The blink side change is in below link:
https://codereview.chromium.org/1301923002

BUG=524285
Committed: https://crrev.com/f23471c866d7d3334d61538eb738d494542b5cac
Cr-Commit-Position: refs/heads/master@{#348412}

[Jin Yang, 2015-08-19 07:21:28]

jin.a.yang@intel.com changed reviewers:
+ bajones@chromium.org, kbr@chromium.org, piman@chromium.org,
sievers@chromium.org, zmo@chromium.org

[bajones, 2015-08-19 16:32:16]

gpu/command_buffer/ LGTM.

[Ken Russell (switch to Gerrit), 2015-08-19 17:49:35]

There are a huge number of whitespace changes under ui/gl that are unrelated to
this patch. Is that because clang-format was upgraded recently? If so, could you
please submit another patch simply regenerating the autogenerated sources
without the CMAA extension, so this patch can be smaller?

I'd also like to point out that WebGL 2.0 exposes multisampled renderbuffers
directly to the application, and it's expected this functionality will be widely
used. Therefore having a faster antialiasing path for the default framebuffer
will be less useful going forward. I think Intel should focus on making their
multisampling support faster.

[piman, 2015-08-19 22:31:30]

As I understand from the sample linked to in
https://software.intel.com/en-us/articles/conservative-morphological-anti-ali...,
it seems like the CMAA technique can be implemented entirely on the client side.
I would prefer going that route for several reasons:
- avoiding the explosion of vendor-specific extensions in the command buffer
(lower maintenance burden, lower security footprint)
- making the technique applicable to more hardware
- making the technique testable and debuggable on all hardware rather than
specific one
- better tracking of the memory for the underlying resources (temporary render
targets) used in the implementation

Thoughts?

[Jin Yang, 2015-08-21 06:01:34]

On 2015/08/19 17:49:35, Ken Russell wrote:
> There are a huge number of whitespace changes under ui/gl that are unrelated
to
> this patch. Is that because clang-format was upgraded recently? If so, could
you
> please submit another patch simply regenerating the autogenerated sources
> without the CMAA extension, so this patch can be smaller?
Done.

> I'd also like to point out that WebGL 2.0 exposes multisampled renderbuffers
> directly to the application, and it's expected this functionality will be
widely
> used. Therefore having a faster antialiasing path for the default framebuffer
> will be less useful going forward. I think Intel should focus on making their
> multisampling support faster.

Agree, we will continue to optimize multisampling on Intel gfx as well.

[Jin Yang, 2015-08-21 06:02:52]

On 2015/08/19 22:31:30, piman (slow to review) wrote:
> As I understand from the sample linked to in
>
https://software.intel.com/en-us/articles/conservative-morphological-anti-ali...,
> it seems like the CMAA technique can be implemented entirely on the client
side.
> I would prefer going that route for several reasons:
> - avoiding the explosion of vendor-specific extensions in the command buffer
> (lower maintenance burden, lower security footprint)
> - making the technique applicable to more hardware
> - making the technique testable and debuggable on all hardware rather than
> specific one
> - better tracking of the memory for the underlying resources (temporary render
> targets) used in the implementation
> 
> Thoughts?

That sounds interesting. The sample code in this article is just for
demonstrating the algorithm of CMAA. AFAIK, our implementation in driver can be
fully optimized but has dependence on different gfx capabilities on Intel
platforms. At the same time, as the CMAA extension becomes a public extension,
it can hide the details of HW dependence, resource management for developer
which they do not need to care much..

[Ken Russell (switch to Gerrit), 2015-08-25 01:13:14]

On 2015/08/21 06:02:52, Jin Yang wrote:
> On 2015/08/19 22:31:30, piman (slow to review) wrote:
> > As I understand from the sample linked to in
> >
>
https://software.intel.com/en-us/articles/conservative-morphological-anti-ali...,
> > it seems like the CMAA technique can be implemented entirely on the client
> side.
> > I would prefer going that route for several reasons:
> > - avoiding the explosion of vendor-specific extensions in the command buffer
> > (lower maintenance burden, lower security footprint)
> > - making the technique applicable to more hardware
> > - making the technique testable and debuggable on all hardware rather than
> > specific one
> > - better tracking of the memory for the underlying resources (temporary
render
> > targets) used in the implementation
> > 
> > Thoughts?
> 
> That sounds interesting. The sample code in this article is just for
> demonstrating the algorithm of CMAA. AFAIK, our implementation in driver can
be
> fully optimized but has dependence on different gfx capabilities on Intel
> platforms. At the same time, as the CMAA extension becomes a public extension,
> it can hide the details of HW dependence, resource management for developer
> which they do not need to care much..

Jin: that doesn't really answer the question -- would you be able / willing to
reimplement this CMAA technique at the Blink level, in DrawingBuffer.cpp or
related helper classes? Or is that infeasible for some reason?

It's undesirable to expose a vendor-specific extension in the command buffer,
because any other vendor with similar functionality will not want to implement
it under the other vendor's extension name. So, after offline discussion with
piman, if it's not feasible to implement this on the client side, please:

1. Define a CHROMIUM_ extension under src/gpu/GLES2/extensions/CHROMIUM/ like
CHROMIUM_screen_space_antialiasing and document the semantics. It can
essentially be the same as the Intel CMAA extension -- applying the antialiasing
to the color attachments of the currently bound framebuffer -- and refer to the
other extension for details.
2. Reimplement this patch to expose some function like
glApplyScreenSpaceAntialiasingCHROMIUM, manually implement it on the service
side, and have the implementation call ApplyFramebufferAttachmentCMAAINTEL.

Thanks. (I also took the liberty of pointing your two CLs at newly-filed Issue
524285 so that the history of this discussion can be tracked.)

[Jin Yang, 2015-08-28 00:58:50]

On 2015/08/25 01:13:14, Ken Russell wrote:
> On 2015/08/21 06:02:52, Jin Yang wrote:
> > On 2015/08/19 22:31:30, piman (slow to review) wrote:
> > > As I understand from the sample linked to in
> > >
> >
>
https://software.intel.com/en-us/articles/conservative-morphological-anti-ali...,
> > > it seems like the CMAA technique can be implemented entirely on the client
> > side.
> > > I would prefer going that route for several reasons:
> > > - avoiding the explosion of vendor-specific extensions in the command
buffer
> > > (lower maintenance burden, lower security footprint)
> > > - making the technique applicable to more hardware
> > > - making the technique testable and debuggable on all hardware rather than
> > > specific one
> > > - better tracking of the memory for the underlying resources (temporary
> render
> > > targets) used in the implementation
> > > 
> > > Thoughts?
> > 
> > That sounds interesting. The sample code in this article is just for
> > demonstrating the algorithm of CMAA. AFAIK, our implementation in driver can
> be
> > fully optimized but has dependence on different gfx capabilities on Intel
> > platforms. At the same time, as the CMAA extension becomes a public
extension,
> > it can hide the details of HW dependence, resource management for developer
> > which they do not need to care much..
> 
> Jin: that doesn't really answer the question -- would you be able / willing to
> reimplement this CMAA technique at the Blink level, in DrawingBuffer.cpp or
> related helper classes? Or is that infeasible for some reason?
> 
> It's undesirable to expose a vendor-specific extension in the command buffer,
> because any other vendor with similar functionality will not want to implement
> it under the other vendor's extension name. So, after offline discussion with
> piman, if it's not feasible to implement this on the client side, please:
> 
> 1. Define a CHROMIUM_ extension under src/gpu/GLES2/extensions/CHROMIUM/ like
> CHROMIUM_screen_space_antialiasing and document the semantics. It can
> essentially be the same as the Intel CMAA extension -- applying the
antialiasing
> to the color attachments of the currently bound framebuffer -- and refer to
the
> other extension for details.
> 2. Reimplement this patch to expose some function like
> glApplyScreenSpaceAntialiasingCHROMIUM, manually implement it on the service
> side, and have the implementation call ApplyFramebufferAttachmentCMAAINTEL.
> 
> Thanks. (I also took the liberty of pointing your two CLs at newly-filed Issue
> 524285 so that the history of this discussion can be tracked.)

Patch is uploaded, please help review it. Thanks

[Ken Russell (switch to Gerrit), 2015-08-28 02:34:03]

Thank you. Looks good overall. One naming issue in several places in the patch.

Also, I'm not an owner of the command buffer code so you'll need either sievers'
or zmo's review (piman's out until next Monday).

https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2chromium_a...
File gpu/GLES2/gl2chromium_autogen.h (right):

https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2chromium_a...
gpu/GLES2/gl2chromium_autogen.h:363: GLES2_GET_FUN(ApplyScreenSpaceAntialiasing)
Should be ApplyScreenSpaceAntialiasingCHROMIUM.

https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2extchromium.h
File gpu/GLES2/gl2extchromium.h (right):

https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2extchromiu...
gpu/GLES2/gl2extchromium.h:527: GL_APICALL void GL_APIENTRY
glApplyScreenSpaceAntialiasing();
Should be glApplyScreenSpaceAntialiasingCHROMIUM.

https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2extchromiu...
gpu/GLES2/gl2extchromium.h:529: typedef void (GL_APIENTRYP
PFNGLAPPLYSCREENSPACEANTIALIASINGPROC) ();
Should be PFNGLAPPLYSCREENSPACEANTIALIASINGCHROMIUMPROC

https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
File gpu/blink/webgraphicscontext3d_impl.cc (right):

https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
gpu/blink/webgraphicscontext3d_impl.cc:261:
DELEGATE_TO_GL(applyScreenSpaceAntialiasing,
Same here.

https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
File gpu/blink/webgraphicscontext3d_impl.h (right):

https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
gpu/blink/webgraphicscontext3d_impl.h:53: virtual void
applyScreenSpaceAntialiasing();
Should be applyScreenSpaceAntialiasingCHROMIUM.

https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/buil...
File gpu/command_buffer/build_gles2_cmd_buffer.py (right):

https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/buil...
gpu/command_buffer/build_gles2_cmd_buffer.py:2136:
'ApplyScreenSpaceAntialiasing': {
CHROMIUM extension suffix here.

https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/buil...
gpu/command_buffer/build_gles2_cmd_buffer.py:2137: 'decoder_func':
'DoApplyScreenSpaceAntialiasing',
Same here.

https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/cmd_...
File gpu/command_buffer/cmd_buffer_functions.txt (right):

https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/cmd_...
gpu/command_buffer/cmd_buffer_functions.txt:345: GL_APICALL void GL_APIENTRY
glApplyScreenSpaceAntialiasing (void);
Should be glApplyScreenSpaceAntialiasingCHROMIUM.

[Jin Yang, 2015-08-28 05:53:07]

On 2015/08/28 02:34:03, Ken Russell wrote:
> Thank you. Looks good overall. One naming issue in several places in the
patch.
> 
> Also, I'm not an owner of the command buffer code so you'll need either
sievers'
> or zmo's review (piman's out until next Monday).
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2chromium_a...
> File gpu/GLES2/gl2chromium_autogen.h (right):
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2chromium_a...
> gpu/GLES2/gl2chromium_autogen.h:363:
GLES2_GET_FUN(ApplyScreenSpaceAntialiasing)
> Should be ApplyScreenSpaceAntialiasingCHROMIUM.
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2extchromium.h
> File gpu/GLES2/gl2extchromium.h (right):
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2extchromiu...
> gpu/GLES2/gl2extchromium.h:527: GL_APICALL void GL_APIENTRY
> glApplyScreenSpaceAntialiasing();
> Should be glApplyScreenSpaceAntialiasingCHROMIUM.
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/GLES2/gl2extchromiu...
> gpu/GLES2/gl2extchromium.h:529: typedef void (GL_APIENTRYP
> PFNGLAPPLYSCREENSPACEANTIALIASINGPROC) ();
> Should be PFNGLAPPLYSCREENSPACEANTIALIASINGCHROMIUMPROC
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
> File gpu/blink/webgraphicscontext3d_impl.cc (right):
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
> gpu/blink/webgraphicscontext3d_impl.cc:261:
> DELEGATE_TO_GL(applyScreenSpaceAntialiasing,
> Same here.
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
> File gpu/blink/webgraphicscontext3d_impl.h (right):
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/blink/webgraphicsco...
> gpu/blink/webgraphicscontext3d_impl.h:53: virtual void
> applyScreenSpaceAntialiasing();
> Should be applyScreenSpaceAntialiasingCHROMIUM.
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/buil...
> File gpu/command_buffer/build_gles2_cmd_buffer.py (right):
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/buil...
> gpu/command_buffer/build_gles2_cmd_buffer.py:2136:
> 'ApplyScreenSpaceAntialiasing': {
> CHROMIUM extension suffix here.
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/buil...
> gpu/command_buffer/build_gles2_cmd_buffer.py:2137: 'decoder_func':
> 'DoApplyScreenSpaceAntialiasing',
> Same here.
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/cmd_...
> File gpu/command_buffer/cmd_buffer_functions.txt (right):
> 
>
https://codereview.chromium.org/1298523003/diff/40001/gpu/command_buffer/cmd_...
> gpu/command_buffer/cmd_buffer_functions.txt:345: GL_APICALL void GL_APIENTRY
> glApplyScreenSpaceAntialiasing (void);
> Should be glApplyScreenSpaceAntialiasingCHROMIUM.

Naming fixed. @sievers, zmo, please also help review the command buffer part.
Thanks!

[Ken Russell (switch to Gerrit), 2015-08-28 20:27:34]

This LGTM from my standpoint.

I'd like to know how we're going to test this code path. Jin, on what Intel GPUs
and with what driver versions is the CMAA extension supported?

[Jin Yang, 2015-08-31 09:28:32]

On 2015/08/28 20:27:34, Ken Russell wrote:
> This LGTM from my standpoint.
> 
> I'd like to know how we're going to test this code path. Jin, on what Intel
GPUs
> and with what driver versions is the CMAA extension supported?

Russell, we will have this extension on Intel CherryTrail GPU with the GL render
string of "Intel(R) HD Graphics for Atom(TM) x5/x7". Currently CherryTrail is
not in the market, but could be available soon. Do you have any good suggestion
on the testing side of the code path?

Thanks,
Jin

[Ken Russell (switch to Gerrit), 2015-09-01 01:18:05]

On 2015/08/31 09:28:32, Jin Yang wrote:
> On 2015/08/28 20:27:34, Ken Russell wrote:
> > This LGTM from my standpoint.
> > 
> > I'd like to know how we're going to test this code path. Jin, on what Intel
> GPUs
> > and with what driver versions is the CMAA extension supported?
> 
> Russell, we will have this extension on Intel CherryTrail GPU with the GL
render
> string of "Intel(R) HD Graphics for Atom(TM) x5/x7". Currently CherryTrail is
> not in the market, but could be available soon. Do you have any good
suggestion
> on the testing side of the code path?
> 
> Thanks,
> Jin

If you could make this extension available on the Intel HD 4000 or 5000 series
that would be helpful. This is the type of hardware we will have in our testing
lab.

Please contact me via email directly about this possibility, or about the
possibility of putting hardware that supports this extension on
http://build.chromium.org/p/chromium.gpu.fyi/console .

[Jin Yang, 2015-09-07 02:08:25]

On 2015/09/01 01:18:05, Ken Russell wrote:
> On 2015/08/31 09:28:32, Jin Yang wrote:
> > On 2015/08/28 20:27:34, Ken Russell wrote:
> > > This LGTM from my standpoint.
> > > 
> > > I'd like to know how we're going to test this code path. Jin, on what
Intel
> > GPUs
> > > and with what driver versions is the CMAA extension supported?
> > 
> > Russell, we will have this extension on Intel CherryTrail GPU with the GL
> render
> > string of "Intel(R) HD Graphics for Atom(TM) x5/x7". Currently CherryTrail
is
> > not in the market, but could be available soon. Do you have any good
> suggestion
> > on the testing side of the code path?
> > 
> > Thanks,
> > Jin
> 
> If you could make this extension available on the Intel HD 4000 or 5000 series
> that would be helpful. This is the type of hardware we will have in our
testing
> lab.
> 
> Please contact me via email directly about this possibility, or about the
> possibility of putting hardware that supports this extension on
> http://build.chromium.org/p/chromium.gpu.fyi/console .

sievers, zmo, PTAL. ken is LGTM for the other parts.

[piman, 2015-09-09 01:37:52]

LGTM+nit

https://codereview.chromium.org/1298523003/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1298523003/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:13471:
glApplyFramebufferAttachmentCMAAINTEL();
nit: indent looks wrong. Apply 'git cl format' ?

[Jin Yang, 2015-09-11 15:36:49]

The CQ bit was checked by jin.a.yang@intel.com

[Jin Yang, 2015-09-11 15:36:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
bajones@chromium.org, kbr@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1298523003/#ps80001
(title: "fix nit")

[commit-bot: I haz the power, 2015-09-11 15:36:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1298523003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1298523003/80001

[commit-bot: I haz the power, 2015-09-11 16:46:54]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-09-11 16:47:40]

Patchset 5 (id:??) landed as
https://crrev.com/f23471c866d7d3334d61538eb738d494542b5cac
Cr-Commit-Position: refs/heads/master@{#348412}

[commit-bot: I haz the power, 2015-09-23 12:23:08]

Patchset 5 (id:??) landed as
https://crrev.com/f23471c866d7d3334d61538eb738d494542b5cac
Cr-Commit-Position: refs/heads/master@{#348412}