Add a shallow command buffer flush that doesn't glFlush

Add a shallow command buffer flush that doesn't glFlush

BUG=131661
TEST=gpu_unittests now checks for the GL_CHROMIUM_shallow_flush string.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=147969

[nduca, 2012-07-18 06:13:06]

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(left):

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:942:
DELEGATE_TO_GL_4(framebufferRenderbuffer, FramebufferRenderbuffer,
Who is advertising the extension as being supported? Look for some of the other
extensions on the wgc3d interface and then grep fro them to see how we typically
advertise extensions.

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(right):

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:943:
gl_->ShallowFlushCHROMIUM();
Was there a reason you couldn't say gl_->helper()->CommandBuffer::flush()?

Copy the design of WGC3DCBI::waitSyncPoint

Then you could skip all the delegation-via-GL bullcrap.

http://codereview.chromium.org/10782005/diff/1/webkit/gpu/webgraphicscontext3...
File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right):

http://codereview.chromium.org/10782005/diff/1/webkit/gpu/webgraphicscontext3...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:1643:
DELEGATE_TO_GL(shallowFlushCHROMIUM, ShallowFlushCHROMIUM)
Why different here than wgc3dcbi? Maybe this is intentional, I forget how this
file works.

http://codereview.chromium.org/10782005/diff/1/webkit/gpu/webgraphicscontext3...
File webkit/gpu/webgraphicscontext3d_in_process_impl.h (right):

http://codereview.chromium.org/10782005/diff/1/webkit/gpu/webgraphicscontext3...
webkit/gpu/webgraphicscontext3d_in_process_impl.h:481: // CHROMIUM_shallow_flush
only applies to contexts that use the command_buffer
// Comments are complete sentences that end with period.

[brianderson, 2012-07-19 03:29:44]

Responses inline.  I have only fixed the missing period in my comment so far.  I
would like to keep the other parts as is, but will wait for your feedback.

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(left):

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:942:
DELEGATE_TO_GL_4(framebufferRenderbuffer, FramebufferRenderbuffer,
It is advertised with other client-side-only extensions in
gles2_implementation.cc.

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(right):

http://codereview.chromium.org/10782005/diff/1/content/common/gpu/client/webg...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:943:
gl_->ShallowFlushCHROMIUM();
In each of the wgc3d implementations I copied the call style of the regular
flush, which was slightly different.

wgc3Dcbi calling through GLES2Implementation results in a single thread check
and a call to GPU_CLIENT_LOG.

wgc3Dipcbi using the delegation macro calls a ClearContext member function for
what looks like debugging purposes.

http://codereview.chromium.org/10782005/diff/1/gpu/command_buffer/client/gles...
File gpu/command_buffer/client/gles2_implementation.cc (right):

http://codereview.chromium.org/10782005/diff/1/gpu/command_buffer/client/gles...
gpu/command_buffer/client/gles2_implementation.cc:2131:
"GL_CHROMIUM_shallow_flush "
The extension is advertised here and I double checked with gdb that it is
advertised when calling getString().

http://codereview.chromium.org/10782005/diff/1/webkit/gpu/webgraphicscontext3...
File webkit/gpu/webgraphicscontext3d_in_process_impl.h (right):

http://codereview.chromium.org/10782005/diff/1/webkit/gpu/webgraphicscontext3...
webkit/gpu/webgraphicscontext3d_in_process_impl.h:481: // CHROMIUM_shallow_flush
only applies to contexts that use the command_buffer
Fixed for next patch set.

[nduca, 2012-07-19 03:33:19]

Why aren't you calling helper()->CommandBuffer::Flush()?

[brianderson, 2012-07-19 17:18:52]

On 2012/07/19 03:33:19, nduca wrote:
> Why aren't you calling helper()->CommandBuffer::Flush()?

It gets called in gles2_implementation.cc.

[nduca, 2012-07-19 17:34:00]

Yes but I told you to call it right in wgc3dcbi

[brianderson, 2012-07-19 18:34:21]

I'm ok with doing that, but want to understand the motivation.  Are we going for
better runtime performance and code clarity?

The advantage of always delegating to the GLES2Implementation, as currently
implemented, allows the single thread check and logging to take place in
GLES2Implementation.  Are we okay with bypassing those by calling
helper()->CommandBuffer::Flush() directly?

[nduca, 2012-07-19 21:28:40]

If I understand correctly, we use GLEs2 implementation for things that we intend
to go over the command buffer to the service side. The shallow flush is just a
client-side method. Hence, it bypassing the gles2implementation.

[brianderson, 2012-07-19 22:32:30]

So it's a little confusing.  GLES2Implementation has a way of reporting
client-side only methods, which makes it seem like putting the shallowFlush
there is okay.  Other than the WGC3D's, it looks like a couple PPAPI (pepper?)
classes also use GLES2Implementation.  Could they ever have a need for a
shallowFlush?

I know the "implementation" of a shallow flush is really simple, but I would
still like to keep GLES2Implemenatation as the one true implementation everyone
eventually calls.

[nduca, 2012-07-20 04:46:00]

Gregg, please advise.

[greggman, 2012-07-20 05:22:42]

LGTM

Calling gl_->ShallowFlushCHROMIUM() isn't much worse than calling
gl_->helper()->Flush();  In fact if you move the impl of ShallowFlushCHROMIUM to
the .h instead of the .cc it should be exactly the same.

[nduca, 2012-07-20 05:23:29]

Thanks! As long as you're happy, I am as well. LGTM.

[commit-bot: I haz the power, 2012-07-23 20:53:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/brianderson@chromium.org/10782005/15001

[brianderson, 2012-07-23 20:54:35]

Git try passes after fixing one of the tests.  Nat, can you commit?

[commit-bot: I haz the power, 2012-07-23 22:30:04]

Change committed as 147969