Bug 110045 – [WebGL] Fix oes-element-index-uint.html test.

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

[Kalyan]

Test seems to use some old functions and misses shaders for runResizedBufferTests.

[Kalyan]

Created attachment 188752 [details]
patch

[Kenneth Russell]

Comment on attachment 188752 [details]
patch

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

It looks like this test is currently broken so yes, let's fix it. However, the better long-term fix is Gregg's incorporation of the WebGL conformance tests as layout tests, which has been recently completed. Would be good if you could devote effort to getting those tests turned on on various ports. r=me

> LayoutTests/fast/canvas/webgl/oes-element-index-uint.html:99
> +    var program = wtu.setupSimpleTextureProgram(gl);

It looks like the only reason for this change is that resources/webgl-test-utils is out of date. Is that correct?

> LayoutTests/fast/canvas/webgl/oes-element-index-uint.html:285
> +    gl.disableVertexAttribArray(vertexLoc);

It looks like a recent upstream fix to webgl-test-utils removes the need for this. Can you test with https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/oes-element-index-uint.html and confirm?

[WebKit Review Bot]

Comment on attachment 188752 [details]
patch

Clearing flags on attachment: 188752

Committed r143373: <http://trac.webkit.org/changeset/143373>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kalyan]

Comment on attachment 188752 [details]
patch

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

> LayoutTests/fast/canvas/webgl/oes-element-index-uint.html:100
>  

When I proposed the change, my assumption was that the test is out of date. I should have cross checked webgl-test-utils with the version in Khronous conformance tests. 
Yes, that was the only reason. Should we try to sync both versions of webgl-test-utils or any time frame when this is planned??

[Kalyan]

Comment on attachment 188752 [details]
patch

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

>> LayoutTests/fast/canvas/webgl/oes-element-index-uint.html:285
>> +    gl.disableVertexAttribArray(vertexLoc);
> 
> It looks like a recent upstream fix to webgl-test-utils removes the need for this. Can you test with https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/oes-element-index-uint.html and confirm?

Yes, it seems unnecessary with latest changes in khronous version of webgl-test-utils

[Kenneth Russell]

(In reply to comment #6)
> (From update of attachment 188752 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188752&action=review
> 
> >> LayoutTests/fast/canvas/webgl/oes-element-index-uint.html:285
> >> +    gl.disableVertexAttribArray(vertexLoc);
> > 
> > It looks like a recent upstream fix to webgl-test-utils removes the need for this. Can you test with https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/oes-element-index-uint.html and confirm?
> 
> Yes, it seems unnecessary with latest changes in khronous version of webgl-test-utils

There are no concrete plans to update the copy of webgl-test-utils under fast/canvas/webgl/ . Gregg Tavares just incorporated the entire WebGL conformance suite verbatim under LayoutTests/webgl/ and that's the version that should be focused on going forward. On the other hand, if you want to update the fast/canvas/webgl/ copy in a separate bug for the interim period, that would be fine.