Bug 41095 – Bring set/get state functions to GLES2 conformance

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

[Zhenyao Mo]

functions as list in WebGL spec 5.13.3

[Zhenyao Mo]

Created attachment 59555 [details]
patch

[Kenneth Russell]

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]

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

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

[Kenneth Russell]

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]

Created attachment 59694 [details]
revised patch: minor fix

[Kenneth Russell]

Looks good.

[Dimitri Glazkov (Google)]

Comment on attachment 59694 [details]
revised patch: minor fix

holy crap that's a long test :)

[WebKit Commit Bot]

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]

All reviewed patches have been landed.  Closing bug.