Mac: Kill lots of AcceleratedWidget code (with fire)

content/browser/compositor/software_output_device_mac.mm

Index: content/browser/compositor/software_output_device_mac.mm
diff --git a/content/browser/compositor/software_output_device_mac.mm b/content/browser/compositor/software_output_device_mac.mm
index 5dacbae755c5eaa97a69fb4544135f1a4da80bf2..ba27392bb0bfb2dd66bba24a244ef40e09a0cd84 100644
--- a/content/browser/compositor/software_output_device_mac.mm
+++ b/content/browser/compositor/software_output_device_mac.mm
@@ -6,22 +6,145 @@
 
 #include "content/browser/compositor/software_output_device_mac.h"

[tapted, 2015/10/26 02:51:41]

nit (while you're here): this line should be swapped with #import Cocoa

[ccameron, 2015/10/26 06:40:33]

Done.

 
+#include "base/mac/foundation_util.h"
+#include "third_party/skia/include/core/SkCanvas.h"
 #include "ui/accelerated_widget_mac/accelerated_widget_mac.h"
 #include "ui/compositor/compositor.h"
+#include "ui/gfx/skia_util.h"
 
 namespace content {
 
 SoftwareOutputDeviceMac::SoftwareOutputDeviceMac(ui::Compositor* compositor)
-    : compositor_(compositor) {
-}
+    : compositor_(compositor), scale_factor_(1), current_index_(0) {}
 
 SoftwareOutputDeviceMac::~SoftwareOutputDeviceMac() {
 }
 
+void SoftwareOutputDeviceMac::Resize(const gfx::Size& pixel_size,
+                                     float scale_factor) {
+  if (pixel_size_ == pixel_size && scale_factor_ == scale_factor)
+    return;
+
+  pixel_size_ = pixel_size;
+  scale_factor_ = scale_factor;
+
+  DiscardBackbuffer();
+}
+
+void SoftwareOutputDeviceMac::CopyPreviousBufferDamage(
+    const gfx::Rect& new_damage_rect) {
+  TRACE_EVENT0("browser", "CopyPreviousBufferDamage");
+
+  // The region to copy is the region drawn last frame, minus the region that
+  // is drawn this frame.
+  SkRegion copy_region(
+      SkRegion(gfx::RectToSkIRect(previous_buffer_damage_rect_)));
+  bool copy_region_nonempty = copy_region.op(
+      gfx::RectToSkIRect(new_damage_rect), SkRegion::kDifference_Op);
+  previous_buffer_damage_rect_ = new_damage_rect;

[tapted, 2015/10/26 02:51:41]

e.g., with regions, this could be 

previous_buffer_damage_region_.swap(new_damage_region);

[ccameron, 2015/10/26 06:40:33]

Done.

+
+  if (!copy_region_nonempty)
+    return;
+
+  {
+    TRACE_EVENT0("browser", "IOSurfaceLock");
+    IOReturn io_result = IOSurfaceLock(
+        io_surfaces_[!current_index_],

[tapted, 2015/10/26 02:51:41]

optional: this comes up a lot - maybe a private helper function like 

IOSurfaceRef SoftwareOutputDeviceMac::BackBuffer() { .. }

or similar. Also for FrontBuffer I guess. But not sure if this really adds
much.. up to you. I guess CopyPreviousBufferDamage could also take an
IOSurfaceRef argument

[ccameron, 2015/10/26 06:40:33]

Created a local variable for this (we don't use the previous buffer elsewhere).

+        kIOSurfaceLockReadOnly | kIOSurfaceLockAvoidSync, nullptr);
+    if (io_result) {
+      DLOG(ERROR) << "Failed to lock previous IOSurface " << io_result;
+      return;
+    }
+  }
+
+  uint8_t* pixels = reinterpret_cast<uint8_t*>(

[tapted, 2015/10/26 02:51:41]

static_cast? (pretty sure void* static casts to any pointer type)

[ccameron, 2015/10/26 06:40:33]

Done.

+      IOSurfaceGetBaseAddress(io_surfaces_[!current_index_]));
+  size_t pitch = IOSurfaceGetBytesPerRow(io_surfaces_[!current_index_]);

[tapted, 2015/10/26 02:51:41]

optional: pitch -> stride? (I'd actually never seen pitch for this before.. -
looks like they're interchangeable, and both used, but stride more common in
Chrome)

[ccameron, 2015/10/26 06:40:33]

Done.

+  size_t bytes_per_element = 4;
+  for (SkRegion::Iterator it(copy_region); !it.done(); it.next()) {
+    const SkIRect& rect = it.rect();
+    canvas_->writePixels(
+        SkImageInfo::MakeN32Premul(rect.width(), rect.height()),
+        pixels + bytes_per_element * rect.x() + pitch * rect.y(), pitch,
+        rect.x(), rect.y());
+  }
+
+  {
+    TRACE_EVENT0("browser", "IOSurfaceUnlock");
+    IOReturn io_result = IOSurfaceUnlock(
+        io_surfaces_[!current_index_],
+        kIOSurfaceLockReadOnly | kIOSurfaceLockAvoidSync, nullptr);
+    if (io_result)
+      DLOG(ERROR) << "Failed to unlock previous IOSurface " << io_result;
+  }
+}
+
+SkCanvas* SoftwareOutputDeviceMac::BeginPaint(
+    const gfx::Rect& new_damage_rect) {
+  for (int i = 0; i < 2; ++i) {

[tapted, 2015/10/26 02:51:41]

Since it's a bit orthogonal to the core purpose, perhaps a helper function, and
here do, e.g.

if (!EnsureBuffersExist())
  return nullptr;

(should this even be instead in the EnsureBackbuffer() override ?)

[ccameron, 2015/10/26 06:40:33]

I moved it to EnsureBuffersExist, since EnsureBackbuffer is void.

+    if (!io_surfaces_[i]) {
+      TRACE_EVENT0("browser", "IOSurfaceCreate");
+      unsigned pixel_format = 'BGRA';
+      unsigned bytes_per_element = 4;
+      NSDictionary* options = @{
+        static_cast<id>(kIOSurfaceWidth) : @(pixel_size_.width()),
+        static_cast<id>(kIOSurfaceHeight) : @(pixel_size_.height()),
+        static_cast<id>(kIOSurfacePixelFormat) : @(pixel_format),
+        static_cast<id>(kIOSurfaceBytesPerElement) : @(bytes_per_element),
+      };
+      io_surfaces_[i].reset(IOSurfaceCreate(
+          base::mac::CFCast<CFDictionaryRef>(options)));

[tapted, 2015/10/26 02:51:41]

whoops (my bad) - this should be NSToCFCast instead of CFCast (equivalent I
guess, but NSToCFCast is more constrained and doesn't need the "strict" version
with a runtime check)

[ccameron, 2015/10/26 06:40:33]

Done.

+    }
+    if (!io_surfaces_[i]) {
+      DLOG(ERROR) << "Failed to allocateIOSurface";

[tapted, 2015/10/26 02:51:41]

nit: space after allocate

[ccameron, 2015/10/26 06:40:33]

Done.

+      return nullptr;
+    }
+  }
+
+  {
+    TRACE_EVENT0("browser", "IOSurfaceLock");
+    IOReturn io_result = IOSurfaceLock(io_surfaces_[current_index_],
+                                       kIOSurfaceLockAvoidSync, nullptr);
+    if (io_result) {
+      DLOG(ERROR) << "Failed to lock IOSurface " << io_result;
+      return nullptr;
+    }
+  }
+
+  SkPMColor* pixels = reinterpret_cast<SkPMColor*>(

[tapted, 2015/10/26 02:51:41]

static_cast

[ccameron, 2015/10/26 06:40:33]

Done.

+      IOSurfaceGetBaseAddress(io_surfaces_[current_index_]));
+  size_t pitch = IOSurfaceGetBytesPerRow(io_surfaces_[current_index_]);

[tapted, 2015/10/26 02:51:41]

(optional: stride?)

[ccameron, 2015/10/26 06:40:33]

Done.

+
+  canvas_ = skia::AdoptRef(SkCanvas::NewRasterDirectN32(
+      pixel_size_.width(), pixel_size_.height(), pixels, pitch));
+
+  CopyPreviousBufferDamage(new_damage_rect);
+  return canvas_.get();
+}
+
 void SoftwareOutputDeviceMac::EndPaint() {
   SoftwareOutputDevice::EndPaint();
-  ui::AcceleratedWidgetMacGotSoftwareFrame(
-      compositor_->widget(), scale_factor_, surface_->getCanvas());
+  {
+    TRACE_EVENT0("browser", "IOSurfaceUnlock");
+    IOReturn io_result = IOSurfaceUnlock(io_surfaces_[current_index_],
+                                         kIOSurfaceLockAvoidSync, nullptr);
+    if (io_result)
+      DLOG(ERROR) << "Failed to unlock IOSurface " << io_result;
+  }
+
+  canvas_ = nullptr;
+  ui::AcceleratedWidgetMacGotIOSurfaceFrame(compositor_->widget(),
+                                            io_surfaces_[current_index_],
+                                            pixel_size_, scale_factor_, false);
+
+  current_index_ = !current_index_;
 }
 
+void SoftwareOutputDeviceMac::DiscardBackbuffer() {
+  for (int i = 0; i < 2; ++i)
+    io_surfaces_[i].reset();
+}
+
+void SoftwareOutputDeviceMac::EnsureBackbuffer() {}
+
 }  // namespace content