745912 - WebGL contexts who asked for alpha:false or depth:false should really not get it

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Giving alpha or depth to a WebGL context that explicitly disabled it is illegal, can lead to wrong rendering. So when the WebGLContextOptions have alpha:false or depth:false we must really honor that at the time of the creation of the OpenGL context.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: First pass at conformance test

Add conformance test for ALPHA/DEPTH/STENCIL_BITS == 0 when the corresponding option is false.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

In fact, I think stencil is not hard-enforced like alpha is. At least that's what Chrome folk told be the other day when I pointed out that Chrome is giving a stencil buffer by default. Would be nice to find where this is said in the spec.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Ah, here it is:
http://www.khronos.org/registry/webgl/specs/latest/#2.2

"The depth, stencil and antialias attributes are requests, not requirements."

So this bug is only about alpha.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Although, it's always nice to avoid allocating a buffer that's not used (thinking about depth buffer in a 2D game...)

Comment 5

[Kelsey Gilbert [:jgilbert]]

I move that we add internal tests to assure that we get (or don't get) buffers in accordance with all context creation attributes. That is, if we request no buffer, we get none, but if we do request one, we get one.

Comment 6

[Kelsey Gilbert [:jgilbert]]

alpha:false is required by spec, but all that is tested in the conformance tests is whether the context claims to have alpha after a context was created with alpha:false.

I think the test should be expanded to test that ALPHA_BITS == 0 for alpha:false, and >0 for alpha:true.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Conformance test added, along with depth and stencil checks.
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/context/context-attributes-alpha-depth-stencil-antialias.html

We do fail it; removing webgl-test-needed.

We also fail it for depth (we give depth when depth:false is required). Renaming the bug.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

Note: what comment 7 does is exactly what comment 6 asked for.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

It's not a bug for depth, right?  These things are requests, so providing a depth buffer even when depth: false should still be fine. (As I thought alpha was too, hmm.)

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

Vlad, the spec has been updated/clarified. While a 'true' value of 'depth' is only a request, a 'false' value must be honored. See section 2.2:

The depth, stencil and antialias attributes, when set to true, are requests, not requirements. The WebGL implementation should make a best effort to honor them. When any of these attributes is set to false, however, the WebGL implementation must not provide the associated functionality.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I admit that I don't understand why -- providing these buffers shouldn't ever lead to incorrect rendering unless the code does something dumb like enables depth test and expects to not have a depth buffer.  I'd argue that that's just dumb code that's incorrect to begin with...

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

The main point why this matters actually goes the converse way: if we give alpha to buggy pages that require no alpha but use alpha nevertheless (hence are buggy), then the page will run in Firefox but not in other browsers.

This happened to us in the converse way: Chrome was incorrectly giving stencil buffers to pages that didn't request it but still relied on it, and these pages were running only in Chrome as a result.

Of course alpha is a bit different as it's true by default. But still.

Another way that this could break is if a page requested alpha=false and then did gl.clearColor(r,g,b,0) expecting an opaque background (alpha=false) and getting a transparent background instead (a=0).

Comment 14

[Kelsey Gilbert [:jgilbert]]

This was fixed long ago.