Bug 41095

Summary:

Bring set/get state functions to GLES2 conformance

Product:

WebKit

Reporter:

Zhenyao Mo <zmo>

Component:

WebGL

Assignee:

Zhenyao Mo <zmo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cmarrin, commit-queue, dglazkov, kbr, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

Bug Blocks:

29230

Attachments:

Description	Flags
patch	none
revised patch: responding to kbr's review	none
revised patch: minor fix	none

Zhenyao Mo

Reported 2010-06-23 12:52:58 PDT

functions as list in WebGL spec 5.13.3

Attachments
patch (117.39 KB, patch) 2010-06-23 12:56 PDT, Zhenyao Mo	no flags	Details Formatted Diff Diff
revised patch: responding to kbr's review (117.24 KB, patch) 2010-06-24 10:13 PDT, Zhenyao Mo	no flags	Details Formatted Diff Diff
revised patch: minor fix (116.91 KB, patch) 2010-06-24 13:48 PDT, Zhenyao Mo	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Zhenyao Mo

Comment 1 2010-06-23 12:56:18 PDT

Created attachment 59555 [details] patch

Kenneth Russell

Comment 2 2010-06-23 17:12:44 PDT

Comment on attachment 59555 [details] patch WebCore/html/canvas/WebGLRenderingContext.cpp:311 + if (!validateBlendMode(mode)) While this name matches the argument name, I think it would be more clear if it were named "validateBlendEquation". WebCore/html/canvas/WebGLRenderingContext.cpp:1664 + } What about validating "mode"? WebCore/html/canvas/WebGLRenderingContext.cpp:3516 + bool WebGLRenderingContext::validateBlendMode(unsigned long mode) See above regarding naming. LayoutTests/fast/canvas/webgl/gl-enum-tests.html:17 + description("This test ensures WebGL various functions fail when passed non OpenGL ES 2.0 enums."); "ensures various WebGL functions" LayoutTests/fast/canvas/webgl/resources/desktop-gl-constants.js:2642 + Added: svn:executable This shouldn't have the executable bit set.

Zhenyao Mo

Comment 3 2010-06-23 17:40:14 PDT

(In reply to comment #2) > (From update of attachment 59555 [details]) > WebCore/html/canvas/WebGLRenderingContext.cpp:311 > + if (!validateBlendMode(mode)) > While this name matches the argument name, I think it would be more clear if it were named "validateBlendEquation". > > > WebCore/html/canvas/WebGLRenderingContext.cpp:1664 > + } > What about validating "mode"? The "mode" enum list for GENERATE_MIPMAP_HINT is the same for GL and GLES, so we don't need to do it twice. Will fix the rest. > > > WebCore/html/canvas/WebGLRenderingContext.cpp:3516 > + bool WebGLRenderingContext::validateBlendMode(unsigned long mode) > See above regarding naming. > > > LayoutTests/fast/canvas/webgl/gl-enum-tests.html:17 > + description("This test ensures WebGL various functions fail when passed non OpenGL ES 2.0 enums."); > "ensures various WebGL functions" > > > LayoutTests/fast/canvas/webgl/resources/desktop-gl-constants.js:2642 > + Added: svn:executable > This shouldn't have the executable bit set.

Zhenyao Mo

Comment 4 2010-06-24 10:13:04 PDT

Created attachment 59667 [details] revised patch: responding to kbr's review

Kenneth Russell

Comment 5 2010-06-24 11:56:16 PDT

Comment on attachment 59667 [details] revised patch: responding to kbr's review Couple more comments; sorry for not realizing these before. > Property changes on: LayoutTests/fast/canvas/webgl/gl-enable-enum-test.html > ___________________________________________________________________ > Added: svn:executable > + * > > Property changes on: LayoutTests/fast/canvas/webgl/gl-enum-tests.html > ___________________________________________________________________ > Added: svn:executable > + * These files shouldn't have the executable bit set either. WebCore/html/canvas/WebGLRenderingContext.h:454 + // Helper function to validate GL a capability. validate GL a capability -> validate a GL capability

Zhenyao Mo

Comment 6 2010-06-24 13:48:42 PDT

Created attachment 59694 [details] revised patch: minor fix

Kenneth Russell

Comment 7 2010-06-24 14:03:36 PDT

Looks good.

Dimitri Glazkov (Google)

Comment 8 2010-06-24 15:12:25 PDT

Comment on attachment 59694 [details] revised patch: minor fix holy crap that's a long test :)

WebKit Commit Bot

Comment 9 2010-06-25 21:23:57 PDT

Comment on attachment 59694 [details] revised patch: minor fix Clearing flags on attachment: 59694 Committed r61938: <http://trac.webkit.org/changeset/61938>

WebKit Commit Bot

Comment 10 2010-06-25 21:24:02 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.