Add a SpinningCube to Mojo's sample_app

Add a SpinningCube to Mojo's sample_app

We don't actually call this code yet, but we're going to need this code in
order to demo a spinning cube. I've verified that this code draws a cube when
called directly from mojo_shell. It should work when called from sample_app
once we figure out how we want to expose GL to Mojo apps.

R=jamesr@chromium.org

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

[jamesr, 2013-11-06 01:42:45]

nifty!

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
File mojo/examples/sample_app/DEPS (right):

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/DEPS:2: # TODO(abarth): Rather than including this
interface directly,
you could use the khronos headers, which provide a declaration of a big pile of
C functions for GL entry points. we'll still need a way for clients to bind that
interface

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
File mojo/examples/sample_app/spinning_cube.cc (right):

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/spinning_cube.cc:139: return 0;  // Why don't we need
to delete the shaders?
this looks copied from cc/output/program_binding.h. In cc, we enforce (via
dchecks) that the only situation where a program creation fails is if the
context is lost, in which case trying to delete things is futile anyway.

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
File mojo/examples/sample_app/spinning_cube.h (right):

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/spinning_cube.h:27: void Draw();
what do you expect will call these functions?

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/spinning_cube.h:34: gpu::gles2::GLES2Interface* gl_;
does this have some sort of ownership stake in the context?

[abarth-chromium, 2013-11-06 02:25:10]

Thanks for taking a look!

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
File mojo/examples/sample_app/DEPS (right):

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/DEPS:2: # TODO(abarth): Rather than including this
interface directly,
On 2013/11/06 01:42:45, jamesr wrote:
> you could use the khronos headers, which provide a declaration of a big pile
of
> C functions for GL entry points. we'll still need a way for clients to bind
that
> interface

Yeah, I was going to worry about that once we had more of the Mojo bindings
working.  This approach made it easy to test for now.

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
File mojo/examples/sample_app/spinning_cube.cc (right):

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/spinning_cube.cc:139: return 0;  // Why don't we need
to delete the shaders?
On 2013/11/06 01:42:45, jamesr wrote:
> this looks copied from cc/output/program_binding.h.

It's actually copied from Simple_VertexShader.c (although I added the comment).

> In cc, we enforce (via
> dchecks) that the only situation where a program creation fails is if the
> context is lost, in which case trying to delete things is futile anyway.

Ok.  I'll replace this with a DCHECK.

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
File mojo/examples/sample_app/spinning_cube.h (right):

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/spinning_cube.h:27: void Draw();
On 2013/11/06 01:42:45, jamesr wrote:
> what do you expect will call these functions?

Probably something in sample_app.cc.  Once we have a Graphics3DClient interface,
the sample_app can listen for OnSwapBuffersComplete and draw a new frame.

https://codereview.chromium.org/61423003/diff/30001/mojo/examples/sample_app/...
mojo/examples/sample_app/spinning_cube.h:34: gpu::gles2::GLES2Interface* gl_;
On 2013/11/06 01:42:45, jamesr wrote:
> does this have some sort of ownership stake in the context?

We're probably going to end up calling static C functions to talk to GL, so this
pointer will go away.  We'll probably need to split this object into two pieces:
one that survives the destruction of the context and one that has the
context-specific state.

[abarth-chromium, 2013-11-06 02:51:02]

Ok, I've taken a stab at addressing your comments.  There's still the open issue
that this code isn't called anywhere yet.  I'm feeling slightly blocked on that
front on by the bindings...  I need to see if I can help out there more
directly.

[jamesr, 2013-11-06 02:57:48]

are you worried about backpressure / flow control or just not having enough
infrastructure to actually spin this thing up for real?

anyway lgtm to checkpoint as is

[abarth-chromium, 2013-11-06 04:13:59]

On 2013/11/06 02:57:48, jamesr wrote:
> are you worried about backpressure / flow control or just not having enough
> infrastructure to actually spin this thing up for real?

Not enough infrastructure.  Currently we can only send strings into the app.

I need to study how backpressure works.  I might bug you about that tomorrow.

Thanks for the review.

[commit-bot: I haz the power, 2013-11-06 04:40:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/61423003/110001

[commit-bot: I haz the power, 2013-11-06 05:02:50]

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

[commit-bot: I haz the power, 2013-11-06 05:17:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/61423003/290001

[commit-bot: I haz the power, 2013-11-06 06:40:58]

Failed to trigger a try job on win7_aura
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-11-06 06:41:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/61423003/320002

[commit-bot: I haz the power, 2013-11-06 08:03:45]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-06 15:36:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/61423003/650001

[abarth-chromium, 2013-11-06 18:44:23]

Committed patchset #7 manually as r233309 (presubmit successful).