[go: nahoru, domu]

[//cc] Add MappableSI flow to ZeroCopyRasterBufferProvider

This CL adds a flow that uses MappableSharedImage rather than
GpuMemoryBuffer to ZeroCopyRasterBufferProvider. The new codepath is
enabled by default with a Finch killswitch. The changes for the new
flow are the following:

- Rather than creating and mapping a GMB, create and map a MappableSI
- If the MappableSI fails to map, immediately destroy it and zero out
  its mailbox.
- Where the code currently checks for failure by the GMB being null,
  check for failure via the mailbox being zero
- Dump the memory of the mapped region via ScopedMapping rather than the
  GMB

Note: As part of this change, we had to disable ZeroCopyRaster unittests
on Fuchsia due to crbug.com/1485883, a pre-existing issue that this
change tickled due to the change to the tests creating buffers via
(Mappable)SharedImage rather than TestGpuMemoryBufferManager.

Bug: 1431326, 1485883
Change-Id: I7a9875a748a42ca606c8b20d0c671f9531de8e82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4881760
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1200997}
diff --git a/cc/raster/zero_copy_raster_buffer_provider.cc b/cc/raster/zero_copy_raster_buffer_provider.cc
index 9284f9c..eef08c3b 100644
--- a/cc/raster/zero_copy_raster_buffer_provider.cc
+++ b/cc/raster/zero_copy_raster_buffer_provider.cc
@@ -9,6 +9,7 @@
 #include <algorithm>
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/memory/raw_ptr.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
@@ -29,6 +30,10 @@
 namespace cc {
 namespace {
 
+BASE_FEATURE(kAlwaysUseMappableSIForZeroCopyRaster,
+             "AlwaysUseMappableSIForZeroCopyRaster",
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
 constexpr static auto kBufferUsage = gfx::BufferUsage::GPU_READ_CPU_READ_WRITE;
 
 // Subclass for InUsePoolResource that holds ownership of a zero-copy backing
@@ -49,10 +54,23 @@
       const base::trace_event::MemoryAllocatorDumpGuid& buffer_dump_guid,
       uint64_t tracing_process_id,
       int importance) const override {
-    if (!gpu_memory_buffer)
-      return;
-    gpu_memory_buffer->OnMemoryDump(pmd, buffer_dump_guid, tracing_process_id,
-                                    importance);
+    if (base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster)) {
+      if (mailbox.IsZero()) {
+        return;
+      }
+      auto mapping = shared_image_interface->MapSharedImage(mailbox);
+      if (!mapping) {
+        return;
+      }
+      mapping->OnMemoryDump(pmd, buffer_dump_guid, tracing_process_id,
+                            importance);
+    } else {
+      if (!gpu_memory_buffer) {
+        return;
+      }
+      gpu_memory_buffer->OnMemoryDump(pmd, buffer_dump_guid, tracing_process_id,
+                                      importance);
+    }
   }
 
   // The SharedImageInterface used to clean up the shared image.
@@ -81,12 +99,20 @@
   ZeroCopyRasterBufferImpl(const ZeroCopyRasterBufferImpl&) = delete;
 
   ~ZeroCopyRasterBufferImpl() override {
-    // If GpuMemoryBuffer allocation failed (https://crbug.com/554541), then
-    // we don't have anything to give to the display compositor, so we report a
-    // zero mailbox that will result in checkerboarding.
-    if (!gpu_memory_buffer_) {
-      DCHECK(backing_->mailbox.IsZero());
-      return;
+    // If MapSharedImage() or GpuMemoryBuffer allocation failed
+    // (https://crbug.com/554541), then we don't have anything to give to the
+    // display compositor, so we report a zero mailbox that will result in
+    // checkerboarding.
+    if (base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster)) {
+      CHECK(!gpu_memory_buffer_);
+      if (backing_->mailbox.IsZero()) {
+        return;
+      }
+    } else {
+      if (!gpu_memory_buffer_) {
+        DCHECK(backing_->mailbox.IsZero());
+        return;
+      }
     }
 
     // This is destroyed on the compositor thread when raster is complete, but
@@ -96,6 +122,8 @@
     // we need to do things in IsResourceReadyToDraw() and OrderingBarrier then?
     gpu::SharedImageInterface* sii = backing_->shared_image_interface;
     if (backing_->mailbox.IsZero()) {
+      CHECK(
+          !base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster));
       uint32_t usage = gpu::SHARED_IMAGE_USAGE_DISPLAY_READ |
                        gpu::SHARED_IMAGE_USAGE_SCANOUT;
       // Make a mailbox for export of the GpuMemoryBuffer to the display
@@ -124,32 +152,68 @@
                 const GURL& url) override {
     TRACE_EVENT0("cc", "ZeroCopyRasterBuffer::Playback");
 
-    if (!gpu_memory_buffer_) {
-      gpu_memory_buffer_ = gpu_memory_buffer_manager_->CreateGpuMemoryBuffer(
-          resource_size_,
-          viz::SinglePlaneSharedImageFormatToBufferFormat(format_),
-          kBufferUsage, gpu::kNullSurfaceHandle, shutdown_event_);
-      // Note that GpuMemoryBuffer allocation can fail.
-      // https://crbug.com/554541
-      if (!gpu_memory_buffer_)
-        return;
-    }
+    std::unique_ptr<gpu::SharedImageInterface::ScopedMapping> mapping;
+    void* memory = nullptr;
+    size_t stride = 0;
 
-    CHECK_EQ(1u, gfx::NumberOfPlanesForLinearBufferFormat(
-                     gpu_memory_buffer_->GetFormat()));
-    bool rv = gpu_memory_buffer_->Map();
-    CHECK(rv);
-    CHECK(gpu_memory_buffer_->memory(0));
-    // RasterBufferProvider::PlaybackToMemory only supports unsigned strides.
-    CHECK_GE(gpu_memory_buffer_->stride(0), 0);
+    if (base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster)) {
+      CHECK(!gpu_memory_buffer_);
+
+      gpu::SharedImageInterface* sii = backing_->shared_image_interface;
+
+      // Create a MappableSI if necessary.
+      if (backing_->mailbox.IsZero()) {
+        uint32_t usage = gpu::SHARED_IMAGE_USAGE_DISPLAY_READ |
+                         gpu::SHARED_IMAGE_USAGE_SCANOUT;
+        backing_->mailbox = sii->CreateSharedImage(
+            format_, resource_size_, resource_color_space_,
+            kTopLeft_GrSurfaceOrigin, kPremul_SkAlphaType, usage,
+            "ZeroCopyRasterTile", gpu::kNullSurfaceHandle, kBufferUsage);
+      }
+
+      mapping = sii->MapSharedImage(backing_->mailbox);
+      if (!mapping) {
+        LOG(ERROR) << "MapSharedImage Failed.";
+        sii->DestroySharedImage(gpu::SyncToken(), backing_->mailbox);
+        backing_->mailbox.SetZero();
+        return;
+      }
+      memory = mapping->Memory(0);
+      stride = mapping->Stride(0);
+    } else {
+      if (!gpu_memory_buffer_) {
+        gpu_memory_buffer_ = gpu_memory_buffer_manager_->CreateGpuMemoryBuffer(
+            resource_size_,
+            viz::SinglePlaneSharedImageFormatToBufferFormat(format_),
+            kBufferUsage, gpu::kNullSurfaceHandle, shutdown_event_);
+        // Note that GpuMemoryBuffer allocation can fail.
+        // https://crbug.com/554541
+        if (!gpu_memory_buffer_) {
+          return;
+        }
+      }
+
+      CHECK_EQ(1u, gfx::NumberOfPlanesForLinearBufferFormat(
+                       gpu_memory_buffer_->GetFormat()));
+      bool rv = gpu_memory_buffer_->Map();
+      CHECK(rv);
+      CHECK(gpu_memory_buffer_->memory(0));
+      // RasterBufferProvider::PlaybackToMemory only supports unsigned strides.
+      CHECK_GE(gpu_memory_buffer_->stride(0), 0);
+
+      memory = gpu_memory_buffer_->memory(0);
+      stride = gpu_memory_buffer_->stride(0);
+    }
 
     // TODO(danakj): Implement partial raster with raster_dirty_rect.
     RasterBufferProvider::PlaybackToMemory(
-        gpu_memory_buffer_->memory(0), format_, resource_size_,
-        gpu_memory_buffer_->stride(0), raster_source, raster_full_rect,
-        raster_full_rect, transform, resource_color_space_,
+        memory, format_, resource_size_, stride, raster_source,
+        raster_full_rect, raster_full_rect, transform, resource_color_space_,
         /*gpu_compositing=*/true, playback_settings);
-    gpu_memory_buffer_->Unmap();
+
+    base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster)
+        ? mapping.reset()
+        : gpu_memory_buffer_->Unmap();
   }
 
   bool SupportsBackgroundThreadPriority() const override { return true; }