Bug 85317 – Don't allocate stencil buffer if stencil flag is false in context creation attributes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Kenneth Russell]

Per discussion on the public_webgl list, it's been agreed that if the stencil flag is set to false in the context creation attributes, the WebGL implementation must not allocate a stencil buffer. This behavior is needed for cross-browser compatibility; otherwise, applications might expect a stencil buffer to be allocated even if they didn't explicitly request one.

Chromium used to implement this behavior, but it was lost during the transition to DrawingBuffer. For best portability, the logic should be re-introduced in WebGLRenderingContext.

[Kenneth Russell]

Firefox bugs that were filed because of Chromium's behavior:
  https://bugzilla.mozilla.org/show_bug.cgi?id=648883
  https://bugzilla.mozilla.org/show_bug.cgi?id=745880

[Kenneth Russell]

Created attachment 139703 [details]
Patch

[Jeff Timanus]

Comment on attachment 139703 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=139703&action=review

LGTM.

I wonder if this CL is a sign of a trend where we will have to push similar validation up to the webgl rendering context . . .

> LayoutTests/fast/canvas/webgl/context-attributes-alpha-depth-stencil-antialias.html:18
> +precision mediump float;

If I understand correctly, webgl shaders always should support the precision specifier.  Is there a reason why this was previously highp?  Making the change to mediump just for the sake of cleaning up?

[Zhenyao Mo]

(In reply to comment #3)
> (From update of attachment 139703 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=139703&action=review
> 
> LGTM.
> 
> I wonder if this CL is a sign of a trend where we will have to push similar validation up to the webgl rendering context . . .
> 
> > LayoutTests/fast/canvas/webgl/context-attributes-alpha-depth-stencil-antialias.html:18
> > +precision mediump float;
> 
> If I understand correctly, webgl shaders always should support the precision specifier.  Is there a reason why this was previously highp?  Making the change to mediump just for the sake of cleaning up?

LGTM.

Originally we used highp in most of conformance tests, and later we cleaned it up because highp support in fragment shader is optional.

I think this test was checked into webkit before the cleanup, and now it's only a sync to the khronos which has the cleanup.

[Dimitri Glazkov (Google)]

Comment on attachment 139703 [details]
Patch

rs=me. If zmo says LGTM, I tend to agree.

[Kenneth Russell]

(In reply to comment #3)
> I wonder if this CL is a sign of a trend where we will have to push similar validation up to the webgl rendering context . . .

Yes, I think it is; see dependent bug 85338.

[WebKit Review Bot]

Comment on attachment 139703 [details]
Patch

Clearing flags on attachment: 139703

Committed r115870: <http://trac.webkit.org/changeset/115870>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.