Enable SurfaceTexture based zero-copy texture uploading on Android platform

Enable SurfaceTexture based zero-copy texture uploading on Android platform.

This is the basic patch to enable zero-copy texture uploading on Android platform. It can run well under  "--enable-map-image --enable-surface-texture-buffer" flags.

BUG=

[junj, 2014-02-25 13:53:03]

My original zero-copy texture uploading prototype based on Android GraphicBuffer
showed that it could save about 10% power on
https://www.corp.google.com/~paullewis/no_crawl/impl-painting/ and got similar
performance(FPS) with the Aysnc texture uploading scheme. That's my motivation
to enable SurfaceTexture based zero-copy texture uploading.

Instead of making big patches in a single shot, I'd like to communicate with
chromium upstream first.

There is one code dependence broken in the patch.
content/common/gpu/surface_texture_bridge_android.cc includes file
"content/browser/android/child_process_launcher_android.h". It is caused by GPU
thread to call browser side code.

More will be added later to address the single buffer mode for SurfaceTexture,
buffer stride issue, performance data, etc.

[junj, 2014-02-25 15:20:54]

On 2014/02/25 13:53:03, junj wrote:
> My original zero-copy texture uploading prototype based on Android
GraphicBuffer
> showed that it could save about 10% power on
> https://www.corp.google.com/%7Epaullewis/no_crawl/impl-painting/ and got
similar
> performance(FPS) with the Aysnc texture uploading scheme. That's my motivation
> to enable SurfaceTexture based zero-copy texture uploading.
> 
> Instead of making big patches in a single shot, I'd like to communicate with
> chromium upstream first.
> 
> There is one code dependence broken in the patch.
> content/common/gpu/surface_texture_bridge_android.cc includes file
> "content/browser/android/child_process_launcher_android.h". It is caused by
GPU
> thread to call browser side code.
> 
> More will be added later to address the single buffer mode for SurfaceTexture,
> buffer stride issue, performance data, etc.

Besides normal zero-copy texture uploading routine, some tricks are used in the
patch. 
1. introduces a SYNC message(GpuCommandBufferMsg_CreateSurfaceTexture) from
renderer process to GPU threads before regesterGpuMemoryBuffer.
   There is one input parameter and two return values associated with this
message.
   input parameter: GpuMemoryBuffer pointer generated in renderer thread. It
will be used to set ANativeWindow invoked from GPU threads(Java IPC to renderer
native code).
   return value #1: texture id generated in GPU thread, which will be passed to
GPU thread again when regesterGpuMemoryBuffer and creating GLImage.
   return value #2: SurfaceTexture generated in GPU thread, which will be passed
to GPU thread again to construct the GLImage.
2. Most things done with the SYNC IPC.
   2.1 renderer process first allocates a GpuMemoryBuffer.
   2.2 renderer process send the GpuCommandBufferMsg_CreateSurfaceTexture
message, with the generated GpuMemoryBuffer as input parameter.
   2.3 GPU thread receives GpuCommandBufferMsg_CreateSurfaceTexture.
       It generates a Texture and new SurfaceTexture based on it.
       It get the java surface object associated with SurfaceTexture and send it
to renderer process via Java-level IPC, passing GpuMemoryBuffer handle(cast the
pointer to jlong) together as parameter.
       In renderer java code, call native code to set AnativeWindow. It gets the
GpuMemoryBuffer handle again by cast jlong to pointer. Then call
GpuMemoryBuffer->setNativeWindow(ANativeWindow*) in renderer native JNI method.
       After the JAVA-IPC returned, just return the generated texture id and
SurfaceTexture handle to renderer thread.
3. ASYNC regesterGpuMemoryBuffer
   2.4 renderer thread put the two return value(Tex ID and SurfaceTexture) in
handle and passed to GPU thread again in regesterGpuMemoryBuffer
       Message.
   2.5 gpu thread receive the regesterGpuMemoryBuffer message and construct
GLImage object based on received Tex ID and SurfaceTexture handle.
       the texture and SurfaceTexture will be deleted with ~GLImage(). 

After 2 and 3, all necessary objects are set up in sequence and other parts will
work with other normal zero-copy texture uploading logic.

[junj, 2014-02-26 07:07:09]

Add sohanjg and epennerAtGoogle as well.

[reveman, 2014-02-26 08:02:33]

Just a few comments. I haven't had a chance to look at the overall design yet.

https://codereview.chromium.org/177953004/diff/1/cc/resources/resource_provid...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/177953004/diff/1/cc/resources/resource_provid...
cc/resources/resource_provider.cc:2155: // is locked.
We should probably make this a general requirement of the map-image extension
and make sure a DCHECK fails if used incorrectly on all platforms.

