From 5d85819dd21842cc3e6d74676a292cf9d9142f4e Mon Sep 17 00:00:00 2001 From: "sergeyu@chromium.org" Date: Tue, 19 Nov 2013 02:15:47 +0000 Subject: [PATCH] Fix DesktopAndCursorComposer to restore frames to the original state. Screen capturers may reuse frame buffers and they expect that the frame content isn't changed by the frame consumer. DesktopAndCursorComposer draws mouse cursor on generated frames and it was releasing the frames with the mouse cursor on them. Fixed it to restore frame content erasing mouse cursor before returning desktop frames. BUG=crbug.com/316297 R=wez@chromium.org Review URL: https://webrtc-codereview.appspot.com/3899004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5133 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../desktop_and_cursor_composer.cc | 86 +++++++++++++++---- .../desktop_and_cursor_composer_unittest.cc | 25 +++++- .../modules/desktop_capture/desktop_frame.cc | 25 ++++++ .../modules/desktop_capture/desktop_frame.h | 8 ++ .../desktop_capture/desktop_geometry.cc | 5 ++ .../desktop_capture/desktop_geometry.h | 7 ++ .../desktop_capture/screen_capturer_x11.cc | 9 +- 7 files changed, 138 insertions(+), 27 deletions(-) diff --git a/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc b/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc index 3141fee8da..6ed3ae881f 100644 --- a/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc +++ b/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc @@ -51,6 +51,73 @@ void AlphaBlend(uint8_t* dest, int dest_stride, } } +// DesktopFrame wrapper that draws mouse on a frame and restores original +// content before releasing the underlying frame. +class DesktopFrameWithCursor : public DesktopFrame { + public: + // Takes ownership of |frame|. + DesktopFrameWithCursor(DesktopFrame* frame, + const MouseCursor& cursor, + const DesktopVector& position); + virtual ~DesktopFrameWithCursor(); + + private: + scoped_ptr original_frame_; + + DesktopVector restore_position_; + scoped_ptr restore_frame_; + + DISALLOW_COPY_AND_ASSIGN(DesktopFrameWithCursor); +}; + +DesktopFrameWithCursor::DesktopFrameWithCursor(DesktopFrame* frame, + const MouseCursor& cursor, + const DesktopVector& position) + : DesktopFrame(frame->size(), frame->stride(), + frame->data(), frame->shared_memory()), + original_frame_(frame) { + set_dpi(frame->dpi()); + set_capture_time_ms(frame->capture_time_ms()); + mutable_updated_region()->Swap(frame->mutable_updated_region()); + + 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())); + + if (target_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(), + 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(), + cursor.image().data() + + origin_shift.y() * cursor.image().stride() + + origin_shift.x() * DesktopFrame::kBytesPerPixel, + cursor.image().stride(), + target_rect.size()); +} + +DesktopFrameWithCursor::~DesktopFrameWithCursor() { + // Restore original content of the frame. + if (restore_frame_.get()) { + DesktopRect target_rect = DesktopRect::MakeSize(restore_frame_->size()); + target_rect.Translate(restore_position_); + CopyPixelsFrom(restore_frame_->data(), restore_frame_->stride(), + target_rect); + } +} + } // namespace DesktopAndCursorComposer::DesktopAndCursorComposer( @@ -81,22 +148,9 @@ SharedMemory* DesktopAndCursorComposer::CreateSharedMemory(size_t size) { void DesktopAndCursorComposer::OnCaptureCompleted(DesktopFrame* frame) { if (cursor_.get() && cursor_state_ == MouseCursorMonitor::INSIDE) { - DesktopVector image_pos = cursor_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(frame->size())); - DesktopVector origin_shift = target_rect.top_left().subtract(target_origin); - int cursor_width = cursor_->image().size().width(); - AlphaBlend(reinterpret_cast(frame->data()) + - target_rect.top() * frame->stride() + - target_rect.left() * DesktopFrame::kBytesPerPixel, - frame->stride(), - cursor_->image().data() + - (origin_shift.y() * cursor_width + origin_shift.x()) * - DesktopFrame::kBytesPerPixel, - cursor_width * DesktopFrame::kBytesPerPixel, - target_rect.size()); + DesktopFrameWithCursor* frame_with_cursor = + new DesktopFrameWithCursor(frame, *cursor_, cursor_position_); + frame = frame_with_cursor; } callback_->OnCaptureCompleted(frame); diff --git a/webrtc/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc b/webrtc/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc index 0bb723750a..15d6f54611 100644 --- a/webrtc/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc +++ b/webrtc/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc @@ -14,6 +14,7 @@ #include "webrtc/modules/desktop_capture/desktop_capture_options.h" #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/modules/desktop_capture/window_capturer.h" #include "webrtc/system_wrappers/interface/logging.h" #include "webrtc/system_wrappers/interface/scoped_ptr.h" @@ -74,11 +75,19 @@ class FakeScreenCapturer : public DesktopCapturer { *(data++) = GetFakeFramePixelValue(DesktopVector(x, y)); } } - callback_->OnCaptureCompleted(frame); + + last_frame_.reset(SharedDesktopFrame::Wrap(frame)); + + callback_->OnCaptureCompleted(last_frame_->Share()); } + // Returns last fake captured frame. + SharedDesktopFrame* last_frame() { return last_frame_.get(); } + private: Callback* callback_; + + scoped_ptr last_frame_; }; class FakeMouseMonitor : public MouseCursorMonitor { @@ -155,8 +164,9 @@ class DesktopAndCursorComposerTest : public testing::Test, public DesktopCapturer::Callback { public: DesktopAndCursorComposerTest() - : fake_cursor_(new FakeMouseMonitor()), - blender_(new FakeScreenCapturer(), fake_cursor_) { + : fake_screen_(new FakeScreenCapturer()), + fake_cursor_(new FakeMouseMonitor()), + blender_(fake_screen_, fake_cursor_) { } // DesktopCapturer::Callback interface @@ -170,7 +180,9 @@ class DesktopAndCursorComposerTest : public testing::Test, protected: // Owned by |blender_|. + FakeScreenCapturer* fake_screen_; FakeMouseMonitor* fake_cursor_; + DesktopAndCursorComposer blender_; scoped_ptr frame_; }; @@ -213,6 +225,13 @@ TEST_F(DesktopAndCursorComposerTest, Blend) { blender_.Capture(DesktopRegion()); VerifyFrame(*frame_, state, pos); + + // Verify that the cursor is erased before the frame buffer is returned to + // the screen capturer. + frame_.reset(); + VerifyFrame(*fake_screen_->last_frame(), + MouseCursorMonitor::OUTSIDE, + DesktopVector()); } } diff --git a/webrtc/modules/desktop_capture/desktop_frame.cc b/webrtc/modules/desktop_capture/desktop_frame.cc index f293baff08..f26dc9371b 100644 --- a/webrtc/modules/desktop_capture/desktop_frame.cc +++ b/webrtc/modules/desktop_capture/desktop_frame.cc @@ -10,6 +10,7 @@ #include "webrtc/modules/desktop_capture/desktop_frame.h" +#include #include namespace webrtc { @@ -27,6 +28,30 @@ DesktopFrame::DesktopFrame(DesktopSize size, DesktopFrame::~DesktopFrame() {} +void DesktopFrame::CopyPixelsFrom(uint8_t* src_buffer, int src_stride, + const DesktopRect& dest_rect) { + assert(DesktopRect::MakeSize(size()).ContainsRect(dest_rect)); + + uint8_t* dest = data() + stride() * dest_rect.top() + + DesktopFrame::kBytesPerPixel * dest_rect.left(); + for (int y = 0; y < dest_rect.height(); ++y) { + memcpy(dest, src_buffer, DesktopFrame::kBytesPerPixel * dest_rect.width()); + src_buffer += src_stride; + dest += stride(); + } +} + +void DesktopFrame::CopyPixelsFrom(const DesktopFrame& src_frame, + const DesktopVector& src_pos, + const DesktopRect& dest_rect) { + assert(DesktopRect::MakeSize(src_frame.size()).ContainsRect( + DesktopRect::MakeOriginSize(src_pos, dest_rect.size()))); + + CopyPixelsFrom(src_frame.data() + src_frame.stride() * src_pos.y() + + DesktopFrame::kBytesPerPixel * src_pos.x(), + src_frame.stride(), dest_rect); +} + BasicDesktopFrame::BasicDesktopFrame(DesktopSize size) : DesktopFrame(size, kBytesPerPixel * size.width(), new uint8_t[kBytesPerPixel * size.width() * size.height()], diff --git a/webrtc/modules/desktop_capture/desktop_frame.h b/webrtc/modules/desktop_capture/desktop_frame.h index b420a3c564..a38359a3c8 100644 --- a/webrtc/modules/desktop_capture/desktop_frame.h +++ b/webrtc/modules/desktop_capture/desktop_frame.h @@ -53,6 +53,14 @@ class DesktopFrame { int32_t capture_time_ms() const { return capture_time_ms_; } void set_capture_time_ms(int32_t time_ms) { capture_time_ms_ = time_ms; } + // Copies pixels from a buffer or another frame. |dest_rect| rect must lay + // within bounds of this frame. + void CopyPixelsFrom(uint8_t* src_buffer, int src_stride, + const DesktopRect& dest_rect); + void CopyPixelsFrom(const DesktopFrame& src_frame, + const DesktopVector& src_pos, + const DesktopRect& dest_rect); + protected: DesktopFrame(DesktopSize size, int stride, diff --git a/webrtc/modules/desktop_capture/desktop_geometry.cc b/webrtc/modules/desktop_capture/desktop_geometry.cc index cffaefe1f1..1ff7c683c7 100644 --- a/webrtc/modules/desktop_capture/desktop_geometry.cc +++ b/webrtc/modules/desktop_capture/desktop_geometry.cc @@ -19,6 +19,11 @@ bool DesktopRect::Contains(const DesktopVector& point) const { point.y() >= top() && point.y() < bottom(); } +bool DesktopRect::ContainsRect(const DesktopRect& rect) const { + return rect.left() >= left() && rect.right() <= right() && + rect.top() >= top() && rect.bottom() <= bottom(); +} + void DesktopRect::IntersectWith(const DesktopRect& rect) { left_ = std::max(left(), rect.left()); top_ = std::max(top(), rect.top()); diff --git a/webrtc/modules/desktop_capture/desktop_geometry.h b/webrtc/modules/desktop_capture/desktop_geometry.h index 580e0bfa71..e51273d8d2 100644 --- a/webrtc/modules/desktop_capture/desktop_geometry.h +++ b/webrtc/modules/desktop_capture/desktop_geometry.h @@ -91,6 +91,10 @@ class DesktopRect { int32_t right, int32_t bottom) { return DesktopRect(left, top, right, bottom); } + static DesktopRect MakeOriginSize(const DesktopVector& origin, + const DesktopSize& size) { + return MakeXYWH(origin.x(), origin.y(), size.width(), size.height()); + } DesktopRect() : left_(0), top_(0), right_(0), bottom_(0) {} @@ -114,6 +118,9 @@ class DesktopRect { // Returns true if |point| lies within the rectangle boundaries. bool Contains(const DesktopVector& point) const; + // Returns true if |rect| lies within the boundaries of this rectangle. + bool ContainsRect(const DesktopRect& rect) const; + // Finds intersection with |rect|. void IntersectWith(const DesktopRect& rect); diff --git a/webrtc/modules/desktop_capture/screen_capturer_x11.cc b/webrtc/modules/desktop_capture/screen_capturer_x11.cc index 97bdb658de..c5a4c8cb17 100644 --- a/webrtc/modules/desktop_capture/screen_capturer_x11.cc +++ b/webrtc/modules/desktop_capture/screen_capturer_x11.cc @@ -462,14 +462,7 @@ void ScreenCapturerLinux::SynchronizeFrame() { DCHECK(current != last); for (DesktopRegion::Iterator it(last_invalid_region_); !it.IsAtEnd(); it.Advance()) { - const DesktopRect& r = it.rect(); - int offset = r.top() * current->stride() + - r.left() * DesktopFrame::kBytesPerPixel; - for (int i = 0; i < r.height(); ++i) { - memcpy(current->data() + offset, last->data() + offset, - r.width() * DesktopFrame::kBytesPerPixel); - offset += current->size().width() * DesktopFrame::kBytesPerPixel; - } + current->CopyPixelsFrom(*last, it.rect().top_left(), it.rect()); } }