Bug 47964 - If WebGL is running on top of a strict version of OpenGL ES it should make sure attribs have buffers assigned at all times
Summary: If WebGL is running on top of a strict version of OpenGL ES it should make su...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-19 19:00 PDT by Gregg Tavares
Modified: 2010-10-27 14:03 PDT (History)
7 users (show)

See Also:


Attachments
patch (3.07 KB, patch)
2010-10-20 12:05 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gregg Tavares 2010-10-19 19:00:34 PDT
Chromium has a strict version of OpenGL ES 2.0 that does all the attrib array bounds checking needed by WebGL so it would be nice if WebGL in webkit didn't redundantly do bounds checking.

Unfortunately, when removing the current bounds checking the issue comes up that WebGL can call drawArrays or drawElements when attributes are enabled but no buffer has been assigned. This crashes OpenGL (both the Chromium version and real OpenGL)

The suggested solution is to create a dummy buffer, assign it to every attribute at initialization time,

If the user deletes a buffer or calls vertexAttribPointer with NULL then instead of putting NULL in the buffer, assign the dummy buffer to the attrib in question

This avoids having to check that buffers are assigned at draw time. It add the complication that when querying the state of an attrib you have to shadow its state since if the dummy buffer is assigned you should be telling the user that no buffer is assigned.
Comment 1 Gregg Tavares 2010-10-20 10:44:12 PDT
Actually, rethinking this, WebGL still has to verify that attribs that are enabled have buffers assigned to them.

The things that need to happen are....If WebGL us running on a strict version of OpenGL ES then

1) bounds do not need to be checked 
2) index buffers do not need to be shadowed
Comment 2 Zhenyao Mo 2010-10-20 12:05:36 PDT
Created attachment 71315 [details]
patch

With this patch in, we can turn off bounds checking in WebGLRenderingContext if using command buffer port.  This is to avoid bounds checking twice.
Comment 3 Kenneth Russell 2010-10-26 14:21:43 PDT
Comment on attachment 71315 [details]
patch

The patch looks fine, but is this situation covered by existing layout tests? If not, one is needed (upstream in Khronos as well as pulled down to WebKit).
Comment 4 Zhenyao Mo 2010-10-26 15:19:50 PDT
(In reply to comment #3)
> (From update of attachment 71315 [details])
> The patch looks fine, but is this situation covered by existing layout tests? If not, one is needed (upstream in Khronos as well as pulled down to WebKit).

Yes, it is covered.  Without this patch, if we turn off the bounds-checking in WebGLRenderingContext and turn it on in command buffer, some texts will crash.
Comment 5 Kenneth Russell 2010-10-26 15:25:08 PDT
Comment on attachment 71315 [details]
patch

OK. Looks good to me then.
Comment 6 Zhenyao Mo 2010-10-27 10:10:26 PDT
Comment on attachment 71315 [details]
patch

Clearing flags on attachment: 71315

Committed r70661: <http://trac.webkit.org/changeset/70661>
Comment 7 Zhenyao Mo 2010-10-27 10:10:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 WebKit Review Bot 2010-10-27 14:03:55 PDT
http://trac.webkit.org/changeset/70661 might have broken GTK Linux 32-bit Debug