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

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

[Gregg Tavares]

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.

[Gregg Tavares]

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

[Zhenyao Mo]

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.

[Kenneth Russell]

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).

[Zhenyao Mo]

(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.

[Kenneth Russell]

Comment on attachment 71315 [details]
patch

OK. Looks good to me then.

[Zhenyao Mo]

Comment on attachment 71315 [details]
patch

Clearing flags on attachment: 71315

Committed r70661: <http://trac.webkit.org/changeset/70661>

[Zhenyao Mo]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/70661 might have broken GTK Linux 32-bit Debug