Update GPU memory buffers to use StrideInBytes internally.

content/common/gpu/client/gpu_memory_buffer_impl.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 CONTENT_COMMON_GPU_CLIENT_GPU_MEMORY_BUFFER_IMPL_H_
 #define CONTENT_COMMON_GPU_CLIENT_GPU_MEMORY_BUFFER_IMPL_H_
 
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "content/common/content_export.h"
@@ skipping 15 matching lines @@
   // thread as this function was called on and instance was created on.
   static scoped_ptr<GpuMemoryBufferImpl> CreateFromHandle(
       const gfx::GpuMemoryBufferHandle& handle,
       const gfx::Size& size,
       Format format,
       const DestructionCallback& callback);
 
   // 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;
   Format GetFormat() const override;
   ClientBuffer AsClientBuffer() override;
 
   void set_destruction_sync_point(uint32 sync_point) {
     destruction_sync_point_ = sync_point;
   }
 
@@ skipping 10 matching lines @@
   bool mapped_;
   uint32 destruction_sync_point_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_COMMON_GPU_CLIENT_GPU_MEMORY_BUFFER_IMPL_H_