https://codereview.chromium.org/177953004/diff/1/cc/trees/layer_tree_host_imp...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/177953004/diff/1/cc/trees/layer_tree_host_imp...
cc/trees/layer_tree_host_impl.cc:114: // #if !defined(OS_ANDROID)
Someone needs to take ownership of crbug.com/322780 and get this ifdef removed
before we can land any surface texture support. It's really just a matter of
removing the ifdef and making sure we don't regress.

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/co...
File content/common/gpu/client/command_buffer_proxy_impl.cc (right):

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/co...
content/common/gpu/client/command_buffer_proxy_impl.cc:421: #endif
I haven't looked at what this code does in detail but this is not a good place
for platform specific code. Can this be moved to
GpuMemoryBufferImplSurfaceTexture?

https://codereview.chromium.org/177953004/diff/1/content/public/common/conten...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/177953004/diff/1/content/public/common/conten...
content/public/common/content_switches.h:295: CONTENT_EXPORT extern const char
KEnableSurfaceTextureBuffer[];
We shouldn't need this switch. If a platform has a native implementation of
0-copy then we always use it instead of the fake shared memory backed version
which is mostly for testing. ie. there's no kEnableIOSurfaceGpuMemoryBuffer for
macos.

https://codereview.chromium.org/177953004/diff/1/content/renderer/render_thre...
File content/renderer/render_thread_impl.cc (left):

https://codereview.chromium.org/177953004/diff/1/content/renderer/render_thre...
content/renderer/render_thread_impl.cc:1110: &handle);
Please don't change any of code in this function. This is currently not a place
for platform or implementation specific code.

Even if you're not going to allocate the SurfaceTexture on the browser process
side (but that might be better, if possible?) still perform this sync IPC for
now and have it return a gfx::GpuMemoryBufferHandle with type set to
gfx::SURFACE_TEXTURE_BUFFER.

Don't worry about the unnecessary round-trip for now. I'd like to see a first
implementation of this that is as clean as possible. Once we've landed that and
have some good data on how it performs we can investigate Android specific
shortcuts that will improve performance.

The cost of the round-trip here has to be reduced anyhow (crbug.com/346712) so
it's likely not going to be a problem in the long run.

https://codereview.chromium.org/177953004/diff/1/ui/gfx/gpu_memory_buffer.h
File ui/gfx/gpu_memory_buffer.h (right):

https://codereview.chromium.org/177953004/diff/1/ui/gfx/gpu_memory_buffer.h#n...
ui/gfx/gpu_memory_buffer.h:48: unsigned int texture_id;
I think it should be possible to create the surfacetexture with a bogus
texture-id and then bind it to a gpu service provided id in
GLImage::BindTexImage. That would be much preferred as it's how GLImage work on
all other platforms. Only problem with that is that the surface texture takes
ownership of the texture-id when bound and will delete it but this might still
be easier to work with than using a different texture-id for
GLImageSurfaceTexture.

https://codereview.chromium.org/177953004/diff/1/ui/gfx/gpu_memory_buffer.h#n...
ui/gfx/gpu_memory_buffer.h:49: void* surface_texture_handle;
hm, this should instead be something that can be serialized.

[Hongbo Min, 2014-03-18 06:10:02]

Some comments for your reference.

https://codereview.chromium.org/177953004/diff/1/content/app/android/child_pr...
File content/app/android/child_process_service.cc (right):

https://codereview.chromium.org/177953004/diff/1/content/app/android/child_pr...
content/app/android/child_process_service.cc:146: window =
ANativeWindow_fromSurface(env, jsurface);
When would `window` be released?

https://codereview.chromium.org/177953004/diff/1/content/app/android/child_pr...
content/app/android/child_process_service.cc:147:
GpuMemoryBufferImplSurfaceTexture* gpu_memory_buffer_handle =
Return as early as possible if window is NULL.

https://codereview.chromium.org/177953004/diff/1/content/browser/android/chil...
File content/browser/android/child_process_launcher_android.cc (right):

https://codereview.chromium.org/177953004/diff/1/content/browser/android/chil...
content/browser/android/child_process_launcher_android.cc:168:
gfx::ScopedJavaSurface surface(surface_texture.get());
Is it safe that the Surface is released  after exiting this function scope?

https://codereview.chromium.org/177953004/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/177953004/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:1148:
switches::KEnableSurfaceTextureBuffer,
nit: KEnable..->kEnable..

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
File content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc
(right):

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:39: return;
Error log if ANativeWindow_setBuffersGeometry fails?

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:46: if
(!native_window_)
Need to return if it is already mapped.

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:49: if
(ANativeWindow_lock(native_window_, &buffer, 0))
It may need to have a pair calling of ANativeWindow_unlockAndPost if
ANativeWindow_lock returns 0.

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:68: if
(!native_window_)
Does it also need to return if 'mapped' is false? so that Release build won't
run ANativeWindow_unlockAndPost.

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/surface_t...
File content/common/gpu/surface_texture_bridge_android.cc (right):

