Import GLFence in EndOverlayAccess for AHB

When using SurfaceControl after surface has been returned we need to
wait until fence provided by system will be signaled. When using GL
for compositing, this is implemented as importing provided file
descriptor as egl fence and issue ServerWait().

GLRenderer does it when we get surface back. With SkiaRenderer it
happens on next BeginWrite for the SharedImage. Import is expensive
operation and BeginWrite is on critical path.

This CL moves import to EndOverlayAccess as we will do it anyway.
Then at BeginWrite time we only issue ServerWait().

In local tests with scrolling simple page it saves 200us in
DrawToSwapUs metric.

Change-Id: I6efb2783d0e287dc5e4fbb5707c507328e75a32f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2019607Reviewed-by: Jonathan Backer <backer@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735577}

gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer.cc

@@ -191,7 +191,8 @@ class SharedImageBackingAHB : public ClearTrackingSharedImageBacking {
                         base::android::ScopedHardwareBufferHandle handle,
                         size_t estimated_size,
                         bool is_thread_safe,
-                        base::ScopedFD initial_upload_fd);
+                        base::ScopedFD initial_upload_fd,
+                        bool using_gl);
 
   ~SharedImageBackingAHB() override;
 
@@ -213,6 +214,7 @@ class SharedImageBackingAHB : public ClearTrackingSharedImageBacking {
                base::ScopedFD end_read_fd);
   gl::GLImage* BeginOverlayAccess();
   void EndOverlayAccess();
