Prevent extra copy in RasterDecoder::CopySubTextureInternalSKIA
Currently when using OneCopyRasterBufferProvider we perform an upload
from CPU shared memory to the GPU and then a GPU to GPU copy during
raster playback. This CL allows direct upload of CPU shared memory
to the output shared image texture through the use of a new hint in
RasterDecoder::CopySubTexture that indicates CPU memory should be used
directly. This also allows us to unify how the copy is implemented
in RasterDecoder to always use Skia.
Bug: 984045
Change-Id: I6ac7d238cf3b2f43562481e2e1b36b3c4670a6f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517149
Commit-Queue: Nathan Zabriskie <nazabris@microsoft.com>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841147}
diff --git a/gpu/command_buffer/service/raster_decoder.cc b/gpu/command_buffer/service/raster_decoder.cc
index 80b68fa9..526399e 100644
--- a/gpu/command_buffer/service/raster_decoder.cc
+++ b/gpu/command_buffer/service/raster_decoder.cc
@@ -572,6 +572,20 @@
GLboolean unpack_flip_y,
const Mailbox& source_mailbox,
const Mailbox& dest_mailbox);
+ bool TryCopySubTextureINTERNALMemory(
+ GLint xoffset,
+ GLint yoffset,
+ GLint x,
+ GLint y,
+ GLsizei width,
+ GLsizei height,
+ gfx::Rect dest_cleared_rect,
+ GLboolean unpack_flip_y,
+ const Mailbox& source_mailbox,
+ SharedImageRepresentationSkia* dest_shared_image,
+ SharedImageRepresentationSkia::ScopedWriteAccess* dest_scoped_access,
+ const std::vector<GrBackendSemaphore>& begin_semaphores,
+ std::vector<GrBackendSemaphore>& end_semaphores);
void DoWritePixelsINTERNAL(GLint x_offset,
GLint y_offset,
GLuint src_width,
@@ -2258,24 +2272,13 @@
const Mailbox& dest_mailbox) {
DCHECK(source_mailbox != dest_mailbox);
- // Use Skia to copy texture if raster's gr_context() is not using GL.
- auto source_shared_image = shared_image_representation_factory_.ProduceSkia(
- source_mailbox, shared_context_state_);
auto dest_shared_image = shared_image_representation_factory_.ProduceSkia(
dest_mailbox, shared_context_state_);
- if (!source_shared_image || !dest_shared_image) {
+ if (!dest_shared_image) {
LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTexture", "unknown mailbox");
return;
}
- gfx::Size source_size = source_shared_image->size();
- gfx::Rect source_rect(x, y, width, height);
- if (!gfx::Rect(source_size).Contains(source_rect)) {
- LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTexture",
- "source texture bad dimensions.");
- return;
- }
-
gfx::Size dest_size = dest_shared_image->size();
gfx::Rect dest_rect(xoffset, yoffset, width, height);
if (!gfx::Rect(dest_size).Contains(dest_rect)) {
@@ -2312,12 +2315,31 @@
return;
}
- // With OneCopyRasterBufferProvider, source_shared_image->BeginReadAccess()
- // will copy pixels from SHM GMB to the texture in |source_shared_image|,
- // and then use drawImageRect() to draw that texure to the target
- // |dest_shared_image|. We can save one copy by drawing the SHM GMB to the
- // target |dest_shared_image| directly.
- // TODO(penghuang): get rid of the one extra copy. https://crbug.com/984045
+ // Attempt to upload directly from CPU shared memory to destination texture.
+ if (TryCopySubTextureINTERNALMemory(
+ xoffset, yoffset, x, y, width, height, new_cleared_rect,
+ unpack_flip_y, source_mailbox, dest_shared_image.get(),
+ dest_scoped_access.get(), begin_semaphores, end_semaphores)) {
+ return;
+ }
+
+ // Fall back to GPU->GPU copy if src image is not CPU-backed.
+ auto source_shared_image = shared_image_representation_factory_.ProduceSkia(
+ source_mailbox, shared_context_state_);
+ if (!source_shared_image) {
+ LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTexture",
+ "unknown source image mailbox.");
+ return;
+ }
+
+ gfx::Size source_size = source_shared_image->size();
+ gfx::Rect source_rect(x, y, width, height);
+ if (!gfx::Rect(source_size).Contains(source_rect)) {
+ LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTexture",
+ "source texture bad dimensions.");
+ return;
+ }
+
std::unique_ptr<SharedImageRepresentationSkia::ScopedReadAccess>
source_scoped_access = source_shared_image->BeginScopedReadAccess(
&begin_semaphores, &end_semaphores);
@@ -2358,6 +2380,59 @@
}
}
+bool RasterDecoderImpl::TryCopySubTextureINTERNALMemory(
+ GLint xoffset,
+ GLint yoffset,
+ GLint x,
+ GLint y,
+ GLsizei width,
+ GLsizei height,
+ gfx::Rect dest_cleared_rect,
+ GLboolean unpack_flip_y,
+ const Mailbox& source_mailbox,
+ SharedImageRepresentationSkia* dest_shared_image,
+ SharedImageRepresentationSkia::ScopedWriteAccess* dest_scoped_access,
+ const std::vector<GrBackendSemaphore>& begin_semaphores,
+ std::vector<GrBackendSemaphore>& end_semaphores) {
+ if (unpack_flip_y || x != 0 || y != 0)
+ return false;
+
+ auto source_shared_image =
+ shared_image_representation_factory_.ProduceMemory(source_mailbox);
+ if (!source_shared_image)
+ return false;
+
+ gfx::Size source_size = source_shared_image->size();
+ gfx::Rect source_rect(x, y, width, height);
+ if (!gfx::Rect(source_size).Contains(source_rect))
+ return false;
+
+ auto scoped_read_access = source_shared_image->BeginScopedReadAccess();
+ if (!scoped_read_access)
+ return false;
+
+ SkPixmap pm = scoped_read_access->pixmap();
+ if (pm.width() != source_rect.width() || pm.height() != source_rect.height())
+ return false;
+
+ if (!begin_semaphores.empty()) {
+ bool result = dest_scoped_access->surface()->wait(
+ begin_semaphores.size(), begin_semaphores.data(),
+ /*deleteSemaphoresAfterWait=*/false);
+ DCHECK(result);
+ }
+
+ dest_scoped_access->surface()->writePixels(pm, xoffset, yoffset);
+
+ FlushAndSubmitIfNecessary(dest_scoped_access->surface(),
+ std::move(end_semaphores));
+ if (!dest_shared_image->IsCleared()) {
+ dest_shared_image->SetClearedRect(dest_cleared_rect);
+ }
+
+ return true;
+}
+
void RasterDecoderImpl::DoWritePixelsINTERNAL(GLint x_offset,
GLint y_offset,
GLuint src_width,
diff --git a/gpu/command_buffer/service/shared_image_backing.cc b/gpu/command_buffer/service/shared_image_backing.cc
index f624824..9bc23bc 100644
--- a/gpu/command_buffer/service/shared_image_backing.cc
+++ b/gpu/command_buffer/service/shared_image_backing.cc
@@ -96,6 +96,12 @@
return nullptr;
}
+std::unique_ptr<SharedImageRepresentationMemory>
+SharedImageBacking::ProduceMemory(SharedImageManager* manager,
+ MemoryTypeTracker* tracker) {
+ return nullptr;
+}
+
void SharedImageBacking::AddRef(SharedImageRepresentation* representation) {
AutoLock auto_lock(this);
diff --git a/gpu/command_buffer/service/shared_image_backing.h b/gpu/command_buffer/service/shared_image_backing.h
index f14f558..8e6ba3a 100644
--- a/gpu/command_buffer/service/shared_image_backing.h
+++ b/gpu/command_buffer/service/shared_image_backing.h
@@ -51,6 +51,7 @@
class SharedImageRepresentationSkia;
class SharedImageRepresentationDawn;
class SharedImageRepresentationOverlay;
+class SharedImageRepresentationMemory;
class SharedImageRepresentationVaapi;
class MemoryTypeTracker;
class SharedImageFactory;
@@ -173,6 +174,9 @@
SharedImageManager* manager,
MemoryTypeTracker* tracker,
VaapiDependenciesFactory* dep_factory);
+ virtual std::unique_ptr<SharedImageRepresentationMemory> ProduceMemory(
+ SharedImageManager* manager,
+ MemoryTypeTracker* tracker);
// Used by subclasses during destruction.
bool have_context() const EXCLUSIVE_LOCKS_REQUIRED(lock_);
diff --git a/gpu/command_buffer/service/shared_image_backing_gl_image.cc b/gpu/command_buffer/service/shared_image_backing_gl_image.cc
index ac2540a..11c18a0 100644
--- a/gpu/command_buffer/service/shared_image_backing_gl_image.cc
+++ b/gpu/command_buffer/service/shared_image_backing_gl_image.cc
@@ -532,6 +532,37 @@
cached_promise_texture_, tracker);
}
+SharedImageRepresentationMemoryImpl::SharedImageRepresentationMemoryImpl(
+ SharedImageManager* manager,
+ SharedImageBacking* backing,
+ MemoryTypeTracker* tracker,
+ scoped_refptr<gl::GLImageMemory> image_memory)
+ : SharedImageRepresentationMemory(manager, backing, tracker),
+ image_memory_(std::move(image_memory)) {}
+
+SharedImageRepresentationMemoryImpl::~SharedImageRepresentationMemoryImpl() =
+ default;
+
+SkPixmap SharedImageRepresentationMemoryImpl::BeginReadAccess() {
+ SkImageInfo info = SkImageInfo::Make(
+ backing()->size().width(), backing()->size().height(),
+ viz::ResourceFormatToClosestSkColorType(true, backing()->format()),
+ backing()->alpha_type(), backing()->color_space().ToSkColorSpace());
+ return SkPixmap(info, image_memory_->memory(), image_memory_->stride());
+}
+
+std::unique_ptr<SharedImageRepresentationMemory>
+SharedImageBackingGLImage::ProduceMemory(SharedImageManager* manager,
+ MemoryTypeTracker* tracker) {
+ gl::GLImageMemory* image_memory =
+ gl::GLImageMemory::FromGLImage(image_.get());
+ if (!image_memory)
+ return nullptr;
+
+ return std::make_unique<SharedImageRepresentationMemoryImpl>(
+ manager, this, tracker, base::WrapRefCounted(image_memory));
+}
+
std::unique_ptr<SharedImageRepresentationGLTexture>
SharedImageBackingGLImage::ProduceRGBEmulationGLTexture(
SharedImageManager* manager,
diff --git a/gpu/command_buffer/service/shared_image_backing_gl_image.h b/gpu/command_buffer/service/shared_image_backing_gl_image.h
index dfc53eb..ba96fd86 100644
--- a/gpu/command_buffer/service/shared_image_backing_gl_image.h
+++ b/gpu/command_buffer/service/shared_image_backing_gl_image.h
@@ -9,6 +9,7 @@
#include "gpu/command_buffer/service/shared_image_backing_gl_common.h"
#include "gpu/gpu_gles2_export.h"
#include "ui/gl/gl_fence.h"
+#include "ui/gl/gl_image_memory.h"
namespace gpu {
@@ -143,6 +144,23 @@
scoped_refptr<gl::GLImage> gl_image_;
};
+class SharedImageRepresentationMemoryImpl
+ : public SharedImageRepresentationMemory {
+ public:
+ SharedImageRepresentationMemoryImpl(
+ SharedImageManager* manager,
+ SharedImageBacking* backing,
+ MemoryTypeTracker* tracker,
+ scoped_refptr<gl::GLImageMemory> image_memory);
+ ~SharedImageRepresentationMemoryImpl() override;
+
+ protected:
+ SkPixmap BeginReadAccess() override;
+
+ private:
+ scoped_refptr<gl::GLImageMemory> image_memory_;
+};
+
// Implementation of SharedImageBacking that creates a GL Texture that is backed
// by a GLImage and stores it as a gles2::Texture. Can be used with the legacy
// mailbox implementation.
@@ -200,6 +218,9 @@
SharedImageManager* manager,
MemoryTypeTracker* tracker,
scoped_refptr<SharedContextState> context_state) override;
+ std::unique_ptr<SharedImageRepresentationMemory> ProduceMemory(
+ SharedImageManager* manager,
+ MemoryTypeTracker* tracker) override;
std::unique_ptr<SharedImageRepresentationGLTexture>
ProduceRGBEmulationGLTexture(SharedImageManager* manager,
MemoryTypeTracker* tracker) override;
diff --git a/gpu/command_buffer/service/shared_image_factory.cc b/gpu/command_buffer/service/shared_image_factory.cc
index fbcc667..490c25bc 100644
--- a/gpu/command_buffer/service/shared_image_factory.cc
+++ b/gpu/command_buffer/service/shared_image_factory.cc
@@ -622,4 +622,9 @@
return manager_->ProduceOverlay(mailbox, tracker_.get());
}
+std::unique_ptr<SharedImageRepresentationMemory>
+SharedImageRepresentationFactory::ProduceMemory(const gpu::Mailbox& mailbox) {
+ return manager_->ProduceMemory(mailbox, tracker_.get());
+}
+
} // namespace gpu
diff --git a/gpu/command_buffer/service/shared_image_factory.h b/gpu/command_buffer/service/shared_image_factory.h
index f4668e1..19e2d3f 100644
--- a/gpu/command_buffer/service/shared_image_factory.h
+++ b/gpu/command_buffer/service/shared_image_factory.h
@@ -212,6 +212,8 @@
WGPUDevice device);
std::unique_ptr<SharedImageRepresentationOverlay> ProduceOverlay(
const Mailbox& mailbox);
+ std::unique_ptr<SharedImageRepresentationMemory> ProduceMemory(
+ const Mailbox& mailbox);
private:
SharedImageManager* const manager_;
diff --git a/gpu/command_buffer/service/shared_image_manager.cc b/gpu/command_buffer/service/shared_image_manager.cc
index 578b38c7..f056db50 100644
--- a/gpu/command_buffer/service/shared_image_manager.cc
+++ b/gpu/command_buffer/service/shared_image_manager.cc
@@ -297,6 +297,24 @@
return representation;
}
+std::unique_ptr<SharedImageRepresentationMemory>
+SharedImageManager::ProduceMemory(const Mailbox& mailbox,
+ MemoryTypeTracker* tracker) {
+ CALLED_ON_VALID_THREAD();
+
+ AutoLock autolock(this);
+ auto found = images_.find(mailbox);
+ if (found == images_.end()) {
+ LOG(ERROR) << "SharedImageManager::Producememory: Trying to Produce a "
+ "Memory representation from a non-existent mailbox.";
+ return nullptr;
+ }
+
+ // This is expected to fail based on the SharedImageBacking type, so don't log
+ // error here. Caller is expected to handle nullptr.
+ return (*found)->ProduceMemory(this, tracker);
+}
+
void SharedImageManager::OnRepresentationDestroyed(
const Mailbox& mailbox,
SharedImageRepresentation* representation) {
diff --git a/gpu/command_buffer/service/shared_image_manager.h b/gpu/command_buffer/service/shared_image_manager.h
index 86200088..f7c5249 100644
--- a/gpu/command_buffer/service/shared_image_manager.h
+++ b/gpu/command_buffer/service/shared_image_manager.h
@@ -68,6 +68,9 @@
const Mailbox& mailbox,
MemoryTypeTracker* ref,
VaapiDependenciesFactory* dep_factory);
+ std::unique_ptr<SharedImageRepresentationMemory> ProduceMemory(
+ const Mailbox& mailbox,
+ MemoryTypeTracker* ref);
// Called by SharedImageRepresentation in the destructor.
void OnRepresentationDestroyed(const Mailbox& mailbox,
diff --git a/gpu/command_buffer/service/shared_image_representation.cc b/gpu/command_buffer/service/shared_image_representation.cc
index 7a1311b..85af5f30 100644
--- a/gpu/command_buffer/service/shared_image_representation.cc
+++ b/gpu/command_buffer/service/shared_image_representation.cc
@@ -361,4 +361,20 @@
base::PassKey<SharedImageRepresentationVaapi>(), this);
}
+SharedImageRepresentationMemory::ScopedReadAccess::ScopedReadAccess(
+ base::PassKey<SharedImageRepresentationMemory> pass_key,
+ SharedImageRepresentationMemory* representation,
+ SkPixmap pixmap)
+ : ScopedAccessBase(representation), pixmap_(pixmap) {}
+
+SharedImageRepresentationMemory::ScopedReadAccess::~ScopedReadAccess() =
+ default;
+
+std::unique_ptr<SharedImageRepresentationMemory::ScopedReadAccess>
+SharedImageRepresentationMemory::BeginScopedReadAccess() {
+ return std::make_unique<ScopedReadAccess>(
+ base::PassKey<SharedImageRepresentationMemory>(), this,
+ BeginReadAccess());
+}
+
} // namespace gpu
diff --git a/gpu/command_buffer/service/shared_image_representation.h b/gpu/command_buffer/service/shared_image_representation.h
index 0ec7326f0..246394d 100644
--- a/gpu/command_buffer/service/shared_image_representation.h
+++ b/gpu/command_buffer/service/shared_image_representation.h
@@ -463,6 +463,34 @@
virtual gl::GLImage* GetGLImage() = 0;
};
+class GPU_GLES2_EXPORT SharedImageRepresentationMemory
+ : public SharedImageRepresentation {
+ public:
+ class GPU_GLES2_EXPORT ScopedReadAccess
+ : public ScopedAccessBase<SharedImageRepresentationMemory> {
+ public:
+ ScopedReadAccess(base::PassKey<SharedImageRepresentationMemory> pass_key,
+ SharedImageRepresentationMemory* representation,
+ SkPixmap pixmap);
+ ~ScopedReadAccess();
+
+ SkPixmap pixmap() { return pixmap_; }
+
+ private:
+ SkPixmap pixmap_;
+ };
+
+ SharedImageRepresentationMemory(SharedImageManager* manager,
+ SharedImageBacking* backing,
+ MemoryTypeTracker* tracker)
+ : SharedImageRepresentation(manager, backing, tracker) {}
+
+ std::unique_ptr<ScopedReadAccess> BeginScopedReadAccess();
+
+ protected:
+ virtual SkPixmap BeginReadAccess() = 0;
+};
+
// An interface that allows a SharedImageBacking to hold a reference to VA-API
// surface without depending on //media/gpu/vaapi targets.
class VaapiDependencies {
diff --git a/gpu/command_buffer/service/wrapped_sk_image.cc b/gpu/command_buffer/service/wrapped_sk_image.cc
index 3e4e42d..e4abe43 100644
--- a/gpu/command_buffer/service/wrapped_sk_image.cc
+++ b/gpu/command_buffer/service/wrapped_sk_image.cc
@@ -138,12 +138,20 @@
sk_sp<SkPromiseImageTexture> promise_texture() { return promise_texture_; }
+ const SharedMemoryRegionWrapper& shared_memory_wrapper() {
+ return shared_memory_wrapper_;
+ }
+
protected:
std::unique_ptr<SharedImageRepresentationSkia> ProduceSkia(
SharedImageManager* manager,
MemoryTypeTracker* tracker,
scoped_refptr<SharedContextState> context_state) override;
+ std::unique_ptr<SharedImageRepresentationMemory> ProduceMemory(
+ SharedImageManager* manager,
+ MemoryTypeTracker* tracker) override;
+
private:
friend class gpu::raster::WrappedSkImageFactory;
@@ -299,14 +307,14 @@
DISALLOW_COPY_AND_ASSIGN(WrappedSkImage);
};
-class WrappedSkImageRepresentation : public SharedImageRepresentationSkia {
+class WrappedSkImageRepresentationSkia : public SharedImageRepresentationSkia {
public:
- WrappedSkImageRepresentation(SharedImageManager* manager,
- SharedImageBacking* backing,
- MemoryTypeTracker* tracker)
+ WrappedSkImageRepresentationSkia(SharedImageManager* manager,
+ SharedImageBacking* backing,
+ MemoryTypeTracker* tracker)
: SharedImageRepresentationSkia(manager, backing, tracker) {}
- ~WrappedSkImageRepresentation() override { DCHECK(!write_surface_); }
+ ~WrappedSkImageRepresentationSkia() override { DCHECK(!write_surface_); }
sk_sp<SkSurface> BeginWriteAccess(
int final_msaa_count,
@@ -364,6 +372,29 @@
SkSurface* write_surface_ = nullptr;
};
+class WrappedSkImageRepresentationMemory
+ : public SharedImageRepresentationMemory {
+ public:
+ WrappedSkImageRepresentationMemory(SharedImageManager* manager,
+ SharedImageBacking* backing,
+ MemoryTypeTracker* tracker)
+ : SharedImageRepresentationMemory(manager, backing, tracker) {}
+
+ protected:
+ SkPixmap BeginReadAccess() override {
+ SkImageInfo info = MakeSkImageInfo(wrapped_sk_image()->size(),
+ wrapped_sk_image()->format());
+ return SkPixmap(info,
+ wrapped_sk_image()->shared_memory_wrapper().GetMemory(),
+ wrapped_sk_image()->shared_memory_wrapper().GetStride());
+ }
+
+ private:
+ WrappedSkImage* wrapped_sk_image() {
+ return static_cast<WrappedSkImage*>(backing());
+ }
+};
+
} // namespace
WrappedSkImageFactory::WrappedSkImageFactory(
@@ -462,7 +493,18 @@
return nullptr;
DCHECK_EQ(context_state_, context_state.get());
- return std::make_unique<WrappedSkImageRepresentation>(manager, this, tracker);
+ return std::make_unique<WrappedSkImageRepresentationSkia>(manager, this,
+ tracker);
+}
+
+std::unique_ptr<SharedImageRepresentationMemory> WrappedSkImage::ProduceMemory(
+ SharedImageManager* manager,
+ MemoryTypeTracker* tracker) {
+ if (!shared_memory_wrapper_.IsValid())
+ return nullptr;
+
+ return std::make_unique<WrappedSkImageRepresentationMemory>(manager, this,
+ tracker);
}
} // namespace raster
