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 <jamiewalch@chromium.org>
> Reviewed-by: Zijie He <zijiehe@chromium.org>
> Commit-Queue: Brave Yao <braveyao@webrtc.org>
> 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 <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23280}diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc
index cad53f1..6710bdf 100644
--- a/modules/desktop_capture/desktop_and_cursor_composer.cc
+++ b/modules/desktop_capture/desktop_and_cursor_composer.cc
@@ -170,11 +170,8 @@
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<DesktopFrameWithCursor>(
std::move(frame), *cursor_, relative_position);
}
diff --git a/modules/desktop_capture/desktop_frame.cc b/modules/desktop_capture/desktop_frame.cc
index f50bd2a..5a62d4d 100644
--- a/modules/desktop_capture/desktop_frame.cc
+++ b/modules/desktop_capture/desktop_frame.cc
@@ -28,7 +28,6 @@
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 @@
}
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 421d4b4..7bd9596 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 @@
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 ebe77c8..20a2b93 100644
--- a/modules/desktop_capture/mac/screen_capturer_mac.mm
+++ b/modules/desktop_capture/mac/screen_capturer_mac.mm
@@ -86,6 +86,10 @@
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 d30eb4b..e919fec 100644
--- a/modules/desktop_capture/mac/window_list_utils.cc
+++ b/modules/desktop_capture/mac/window_list_utils.cc
@@ -13,7 +13,6 @@
#include <ApplicationServices/ApplicationServices.h>
#include <algorithm>
-#include <cmath>
#include <iterator>
#include "rtc_base/checks.h"
@@ -61,6 +60,37 @@
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,
@@ -235,24 +265,6 @@
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));
@@ -271,6 +283,12 @@
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 @@
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 b54ca3d..8a79f7e 100644
--- a/modules/desktop_capture/mac/window_list_utils.h
+++ b/modules/desktop_capture/mac/window_list_utils.h
@@ -59,15 +59,6 @@
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 @@
// 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 @@
// 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 e969d8e..2e98594 100644
--- a/modules/desktop_capture/mouse_cursor_monitor.h
+++ b/modules/desktop_capture/mouse_cursor_monitor.h
@@ -59,8 +59,6 @@
// 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 de88764..a9780b5 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::Capture() {
assert(callback_);
+ CursorState state = INSIDE;
+
CGEventRef event = CGEventCreate(NULL);
CGPoint gc_position = CGEventGetLocation(event);
CFRelease(event);
@@ -128,7 +130,108 @@
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<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));
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 95e622a..89a5366 100644
--- a/modules/desktop_capture/window_capturer_mac.mm
+++ b/modules/desktop_capture/window_capturer_mac.mm
@@ -210,10 +210,17 @@
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 7f05121..6df0d4d 100644
--- a/modules/desktop_capture/window_finder_mac.mm
+++ b/modules/desktop_capture/window_finder_mac.mm
@@ -28,17 +28,28 @@
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;
}
