Reland "[desktopCapture] Unify the position info in DIP coordinates on Mac."
This is a reland of 89653d5db46419d2a80898635cb27fed64898db2
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 <jamiewalch@chromium.org>
> Reviewed-by: Zijie He <zijiehe@chromium.org>
> Commit-Queue: Brave Yao <braveyao@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#23263}
Bug: webrtc:9178
Change-Id: I97d9150f7b9a4ed6671733b75613ea9c315d5c1d
Reviewed-on: https://webrtc-review.googlesource.com/77481
Reviewed-by: Zijie He <zijiehe@chromium.org>
Commit-Queue: Brave Yao <braveyao@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23289}diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc
index 6710bdf..cad53f1 100644
--- a/modules/desktop_capture/desktop_and_cursor_composer.cc
+++ b/modules/desktop_capture/desktop_and_cursor_composer.cc
@@ -170,8 +170,11 @@
if (frame && cursor_) {
if (frame->rect().Contains(cursor_position_) &&
!desktop_capturer_->IsOccluded(cursor_position_)) {
- const DesktopVector relative_position =
+ const float scale = frame->scale_factor();
+ DesktopVector relative_position =
cursor_position_.subtract(frame->top_left());
+ relative_position.set(relative_position.x() * scale,
+ relative_position.y() * scale);
frame = rtc::MakeUnique<DesktopFrameWithCursor>(
std::move(frame), *cursor_, relative_position);
}
diff --git a/modules/desktop_capture/desktop_frame.cc b/modules/desktop_capture/desktop_frame.cc
index 5a62d4d..67f2009 100644
--- a/modules/desktop_capture/desktop_frame.cc
+++ b/modules/desktop_capture/desktop_frame.cc
@@ -59,7 +59,18 @@
}
DesktopRect DesktopFrame::rect() const {
- return DesktopRect::MakeOriginSize(top_left(), size());
+ 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;
}
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 7bd9596..421d4b4 100644
--- a/modules/desktop_capture/desktop_frame.h
+++ b/modules/desktop_capture/desktop_frame.h
@@ -22,6 +22,8 @@
namespace webrtc {
+const int kStandardDPI = 96;
+
// DesktopFrame represents a video frame captured from the screen.
class DesktopFrame {
public:
@@ -30,15 +32,21 @@
virtual ~DesktopFrame();
- // Returns the rectangle in full desktop coordinates to indicate the area
- // covered by the DesktopFrame.
+ // Returns the rectangle in full desktop coordinates to indicate it covers
+ // the area of top_left() to top_letf() + size() / scale_factor().
DesktopRect rect() const;
- // Size of the frame.
+ // 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.
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).
+ // monitor should start from (0, 0). The desktop coordinates may be scaled by
+ // OS, but this is always consistent with the MouseCursorMonitor.
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 20a2b93..ebe77c8 100644
--- a/modules/desktop_capture/mac/screen_capturer_mac.mm
+++ b/modules/desktop_capture/mac/screen_capturer_mac.mm
@@ -86,10 +86,6 @@
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<int>(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 e919fec..d30eb4b 100644
--- a/modules/desktop_capture/mac/window_list_utils.cc
+++ b/modules/desktop_capture/mac/window_list_utils.cc
@@ -13,6 +13,7 @@
#include <ApplicationServices/ApplicationServices.h>
#include <algorithm>
+#include <cmath>
#include <iterator>
#include "rtc_base/checks.h"
@@ -60,37 +61,6 @@
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<bool(CFDictionaryRef)> on_window,
@@ -265,6 +235,24 @@
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<float>::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<CFDictionaryRef>(
CFDictionaryGetValue(window, kCGWindowBounds));
@@ -283,12 +271,6 @@
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,
@@ -300,10 +282,4 @@
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 8a79f7e..b54ca3d 100644
--- a/modules/desktop_capture/mac/window_list_utils.h
+++ b/modules/desktop_capture/mac/window_list_utils.h
@@ -59,6 +59,15 @@
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
@@ -67,11 +76,6 @@
// 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.
@@ -79,11 +83,6 @@
// 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 2e98594..e969d8e 100644
--- a/modules/desktop_capture/mouse_cursor_monitor.h
+++ b/modules/desktop_capture/mouse_cursor_monitor.h
@@ -59,6 +59,8 @@
// 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 a9780b5..de88764 100644
--- a/modules/desktop_capture/mouse_cursor_monitor_mac.mm
+++ b/modules/desktop_capture/mouse_cursor_monitor_mac.mm
@@ -111,8 +111,6 @@
void MouseCursorMonitorMac::Capture() {
assert(callback_);
- CursorState state = INSIDE;
-
CGEventRef event = CGEventCreate(NULL);
CGPoint gc_position = CGEventGetLocation(event);
CFRelease(event);
@@ -130,108 +128,7 @@
if (mode_ != SHAPE_AND_POSITION)
return;
- // 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<CFDictionaryRef>(
- CFArrayGetValueAtIndex(window_array, i));
-
- // Skip the Dock window. Dock window covers the whole screen, but it is
- // transparent.
- CFStringRef window_name = reinterpret_cast<CFStringRef>(
- CFDictionaryGetValue(window, kCGWindowName));
- if (window_name && CFStringCompare(window_name, CFSTR("Dock"), 0) == 0)
- continue;
-
- CFDictionaryRef window_bounds = reinterpret_cast<CFDictionaryRef>(
- CFDictionaryGetValue(window, kCGWindowBounds));
- CFNumberRef window_number = reinterpret_cast<CFNumberRef>(
- 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<CGDirectDisplayID>(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));
+ // Always report cursor position in DIP pixel.
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 89a5366..95e622a 100644
--- a/modules/desktop_capture/window_capturer_mac.mm
+++ b/modules/desktop_capture/window_capturer_mac.mm
@@ -210,17 +210,10 @@
frame->mutable_updated_region()->SetRect(
DesktopRect::MakeSize(frame->size()));
- 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);
+ 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));
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 6df0d4d..7f05121 100644
--- a/modules/desktop_capture/window_finder_mac.mm
+++ b/modules/desktop_capture/window_finder_mac.mm
@@ -28,28 +28,17 @@
WindowId WindowFinderMac::GetWindowUnderPoint(DesktopVector point) {
WindowId id = kNullWindowId;
- 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);
+ GetWindowList(
+ [&id, point](CFDictionaryRef window) {
+ DesktopRect bounds;
+ bounds = GetWindowBounds(window);
+ if (bounds.Contains(point)) {
+ id = GetWindowId(window);
+ return false;
+ }
+ return true;
+ },
+ true);
return id;
}
