Revert "gpu: Map entire shared memory region for plane backings"

This reverts commit 542b8dd.

Reason for revert: Needed to revert an earlier CL.

Bug: b/264689730, 1408794
Change-Id: I95493153fa85d0e17033b5dc23c0fde967ba40d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189504
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#599}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

gpu/command_buffer/service/shared_image/compound_image_backing.cc

@@ -309,33 +309,39 @@ std::unique_ptr<SharedImageBacking> CompoundImageBacking::CreateSharedMemory(
     bool allow_shm_overlays,
     const Mailbox& mailbox,
     gfx::GpuMemoryBufferHandle handle,
-    gfx::BufferFormat format,
+    gfx::BufferFormat buffer_format,
     gfx::BufferPlane plane,
     const gfx::Size& size,
     const gfx::ColorSpace& color_space,
     GrSurfaceOrigin surface_origin,
     SkAlphaType alpha_type,
     uint32_t usage) {
-  DCHECK(IsValidSharedMemoryBufferFormat(size, format, plane));
+  DCHECK(IsValidSharedMemoryBufferFormat(size, buffer_format, plane));
+
+  const gfx::Size plane_size = GetPlaneSize(plane, size);
+  const viz::ResourceFormat plane_format =
+      viz::GetResourceFormat(GetPlaneBufferFormat(plane, buffer_format));
+
+  const size_t plane_index = GetPlaneIndex(plane, buffer_format);
+  handle.offset +=
+      gfx::BufferOffsetForBufferFormat(size, buffer_format, plane_index);
 
   SharedMemoryRegionWrapper shm_wrapper;
-  if (!shm_wrapper.Initialize(handle, size, format, plane)) {
+  if (!shm_wrapper.Initialize(handle, plane_size, plane_format)) {
     DLOG(ERROR) << "Failed to create SharedMemoryRegionWrapper";
     return nullptr;
   }
 
-  const gfx::Size plane_size = GetPlaneSize(plane, size);
-  const auto plane_format = viz::SharedImageFormat::SinglePlane(
-      viz::GetResourceFormat(GetPlaneBufferFormat(plane, format)));
+  auto si_format = viz::SharedImageFormat::SinglePlane(plane_format);
 
   auto shm_backing = std::make_unique<SharedMemoryImageBacking>(
-      mailbox, plane_format, plane_size, color_space, surface_origin,
-      alpha_type, SHARED_IMAGE_USAGE_CPU_WRITE, std::move(shm_wrapper));
+      mailbox, si_format, plane_size, color_space, surface_origin, alpha_type,
+      SHARED_IMAGE_USAGE_CPU_WRITE, std::move(shm_wrapper));
   shm_backing->SetNotRefCounted();
 
   return base::WrapUnique(new CompoundImageBacking(
-      mailbox, plane_format, plane_size, color_space, surface_origin,
-      alpha_type, usage, allow_shm_overlays, std::move(shm_backing),
+      mailbox, si_format, plane_size, color_space, surface_origin, alpha_type,
+      usage, allow_shm_overlays, std::move(shm_backing),
       gpu_backing_factory->GetWeakPtr()));
 }
 

gpu/command_buffer/service/shared_image/shared_image_factory.cc

