Ozone: Dont hardcode format to YUV when using Overlay Composition.

ui/ozone/platform/drm/gpu/gbm_buffer.cc

Index: ui/ozone/platform/drm/gpu/gbm_buffer.cc
diff --git a/ui/ozone/platform/drm/gpu/gbm_buffer.cc b/ui/ozone/platform/drm/gpu/gbm_buffer.cc
index 6547146ffd88ae74c40ad34c3839e533c61beb8d..199b6988b86cc926fe644ed430de60d6a68d083f 100644
--- a/ui/ozone/platform/drm/gpu/gbm_buffer.cc
+++ b/ui/ozone/platform/drm/gpu/gbm_buffer.cc
@@ -23,11 +23,6 @@
 #include "ui/ozone/public/ozone_platform.h"  // nogncheck
 #include "ui/ozone/public/surface_factory_ozone.h"
 
-namespace {
-// Optimal format for rendering on overlay.
-const gfx::BufferFormat kOverlayRenderFormat = gfx::BufferFormat::UYVY_422;
-}  // namespace
-
 namespace ui {
 
 GbmBuffer::GbmBuffer(const scoped_refptr<GbmDevice>& gbm,
@@ -91,26 +86,13 @@ bool GbmPixmap::InitializeFromBuffer(const scoped_refptr<GbmBuffer>& buffer) {
 void GbmPixmap::SetProcessingCallback(
     const ProcessingCallback& processing_callback) {
   processing_callback_ = processing_callback;

[kalyank, 2016/01/11 08:19:51]

This is called on Main thread while CopyBuffer is called in Drmthread. However,
SetProcessingCallback is never called after initial setup. 

+  copy_buffer_handler_ = base::Bind(&GbmPixmap::CopyBuffer, this);
 }
 
 scoped_refptr<NativePixmap> GbmPixmap::GetProcessedPixmap(
     gfx::Size target_size,
     gfx::BufferFormat target_format) {
-  ui::OzonePlatform* platform = ui::OzonePlatform::GetInstance();
-  ui::SurfaceFactoryOzone* factory = platform->GetSurfaceFactoryOzone();
-  // Create a buffer from Ozone.
-  scoped_refptr<ui::NativePixmap> processed_pixmap =
-      factory->CreateNativePixmap(gfx::kNullAcceleratedWidget, target_size,
-                                  target_format, gfx::BufferUsage::SCANOUT);
-  if (!processed_pixmap) {
-    LOG(ERROR) << "Failed creating an Ozone NativePixmap for processing";
-    return nullptr;
-  }
-  if (!processing_callback_.Run(this, processed_pixmap)) {
-    LOG(ERROR) << "Failed processing NativePixmap";
-    return nullptr;
-  }
-  return processed_pixmap;
+  return nullptr;

[kalyank, 2016/01/11 08:19:51]

This is not used any more. Call to CreateNativePixmap From CopyBuffer results in
a deadlock, as CopyBuffer is called from DrmThread and CreateNativePixmap try to
post a synchronous task to DrmThread.

[dnicoara, 2016/01/13 17:26:51]

Took a quick look and this isn't used anywhere anymore, right? Perhaps we can
remove this API entry in a followup?

[kalyank, 2016/01/13 19:52:33]

We should be, this is public interface and wasn't sure if any other ozone
implementations depend on this.

[dnicoara, 2016/01/13 20:21:53]

Acknowledged. Haven't seen any, so lets just remove it in a followup.

[kalyank, 2016/01/14 06:24:05]

k, will post a followup patch for this.

 }
 
 gfx::NativePixmapHandle GbmPixmap::ExportHandle() {
@@ -155,20 +137,6 @@ bool GbmPixmap::ScheduleOverlayPlane(gfx::AcceleratedWidget widget,
                                      gfx::OverlayTransform plane_transform,
                                      const gfx::Rect& display_bounds,
                                      const gfx::RectF& crop_rect) {
-  gfx::Size target_size;
-  gfx::BufferFormat target_format;
-  if (plane_z_order && ShouldApplyProcessing(display_bounds, crop_rect,
-                                             &target_size, &target_format)) {
-    scoped_refptr<NativePixmap> processed_pixmap =
-        GetProcessedPixmap(target_size, target_format);
-    if (processed_pixmap) {
-      return processed_pixmap->ScheduleOverlayPlane(
-          widget, plane_z_order, plane_transform, display_bounds, crop_rect);
-    } else {
-      return false;
-    }
-  }
-
   // TODO(reveman): Add support for imported buffers. crbug.com/541558
   if (!buffer_) {
     PLOG(ERROR) << "ScheduleOverlayPlane requires a buffer.";
@@ -176,35 +144,38 @@ bool GbmPixmap::ScheduleOverlayPlane(gfx::AcceleratedWidget widget,
   }
 
   DCHECK(buffer_->GetUsage() == gfx::BufferUsage::SCANOUT);
-  surface_manager_->GetSurface(widget)->QueueOverlayPlane(OverlayPlane(
-      buffer_, plane_z_order, plane_transform, display_bounds, crop_rect));
+
+  surface_manager_->GetSurface(widget)->QueueOverlayPlane(
+      OverlayPlane(buffer_, plane_z_order, plane_transform, display_bounds,
+                   crop_rect, copy_buffer_handler_));
+
   return true;
 }
 
-bool GbmPixmap::ShouldApplyProcessing(const gfx::Rect& display_bounds,
-                                      const gfx::RectF& crop_rect,
-                                      gfx::Size* target_size,
-                                      gfx::BufferFormat* target_format) {
-  if (crop_rect.width() == 0 || crop_rect.height() == 0) {
-    PLOG(ERROR) << "ShouldApplyProcessing passed zero processing target.";
+bool GbmPixmap::CopyBuffer(const scoped_refptr<ScanoutBuffer>& source_buffer,
+                           scoped_refptr<ScanoutBuffer>& target_buffer) {
+  DCHECK(source_buffer->GetHandle() == buffer_->GetHandle());

[kalyank, 2016/01/11 08:19:51]

This will be called in Drmthread by OverlayValidator.

[dnicoara, 2016/01/13 17:26:51]

Perhaps add some comments explaining this since it won't be obvious why the
pixmap was initialized this way.

[kalyank, 2016/01/13 19:52:33]

Done.

+  ui::OzonePlatform* platform = ui::OzonePlatform::GetInstance();
+  ui::SurfaceFactoryOzone* factory = platform->GetSurfaceFactoryOzone();

[dnicoara, 2016/01/13 17:26:51]

You could just drop this since this is |surface_manager_|

[kalyank, 2016/01/13 19:52:33]

Done.

+  scoped_refptr<GbmPixmap> pixmap(
+      new GbmPixmap(static_cast<GbmSurfaceFactory*>(factory)));
+
+  scoped_refptr<GbmBuffer> gbm_buffer(
+      static_cast<GbmBuffer*>(target_buffer.get()));
+
+  if (!pixmap->InitializeFromBuffer(gbm_buffer))
     return false;
-  }
 
-  if (!buffer_) {
-    PLOG(ERROR) << "ShouldApplyProcessing requires a buffer.";
+  DCHECK(!processing_callback_.is_null());
+  DCHECK(GetBufferSize() != pixmap->GetBufferSize() ||
+         GetBufferFormat() != pixmap->GetBufferFormat());
+
+  if (!processing_callback_.Run(this, pixmap)) {
+    LOG(ERROR) << "Failed processing NativePixmap";
     return false;
   }
 
-  // TODO(william.xie): Figure out the optimal render format for overlay.
-  // See http://crbug.com/553264.
-  *target_format = kOverlayRenderFormat;
-  gfx::Size pixmap_size = buffer_->GetSize();
-  // If the required size is not integer-sized, round it to the next integer.
-  *target_size = gfx::ToCeiledSize(
-      gfx::SizeF(display_bounds.width() / crop_rect.width(),
-                 display_bounds.height() / crop_rect.height()));
-
-  return pixmap_size != *target_size || GetBufferFormat() != *target_format;
+  return true;
 }
 
 }  // namespace ui