ui: Use single buffer SurfaceTexture mode for native GpuMemoryBuffers on Android.

ui: Use single buffer SurfaceTexture mode for native GpuMemoryBuffers on Android.

This makes the surface texture implementation of GLImage require a copy
which allows us to release the ST properly and use single buffered mode.

The result is that we gain control over the memory usage and can support
persistent map usage.  

BUG=
TEST=gl_unittests --gtest_filter=GLImageSurfaceTexture*

[reveman, 2015-11-03 14:39:18]

Description was changed from

==========
ui: Use single buffer SurfaceTexture mode for native GpuMemoryBuffers on
Android.

BUG=
==========

to

==========
ui: Use single buffer SurfaceTexture mode for native GpuMemoryBuffers on
Android.

This makes the surface texture implementation of GLImage require a copy
which allows us to release the ST properly and use single buffered mode.

The result is that we gain control over the memory usage and can support
persistent map usage.  

BUG=
TEST=gl_unittests --gtest_filter=GLImageSurfaceTexture*
==========

[reveman, 2015-11-03 15:08:08]

reveman@chromium.org changed reviewers:
+ dcastagna@chromium.org

[Daniele Castagna, 2015-11-03 19:57:34]

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
File ui/gl/gl_image_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.cc:70: bool
GLImageSurfaceTexture::CopyTexSubImage(unsigned target,
Why implement CopyTexSubImage that supports only full rect?
To avoid re-allocating texture storage?

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.cc:126: glVertexAttribPointer(0, 2, GL_FLOAT,
GL_FALSE, sizeof(GLfloat) * 2, 0);
As pointed out by piman@, this needs to be set and restored at every draw.

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.cc:133: gfx::ScopedActiveTexture
active_texture(GL_TEXTURE0);
As mentioned by piman in the previous CL, we need to set/restore all the drawing
related state we're assuming here.
Should we add more Scoped* classes or do you have a better idea?

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
File ui/gl/gl_image_surface_texture.h (right):

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.h:50: unsigned framebuffer_ = 0;
Should we try to share this state before this CL? In this was we should also be
able to avoid some code duplication.

[reveman, 2015-12-05 23:09:08]

ptal

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
File ui/gl/gl_image_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.cc:70: bool
GLImageSurfaceTexture::CopyTexSubImage(unsigned target,
On 2015/11/03 at 19:57:34, Daniele Castagna wrote:
> Why implement CopyTexSubImage that supports only full rect?
> To avoid re-allocating texture storage?

This is what the compositor use for one-copy.

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.cc:126: glVertexAttribPointer(0, 2, GL_FLOAT,
GL_FALSE, sizeof(GLfloat) * 2, 0);
On 2015/11/03 at 19:57:34, Daniele Castagna wrote:
> As pointed out by piman@, this needs to be set and restored at every draw.

Fixed by rebase.

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.cc:133: gfx::ScopedActiveTexture
active_texture(GL_TEXTURE0);
On 2015/11/03 at 19:57:34, Daniele Castagna wrote:
> As mentioned by piman in the previous CL, we need to set/restore all the
drawing related state we're assuming here.
> Should we add more Scoped* classes or do you have a better idea?

Latest patch should be consistent with what we do in the IOSurface case.

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
File ui/gl/gl_image_surface_texture.h (right):

https://codereview.chromium.org/1419623008/diff/1/ui/gl/gl_image_surface_text...
ui/gl/gl_image_surface_texture.h:50: unsigned framebuffer_ = 0;
On 2015/11/03 at 19:57:34, Daniele Castagna wrote:
> Should we try to share this state before this CL? In this was we should also
be able to avoid some code duplication.

Long term that would be nice but there are some open questions about how to best
do that so I'd rather not have this important cleanup blocked on that.

[Daniele Castagna, 2015-12-05 23:43:57]

https://codereview.chromium.org/1419623008/diff/20001/content/common/gpu/gpu_...
File content/common/gpu/gpu_memory_buffer_factory_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/content/common/gpu/gpu_...
content/common/gpu/gpu_memory_buffer_factory_surface_texture.cc:46: // texture
id instread of a real one.
instread!

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
File ui/gl/gl_image_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:86: // Framebuffer.
nit: this can be removed.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:112: base::StringPrintf("#extension
GL_OES_EGL_image_external : require\n%s",
nit: Can this be part of kFragmentShader.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:138: glViewport(0, 0, size_.width(),
size_.height());
This could be removed.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
File ui/gl/gl_image_surface_texture_unittest.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture_unittest.cc:16: namespace gfx {
This should be gl.

[Daniele Castagna, 2015-12-05 23:43:57]

https://codereview.chromium.org/1419623008/diff/20001/content/common/gpu/gpu_...
File content/common/gpu/gpu_memory_buffer_factory_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/content/common/gpu/gpu_...
content/common/gpu/gpu_memory_buffer_factory_surface_texture.cc:46: // texture
id instread of a real one.
instread!

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
File ui/gl/gl_image_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:86: // Framebuffer.
nit: this can be removed.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:112: base::StringPrintf("#extension
GL_OES_EGL_image_external : require\n%s",
nit: Can this be part of kFragmentShader.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:138: glViewport(0, 0, size_.width(),
size_.height());
This could be removed.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
File ui/gl/gl_image_surface_texture_unittest.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture_unittest.cc:16: namespace gfx {
This should be gl.

[reveman, 2015-12-05 23:54:07]

https://codereview.chromium.org/1419623008/diff/20001/content/common/gpu/gpu_...
File content/common/gpu/gpu_memory_buffer_factory_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/content/common/gpu/gpu_...
content/common/gpu/gpu_memory_buffer_factory_surface_texture.cc:46: // texture
id instread of a real one.
On 2015/12/05 at 23:43:56, Daniele Castagna wrote:
> instread!

Done

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
File ui/gl/gl_image_surface_texture.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:86: // Framebuffer.
On 2015/12/05 at 23:43:57, Daniele Castagna wrote:
> nit: this can be removed.

Done.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:112: base::StringPrintf("#extension
GL_OES_EGL_image_external : require\n%s",
On 2015/12/05 at 23:43:57, Daniele Castagna wrote:
> nit: Can this be part of kFragmentShader.

Done.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture.cc:138: glViewport(0, 0, size_.width(),
size_.height());
On 2015/12/05 at 23:43:57, Daniele Castagna wrote:
> This could be removed.

Done.

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
File ui/gl/gl_image_surface_texture_unittest.cc (right):

https://codereview.chromium.org/1419623008/diff/20001/ui/gl/gl_image_surface_...
ui/gl/gl_image_surface_texture_unittest.cc:16: namespace gfx {
On 2015/12/05 at 23:43:57, Daniele Castagna wrote:
> This should be gl.

Done.

[Daniele Castagna, 2015-12-05 23:55:45]

lgtm