[Fuchsia] Synchronize primary plane and overlay updates

This CL merges acquire fences for primary plane and overlays so
that the updates happen at the same time. We could see artifacts
when they were out of sync, as the underlay strategy expects a
perfect fit under transparent layer. This CL makes sure that the
updates happen in sync.

Bug: 1144320, 1127984
Change-Id: I4b6bc776e0f3c2c8b0d6c4811245cc2cbcc3b9fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514465Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824202}

components/viz/service/display_embedder/output_presenter_fuchsia.cc

@@ -44,6 +44,20 @@ void GrSemaphoresToZxEvents(gpu::VulkanImplementation* vulkan_implementation,
   }
 }
 
+// Duplicates the given zx::events and stores in gfx::GpuFences.
+std::vector<gfx::GpuFence> ZxEventsToGpuFences(
+    const std::vector<zx::event>& events) {
+  std::vector<gfx::GpuFence> fences;
+  for (const auto& event : events) {
+    gfx::GpuFenceHandle handle;
+    zx_status_t status =
+        event.duplicate(ZX_RIGHT_SAME_RIGHTS, &handle.owned_event);
+    ZX_DCHECK(status == ZX_OK, status);
+    fences.emplace_back(std::move(handle));
+  }
+  return fences;
+}
+
 class PresenterImageFuchsia : public OutputPresenter::Image {
  public:
   explicit PresenterImageFuchsia(uint32_t image_id);
@@ -113,6 +127,18 @@ void PresenterImageFuchsia::TakeSemaphores(
 
 }  // namespace
 
+OutputPresenterFuchsia::PendingOverlay::PendingOverlay(
+    OverlayCandidate candidate,
+    std::vector<gfx::GpuFence> release_fences)
+    : candidate(std::move(candidate)),
+      release_fences(std::move(release_fences)) {}
+OutputPresenterFuchsia::PendingOverlay::~PendingOverlay() = default;
+
+OutputPresenterFuchsia::PendingOverlay::PendingOverlay(PendingOverlay&&) =
+    default;
+OutputPresenterFuchsia::PendingOverlay&
+OutputPresenterFuchsia::PendingOverlay::operator=(PendingOverlay&&) = default;
+
 OutputPresenterFuchsia::PendingFrame::PendingFrame() = default;
 OutputPresenterFuchsia::PendingFrame::~PendingFrame() = default;
 
@@ -360,8 +386,9 @@ void OutputPresenterFuchsia::SchedulePrimaryPlane(
     bool is_submitted) {
   auto* image_fuchsia = static_cast<PresenterImageFuchsia*>(image);
 
-  DCHECK(!next_frame_);
-  next_frame_ = PendingFrame();
+  if (!next_frame_)
+    next_frame_ = PendingFrame();
+  DCHECK(!next_frame_->buffer_collection_id);
   next_frame_->image_id = image_fuchsia->image_id();
   next_frame_->buffer_collection_id = last_buffer_collection_id_;
 
@@ -388,21 +415,18 @@ void OutputPresenterFuchsia::SchedulePrimaryPlane(
 void OutputPresenterFuchsia::ScheduleOverlays(
     SkiaOutputSurface::OverlayList overlays,
     std::vector<ScopedOverlayAccess*> accesses) {
+  if (!next_frame_)
+    next_frame_ = PendingFrame();
+
   for (size_t i = 0; i < overlays.size(); ++i) {
-    auto& overlay = overlays[i];
-    DCHECK(overlay.mailbox.IsSharedImage());
-    auto pixmap =
-        dependency_->GetSharedImageManager()->GetNativePixmap(overlay.mailbox);
-    if (!pixmap) {
-      LOG(ERROR) << "Cannot access SysmemNativePixmap";
-      continue;
+    next_frame_->overlays.emplace_back(std::move(overlays[i]),
+                                       accesses[i]->TakeReleaseFences());
+    // Merge all fences under |acquire_fences|, because we are trying to sync
+    // primary plane and overlay updates to avoid artifacts.
+    for (auto& fence : accesses[i]->TakeAcquireFences()) {
+      next_frame_->acquire_fences.push_back(
+          std::move(fence.GetGpuFenceHandle().Clone().owned_event));
     }
-
-    pixmap->ScheduleOverlayPlane(
-        dependency_->GetSurfaceHandle(), overlay.plane_z_order,
-        overlay.transform, gfx::ToRoundedRect(overlay.display_rect),
-        overlay.uv_rect, !overlay.is_opaque, accesses[i]->TakeAcquireFences(),
-        accesses[i]->TakeReleaseFences());
   }
 }
 
@@ -410,19 +434,36 @@ void OutputPresenterFuchsia::PresentNextFrame() {
   DCHECK(!present_is_pending_);
   DCHECK(!pending_frames_.empty());
 
+  auto& frame = pending_frames_.front();
   TRACE_EVENT_NESTABLE_ASYNC_END1("viz", "OutputPresenterFuchsia::PresentQueue",
                                   TRACE_ID_LOCAL(this), "image_id",
-                                  pending_frames_.front().image_id);
+                                  frame.image_id);
   TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(
       "viz", "OutputPresenterFuchsia::PresentFrame", TRACE_ID_LOCAL(this),
-      "image_id", pending_frames_.front().image_id);
+      "image_id", frame.image_id);
 
   present_is_pending_ = true;
-  uint64_t target_presentation_time = zx_clock_get_monotonic();
+
+  for (size_t i = 0; i < frame.overlays.size(); ++i) {
+    auto& overlay = frame.overlays[i].candidate;
+    DCHECK(overlay.mailbox.IsSharedImage());
+    auto pixmap =
+        dependency_->GetSharedImageManager()->GetNativePixmap(overlay.mailbox);
+    if (!pixmap) {
+      LOG(ERROR) << "Cannot access SysmemNativePixmap";
+      continue;
+    }
+    pixmap->ScheduleOverlayPlane(dependency_->GetSurfaceHandle(),
+                                 overlay.plane_z_order, overlay.transform,
+                                 gfx::ToRoundedRect(overlay.display_rect),
+                                 overlay.uv_rect, !overlay.is_opaque,
+                                 ZxEventsToGpuFences(frame.acquire_fences),
+                                 std::move(frame.overlays[i].release_fences));
+  }
+
   image_pipe_->PresentImage(
-      pending_frames_.front().image_id, target_presentation_time,
-      std::move(pending_frames_.front().acquire_fences),
-      std::move(pending_frames_.front().release_fences),
+      pending_frames_.front().image_id, zx_clock_get_monotonic(),
+      std::move(frame.acquire_fences), std::move(frame.release_fences),
       fit::bind_member(this, &OutputPresenterFuchsia::OnPresentComplete));
 }
 

components/viz/service/display_embedder/output_presenter_fuchsia.h

@@ -63,6 +63,18 @@ class VIZ_SERVICE_EXPORT OutputPresenterFuchsia : public OutputPresenter {
                         std::vector<ScopedOverlayAccess*> accesses) final;
 
  private:
+  struct PendingOverlay {
+    PendingOverlay(OverlayCandidate candidate,
+                   std::vector<gfx::GpuFence> release_fences);
+    ~PendingOverlay();
+
+    PendingOverlay(PendingOverlay&&);
+    PendingOverlay& operator=(PendingOverlay&&);
+
+    OverlayCandidate candidate;
+    std::vector<gfx::GpuFence> release_fences;
+  };
+
   struct PendingFrame {
     PendingFrame();
     ~PendingFrame();
@@ -70,8 +82,8 @@ class VIZ_SERVICE_EXPORT OutputPresenterFuchsia : public OutputPresenter {
     PendingFrame(PendingFrame&&);
     PendingFrame& operator=(PendingFrame&&);
 
-    uint32_t buffer_collection_id;
-    uint32_t image_id;
+    uint32_t buffer_collection_id = 0;
+    uint32_t image_id = 0;
 
     std::vector<zx::event> acquire_fences;
     std::vector<zx::event> release_fences;
@@ -82,6 +94,9 @@ class VIZ_SERVICE_EXPORT OutputPresenterFuchsia : public OutputPresenter {
     // Indicates that this is the last frame for this buffer collection and that
     // the collection can be removed after the frame is presented.
     bool remove_buffer_collection = false;
+
+    // Vector of overlays that are associated with this frame.
+    std::vector<PendingOverlay> overlays;
   };
 
   void PresentNextFrame();

ui/ozone/platform/scenic/scenic_surface.cc

@@ -119,10 +119,12 @@ bool ScenicSurface::PresentOverlayView(
   return true;
 }
 
-bool ScenicSurface::UpdateOverlayViewPosition(gfx::SysmemBufferCollectionId id,
-                                              int plane_z_order,
-                                              const gfx::Rect& display_bounds,
-                                              const gfx::RectF& crop_rect) {
+bool ScenicSurface::UpdateOverlayViewPosition(
+    gfx::SysmemBufferCollectionId id,
+    int plane_z_order,
+    const gfx::Rect& display_bounds,
+    const gfx::RectF& crop_rect,
+    std::vector<zx::event> acquire_fences) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(overlays_.count(id));
   auto& overlay_view_info = overlays_.at(id);
@@ -136,6 +138,10 @@ bool ScenicSurface::UpdateOverlayViewPosition(gfx::SysmemBufferCollectionId id,
   overlay_view_info.plane_z_order = plane_z_order;
   overlay_view_info.display_bounds = display_bounds;
   overlay_view_info.crop_rect = crop_rect;
+
+  for (auto& fence : acquire_fences)
+    scenic_session_.EnqueueAcquireFence(std::move(fence));
+
   // TODO(crbug.com/1143514): Only queue commands for the affected overlays
   // instead of the whole scene.
   UpdateViewHolderScene();

ui/ozone/platform/scenic/scenic_surface.h

@@ -57,7 +57,8 @@ class ScenicSurface : public ui::PlatformWindowSurface {
   bool UpdateOverlayViewPosition(gfx::SysmemBufferCollectionId id,
                                  int plane_z_order,
                                  const gfx::Rect& display_bounds,
-                                 const gfx::RectF& crop_rect);
+                                 const gfx::RectF& crop_rect,
+                                 std::vector<zx::event> acquire_fences);
 
   // Remove ViewHolder specified by |id|.
   bool RemoveOverlayView(gfx::SysmemBufferCollectionId id);

ui/ozone/platform/scenic/sysmem_native_pixmap.cc

@@ -4,6 +4,7 @@
 
 #include "ui/ozone/platform/scenic/sysmem_native_pixmap.h"
 
+#include "base/fuchsia/fuchsia_logging.h"
 #include "base/logging.h"
 #include "ui/gfx/geometry/rect_f.h"
 #include "ui/gfx/gpu_fence.h"
@@ -19,6 +20,18 @@ zx::event GpuFenceToZxEvent(gfx::GpuFence fence) {
   return fence.GetGpuFenceHandle().Clone().owned_event;
 }
 
+std::vector<zx::event> DuplicateZxEvents(
+    const std::vector<zx::event>& acquire_events) {
+  std::vector<zx::event> duped_events;
+  for (auto& event : acquire_events) {
+    duped_events.emplace_back();
+    zx_status_t status =
+        event.duplicate(ZX_RIGHT_SAME_RIGHTS, &duped_events.back());
+    ZX_DCHECK(status == ZX_OK, status);
+  }
+  return duped_events;
+}
+
 }  // namespace
 
 SysmemNativePixmap::SysmemNativePixmap(
@@ -98,9 +111,6 @@ bool SysmemNativePixmap::ScheduleOverlayPlane(
     DLOG(ERROR) << "Failed to attach to surface.";
     return false;
   }
-  surface->UpdateOverlayViewPosition(buffer_collection_id, plane_z_order,
-                                     display_bounds, crop_rect);
-  overlay_view->SetBlendMode(enable_blend);
 
   // Convert gfx::GpuFence to zx::event for PresentImage call.
   std::vector<zx::event> acquire_events;
@@ -109,6 +119,12 @@ bool SysmemNativePixmap::ScheduleOverlayPlane(
   std::vector<zx::event> release_events;
   for (auto& fence : release_fences)
     release_events.push_back(GpuFenceToZxEvent(std::move(fence)));
+
+  surface->UpdateOverlayViewPosition(buffer_collection_id, plane_z_order,
+                                     display_bounds, crop_rect,
+                                     DuplicateZxEvents(acquire_events));
+
+  overlay_view->SetBlendMode(enable_blend);
   overlay_view->PresentImage(handle_.buffer_index, std::move(acquire_events),
                              std::move(release_events));