[desktopCapture] clean up relative positon processing
After deploying the new DesktopAndCursorComposer ctor in chromium in cl
https://chromium-review.googlesource.com/c/chromium/src/+/980668
The old ctor and relative stuffs can be removed now.
Bug: webrtc:9072
Change-Id: Ibbf23a374883c096b13169bd5289a2d4ece539fa
Reviewed-on: https://webrtc-review.googlesource.com/65341
Commit-Queue: Brave Yao <braveyao@webrtc.org>
Reviewed-by: Zijie He <zijiehe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#22679}diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc
index bc46a5c..6710bdf 100644
--- a/modules/desktop_capture/desktop_and_cursor_composer.cc
+++ b/modules/desktop_capture/desktop_and_cursor_composer.cc
@@ -128,25 +128,15 @@
} // namespace
DesktopAndCursorComposer::DesktopAndCursorComposer(
- DesktopCapturer* desktop_capturer,
- MouseCursorMonitor* mouse_monitor)
- : 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(),
- true) {}
+ MouseCursorMonitor::Create(options).release()) {}
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) {
+ MouseCursorMonitor* mouse_monitor)
+ : desktop_capturer_(desktop_capturer), mouse_monitor_(mouse_monitor) {
RTC_DCHECK(desktop_capturer_);
}
@@ -178,19 +168,12 @@
DesktopCapturer::Result result,
std::unique_ptr<DesktopFrame> frame) {
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_);
- }
+ 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);
}
}
@@ -204,17 +187,12 @@
void DesktopAndCursorComposer::OnMouseCursorPosition(
MouseCursorMonitor::CursorState state,
const DesktopVector& position) {
- if (!use_desktop_relative_cursor_position_) {
- cursor_state_ = state;
- cursor_position_ = position;
- }
+ RTC_NOTREACHED();
}
void DesktopAndCursorComposer::OnMouseCursorPosition(
const DesktopVector& position) {
- if (use_desktop_relative_cursor_position_) {
- cursor_position_ = position;
- }
+ cursor_position_ = position;
}
} // namespace webrtc
diff --git a/modules/desktop_capture/desktop_and_cursor_composer.h b/modules/desktop_capture/desktop_and_cursor_composer.h
index c88075a..7dff710 100644
--- a/modules/desktop_capture/desktop_and_cursor_composer.h
+++ b/modules/desktop_capture/desktop_and_cursor_composer.h
@@ -20,23 +20,12 @@
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,
public DesktopCapturer::Callback,
public MouseCursorMonitor::Callback {
public:
- // Creates a new blender that captures mouse cursor using |mouse_monitor| and
- // renders it into the frames generated by |desktop_capturer|. If
- // |mouse_monitor| is NULL the frames are passed unmodified. Takes ownership
- // of both arguments.
- // Deprecated: use the constructor below.
- DesktopAndCursorComposer(DesktopCapturer* desktop_capturer,
- MouseCursorMonitor* mouse_monitor);
-
// Creates a new blender that captures mouse cursor using
// MouseCursorMonitor::Create(options) and renders it into the frames
// generated by |desktop_capturer|.
@@ -54,14 +43,12 @@
private:
// Allows test cases to use a fake MouseCursorMonitor implementation.
- friend class DesktopAndCursorComposerTest<true>;
- friend class DesktopAndCursorComposerTest<false>;
+ friend class DesktopAndCursorComposerTest;
// 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);
+ MouseCursorMonitor* mouse_monitor);
// DesktopCapturer::Callback interface.
void OnCaptureResult(DesktopCapturer::Result result,
@@ -75,20 +62,10 @@
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_;
RTC_DISALLOW_COPY_AND_ASSIGN(DesktopAndCursorComposer);
diff --git a/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc b/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc
index 0725fac..9c3ab7b 100644
--- a/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc
+++ b/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc
@@ -131,7 +131,6 @@
callback_->OnMouseCursor(new MouseCursor(image.release(), hotspot_));
}
- callback_->OnMouseCursorPosition(state_, position_);
callback_->OnMouseCursorPosition(position_);
}
@@ -167,16 +166,13 @@
} // 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_,
- use_desktop_relative_cursor_position) {
+ blender_(fake_screen_, fake_cursor_) {
blender_.Start(this);
}
@@ -195,22 +191,7 @@
std::unique_ptr<DesktopFrame> frame_;
};
-using DesktopAndCursorComposerWithRelativePositionTest =
- DesktopAndCursorComposerTest<false>;
-
-// Verify DesktopAndCursorComposer can handle the case when the screen capturer
-// fails.
-TEST_F(DesktopAndCursorComposerWithRelativePositionTest, Error) {
- fake_cursor_->SetHotspot(DesktopVector());
- fake_cursor_->SetState(MouseCursorMonitor::INSIDE, DesktopVector());
- fake_screen_->SetNextFrame(nullptr);
-
- blender_.CaptureFrame();
-
- EXPECT_FALSE(frame_);
-}
-
-TEST_F(DesktopAndCursorComposerWithRelativePositionTest, Blend) {
+TEST_F(DesktopAndCursorComposerTest, CursorShouldBeIgnoredIfNoFrameCaptured) {
struct {
int x, y;
int hotspot_x, hotspot_y;
@@ -245,23 +226,15 @@
std::unique_ptr<SharedDesktopFrame> frame(
SharedDesktopFrame::Wrap(CreateTestFrame()));
- fake_screen_->SetNextFrame(frame->Share());
blender_.CaptureFrame();
-
- VerifyFrame(*frame_, state, pos);
-
- // Verify that the cursor is erased before the frame buffer is returned to
- // the screen capturer.
- frame_.reset();
- VerifyFrame(*frame, MouseCursorMonitor::OUTSIDE, DesktopVector());
+ // If capturer captured nothing, then cursor should be ignored, not matter
+ // its state or position.
+ EXPECT_EQ(frame_, nullptr);
}
}
-using DesktopAndCursorComposerWithAbsolutePositionTest =
- DesktopAndCursorComposerTest<true>;
-
-TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest,
+TEST_F(DesktopAndCursorComposerTest,
CursorShouldBeIgnoredIfItIsOutOfDesktopFrame) {
std::unique_ptr<SharedDesktopFrame> frame(
SharedDesktopFrame::Wrap(CreateTestFrame()));
@@ -298,8 +271,7 @@
}
}
-TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest,
- IsOccludedShouldBeConsidered) {
+TEST_F(DesktopAndCursorComposerTest, IsOccludedShouldBeConsidered) {
std::unique_ptr<SharedDesktopFrame> frame(
SharedDesktopFrame::Wrap(CreateTestFrame()));
frame->set_top_left(DesktopVector(100, 200));
@@ -329,7 +301,7 @@
}
}
-TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest, CursorIncluded) {
+TEST_F(DesktopAndCursorComposerTest, CursorIncluded) {
std::unique_ptr<SharedDesktopFrame> frame(
SharedDesktopFrame::Wrap(CreateTestFrame()));
frame->set_top_left(DesktopVector(100, 200));
diff --git a/modules/desktop_capture/mouse_cursor_monitor_mac.mm b/modules/desktop_capture/mouse_cursor_monitor_mac.mm
index b4a80e9..a9780b5 100644
--- a/modules/desktop_capture/mouse_cursor_monitor_mac.mm
+++ b/modules/desktop_capture/mouse_cursor_monitor_mac.mm
@@ -232,8 +232,6 @@
// Convert Density Independent Pixel to physical pixel.
position = DesktopVector(round(position.x() * scale),
round(position.y() * scale));
- // TODO(zijiehe): Remove this overload.
- callback_->OnMouseCursorPosition(state, position);
callback_->OnMouseCursorPosition(
position.subtract(configuration.bounds.top_left()));
}
diff --git a/modules/desktop_capture/mouse_cursor_monitor_unittest.cc b/modules/desktop_capture/mouse_cursor_monitor_unittest.cc
index 9af0d68..812873f 100644
--- a/modules/desktop_capture/mouse_cursor_monitor_unittest.cc
+++ b/modules/desktop_capture/mouse_cursor_monitor_unittest.cc
@@ -33,14 +33,16 @@
void OnMouseCursorPosition(MouseCursorMonitor::CursorState state,
const DesktopVector& position) override {
- state_ = state;
+ RTC_NOTREACHED();
+ }
+
+ void OnMouseCursorPosition(const DesktopVector& position) override {
position_ = position;
position_received_ = true;
}
protected:
std::unique_ptr<MouseCursor> cursor_image_;
- MouseCursorMonitor::CursorState state_;
DesktopVector position_;
bool position_received_;
};
@@ -78,7 +80,6 @@
cursor_image_->image()->size().height());
EXPECT_TRUE(position_received_);
- EXPECT_EQ(MouseCursorMonitor::INSIDE, state_);
}
TEST_F(MouseCursorMonitorTest, MAYBE(FromWindow)) {
diff --git a/modules/desktop_capture/mouse_cursor_monitor_win.cc b/modules/desktop_capture/mouse_cursor_monitor_win.cc
index a50743c..0c0a7b7 100644
--- a/modules/desktop_capture/mouse_cursor_monitor_win.cc
+++ b/modules/desktop_capture/mouse_cursor_monitor_win.cc
@@ -165,8 +165,6 @@
position = position.subtract(rect.top_left());
}
- // TODO(zijiehe): Remove this overload.
- callback_->OnMouseCursorPosition(inside ? INSIDE : OUTSIDE, position);
callback_->OnMouseCursorPosition(position);
}
diff --git a/modules/desktop_capture/mouse_cursor_monitor_x11.cc b/modules/desktop_capture/mouse_cursor_monitor_x11.cc
index aeb864e..039f9c3 100644
--- a/modules/desktop_capture/mouse_cursor_monitor_x11.cc
+++ b/modules/desktop_capture/mouse_cursor_monitor_x11.cc
@@ -201,8 +201,6 @@
}
}
- // TODO(zijiehe): Remove this overload.
- callback_->OnMouseCursorPosition(state, DesktopVector(win_x, win_y));
// X11 always starts the coordinate from (0, 0), so we do not need to
// translate here.
callback_->OnMouseCursorPosition(DesktopVector(root_x, root_y));
