Use webrtc::DesktopCapturer for screen capturer implementation.

media/video/capture/screen/screen_capturer_helper.cc

Index: media/video/capture/screen/screen_capturer_helper.cc
diff --git a/media/video/capture/screen/screen_capturer_helper.cc b/media/video/capture/screen/screen_capturer_helper.cc
index 2f4534260c9cb0190481a9a3115988b4f6dce087..3753693afc5aac6fbc89737f6b3a8489b031197a 100644
--- a/media/video/capture/screen/screen_capturer_helper.cc
+++ b/media/video/capture/screen/screen_capturer_helper.cc
@@ -12,7 +12,6 @@
 namespace media {
 
 ScreenCapturerHelper::ScreenCapturerHelper() :
-    size_most_recent_(SkISize::Make(0, 0)),
     log_grid_size_(0) {
 }
 
@@ -25,33 +24,55 @@ void ScreenCapturerHelper::ClearInvalidRegion() {
 }
 
 void ScreenCapturerHelper::InvalidateRegion(
-    const SkRegion& invalid_region) {
+    const webrtc::DesktopRegion& invalid_region) {
   base::AutoLock auto_invalid_region_lock(invalid_region_lock_);
-  invalid_region_.op(invalid_region, SkRegion::kUnion_Op);
+  for (webrtc::DesktopRegion::Iterator it(invalid_region);
+       !it.IsAtEnd(); it.Advance()) {
+    invalid_region_.op(SkIRect::MakeLTRB(it.rect().left(), it.rect().top(),

[alexeypa (please no reviews), 2013/04/26 21:33:58]

nit: Add a TODO to replace SkRegion with webrtc::DesktopRegion?

[Sergey Ulanov, 2013/05/07 22:25:50]

I don't think we need TODO for it - it's something we will have to do anyway
before this code is moved to webrtc and there are many where such todo would
need to be added.

+                                         it.rect().top(), it.rect().bottom()),
+                       SkRegion::kUnion_Op);
+  }
 }
 
-void ScreenCapturerHelper::InvalidateScreen(const SkISize& size) {
+void ScreenCapturerHelper::InvalidateScreen(const webrtc::DesktopSize& size) {
   base::AutoLock auto_invalid_region_lock(invalid_region_lock_);
   invalid_region_.op(SkIRect::MakeWH(size.width(), size.height()),
                      SkRegion::kUnion_Op);
 }
 
 void ScreenCapturerHelper::InvalidateFullScreen() {
-  if (!size_most_recent_.isZero())
+  if (!size_most_recent_.is_empty())
     InvalidateScreen(size_most_recent_);
 }
 
-void ScreenCapturerHelper::SwapInvalidRegion(SkRegion* invalid_region) {
+void ScreenCapturerHelper::SwapInvalidRegion(

[Wez, 2013/04/26 18:48:14]

nit: IIRC this is a Swap simply to avoid unnecessary re-allocations; should be
fine to TakeInvalidRegion(), for example.

[Sergey Ulanov, 2013/05/07 22:25:50]

The X11 capturer seems to rely on Swap() - it calls InvalidateRegion() and then
SwapInvalidRegion().

[Sergey Ulanov, 2013/05/08 01:26:49]

Actually it looks like Swap wasn't necessary for X11 capturer either. Replaced
this method with TakeInvalidRegion().

+    webrtc::DesktopRegion* invalid_region) {
+  SkRegion sk_invalid_region;
+  for (webrtc::DesktopRegion::Iterator it(*invalid_region);
+       !it.IsAtEnd(); it.Advance()) {
+    sk_invalid_region.op(SkIRect::MakeLTRB(it.rect().left(), it.rect().top(),
+                                           it.rect().top(), it.rect().bottom()),
+                         SkRegion::kUnion_Op);
+  }
+
   {
     base::AutoLock auto_invalid_region_lock(invalid_region_lock_);
-    invalid_region->swap(invalid_region_);
+    sk_invalid_region.swap(invalid_region_);
   }
+
   if (log_grid_size_ > 0) {
     scoped_ptr<SkRegion> expanded_region(
-        ExpandToGrid(*invalid_region, log_grid_size_));
-    invalid_region->swap(*expanded_region);
-    invalid_region->op(SkRegion(SkIRect::MakeSize(size_most_recent_)),
-                       SkRegion::kIntersect_Op);
+        ExpandToGrid(sk_invalid_region, log_grid_size_));
+    sk_invalid_region.swap(*expanded_region);
+    sk_invalid_region.op(SkIRect::MakeWH(size_most_recent_.width(),
+                                         size_most_recent_.height()),
+                         SkRegion::kIntersect_Op);
+  }
+  invalid_region->Clear();
+  for (SkRegion::Iterator it(sk_invalid_region); !it.done(); it.next()) {
+    invalid_region->AddRect(webrtc::DesktopRect::MakeLTRB(
+        it.rect().left(), it.rect().top(),
+        it.rect().right(), it.rect().bottom()));
   }
 }
 
@@ -59,11 +80,12 @@ void ScreenCapturerHelper::SetLogGridSize(int log_grid_size) {
   log_grid_size_ = log_grid_size;
 }
 
-const SkISize& ScreenCapturerHelper::size_most_recent() const {
+const webrtc::DesktopSize& ScreenCapturerHelper::size_most_recent() const {
   return size_most_recent_;
 }
 
-void ScreenCapturerHelper::set_size_most_recent(const SkISize& size) {
+void ScreenCapturerHelper::set_size_most_recent(
+    const webrtc::DesktopSize& size) {
   size_most_recent_ = size;
 }
 
@@ -85,34 +107,19 @@ scoped_ptr<SkRegion> ScreenCapturerHelper::ExpandToGrid(
   DCHECK(log_grid_size >= 1);
   int grid_size = 1 << log_grid_size;
   int grid_size_mask = ~(grid_size - 1);
-  // Count the rects in the region.
-  int rectNum = 0;
-  SkRegion::Iterator iter(region);
-  while (!iter.done()) {
-    iter.next();
-    ++rectNum;
-  }
+
   // Expand each rect.
-  scoped_ptr<SkIRect[]> rects(new SkIRect[rectNum]);
-  iter.rewind();
-  int rectI = 0;
-  while (!iter.done()) {
+  scoped_ptr<SkRegion> expanded_region(new SkRegion());
+  for (SkRegion::Iterator iter(region); !iter.done(); iter.next()) {
     SkIRect rect = iter.rect();
-    iter.next();
-    int left = std::min(rect.left(), rect.right());
-    int right = std::max(rect.left(), rect.right());
-    int top = std::min(rect.top(), rect.bottom());
-    int bottom = std::max(rect.top(), rect.bottom());
-    left = DownToMultiple(left, grid_size_mask);
-    right = UpToMultiple(right, grid_size, grid_size_mask);
-    top = DownToMultiple(top, grid_size_mask);
-    bottom = UpToMultiple(bottom, grid_size, grid_size_mask);
-    rects[rectI++] = SkIRect::MakeLTRB(left, top, right, bottom);
+    int left = DownToMultiple(rect.left(), grid_size_mask);
+    int right = UpToMultiple(rect.right(), grid_size, grid_size_mask);
+    int top = DownToMultiple(rect.top(), grid_size_mask);
+    int bottom = UpToMultiple(rect.bottom(), grid_size, grid_size_mask);
+    expanded_region->op(SkIRect::MakeLTRB(left, right, top, bottom),

[alexeypa (please no reviews), 2013/04/26 21:33:58]

nit: SkRegion::setRects() is potentially more optimal than calling op(...,
kUnion) in a loop. It can sort the rects construct the region utilizing this
fact.  

[Sergey Ulanov, 2013/05/07 22:25:50]

It doesn't at the moment and I suspect the difference that optimization makes is
marginal at best, not enough to justify making this code more complex. Also we
are going to switch to DesktopRegion in the future anyway. In either case, it's
not related to this CL, so I reverted this change.

+                        SkRegion::kUnion_Op);
   }
-  // Make the union of the expanded rects.
-  scoped_ptr<SkRegion> regionNew(new SkRegion());
-  regionNew->setRects(rects.get(), rectNum);
-  return regionNew.Pass();
+  return expanded_region.Pass();
 }
 
 }  // namespace media