Implement more of the JavaScript GL API.

Implement more of the JavaScript GL API.

Right now I am just translating as directly as possible from the C spinning cube demo -- once it all works I will circle back through and refine the JavaScript.

R=abarth@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242230

[abarth-chromium, 2013-12-13 21:30:34]

lgtm

https://codereview.chromium.org/114883003/diff/40001/mojo/apps/js/bindings/gl...
File mojo/apps/js/bindings/gl/context.cc (right):

https://codereview.chromium.org/114883003/diff/40001/mojo/apps/js/bindings/gl...
mojo/apps/js/bindings/gl/context.cc:74: glGetShaderiv(shader, GL_COMPILE_STATUS,
&compiled);
Maybe http://www.khronos.org/opengles/sdk/docs/man/xhtml/glGetError.xml ?

https://codereview.chromium.org/114883003/diff/40001/mojo/apps/js/bindings/gl...
mojo/apps/js/bindings/gl/context.cc:96: return std::string(info_log,
info_log_length);
You can avoid one of these copies by resizing the std::string on the stack and
writing directly into it.

[Aaron Boodman, 2013-12-13 21:43:57]

https://codereview.chromium.org/114883003/diff/40001/mojo/apps/js/bindings/gl...
File mojo/apps/js/bindings/gl/context.cc (right):

https://codereview.chromium.org/114883003/diff/40001/mojo/apps/js/bindings/gl...
mojo/apps/js/bindings/gl/context.cc:74: glGetShaderiv(shader, GL_COMPILE_STATUS,
&compiled);
On 2013/12/13 21:30:35, abarth wrote:
> Maybe http://www.khronos.org/opengles/sdk/docs/man/xhtml/glGetError.xml ?

But I want the textual error message...

https://codereview.chromium.org/114883003/diff/40001/mojo/apps/js/bindings/gl...
mojo/apps/js/bindings/gl/context.cc:96: return std::string(info_log,
info_log_length);
On 2013/12/13 21:30:35, abarth wrote:
> You can avoid one of these copies by resizing the std::string on the stack and
> writing directly into it.

Good point. Fixed.

[abarth-chromium, 2013-12-14 03:01:07]

lgtm

[Aaron Boodman, 2013-12-18 00:23:08]

ptal - it draws the first frame now

[abarth-chromium, 2013-12-18 01:19:30]

LGTM

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/bindings/g...
File mojo/apps/js/bindings/gl/context.cc (right):

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/bindings/g...
mojo/apps/js/bindings/gl/context.cc:22: *out =
reinterpret_cast<GLvoid*>(int_val);
Woah.  Where is this used?  I would have expected us to use array buffers rather
than encoding a void* as a JS number.

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/main.js
File mojo/apps/js/main.js (right):

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/main.js#ne...
mojo/apps/js/main.js:164:
console.log(['NativeViewportClientImpl.prototype.DidOpen']);
Why the [ ] ?

[Aaron Boodman, 2013-12-18 02:19:07]

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/bindings/g...
File mojo/apps/js/bindings/gl/context.cc (right):

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/bindings/g...
mojo/apps/js/bindings/gl/context.cc:22: *out =
reinterpret_cast<GLvoid*>(int_val);
On 2013/12/18 01:19:30, abarth wrote:
> Woah.  Where is this used?  I would have expected us to use array buffers
rather
> than encoding a void* as a JS number.

It's used by glVertexAttribPointer:
http://www.opengl.org/sdk/docs/man/xhtml/glVertexAttribPointer.xml.

They are using void* to encode an offset. Perhaps I should just manually handle
that case so that it's more self-documenting and harder to seep into other
places.

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/main.js
File mojo/apps/js/main.js (right):

https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/main.js#ne...
mojo/apps/js/main.js:164:
console.log(['NativeViewportClientImpl.prototype.DidOpen']);
On 2013/12/18 01:19:30, abarth wrote:
> Why the [ ] ?

Jochen recently changed it to be required:

http://git.chromium.org/gitweb/?p=chromium.git;a=commit;h=aebb36a7011a8fdc5d0...

That was probably an accident. I'll change it back.

[abarth-chromium, 2013-12-18 03:14:50]

On 2013/12/18 02:19:07, Aaron Boodman wrote:
>
https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/bindings/g...
> File mojo/apps/js/bindings/gl/context.cc (right):
> 
>
https://codereview.chromium.org/114883003/diff/100001/mojo/apps/js/bindings/g...
> mojo/apps/js/bindings/gl/context.cc:22: *out =
> reinterpret_cast<GLvoid*>(int_val);
> On 2013/12/18 01:19:30, abarth wrote:
> > Woah.  Where is this used?  I would have expected us to use array buffers
> rather
> > than encoding a void* as a JS number.
> 
> It's used by glVertexAttribPointer:
> http://www.opengl.org/sdk/docs/man/xhtml/glVertexAttribPointer.xml.
> 
> They are using void* to encode an offset. Perhaps I should just manually
handle
> that case so that it's more self-documenting and harder to seep into other
> places.

WebGL uses GLintptr, which seems a bit more sane:

    void vertexAttribPointer(GLuint indx, GLint size, GLenum type, 
                             GLboolean normalized, GLsizei stride, GLintptr
offset);

IMHO, having a stub would be worthwhile to make the code look less scary.  :)

[Aaron Boodman, 2013-12-20 00:59:31]

Adam, you may want to have a look at the changes to main.js.

[commit-bot: I haz the power, 2013-12-20 01:01:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/114883003/180001

[commit-bot: I haz the power, 2013-12-20 01:18:29]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[abarth-chromium, 2013-12-20 04:21:01]

https://codereview.chromium.org/114883003/diff/180001/mojo/apps/js/bindings/c...
File mojo/apps/js/bindings/codec.js (right):

https://codereview.chromium.org/114883003/diff/180001/mojo/apps/js/bindings/c...
mojo/apps/js/bindings/codec.js:129: return result;
These last few lines are dead now.  You can probably remove them.  Once you do,
load64 won't have any callers and you can remove it too.

[Aaron Boodman, 2013-12-20 05:42:23]

https://codereview.chromium.org/114883003/diff/180001/mojo/apps/js/bindings/c...
File mojo/apps/js/bindings/codec.js (right):

https://codereview.chromium.org/114883003/diff/180001/mojo/apps/js/bindings/c...
mojo/apps/js/bindings/codec.js:129: return result;
On 2013/12/20 04:21:02, abarth wrote:
> These last few lines are dead now.  You can probably remove them.  Once you
do,
> load64 won't have any callers and you can remove it too.

Eep. Thanks.

[commit-bot: I haz the power, 2013-12-20 07:22:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/114883003/200001

[commit-bot: I haz the power, 2013-12-20 08:22:31]

Retried try job too often on mac_rel for step(s) app_list_unittests,
base_unittests, browser_tests, cacheinvalidation_unittests, cc_unittests,
check_deps, check_deps2git, chromedriver2_unittests, components_unittests,
content_browsertests, content_unittests, crypto_unittests,
google_apis_unittests, gpu_unittests, interactive_ui_tests, ipc_tests,
jingle_unittests, media_unittests, message_center_unittests, nacl_integration,
net_unittests, ppapi_unittests, printing_unittests, remoting_unittests,
sql_unittests, sync_integration_tests, sync_unit_tests, telemetry_unittests,
unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-12-20 20:57:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/114883003/230001

[commit-bot: I haz the power, 2013-12-20 23:09:24]

Retried try job too often on win_x64_rel for step(s) base_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[Aaron Boodman, 2013-12-21 00:51:05]

Committed patchset #12 manually as r242230 (presubmit successful).