Add tokens and entry points for KHR_blend_equation_advanced

Add tokens and entry points for KHR_blend_equation_advanced

Also adds glBlendEquation, which the extension interacts with, to the
core of GrGLInterface. Validation of this function is temporarily
disabled until Chrome hooks it up.


BUG=skia:

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

[Mark Kilgard, 2015-04-02 22:41:42]

markkilgard@gmail.com changed reviewers:
+ markkilgard@gmail.com

[Mark Kilgard, 2015-04-02 22:41:43]

LGTM

my only comment is probably not worth the EXT_blend_subtract because it would
have a glBlendEquationEXT suffix.  All recently OpenGL has glBlendEquation.

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLAssembleInte...
File src/gpu/gl/GrGLAssembleInterface.cpp (right):

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLAssembleInte...
src/gpu/gl/GrGLAssembleInterface.cpp:87: GET_PROC(BlendEquation);
technically, EXT_blend_subtract (which requires EXT_blend_minmax) would get a
glBlendEquationEXT routine

I think it is safe to just assume 1.4 or ARB_imaging

anything supporting *just* EXT_blend_equation_subtract would be ancient

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
File src/gpu/gl/GrGLNoOpInterface.cpp (right):

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
src/gpu/gl/GrGLNoOpInterface.cpp:31: "GL_EXT_blend_subtract",
If you remove EXT_blend_subtract, remove this too, right?

[Chris Dalton, 2015-04-17 07:53:42]

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLAssembleInte...
File src/gpu/gl/GrGLAssembleInterface.cpp (right):

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLAssembleInte...
src/gpu/gl/GrGLAssembleInterface.cpp:87: GET_PROC(BlendEquation);
On 2015/04/02 22:41:43, Mark Kilgard wrote:
> technically, EXT_blend_subtract (which requires EXT_blend_minmax) would get a
> glBlendEquationEXT routine
> 
> I think it is safe to just assume 1.4 or ARB_imaging
> 
> anything supporting *just* EXT_blend_equation_subtract would be ancient

Agreed. However, I did it this way to mimic how glBlendColor is loaded above.
That one also had the bug of not loading glBlendColorEXT with the EXT...

I fixed them both in patch 2 to load the EXT, but if the general consensus is to
just ignore the EXT versions completely I don't mind going that route either.

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
File src/gpu/gl/GrGLNoOpInterface.cpp (right):

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
src/gpu/gl/GrGLNoOpInterface.cpp:31: "GL_EXT_blend_subtract",
On 2015/04/02 22:41:43, Mark Kilgard wrote:
> If you remove EXT_blend_subtract, remove this too, right?

I don't mind taking it out, but I think we should stay consistent and also
remove GL_EXT_blend_color if we omit this one.

[bsalomon, 2015-04-21 15:21:50]

bsalomon@google.com changed reviewers:
+ bsalomon@google.com

[bsalomon, 2015-04-21 15:21:51]

lgtm

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
File src/gpu/gl/GrGLNoOpInterface.cpp (right):

https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
src/gpu/gl/GrGLNoOpInterface.cpp:31: "GL_EXT_blend_subtract",
On 2015/04/17 07:53:42, Chris Dalton wrote:
> On 2015/04/02 22:41:43, Mark Kilgard wrote:
> > If you remove EXT_blend_subtract, remove this too, right?
> 
> I don't mind taking it out, but I think we should stay consistent and also
> remove GL_EXT_blend_color if we omit this one.

It seems ok to remove both.

[Chris Dalton, 2015-04-21 19:21:15]

On 2015/04/21 15:21:51, bsalomon wrote:
> lgtm
> 
>
https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
> File src/gpu/gl/GrGLNoOpInterface.cpp (right):
> 
>
https://codereview.chromium.org/1039693004/diff/1/src/gpu/gl/GrGLNoOpInterfac...
> src/gpu/gl/GrGLNoOpInterface.cpp:31: "GL_EXT_blend_subtract",
> On 2015/04/17 07:53:42, Chris Dalton wrote:
> > On 2015/04/02 22:41:43, Mark Kilgard wrote:
> > > If you remove EXT_blend_subtract, remove this too, right?
> > 
> > I don't mind taking it out, but I think we should stay consistent and also
> > remove GL_EXT_blend_color if we omit this one.
> 
> It seems ok to remove both.

Agreed (I just noticed the no-op context version appears to be 4.0 anyway :)

[Chris Dalton, 2015-04-21 19:28:45]

The CQ bit was checked by cdalton@nvidia.com

[Chris Dalton, 2015-04-21 19:28:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
markkilgard@gmail.com, bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1039693004/#ps40001
(title: "remove EXT_blend_color/subtract from no-op context extensions")

[commit-bot: I haz the power, 2015-04-21 19:28:54]

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

[commit-bot: I haz the power, 2015-04-21 19:34:12]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/8e578859f80b46a63144add215955221017d3609

[tomhudson, 2015-04-21 21:16:29]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1101593002/ by tomhudson@google.com.

The reason for reverting is: We're getting repeated crashes since this CL landed
when we try to evaluate a roll into Chrome on our bots.
(https://uberchromegw.corp.google.com/i/client.skia/builders/Linux%20Tests/bui...)

Our initial reflex is to put a #if 0 around this code (just like the immediately
following lines 334-338 in GrGLInterface.cpp), but the error isn't what the GPU
team expected to see if that was all we needed, so we're reverting now and
asking you to resubmit tomorrow after running more thorough tests..

[bsalomon, 2015-04-22 13:31:03]

I think the problem is that Chrome is exposing the extension, we're not grabbing
the function pointers when setting up GrGLInterface. That causes validate() to
fail which in turn causes GrContext creation to fail. Chrome is then dying on a
NULL GrContext when attempting to apply image filters.

The way to land this is to land it with the validation check #if 0'ed out and
make the caps always return false. Then submit a Chrome change which hooks up
the functions here:

https://code.google.com/p/chromium/codesearch#chromium/src/gpu/skia_bindings/...

and here:

https://code.google.com/p/chromium/codesearch#chromium/src/ui/gl/gl_bindings_...

Finally, another Skia change that enables the validation and caps.

[Chris Dalton, 2015-04-22 17:02:37]

On 2015/04/22 13:31:03, bsalomon wrote:
> I think the problem is that Chrome is exposing the extension, we're not
grabbing
> the function pointers when setting up GrGLInterface. That causes validate() to
> fail which in turn causes GrContext creation to fail. Chrome is then dying on
a
> NULL GrContext when attempting to apply image filters.
> 
> The way to land this is to land it with the validation check #if 0'ed out and
> make the caps always return false. Then submit a Chrome change which hooks up
> the functions here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/skia_bindings/...
> 
> and here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gl/gl_bindings_...
> 
> Finally, another Skia change that enables the validation and caps.

Brian, thanks for explaining the process. I've updated the issue to disable
validation for BlendEquation. Can we re-commit from here or do I need to make a
new issue?

[bsalomon, 2015-04-22 17:19:29]

On 2015/04/22 17:02:37, Chris Dalton wrote:
> On 2015/04/22 13:31:03, bsalomon wrote:
> > I think the problem is that Chrome is exposing the extension, we're not
> grabbing
> > the function pointers when setting up GrGLInterface. That causes validate()
to
> > fail which in turn causes GrContext creation to fail. Chrome is then dying
on
> a
> > NULL GrContext when attempting to apply image filters.
> > 
> > The way to land this is to land it with the validation check #if 0'ed out
and
> > make the caps always return false. Then submit a Chrome change which hooks
up
> > the functions here:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/skia_bindings/...
> > 
> > and here:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gl/gl_bindings_...
> > 
> > Finally, another Skia change that enables the validation and caps.
> 
> Brian, thanks for explaining the process. I've updated the issue to disable
> validation for BlendEquation. Can we re-commit from here or do I need to make
a
> new issue?

I think you need to also disable calling the function or we'll call a NULL func
ptr. I'd do that by neutering the caps. Using this issue is totally fine, you
just need to reopen it under "Edit issue".

[Chris Dalton, 2015-04-22 17:22:59]

On 2015/04/22 17:02:37, Chris Dalton wrote:
> On 2015/04/22 13:31:03, bsalomon wrote:
> > I think the problem is that Chrome is exposing the extension, we're not
> grabbing
> > the function pointers when setting up GrGLInterface. That causes validate()
to
> > fail which in turn causes GrContext creation to fail. Chrome is then dying
on
> a
> > NULL GrContext when attempting to apply image filters.
> > 
> > The way to land this is to land it with the validation check #if 0'ed out
and
> > make the caps always return false. Then submit a Chrome change which hooks
up
> > the functions here:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/skia_bindings/...
> > 
> > and here:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gl/gl_bindings_...
> > 
> > Finally, another Skia change that enables the validation and caps.
> 
> Brian, thanks for explaining the process. I've updated the issue to disable
> validation for BlendEquation. Can we re-commit from here or do I need to make
a
> new issue?

Can you explain what you mean by making the caps return false? glBlendEquation
is core functionality (like glBlendColor), so there aren't any caps for it; it's
expected to always be there. Nobody is calling it yet, though.

[bsalomon, 2015-04-22 17:28:06]

On 2015/04/22 at 17:22:59, cdalton wrote:
> On 2015/04/22 17:02:37, Chris Dalton wrote:
> > On 2015/04/22 13:31:03, bsalomon wrote:
> > > I think the problem is that Chrome is exposing the extension, we're not
> > grabbing
> > > the function pointers when setting up GrGLInterface. That causes
validate() to
> > > fail which in turn causes GrContext creation to fail. Chrome is then dying
on
> > a
> > > NULL GrContext when attempting to apply image filters.
> > > 
> > > The way to land this is to land it with the validation check #if 0'ed out
and
> > > make the caps always return false. Then submit a Chrome change which hooks
up
> > > the functions here:
> > > 
> > >
> >
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/skia_bindings/...
> > > 
> > > and here:
> > > 
> > >
> >
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gl/gl_bindings_...
> > > 
> > > Finally, another Skia change that enables the validation and caps.
> > 
> > Brian, thanks for explaining the process. I've updated the issue to disable
> > validation for BlendEquation. Can we re-commit from here or do I need to
make a
> > new issue?
> 
> Can you explain what you mean by making the caps return false? glBlendEquation
is core functionality (like glBlendColor), so there aren't any caps for it; it's
expected to always be there. Nobody is calling it yet, though.

Nevermind that comment. This looks good to go.

[Chris Dalton, 2015-04-22 17:32:41]

The CQ bit was checked by cdalton@nvidia.com

[Chris Dalton, 2015-04-22 17:32:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
markkilgard@gmail.com, bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1039693004/#ps60001
(title: "Disable fBlendEquation validation")

[commit-bot: I haz the power, 2015-04-22 17:33:35]

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

[commit-bot: I haz the power, 2015-04-22 17:39:07]

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

[tomhudson, 2015-04-23 20:19:32]

Just for future reference, this changed patch did NOT actually disable
validation.
As I said in my notes with the original revert, lines 330-334 needed to be
addressed as well.
We're now responsible for a bad Chrome build and potentially many wasted
engineer-hours (https://code.google.com/p/chromium/issues/detail?id=480597).

Brian's got a fix in at https://codereview.chromium.org/1101143002.

[bsalomon, 2015-04-23 20:21:29]

On 2015/04/23 20:19:32, tomhudson wrote:
> Just for future reference, this changed patch did NOT actually disable
> validation.
> As I said in my notes with the original revert, lines 330-334 needed to be
> addressed as well.
> We're now responsible for a bad Chrome build and potentially many wasted
> engineer-hours (https://code.google.com/p/chromium/issues/detail?id=480597).
> 
> Brian's got a fix in at https://codereview.chromium.org/1101143002.

My fault for not understanding that it wasn't just BlendEquation that was
causing the issue. I assumed that Chromium did not expose advanced blending via
the command buffer.