From e1c3c01a9087e85b3581832da09ebea6c718e5d8 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Thu, 17 May 2018 12:11:18 +0000 Subject: [PATCH] Revert "[desktopCapture] Unify the position info in DIP coordinates on Mac." This reverts commit 89653d5db46419d2a80898635cb27fed64898db2. Reason for revert: Tentatively revert since I believe this break remoting unittests on Asan/Tsan https://chromium-review.googlesource.com/c/chromium/src/+/1063330 https://chromium-swarm.appspot.com/task?id=3d8692bedcc85c10&refresh=10&show_raw=1 Original change's description: > [desktopCapture] Unify the position info in DIP coordinates on Mac. > > On OSX, the logical(DIP) and physical coordinates are used mixingly. > For example, the captured image has its size in physical pixels(2x) and > location in logical(DIP) pixels. Same to the cursor position. This > causes trouble when we check the relative position of image and cursor > when there are multiple monitors with different DIP setting connected. > > This cl proposed a solution to use DIP pixel for any location info, > i.e. top-left of a frame and cursor position. Also propose a method to > get the current scale factor of a window across multiple monitors. And > save the current scale factor in DPI of the capture frame. > Then we can check relative position of cursor and frame correctly > in DIP pixel and compose them in physical pixel. > > Bug: webrtc:9178 > Change-Id: I3c076aeac2d6f2c1f63d000d7fff03500aa375ac > Reviewed-on: https://webrtc-review.googlesource.com/71621 > Reviewed-by: Jamie Walch > Reviewed-by: Zijie He > Commit-Queue: Brave Yao > Cr-Commit-Position: refs/heads/master@{#23263} TBR=zijiehe@chromium.org,jamiewalch@chromium.org,perkj@webrtc.org,braveyao@webrtc.org Change-Id: Ica02365925623e21b256d20a21b5625e7ed6f49b No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:9178 Reviewed-on: https://webrtc-review.googlesource.com/77461 Reviewed-by: Per Kjellander Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/master@{#23280} --- .../desktop_and_cursor_composer.cc | 5 +- modules/desktop_capture/desktop_frame.cc | 14 +-- modules/desktop_capture/desktop_frame.h | 16 +-- .../mac/screen_capturer_mac.mm | 4 + .../desktop_capture/mac/window_list_utils.cc | 62 +++++++---- .../desktop_capture/mac/window_list_utils.h | 19 ++-- .../desktop_capture/mouse_cursor_monitor.h | 2 - .../mouse_cursor_monitor_mac.mm | 105 +++++++++++++++++- .../desktop_capture/window_capturer_mac.mm | 15 ++- modules/desktop_capture/window_finder_mac.mm | 33 ++++-- 10 files changed, 200 insertions(+), 75 deletions(-) diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc index cad53f1adb..6710bdfd87 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer.cc +++ b/modules/desktop_capture/desktop_and_cursor_composer.cc @@ -170,11 +170,8 @@ void DesktopAndCursorComposer::OnCaptureResult( if (frame && cursor_) { if (frame->rect().Contains(cursor_position_) && !desktop_capturer_->IsOccluded(cursor_position_)) { - const float scale = frame->scale_factor(); - DesktopVector relative_position = + const DesktopVector relative_position = cursor_position_.subtract(frame->top_left()); - relative_position.set(relative_position.x() * scale, - relative_position.y() * scale); frame = rtc::MakeUnique( std::move(frame), *cursor_, relative_position); } diff --git a/modules/desktop_capture/desktop_frame.cc b/modules/desktop_capture/desktop_frame.cc index f50bd2ad01..5a62d4d9cc 100644 --- a/modules/desktop_capture/desktop_frame.cc +++ b/modules/desktop_capture/desktop_frame.cc @@ -28,7 +28,6 @@ DesktopFrame::DesktopFrame(DesktopSize size, shared_memory_(shared_memory), size_(size), stride_(stride), - dpi_(DesktopVector(kStandardDPI, kStandardDPI)), capture_time_ms_(0), capturer_id_(DesktopCapturerId::kUnknown) { RTC_DCHECK(size_.width() >= 0); @@ -60,18 +59,7 @@ void DesktopFrame::CopyPixelsFrom(const DesktopFrame& src_frame, } DesktopRect DesktopFrame::rect() const { - const float scale = scale_factor(); - // Only scale the size. - return DesktopRect::MakeXYWH(top_left().x(), top_left().y(), - size().width() / scale, size().height() / scale); -} - -float DesktopFrame::scale_factor() const { - float scale = 1.0f; - if (!dpi().is_zero() && dpi().x() == dpi().y()) - scale = dpi().x() / kStandardDPI; - - return scale; + return DesktopRect::MakeOriginSize(top_left(), size()); } uint8_t* DesktopFrame::GetFrameDataAtPos(const DesktopVector& pos) const { diff --git a/modules/desktop_capture/desktop_frame.h b/modules/desktop_capture/desktop_frame.h index 421d4b4480..7bd9596580 100644 --- a/modules/desktop_capture/desktop_frame.h +++ b/modules/desktop_capture/desktop_frame.h @@ -22,8 +22,6 @@ namespace webrtc { -const int kStandardDPI = 96; - // DesktopFrame represents a video frame captured from the screen. class DesktopFrame { public: @@ -32,21 +30,15 @@ class DesktopFrame { virtual ~DesktopFrame(); - // Returns the rectangle in full desktop coordinates to indicate it covers - // the area of top_left() to top_letf() + size() / scale_factor(). + // Returns the rectangle in full desktop coordinates to indicate the area + // covered by the DesktopFrame. DesktopRect rect() const; - // Returns the scale factor from DIPs to physical pixels of the frame. - // Assumes same scale in both X and Y directions at present. - float scale_factor() const; - - // Size of the frame. In physical coordinates, mapping directly from the - // underlying buffer. + // Size of the frame. const DesktopSize& size() const { return size_; } // The top-left of the frame in full desktop coordinates. E.g. the top left - // monitor should start from (0, 0). The desktop coordinates may be scaled by - // OS, but this is always consistent with the MouseCursorMonitor. + // monitor should start from (0, 0). const DesktopVector& top_left() const { return top_left_; } void set_top_left(const DesktopVector& top_left) { top_left_ = top_left; } diff --git a/modules/desktop_capture/mac/screen_capturer_mac.mm b/modules/desktop_capture/mac/screen_capturer_mac.mm index ebe77c8d3f..20a2b93340 100644 --- a/modules/desktop_capture/mac/screen_capturer_mac.mm +++ b/modules/desktop_capture/mac/screen_capturer_mac.mm @@ -86,6 +86,10 @@ class DisplayStreamManager { namespace { +// Standard Mac displays have 72dpi, but we report 96dpi for +// consistency with Windows and Linux. +const int kStandardDPI = 96; + // Scales all coordinates of a rect by a specified factor. DesktopRect ScaleAndRoundCGRect(const CGRect& rect, float scale) { return DesktopRect::MakeLTRB(static_cast(floor(rect.origin.x * scale)), diff --git a/modules/desktop_capture/mac/window_list_utils.cc b/modules/desktop_capture/mac/window_list_utils.cc index d30eb4be03..e919fec5fc 100644 --- a/modules/desktop_capture/mac/window_list_utils.cc +++ b/modules/desktop_capture/mac/window_list_utils.cc @@ -13,7 +13,6 @@ #include #include -#include #include #include "rtc_base/checks.h" @@ -61,6 +60,37 @@ bool GetWindowRef(CGWindowID id, return result; } +// Scales the |rect| according to the DIP to physical pixel scale of |rect|. +// |rect| is in unscaled system coordinate, i.e. it's device-independent and the +// primary monitor starts from (0, 0). If |rect| overlaps multiple monitors, the +// returned size may not be accurate when monitors have different DIP settings. +// If |rect| is entirely out of the display, this function returns |rect|. +DesktopRect ApplyScaleFactorOfRect( + const MacDesktopConfiguration& desktop_config, + DesktopRect rect) { + // TODO(http://crbug.com/778049): How does Mac OSX decide the scale factor + // if one window is across two monitors with different DPIs. + float scales[] = { + GetScaleFactorAtPosition(desktop_config, rect.top_left()), + GetScaleFactorAtPosition(desktop_config, + DesktopVector(rect.left() + rect.width() / 2, + rect.top() + rect.height() / 2)), + GetScaleFactorAtPosition( + desktop_config, DesktopVector(rect.right(), rect.bottom())), + }; + // Since GetScaleFactorAtPosition() returns 1 if the position is out of the + // display, we always prefer a value which not equals to 1. + float scale = *std::max_element(std::begin(scales), std::end(scales)); + if (scale == 1) { + scale = *std::min_element(std::begin(scales), std::end(scales)); + } + + return DesktopRect::MakeXYWH(rect.left() * scale, + rect.top() * scale, + rect.width() * scale, + rect.height() * scale); +} + } // namespace bool GetWindowList(rtc::FunctionView on_window, @@ -235,24 +265,6 @@ float GetScaleFactorAtPosition(const MacDesktopConfiguration& desktop_config, return 1; } -float GetWindowScaleFactor(CGWindowID id, DesktopSize size) { - DesktopRect window_bounds = GetWindowBounds(id); - float scale = 1.0f; - - if (!window_bounds.is_empty() && !size.is_empty()) { - float scale_x = size.width() / window_bounds.width(); - float scale_y = size.height() / window_bounds.height(); - // Currently the scale in X and Y directions must be same. - if ((std::fabs(scale_x - scale_y) <= - std::numeric_limits::epsilon() * std::max(scale_x, scale_y)) && - scale_x > 0.0f) { - scale = scale_x; - } - } - - return scale; -} - DesktopRect GetWindowBounds(CFDictionaryRef window) { CFDictionaryRef window_bounds = reinterpret_cast( CFDictionaryGetValue(window, kCGWindowBounds)); @@ -271,6 +283,12 @@ DesktopRect GetWindowBounds(CFDictionaryRef window) { gc_window_rect.size.height); } +DesktopRect GetWindowBounds(const MacDesktopConfiguration& desktop_config, + CFDictionaryRef window) { + DesktopRect rect = GetWindowBounds(window); + return ApplyScaleFactorOfRect(desktop_config, rect); +} + DesktopRect GetWindowBounds(CGWindowID id) { DesktopRect result; if (GetWindowRef(id, @@ -282,4 +300,10 @@ DesktopRect GetWindowBounds(CGWindowID id) { return DesktopRect(); } +DesktopRect GetWindowBounds(const MacDesktopConfiguration& desktop_config, + CGWindowID id) { + DesktopRect rect = GetWindowBounds(id); + return ApplyScaleFactorOfRect(desktop_config, rect); +} + } // namespace webrtc diff --git a/modules/desktop_capture/mac/window_list_utils.h b/modules/desktop_capture/mac/window_list_utils.h index b54ca3d346..8a79f7ecd5 100644 --- a/modules/desktop_capture/mac/window_list_utils.h +++ b/modules/desktop_capture/mac/window_list_utils.h @@ -59,15 +59,6 @@ WindowId GetWindowId(CFDictionaryRef window); float GetScaleFactorAtPosition(const MacDesktopConfiguration& desktop_config, DesktopVector position); -// Returns the DIP to physical pixel scale factor of the window with |id|. -// The bounds of the window with |id| is in DIP coordinates and |size| is the -// CGImage size of the window with |id| in physical coordinates. Comparing them -// can give the current scale factor. -// If the window overlaps multiple monitors, OS will decide on which monitor the -// window is displayed and use its scale factor to the window. So this method -// still works. -float GetWindowScaleFactor(CGWindowID id, DesktopSize size); - // Returns the bounds of |window|. If |window| is not a window or the bounds // cannot be retrieved, this function returns an empty DesktopRect. The returned // DesktopRect is in system coordinate, i.e. the primary monitor always starts @@ -76,6 +67,11 @@ float GetWindowScaleFactor(CGWindowID id, DesktopSize size); // MacDesktopConfiguration. DesktopRect GetWindowBounds(CFDictionaryRef window); +// Same as GetWindowBounds(CFDictionaryRef), but this function stretches the +// result with the scale factor. +DesktopRect GetWindowBounds(const MacDesktopConfiguration& desktop_config, + CFDictionaryRef window); + // Returns the bounds of window with |id|. If |id| does not represent a window // or the bounds cannot be retrieved, this function returns an empty // DesktopRect. The returned DesktopRect is in system coordinates. @@ -83,6 +79,11 @@ DesktopRect GetWindowBounds(CFDictionaryRef window); // MacDesktopConfiguration. DesktopRect GetWindowBounds(CGWindowID id); +// Same as GetWindowBounds(CGWindowID), but this function stretches the result +// with the scale factor. +DesktopRect GetWindowBounds(const MacDesktopConfiguration& desktop_config, + CGWindowID id); + } // namespace webrtc #endif // MODULES_DESKTOP_CAPTURE_MAC_WINDOW_LIST_UTILS_H_ diff --git a/modules/desktop_capture/mouse_cursor_monitor.h b/modules/desktop_capture/mouse_cursor_monitor.h index e969d8eb31..2e98594d63 100644 --- a/modules/desktop_capture/mouse_cursor_monitor.h +++ b/modules/desktop_capture/mouse_cursor_monitor.h @@ -59,8 +59,6 @@ class MouseCursorMonitor { // Called in response to Capture(). |position| indicates cursor absolute // position on the system in fullscreen coordinate, i.e. the top-left // monitor always starts from (0, 0). - // The coordinates of the position is controlled by OS, but it's always - // consistent with DesktopFrame.rect().top_left(). // TODO(zijiehe): Ensure all implementations return the absolute position. // TODO(zijiehe): Make this function pure virtual after Chromium changes. // TODO(zijiehe): Current this overload works correctly only when capturing diff --git a/modules/desktop_capture/mouse_cursor_monitor_mac.mm b/modules/desktop_capture/mouse_cursor_monitor_mac.mm index de8876438f..a9780b52b5 100644 --- a/modules/desktop_capture/mouse_cursor_monitor_mac.mm +++ b/modules/desktop_capture/mouse_cursor_monitor_mac.mm @@ -111,6 +111,8 @@ void MouseCursorMonitorMac::Init(Callback* callback, Mode mode) { void MouseCursorMonitorMac::Capture() { assert(callback_); + CursorState state = INSIDE; + CGEventRef event = CGEventCreate(NULL); CGPoint gc_position = CGEventGetLocation(event); CFRelease(event); @@ -128,7 +130,108 @@ void MouseCursorMonitorMac::Capture() { if (mode_ != SHAPE_AND_POSITION) return; - // Always report cursor position in DIP pixel. + // If we are capturing cursor for a specific window then we need to figure out + // if the current mouse position is covered by another window and also adjust + // |position| to make it relative to the window origin. + if (window_id_ != kCGNullWindowID) { + CGWindowID on_screen_window = window_id_; + if (full_screen_chrome_window_detector_) { + CGWindowID full_screen_window = + full_screen_chrome_window_detector_->FindFullScreenWindow(window_id_); + + if (full_screen_window != kCGNullWindowID) + on_screen_window = full_screen_window; + } + + // Get list of windows that may be covering parts of |on_screen_window|. + // CGWindowListCopyWindowInfo() returns windows in order from front to back, + // so |on_screen_window| is expected to be the last in the list. + CFArrayRef window_array = + CGWindowListCopyWindowInfo(kCGWindowListOptionOnScreenOnly | + kCGWindowListOptionOnScreenAboveWindow | + kCGWindowListOptionIncludingWindow, + on_screen_window); + bool found_window = false; + if (window_array) { + CFIndex count = CFArrayGetCount(window_array); + for (CFIndex i = 0; i < count; ++i) { + CFDictionaryRef window = reinterpret_cast( + CFArrayGetValueAtIndex(window_array, i)); + + // Skip the Dock window. Dock window covers the whole screen, but it is + // transparent. + CFStringRef window_name = reinterpret_cast( + CFDictionaryGetValue(window, kCGWindowName)); + if (window_name && CFStringCompare(window_name, CFSTR("Dock"), 0) == 0) + continue; + + CFDictionaryRef window_bounds = reinterpret_cast( + CFDictionaryGetValue(window, kCGWindowBounds)); + CFNumberRef window_number = reinterpret_cast( + CFDictionaryGetValue(window, kCGWindowNumber)); + + if (window_bounds && window_number) { + CGRect gc_window_rect; + if (!CGRectMakeWithDictionaryRepresentation(window_bounds, + &gc_window_rect)) { + continue; + } + DesktopRect window_rect = + DesktopRect::MakeXYWH(gc_window_rect.origin.x, + gc_window_rect.origin.y, + gc_window_rect.size.width, + gc_window_rect.size.height); + + CGWindowID window_id; + if (!CFNumberGetValue(window_number, kCFNumberIntType, &window_id)) + continue; + + if (window_id == on_screen_window) { + found_window = true; + if (!window_rect.Contains(position)) + state = OUTSIDE; + position = position.subtract(window_rect.top_left()); + + assert(i == count - 1); + break; + } else if (window_rect.Contains(position)) { + state = OUTSIDE; + position.set(-1, -1); + break; + } + } + } + CFRelease(window_array); + } + if (!found_window) { + // If we failed to get list of windows or the window wasn't in the list + // pretend that the cursor is outside the window. This can happen, e.g. if + // the window was closed. + state = OUTSIDE; + position.set(-1, -1); + } + } else { + assert(screen_id_ >= kFullDesktopScreenId); + if (screen_id_ != kFullDesktopScreenId) { + // For single screen capturing, convert the position to relative to the + // target screen. + const MacDisplayConfiguration* config = + configuration.FindDisplayConfigurationById( + static_cast(screen_id_)); + if (config) { + if (!config->pixel_bounds.Contains(position)) + state = OUTSIDE; + position = position.subtract(config->bounds.top_left()); + } else { + // The target screen is no longer valid. + state = OUTSIDE; + position.set(-1, -1); + } + } + } + // Convert Density Independent Pixel to physical pixel. + position = DesktopVector(round(position.x() * scale), + round(position.y() * scale)); callback_->OnMouseCursorPosition( position.subtract(configuration.bounds.top_left())); } diff --git a/modules/desktop_capture/window_capturer_mac.mm b/modules/desktop_capture/window_capturer_mac.mm index 95e622a77a..89a53660ca 100644 --- a/modules/desktop_capture/window_capturer_mac.mm +++ b/modules/desktop_capture/window_capturer_mac.mm @@ -210,10 +210,17 @@ void WindowCapturerMac::CaptureFrame() { frame->mutable_updated_region()->SetRect( DesktopRect::MakeSize(frame->size())); - frame->set_top_left(GetWindowBounds(on_screen_window).top_left()); - - float scale_factor = GetWindowScaleFactor(window_id_, frame->size()); - frame->set_dpi(DesktopVector(kStandardDPI * scale_factor, kStandardDPI * scale_factor)); + DesktopVector top_left; + if (configuration_monitor_) { + configuration_monitor_->Lock(); + auto configuration = configuration_monitor_->desktop_configuration(); + configuration_monitor_->Unlock(); + top_left = GetWindowBounds(configuration, on_screen_window).top_left(); + top_left = top_left.subtract(configuration.bounds.top_left()); + } else { + top_left = GetWindowBounds(on_screen_window).top_left(); + } + frame->set_top_left(top_left); callback_->OnCaptureResult(Result::SUCCESS, std::move(frame)); diff --git a/modules/desktop_capture/window_finder_mac.mm b/modules/desktop_capture/window_finder_mac.mm index 7f051213b1..6df0d4d9b7 100644 --- a/modules/desktop_capture/window_finder_mac.mm +++ b/modules/desktop_capture/window_finder_mac.mm @@ -28,17 +28,28 @@ WindowFinderMac::~WindowFinderMac() = default; WindowId WindowFinderMac::GetWindowUnderPoint(DesktopVector point) { WindowId id = kNullWindowId; - GetWindowList( - [&id, point](CFDictionaryRef window) { - DesktopRect bounds; - bounds = GetWindowBounds(window); - if (bounds.Contains(point)) { - id = GetWindowId(window); - return false; - } - return true; - }, - true); + MacDesktopConfiguration configuration_holder; + MacDesktopConfiguration* configuration = nullptr; + if (configuration_monitor_) { + configuration_monitor_->Lock(); + configuration_holder = configuration_monitor_->desktop_configuration(); + configuration_monitor_->Unlock(); + configuration = &configuration_holder; + } + GetWindowList([&id, point, configuration](CFDictionaryRef window) { + DesktopRect bounds; + if (configuration) { + bounds = GetWindowBounds(*configuration, window); + } else { + bounds = GetWindowBounds(window); + } + if (bounds.Contains(point)) { + id = GetWindowId(window); + return false; + } + return true; + }, + true); return id; }