Bug 40437 - getParameter(COLOR_WRITEMASK) needs to return Array
Summary: getParameter(COLOR_WRITEMASK) needs to return Array
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-10 11:58 PDT by Kenneth Russell
Modified: 2010-06-25 16:15 PDT (History)
6 users (show)

See Also:


Attachments
patch (10.13 KB, patch)
2010-06-22 18:23 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
revised patch : responding to kbr's review (10.42 KB, patch)
2010-06-23 17:15 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-06-10 11:58:14 PDT
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.
Comment 1 Zhenyao Mo 2010-06-22 18:23:26 PDT
Created attachment 59454 [details]
patch
Comment 2 Kenneth Russell 2010-06-23 16:36:51 PDT
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.
Comment 3 Zhenyao Mo 2010-06-23 17:15:43 PDT
Created attachment 59584 [details]
revised patch : responding to kbr's review
Comment 4 Kenneth Russell 2010-06-23 17:27:35 PDT
Comment on attachment 59584 [details]
revised patch : responding to kbr's review

Looks good.
Comment 5 Dimitri Glazkov (Google) 2010-06-24 09:56:45 PDT
Comment on attachment 59584 [details]
revised patch : responding to kbr's review

ok.
Comment 6 WebKit Commit Bot 2010-06-25 15:47:20 PDT
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>
Comment 7 WebKit Commit Bot 2010-06-25 15:47:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Eric Seidel (no email) 2010-06-25 16:14:18 PDT
Committed r61913: <http://trac.webkit.org/changeset/61913>
Comment 9 Eric Seidel (no email) 2010-06-25 16:15:06 PDT
Sorry.  git confused the ChagneLogs. :(