https://codereview.chromium.org/177953004/diff/1/content/common/gpu/surface_t...
content/common/gpu/surface_texture_bridge_android.cc:21: using
gpu::gles2::GLES2Decoder;
You may want to remove these 3 unused 'using' stmt.

https://codereview.chromium.org/177953004/diff/1/content/content_common.gypi
File content/content_common.gypi (right):

https://codereview.chromium.org/177953004/diff/1/content/content_common.gypi#...
content/content_common.gypi:203:
'common/gpu/client/gpu_memory_buffer_impl_surface_texture.h',
nit:alpha order.

[junj, 2014-03-18 06:27:41]

On 2014/03/18 06:10:02, Hongbo Min wrote:
> Some comments for your reference.
> 
>
https://codereview.chromium.org/177953004/diff/1/content/app/android/child_pr...
> File content/app/android/child_process_service.cc (right):
> 
>
https://codereview.chromium.org/177953004/diff/1/content/app/android/child_pr...
> content/app/android/child_process_service.cc:146: window =
> ANativeWindow_fromSurface(env, jsurface);
> When would `window` be released?
> 
>
https://codereview.chromium.org/177953004/diff/1/content/app/android/child_pr...
> content/app/android/child_process_service.cc:147:
> GpuMemoryBufferImplSurfaceTexture* gpu_memory_buffer_handle =
> Return as early as possible if window is NULL.
> 
>
https://codereview.chromium.org/177953004/diff/1/content/browser/android/chil...
> File content/browser/android/child_process_launcher_android.cc (right):
> 
>
https://codereview.chromium.org/177953004/diff/1/content/browser/android/chil...
> content/browser/android/child_process_launcher_android.cc:168:
> gfx::ScopedJavaSurface surface(surface_texture.get());
> Is it safe that the Surface is released  after exiting this function scope?
> 
>
https://codereview.chromium.org/177953004/diff/1/content/browser/renderer_hos...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/177953004/diff/1/content/browser/renderer_hos...
> content/browser/renderer_host/render_process_host_impl.cc:1148:
> switches::KEnableSurfaceTextureBuffer,
> nit: KEnable..->kEnable..
> 
>
https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
> File content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc
> (right):
> 
>
https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
> content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:39:
return;
> Error log if ANativeWindow_setBuffersGeometry fails?
> 
>
https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
> content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:46: if
> (!native_window_)
> Need to return if it is already mapped.
> 
>
https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
> content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:49: if
> (ANativeWindow_lock(native_window_, &buffer, 0))
> It may need to have a pair calling of ANativeWindow_unlockAndPost if
> ANativeWindow_lock returns 0.
> 
>
https://codereview.chromium.org/177953004/diff/1/content/common/gpu/client/gp...
> content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc:68: if
> (!native_window_)
> Does it also need to return if 'mapped' is false? so that Release build won't
> run ANativeWindow_unlockAndPost.
> 
>
https://codereview.chromium.org/177953004/diff/1/content/common/gpu/surface_t...
> File content/common/gpu/surface_texture_bridge_android.cc (right):
> 
>
https://codereview.chromium.org/177953004/diff/1/content/common/gpu/surface_t...
> content/common/gpu/surface_texture_bridge_android.cc:21: using
> gpu::gles2::GLES2Decoder;
> You may want to remove these 3 unused 'using' stmt.
> 
> https://codereview.chromium.org/177953004/diff/1/content/content_common.gypi
> File content/content_common.gypi (right):
> 
>
https://codereview.chromium.org/177953004/diff/1/content/content_common.gypi#...
> content/content_common.gypi:203:
> 'common/gpu/client/gpu_memory_buffer_impl_surface_texture.h',
> nit:alpha order.

Hi, Hongbo. Thanks for reviewing the patch. The work in this patch was suspended
since Samsung guys were also doing the similar work. I agreed with Samsung guys
to work together on this and I expect sohanjg would land his first patch on
this. Therefore, I stopped improving this basic patch.

I have several pending patches as well to support attach/detach texture and
single-buffer mode for SurfaceTexture. But it should wait after  sohanjg's first
patch landed.

[reveman, 2014-03-18 08:58:36]

Sohan landed single buffer surface texture support and is working on proper
TEXTURE_EXTERNAL_OES support.

I've taken over the work to land initial support for a surface texture backed
GpuMemoryBuffer type. A patch for that can be found here:
https://codereview.chromium.org/195583003/