Summary: | Remove LegacyDefaultOptionalArguments flag from WebGL | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Pilgrim (Google) <pilgrim> | ||||||
Component: | WebGL | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, kbr, webkit.review.bot, zmo | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Mark Pilgrim (Google)
2011-07-14 12:36:43 PDT
Created attachment 100844 [details]
Patch
Should we discuss this on the public_webgl mailing list first? As far as I know, opera already passes all the conformance tests as they are. So changing this behavior and changing the tests should be discussed and agreed upon, IMO. Comment on attachment 100844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100844&action=review Generally looks good but needs a little more work on the test changes. > LayoutTests/fast/canvas/webgl/bad-arguments-test.html:62 > + if (argument == undefined) { > + shouldThrow("context.attachShader(argument)"); > + } else { > + func("context.attachShader(argument)"); > + } It looks like this was fixed upstream in the Khronos repository, but differently. Could you please copy the associated code from https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/bad-arguments-test.html ? > LayoutTests/fast/canvas/webgl/context-lost.html:136 > + "gl.blendFunc(gl.ONE, gl.ONE)", > + "gl.blendFuncSeparate(gl.ONE, gl.ONE, gl.ONE, gl.ONE)", Thanks for fixing this one. I've copied this fix into the Khronos repository. > LayoutTests/fast/canvas/webgl/null-object-behaviour.html:-25 > -shouldGenerateGLError(context, context.INVALID_VALUE, "context.compileShader()"); > -shouldGenerateGLError(context, context.INVALID_VALUE, "context.linkProgram()"); > -shouldGenerateGLError(context, context.INVALID_VALUE, "context.attachShader()"); It looks like these have been fixed upstream, but differently. Could you please copy the associated code from https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/null-object-behaviour.html ? > LayoutTests/fast/canvas/webgl/null-object-behaviour.html:-30 > -shouldGenerateGLError(context, context.INVALID_VALUE, "context.shaderSource()"); Same problem here. (In reply to comment #2) > Should we discuss this on the public_webgl mailing list first? As far as I know, opera already passes all the conformance tests as they are. So changing this behavior and changing the tests should be discussed and agreed upon, IMO. The changes to two out of the three tests are already in the Khronos repo. The fix to the lost context test is obvious and noncontroversial, and I've just committed it to the Khronos repo. Created attachment 100858 [details]
Patch
Comment on attachment 100858 [details]
Patch
Looks great. Thanks.
Comment on attachment 100858 [details] Patch Clearing flags on attachment: 100858 Committed r91053: <http://trac.webkit.org/changeset/91053> All reviewed patches have been landed. Closing bug. |