Remove hasMixedSamples() from GrPipelineBuilder

Remove hasMixedSamples() from GrPipelineBuilder

This info is unknown until after any clip has been applied to the
stencil settings, so it's misleading to include in the builder.

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

Committed: https://skia.googlesource.com/skia/+/3ccf2e75ed92adb3c2c289e0cb95901d162df394

[Chris Dalton, 2016-05-05 19:44:03]

Description was changed from

==========
Remove hasMixedSamples() from GrPipelineBuilder

This info is unknown until after any clip has been applied to the
stencil settings, so it's misleading to include in the builder.

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

to

==========
Remove hasMixedSamples() from GrPipelineBuilder

This info is unknown until after any clip has been applied to the
stencil settings, so it's misleading to include in the builder.

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

[Chris Dalton, 2016-05-05 19:45:43]

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

[Chris Dalton, 2016-05-05 19:45:44]

This is part of the clip refactoring in preparation for window rectangles.

[egdaniel, 2016-05-05 20:04:56]

https://codereview.chromium.org/1952323002/diff/1/src/gpu/effects/GrPorterDuf...
File src/gpu/effects/GrPorterDuffXferProcessor.cpp (right):

https://codereview.chromium.org/1952323002/diff/1/src/gpu/effects/GrPorterDuf...
src/gpu/effects/GrPorterDuffXferProcessor.cpp:923:
SkASSERT(!caps.usesMixedSamples());
can you add the same dual source blending comment here as above. Otherwise I'll
forget and some time down the road be staring at this way to long trying to
figure out why we don't have mixedSamples.

On a related note, when you set usesMixedSamples on caps are you explictly
checking or asserting dual source blending is available? Or are you just
assuming that is the case knowing how the extension works? If it is the latter,
I would request adding an assert there since we are now relying on that property
here.

[Chris Dalton, 2016-05-06 16:06:50]

https://codereview.chromium.org/1952323002/diff/1/src/gpu/effects/GrPorterDuf...
File src/gpu/effects/GrPorterDuffXferProcessor.cpp (right):

https://codereview.chromium.org/1952323002/diff/1/src/gpu/effects/GrPorterDuf...
src/gpu/effects/GrPorterDuffXferProcessor.cpp:923:
SkASSERT(!caps.usesMixedSamples());
On 2016/05/05 20:04:56, egdaniel wrote:
> can you add the same dual source blending comment here as above. Otherwise
I'll
> forget and some time down the road be staring at this way to long trying to
> figure out why we don't have mixedSamples.

Oops, I didn't notice I missed this one.

> On a related note, when you set usesMixedSamples on caps are you explictly
> checking or asserting dual source blending is available? Or are you just
> assuming that is the case knowing how the extension works? If it is the
latter,
> I would request adding an assert there since we are now relying on that
property> here.


Yeah, we check when building the caps. It's possible to have one extension
without the other, but if dual source blending is not supported then we ignore
NV_framebuffer_mixed_samples.

The reason is because HW coverage modulation happens in ROP (after fragment
shading), so it would be impossible to make certain xfer modes work without dual
source blending when using mixed samples.

[egdaniel, 2016-05-06 16:10:04]

lgtm

https://codereview.chromium.org/1952323002/diff/1/src/gpu/effects/GrPorterDuf...
File src/gpu/effects/GrPorterDuffXferProcessor.cpp (right):

https://codereview.chromium.org/1952323002/diff/1/src/gpu/effects/GrPorterDuf...
src/gpu/effects/GrPorterDuffXferProcessor.cpp:923:
SkASSERT(!caps.usesMixedSamples());
On 2016/05/06 16:06:50, Chris Dalton wrote:
> On 2016/05/05 20:04:56, egdaniel wrote:
> > can you add the same dual source blending comment here as above. Otherwise
> I'll
> > forget and some time down the road be staring at this way to long trying to
> > figure out why we don't have mixedSamples.
> 
> Oops, I didn't notice I missed this one.
> 
> > On a related note, when you set usesMixedSamples on caps are you explictly
> > checking or asserting dual source blending is available? Or are you just
> > assuming that is the case knowing how the extension works? If it is the
> latter,
> > I would request adding an assert there since we are now relying on that
> property> here.
> 
> 
> Yeah, we check when building the caps. It's possible to have one extension
> without the other, but if dual source blending is not supported then we ignore
> NV_framebuffer_mixed_samples.
> 
> The reason is because HW coverage modulation happens in ROP (after fragment
> shading), so it would be impossible to make certain xfer modes work without
dual
> source blending when using mixed samples.

Sounds good. Just wanted to make sure we had some check.

[Chris Dalton, 2016-05-06 16:18:40]

The CQ bit was checked by cdalton@nvidia.com

[commit-bot: I haz the power, 2016-05-06 16:18:42]

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

[commit-bot: I haz the power, 2016-05-06 16:20:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-06 16:20:14]

Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[bsalomon, 2016-05-06 16:26:33]

On 2016/05/06 16:20:14, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
>
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

lgtm

[Chris Dalton, 2016-05-06 16:27:11]

The CQ bit was checked by cdalton@nvidia.com

[commit-bot: I haz the power, 2016-05-06 16:27:24]

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

[commit-bot: I haz the power, 2016-05-06 16:41:21]

Description was changed from

==========
Remove hasMixedSamples() from GrPipelineBuilder

This info is unknown until after any clip has been applied to the
stencil settings, so it's misleading to include in the builder.

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

to

==========
Remove hasMixedSamples() from GrPipelineBuilder

This info is unknown until after any clip has been applied to the
stencil settings, so it's misleading to include in the builder.

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/+/3ccf2e75ed92adb3c2c289e0cb95901d162df394
==========

[commit-bot: I haz the power, 2016-05-06 16:41:22]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/3ccf2e75ed92adb3c2c289e0cb95901d162df394