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}

gpu/command_buffer/service/raster_decoder.cc

@@ -572,6 +572,20 @@ class RasterDecoderImpl final : public RasterDecoder,
                                     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 @@ void RasterDecoderImpl::DoCopySubTextureINTERNALSkia(
     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 @@ void RasterDecoderImpl::DoCopySubTextureINTERNALSkia(
     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 @@ void RasterDecoderImpl::DoCopySubTextureINTERNALSkia(
   }
 }
 
+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,

gpu/command_buffer/service/shared_image_backing.cc

@@ -96,6 +96,12 @@ SharedImageBacking::ProduceVASurface(SharedImageManager* manager,
   return nullptr;
 }
 
+std::unique_ptr<SharedImageRepresentationMemory>
+SharedImageBacking::ProduceMemory(SharedImageManager* manager,
+                                  MemoryTypeTracker* tracker) {
+  return nullptr;
+}
+
 void SharedImageBacking::AddRef(SharedImageRepresentation* representation) {
   AutoLock auto_lock(this);
 

gpu/command_buffer/service/shared_image_backing.h

@@ -51,6 +51,7 @@ class SharedImageRepresentationGLTexturePassthrough;
 class SharedImageRepresentationSkia;
 class SharedImageRepresentationDawn;
 class SharedImageRepresentationOverlay;
+class SharedImageRepresentationMemory;
 class SharedImageRepresentationVaapi;
 class MemoryTypeTracker;
 class SharedImageFactory;
@@ -173,6 +174,9 @@ class GPU_GLES2_EXPORT SharedImageBacking {
       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_);

gpu/command_buffer/service/shared_image_backing_gl_image.cc

@@ -532,6 +532,37 @@ SharedImageBackingGLImage::ProduceSkia(
       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,

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 @@ class SharedImageRepresentationOverlayImpl
   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 @@ class GPU_GLES2_EXPORT SharedImageBackingGLImage
       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;

gpu/command_buffer/service/shared_image_factory.cc

@@ -622,4 +622,9 @@ SharedImageRepresentationFactory::ProduceOverlay(const gpu::Mailbox& mailbox) {
   return manager_->ProduceOverlay(mailbox, tracker_.get());
 }
 
+std::unique_ptr<SharedImageRepresentationMemory>
+SharedImageRepresentationFactory::ProduceMemory(const gpu::Mailbox& mailbox) {
+  return manager_->ProduceMemory(mailbox, tracker_.get());
+}
+
 }  // namespace gpu

gpu/command_buffer/service/shared_image_factory.h

@@ -212,6 +212,8 @@ class GPU_GLES2_EXPORT SharedImageRepresentationFactory {
       WGPUDevice device);
   std::unique_ptr<SharedImageRepresentationOverlay> ProduceOverlay(
       const Mailbox& mailbox);
+  std::unique_ptr<SharedImageRepresentationMemory> ProduceMemory(
+      const Mailbox& mailbox);
 
  private:
   SharedImageManager* const manager_;

gpu/command_buffer/service/shared_image_manager.cc

@@ -297,6 +297,24 @@ SharedImageManager::ProduceVASurface(const Mailbox& mailbox,
   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) {

gpu/command_buffer/service/shared_image_manager.h

@@ -68,6 +68,9 @@ class GPU_GLES2_EXPORT SharedImageManager {
       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,

gpu/command_buffer/service/shared_image_representation.cc

@@ -361,4 +361,20 @@ SharedImageRepresentationVaapi::BeginScopedWriteAccess() {
       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

gpu/command_buffer/service/shared_image_representation.h

@@ -463,6 +463,34 @@ class GPU_GLES2_EXPORT SharedImageRepresentationOverlay
   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 {

gpu/command_buffer/service/wrapped_sk_image.cc

@@ -138,12 +138,20 @@ class WrappedSkImage : public ClearTrackingSharedImageBacking {
 
   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 @@ class WrappedSkImage : public ClearTrackingSharedImageBacking {
   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 @@ class WrappedSkImageRepresentation : public SharedImageRepresentationSkia {
   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 @@ std::unique_ptr<SharedImageRepresentationSkia> WrappedSkImage::ProduceSkia(
     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