Update GPU memory buffers to use StrideInBytes internally.

content/common/gpu/client/gpu_memory_buffer_impl.h

Index: content/common/gpu/client/gpu_memory_buffer_impl.h
diff --git a/content/common/gpu/client/gpu_memory_buffer_impl.h b/content/common/gpu/client/gpu_memory_buffer_impl.h
index dc792246e1fac1d29d2f02271b5dc03735b37d90..cd462e174437b0c356570a0e40ea119063d700b6 100644
--- a/content/common/gpu/client/gpu_memory_buffer_impl.h
+++ b/content/common/gpu/client/gpu_memory_buffer_impl.h
@@ -33,9 +33,15 @@ class CONTENT_EXPORT GpuMemoryBufferImpl : public gfx::GpuMemoryBuffer {
   // Type-checking upcast routine. Returns an NULL on failure.
   static GpuMemoryBufferImpl* FromClientBuffer(ClientBuffer buffer);
 
-  // Returns the number of bytes per pixel that must be used by an
+  // Returns the number of bits per pixel that must be used by an
   // implementation when using |format|.
-  static size_t BytesPerPixel(Format format);
+  static size_t BitsPerPixel(Format format);
+
+  // Returns the number of bytes that an implementation must use to store
+  // |pixel_count| number of pixels.
+  static size_t PixelsToBytes(
+      size_t pixel_count,
+      gfx::GpuMemoryBuffer::Format format);

[reveman, 2014/12/15 17:33:34]

Hm, this doesn't work with stride != width * bytes_per_pixel. I assume that
stride is never less than 1 byte for compressed formats as that would require
more significant changes to the GpuMemoryBuffer API, correct? Could we introduce
a BytesPerRow or StrideInBytes function instead of this?

nit: gfx::GpuMemoryBuffer:: prefix unnecessary

[christiank, 2014/12/16 12:02:52]

You're right. This code is based on the assumption that width and height are
evenly dividable by 4 (texture compression operates on 4x4 pixel blocks), which
happens to work with the sizes we're using.

The stride can never be less than 1 byte. However, for some image sizes it can
be an uneven number of bytes. Consider an image with height: 5 and width: 4. It
will be compressed as two 4x4 blocks, where the unused pixels in the last block
are ignored. The final compressed size will be 2 * 8 = 16 bytes. Calculating the
stride from this would yield 16 / 5 = 3.2.

Since compression works on 4x4 blocks it could make more sense to think of a row
as 4 pixels high. In that case the above image would be interpreted as two rows
with a stride of 8 bytes. I am not sure if that fits within the current model
though.

I fear that supporting images with a width and height that's not evenly
dividable by 4 might require more significant changes after all.

What do you think of all this? Is it feasible?

[reveman, 2014/12/16 16:40:12]

Thanks for explaining.

Not supporting some dimensions with compressed formats sgtm. Such that
GpuMemoryBuffer::GetStride can always return a value that makes sense. We just
need to make sure we reject any attempts to allocate buffers with invalid
dimensions in the browser process correctly.

So how about we change this function to StrideInBytes with a "switch (format)"
statement and appropriate DCHECKs? Btw, looks like a BitsPerPixel function is
not really necessary and StrideInBytes alone is sufficient.

[christiank, 2014/12/18 13:58:29]

Sounds good to me. I have now replaced PixelsToBytes and BitsPerPixel with a new
function StrideInBytes.

I guess the format specific checks will come in the follow up review when I add
support for compressed formats.

 
   // Overridden from gfx::GpuMemoryBuffer:
   bool IsMapped() const override;