Bug 85317

Summary: Don't allocate stencil buffer if stencil flag is false in context creation attributes
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Kenneth Russell <kbr>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dino, fishd, gman, twiz, webkit.review.bot, zmo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 85338    
Attachments:
Description Flags
Patch none

Description Kenneth Russell 2012-05-01 15:26:04 PDT
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.
Comment 1 Kenneth Russell 2012-05-01 15:27:03 PDT
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
Comment 2 Kenneth Russell 2012-05-01 16:22:48 PDT
Created attachment 139703 [details]
Patch
Comment 3 Jeff Timanus 2012-05-01 20:08:24 PDT
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?
Comment 4 Zhenyao Mo 2012-05-01 20:30:25 PDT
(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.
Comment 5 Dimitri Glazkov (Google) 2012-05-02 12:40:01 PDT
Comment on attachment 139703 [details]
Patch

rs=me. If zmo says LGTM, I tend to agree.
Comment 6 Kenneth Russell 2012-05-02 12:41:12 PDT
(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.
Comment 7 WebKit Review Bot 2012-05-02 12:57:02 PDT
Comment on attachment 139703 [details]
Patch

Clearing flags on attachment: 139703

Committed r115870: <http://trac.webkit.org/changeset/115870>
Comment 8 WebKit Review Bot 2012-05-02 12:57:16 PDT
All reviewed patches have been landed.  Closing bug.