Make compound backing work with GLTextureImageBacking

Make GLTextureImageBacking work with compound image backing and have it replace GLImageBacking for shared memory GMBs. GLTextureImageBackingFactory ignores scanout usage for shared memory GMBs to match existing behaviour in GLImageBackingFactory. This also removes the GLImageBackingFactory functionality to work with shared memory GMBs and having two instances of the factory. Unfortunately GLTextureImageBackingFactory needs two instances, one for regular GL textures and one for shared memory GMBs, to ensure we pick the right backing type on Windows. This should be a temporary state until usage flags to specify when D3D backing, eg. supporting CopyToGpuMemoryBuffer() and ProduceMemory() for overlay path, are needed get added. Bug: 1293509 Change-Id: I6eb071c8b27f92a2cbcafbb6145f4c05ce04c000 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3774213 Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org> Commit-Queue: Kyle Charbonneau <kylechar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1035129}
chromium · Aug 15, 2022 · 8d6fa86 · 8d6fa86
1 parent fe1a3b4
commit 8d6fa86
Show file tree

Hide file tree

Showing 14 changed files with 94 additions and 106 deletions.
diff --git a/gpu/command_buffer/service/shared_image/compound_image_backing.cc b/gpu/command_buffer/service/shared_image/compound_image_backing.cc
@@ -306,17 +306,19 @@ std::unique_ptr<SharedImageBacking> CompoundImageBacking::CreateSharedMemory(
   }
 
   auto shm_backing = std::make_unique<SharedMemoryImageBacking>(
-      gpu::Mailbox(), format, size, color_space, surface_origin, alpha_type,
+      mailbox, format, size, color_space, surface_origin, alpha_type,
       SHARED_IMAGE_USAGE_CPU_WRITE, std::move(shm_wrapper));
+  shm_backing->SetNotReferencedCounted();
 
   auto gpu_backing = gpu_backing_factory->CreateSharedImage(
-      gpu::Mailbox(), format, surface_handle, size, color_space, surface_origin,
+      mailbox, format, surface_handle, size, color_space, surface_origin,
       alpha_type, usage | SHARED_IMAGE_USAGE_CPU_UPLOAD,
       /*is_thread_safe=*/false);
   if (!gpu_backing) {
     DLOG(ERROR) << "Failed to create GPU backing";
     return nullptr;
   }
