GPU client side changes for GpuMemoryBuffers

gpu/command_buffer/client/gpu_memory_buffer_factory.h

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef GPU_COMMAND_BUFFER_CLIENT_GPU_MEMORY_BUFFER_FACTORY_H_
 #define GPU_COMMAND_BUFFER_CLIENT_GPU_MEMORY_BUFFER_FACTORY_H_
 
 #include "gles2_impl_export.h"
 #include "gpu/command_buffer/client/gpu_memory_buffer.h"
 
 namespace gpu {
+namespace gles2 {
 
 // Getter and setter for a GpuMemoryBuffer factory for the current process.
 // Currently it is only used for Android Webview where both browser and
 // renderer are within the same process.
 
 // It is not valid to call this method before the setter is called.
 GLES2_IMPL_EXPORT const GpuMemoryBuffer::Creator&
     GetProcessDefaultGpuMemoryBufferFactory();

[reveman, 2013/05/08 04:23:06]

This is really confusing. GpuMemoryBuffer::Creator is a callback?!

This looks like a service side GpuMemoryBufferFactory. Should this live in
gpu/command_buffer/service/ and be a proper factory interface, just not a
callback?

[joth, 2013/05/08 16:26:38]

jamesr very specifically requested it be a Callback in preference to a
one-method factory. By same rationale, GpuMemoryBufferFactory could actually be
a callback just as well.
(although your comment below about calling it
ImageFactory::CreateGpuMemoryBuffer sounds like you predict other sorts of
methods will be added too? In which case keeping as class makes sense.)

-- see also my other comment that we can actually remove this from gpu if it's
not useful now we have a higher level factory. However.... I'd suggest doing
that as a followup as this patch is already complicated enough.

[reveman, 2013/05/08 20:02:41]

If it's a callback, should we call it a callback and not a
GpuMemoryBufferFactory or GpuMemoryBuffer::Creator? Code like this:
GetProcessDefaultGpuMemoryBufferFactory().Run(width, height);
in webgraphicscontext3d_in_process_command_buffer_impl.cc is confusing.

[kaanb, 2013/05/08 20:41:35]

How about GpuMemoryBufferCreateCallback?

[reveman, 2013/05/08 23:33:25]

sgtm

[kaanb, 2013/05/09 05:13:13]

I realized that to rename it I'll need to touch android_webview package, so
let's punt it to a smaller follow-up patch.

 
 // It is illegal to call the setter more than once.
 GLES2_IMPL_EXPORT void SetProcessDefaultGpuMemoryBufferFactory(
     const GpuMemoryBuffer::Creator& factory);
 
+class GpuMemoryBufferFactory {

[reveman, 2013/05/08 04:23:06]

I think this should be called ImageFactory or GLImageFactory. That way you get
gpu::client::ImageFactory::CreateGpuMemoryBuffer, which makes sense.

[kaanb, 2013/05/09 05:13:13]

Done.

[kaanb, 2013/05/09 05:13:13]

Done.

+ public:
+  virtual ~GpuMemoryBufferFactory() {}
+  // Craete a GpuMemoryBuffer and makes it available to the
+  // service side by inserting it to the ImageManager.
+  virtual scoped_ptr<GpuMemoryBuffer> CreateGpuMemoryBuffer(
+      int width, int height, unsigned int* image_id) = 0;
+};
+
+}  // namespace gles2
 }  // namespace gpu
 
 #endif  // GPU_COMMAND_BUFFER_CLIENT_GPU_MEMORY_BUFFER_FACTORY_H_