Switch over to using SkRegions to calculate dirty areas.

remoting/host/capturer_mac.cc

Index: remoting/host/capturer_mac.cc
diff --git a/remoting/host/capturer_mac.cc b/remoting/host/capturer_mac.cc
index bb9f8591672e041e2ee6d478a244912596f28ecb..95e819f3830bd5ba296cb5c3bde1476ac1ed3d51 100644
--- a/remoting/host/capturer_mac.cc
+++ b/remoting/host/capturer_mac.cc
@@ -15,6 +15,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "remoting/base/util.h"
 #include "remoting/host/capturer_helper.h"
+#include "skia/ext/skia_utils_mac.h"
 
 namespace remoting {
 
@@ -134,19 +135,20 @@ class CapturerMac : public Capturer {
   // Capturer interface.
   virtual void ScreenConfigurationChanged() OVERRIDE;
   virtual media::VideoFrame::Format pixel_format() const OVERRIDE;
-  virtual void ClearInvalidRects() OVERRIDE;
-  virtual void InvalidateRects(const InvalidRects& inval_rects) OVERRIDE;
+  virtual void ClearInvalidRegion() OVERRIDE;
+  virtual void InvalidateRegion(const SkRegion& inval_region) OVERRIDE;
   virtual void InvalidateScreen(const gfx::Size& size) OVERRIDE;
   virtual void InvalidateFullScreen() OVERRIDE;
-  virtual void CaptureInvalidRects(CaptureCompletedCallback* callback) OVERRIDE;
+  virtual void CaptureInvalidRegion(CaptureCompletedCallback* callback)
+      OVERRIDE;
   virtual const gfx::Size& size_most_recent() const OVERRIDE;
 
  private:
-  void GlBlitFast(const VideoFrameBuffer& buffer, const InvalidRects& rects);
+  void GlBlitFast(const VideoFrameBuffer& buffer, const SkRegion& region);
   void GlBlitSlow(const VideoFrameBuffer& buffer);
-  void CgBlit(const VideoFrameBuffer& buffer, const InvalidRects& rects);
-  void CaptureRects(const InvalidRects& rects,
-                    CaptureCompletedCallback* callback);
+  void CgBlit(const VideoFrameBuffer& buffer, const SkRegion& region);
+  void CaptureRegion(const SkRegion& region,
+                     CaptureCompletedCallback* callback);
 
   void ScreenRefresh(CGRectCount count, const CGRect *rect_array);
   void ScreenUpdateMove(CGScreenUpdateMoveDelta delta,
@@ -180,8 +182,8 @@ class CapturerMac : public Capturer {
   // a particular screen resolution.
   uint8* last_buffer_;
 
-  // Contains a list of invalid rectangles in the last capture.
-  InvalidRects last_invalid_rects_;
+  // Contains an invalid region from the last capture.
+  SkRegion last_invalid_region_;
 
   // Format of pixels returned in buffer.
   media::VideoFrame::Format pixel_format_;
@@ -240,8 +242,8 @@ void CapturerMac::ReleaseBuffers() {
 
 void CapturerMac::ScreenConfigurationChanged() {
   ReleaseBuffers();
-  InvalidRects rects;
-  helper_.SwapInvalidRects(rects);
+  SkRegion region;
+  helper_.SwapInvalidRegion(region);

[Wez, 2011/08/08 20:49:34]

Should be clear, not swap?

[dmac, 2011/08/10 20:30:36]

Done.

   last_buffer_ = NULL;
 
   CGDirectDisplayID mainDevice = CGMainDisplayID();
@@ -280,12 +282,12 @@ media::VideoFrame::Format CapturerMac::pixel_format() const {
   return pixel_format_;
 }
 
-void CapturerMac::ClearInvalidRects() {
-  helper_.ClearInvalidRects();
+void CapturerMac::ClearInvalidRegion() {
+  helper_.ClearInvalidRegion();
 }
 
-void CapturerMac::InvalidateRects(const InvalidRects& inval_rects) {
-  helper_.InvalidateRects(inval_rects);
+void CapturerMac::InvalidateRegion(const SkRegion& inval_region) {
+  helper_.InvalidateRegion(inval_region);
 }
 
 void CapturerMac::InvalidateScreen(const gfx::Size& size) {
@@ -296,25 +298,25 @@ void CapturerMac::InvalidateFullScreen() {
   helper_.InvalidateFullScreen();
 }
 
-void CapturerMac::CaptureInvalidRects(CaptureCompletedCallback* callback) {
+void CapturerMac::CaptureInvalidRegion(CaptureCompletedCallback* callback) {
   scoped_refptr<CaptureData> data;
   if (capturing_) {
-    InvalidRects rects;
-    helper_.SwapInvalidRects(rects);
+    SkRegion region;
+    helper_.SwapInvalidRegion(region);
     VideoFrameBuffer& current_buffer = buffers_[current_buffer_];
     current_buffer.Update();
 
     bool flip = true;  // GL capturers need flipping.
     if (cgl_context_) {
       if (pixel_buffer_object_.get() != 0) {
-        GlBlitFast(current_buffer, rects);
+        GlBlitFast(current_buffer, region);
       } else {
         // See comment in scoped_pixel_buffer_object::Init about why the slow
         // path is always used on 10.5.
         GlBlitSlow(current_buffer);
       }
     } else {
-      CgBlit(current_buffer, rects);
+      CgBlit(current_buffer, region);
       flip = false;
     }
 
@@ -329,7 +331,7 @@ void CapturerMac::CaptureInvalidRects(CaptureCompletedCallback* callback) {
 
     data = new CaptureData(planes, gfx::Size(current_buffer.size()),
                            pixel_format());
-    data->mutable_dirty_rects() = rects;
+    data->mutable_dirty_region() = region;
 
     current_buffer_ = (current_buffer_ + 1) % kNumBuffers;
     helper_.set_size_most_recent(data->size());
@@ -340,31 +342,25 @@ void CapturerMac::CaptureInvalidRects(CaptureCompletedCallback* callback) {
 }
 
 void CapturerMac::GlBlitFast(const VideoFrameBuffer& buffer,
-           const InvalidRects& rects) {
+           const SkRegion& region) {
   if (last_buffer_) {
     // We are doing double buffer for the capture data so we just need to copy
-    // invalid rects in the last capture in the current buffer.
-    // TODO(hclam): |last_invalid_rects_| and |rects| can overlap and this
-    // causes extra copies on the overlapped region. Subtract |rects| from
-    // |last_invalid_rects_| to do a minimal amount of copy when we have proper
-    // region algorithms implemented.
+    // the invalid region from the last capture in the current buffer.

[Wez, 2011/08/08 20:49:34]

Same optimization Alpha suggests for the Linux code applies here.

[dmac, 2011/08/10 20:30:36]

Added comment

 
     // Since the image obtained from OpenGL is upside-down, need to do some
     // magic here to copy the correct rectangle.
     const int y_offset = (buffer.size().height() - 1) * buffer.bytes_per_row();
-    for (InvalidRects::iterator i = last_invalid_rects_.begin();
-         i != last_invalid_rects_.end();
-         ++i) {
+    for(SkRegion::Iterator i(last_invalid_region_); !i.done(); i.next()) {
       CopyRect(last_buffer_ + y_offset,
                -buffer.bytes_per_row(),
                buffer.ptr() + y_offset,
                -buffer.bytes_per_row(),
                4,  // Bytes for pixel for RGBA.
-               *i);
+               i.rect());
     }
   }
   last_buffer_ = buffer.ptr();
-  last_invalid_rects_ = rects;
+  last_invalid_region_ = region;
 
   CGLContextObj CGL_MACRO_CONTEXT = cgl_context_;
   glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, pixel_buffer_object_.get());
@@ -380,13 +376,13 @@ void CapturerMac::GlBlitFast(const VideoFrameBuffer& buffer,
     // Copy only from the dirty rects. Since the image obtained from OpenGL is
     // upside-down we need to do some magic here to copy the correct rectangle.
     const int y_offset = (buffer.size().height() - 1) * buffer.bytes_per_row();
-    for (InvalidRects::iterator i = rects.begin(); i != rects.end(); ++i) {
+    for(SkRegion::Iterator i(region); !i.done(); i.next()) {
       CopyRect(ptr + y_offset,
          -buffer.bytes_per_row(),
          buffer.ptr() + y_offset,
          -buffer.bytes_per_row(),
          4,  // Bytes for pixel for RGBA.
-         *i);
+         i.rect());
     }
   }
   if (!glUnmapBufferARB(GL_PIXEL_PACK_BUFFER_ARB)) {
@@ -415,7 +411,7 @@ void CapturerMac::GlBlitSlow(const VideoFrameBuffer& buffer) {
 }
 
 void CapturerMac::CgBlit(const VideoFrameBuffer& buffer,
-                         const InvalidRects& rects) {
+                         const SkRegion& region) {
   if (last_buffer_)
     memcpy(buffer.ptr(), last_buffer_,
            buffer.bytes_per_row() * buffer.size().height());
@@ -425,13 +421,13 @@ void CapturerMac::CgBlit(const VideoFrameBuffer& buffer,
       reinterpret_cast<uint8*>(CGDisplayBaseAddress(main_display));
   int src_bytes_per_row = CGDisplayBytesPerRow(main_display);
   int src_bytes_per_pixel = CGDisplayBitsPerPixel(main_display) / 8;
-  for (InvalidRects::iterator i = rects.begin(); i != rects.end(); ++i) {
+  for(SkRegion::Iterator i(region); !i.done(); i.next()) {
     CopyRect(display_base_address,
              src_bytes_per_row,
              buffer.ptr(),
              buffer.bytes_per_row(),
              src_bytes_per_pixel,
-             *i);
+             i.rect());
   }
 }
 
@@ -440,24 +436,31 @@ const gfx::Size& CapturerMac::size_most_recent() const {
 }
 
 void CapturerMac::ScreenRefresh(CGRectCount count, const CGRect *rect_array) {
-  InvalidRects rects;
+  SkIRect skirect_array[count];
   for (CGRectCount i = 0; i < count; ++i) {
-    rects.insert(gfx::Rect(rect_array[i]));
+    skirect_array[i] = gfx::CGRectToSkIRect(rect_array[i]);
   }
-  InvalidateRects(rects);
+  SkRegion region;
+  region.setRects(skirect_array, count);
+  InvalidateRegion(region);
 }
 
 void CapturerMac::ScreenUpdateMove(CGScreenUpdateMoveDelta delta,
                                    size_t count,
                                    const CGRect *rect_array) {
-  InvalidRects rects;
+  SkIRect skirect_old_array[count];
+  SkIRect skirect_new_array[count];
   for (CGRectCount i = 0; i < count; ++i) {
     CGRect rect = rect_array[i];
-    rects.insert(gfx::Rect(rect));
+    skirect_old_array[i] = gfx::CGRectToSkIRect(rect);
     rect = CGRectOffset(rect, delta.dX, delta.dY);
-    rects.insert(gfx::Rect(rect));
+    skirect_new_array[i] = gfx::CGRectToSkIRect(rect);
   }
-  InvalidateRects(rects);
+  SkRegion region;
+  region.setRects(skirect_old_array, count);
+  InvalidateRegion(region);
+  region.setRects(skirect_new_array, count);
+  InvalidateRegion(region);

[Wez, 2011/08/08 20:49:34]

Do we need to invalidate the source region for the copy here?  Surely only the
destination changes, and the source will be notified via a separate refresh
operation?

[dmac, 2011/08/10 20:30:36]

Good point. Done.

 }
 
 void CapturerMac::ScreenRefreshCallback(CGRectCount count,