The chromoting host expands dirty regions to a set of 16x16 blocks,

to prevent ghosting artefacts.

BUG=112830


Review URL: http://codereview.chromium.org/9391022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@123513 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 318 insertions, 7 deletions

diff --git a/remoting/host/capturer_helper.cc b/remoting/host/capturer_helper.cc
index cd7c465..4081e72 100644
--- a/remoting/host/capturer_helper.cc
+++ b/remoting/host/capturer_helper.cc
@@ -1,12 +1,19 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/capturer_helper.h"
 
+#include <algorithm>
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+
 namespace remoting {
 
-CapturerHelper::CapturerHelper() : size_most_recent_(SkISize::Make(0, 0)) {
+CapturerHelper::CapturerHelper() :
+    size_most_recent_(SkISize::Make(0, 0)),
+    log_grid_size_(0) {
 }
 
 CapturerHelper::~CapturerHelper() {
@@ -34,8 +41,21 @@ void CapturerHelper::InvalidateFullScreen() {
 }
 
 void CapturerHelper::SwapInvalidRegion(SkRegion* invalid_region) {
-  base::AutoLock auto_invalid_region_lock(invalid_region_lock_);
-  invalid_region->swap(invalid_region_);
+  {
+    base::AutoLock auto_invalid_region_lock(invalid_region_lock_);
+    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);
+  }
+}
+
+void CapturerHelper::SetLogGridSize(int log_grid_size) {
+  log_grid_size_ = log_grid_size;
 }
 
 const SkISize& CapturerHelper::size_most_recent() const {
@@ -46,4 +66,51 @@ void CapturerHelper::set_size_most_recent(const SkISize& size) {
   size_most_recent_ = size;
 }
 
+// Returns the largest multiple of |n| that is <= |x|.
+// |n| must be a power of 2. |nMask| is ~(|n| - 1).
+static int DownToMultiple(int x, int nMask) {
+  return (x & nMask);
+}
+
+// Returns the smallest multiple of |n| that is >= |x|.
+// |n| must be a power of 2. |nMask| is ~(|n| - 1).
+static int UpToMultiple(int x, int n, int nMask) {
+  return ((x + n - 1) & nMask);
+}
+
+scoped_ptr<SkRegion> CapturerHelper::ExpandToGrid(const SkRegion& region,
+                                                  int log_grid_size) {
+  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_array<SkIRect> rects(new SkIRect[rectNum]);
+  iter.rewind();
+  int rectI = 0;
+  while (!iter.done()) {
+    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);
+  }
+  // Make the union of the expanded rects.
+  scoped_ptr<SkRegion> regionNew(new SkRegion());
+  regionNew->setRects(rects.get(), rectNum);
+  return regionNew.Pass();
+}
+
 }  // namespace remoting
diff --git a/remoting/host/capturer_helper.h b/remoting/host/capturer_helper.h
index 10c95db..73cac99 100644
--- a/remoting/host/capturer_helper.h
+++ b/remoting/host/capturer_helper.h
@@ -1,10 +1,11 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef REMOTING_HOST_CAPTURER_HELPER_H_
 #define REMOTING_HOST_CAPTURER_HELPER_H_
 
+#include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
 #include "third_party/skia/include/core/SkRegion.h"
 
@@ -38,6 +39,24 @@ class CapturerHelper {
   const SkISize& size_most_recent() const;
   void set_size_most_recent(const SkISize& size);
 
+  // Lossy compression can result in color values leaking between pixels in one
+  // block. If part of a block changes, then unchanged parts of that block can
+  // be changed in the compressed output. So we need to re-render an entire
+  // block whenever part of the block changes.
+  //
+  // If |log_grid_size| is >= 1, then this function makes SwapInvalidRegion
+  // produce an invalid region expanded so that its vertices lie on a grid of
+  // size 2 ^ |log_grid_size|. The expanded region is then clipped to the size
+  // of the most recently captured screen, as previously set by
+  // set_size_most_recent().
+  // If |log_grid_size| is <= 0, then the invalid region is not expanded.
+  void SetLogGridSize(int log_grid_size);
+
+  // Expands a region so that its vertices all lie on a grid.
+  // The grid size must be >= 2, so |log_grid_size| must be >= 1.
+  static scoped_ptr<SkRegion> ExpandToGrid(const SkRegion& region,
+                                           int log_grid_size);
+
  private:
   // A region that has been manually invalidated (through InvalidateRegion).
   // These will be returned as dirty_region in the capture data during the next
@@ -50,6 +69,11 @@ class CapturerHelper {
   // The size of the most recently captured screen.
   SkISize size_most_recent_;
 
+  // The log (base 2) of the size of the grid to which the invalid region is
+  // expanded.
+  // If the value is <= 0, then the invalid region is not expanded to a grid.
+  int log_grid_size_;
+
   DISALLOW_COPY_AND_ASSIGN(CapturerHelper);
 };
 
diff --git a/remoting/host/capturer_helper_unittest.cc b/remoting/host/capturer_helper_unittest.cc
new file mode 100644
index 0000000..d1c9776
--- /dev/null
+++ b/remoting/host/capturer_helper_unittest.cc
@@ -0,0 +1,215 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "remoting/host/capturer_helper.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+class CapturerHelperTest : public testing::Test {
+ protected:
+  CapturerHelper capturer_helper_;
+};
+
+bool Equals(const SkRegion& region1, const SkRegion& region2) {
+  SkRegion::Iterator iter1(region1);
+  SkRegion::Iterator iter2(region2);
+  while (!iter1.done()) {
+    SkIRect rect1 = iter1.rect();
+    iter1.next();
+    if (iter2.done()) {
+      return false;
+    }
+    SkIRect rect2 = iter2.rect();
+    iter2.next();
+    if (rect1 != rect2) {
+      return false;
+    }
+  }
+  if (!iter2.done()) {
+    return false;
+  }
+  return true;
+}
+
+TEST_F(CapturerHelperTest, ClearInvalidRegion) {
+  SkRegion region;
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(1, 2, 3, 4)));
+  capturer_helper_.ClearInvalidRegion();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(region.isEmpty());
+}
+
+TEST_F(CapturerHelperTest, InvalidateRegion) {
+  SkRegion region;
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeEmpty()), region));
+
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(1, 2, 3, 4)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(1, 2, 3, 4)), region));
+
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(1, 2, 3, 4)));
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(4, 2, 3, 4)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(1, 2, 6, 4)), region));
+}
+
+TEST_F(CapturerHelperTest, InvalidateScreen) {
+  SkRegion region;
+  capturer_helper_.InvalidateScreen(SkISize::Make(12, 34));
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeWH(12, 34)), region));
+}
+
+TEST_F(CapturerHelperTest, InvalidateFullScreen) {
+  SkRegion region;
+  capturer_helper_.set_size_most_recent(SkISize::Make(12, 34));
+  capturer_helper_.InvalidateFullScreen();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeWH(12, 34)), region));
+}
+
+TEST_F(CapturerHelperTest, SizeMostRecent) {
+  ASSERT_EQ(SkISize::Make(0, 0), capturer_helper_.size_most_recent());
+  capturer_helper_.set_size_most_recent(SkISize::Make(12, 34));
+  ASSERT_EQ(SkISize::Make(12, 34), capturer_helper_.size_most_recent());
+}
+
+TEST_F(CapturerHelperTest, SetLogGridSize) {
+  capturer_helper_.set_size_most_recent(SkISize::Make(10, 10));
+
+  SkRegion region;
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeEmpty()), region));
+
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)), region));
+
+  capturer_helper_.SetLogGridSize(-1);
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)), region));
+
+  capturer_helper_.SetLogGridSize(0);
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)), region));
+
+  capturer_helper_.SetLogGridSize(1);
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(6, 6, 2, 2)), region));
+
+  capturer_helper_.SetLogGridSize(2);
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(4, 4, 4, 4)), region));
+
+  capturer_helper_.SetLogGridSize(0);
+  capturer_helper_.InvalidateRegion(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)));
+  region.setEmpty();
+  capturer_helper_.SwapInvalidRegion(&region);
+  ASSERT_TRUE(Equals(SkRegion(SkIRect::MakeXYWH(7, 7, 1, 1)), region));
+}
+
+void TestExpandRegionToGrid(const SkRegion& region, int log_grid_size,
+                            const SkRegion& expandedRegionExpected) {
+  scoped_ptr<SkRegion> expandedRegion1(
+      CapturerHelper::ExpandToGrid(region, log_grid_size));
+  ASSERT_TRUE(Equals(expandedRegionExpected, *expandedRegion1));
+  scoped_ptr<SkRegion> expandedRegion2(
+      CapturerHelper::ExpandToGrid(*expandedRegion1, log_grid_size));
+  ASSERT_TRUE(Equals(*expandedRegion1, *expandedRegion2));
+}
+
+void TestExpandRectToGrid(int l, int t, int r, int b, int log_grid_size,
+                          int lExpanded, int tExpanded,
+                          int rExpanded, int bExpanded) {
+  TestExpandRegionToGrid(SkRegion(SkIRect::MakeLTRB(l, t, r, b)), log_grid_size,
+                         SkRegion(SkIRect::MakeLTRB(lExpanded, tExpanded,
+                                                    rExpanded, bExpanded)));
+}
+
+TEST_F(CapturerHelperTest, ExpandToGrid) {
+  const int LOG_GRID_SIZE = 4;
+  const int GRID_SIZE = 1 << LOG_GRID_SIZE;
+  for (int i = -2; i <= 2; i++) {
+    int x = i * GRID_SIZE;
+    for (int j = -2; j <= 2; j++) {
+      int y = j * GRID_SIZE;
+      TestExpandRectToGrid(x + 0, y + 0, x + 1, y + 1, LOG_GRID_SIZE,
+                           x + 0, y + 0, x + GRID_SIZE, y + GRID_SIZE);
+      TestExpandRectToGrid(x + 0, y + GRID_SIZE - 1, x + 1, y + GRID_SIZE,
+                           LOG_GRID_SIZE,
+                           x + 0, y + 0, x + GRID_SIZE, y + GRID_SIZE);
+      TestExpandRectToGrid(x + GRID_SIZE - 1, y + GRID_SIZE - 1,
+                           x + GRID_SIZE, y + GRID_SIZE, LOG_GRID_SIZE,
+                           x + 0, y + 0, x + GRID_SIZE, y + GRID_SIZE);
+      TestExpandRectToGrid(x + GRID_SIZE - 1, y + 0,
+                           x + GRID_SIZE, y + 1, LOG_GRID_SIZE,
+                           x + 0, y + 0, x + GRID_SIZE, y + GRID_SIZE);
+      TestExpandRectToGrid(x - 1, y + 0, x + 1, y + 1, LOG_GRID_SIZE,
+                           x - GRID_SIZE, y + 0, x + GRID_SIZE, y + GRID_SIZE);
+      TestExpandRectToGrid(x - 1, y - 1, x + 1, y + 0, LOG_GRID_SIZE,
+                           x - GRID_SIZE, y - GRID_SIZE, x + GRID_SIZE, y);
+      TestExpandRectToGrid(x + 0, y - 1, x + 1, y + 1, LOG_GRID_SIZE,
+                           x, y - GRID_SIZE, x + GRID_SIZE, y + GRID_SIZE);
+      TestExpandRectToGrid(x - 1, y - 1, x + 0, y + 1, LOG_GRID_SIZE,
+                           x - GRID_SIZE, y - GRID_SIZE, x, y + GRID_SIZE);
+
+      SkRegion region(SkIRect::MakeLTRB(x - 1, y - 1, x + 1, y + 1));
+      region.op(SkIRect::MakeLTRB(x - 1, y - 1, x + 0, y + 0),
+                SkRegion::kDifference_Op);
+      SkRegion expandedRegionExpected(SkIRect::MakeLTRB(
+          x - GRID_SIZE, y - GRID_SIZE, x + GRID_SIZE, y + GRID_SIZE));
+      expandedRegionExpected.op(
+          SkIRect::MakeLTRB(x - GRID_SIZE, y - GRID_SIZE, x + 0, y + 0),
+                            SkRegion::kDifference_Op);
+      TestExpandRegionToGrid(region, LOG_GRID_SIZE, expandedRegionExpected);
+
+      region.setRect(SkIRect::MakeLTRB(x - 1, y - 1, x + 1, y + 1));
+      region.op(SkIRect::MakeLTRB(x - 1, y + 0, x + 0, y + 1),
+                SkRegion::kDifference_Op);
+      expandedRegionExpected.setRect(SkIRect::MakeLTRB(
+          x - GRID_SIZE, y - GRID_SIZE, x + GRID_SIZE, y + GRID_SIZE));
+      expandedRegionExpected.op(
+          SkIRect::MakeLTRB(x - GRID_SIZE, y + 0, x + 0, y + GRID_SIZE),
+                            SkRegion::kDifference_Op);
+      TestExpandRegionToGrid(region, LOG_GRID_SIZE, expandedRegionExpected);
+
+      region.setRect(SkIRect::MakeLTRB(x - 1, y - 1, x + 1, y + 1));
+      region.op(SkIRect::MakeLTRB(x + 0, y + 0, x + 1, y + 1),
+                SkRegion::kDifference_Op);
+      expandedRegionExpected.setRect(SkIRect::MakeLTRB(
+          x - GRID_SIZE, y - GRID_SIZE, x + GRID_SIZE, y + GRID_SIZE));
+      expandedRegionExpected.op(
+          SkIRect::MakeLTRB(x + 0, y + 0, x + GRID_SIZE, y + GRID_SIZE),
+                            SkRegion::kDifference_Op);
+      TestExpandRegionToGrid(region, LOG_GRID_SIZE, expandedRegionExpected);
+
+      region.setRect(SkIRect::MakeLTRB(x - 1, y - 1, x + 1, y + 1));
+      region.op(SkIRect::MakeLTRB(x + 0, y - 1, x + 1, y + 0),
+                SkRegion::kDifference_Op);
+      expandedRegionExpected.setRect(SkIRect::MakeLTRB(
+          x - GRID_SIZE, y - GRID_SIZE, x + GRID_SIZE, y + GRID_SIZE));
+      expandedRegionExpected.op(
+          SkIRect::MakeLTRB(x + 0, y - GRID_SIZE, x + GRID_SIZE, y + 0),
+                            SkRegion::kDifference_Op);
+      TestExpandRegionToGrid(region, LOG_GRID_SIZE, expandedRegionExpected);
+    }
+  }
+}
+
+}  // namespace remoting
diff --git a/remoting/host/capturer_linux.cc b/remoting/host/capturer_linux.cc
index 9c90716..85e03ab 100644
--- a/remoting/host/capturer_linux.cc
+++ b/remoting/host/capturer_linux.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -172,6 +172,7 @@ CapturerLinux::CapturerLinux()
       current_buffer_(0),
       pixel_format_(media::VideoFrame::RGB32),
       last_buffer_(NULL) {
+  helper_.SetLogGridSize(4);
 }
 
 CapturerLinux::~CapturerLinux() {
@@ -284,7 +285,6 @@ void CapturerLinux::CaptureInvalidRegion(
   scoped_refptr<CaptureData> capture_data(CaptureFrame());
 
   current_buffer_ = (current_buffer_ + 1) % kNumBuffers;
-  helper_.set_size_most_recent(capture_data->size());
 
   callback.Run(capture_data);
 }
@@ -333,6 +333,10 @@ CaptureData* CapturerLinux::CaptureFrame() {
   CaptureData* capture_data = new CaptureData(planes, buffer.size(),
                                               media::VideoFrame::RGB32);
 
+  // Pass the screen size to the helper, so it can clip the invalid region if it
+  // expands that region to a grid.
+  helper_.set_size_most_recent(capture_data->size());
+
   // In the DAMAGE case, ensure the frame is up-to-date with the previous frame
   // if any.  If there isn't a previous frame, that means a screen-resolution
   // change occurred, and |invalid_rects| will be updated to include the whole
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index 6207cc7..4121453 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -932,6 +932,7 @@
         'base/base_mock_objects.h',
         'base/util_unittest.cc',
         'client/mouse_input_filter_unittest.cc',
+        'host/capturer_helper_unittest.cc',
         'host/capturer_linux_unittest.cc',
         'host/capturer_mac_unittest.cc',
         'host/capturer_win_unittest.cc',