+  gpu_backing->SetNotReferencedCounted();
 
   return std::make_unique<CompoundImageBacking>(
       mailbox, format, size, color_space, surface_origin, alpha_type, usage,

diff --git a/gpu/command_buffer/service/shared_image/gl_image_backing_factory.cc b/gpu/command_buffer/service/shared_image/gl_image_backing_factory.cc
@@ -21,7 +21,6 @@
 #include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gl/buffer_format_utils.h"
-#include "ui/gl/gl_image_shared_memory.h"
 #include "ui/gl/progress_reporter.h"
 
 namespace gpu {
@@ -41,14 +40,12 @@ GLImageBackingFactory::GLImageBackingFactory(
     const GpuDriverBugWorkarounds& workarounds,
     const gles2::FeatureInfo* feature_info,
     ImageFactory* image_factory,
-    gl::ProgressReporter* progress_reporter,
-    const bool for_shared_memory_gmbs)
+    gl::ProgressReporter* progress_reporter)
     : GLCommonImageBackingFactory(gpu_preferences,
                                   workarounds,
                                   feature_info,
                                   progress_reporter),
-      image_factory_(image_factory),
-      for_shared_memory_gmbs_(for_shared_memory_gmbs) {
+      image_factory_(image_factory) {
   gpu_memory_buffer_formats_ =
       feature_info->feature_flags().gpu_memory_buffer_formats;
   // Return if scanout images are not supported
@@ -153,10 +150,8 @@ std::unique_ptr<SharedImageBacking> GLImageBackingFactory::CreateSharedImage(
     return nullptr;
   }
 
-  const gfx::GpuMemoryBufferType handle_type = handle.type;
   GLenum target =
-      (handle_type == gfx::SHARED_MEMORY_BUFFER ||
-       !NativeBufferNeedsPlatformSpecificTextureTarget(buffer_format, plane))
+      !NativeBufferNeedsPlatformSpecificTextureTarget(buffer_format, plane)
           ? GL_TEXTURE_2D
           : gpu::GetPlatformSpecificTextureTarget();
   scoped_refptr<gl::GLImage> image =
@@ -181,8 +176,8 @@ std::unique_ptr<SharedImageBacking> GLImageBackingFactory::CreateSharedImage(
   texture_2d_support =
       (gpu::GetPlatformSpecificTextureTarget() == GL_TEXTURE_2D);
 #endif  // BUILDFLAG(IS_MAC)
-  DCHECK(handle_type == gfx::SHARED_MEMORY_BUFFER || target != GL_TEXTURE_2D ||
-         texture_2d_support || image->ShouldBindOrCopy() == gl::GLImage::BIND);
+  DCHECK(target != GL_TEXTURE_2D || texture_2d_support ||
+         image->ShouldBindOrCopy() == gl::GLImage::BIND);
 #endif  // DCHECK_IS_ON()
   if (usage & SHARED_IMAGE_USAGE_MACOS_VIDEO_TOOLBOX)
     image->DisableInUseByWindowServer();
@@ -221,20 +216,6 @@ scoped_refptr<gl::GLImage> GLImageBackingFactory::MakeGLImage(
     gfx::BufferPlane plane,
     SurfaceHandle surface_handle,
     const gfx::Size& size) {
-  if (handle.type == gfx::SHARED_MEMORY_BUFFER) {
-    if (plane != gfx::BufferPlane::DEFAULT)
-      return nullptr;
-    auto image = base::MakeRefCounted<gl::GLImageSharedMemory>(size);
-    if (color_space.IsValid())
-      image->SetColorSpace(color_space);
-    if (!image->Initialize(handle.region, handle.id, format, handle.offset,
-                           handle.stride)) {
-      return nullptr;
-    }
-
-    return image;
-  }
-
   if (!image_factory_)
     return nullptr;
 
@@ -256,10 +237,8 @@ bool GLImageBackingFactory::IsSupported(uint32_t usage,
   if (thread_safe) {
     return false;
   }
-  // If the GLImage factory is created specifically for SHARED_MEMORY Gmbs,
-  // make sure that it used for that purpose based on flag
-  if ((for_shared_memory_gmbs_ && gmb_type != gfx::SHARED_MEMORY_BUFFER) ||
-      (!for_shared_memory_gmbs_ && gmb_type == gfx::SHARED_MEMORY_BUFFER)) {
+  // Never used with shared memory GMBs.
+  if (gmb_type == gfx::SHARED_MEMORY_BUFFER) {
     return false;
   }
   if (usage & SHARED_IMAGE_USAGE_CPU_UPLOAD) {

diff --git a/gpu/command_buffer/service/shared_image/gl_image_backing_factory.h b/gpu/command_buffer/service/shared_image/gl_image_backing_factory.h
@@ -34,17 +34,14 @@ class ImageFactory;
 class GPU_GLES2_EXPORT GLImageBackingFactory
     : public GLCommonImageBackingFactory {
  public:
-  // for_shared_memory_gmbs is a temporary parameter which is used for checking
-  // if gfx::SHARED_MEMORY_BUFFER is supported by the factory.
   // It is used for migrating GLImage backing, for part that works with
   // SharedMemory GMB with SharedMemoryImageBacking and Composite backings, and
   // all other parts with OzoneImageBacking and other backings.
   GLImageBackingFactory(const GpuPreferences& gpu_preferences,
                         const GpuDriverBugWorkarounds& workarounds,
                         const gles2::FeatureInfo* feature_info,
                         ImageFactory* image_factory,
-                        gl::ProgressReporter* progress_reporter,
-                        const bool for_shared_memory_gmbs);
+                        gl::ProgressReporter* progress_reporter);
   ~GLImageBackingFactory() override;
 
   // SharedImageBackingFactory implementation.
@@ -121,9 +118,6 @@ class GPU_GLES2_EXPORT GLImageBackingFactory
   // Factory used to generate GLImages for SCANOUT backings.
   const raw_ptr<ImageFactory> image_factory_ = nullptr;
 
-  // Whether factory is specifically for SHARED_MEMORY Gmbs
-  const bool for_shared_memory_gmbs_ = false;
-
   BufferFormatInfo buffer_format_info_[viz::RESOURCE_FORMAT_MAX + 1];
   GpuMemoryBufferFormatSet gpu_memory_buffer_formats_;
 };

diff --git a/gpu/command_buffer/service/shared_image/gl_image_backing_factory_unittest.cc b/gpu/command_buffer/service/shared_image/gl_image_backing_factory_unittest.cc
@@ -109,10 +109,7 @@ class GLImageBackingFactoryTestBase
     preferences.use_passthrough_cmd_decoder = use_passthrough();
     backing_factory_ = std::make_unique<GLImageBackingFactory>(
         preferences, workarounds, context_state_->feature_info(), factory,
-        &progress_reporter_, /*for_shared_memory_gmbs=*/false);
-    backing_factory_shmem_ = std::make_unique<GLImageBackingFactory>(
-        preferences, workarounds, context_state_->feature_info(), factory,
-        &progress_reporter_, /*for_shared_memory_gmbs=*/true);
+        &progress_reporter_);
 
     memory_type_tracker_ = std::make_unique<MemoryTypeTracker>(nullptr);
     shared_image_representation_factory_ =
@@ -142,7 +139,6 @@ class GLImageBackingFactoryTestBase
   scoped_refptr<gl::GLContext> context_;
   scoped_refptr<SharedContextState> context_state_;
   std::unique_ptr<GLImageBackingFactory> backing_factory_;
-  std::unique_ptr<GLImageBackingFactory> backing_factory_shmem_;
   gles2::MailboxManagerImpl mailbox_manager_;
   std::unique_ptr<SharedImageManager> shared_image_manager_;
   std::unique_ptr<MemoryTypeTracker> memory_type_tracker_;
@@ -751,44 +747,6 @@ TEST_P(GLImageBackingFactoryWithGMBTest, GpuMemoryBufferImportNative) {
   EXPECT_GT(stub_image->update_counter(), update_counter);
 }
 
-TEST_P(GLImageBackingFactoryWithGMBTest, GpuMemoryBufferImportSharedMemory) {
-  auto mailbox = Mailbox::GenerateForSharedImage();
-  gfx::Size size(256, 256);
-  gfx::BufferFormat format = viz::BufferFormat(get_format());
-  auto color_space = gfx::ColorSpace::CreateSRGB();
-  GrSurfaceOrigin surface_origin = kTopLeft_GrSurfaceOrigin;
-  SkAlphaType alpha_type = kPremul_SkAlphaType;
-  uint32_t usage = SHARED_IMAGE_USAGE_GLES2;
-
-  size_t shm_size = 0u;
-  ASSERT_TRUE(gfx::BufferSizeForBufferFormatChecked(size, format, &shm_size));
-  gfx::GpuMemoryBufferHandle handle;
-  handle.type = gfx::SHARED_MEMORY_BUFFER;
-  handle.region = base::UnsafeSharedMemoryRegion::Create(shm_size);
-  ASSERT_TRUE(handle.region.IsValid());
-  handle.offset = 0;
-  handle.stride = static_cast<uint32_t>(
-      gfx::RowSizeForBufferFormat(size.width(), format, 0));
-
-  auto backing = backing_factory_shmem_->CreateSharedImage(
-      mailbox, kClientId, std::move(handle), format, gfx::BufferPlane::DEFAULT,
-      kNullSurfaceHandle, size, color_space, surface_origin, alpha_type, usage);
-  if (!can_create_scanout_or_gmb_shared_image(get_format())) {
-    EXPECT_FALSE(backing);
-    return;
-  }
-  ASSERT_TRUE(backing);
-
-  std::unique_ptr<SharedImageRepresentationFactoryRef> ref =
-      shared_image_manager_->Register(std::move(backing),
-                                      memory_type_tracker_.get());
-  scoped_refptr<gl::GLImage> image = GetImageFromMailbox(mailbox);
-  ASSERT_EQ(image->GetType(), gl::GLImage::Type::MEMORY);
-  auto* shm_image = static_cast<gl::GLImageSharedMemory*>(image.get());
-  EXPECT_EQ(size, shm_image->GetSize());
-  EXPECT_EQ(format, shm_image->format());
-}
-
 TEST_P(GLImageBackingFactoryWithGMBTest,
        GpuMemoryBufferImportNative_WithRGBEmulation) {
   if (use_passthrough())

diff --git a/gpu/command_buffer/service/shared_image/gl_texture_image_backing.cc b/gpu/command_buffer/service/shared_image/gl_texture_image_backing.cc
@@ -42,6 +42,7 @@
 #include "ui/gl/gl_image_native_pixmap.h"
 #include "ui/gl/gl_image_shared_memory.h"
 #include "ui/gl/gl_implementation.h"
+#include "ui/gl/gl_utils.h"
 #include "ui/gl/gl_version_info.h"
 #include "ui/gl/scoped_binders.h"
 #include "ui/gl/scoped_make_current.h"
@@ -159,6 +160,30 @@ void GLTextureImageBacking::SetClearedRect(const gfx::Rect& cleared_rect) {
   ClearTrackingSharedImageBacking::SetClearedRect(cleared_rect);
 }
 
+void GLTextureImageBacking::Update(std::unique_ptr<gfx::GpuFence> in_fence) {}
+
+bool GLTextureImageBacking::UploadFromMemory(const SkPixmap& pixmap) {
+  DCHECK(gl::GLContext::GetCurrent());
+
+  const GLuint texture_id = GetGLServiceId();
+  const GLenum gl_format = texture_params_.format;
+  const GLenum gl_type = texture_params_.type;
+  const GLenum gl_target = texture_params_.target;
+
+  gl::GLApi* api = gl::g_current_gl_context;
+  gl::ScopedTextureBinder scoped_texture_binder(gl_target, texture_id);
+  gl::ScopedPixelStore unpack_row_length(GL_UNPACK_ROW_LENGTH, 0);
+  gl::ScopedPixelStore unpack_skip_pixels(GL_UNPACK_SKIP_PIXELS, 0);
+  gl::ScopedPixelStore unpack_skip_rows(GL_UNPACK_SKIP_ROWS, 0);
+  gl::ScopedPixelStore unpack_alignment(GL_UNPACK_ALIGNMENT, 4);
+
+  api->glTexSubImage2DFn(gl_target, 0, 0, 0, size().width(), size().height(),
+                         gl_format, gl_type, pixmap.addr());
+  DCHECK_EQ(api->glGetErrorFn(), static_cast<GLenum>(GL_NO_ERROR));
+
+  return true;
+}
+
 bool GLTextureImageBacking::ProduceLegacyMailbox(
     MailboxManager* mailbox_manager) {
   if (IsPassthrough())
@@ -231,16 +256,14 @@ std::unique_ptr<SkiaImageRepresentation> GLTextureImageBacking::ProduceSkia(
       tracker);
 }
 
-void GLTextureImageBacking::Update(std::unique_ptr<gfx::GpuFence> in_fence) {}
-
 void GLTextureImageBacking::InitializeGLTexture(
     GLuint service_id,
     const InitializeGLTextureParams& params) {
   GLTextureImageBackingHelper::MakeTextureAndSetParameters(
       params.target, service_id, params.framebuffer_attachment_angle,
       IsPassthrough() ? &passthrough_texture_ : nullptr,
       IsPassthrough() ? nullptr : &texture_);
-
+  texture_params_ = params;
   if (IsPassthrough()) {
     passthrough_texture_->SetEstimatedSize(EstimatedSize(format(), size()));
     SetClearedRect(params.is_cleared ? gfx::Rect(size()) : gfx::Rect());

diff --git a/gpu/command_buffer/service/shared_image/gl_texture_image_backing.h b/gpu/command_buffer/service/shared_image/gl_texture_image_backing.h
@@ -65,13 +65,16 @@ class GLTextureImageBacking : public ClearTrackingSharedImageBacking {
       MemoryTypeTracker* tracker,
       scoped_refptr<SharedContextState> context_state) override;
   void Update(std::unique_ptr<gfx::GpuFence> in_fence) override;
+  bool UploadFromMemory(const SkPixmap& pixmap) override;
 
   bool IsPassthrough() const { return is_passthrough_; }
 
   const bool is_passthrough_;
   gles2::Texture* texture_ = nullptr;
   scoped_refptr<gles2::TexturePassthrough> passthrough_texture_;
 
+  GLTextureImageBackingHelper::InitializeGLTextureParams texture_params_;
+
   sk_sp<SkPromiseImageTexture> cached_promise_texture_;
   scoped_refptr<gl::GLImageEGL> image_egl_;
 };

diff --git a/gpu/command_buffer/service/shared_image/gl_texture_image_backing_factory.cc b/gpu/command_buffer/service/shared_image/gl_texture_image_backing_factory.cc
@@ -37,11 +37,13 @@ GLTextureImageBackingFactory::GLTextureImageBackingFactory(
     const GpuPreferences& gpu_preferences,
     const GpuDriverBugWorkarounds& workarounds,
     const gles2::FeatureInfo* feature_info,
-    gl::ProgressReporter* progress_reporter)
+    gl::ProgressReporter* progress_reporter,
+    bool for_cpu_upload_usage)
     : GLCommonImageBackingFactory(gpu_preferences,
                                   workarounds,
                                   feature_info,
-                                  progress_reporter) {}
+                                  progress_reporter),
+      for_cpu_upload_usage_(for_cpu_upload_usage) {}
 
 GLTextureImageBackingFactory::~GLTextureImageBackingFactory() = default;
 
@@ -134,10 +136,19 @@ bool GLTextureImageBackingFactory::IsSupported(
     return false;
   }
 
-  constexpr uint32_t kInvalidUsages = SHARED_IMAGE_USAGE_VIDEO_DECODE |
-                                      SHARED_IMAGE_USAGE_SCANOUT |
-                                      SHARED_IMAGE_USAGE_CPU_UPLOAD;
+  bool has_cpu_upload_usage = usage & SHARED_IMAGE_USAGE_CPU_UPLOAD;
 
+  if (for_cpu_upload_usage_ != has_cpu_upload_usage)
+    return false;
+
+  if (has_cpu_upload_usage) {
+    // Drop scanout usage for shared memory GMBs to match legacy behaviour
+    // from GLImageBackingFactory.
+    usage = usage & ~SHARED_IMAGE_USAGE_SCANOUT;
+  }
+
+  constexpr uint32_t kInvalidUsages =
+      SHARED_IMAGE_USAGE_VIDEO_DECODE | SHARED_IMAGE_USAGE_SCANOUT;
   if (usage & kInvalidUsages) {
     return false;
   }

diff --git a/gpu/command_buffer/service/shared_image/gl_texture_image_backing_factory.h b/gpu/command_buffer/service/shared_image/gl_texture_image_backing_factory.h
@@ -31,10 +31,14 @@ struct Mailbox;
 class GPU_GLES2_EXPORT GLTextureImageBackingFactory
     : public GLCommonImageBackingFactory {
  public:
+  // The `for_cpu_upload_usage` parameter controls if this factory accepts
+  // `SHARED_IMAGE_USAGE_CPU_UPLOAD`. It is strict, if true the usage must
+  // include CPU upload and if false it must not.
   GLTextureImageBackingFactory(const GpuPreferences& gpu_preferences,
                                const GpuDriverBugWorkarounds& workarounds,
                                const gles2::FeatureInfo* feature_info,
-                               gl::ProgressReporter* progress_reporter);
+                               gl::ProgressReporter* progress_reporter,
+                               bool for_cpu_upload_usage);
   ~GLTextureImageBackingFactory() override;
 
   // SharedImageBackingFactory implementation.
@@ -97,6 +101,8 @@ class GPU_GLES2_EXPORT GLTextureImageBackingFactory
       SkAlphaType alpha_type,
       uint32_t usage,
       base::span<const uint8_t> pixel_data);
+
+  const bool for_cpu_upload_usage_;
 };
 
 }  // namespace gpu

diff --git a/gpu/command_buffer/service/shared_image/gl_texture_image_backing_factory_unittest.cc b/gpu/command_buffer/service/shared_image/gl_texture_image_backing_factory_unittest.cc
@@ -105,7 +105,7 @@ class GLTextureImageBackingFactoryTestBase
     preferences.use_passthrough_cmd_decoder = use_passthrough();
     backing_factory_ = std::make_unique<GLTextureImageBackingFactory>(
         preferences, workarounds, context_state_->feature_info(),
-        &progress_reporter_);
+        &progress_reporter_, /*for_cpu_upload_usage=*/false);
 
     memory_type_tracker_ = std::make_unique<MemoryTypeTracker>(nullptr);
     shared_image_representation_factory_ =

diff --git a/gpu/command_buffer/service/shared_image/iosurface_image_backing_factory_unittest.cc b/gpu/command_buffer/service/shared_image/iosurface_image_backing_factory_unittest.cc
@@ -71,7 +71,7 @@ class IOSurfaceImageBackingFactoryTest : public testing::Test {
     backing_factory_ = std::make_unique<GLImageBackingFactory>(
         preferences, workarounds, context_state_->feature_info(),
         &image_factory_,
-        /*progress_reporter=*/nullptr, /*for_shared_memory_gmbs=*/false);
+        /*progress_reporter=*/nullptr);
 
     memory_type_tracker_ = std::make_unique<MemoryTypeTracker>(nullptr);
     shared_image_representation_factory_ =

diff --git a/gpu/command_buffer/service/shared_image/shared_image_backing.cc b/gpu/command_buffer/service/shared_image/shared_image_backing.cc
@@ -177,8 +177,14 @@ SharedImageBacking::ProduceLegacyOverlay(SharedImageManager* manager,
 }
 #endif
 
+void SharedImageBacking::SetNotReferencedCounted() {
+  DCHECK(!HasAnyRefs());
+  is_reference_counted_ = false;
+}
+
 void SharedImageBacking::AddRef(SharedImageRepresentation* representation) {
   AutoLock auto_lock(this);
+  DCHECK(is_reference_counted_);
 
   bool first_ref = refs_.empty();
   refs_.push_back(representation);
@@ -190,6 +196,7 @@ void SharedImageBacking::AddRef(SharedImageRepresentation* representation) {
 
 void SharedImageBacking::ReleaseRef(SharedImageRepresentation* representation) {
   AutoLock auto_lock(this);
+  DCHECK(is_reference_counted_);
 
   auto found = std::find(refs_.begin(), refs_.end(), representation);
   DCHECK(found != refs_.end());