Bug 110045 - [WebGL] Fix oes-element-index-uint.html test.
Summary: [WebGL] Fix oes-element-index-uint.html test.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Kalyan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-16 20:25 PST by Kalyan
Modified: 2013-02-20 10:00 PST (History)
5 users (show)

See Also:


Attachments
patch (4.77 KB, patch)
2013-02-16 21:09 PST, Kalyan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kalyan 2013-02-16 20:25:08 PST
Test seems to use some old functions and misses shaders for runResizedBufferTests.
Comment 1 Kalyan 2013-02-16 21:09:40 PST
Created attachment 188752 [details]
patch
Comment 2 Kenneth Russell 2013-02-19 12:30:40 PST
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?
Comment 3 WebKit Review Bot 2013-02-19 12:45:30 PST
Comment on attachment 188752 [details]
patch

Clearing flags on attachment: 188752

Committed r143373: <http://trac.webkit.org/changeset/143373>
Comment 4 WebKit Review Bot 2013-02-19 12:45:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Kalyan 2013-02-19 21:41:30 PST
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??
Comment 6 Kalyan 2013-02-20 04:51:24 PST
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
Comment 7 Kenneth Russell 2013-02-20 10:00:35 PST
(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.