Add multi-planar functions to GpuMemoryBuffer

ui/gfx/gpu_memory_buffer.h

Index: ui/gfx/gpu_memory_buffer.h
diff --git a/ui/gfx/gpu_memory_buffer.h b/ui/gfx/gpu_memory_buffer.h
index 3aa0964c36886233cb5ce7c474834abb51b60606..ccf08603689c51a6b2b267632cd0baba54ce7667 100644
--- a/ui/gfx/gpu_memory_buffer.h
+++ b/ui/gfx/gpu_memory_buffer.h
@@ -63,10 +63,22 @@ class GFX_EXPORT GpuMemoryBuffer {
 
   // Maps the buffer into the client's address space so it can be written to by
   // the CPU. This call may block, for instance if the GPU needs to finish
-  // accessing the buffer or if CPU caches need to be synchronized. Returns NULL
-  // on failure.
+  // accessing the buffer or if CPU caches need to be synchronized.
+  // If the buffer contains a single plane, it returns a pointer to that single
+  // plane. If the buffer contains multiple planes, it returns a pointer to the
+  // plane at index 0.
+  // Returns NULL on failure.
   virtual void* Map() = 0;

[reveman, 2015/03/21 03:03:55]

I don't like having multiple versions of Map(). Let's just have one Map()
function that returns all planes.

[emircan, 2015/03/23 19:21:35]

Done.

 
+  // Maps the plane of the buffer at index |plane_index| into the client's
+  // address space so it can be written to by the CPU. This call may block, for
+  // instance if the GPU needs to finish accessing the buffer or if CPU caches
+  // need to be synchronized.
+  // If |plane_index| points to an index greater than or equal to the return
+  // value of GetNumberOfPlanes(), returns NULL.
+  // Returns NULL on failure.
+  virtual void* Map(size_t plane_index) = 0;

[reveman, 2015/03/21 03:03:55]

I'd prefer if Map took an array of void* and return true if successful. What
does Unmap() otherwise do? Unmap all currently mapped buffers? And what does
IsMapped return?

[emircan, 2015/03/23 19:21:35]

Done.

My reasoning to do it initially this way was that, writing to where (void**)
points can be dangerous as we cannot verify if given array is properly
allocated. 

+
   // Unmaps the buffer. It's illegal to use the pointer returned by Map() after
   // this has been called.
   virtual void Unmap() = 0;
@@ -74,12 +86,24 @@ class GFX_EXPORT GpuMemoryBuffer {
   // Returns true iff the buffer is mapped.
   virtual bool IsMapped() const = 0;
 
+  // Returns the number of planes based on the format of the buffer.
+  virtual size_t GetNumberOfPlanes() const = 0;

[reveman, 2015/03/21 03:03:55]

Do we really need this? Is the number not implied from the format?

[emircan, 2015/03/23 19:21:35]

It is implied and derived from format in the constructor. I think it would be
better to have this function than making multiple static function calls.

+
   // Returns the format for the buffer.
   virtual Format GetFormat() const = 0;
 
   // Returns the stride in bytes for the buffer.
+  // If the buffer contains a single plane, it returns the stride in bytes for
+  // that single plane. If the buffer contains multiple planes, it returns the
+  // stride in bytes for the plane at index 0.
   virtual uint32 GetStride() const = 0;

[reveman, 2015/03/21 03:03:55]

Same here. I prefer if we just have one version of this.

[emircan, 2015/03/23 19:21:35]

Done.

 
+  // Returns the stride in bytes for the plane of the buffer at index
+  // |plane_index|.
+  // If |plane_index| points to an index greater than or equal to the return
+  // value of GetNumberOfPlanes(), returns 0.
+  virtual uint32 GetStride(size_t plane_index) const = 0;

[reveman, 2015/03/21 03:03:55]

For consistency, I would prefer GetStride(uint32* stride), where |stride| is an
array where the size is implied by format.

[emircan, 2015/03/23 19:21:35]

Done.

+
   // Returns a platform specific handle for this buffer.
   virtual GpuMemoryBufferHandle GetHandle() const = 0;