Accelerate the lighten blendmode if GL_EXT_blend_minmax is enabled.

Accelerate the 'lighten' blendmode.

If GL_EXT_blend_minmax is available then make use of that
to accelerate blending for the 'lighten' blendmode.

BUG=243223
Committed: https://crrev.com/fad658b4d6af2be70c1d39b98ffa92c578f31006
Cr-Commit-Position: refs/heads/master@{#299080}

[Erik Dahlström (inactive), 2014-09-30 15:33:14]

ed@opera.com changed reviewers:
+ enne@chromium.org

[rosca, 2014-09-30 17:46:29]

rosca@adobe.com changed reviewers:
+ rosca@adobe.com

[rosca, 2014-09-30 17:46:30]

https://codereview.chromium.org/614953002/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/1/cc/output/gl_renderer.cc#new...
cc/output/gl_renderer.cc:725: GLC(gl_, gl_->BlendEquation(GL_FUNC_ADD));
This reinitialization will be called for each render pass quad when blending can
be done with coefficients, including the SrcOver case (no mix-blend-mode
specified in CSS). I think we should do the reinitialization symmetrically, or
at least not call RestoreBlendFuncToDefault when the operator is SrcOver.

[enne (OOO), 2014-10-01 18:43:53]

https://codereview.chromium.org/614953002/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/1/cc/output/gl_renderer.cc#new...
cc/output/gl_renderer.cc:725: GLC(gl_, gl_->BlendEquation(GL_FUNC_ADD));
On 2014/09/30 at 17:46:30, Rosca wrote:
> This reinitialization will be called for each render pass quad when blending
can be done with coefficients, including the SrcOver case (no mix-blend-mode
specified in CSS). I think we should do the reinitialization symmetrically, or
at least not call RestoreBlendFuncToDefault when the operator is SrcOver.

Ok, that seems reasonable.  Maybe add a comment to ApplyBlendModeUsingBlendFunc
that any state changed there must be reset back to defaults in
RestoreBlendFuncToDefault?  (And maybe on second glance these functions should
all just take the blend mode, rather than the full quad, since that's all
they're really using?)

[Erik Dahlström (inactive), 2014-10-03 15:12:38]

On 2014/10/01 18:43:53, enne wrote:
> https://codereview.chromium.org/614953002/diff/1/cc/output/gl_renderer.cc
> File cc/output/gl_renderer.cc (right):
> 
>
https://codereview.chromium.org/614953002/diff/1/cc/output/gl_renderer.cc#new...
> cc/output/gl_renderer.cc:725: GLC(gl_, gl_->BlendEquation(GL_FUNC_ADD));
> On 2014/09/30 at 17:46:30, Rosca wrote:
> > This reinitialization will be called for each render pass quad when blending
> can be done with coefficients, including the SrcOver case (no mix-blend-mode
> specified in CSS). I think we should do the reinitialization symmetrically, or
> at least not call RestoreBlendFuncToDefault when the operator is SrcOver.
> 
> Ok, that seems reasonable.  Maybe add a comment to
ApplyBlendModeUsingBlendFunc
> that any state changed there must be reset back to defaults in
> RestoreBlendFuncToDefault?  (And maybe on second glance these functions should
> all just take the blend mode, rather than the full quad, since that's all
> they're really using?)

Done.

[rosca, 2014-10-03 15:33:06]

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1008: CanApplyBlendModeUsingBlendFunc(blend_mode));
This expression is true for srcOver all the time. Could you make it something
like
(ShouldDrawWithBlending() || (!IsDefaultBlendMode() &&
CanApplyBlendModeUsingBlendFunc(blend_mode))?

[Erik Dahlström (inactive), 2014-10-06 11:04:16]

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1008: CanApplyBlendModeUsingBlendFunc(blend_mode));
On 2014/10/03 15:33:05, rosca wrote:
> This expression is true for srcOver all the time. Could you make it something
> like
> (ShouldDrawWithBlending() || (!IsDefaultBlendMode() &&
> CanApplyBlendModeUsingBlendFunc(blend_mode))?

Done.

[enne (OOO), 2014-10-07 17:51:18]

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1008: CanApplyBlendModeUsingBlendFunc(blend_mode));
On 2014/10/03 15:33:05, rosca wrote:
> This expression is true for srcOver all the time. Could you make it something
> like
> (ShouldDrawWithBlending() || (!IsDefaultBlendMode() &&
> CanApplyBlendModeUsingBlendFunc(blend_mode))?

I think the previous code was easier to read.  The default blend mode can be
applied via blend func, and so there's no reason to treat it differently.

https://codereview.chromium.org/614953002/diff/80001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/80001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1083: if (background_texture &&
!CanApplyBlendModeUsingBlendFunc(blend_mode)) {
I think this CanApplyBlendModeUsingBlendFunc check is redundant here.  Maybe
just DCHECK it?

[rosca, 2014-10-07 21:46:08]

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1008: CanApplyBlendModeUsingBlendFunc(blend_mode));
On 2014/10/07 17:51:17, enne wrote:
> I think the previous code was easier to read.  The default blend mode can be
> applied via blend func, and so there's no reason to treat it differently.
Are you referring to function names? Otherwise, the old code was enabling blend
all the time, including for opaque source over layers.
ShouldDrawWithBlending() is responsible for enabling/disabling blend when the
blend mode is srcover (the default). CanApplyBlendModeUsingBlendFunc() should be
tested only if we don't use src over.

[Erik Dahlström (inactive), 2014-10-08 15:25:20]

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1008: CanApplyBlendModeUsingBlendFunc(blend_mode));
On 2014/10/07 21:46:08, rosca wrote:
> On 2014/10/07 17:51:17, enne wrote:
> > I think the previous code was easier to read.  The default blend mode can be
> > applied via blend func, and so there's no reason to treat it differently.
> Are you referring to function names? Otherwise, the old code was enabling
blend
> all the time, including for opaque source over layers.
> ShouldDrawWithBlending() is responsible for enabling/disabling blend when the
> blend mode is srcover (the default). CanApplyBlendModeUsingBlendFunc() should
be
> tested only if we don't use src over.

As rosca says enabling blending is not necessary for the srcOver mode. I'd
prefer to keep it the way it is now.

https://codereview.chromium.org/614953002/diff/80001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/80001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1083: if (background_texture &&
!CanApplyBlendModeUsingBlendFunc(blend_mode)) {
On 2014/10/07 17:51:17, enne wrote:
> I think this CanApplyBlendModeUsingBlendFunc check is redundant here.  Maybe
> just DCHECK it?

No, I don't think it's redundant, have a look at the rebased version which
hopefully is a bit more clear.

[enne (OOO), 2014-10-08 20:47:21]

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/614953002/diff/60001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:1008: CanApplyBlendModeUsingBlendFunc(blend_mode));
On 2014/10/08 15:25:19, Erik Dahlström wrote:
> On 2014/10/07 21:46:08, rosca wrote:
> > On 2014/10/07 17:51:17, enne wrote:
> > > I think the previous code was easier to read.  The default blend mode can
be
> > > applied via blend func, and so there's no reason to treat it differently.
> > Are you referring to function names? Otherwise, the old code was enabling
> blend
> > all the time, including for opaque source over layers.
> > ShouldDrawWithBlending() is responsible for enabling/disabling blend when
the
> > blend mode is srcover (the default). CanApplyBlendModeUsingBlendFunc()
should
> be
> > tested only if we don't use src over.
> 
> As rosca says enabling blending is not necessary for the srcOver mode. I'd
> prefer to keep it the way it is now.

