Revert "Viz/Ozone/DRM: Provide NativePixmap for the primary plane"

This reverts commit 0ea41733.

Reason for revert: In this CL, GetOverlayMailbox() calls BufferQueue::GetCurrentBuffer(). Unfortunately, it turns out there are cases where this causes the GPU to draw to a buffer that's being scanned out (see https://crbug.com/1047030 for an example). It may also cause problems with damage tracking. Reverting this CL to try to fix the issue before the M81 branch point :(

Original change's description:
> Viz/Ozone/DRM: Provide NativePixmap for the primary plane
> 
> Plumb mailbox for the primary plane to OverlayProcessorOzone and use
> SharedImageInterface to obtain the corresponding NativePixmap.
> 
> Bug: 756454
> Change-Id: Ic537d05253bc68c27a8fed961de8547ace359e82
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2016327
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Commit-Queue: Saman Sami <samans@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#736065}

TBR=rjkroege@chromium.org,samans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 756454
Change-Id: I6f42b94d1939597f7c34f0d0433d45a02716aa58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2031723Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737238}

components/viz/service/display/direct_renderer.cc

@@ -354,8 +354,7 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
       current_frame()->output_surface_plane =
           overlay_processor_->ProcessOutputSurfaceAsOverlay(
               device_viewport_size, output_surface_->GetOverlayBufferFormat(),
-              reshape_device_color_space_, reshape_has_alpha_,
-              output_surface_->GetOverlayMailbox());
+              reshape_device_color_space_, reshape_has_alpha_);
       primary_plane = &(current_frame()->output_surface_plane.value());
     }
 

components/viz/service/display/output_surface.cc

@@ -82,9 +82,4 @@ void OutputSurface::SetGpuVSyncCallback(GpuVSyncCallback callback) {
 void OutputSurface::SetGpuVSyncEnabled(bool enabled) {
   NOTREACHED();
 }
-
-gpu::Mailbox OutputSurface::GetOverlayMailbox() const {
-  return gpu::Mailbox();
-}
-
 }  // namespace viz

components/viz/service/display/output_surface.h

@@ -19,7 +19,6 @@
 #include "components/viz/common/resources/returned_resource.h"
 #include "components/viz/service/display/software_output_device.h"
 #include "components/viz/service/viz_service_export.h"
-#include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/command_buffer/common/texture_in_use_response.h"
 #include "gpu/ipc/common/surface_handle.h"
 #include "gpu/ipc/gpu_task_scheduler_helper.h"
