Florian Bösch Reporter
	Description • 10 years ago

The WebGL extension EXT_blend_minmax is now in draft and may be implemented: http://www.khronos.org/registry/webgl/extensions/EXT_blend_minmax/

	Florian Bösch Reporter
	Comment 1 • 10 years ago

Chrome ticket: https://code.google.com/p/chromium/issues/detail?id=344451

	Florian Bösch Reporter
	Comment 2 • 10 years ago

Webkit ticket: https://bugs.webkit.org/show_bug.cgi?id=128974

	Guillaume Abadie Assignee
	Comment 3 • 10 years ago

Since I've implemented it in WebGL 2, taking it!

	Guillaume Abadie Assignee
	Comment 4 • 10 years ago

This patch implements the WebGL extension EXT_blend_minmax passing WebGL conformance 1.0.3 tests.

Push to try:
https://tbpl.mozilla.org/?tree=Try&rev=2beffa2cceb0

	Guillaume Abadie Assignee
	Comment 5 • 10 years ago

Forgotten to add content/canvas/src/WebGLExtensionBlendMinMax.cpp... Might be useful in the patch! ^^

Push to try:
https://tbpl.mozilla.org/?tree=Try&rev=c7b755d5e201

	Kelsey Gilbert [:jgilbert]
	Comment 6 • 10 years ago

Comment on attachment 8436451 [details] [diff] [review]
ext_blend_minmax.patch - revision 2

Review of attachment 8436451 [details] [diff] [review]:
-----------------------------------------------------------------

BZ: We're adding more WG-approved IDL.

	Boris Zbarsky [:bzbarsky]
	Comment 7 • 10 years ago

Comment on attachment 8436451 [details] [diff] [review]
ext_blend_minmax.patch - revision 2

r=me

	Kelsey Gilbert [:jgilbert]
	Comment 8 • 10 years ago

Comment on attachment 8436451 [details] [diff] [review]
ext_blend_minmax.patch - revision 2

Review of attachment 8436451 [details] [diff] [review]:
-----------------------------------------------------------------

BZ: We're adding more WG-approved IDL.

::: content/canvas/src/WebGLContextExtensions.cpp
@@ +159,5 @@
>              return WebGLExtensionDrawBuffers::IsSupported(this);
>          case WebGLExtensionID::EXT_frag_depth:
>              return WebGLExtensionFragDepth::IsSupported(this);
> +        case WebGLExtensionID::EXT_blend_minmax:
> +            return WebGLExtensionBlendMinMax::IsSupported(this);

Actually, I need to you put this in the Draft Extension list.

	Guillaume Abadie Assignee
	Comment 9 • 10 years ago

Your are very lucky that inbound was closed last night when I tried to push! =)

	Kelsey Gilbert [:jgilbert]
	Comment 10 • 10 years ago

Comment on attachment 8438061 [details] [diff] [review]
ext_blend_minmax.patch - revision 3

Review of attachment 8438061 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

	Guillaume Abadie Assignee
	Comment 11 • 10 years ago

(In reply to Jeff Gilbert [:jgilbert] from comment #10)
> Comment on attachment 8438061 [details] [diff] [review]
> ext_blend_minmax.patch - revision 3
> 
> Review of attachment 8438061 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks!

Pleasure is all mine. But I couldn't land it... I had this message:

remote: ************************** ERROR ****************************
remote: 
remote: WebIDL file dom/webidl/WebGLRenderingContext.webidl altered in changeset 67d859377462 without DOM peer review
remote: 
remote: 
remote: 
remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
remote: 
remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote: 
remote: *************************************************************

But my commit message is:
Bug 973815 - Implements WebGL extension EXT_blend_minmax - r=jgilbert, bzbarsky 

What did I do wrong?

	Boris Zbarsky [:bzbarsky]
	Comment 12 • 10 years ago

The space after the ','?  ehsan, what does the hook actually match?

	(no longer active)
	Comment 13 • 10 years ago

(In reply to Boris Zbarsky [:bz] from comment #12)
> The space after the ','?  ehsan, what does the hook actually match?

It looks for all r=... tokens separated by whitespace and then gets the part after the equal sign and breaks it on the comma character and looks at each token.  For example here the hook sees "r=jgilbert," -> "jgilbert," -> ["jgilbert"], and the "bzbarsky" part is completely invisible to it.

Can you please fix your commit message to drop that whitespace?  I see no sane way of making the hook accept this commit message without adding footguns to it.  Sorry!

	Guillaume Abadie Assignee
	Comment 14 • 10 years ago

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #13)
> (In reply to Boris Zbarsky [:bz] from comment #12)
> > The space after the ','?  ehsan, what does the hook actually match?
> 
> It looks for all r=... tokens separated by whitespace and then gets the part
> after the equal sign and breaks it on the comma character and looks at each
> token.  For example here the hook sees "r=jgilbert," -> "jgilbert," ->
> ["jgilbert"], and the "bzbarsky" part is completely invisible to it.
> 
> Can you please fix your commit message to drop that whitespace?  I see no
> sane way of making the hook accept this commit message without adding
> footguns to it.  Sorry!

Thanks Ehsan! =)

	Guillaume Abadie Assignee
	Comment 15 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca5b7ed0dd5a

	(no longer active)
	Comment 16 • 10 years ago

Sorry, backed this out because of build bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f903c0f48969
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ca5b7ed0dd5a

	Guillaume Abadie Assignee
	Comment 17 • 10 years ago

Relanded:

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5a6e235e674

I suspect we must have a trailing "using namespace mozilla:gl;" somewhere in a header file when not building to B2G, but I couldn't find it...

	Boris Zbarsky [:bzbarsky]
	Comment 18 • 10 years ago

It's probably in a .cpp file and propagating across to other files due to unified builds.

	Phil Ringnalda (:philor)
	Comment 19 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/d5a6e235e674

	Florian Scholz (Open Web Docs)
	Comment 20 • 8 years ago

https://developer.mozilla.org/en-US/Firefox/Releases/33#WebGL

https://developer.mozilla.org/en-US/docs/Web/API/EXT_blend_minmax
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/blendEquation
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/blendEquationSeparate