Oh, quite right.  Sorry, I misunderstood what was going on here.

[enne (OOO), 2014-10-08 20:48:57]

lgtm

[Erik Dahlström (inactive), 2014-10-09 07:04:03]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-10-09 07:04:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614953002/100001

[commit-bot: I haz the power, 2014-10-09 07:16:39]

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

[commit-bot: I haz the power, 2014-10-09 07:16:40]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Erik Dahlström (inactive), 2014-10-09 07:21:11]

ed@opera.com changed reviewers:
+ jbauman@chromium.org, sievers@chromium.org

[Erik Dahlström (inactive), 2014-10-09 07:23:49]

I somehow managed to forget about the parts outside of cc, my bad.

Missing LGTM from an OWNER for these files:
    content/common/gpu/gpu_messages.h
    gpu/command_buffer/common/capabilities.cc
    gpu/command_buffer/common/capabilities.h
    gpu/command_buffer/service/feature_info.cc
    gpu/command_buffer/service/feature_info.h
    gpu/command_buffer/service/gles2_cmd_decoder.cc

Ping @sievers, @jbauman. PTAL.

[Mike West, 2014-10-09 07:27:39]

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org

[Mike West, 2014-10-09 07:27:40]

content/common/gpu/gpu_messages.h LGTM.

[jbauman, 2014-10-09 22:22:53]

gpu/command_buffer/ lgtm

[piman, 2014-10-09 22:32:14]

piman@chromium.org changed reviewers:
+ piman@chromium.org

[piman, 2014-10-09 22:32:15]

https://codereview.chromium.org/614953002/diff/100001/cc/output/gl_renderer.h
File cc/output/gl_renderer.h (right):

https://codereview.chromium.org/614953002/diff/100001/cc/output/gl_renderer.h...
cc/output/gl_renderer.h:442: bool use_blend_minmax;
drive-by nit: use_blend_minmax_

[Erik Dahlström (inactive), 2014-10-10 07:57:48]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-10-10 07:58:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614953002/160001

[commit-bot: I haz the power, 2014-10-10 09:00:06]

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2014-10-10 09:01:00]

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