+  std::unique_ptr<gl::GLFence> TakeReadGLFence();
 
  protected:
   std::unique_ptr<SharedImageRepresentationGLTexture> ProduceGLTexture(
@@ -245,8 +247,11 @@ class SharedImageBackingAHB : public ClearTrackingSharedImageBacking {
   base::flat_set<const SharedImageRepresentation*> active_readers_
       GUARDED_BY(lock_);
 
+  std::unique_ptr<gl::GLFence> read_gl_fence_ GUARDED_BY(lock_);
+
   scoped_refptr<OverlayImage> overlay_image_ GUARDED_BY(lock_);
   bool is_overlay_accessing_ GUARDED_BY(lock_) = false;
+  const bool using_gl_;
 
   DISALLOW_COPY_AND_ASSIGN(SharedImageBackingAHB);
 };
@@ -283,8 +288,15 @@ class SharedImageRepresentationGLTextureAHB
       if (!ahb_backing()->BeginWrite(&sync_fd))
         return false;
 
-      if (!InsertEglFenceAndWait(std::move(sync_fd)))
-        return false;
+      auto gl_fence = ahb_backing()->TakeReadGLFence();
+      if (gl_fence) {
+        gl_fence->ServerWait();
+      }
+
+      if (sync_fd.is_valid()) {
+        if (!InsertEglFenceAndWait(std::move(sync_fd)))
+          return false;
+      }
     }
 
     if (mode == GL_SHARED_IMAGE_ACCESS_MODE_READ_CHROMIUM) {
@@ -548,7 +560,8 @@ SharedImageBackingAHB::SharedImageBackingAHB(
     base::android::ScopedHardwareBufferHandle handle,
     size_t estimated_size,
     bool is_thread_safe,
-    base::ScopedFD initial_upload_fd)
+    base::ScopedFD initial_upload_fd,
+    bool using_gl)
     : ClearTrackingSharedImageBacking(mailbox,
                                       format,
                                       size,
@@ -557,7 +570,8 @@ SharedImageBackingAHB::SharedImageBackingAHB(
                                       estimated_size,
                                       is_thread_safe),
       hardware_buffer_handle_(std::move(handle)),
-      write_sync_fd_(std::move(initial_upload_fd)) {
+      write_sync_fd_(std::move(initial_upload_fd)),
+      using_gl_(using_gl) {
   DCHECK(hardware_buffer_handle_.is_valid());
 }
 
@@ -782,6 +796,15 @@ void SharedImageBackingAHB::EndOverlayAccess() {
 
   auto fence_fd = overlay_image_->TakeEndFence();
   read_sync_fd_ = gl::MergeFDs(std::move(read_sync_fd_), std::move(fence_fd));
+
+  if (using_gl_ && read_sync_fd_.is_valid()) {
+    read_gl_fence_ = CreateEglFence(std::move(read_sync_fd_));
+  }
+}
+
+std::unique_ptr<gl::GLFence> SharedImageBackingAHB::TakeReadGLFence() {
+  AutoLock auto_lock(this);
+  return std::move(read_gl_fence_);
 }
 
 gles2::Texture* SharedImageBackingAHB::GenGLTexture() {
@@ -843,7 +866,9 @@ gles2::Texture* SharedImageBackingAHB::GenGLTexture() {
 
 SharedImageBackingFactoryAHB::SharedImageBackingFactoryAHB(
     const GpuDriverBugWorkarounds& workarounds,
-    const GpuFeatureInfo& gpu_feature_info) {
+    const GpuFeatureInfo& gpu_feature_info,
+    SharedContextState* context_state)
+    : context_state_(context_state) {
   scoped_refptr<gles2::FeatureInfo> feature_info =
       new gles2::FeatureInfo(workarounds, gpu_feature_info);
   feature_info->Initialize(ContextType::CONTEXT_TYPE_OPENGLES2, false,
@@ -1057,9 +1082,11 @@ std::unique_ptr<SharedImageBacking> SharedImageBackingFactoryAHB::MakeBacking(
     initial_upload_fd = base::ScopedFD(fence);
   }
 
+  bool using_gl = context_state_ && context_state_->GrContextIsGL();
+
   auto backing = std::make_unique<SharedImageBackingAHB>(
       mailbox, format, size, color_space, usage, std::move(handle),
-      estimated_size, is_thread_safe, std::move(initial_upload_fd));
+      estimated_size, is_thread_safe, std::move(initial_upload_fd), using_gl);
 
   // If we uploaded initial data, set the backing as cleared.
   if (!pixel_data.empty())
@@ -1132,10 +1159,12 @@ SharedImageBackingFactoryAHB::CreateSharedImage(
     return nullptr;
   }
 
+  bool using_gl = context_state_ && context_state_->GrContextIsGL();
+
   return std::make_unique<SharedImageBackingAHB>(
       mailbox, resource_format, size, color_space, usage,
       std::move(handle.android_hardware_buffer), estimated_size, false,
-      base::ScopedFD());
+      base::ScopedFD(), using_gl);
 }
 
 }  // namespace gpu

gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer.h

@@ -17,6 +17,7 @@ class ColorSpace;
 }  // namespace gfx
 
 namespace gpu {
+class SharedContextState;
 class SharedImageBacking;
 class GpuDriverBugWorkarounds;
 struct GpuFeatureInfo;
@@ -28,7 +29,8 @@ class GPU_GLES2_EXPORT SharedImageBackingFactoryAHB
     : public SharedImageBackingFactory {
  public:
   SharedImageBackingFactoryAHB(const GpuDriverBugWorkarounds& workarounds,
-                               const GpuFeatureInfo& gpu_feature_info);
+                               const GpuFeatureInfo& gpu_feature_info,
+                               SharedContextState* context_state);
   ~SharedImageBackingFactoryAHB() override;
 
   // SharedImageBackingFactory implementation.
@@ -94,6 +96,8 @@ class GPU_GLES2_EXPORT SharedImageBackingFactoryAHB
   // Used to limit the max size of AHardwareBuffer.
   int32_t max_gl_texture_size_ = 0;
 
+  SharedContextState* const context_state_;
+
   DISALLOW_COPY_AND_ASSIGN(SharedImageBackingFactoryAHB);
 };
 

gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer_unittest.cc

@@ -65,7 +65,7 @@ class SharedImageBackingFactoryAHBTest : public testing::Test {
     context_state_->InitializeGL(GpuPreferences(), std::move(feature_info));
 
     backing_factory_ = std::make_unique<SharedImageBackingFactoryAHB>(
-        workarounds, GpuFeatureInfo());
+        workarounds, GpuFeatureInfo(), context_state_.get());
 
     memory_type_tracker_ = std::make_unique<MemoryTypeTracker>(nullptr);
     shared_image_representation_factory_ =

gpu/command_buffer/service/shared_image_factory.cc

@@ -98,7 +98,7 @@ SharedImageFactory::SharedImageFactory(
 #elif defined(OS_ANDROID) && BUILDFLAG(ENABLE_VULKAN)
   // For Android
   interop_backing_factory_ = std::make_unique<SharedImageBackingFactoryAHB>(
-      workarounds, gpu_feature_info);
+      workarounds, gpu_feature_info, context_state);
 #elif defined(OS_MACOSX)
   // OSX
   DCHECK(!using_vulkan_);

gpu/ipc/common/android/android_image_reader_utils.cc

@@ -51,16 +51,9 @@ bool DeleteAImageAsync(AImage* image,
   return true;
 }
 
-bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd) {
+std::unique_ptr<gl::GLFence> CreateEglFence(base::ScopedFD acquire_fence_fd) {
   int fence_fd = acquire_fence_fd.release();
 
-  // If fence_fd is -1, we do not need synchronization fence and image is ready
-  // to be used immediately. Also we dont need to close any fd. Else we need to
-  // create a sync fence which is used to signal when the buffer is ready to be
-  // consumed.
-  if (fence_fd == -1)
-    return true;
-
   // Create attribute-value list with the fence_fd.
   EGLint attribs[] = {EGL_SYNC_NATIVE_FENCE_FD_ANDROID, fence_fd, EGL_NONE};
 
@@ -72,16 +65,30 @@ bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd) {
 
   // If above method fails to create an egl_fence, we need to close the file
   // descriptor.
-  if (egl_fence == nullptr) {
+  if (!egl_fence) {
     // Create a scoped FD to close fence_fd.
     base::ScopedFD temp_fd(fence_fd);
     LOG(ERROR) << " Failed to created egl fence object ";
-    return false;
   }
 
-  // Make the server wait and not the client.
-  egl_fence->ServerWait();
-  return true;
+  return egl_fence;
+}
+
+bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd) {
+  // If acquire_fence_fd is not valid we do not need synchronization fence and
+  // image is ready to be used immediately. Also we dont need to close any fd.
+  // Else we need to create a sync fence which is used to signal when the buffer
+  // is ready to be consumed.
+  if (!acquire_fence_fd.is_valid())
+    return true;
+
+  auto egl_fence = CreateEglFence(std::move(acquire_fence_fd));
+  if (egl_fence) {
+    // Make the server wait and not the client.
+    egl_fence->ServerWait();
+    return true;
+  }
+  return false;
 }
 
 bool CreateAndBindEglImage(const AImage* image,

gpu/ipc/common/android/android_image_reader_utils.h

@@ -12,6 +12,10 @@
 #include "gpu/gpu_export.h"
 #include "ui/gl/gl_bindings.h"
 
+namespace gl {
+class GLFence;
+}
+
 namespace gpu {
 
 // Create and inserts an egl fence and exports a ScopedFD from it.
@@ -21,9 +25,14 @@ GPU_EXPORT base::ScopedFD CreateEglFenceAndExportFd();
 bool DeleteAImageAsync(AImage* image,
                        base::android::AndroidImageReader* loader);
 
-// Create and insert an EGL fence and imports the provided fence fd.
+// Create and insert an EGL fence and imports the provided fence fd and issues
+// ServerWait on it.
 GPU_EXPORT bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd);
 
+// Create and insert an EGL fence and imports the provided fence fd.
+GPU_EXPORT std::unique_ptr<gl::GLFence> CreateEglFence(
+    base::ScopedFD acquire_fence_fd);
+
 // Create an EGL image from the AImage via AHardwarebuffer. Bind this EGL image
 // to the texture target target_id. This changes the texture binding on the
 // current context.