Add support for GL_EXT_raster_multisample

Add support for GL_EXT_raster_multisample

Updates the GrGLGpu to call glRasterSamplesEXT when multisample is
enabled for a mixed sampled render target, but the stencil test is not
active.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1666563003

Committed: https://skia.googlesource.com/skia/+/af8bc7d7a4fb1fbbc15bb0881a360ff0d7551ea2

[Chris Dalton, 2016-02-03 18:06:31]

Description was changed from

==========
Add support for GL_EXT_raster_multisample

Updates the GrGLGpu to call glRasterSamplesEXT when multisample is
enabled for a mixed sampled render target, but the stencil test is not
active.

BUG=skia:
==========

to

==========
Add support for GL_EXT_raster_multisample

Updates the GrGLGpu to call glRasterSamplesEXT when multisample is
enabled for a mixed sampled render target, but the stencil test is not
active.

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[Chris Dalton, 2016-02-03 18:16:47]

cdalton@nvidia.com changed reviewers:
+ bsalomon@google.com, egdaniel@google.com

[Chris Dalton, 2016-02-03 18:16:47]

https://codereview.chromium.org/1666563003/diff/1/src/gpu/gl/GrGLGpu.cpp
File src/gpu/gl/GrGLGpu.cpp (right):

https://codereview.chromium.org/1666563003/diff/1/src/gpu/gl/GrGLGpu.cpp#newc...
src/gpu/gl/GrGLGpu.cpp:2722: void GrGLGpu::flushHWAAState(GrRenderTarget* rt,
bool useHWAA, bool stencilEnabled) {
I'm not thrilled about this extra "stencilEnabled" parameter. Definitely open to
better ideas.

[egdaniel, 2016-02-03 18:40:49]

https://codereview.chromium.org/1666563003/diff/1/src/gpu/gl/GrGLGpu.cpp
File src/gpu/gl/GrGLGpu.cpp (right):

https://codereview.chromium.org/1666563003/diff/1/src/gpu/gl/GrGLGpu.cpp#newc...
src/gpu/gl/GrGLGpu.cpp:2748: SkASSERT(rt->numStencilSamples() <=
this->caps()->maxRasterSamples());
If I recall, when we set the rt stencil samples, we do some clamp to max sample
value. I feel like we either need to explicitly take raster samples into account
there or have an assert somewhere that maxStencilSamples <= maxRasterSamples.

If we believe that maxRasterSamples could be less than maxStencilSamples then
we'll just need to do the explicit check where we set it.

[Chris Dalton, 2016-02-03 18:54:26]

Come to think of it, maybe we ought to just clamp MAX_SAMPLES. The real world
effect would be clamping MAX_SAMPLES down from 32 to 16 on desktop NVIDIA
platforms.

https://codereview.chromium.org/1666563003/diff/1/src/gpu/gl/GrGLGpu.cpp
File src/gpu/gl/GrGLGpu.cpp (right):

https://codereview.chromium.org/1666563003/diff/1/src/gpu/gl/GrGLGpu.cpp#newc...
src/gpu/gl/GrGLGpu.cpp:2748: SkASSERT(rt->numStencilSamples() <=
this->caps()->maxRasterSamples());
On 2016/02/03 18:40:49, egdaniel wrote:
> If I recall, when we set the rt stencil samples, we do some clamp to max
sample
> value. I feel like we either need to explicitly take raster samples into
account
> there or have an assert somewhere that maxStencilSamples <= maxRasterSamples.
> 
> If we believe that maxRasterSamples could be less than maxStencilSamples then
> we'll just need to do the explicit check where we set it. 

Yeah, unfortunately I verified this morning that there are platforms where
MAX_SAMPLES > MAX_RASTER_SAMPLES. And regardless, there is nothing in the spec
that says we can count on MAX_SAMPLES being < MAX_RASTER_SAMPLES. So unless we
clamp MAX_SAMPLES ourselves, we will need to do the explicit check on
MAX_RASTER_SAMPLES.

[Chris Dalton, 2016-02-03 22:12:03]

Added a clamp.

[egdaniel, 2016-02-03 22:33:09]

this lgtm. Brian thoughts?

[bsalomon, 2016-02-04 21:30:02]

lgtm

https://codereview.chromium.org/1666563003/diff/40001/include/gpu/GrCaps.h
File include/gpu/GrCaps.h (right):

https://codereview.chromium.org/1666563003/diff/40001/include/gpu/GrCaps.h#ne...
include/gpu/GrCaps.h:227: int maxRasterSamples() const {
Maybe this could be simplified and just return 0 if raster ms is not supported
rather than a bool and a int getter? Or the impl for rasterMSSupport could just
be return SkToBool(this->maxRasterSamples())

Just a suggestion

[Chris Dalton, 2016-02-04 22:39:00]

Thanks for the suggestion, I think this is looking a lot better.

[bsalomon, 2016-02-05 13:12:04]

On 2016/02/04 22:39:00, Chris Dalton wrote:
> Thanks for the suggestion, I think this is looking a lot better.

lgtm

[Chris Dalton, 2016-02-05 17:05:49]

The CQ bit was checked by cdalton@nvidia.com

[Chris Dalton, 2016-02-05 17:05:49]

The patchset sent to the CQ was uploaded after l-g-t-m from egdaniel@google.com
Link to the patchset: https://codereview.chromium.org/1666563003/#ps60001
(title: "remove rasterMultisampleSupport")

[commit-bot: I haz the power, 2016-02-05 17:05:58]

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

[commit-bot: I haz the power, 2016-02-05 17:35:22]

Description was changed from

==========
Add support for GL_EXT_raster_multisample

Updates the GrGLGpu to call glRasterSamplesEXT when multisample is
enabled for a mixed sampled render target, but the stencil test is not
active.

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Add support for GL_EXT_raster_multisample

Updates the GrGLGpu to call glRasterSamplesEXT when multisample is
enabled for a mixed sampled render target, but the stencil test is not
active.

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/af8bc7d7a4fb1fbbc15bb0881a360ff0d7551ea2
==========

[commit-bot: I haz the power, 2016-02-05 17:35:23]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/af8bc7d7a4fb1fbbc15bb0881a360ff0d7551ea2