[//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; }