@@ -146,9 +145,6 @@ class VIZ_SERVICE_EXPORT OutputSurface {
   // Get the texture for the main image's overlay.
   virtual unsigned GetOverlayTextureId() const = 0;
 
-  // Returns the |mailbox| corresponding to the main image's overlay.
-  virtual gpu::Mailbox GetOverlayMailbox() const;
-
   // Get the format for the main image's overlay.
   virtual gfx::BufferFormat GetOverlayBufferFormat() const = 0;
 

components/viz/service/display/overlay_processor_interface.cc

@@ -141,15 +141,13 @@ OverlayProcessorInterface::ProcessOutputSurfaceAsOverlay(
     const gfx::Size& viewport_size,
     const gfx::BufferFormat& buffer_format,
     const gfx::ColorSpace& color_space,
-    bool has_alpha,
-    const gpu::Mailbox& mailbox) {
+    bool has_alpha) {
   OutputSurfaceOverlayPlane overlay_plane;
   overlay_plane.transform = gfx::OverlayTransform::OVERLAY_TRANSFORM_NONE;
   overlay_plane.resource_size = viewport_size;
   overlay_plane.format = buffer_format;
   overlay_plane.color_space = color_space;
   overlay_plane.enable_blending = has_alpha;
-  overlay_plane.mailbox = mailbox;
 
   // Adjust transformation and display_rect based on display rotation.
   overlay_plane.display_rect =

components/viz/service/display/overlay_processor_interface.h

@@ -79,8 +79,6 @@ class VIZ_SERVICE_EXPORT OverlayProcessorInterface {
     // TODO(weiliangc): Should be replaced by SharedImage mailbox.
     // Gpu fence to wait for before overlay is ready for display.
     unsigned gpu_fence_id;
-    // Mailbox corresponding to the buffer backing the primary plane.
-    gpu::Mailbox mailbox;
   };
 
   // TODO(weiliangc): Eventually the asymmetry between primary plane and
@@ -90,8 +88,7 @@ class VIZ_SERVICE_EXPORT OverlayProcessorInterface {
       const gfx::Size& viewport_size,
       const gfx::BufferFormat& buffer_format,
       const gfx::ColorSpace& color_space,
-      bool has_alpha,
-      const gpu::Mailbox& mailbox);
+      bool has_alpha);
 
   static std::unique_ptr<OverlayProcessorInterface> CreateOverlayProcessor(
       gpu::SurfaceHandle surface_handle,

components/viz/service/display/overlay_processor_ozone.cc

@@ -131,18 +131,6 @@ void OverlayProcessorOzone::CheckOverlaySupport(
     // For ozone-cast, there will not be a primary_plane.
     if (primary_plane) {
       ConvertToOzoneOverlaySurface(*primary_plane, &(*ozone_surface_iterator));
-      if (shared_image_interface_) {
-        bool result = SetNativePixmapForCandidate(&(*ozone_surface_iterator),
-                                                  primary_plane->mailbox);
-        // We cannot validate an overlay configuration without the buffer for
-        // primary plane present.
-        if (!result) {
-          for (auto& candidate : *surfaces) {
-            candidate.overlay_handled = false;
-          }
-          return;
-        }
-      }
       ozone_surface_iterator++;
     }
 
@@ -153,11 +141,29 @@ void OverlayProcessorOzone::CheckOverlaySupport(
       ConvertToOzoneOverlaySurface(*surface_iterator,
                                    &(*ozone_surface_iterator));
       if (shared_image_interface_) {
-        bool result = SetNativePixmapForCandidate(&(*ozone_surface_iterator),
-                                                  surface_iterator->mailbox);
-        // Skip the candidate if the corresponding NativePixmap is not found.
-        if (!result) {
-          *ozone_surface_iterator = ui::OverlaySurfaceCandidate();
+        UMA_HISTOGRAM_BOOLEAN(
+            "Compositing.Display.OverlayProcessorOzone."
+            "IsCandidateSharedImage",
+            surface_iterator->mailbox.IsSharedImage());
+        if (surface_iterator->mailbox.IsSharedImage()) {
+          (*ozone_surface_iterator).native_pixmap =
+              shared_image_interface_->GetNativePixmap(
+                  surface_iterator->mailbox);
+          if (!ozone_surface_iterator->native_pixmap) {
+            // SharedImage creation and destruction happens on a different
+            // thread so there is no guarantee that we can always look them up
+            // successfully. If a SharedImage doesn't exist, ignore the
+            // candidate. We will try again next frame.
+            DLOG(ERROR)
+                << "Unable to find the NativePixmap corresponding to the "
+                   "overlay candidate";
+            *ozone_surface_iterator = ui::OverlaySurfaceCandidate();
+            ReportSharedImageExists(false);
+          } else {
+            (*ozone_surface_iterator).native_pixmap_unique_id =
+                MailboxToUInt32(surface_iterator->mailbox);
+            ReportSharedImageExists(true);
+          }
         }
       }
     }
@@ -189,35 +195,4 @@ gfx::Rect OverlayProcessorOzone::GetOverlayDamageRectForOutputSurface(
   return ToEnclosedRect(overlay.display_rect);
 }
 
-bool OverlayProcessorOzone::SetNativePixmapForCandidate(
-    ui::OverlaySurfaceCandidate* candidate,
-    const gpu::Mailbox& mailbox) {
-  DCHECK(shared_image_interface_);
-
-  UMA_HISTOGRAM_BOOLEAN(
-      "Compositing.Display.OverlayProcessorOzone."
-      "IsCandidateSharedImage",
-      mailbox.IsSharedImage());
-
-  if (!mailbox.IsSharedImage())
-    return false;
-
-  candidate->native_pixmap = shared_image_interface_->GetNativePixmap(mailbox);
-
-  if (!candidate->native_pixmap) {
-    // SharedImage creation and destruction happens on a different
-    // thread so there is no guarantee that we can always look them up
-    // successfully. If a SharedImage doesn't exist, ignore the
-    // candidate. We will try again next frame.
-    DLOG(ERROR) << "Unable to find the NativePixmap corresponding to the "
-                   "overlay candidate";
-    ReportSharedImageExists(false);
-    return false;
-  }
-
-  candidate->native_pixmap_unique_id = MailboxToUInt32(mailbox);
-  ReportSharedImageExists(true);
-  return true;
-}
-
 }  // namespace viz

components/viz/service/display/overlay_processor_ozone.h

@@ -36,12 +36,6 @@ class VIZ_SERVICE_EXPORT OverlayProcessorOzone
       const OverlayCandidate& candidate) const override;
 
  private:
-  // Populates |native_pixmap| and |native_pixmap_unique_id| in |candidate|
-  // based on |mailbox|. Return false if the corresponding NativePixmap cannot
-  // be found.
-  bool SetNativePixmapForCandidate(ui::OverlaySurfaceCandidate* candidate,
-                                   const gpu::Mailbox& mailbox);
-
   const bool overlay_enabled_;
 
   std::unique_ptr<ui::OverlayCandidatesOzone> overlay_candidates_;

components/viz/service/display/overlay_unittest.cc

@@ -1855,7 +1855,7 @@ TEST_F(UnderlayTest, PrimaryPlaneOverlayIsTransparentWithUnderlay) {
 
   auto output_surface_plane = overlay_processor_->ProcessOutputSurfaceAsOverlay(
       kDisplaySize, kDefaultBufferFormat, gfx::ColorSpace(),
-      false /* has_alpha */, gpu::Mailbox());
+      false /* has_alpha */);
   OverlayProcessorInterface::OutputSurfaceOverlayPlane* primary_plane =
       &output_surface_plane;
 
@@ -2202,7 +2202,7 @@ TEST_F(UnderlayCastTest, PrimaryPlaneOverlayIsAlwaysTransparent) {
   RenderPassList pass_list;
   pass_list.push_back(std::move(pass));
   auto output_surface_plane = overlay_processor_->ProcessOutputSurfaceAsOverlay(
-      kDisplaySize, kDefaultBufferFormat, gfx::ColorSpace(), gpu::Mailbox());
+      kDisplaySize, kDefaultBufferFormat, gfx::ColorSpace());
 
   overlay_processor_->ProcessForOverlays(
       resource_provider_.get(), &pass_list, GetIdentityColorMatrix(),

components/viz/service/display_embedder/buffer_queue.cc

@@ -37,13 +37,10 @@ void BufferQueue::SetSyncTokenProvider(SyncTokenProvider* sync_token_provider) {
 
 gpu::Mailbox BufferQueue::GetCurrentBuffer(
     gpu::SyncToken* creation_sync_token) {
+  DCHECK(creation_sync_token);
   if (!current_surface_)
-    current_surface_ = GetNextSurface();
-  if (!current_surface_)
-    return gpu::Mailbox();
-  if (creation_sync_token)
-    *creation_sync_token = current_surface_->creation_sync_token;
-  return current_surface_->mailbox;
+    current_surface_ = GetNextSurface(creation_sync_token);
+  return current_surface_ ? current_surface_->mailbox : gpu::Mailbox();
 }
 
 void BufferQueue::UpdateBufferDamage(const gfx::Rect& damage) {
@@ -127,7 +124,9 @@ void BufferQueue::FreeSurface(std::unique_ptr<AllocatedSurface> surface,
   allocated_count_--;
 }
 
-std::unique_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface() {
+std::unique_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface(
+    gpu::SyncToken* creation_sync_token) {
+  DCHECK(creation_sync_token);
   if (!available_surfaces_.empty()) {
     std::unique_ptr<AllocatedSurface> surface =
         std::move(available_surfaces_.back());
@@ -159,20 +158,16 @@ std::unique_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface() {
   }
 
   allocated_count_++;
-  gpu::SyncToken creation_sync_token = sii_->GenUnverifiedSyncToken();
-  return std::make_unique<AllocatedSurface>(
-      std::move(buffer), mailbox, creation_sync_token, gfx::Rect(size_));
+  *creation_sync_token = sii_->GenUnverifiedSyncToken();
+  return std::make_unique<AllocatedSurface>(std::move(buffer), mailbox,
+                                            gfx::Rect(size_));
 }
 
 BufferQueue::AllocatedSurface::AllocatedSurface(
     std::unique_ptr<gfx::GpuMemoryBuffer> buffer,
     const gpu::Mailbox& mailbox,
-    const gpu::SyncToken& creation_sync_token,
     const gfx::Rect& rect)
-    : buffer(buffer.release()),
-      mailbox(mailbox),
-      creation_sync_token(creation_sync_token),
-      damage(rect) {}
+    : buffer(buffer.release()), mailbox(mailbox), damage(rect) {}
 
 BufferQueue::AllocatedSurface::~AllocatedSurface() {
   DCHECK(!buffer);

components/viz/service/display_embedder/buffer_queue.h

@@ -16,7 +16,6 @@
 #include "base/memory/ref_counted.h"
 #include "components/viz/service/viz_service_export.h"
 #include "gpu/command_buffer/common/mailbox.h"
-#include "gpu/command_buffer/common/sync_token.h"
 #include "gpu/ipc/common/surface_handle.h"
 #include "ui/gfx/buffer_types.h"
 #include "ui/gfx/color_space.h"
@@ -123,7 +122,6 @@ class VIZ_SERVICE_EXPORT BufferQueue {
   struct VIZ_SERVICE_EXPORT AllocatedSurface {
     AllocatedSurface(std::unique_ptr<gfx::GpuMemoryBuffer> buffer,
                      const gpu::Mailbox& mailbox,
-                     const gpu::SyncToken& creation_sync_token,
                      const gfx::Rect& rect);
     ~AllocatedSurface();
 
@@ -131,9 +129,6 @@ class VIZ_SERVICE_EXPORT BufferQueue {
     // SurfaceHandle, we don't have to keep track of |buffer|.
     std::unique_ptr<gfx::GpuMemoryBuffer> buffer;
     gpu::Mailbox mailbox;
-    // SyncToken generated when |mailbox| was created. The client of BufferQueue
-    // should insert this token in its command stream before using |mailbox|.
-    gpu::SyncToken creation_sync_token;
     gfx::Rect damage;  // This is the damage for this frame from the previous.
   };
 
@@ -143,8 +138,11 @@ class VIZ_SERVICE_EXPORT BufferQueue {
   void UpdateBufferDamage(const gfx::Rect& damage);
 
   // Return a buffer that is available to be drawn into or nullptr if there is
-  // no available buffer and one cannot be created.
-  std::unique_ptr<AllocatedSurface> GetNextSurface();
+  // no available buffer and one cannot be created. If a new buffer is created
+  // *|creation_sync_token| is set to a sync token that the client must wait on
+  // before using the buffer.
+  std::unique_ptr<AllocatedSurface> GetNextSurface(
+      gpu::SyncToken* creation_sync_token);
 
   gpu::SharedImageInterface* const sii_;
   gfx::Size size_;

components/viz/service/display_embedder/gl_output_surface_buffer_queue.cc

@@ -199,10 +199,6 @@ unsigned GLOutputSurfaceBufferQueue::GetOverlayTextureId() const {
   return last_bound_texture_;
 }
 
-gpu::Mailbox GLOutputSurfaceBufferQueue::GetOverlayMailbox() const {
-  return buffer_queue_->GetCurrentBuffer(nullptr);
-}
-
 gfx::BufferFormat GLOutputSurfaceBufferQueue::GetOverlayBufferFormat() const {
   DCHECK(buffer_queue_);
   return buffer_queue_->buffer_format();

components/viz/service/display_embedder/gl_output_surface_buffer_queue.h

@@ -60,7 +60,6 @@ class VIZ_SERVICE_EXPORT GLOutputSurfaceBufferQueue
   uint32_t GetFramebufferCopyTextureFormat() override;
   bool IsDisplayedAsOverlayPlane() const override;
   unsigned GetOverlayTextureId() const override;
-  gpu::Mailbox GetOverlayMailbox() const override;
   gfx::BufferFormat GetOverlayBufferFormat() const override;
 
   // GLOutputSurface: