viz/cros: Use fence when scheduling overlay as primary plane

Currently, enabling SkiaRenderer on Chrome OS causes the entire
screen to flicker due to partial enablement of HW overlays [1].
To remedy these synchronization issues, we need to schedule the
primary plane overlay with a valid fence.

On Android, this is accomplished via platform-specific code that
extracts a fence from a GLImage [2][3]. In this CL, we take a more
robust approach by creating the fence on OverlayRepresentation read.
The OutputPresenter can subsequently use this fence if it exists/
the SharedImage type supports it.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2342262
[2] https://source.chromium.org/chromium/chromium/src/+/master:gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer.cc;l=75;drc=23f61cb65a94208dc2c4728e895e87d47f64a8b6
[3] https://source.chromium.org/chromium/chromium/src/+/master:ui/gl/gl_surface_egl_surface_control.cc;l=323;drc=5c860faff8acca32094b87f229b630519326372e

Bug: 1114290
Test: CrOS with SkiaRenderer doesn't flicker
Change-Id: Ie429ef6dab66fce18d6b3f37133c4bb9ecf602cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2404148Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Brian Ho <hob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809768}

components/viz/service/display_embedder/output_presenter_gl.cc

@@ -134,8 +134,11 @@ int PresenterImageGL::present_count() const {
 
 gl::GLImage* PresenterImageGL::GetGLImage(
     std::unique_ptr<gfx::GpuFence>* fence) {
-  if (scoped_overlay_read_access_)
+  if (scoped_overlay_read_access_) {
+    if (fence)
+      *fence = scoped_overlay_read_access_->TakeFence();
     return scoped_overlay_read_access_->gl_image();
+  }
 
   DCHECK(scoped_gl_read_access_);
 
@@ -314,7 +317,8 @@ void OutputPresenterGL::SchedulePrimaryPlane(
   std::unique_ptr<gfx::GpuFence> fence;
   // If the submitted_image() is being scheduled, we don't new a new fence.
   auto* gl_image = reinterpret_cast<PresenterImageGL*>(image)->GetGLImage(
-      is_submitted ? nullptr : &fence);
+      (is_submitted || !gl_surface_->SupportsPlaneGpuFences()) ? nullptr
+                                                               : &fence);
 
   // Output surface is also z-order 0.
   constexpr int kPlaneZOrder = 0;
@@ -356,7 +360,7 @@ void OutputPresenterGL::ScheduleOverlays(
       gl_surface_->ScheduleOverlayPlane(
           overlay.plane_z_order, overlay.transform, gl_image,
           ToNearestRect(overlay.display_rect), overlay.uv_rect,
-          !overlay.is_opaque, nullptr /* gpu_fence */);
+          !overlay.is_opaque, accesses[i]->TakeFence());
     }
 #elif defined(OS_APPLE)
     gl_surface_->ScheduleCALayer(ui::CARendererLayerParams(

gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer.cc

@@ -240,6 +240,7 @@ class SharedImageRepresentationOverlayAHB
   }
 
   gl::GLImage* GetGLImage() override { return gl_image_; }
+  std::unique_ptr<gfx::GpuFence> GetReadFence() override { return nullptr; }
 
   gl::GLImage* gl_image_ = nullptr;
 };

gpu/command_buffer/service/shared_image_backing_gl_image.cc

@@ -73,14 +73,20 @@ gles2::Texture* SharedImageRepresentationGLTextureImpl::GetTexture() {
 }
 
 bool SharedImageRepresentationGLTextureImpl::BeginAccess(GLenum mode) {
+  DCHECK(mode_ == 0);
+  mode_ = mode;
   if (client_ && mode != GL_SHARED_IMAGE_ACCESS_MODE_OVERLAY_CHROMIUM)
     return client_->SharedImageRepresentationGLTextureBeginAccess();
   return true;
 }
 
 void SharedImageRepresentationGLTextureImpl::EndAccess() {
+  DCHECK(mode_ != 0);
+  GLenum current_mode = mode_;
+  mode_ = 0;
   if (client_)
-    return client_->SharedImageRepresentationGLTextureEndAccess();
+    return client_->SharedImageRepresentationGLTextureEndAccess(
+        current_mode != GL_SHARED_IMAGE_ACCESS_MODE_READWRITE_CHROMIUM);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -111,14 +117,20 @@ SharedImageRepresentationGLTexturePassthroughImpl::GetTexturePassthrough() {
 
 bool SharedImageRepresentationGLTexturePassthroughImpl::BeginAccess(
     GLenum mode) {
+  DCHECK(mode_ == 0);
+  mode_ = mode;
   if (client_ && mode != GL_SHARED_IMAGE_ACCESS_MODE_OVERLAY_CHROMIUM)
     return client_->SharedImageRepresentationGLTextureBeginAccess();
   return true;
 }
 
 void SharedImageRepresentationGLTexturePassthroughImpl::EndAccess() {
+  DCHECK(mode_ != 0);
+  GLenum current_mode = mode_;
+  mode_ = 0;
   if (client_)
-    return client_->SharedImageRepresentationGLTextureEndAccess();
+    return client_->SharedImageRepresentationGLTextureEndAccess(
+        current_mode != GL_SHARED_IMAGE_ACCESS_MODE_READWRITE_CHROMIUM);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -191,7 +203,7 @@ void SharedImageRepresentationSkiaImpl::EndWriteAccess(
   write_surface_ = nullptr;
 
   if (client_)
-    client_->SharedImageRepresentationGLTextureEndAccess();
+    client_->SharedImageRepresentationGLTextureEndAccess(false /* readonly */);
 }
 
 sk_sp<SkPromiseImageTexture> SharedImageRepresentationSkiaImpl::BeginReadAccess(
@@ -208,7 +220,7 @@ sk_sp<SkPromiseImageTexture> SharedImageRepresentationSkiaImpl::BeginReadAccess(
 
 void SharedImageRepresentationSkiaImpl::EndReadAccess() {
   if (client_)
-    client_->SharedImageRepresentationGLTextureEndAccess();
+    client_->SharedImageRepresentationGLTextureEndAccess(true /* readonly */);
 }
 
 bool SharedImageRepresentationSkiaImpl::SupportsMultipleConcurrentReadAccess() {
@@ -246,6 +258,12 @@ gl::GLImage* SharedImageRepresentationOverlayImpl::GetGLImage() {
   return gl_image_.get();
 }
 
+std::unique_ptr<gfx::GpuFence>
+SharedImageRepresentationOverlayImpl::GetReadFence() {
+  auto* gl_backing = static_cast<SharedImageBackingGLImage*>(backing());
+  return gl_backing->GetLastWriteGpuFence();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // SharedImageBackingGLImage
 
@@ -359,6 +377,11 @@ GLuint SharedImageBackingGLImage::GetGLServiceId() const {
   return 0;
 }
 
+std::unique_ptr<gfx::GpuFence>
+SharedImageBackingGLImage::GetLastWriteGpuFence() {
+  return last_write_gl_fence_ ? last_write_gl_fence_->GetGpuFence() : nullptr;
+}
+
 scoped_refptr<gfx::NativePixmap> SharedImageBackingGLImage::GetNativePixmap() {
   return image_->GetNativePixmap();
 }
@@ -563,7 +586,8 @@ bool SharedImageBackingGLImage::
   return BindOrCopyImageIfNeeded();
 }
 
-void SharedImageBackingGLImage::SharedImageRepresentationGLTextureEndAccess() {
+void SharedImageBackingGLImage::SharedImageRepresentationGLTextureEndAccess(
+    bool readonly) {
 #if defined(OS_MAC)
   // If this image could potentially be shared with Metal via WebGPU, then flush
   // the GL context to ensure Metal will see it.
@@ -587,6 +611,14 @@ void SharedImageBackingGLImage::SharedImageRepresentationGLTextureEndAccess() {
       image_bind_or_copy_needed_ = true;
     }
   }
+#else
+  // If the image will be used for an overlay, we insert a fence that can be
+  // used by OutputPresenter to synchronize image writes with presentation.
+  if (!readonly && usage() & SHARED_IMAGE_USAGE_SCANOUT &&
+      gl::GLFence::IsGpuFenceSupported()) {
+    last_write_gl_fence_ = gl::GLFence::CreateForGpuFence();
+    DCHECK(last_write_gl_fence_);
+  }
 #endif
 }
 

gpu/command_buffer/service/shared_image_backing_gl_image.h

@@ -8,6 +8,7 @@
 #include "gpu/command_buffer/service/shared_image_backing.h"
 #include "gpu/command_buffer/service/shared_image_backing_gl_common.h"
 #include "gpu/gpu_gles2_export.h"
+#include "ui/gl/gl_fence.h"
 
 namespace gpu {
 
@@ -16,7 +17,7 @@ namespace gpu {
 class SharedImageRepresentationGLTextureClient {
  public:
   virtual bool SharedImageRepresentationGLTextureBeginAccess() = 0;
-  virtual void SharedImageRepresentationGLTextureEndAccess() = 0;
+  virtual void SharedImageRepresentationGLTextureEndAccess(bool readonly) = 0;
   virtual void SharedImageRepresentationGLTextureRelease(bool have_context) = 0;
 };
 
@@ -41,6 +42,7 @@ class SharedImageRepresentationGLTextureImpl
 
   SharedImageRepresentationGLTextureClient* const client_ = nullptr;
   gles2::Texture* texture_;
+  GLenum mode_ = 0;
 };
 
 // Representation of a SharedImageBackingGLTexture or
@@ -69,6 +71,7 @@ class SharedImageRepresentationGLTexturePassthroughImpl
 
   SharedImageRepresentationGLTextureClient* const client_ = nullptr;
   scoped_refptr<gles2::TexturePassthrough> texture_passthrough_;
+  GLenum mode_ = 0;
 };
 
 // Skia representation for both SharedImageBackingGLCommon.
@@ -131,6 +134,7 @@ class SharedImageRepresentationOverlayImpl
   bool BeginReadAccess() override;
   void EndReadAccess() override;
   gl::GLImage* GetGLImage() override;
+  std::unique_ptr<gfx::GpuFence> GetReadFence() override;
 
   scoped_refptr<gl::GLImage> gl_image_;
 };
@@ -163,6 +167,7 @@ class GPU_GLES2_EXPORT SharedImageBackingGLImage
 
   GLenum GetGLTarget() const;
   GLuint GetGLServiceId() const;
+  std::unique_ptr<gfx::GpuFence> GetLastWriteGpuFence();
 
  private:
   // SharedImageBacking:
@@ -198,7 +203,7 @@ class GPU_GLES2_EXPORT SharedImageBackingGLImage
 
   // SharedImageRepresentationGLTextureClient:
   bool SharedImageRepresentationGLTextureBeginAccess() override;
-  void SharedImageRepresentationGLTextureEndAccess() override;
+  void SharedImageRepresentationGLTextureEndAccess(bool readonly) override;
   void SharedImageRepresentationGLTextureRelease(bool have_context) override;
 
   bool IsPassthrough() const { return is_passthrough_; }
@@ -228,6 +233,7 @@ class GPU_GLES2_EXPORT SharedImageBackingGLImage
   scoped_refptr<gles2::TexturePassthrough> passthrough_texture_;
 
   sk_sp<SkPromiseImageTexture> cached_promise_texture_;
+  std::unique_ptr<gl::GLFence> last_write_gl_fence_;
 
   base::WeakPtrFactory<SharedImageBackingGLImage> weak_factory_;
 };

gpu/command_buffer/service/shared_image_representation.cc

@@ -8,6 +8,7 @@
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "third_party/skia/include/core/SkPromiseImageTexture.h"
 #include "third_party/skia/include/gpu/GrDirectContext.h"
+#include "ui/gl/gl_fence.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/scoped_hardware_buffer_fence_sync.h"
@@ -227,8 +228,15 @@ sk_sp<SkPromiseImageTexture> SharedImageRepresentationSkia::BeginReadAccess(
 SharedImageRepresentationOverlay::ScopedReadAccess::ScopedReadAccess(
     util::PassKey<SharedImageRepresentationOverlay> pass_key,
     SharedImageRepresentationOverlay* representation,
-    gl::GLImage* gl_image)
-    : ScopedAccessBase(representation), gl_image_(gl_image) {}
+    gl::GLImage* gl_image,
+    std::unique_ptr<gfx::GpuFence> fence)
+    : ScopedAccessBase(representation),
+      gl_image_(gl_image),
+      fence_(std::move(fence)) {}
+
+SharedImageRepresentationOverlay::ScopedReadAccess::~ScopedReadAccess() {
+  representation()->EndReadAccess();
+}
 
 std::unique_ptr<SharedImageRepresentationOverlay::ScopedReadAccess>
 SharedImageRepresentationOverlay::BeginScopedReadAccess(bool needs_gl_image) {
@@ -244,7 +252,7 @@ SharedImageRepresentationOverlay::BeginScopedReadAccess(bool needs_gl_image) {
 
   return std::make_unique<ScopedReadAccess>(
       util::PassKey<SharedImageRepresentationOverlay>(), this,
-      needs_gl_image ? GetGLImage() : nullptr);
+      needs_gl_image ? GetGLImage() : nullptr, GetReadFence());
 }
 
 SharedImageRepresentationDawn::ScopedAccess::ScopedAccess(

gpu/command_buffer/service/shared_image_representation.h

@@ -392,20 +392,22 @@ class GPU_GLES2_EXPORT SharedImageRepresentationOverlay
                                    MemoryTypeTracker* tracker)
       : SharedImageRepresentation(manager, backing, tracker) {}
 
-  class ScopedReadAccess
+  class GPU_GLES2_EXPORT ScopedReadAccess
       : public ScopedAccessBase<SharedImageRepresentationOverlay> {
    public:
     ScopedReadAccess(util::PassKey<SharedImageRepresentationOverlay> pass_key,
                      SharedImageRepresentationOverlay* representation,
-                     gl::GLImage* gl_image);
-    ~ScopedReadAccess() { representation()->EndReadAccess(); }
+                     gl::GLImage* gl_image,
+                     std::unique_ptr<gfx::GpuFence> fence);
+    ~ScopedReadAccess();
 
-    gl::GLImage* gl_image() const {
-      return gl_image_;
-    }
+    gl::GLImage* gl_image() const { return gl_image_; }
+
+    std::unique_ptr<gfx::GpuFence> TakeFence() { return std::move(fence_); }
 
    private:
     gl::GLImage* gl_image_;
+    std::unique_ptr<gfx::GpuFence> fence_;
   };
 
 #if defined(OS_ANDROID)
@@ -426,6 +428,9 @@ class GPU_GLES2_EXPORT SharedImageRepresentationOverlay
   // TODO(penghuang): Refactor it to not depend on GL.
   // Get the backing as GLImage for GLSurface::ScheduleOverlayPlane.
   virtual gl::GLImage* GetGLImage() = 0;
+  // Optionally returns a fence to synchronize writes on the SharedImage with
+  // overlay presentation.
+  virtual std::unique_ptr<gfx::GpuFence> GetReadFence() = 0;
 };
 
 // An interface that allows a SharedImageBacking to hold a reference to VA-API

gpu/command_buffer/service/shared_image_representation_d3d.cc

@@ -150,4 +150,9 @@ gl::GLImage* SharedImageRepresentationOverlayD3D::GetGLImage() {
   return static_cast<SharedImageBackingD3D*>(backing())->GetGLImage();
 }
 
+std::unique_ptr<gfx::GpuFence>
+SharedImageRepresentationOverlayD3D::GetReadFence() {
+  return nullptr;
+}
+
 }  // namespace gpu

gpu/command_buffer/service/shared_image_representation_d3d.h

@@ -81,6 +81,7 @@ class SharedImageRepresentationOverlayD3D
   void EndReadAccess() override;
 
   gl::GLImage* GetGLImage() override;
+  std::unique_ptr<gfx::GpuFence> GetReadFence() override;
 };
 
 }  // namespace gpu

gpu/command_buffer/service/shared_image_video.cc

@@ -433,6 +433,8 @@ class SharedImageRepresentationOverlayVideo
     return stream_image_.get();
   }
 
+  std::unique_ptr<gfx::GpuFence> GetReadFence() override { return nullptr; }
+
   void NotifyOverlayPromotion(bool promotion,
                               const gfx::Rect& bounds) override {
     stream_image_->NotifyOverlayPromotion(promotion, bounds);

gpu/command_buffer/service/test_shared_image_backing.cc

@@ -118,6 +118,7 @@ class TestSharedImageRepresentationOverlay
   bool BeginReadAccess() override { return true; }
   void EndReadAccess() override {}
   gl::GLImage* GetGLImage() override { return nullptr; }
+  std::unique_ptr<gfx::GpuFence> GetReadFence() override { return nullptr; }
 
 #if defined(OS_ANDROID)
   void NotifyOverlayPromotion(bool promotion,