Use desktop relative mouse cursor position in DesktopAndCursorComposer

This change adds one temporary use_desktop_relative_cursor_position_ flag in
DesktopAndCursorComposer. It's automatically set according to the consturctor
used.
When the flag is true, DesktopAndCursorComposer uses the newly added
MouseCursorMonitor::Callback::OnCursorPosition(), which is the absolute position
of the cursor in the full desktop coordinate, and DesktopCapturer::IsOccluded()
to decide whether the mouse cursor should be drawn on the DesktopFrame.
When the flag is false, the behavior of DesktopAndCursorComposer is unchanged.

This flag will be removed together with the deprecated constructor of
DesktopAndCursorComposer.

Currently the new DesktopAndCursorComposer constructor is not used, so no
behavior change is expected.

Bug: webrtc:7950
Change-Id: I7235e32fa325a21c4a2594613764a9f81d76dfbc
Reviewed-on: https://chromium-review.googlesource.com/641075
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19592}

webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc

@@ -20,6 +20,7 @@
 #include "webrtc/modules/desktop_capture/mouse_cursor_monitor.h"
 #include "webrtc/rtc_base/checks.h"
 #include "webrtc/rtc_base/constructormagic.h"
+#include "webrtc/rtc_base/ptr_util.h"
 
 namespace webrtc {
 
@@ -116,7 +117,7 @@ DesktopFrameWithCursor::DesktopFrameWithCursor(
 
 DesktopFrameWithCursor::~DesktopFrameWithCursor() {
   // Restore original content of the frame.
-  if (restore_frame_.get()) {
+  if (restore_frame_) {
     DesktopRect target_rect = DesktopRect::MakeSize(restore_frame_->size());
     target_rect.Translate(restore_position_);
     CopyPixelsFrom(restore_frame_->data(), restore_frame_->stride(),
@@ -129,22 +130,31 @@ DesktopFrameWithCursor::~DesktopFrameWithCursor() {
 DesktopAndCursorComposer::DesktopAndCursorComposer(
     DesktopCapturer* desktop_capturer,
     MouseCursorMonitor* mouse_monitor)
-    : desktop_capturer_(desktop_capturer),
-      mouse_monitor_(mouse_monitor) {
-  RTC_DCHECK(desktop_capturer_);
-}
+    : DesktopAndCursorComposer(desktop_capturer, mouse_monitor, false) {}
 
 DesktopAndCursorComposer::DesktopAndCursorComposer(
     std::unique_ptr<DesktopCapturer> desktop_capturer,
     const DesktopCaptureOptions& options)
     : DesktopAndCursorComposer(desktop_capturer.release(),
-                               MouseCursorMonitor::Create(options).release()) {}
+                               MouseCursorMonitor::Create(options).release(),
+                               true) {}
+
+DesktopAndCursorComposer::DesktopAndCursorComposer(
+    DesktopCapturer* desktop_capturer,
+    MouseCursorMonitor* mouse_monitor,
+    bool use_desktop_relative_cursor_position)
+    : desktop_capturer_(desktop_capturer),
+      mouse_monitor_(mouse_monitor),
+      use_desktop_relative_cursor_position_(
+          use_desktop_relative_cursor_position) {
+  RTC_DCHECK(desktop_capturer_);
+}
 
 DesktopAndCursorComposer::~DesktopAndCursorComposer() = default;
 
 void DesktopAndCursorComposer::Start(DesktopCapturer::Callback* callback) {
   callback_ = callback;
-  if (mouse_monitor_.get())
+  if (mouse_monitor_)
     mouse_monitor_->Init(this, MouseCursorMonitor::SHAPE_AND_POSITION);
   desktop_capturer_->Start(this);
 }
@@ -155,7 +165,7 @@ void DesktopAndCursorComposer::SetSharedMemoryFactory(
 }
 
 void DesktopAndCursorComposer::CaptureFrame() {
-  if (mouse_monitor_.get())
+  if (mouse_monitor_)
     mouse_monitor_->Capture();
   desktop_capturer_->CaptureFrame();
 }
@@ -167,9 +177,21 @@ void DesktopAndCursorComposer::SetExcludedWindow(WindowId window) {
 void DesktopAndCursorComposer::OnCaptureResult(
     DesktopCapturer::Result result,
     std::unique_ptr<DesktopFrame> frame) {
-  if (frame && cursor_ && cursor_state_ == MouseCursorMonitor::INSIDE) {
-    frame = std::unique_ptr<DesktopFrameWithCursor>(new DesktopFrameWithCursor(
-        std::move(frame), *cursor_, cursor_position_));
+  if (frame && cursor_) {
+    if (use_desktop_relative_cursor_position_) {
+      if (frame->rect().Contains(cursor_position_) &&
+          !desktop_capturer_->IsOccluded(cursor_position_)) {
+        const DesktopVector relative_position =
+            cursor_position_.subtract(frame->top_left());
+        frame = rtc::MakeUnique<DesktopFrameWithCursor>(
+            std::move(frame), *cursor_, relative_position);
+      }
+    } else {
+      if (cursor_state_ == MouseCursorMonitor::INSIDE) {
+        frame = rtc::MakeUnique<DesktopFrameWithCursor>(
+            std::move(frame), *cursor_, cursor_position_);
+      }
+    }
   }
 
   callback_->OnCaptureResult(result, std::move(frame));
@@ -182,8 +204,17 @@ void DesktopAndCursorComposer::OnMouseCursor(MouseCursor* cursor) {
 void DesktopAndCursorComposer::OnMouseCursorPosition(
     MouseCursorMonitor::CursorState state,
     const DesktopVector& position) {
-  cursor_state_ = state;
-  cursor_position_ = position;
+  if (!use_desktop_relative_cursor_position_) {
+    cursor_state_ = state;
+    cursor_position_ = position;
+  }
+}
+
+void DesktopAndCursorComposer::OnMouseCursorPosition(
+    const DesktopVector& position) {
+  if (use_desktop_relative_cursor_position_) {
+    cursor_position_ = position;
+  }
 }
 
 }  // namespace webrtc

webrtc/modules/desktop_capture/desktop_and_cursor_composer.h

@@ -20,6 +20,9 @@
 
 namespace webrtc {
 
+template <bool use_desktop_relative_cursor_position>
+class DesktopAndCursorComposerTest;
+
 // A wrapper for DesktopCapturer that also captures mouse using specified
 // MouseCursorMonitor and renders it on the generated streams.
 class DesktopAndCursorComposer : public DesktopCapturer,
@@ -50,6 +53,16 @@ class DesktopAndCursorComposer : public DesktopCapturer,
   void SetExcludedWindow(WindowId window) override;
 
  private:
+  // Allows test cases to use a fake MouseCursorMonitor implementation.
+  friend class DesktopAndCursorComposerTest<true>;
+  friend class DesktopAndCursorComposerTest<false>;
+
+  // Constructor to delegate both deprecated and new constructors and allows
+  // test cases to use a fake MouseCursorMonitor implementation.
+  DesktopAndCursorComposer(DesktopCapturer* desktop_capturer,
+                           MouseCursorMonitor* mouse_monitor,
+                           bool use_desktop_relative_cursor_position);
+
   // DesktopCapturer::Callback interface.
   void OnCaptureResult(DesktopCapturer::Result result,
                        std::unique_ptr<DesktopFrame> frame) override;
@@ -58,13 +71,23 @@ class DesktopAndCursorComposer : public DesktopCapturer,
   void OnMouseCursor(MouseCursor* cursor) override;
   void OnMouseCursorPosition(MouseCursorMonitor::CursorState state,
                              const DesktopVector& position) override;
+  void OnMouseCursorPosition(const DesktopVector& position) override;
 
   const std::unique_ptr<DesktopCapturer> desktop_capturer_;
   const std::unique_ptr<MouseCursorMonitor> mouse_monitor_;
+  // This is a temporary flag to decide how to use the |mouse_monitor_|.
+  // If it's true, DesktopAndCursorComposer will use the absolute position from
+  // MouseCursorMonitor but ignore the MouseCursorMonitor::CursorState.
+  // Otherwise MouseCursorMonitor::CursorState is respected. This flag is false
+  // when the deprecated constructor is used, and true when the new one is used.
+  // This flag will be removed together with the deprecated constructor.
+  const bool use_desktop_relative_cursor_position_;
 
   DesktopCapturer::Callback* callback_;
 
   std::unique_ptr<MouseCursor> cursor_;
+  // This field is irrelevant if |use_desktop_relative_cursor_position_| is
+  // true.
   MouseCursorMonitor::CursorState cursor_state_;
   DesktopVector cursor_position_;
 

webrtc/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc

@@ -16,6 +16,7 @@
 #include "webrtc/modules/desktop_capture/desktop_frame.h"
 #include "webrtc/modules/desktop_capture/mouse_cursor.h"
 #include "webrtc/modules/desktop_capture/shared_desktop_frame.h"
+#include "webrtc/rtc_base/arraysize.h"
 #include "webrtc/test/gtest.h"
 
 namespace webrtc {
@@ -42,8 +43,7 @@ uint32_t GetFakeFramePixelValue(const DesktopVector& p) {
 }
 
 uint32_t GetFramePixel(const DesktopFrame& frame, const DesktopVector& pos) {
-  return *reinterpret_cast<uint32_t*>(frame.data() + pos.y() * frame.stride() +
-                                      pos.x() * DesktopFrame::kBytesPerPixel);
+  return *reinterpret_cast<uint32_t*>(frame.GetFrameDataAtPos(pos));
 }
 
 // Blends two pixel values taking into account alpha.
@@ -85,10 +85,15 @@ class FakeScreenCapturer : public DesktopCapturer {
     next_frame_ = std::move(next_frame);
   }
 
+  bool IsOccluded(const DesktopVector& pos) override { return is_occluded_; }
+
+  void set_is_occluded(bool value) { is_occluded_ = value; }
+
  private:
   Callback* callback_ = nullptr;
 
   std::unique_ptr<DesktopFrame> next_frame_;
+  bool is_occluded_ = false;
 };
 
 class FakeMouseMonitor : public MouseCursorMonitor {
@@ -127,6 +132,7 @@ class FakeMouseMonitor : public MouseCursorMonitor {
     }
 
     callback_->OnMouseCursorPosition(state_, position_);
+    callback_->OnMouseCursorPosition(position_);
   }
 
  private:
@@ -159,13 +165,20 @@ void VerifyFrame(const DesktopFrame& frame,
   }
 }
 
+}  // namespace
+
+template <bool use_desktop_relative_cursor_position>
 class DesktopAndCursorComposerTest : public testing::Test,
                                      public DesktopCapturer::Callback {
  public:
   DesktopAndCursorComposerTest()
       : fake_screen_(new FakeScreenCapturer()),
         fake_cursor_(new FakeMouseMonitor()),
-        blender_(fake_screen_, fake_cursor_) {}
+        blender_(fake_screen_,
+                 fake_cursor_,
+                 use_desktop_relative_cursor_position) {
+    blender_.Start(this);
+  }
 
   // DesktopCapturer::Callback interface
   void OnCaptureResult(DesktopCapturer::Result result,
@@ -182,11 +195,12 @@ class DesktopAndCursorComposerTest : public testing::Test,
   std::unique_ptr<DesktopFrame> frame_;
 };
 
+using DesktopAndCursorComposerWithRelativePositionTest =
+    DesktopAndCursorComposerTest<false>;
+
 // Verify DesktopAndCursorComposer can handle the case when the screen capturer
 // fails.
-TEST_F(DesktopAndCursorComposerTest, Error) {
-  blender_.Start(this);
-
+TEST_F(DesktopAndCursorComposerWithRelativePositionTest, Error) {
   fake_cursor_->SetHotspot(DesktopVector());
   fake_cursor_->SetState(MouseCursorMonitor::INSIDE, DesktopVector());
   fake_screen_->SetNextFrame(nullptr);
@@ -196,7 +210,7 @@ TEST_F(DesktopAndCursorComposerTest, Error) {
   EXPECT_FALSE(frame_);
 }
 
-TEST_F(DesktopAndCursorComposerTest, Blend) {
+TEST_F(DesktopAndCursorComposerWithRelativePositionTest, Blend) {
   struct {
     int x, y;
     int hotspot_x, hotspot_y;
@@ -217,9 +231,7 @@ TEST_F(DesktopAndCursorComposerTest, Blend) {
     {0, 0, 0, 0, false},
   };
 
-  blender_.Start(this);
-
-  for (size_t i = 0; i < (sizeof(tests) / sizeof(tests[0])); ++i) {
+  for (size_t i = 0; i < arraysize(tests); i++) {
     SCOPED_TRACE(i);
 
     DesktopVector hotspot(tests[i].hotspot_x, tests[i].hotspot_y);
@@ -246,6 +258,111 @@ TEST_F(DesktopAndCursorComposerTest, Blend) {
   }
 }
 
-}  // namespace
+using DesktopAndCursorComposerWithAbsolutePositionTest =
+    DesktopAndCursorComposerTest<true>;
+
+TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest,
+       CursorShouldBeIgnoredIfItIsOutOfDesktopFrame) {
+  std::unique_ptr<SharedDesktopFrame> frame(
+      SharedDesktopFrame::Wrap(CreateTestFrame()));
+  frame->set_top_left(DesktopVector(100, 200));
+  // The frame covers (100, 200) - (200, 300).
+
+  struct {
+    int x;
+    int y;
+  } tests[] = {
+    { 0, 0 },
+    { 50, 50 },
+    { 50, 150 },
+    { 100, 150 },
+    { 50, 200 },
+    { 99, 200 },
+    { 100, 199 },
+    { 200, 300 },
+    { 200, 299 },
+    { 199, 300 },
+    { -1, -1 },
+    { -10000, -10000 },
+    { 10000, 10000 },
+  };
+  for (size_t i = 0; i < arraysize(tests); i++) {
+    SCOPED_TRACE(i);
+
+    fake_screen_->SetNextFrame(frame->Share());
+    // The CursorState is ignored when using absolute cursor position.
+    fake_cursor_->SetState(MouseCursorMonitor::OUTSIDE,
+                           DesktopVector(tests[i].x, tests[i].y));
+    blender_.CaptureFrame();
+    VerifyFrame(*frame_, MouseCursorMonitor::OUTSIDE, DesktopVector(0, 0));
+  }
+}
+
+TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest,
+       IsOccludedShouldBeConsidered) {
+  std::unique_ptr<SharedDesktopFrame> frame(
+      SharedDesktopFrame::Wrap(CreateTestFrame()));
+  frame->set_top_left(DesktopVector(100, 200));
+  // The frame covers (100, 200) - (200, 300).
+
+  struct {
+    int x;
+    int y;
+  } tests[] = {
+    { 100, 200 },
+    { 101, 200 },
+    { 100, 201 },
+    { 101, 201 },
+    { 150, 250 },
+    { 199, 299 },
+  };
+  fake_screen_->set_is_occluded(true);
+  for (size_t i = 0; i < arraysize(tests); i++) {
+    SCOPED_TRACE(i);
+
+    fake_screen_->SetNextFrame(frame->Share());
+    // The CursorState is ignored when using absolute cursor position.
+    fake_cursor_->SetState(MouseCursorMonitor::OUTSIDE,
+                           DesktopVector(tests[i].x, tests[i].y));
+    blender_.CaptureFrame();
+    VerifyFrame(*frame_, MouseCursorMonitor::OUTSIDE, DesktopVector());
+  }
+}
+
+TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest, CursorIncluded) {
+  std::unique_ptr<SharedDesktopFrame> frame(
+      SharedDesktopFrame::Wrap(CreateTestFrame()));
+  frame->set_top_left(DesktopVector(100, 200));
+  // The frame covers (100, 200) - (200, 300).
+
+  struct {
+    int x;
+    int y;
+  } tests[] = {
+    { 100, 200 },
+    { 101, 200 },
+    { 100, 201 },
+    { 101, 201 },
+    { 150, 250 },
+    { 199, 299 },
+  };
+  for (size_t i = 0; i < arraysize(tests); i++) {
+    SCOPED_TRACE(i);
+
+    const DesktopVector abs_pos(tests[i].x, tests[i].y);
+    const DesktopVector rel_pos(abs_pos.subtract(frame->top_left()));
+
+    fake_screen_->SetNextFrame(frame->Share());
+    // The CursorState is ignored when using absolute cursor position.
+    fake_cursor_->SetState(MouseCursorMonitor::OUTSIDE, abs_pos);
+    blender_.CaptureFrame();
+    VerifyFrame(*frame_, MouseCursorMonitor::INSIDE, rel_pos);
+
+    // Verify that the cursor is erased before the frame buffer is returned to
+    // the screen capturer.
+    frame_.reset();
+    VerifyFrame(*frame, MouseCursorMonitor::OUTSIDE, DesktopVector());
+  }
+}
 
 }  // namespace webrtc

webrtc/modules/desktop_capture/desktop_frame.cc

@@ -58,6 +58,10 @@ void DesktopFrame::CopyPixelsFrom(const DesktopFrame& src_frame,
                  src_frame.stride(), dest_rect);
 }
 
+DesktopRect DesktopFrame::rect() const {
+  return DesktopRect::MakeOriginSize(top_left(), size());
+}
+
 uint8_t* DesktopFrame::GetFrameDataAtPos(const DesktopVector& pos) const {
   return data() + stride() * pos.y() + DesktopFrame::kBytesPerPixel * pos.x();
 }

webrtc/modules/desktop_capture/desktop_frame.h

@@ -30,6 +30,10 @@ class DesktopFrame {
 
   virtual ~DesktopFrame();
 
+  // Returns the rectangle in full desktop coordinates to indicate the area
+  // covered by the DesktopFrame.
+  DesktopRect rect() const;
+
   // Size of the frame.
   const DesktopSize& size() const { return size_; }
 

webrtc/modules/desktop_capture/shared_desktop_frame.cc

@@ -40,7 +40,9 @@ bool SharedDesktopFrame::ShareFrameWith(const SharedDesktopFrame& other) const {
 }
 
 std::unique_ptr<SharedDesktopFrame> SharedDesktopFrame::Share() {
-  return std::unique_ptr<SharedDesktopFrame>(new SharedDesktopFrame(core_));
+  std::unique_ptr<SharedDesktopFrame> result(new SharedDesktopFrame(core_));
+  result->CopyFrameInfoFrom(*this);
+  return result;
 }
 
 bool SharedDesktopFrame::IsShared() {