@@ -449,12 +449,10 @@ bool SharedImageFactory::CreateSharedImage(const Mailbox& mailbox,
 
   std::unique_ptr<SharedImageBacking> backing;
   if (use_compound) {
-    // Only allow shmem overlays for NV12 on Windows.
 #if BUILDFLAG(IS_WIN)
-    const bool allow_shm_overlays =
-        format == gfx::BufferFormat::YUV_420_BIPLANAR;
+    constexpr bool allow_shm_overlays = true;
 #else
-    const bool allow_shm_overlays = false;
+    constexpr bool allow_shm_overlays = false;
 #endif
     backing = CompoundImageBacking::CreateSharedMemory(
         factory, allow_shm_overlays, mailbox, std::move(handle), format, plane,

gpu/command_buffer/service/shared_image/shared_memory_image_backing_factory.cc

@@ -60,11 +60,12 @@ SharedMemoryImageBackingFactory::CreateSharedImage(
     SkAlphaType alpha_type,
     uint32_t usage) {
   DCHECK(handle.type == gfx::SHARED_MEMORY_BUFFER);
+  viz::ResourceFormat format = viz::GetResourceFormat(buffer_format);
   SharedMemoryRegionWrapper shm_wrapper;
-  if (!shm_wrapper.Initialize(handle, size, buffer_format, plane)) {
+  if (!shm_wrapper.Initialize(handle, size, format)) {
     return nullptr;
   }
-  const auto format = viz::GetResourceFormat(buffer_format);
+
   auto backing = std::make_unique<SharedMemoryImageBacking>(
       mailbox, viz::SharedImageFormat::SinglePlane(format), size, color_space,
       surface_origin, alpha_type, usage, std::move(shm_wrapper));

gpu/command_buffer/service/shared_memory_region_wrapper.cc

@@ -7,34 +7,48 @@
 #include "base/logging.h"
 #include "base/numerics/checked_math.h"
 #include "base/system/sys_info.h"
-#include "gpu/command_buffer/common/gpu_memory_buffer_support.h"
-#include "ui/gfx/buffer_format_util.h"
+#include "components/viz/common/resources/resource_format_utils.h"
+#include "components/viz/common/resources/resource_sizes.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 
 namespace gpu {
 namespace {
 
 // Validate that |stride| will work for pixels with |size| and |format|.
 bool ValidateStride(const gfx::Size size,
-                    gfx::BufferFormat format,
+                    viz::ResourceFormat format,
                     uint32_t stride) {
   if (!base::IsValueInRangeForNumericType<size_t>(stride))
     return false;
 
-  // Use plane index 0 since we can't handle different plane strides anyway.
-  size_t alignment = gfx::RowByteAlignmentForBufferFormat(format, /*plane=*/0);
-  if (stride % alignment != 0)
-    return false;
-
-  size_t min_width_in_bytes = 0;
-  if (!gfx::RowSizeForBufferFormatChecked(size.width(), format, /*plane=*/0,
-                                          &min_width_in_bytes)) {
+  uint32_t min_width_in_bytes = 0;
+  if (!viz::ResourceSizes::MaybeWidthInBytes(size.width(), format,
+                                             &min_width_in_bytes)) {
     return false;
   }
 
   if (stride < min_width_in_bytes)
     return false;
 
+  // Check that stride is a multiple of pixel byte size.
+  int bits_per_pixel = viz::BitsPerPixel(format);
+  switch (bits_per_pixel) {
+    case 64:
+    case 32:
+    case 16:
+      if (stride % (bits_per_pixel / 8) != 0)
+        return false;
+      break;
+    case 8:
+    case 4:
+      break;
+    default:
+      // YVU420, YUV_420_BIPLANAR, and YUVA_420_TRIPLANAR format aren't
+      // supported.
+      NOTREACHED();
+      return false;
+  }
+
   return true;
 }
 
@@ -50,8 +64,7 @@ SharedMemoryRegionWrapper::~SharedMemoryRegionWrapper() = default;
 bool SharedMemoryRegionWrapper::Initialize(
     const gfx::GpuMemoryBufferHandle& handle,
     const gfx::Size& size,
-    gfx::BufferFormat format,
-    gfx::BufferPlane plane) {
+    viz::ResourceFormat format) {
   DCHECK(!mapping_.IsValid());
 
   if (!handle.region.IsValid()) {
@@ -64,49 +77,30 @@ bool SharedMemoryRegionWrapper::Initialize(
     return false;
   }
 
-  size_t buffer_size;
-  if (!gfx::BufferSizeForBufferFormatChecked(size, format, &buffer_size)) {
-    DLOG(ERROR) << "Invalid GMB size.";
-    return false;
-  }
-
   // Minimize the amount of address space we use but make sure offset is a
   // multiple of page size as required by MapAt().
-  // TODO(sunnyps): This doesn't seem to be a requirement of MapAt() anymore.
-  const size_t allocation_granularity =
-      base::SysInfo::VMAllocationGranularity();
-  const size_t memory_offset = handle.offset % allocation_granularity;
-  const size_t map_offset =
+  size_t allocation_granularity = base::SysInfo::VMAllocationGranularity();
+  size_t memory_offset = handle.offset % allocation_granularity;
+  size_t map_offset =
       allocation_granularity * (handle.offset / allocation_granularity);
 
-  base::CheckedNumeric<size_t> checked_size = buffer_size;
+  base::CheckedNumeric<size_t> checked_size = handle.stride;
+  checked_size *= size.height();
   checked_size += memory_offset;
   if (!checked_size.IsValid()) {
-    DLOG(ERROR) << "Invalid shared memory region map size.";
+    DLOG(ERROR) << "Invalid GMB size.";
     return false;
   }
 
-  const size_t map_size = checked_size.ValueOrDie();
-  mapping_ = handle.region.MapAt(static_cast<off_t>(map_offset), map_size);
+  mapping_ = handle.region.MapAt(static_cast<off_t>(map_offset),
+                                 checked_size.ValueOrDie());
+
   if (!mapping_.IsValid()) {
     DLOG(ERROR) << "Failed to map shared memory.";
     return false;
   }
 
-  // Add plane offset separately so that we map the entire buffer even if we're
-  // accessing an individual plane - this helps with shared memory overlays on
-  // Windows by allowing access via the Y plane shared image only.
-  const size_t plane_index = GetPlaneIndex(plane, format);
-  const size_t plane_offset =
-      gfx::BufferOffsetForBufferFormat(size, format, plane_index);
-#if DCHECK_IS_ON()
-  const gfx::Size plane_size = GetPlaneSize(plane, size);
-  const size_t plane_size_bytes =
-      gfx::PlaneSizeForBufferFormat(plane_size, format, plane_index);
-  DCHECK_LE(memory_offset + plane_offset + plane_size_bytes, map_size);
-#endif
-
-  offset_ = memory_offset + plane_offset;
+  offset_ = memory_offset;
   stride_ = handle.stride;
 
   return true;

gpu/command_buffer/service/shared_memory_region_wrapper.h

@@ -8,7 +8,7 @@
 #include "base/containers/span.h"
 #include "base/memory/shared_memory_mapping.h"
 #include "base/unguessable_token.h"
-#include "ui/gfx/buffer_types.h"
+#include "components/viz/common/resources/resource_format.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace gfx {
@@ -31,8 +31,7 @@ class SharedMemoryRegionWrapper {
   // until destruction.
   bool Initialize(const gfx::GpuMemoryBufferHandle& handle,
                   const gfx::Size& size,
-                  gfx::BufferFormat format,
-                  gfx::BufferPlane plane);
+                  viz::ResourceFormat format);
 
   bool IsValid() const;
   uint8_t* GetMemory() const;

ui/gfx/buffer_format_util.h

@@ -29,11 +29,6 @@ GFX_EXPORT size_t NumberOfPlanesForLinearBufferFormat(BufferFormat format);
 GFX_EXPORT size_t SubsamplingFactorForBufferFormat(BufferFormat format,
                                                    size_t plane);
 
-// Returns the alignment requirement to store a row of the given zero-indexed
-// |plane| of |format|.
-GFX_EXPORT size_t RowByteAlignmentForBufferFormat(BufferFormat format,
-                                                  size_t plane);
-
 // Returns the number of bytes used to store a row of the given zero-indexed
 // |plane| of |format|.
 GFX_EXPORT size_t RowSizeForBufferFormat(size_t width,