Plumb through EGL_NV_post_sub_buffer and GLX_MESA_copy_sub_buffer.

Plumb through EGL_NV_post_sub_buffer and GLX_MESA_copy_sub_buffer.

These two extensions allow a partial swap: just pushing part of the backbuffer to the front buffer. This will allow the WK compositor to push a partial update to the screen instead of a full frame update (https://bugs.webkit.org/show_bug.cgi?id=70533).

We should be able to do something similar for TOUCHUI ImageTransportSurfaces (hence the hooks into GLSurface and the glPostSubBufferCHROMIUM command).

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

[jonathan.backer, 2011-11-10 17:42:02]

A preliminary version of WebKit side is here:
https://bugs.webkit.org/show_bug.cgi?id=72028

http://codereview.chromium.org/8512005/diff/1/gpu/command_buffer/common/gles2...
File gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h (right):

http://codereview.chromium.org/8512005/diff/1/gpu/command_buffer/common/gles2...
gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h:1: //
Copyright (c) 2011 The Chromium Authors. All rights reserved.
I'm worried that the script did not work well here. It's changed way too much.

[apatrick_chromium, 2011-11-10 19:33:16]

+gman so he can take a look as well.

http://codereview.chromium.org/8512005/diff/2001/gpu/command_buffer/client/gl...
File gpu/command_buffer/client/gles2_implementation.cc (right):

http://codereview.chromium.org/8512005/diff/2001/gpu/command_buffer/client/gl...
gpu/command_buffer/client/gles2_implementation.cc:2672: }
I think the flush should go here. The EGL_NV_post_sub_buffer spec says this
PostSubBuffer is an implicit flush. You might also want the flow control stuff
GLES2Implementation::SwapBuffers here.

http://codereview.chromium.org/8512005/diff/2001/ui/gfx/gl/gl_surface_egl.cc
File ui/gfx/gl/gl_surface_egl.cc (right):

http://codereview.chromium.org/8512005/diff/2001/ui/gfx/gl/gl_surface_egl.cc#...
ui/gfx/gl/gl_surface_egl.cc:215: EGLBoolean retVal =
eglQuerySurface(GetDisplay(),
Is it really the case that if you ask for a surface with EGL_NV_post_sub_buffer,
it will succeed but then report that it is not supported via QuerySurface? It
would be nice if the feature info code could just assume that if the
EGL_NV_post_sub_buffer extension is reported as available then  it can report it
as available to the client.

[greggman, 2011-11-10 19:39:15]

lgtm

[jonathan.backer, 2011-11-10 21:00:25]

+piman for flow control

http://codereview.chromium.org/8512005/diff/2001/gpu/command_buffer/client/gl...
File gpu/command_buffer/client/gles2_implementation.cc (right):

http://codereview.chromium.org/8512005/diff/2001/gpu/command_buffer/client/gl...
gpu/command_buffer/client/gles2_implementation.cc:2672: }
On 2011/11/10 19:33:16, apatrick_chromium wrote:
> I think the flush should go here. The EGL_NV_post_sub_buffer spec says this
> PostSubBuffer is an implicit flush. You might also want the flow control stuff
> GLES2Implementation::SwapBuffers here.

Done.

http://codereview.chromium.org/8512005/diff/2001/ui/gfx/gl/gl_surface_egl.cc
File ui/gfx/gl/gl_surface_egl.cc (right):

http://codereview.chromium.org/8512005/diff/2001/ui/gfx/gl/gl_surface_egl.cc#...
ui/gfx/gl/gl_surface_egl.cc:215: EGLBoolean retVal =
eglQuerySurface(GetDisplay(),
On 2011/11/10 19:33:16, apatrick_chromium wrote:
> Is it really the case that if you ask for a surface with
EGL_NV_post_sub_buffer,
> it will succeed but then report that it is not supported via QuerySurface? It
> would be nice if the feature info code could just assume that if the
> EGL_NV_post_sub_buffer extension is reported as available then  it can report
it
> as available to the client.

I think so. I was inspired by some ChromeOS WM code.

From the bottom of
http://www.khronos.org/registry/egl/extensions/NV/EGL_NV_post_sub_buffer.txt in
the revision history:

"Allowed EGL_POST_SUB_BUFFER_SUPPORTED_NV to be used as a hint when creating
window surfaces."

FWIW, the Intel GLES stack succeeds with the code as is (still get a window
surface), but correctly says that it doesn't support the extension.

[piman, 2011-11-10 21:14:47]

LGTM overall, just 2 comments.

http://codereview.chromium.org/8512005/diff/9001/gpu/command_buffer/service/g...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

http://codereview.chromium.org/8512005/diff/9001/gpu/command_buffer/service/g...
gpu/command_buffer/service/gles2_cmd_decoder.cc:5682:
DCHECK(surface_->SupportsPostSubBuffer());
I think this needs to be validated, not checked, causing a GL error
(INVALID_OPERATION ?) if not available. A malicious client (compromised
renderer, nacl) can still call this.

http://codereview.chromium.org/8512005/diff/9001/ui/gfx/gl/gl_surface_egl.cc
File ui/gfx/gl/gl_surface_egl.cc (right):

http://codereview.chromium.org/8512005/diff/9001/ui/gfx/gl/gl_surface_egl.cc#...
ui/gfx/gl/gl_surface_egl.cc:205: NULL);
nit: indentation for the line continuation should be 4 spaces.

[jonathan.backer, 2011-11-10 21:40:31]

http://codereview.chromium.org/8512005/diff/9001/gpu/command_buffer/service/g...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

http://codereview.chromium.org/8512005/diff/9001/gpu/command_buffer/service/g...
gpu/command_buffer/service/gles2_cmd_decoder.cc:5682:
DCHECK(surface_->SupportsPostSubBuffer());
On 2011/11/10 21:14:47, piman wrote:
> I think this needs to be validated, not checked, causing a GL error
> (INVALID_OPERATION ?) if not available. A malicious client (compromised
> renderer, nacl) can still call this.

Done.

http://codereview.chromium.org/8512005/diff/9001/ui/gfx/gl/gl_surface_egl.cc
File ui/gfx/gl/gl_surface_egl.cc (right):

http://codereview.chromium.org/8512005/diff/9001/ui/gfx/gl/gl_surface_egl.cc#...
ui/gfx/gl/gl_surface_egl.cc:205: NULL);
On 2011/11/10 21:14:47, piman wrote:
> nit: indentation for the line continuation should be 4 spaces.

Done.

[piman, 2011-11-10 21:42:42]

lgtm

[apatrick_chromium, 2011-11-10 22:35:45]

LGTM