Bug 40437 – getParameter(COLOR_WRITEMASK) needs to return Array

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

Per recent WebGL spec updates, getParameter(COLOR_WRITEMASK) now returns GLboolean[] with 4 elements rather than Uint8Array. This will require the custom JS bindings to return an Array for this query.

[Zhenyao Mo]

Created attachment 59454 [details]
patch

[Kenneth Russell]

Comment on attachment 59454 [details]
patch

Generally looks good but there are a few things I'd like to see changed.


WebCore/html/canvas/WebGLGetInfo.cpp:50
 +      : m_type(kTypeBoolArray)
Let's generalize this a little and take (bool* value, int size). Should also be const bool*.


WebCore/html/canvas/WebGLGetInfo.cpp:149
 +  const bool* WebGLGetInfo::getBoolArray() const
Let's use Vector<bool> as the representation. This can then return const Vector<bool>&.


WebCore/html/canvas/WebGLGetInfo.h:74
 +      WebGLGetInfo(bool* value);
See above comment.


WebCore/html/canvas/WebGLGetInfo.h:97
 +      const bool* getBoolArray() const;
See above comment.


WebCore/html/canvas/WebGLGetInfo.h:116
 +      bool m_boolArray[4];
Let's use Vector<bool> m_boolArray.


WebCore/html/canvas/WebGLRenderingContext.cpp:3184
 +      unsigned char value[4] = {0};
The safety checks on pname and in particular the notImplemented() should be preserved.


WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp:79
 +          for (int ii = 0; ii < 4; ++ii)
See comments above on handling variable length arrays.


WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp:114
 +          v8::Local<v8::Array> array = v8::Array::New(4);
See comments above on handling variable length arrays.

[Zhenyao Mo]

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

[Kenneth Russell]

Comment on attachment 59584 [details]
revised patch : responding to kbr's review

Looks good.

[Dimitri Glazkov (Google)]

Comment on attachment 59584 [details]
revised patch : responding to kbr's review

ok.

[WebKit Commit Bot]

Comment on attachment 59584 [details]
revised patch : responding to kbr's review

Clearing flags on attachment: 59584

Committed r61910: <http://trac.webkit.org/changeset/61910>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Committed r61913: <http://trac.webkit.org/changeset/61913>

[Eric Seidel (no email)]

Sorry.  git confused the ChagneLogs. :(