From 5a74ea0a97ba182f8ed3e99310164def18ee1a11 Mon Sep 17 00:00:00 2001 From: braveyao Date: Thu, 29 Mar 2018 10:36:14 -0700 Subject: [PATCH] [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 Reviewed-by: Zijie He Cr-Commit-Position: refs/heads/master@{#22679} --- .../desktop_and_cursor_composer.cc | 44 +++++-------------- .../desktop_and_cursor_composer.h | 27 +----------- .../desktop_and_cursor_composer_unittest.cc | 44 ++++--------------- .../mouse_cursor_monitor_mac.mm | 2 - .../mouse_cursor_monitor_unittest.cc | 7 +-- .../mouse_cursor_monitor_win.cc | 2 - .../mouse_cursor_monitor_x11.cc | 2 - 7 files changed, 25 insertions(+), 103 deletions(-) diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc index bc46a5c528..6710bdfd87 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer.cc +++ b/modules/desktop_capture/desktop_and_cursor_composer.cc @@ -127,26 +127,16 @@ DesktopFrameWithCursor::~DesktopFrameWithCursor() { } // namespace -DesktopAndCursorComposer::DesktopAndCursorComposer( - DesktopCapturer* desktop_capturer, - MouseCursorMonitor* mouse_monitor) - : DesktopAndCursorComposer(desktop_capturer, mouse_monitor, false) {} - DesktopAndCursorComposer::DesktopAndCursorComposer( std::unique_ptr 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 @@ void DesktopAndCursorComposer::OnCaptureResult( DesktopCapturer::Result result, std::unique_ptr 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( - std::move(frame), *cursor_, relative_position); - } - } else { - if (cursor_state_ == MouseCursorMonitor::INSIDE) { - frame = rtc::MakeUnique( - 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( + std::move(frame), *cursor_, relative_position); } } @@ -204,17 +187,12 @@ void DesktopAndCursorComposer::OnMouseCursor(MouseCursor* cursor) { 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 c88075a986..7dff7101f3 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 -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 @@ class DesktopAndCursorComposer : public DesktopCapturer, private: // Allows test cases to use a fake MouseCursorMonitor implementation. - friend class DesktopAndCursorComposerTest; - friend class DesktopAndCursorComposerTest; + 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 @@ class DesktopAndCursorComposer : public DesktopCapturer, const std::unique_ptr desktop_capturer_; const std::unique_ptr 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 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 0725facfcb..9c3ab7bc81 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 @@ class FakeMouseMonitor : public MouseCursorMonitor { callback_->OnMouseCursor(new MouseCursor(image.release(), hotspot_)); } - callback_->OnMouseCursorPosition(state_, position_); callback_->OnMouseCursorPosition(position_); } @@ -167,16 +166,13 @@ void VerifyFrame(const DesktopFrame& frame, } // namespace -template 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 @@ class DesktopAndCursorComposerTest : public testing::Test, std::unique_ptr frame_; }; -using DesktopAndCursorComposerWithRelativePositionTest = - DesktopAndCursorComposerTest; - -// 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 @@ TEST_F(DesktopAndCursorComposerWithRelativePositionTest, Blend) { std::unique_ptr 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; - -TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest, +TEST_F(DesktopAndCursorComposerTest, CursorShouldBeIgnoredIfItIsOutOfDesktopFrame) { std::unique_ptr frame( SharedDesktopFrame::Wrap(CreateTestFrame())); @@ -298,8 +271,7 @@ TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest, } } -TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest, - IsOccludedShouldBeConsidered) { +TEST_F(DesktopAndCursorComposerTest, IsOccludedShouldBeConsidered) { std::unique_ptr frame( SharedDesktopFrame::Wrap(CreateTestFrame())); frame->set_top_left(DesktopVector(100, 200)); @@ -329,7 +301,7 @@ TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest, } } -TEST_F(DesktopAndCursorComposerWithAbsolutePositionTest, CursorIncluded) { +TEST_F(DesktopAndCursorComposerTest, CursorIncluded) { std::unique_ptr 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 b4a80e9c34..a9780b52b5 100644 --- a/modules/desktop_capture/mouse_cursor_monitor_mac.mm +++ b/modules/desktop_capture/mouse_cursor_monitor_mac.mm @@ -232,8 +232,6 @@ void MouseCursorMonitorMac::Capture() { // 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 9af0d68efc..812873fa61 100644 --- a/modules/desktop_capture/mouse_cursor_monitor_unittest.cc +++ b/modules/desktop_capture/mouse_cursor_monitor_unittest.cc @@ -33,14 +33,16 @@ class MouseCursorMonitorTest : public testing::Test, 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 cursor_image_; - MouseCursorMonitor::CursorState state_; DesktopVector position_; bool position_received_; }; @@ -78,7 +80,6 @@ TEST_F(MouseCursorMonitorTest, MAYBE(FromScreen)) { 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 a50743cc6e..0c0a7b7759 100644 --- a/modules/desktop_capture/mouse_cursor_monitor_win.cc +++ b/modules/desktop_capture/mouse_cursor_monitor_win.cc @@ -165,8 +165,6 @@ void MouseCursorMonitorWin::Capture() { 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 aeb864e2bd..039f9c37b8 100644 --- a/modules/desktop_capture/mouse_cursor_monitor_x11.cc +++ b/modules/desktop_capture/mouse_cursor_monitor_x11.cc @@ -201,8 +201,6 @@ void MouseCursorMonitorX11::Capture() { } } - // 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));