From f3886aea860e7adce3881d3bb955ca75085e1111 Mon Sep 17 00:00:00 2001 From: Jamie Walch Date: Wed, 22 Jan 2020 09:35:59 -0800 Subject: [PATCH] Include cursor rects in updated_region. DesktopAndCursorComposer adds the cursor image to the desktop, but does not change the updated_region, so it generally doesn't encode correctly unless the mouse is moving over a region that is changing. This CL extends the updated region to include the union of the old and new cursor rects, with an optimization for the case where the cursor has neither moved nor changed. Bug: chromium:1043325 Change-Id: I52076c96528820833fda6aa95f5b1fbc0f613909 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166545 Reviewed-by: Sergey Ulanov Commit-Queue: Sergey Ulanov Cr-Commit-Position: refs/heads/master@{#30374} --- .../desktop_and_cursor_composer.cc | 55 +++++--- .../desktop_and_cursor_composer.h | 2 + .../desktop_and_cursor_composer_unittest.cc | 118 +++++++++++++++--- 3 files changed, 141 insertions(+), 34 deletions(-) diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc index 1061ec5157..328cceb419 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer.cc +++ b/modules/desktop_capture/desktop_and_cursor_composer.cc @@ -67,14 +67,19 @@ class DesktopFrameWithCursor : public DesktopFrame { // Takes ownership of |frame|. DesktopFrameWithCursor(std::unique_ptr frame, const MouseCursor& cursor, - const DesktopVector& position); + const DesktopVector& position, + const DesktopRect& previous_cursor_rect, + bool cursor_changed); ~DesktopFrameWithCursor() override; + DesktopRect cursor_rect() const { return cursor_rect_; } + private: const std::unique_ptr original_frame_; DesktopVector restore_position_; std::unique_ptr restore_frame_; + DesktopRect cursor_rect_; RTC_DISALLOW_COPY_AND_ASSIGN(DesktopFrameWithCursor); }; @@ -82,7 +87,9 @@ class DesktopFrameWithCursor : public DesktopFrame { DesktopFrameWithCursor::DesktopFrameWithCursor( std::unique_ptr frame, const MouseCursor& cursor, - const DesktopVector& position) + const DesktopVector& position, + const DesktopRect& previous_cursor_rect, + bool cursor_changed) : DesktopFrame(frame->size(), frame->stride(), frame->data(), @@ -91,30 +98,37 @@ DesktopFrameWithCursor::DesktopFrameWithCursor( MoveFrameInfoFrom(original_frame_.get()); DesktopVector image_pos = position.subtract(cursor.hotspot()); - DesktopRect target_rect = DesktopRect::MakeSize(cursor.image()->size()); - target_rect.Translate(image_pos); - DesktopVector target_origin = target_rect.top_left(); - target_rect.IntersectWith(DesktopRect::MakeSize(size())); + cursor_rect_ = DesktopRect::MakeSize(cursor.image()->size()); + cursor_rect_.Translate(image_pos); + DesktopVector cursor_origin = cursor_rect_.top_left(); + cursor_rect_.IntersectWith(DesktopRect::MakeSize(size())); - if (target_rect.is_empty()) + if (!previous_cursor_rect.equals(cursor_rect_)) { + mutable_updated_region()->AddRect(cursor_rect_); + mutable_updated_region()->AddRect(previous_cursor_rect); + } else if (cursor_changed) { + mutable_updated_region()->AddRect(cursor_rect_); + } + + if (cursor_rect_.is_empty()) return; // Copy original screen content under cursor to |restore_frame_|. - restore_position_ = target_rect.top_left(); - restore_frame_.reset(new BasicDesktopFrame(target_rect.size())); - restore_frame_->CopyPixelsFrom(*this, target_rect.top_left(), + restore_position_ = cursor_rect_.top_left(); + restore_frame_.reset(new BasicDesktopFrame(cursor_rect_.size())); + restore_frame_->CopyPixelsFrom(*this, cursor_rect_.top_left(), DesktopRect::MakeSize(restore_frame_->size())); // Blit the cursor. - uint8_t* target_rect_data = reinterpret_cast(data()) + - target_rect.top() * stride() + - target_rect.left() * DesktopFrame::kBytesPerPixel; - DesktopVector origin_shift = target_rect.top_left().subtract(target_origin); - AlphaBlend(target_rect_data, stride(), + uint8_t* cursor_rect_data = + reinterpret_cast(data()) + cursor_rect_.top() * stride() + + cursor_rect_.left() * DesktopFrame::kBytesPerPixel; + DesktopVector origin_shift = cursor_rect_.top_left().subtract(cursor_origin); + AlphaBlend(cursor_rect_data, stride(), cursor.image()->data() + origin_shift.y() * cursor.image()->stride() + origin_shift.x() * DesktopFrame::kBytesPerPixel, - cursor.image()->stride(), target_rect.size()); + cursor.image()->stride(), cursor_rect_.size()); } DesktopFrameWithCursor::~DesktopFrameWithCursor() { @@ -192,8 +206,12 @@ void DesktopAndCursorComposer::OnCaptureResult( relative_position.set(relative_position.x() * scale, relative_position.y() * scale); #endif - frame = std::make_unique( - std::move(frame), *cursor_, relative_position); + auto frame_with_cursor = std::make_unique( + std::move(frame), *cursor_, relative_position, previous_cursor_rect_, + cursor_changed_); + previous_cursor_rect_ = frame_with_cursor->cursor_rect(); + cursor_changed_ = false; + frame = std::move(frame_with_cursor); } } @@ -201,6 +219,7 @@ void DesktopAndCursorComposer::OnCaptureResult( } void DesktopAndCursorComposer::OnMouseCursor(MouseCursor* cursor) { + cursor_changed_ = true; cursor_.reset(cursor); } diff --git a/modules/desktop_capture/desktop_and_cursor_composer.h b/modules/desktop_capture/desktop_and_cursor_composer.h index 8958d0ea7f..4219c4da30 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer.h +++ b/modules/desktop_capture/desktop_and_cursor_composer.h @@ -78,6 +78,8 @@ class RTC_EXPORT DesktopAndCursorComposer std::unique_ptr cursor_; DesktopVector cursor_position_; + DesktopRect previous_cursor_rect_; + bool cursor_changed_ = false; 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 ad8f38c25e..c9cb56d8c2 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc +++ b/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc @@ -85,6 +85,20 @@ DesktopFrame* CreateTestFrame() { return frame; } +MouseCursor* CreateTestCursor(DesktopVector hotspot) { + std::unique_ptr image( + new BasicDesktopFrame(DesktopSize(kCursorWidth, kCursorHeight))); + uint32_t* data = reinterpret_cast(image->data()); + // Set four pixels near the hotspot and leave all other blank. + for (int y = 0; y < kTestCursorSize; ++y) { + for (int x = 0; x < kTestCursorSize; ++x) { + data[(hotspot.y() + y) * kCursorWidth + (hotspot.x() + x)] = + kTestCursorData[y][x]; + } + } + return new MouseCursor(image.release(), hotspot); +} + class FakeScreenCapturer : public DesktopCapturer { public: FakeScreenCapturer() {} @@ -131,21 +145,8 @@ class FakeMouseMonitor : public MouseCursorMonitor { void Capture() override { if (changed_) { - std::unique_ptr image( - new BasicDesktopFrame(DesktopSize(kCursorWidth, kCursorHeight))); - uint32_t* data = reinterpret_cast(image->data()); - - // Set four pixels near the hotspot and leave all other blank. - for (int y = 0; y < kTestCursorSize; ++y) { - for (int x = 0; x < kTestCursorSize; ++x) { - data[(hotspot_.y() + y) * kCursorWidth + (hotspot_.x() + x)] = - kTestCursorData[y][x]; - } - } - - callback_->OnMouseCursor(new MouseCursor(image.release(), hotspot_)); + callback_->OnMouseCursor(CreateTestCursor(hotspot_)); } - callback_->OnMouseCursorPosition(position_); } @@ -184,9 +185,9 @@ void VerifyFrame(const DesktopFrame& frame, class DesktopAndCursorComposerTest : public ::testing::Test, public DesktopCapturer::Callback { public: - DesktopAndCursorComposerTest() + DesktopAndCursorComposerTest(bool include_cursor = true) : fake_screen_(new FakeScreenCapturer()), - fake_cursor_(new FakeMouseMonitor()), + fake_cursor_(include_cursor ? new FakeMouseMonitor() : nullptr), blender_(fake_screen_, fake_cursor_) { blender_.Start(this); } @@ -206,6 +207,13 @@ class DesktopAndCursorComposerTest : public ::testing::Test, std::unique_ptr frame_; }; +class DesktopAndCursorComposerNoCursorMonitorTest + : public DesktopAndCursorComposerTest { + public: + DesktopAndCursorComposerNoCursorMonitorTest() + : DesktopAndCursorComposerTest(false) {} +}; + TEST_F(DesktopAndCursorComposerTest, CursorShouldBeIgnoredIfNoFrameCaptured) { struct { int x, y; @@ -324,4 +332,82 @@ TEST_F(DesktopAndCursorComposerTest, CursorIncluded) { } } +TEST_F(DesktopAndCursorComposerNoCursorMonitorTest, + UpdatedRegionIncludesOldAndNewCursorRectsIfMoved) { + std::unique_ptr frame( + SharedDesktopFrame::Wrap(CreateTestFrame())); + DesktopRect first_cursor_rect; + { + // Block to scope test_cursor, which is invalidated by OnMouseCursor. + MouseCursor* test_cursor = CreateTestCursor(DesktopVector(0, 0)); + first_cursor_rect = DesktopRect::MakeSize(test_cursor->image()->size()); + blender_.OnMouseCursor(test_cursor); + } + blender_.OnMouseCursorPosition(DesktopVector(0, 0)); + fake_screen_->SetNextFrame(frame->Share()); + blender_.CaptureFrame(); + + DesktopVector cursor_move_offset(1, 1); + DesktopRect second_cursor_rect = first_cursor_rect; + second_cursor_rect.Translate(cursor_move_offset); + blender_.OnMouseCursorPosition(cursor_move_offset); + fake_screen_->SetNextFrame(frame->Share()); + blender_.CaptureFrame(); + + EXPECT_TRUE(frame->updated_region().is_empty()); + DesktopRegion expected_region; + expected_region.AddRect(first_cursor_rect); + expected_region.AddRect(second_cursor_rect); + EXPECT_TRUE(frame_->updated_region().Equals(expected_region)); +} + +TEST_F(DesktopAndCursorComposerNoCursorMonitorTest, + UpdatedRegionIncludesOldAndNewCursorRectsIfShapeChanged) { + std::unique_ptr frame( + SharedDesktopFrame::Wrap(CreateTestFrame())); + DesktopRect first_cursor_rect; + { + // Block to scope test_cursor, which is invalidated by OnMouseCursor. + MouseCursor* test_cursor = CreateTestCursor(DesktopVector(0, 0)); + first_cursor_rect = DesktopRect::MakeSize(test_cursor->image()->size()); + blender_.OnMouseCursor(test_cursor); + } + blender_.OnMouseCursorPosition(DesktopVector(0, 0)); + fake_screen_->SetNextFrame(frame->Share()); + blender_.CaptureFrame(); + + // Create a second cursor, the same shape as the first. Since the code doesn't + // compare the cursor pixels, this is sufficient, and avoids needing two test + // cursor bitmaps. + DesktopRect second_cursor_rect; + { + MouseCursor* test_cursor = CreateTestCursor(DesktopVector(0, 0)); + second_cursor_rect = DesktopRect::MakeSize(test_cursor->image()->size()); + blender_.OnMouseCursor(test_cursor); + } + fake_screen_->SetNextFrame(frame->Share()); + blender_.CaptureFrame(); + + EXPECT_TRUE(frame->updated_region().is_empty()); + DesktopRegion expected_region; + expected_region.AddRect(first_cursor_rect); + expected_region.AddRect(second_cursor_rect); + EXPECT_TRUE(frame_->updated_region().Equals(expected_region)); +} + +TEST_F(DesktopAndCursorComposerNoCursorMonitorTest, + UpdatedRegionUnchangedIfCursorUnchanged) { + std::unique_ptr frame( + SharedDesktopFrame::Wrap(CreateTestFrame())); + blender_.OnMouseCursor(CreateTestCursor(DesktopVector(0, 0))); + blender_.OnMouseCursorPosition(DesktopVector(0, 0)); + fake_screen_->SetNextFrame(frame->Share()); + blender_.CaptureFrame(); + fake_screen_->SetNextFrame(frame->Share()); + blender_.CaptureFrame(); + + EXPECT_TRUE(frame->updated_region().is_empty()); + EXPECT_TRUE(frame_->updated_region().is_empty()); +} + } // namespace webrtc