Repository navigation
fix(macOS): move the pointer once per relative packet #5778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,28 +7,153 @@ | |
| #include <ApplicationServices/ApplicationServices.h> | ||
|
|
||
| // standard includes | ||
| #include <algorithm> | ||
| #include <chrono> | ||
| #include <cstdint> | ||
| #include <memory> | ||
| #include <optional> | ||
| #include <utility> | ||
| #include <vector> | ||
|
|
||
| // local includes | ||
| #include "src/config.h" | ||
| #include "src/logging.h" | ||
| #include "src/platform/macos/mouse_utils.h" | ||
| #include "src/platform/virtualhid_input.h" | ||
|
|
||
| namespace platf { | ||
| namespace { | ||
| constexpr uint32_t max_displays = 16; | ||
|
|
||
| /** | ||
| * @brief Read the pointer position in global CoreGraphics point space. | ||
| * @return The current pointer location, or an empty optional when events are unavailable. | ||
| */ | ||
| std::optional<CGPoint> cursor_point() { | ||
| const auto event = CGEventCreate(nullptr); | ||
| if (!event) { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| const auto point = CGEventGetLocation(event); | ||
| CFRelease(event); | ||
| return point; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Collect the bounds of every active display. | ||
| * @return The union of the display layout, or an empty optional if it is unavailable. | ||
| */ | ||
| std::optional<macos::mouse::bounds_t> active_layout_bounds() { | ||
| uint32_t count = 0; | ||
| if (CGGetActiveDisplayList(0, nullptr, &count) != kCGErrorSuccess || count == 0) { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| CGDirectDisplayID displays[max_displays]; | ||
|
Check warning on line 53 in src/platform/macos/input.cpp
|
||
| count = std::min(count, max_displays); | ||
| if (CGGetActiveDisplayList(count, displays, &count) != kCGErrorSuccess) { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| std::vector<macos::mouse::rect_t> rects; | ||
| rects.reserve(count); | ||
| for (uint32_t i = 0; i < count; ++i) { | ||
| const auto bounds = CGDisplayBounds(displays[i]); | ||
| rects.push_back({bounds.origin.x, bounds.origin.y, bounds.size.width, bounds.size.height}); | ||
| } | ||
|
|
||
| return macos::mouse::layout_bounds(rects); | ||
| } | ||
| } // namespace | ||
|
|
||
| std::optional<util::point_t> get_mouse_loc(input_t & /*input*/) { | ||
| const auto event = CGEventCreate(nullptr); | ||
| if (!event) { | ||
| const auto current = cursor_point(); | ||
| if (!current) { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| const auto current = CGEventGetLocation(event); | ||
| CFRelease(event); | ||
| return util::point_t {current.x, current.y}; | ||
| return util::point_t {current->x, current->y}; | ||
| } | ||
|
|
||
| namespace macos { | ||
| bool move_mouse_relative(int delta_x, int delta_y) { | ||
| // CoreGraphics discards posted events without Accessibility. Report that so the caller | ||
| // can use the virtual HID path instead of silently dropping the movement. | ||
| if (!AXIsProcessTrusted()) { | ||
| return false; | ||
| } | ||
|
|
||
| static const bool logged = [] { | ||
| BOOST_LOG(info) << "macOS relative mouse backend selected: CoreGraphics"; | ||
| return true; | ||
| }(); | ||
| (void) logged; | ||
|
|
||
| static std::optional<mouse::bounds_t> layout; // Only reached from the input thread. | ||
| static std::optional<CGPoint> predicted; | ||
| static std::chrono::steady_clock::time_point last_packet; | ||
|
|
||
| // CGEventGetLocation() reports only what the window server has committed so far. Re-basing | ||
| // every packet on it makes a burst at 500-1000 Hz read the same stale position repeatedly, | ||
| // so each packet overwrites the previous one's movement and the pointer under-travels. | ||
| // Accumulate the chain locally instead, and trust the server again on the first packet or | ||
| // after a gap long enough for the pointer to have been moved by something else. | ||
| const auto now = std::chrono::steady_clock::now(); | ||
| auto base = predicted; | ||
| if (!base || now - last_packet > std::chrono::milliseconds {100}) { | ||
|
Comment on lines
+103
to
+104
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Reconcile cached prediction after another path moves the cursor
|
||
| const auto live = cursor_point(); | ||
| if (!live) { | ||
| return false; | ||
| } | ||
|
|
||
| base = live; | ||
| } | ||
|
|
||
| auto target = CGPoint {base->x + delta_x, base->y + delta_y}; | ||
| if (!layout || target.x < layout->min_x || target.y < layout->min_y || target.x >= layout->max_x || target.y >= layout->max_y) { | ||
| const auto refreshed = active_layout_bounds(); | ||
| if (!refreshed) { | ||
| return false; | ||
| } | ||
|
|
||
| layout = refreshed; | ||
| target = mouse::clamp_to_layout(target, *layout); | ||
| } | ||
|
|
||
| const auto type = mouse::move_event_type( | ||
| CGEventSourceButtonState(kCGEventSourceStateHIDSystemState, kCGMouseButtonLeft), | ||
| CGEventSourceButtonState(kCGEventSourceStateHIDSystemState, kCGMouseButtonRight), | ||
| CGEventSourceButtonState(kCGEventSourceStateHIDSystemState, kCGMouseButtonCenter) | ||
| ); | ||
|
|
||
| const auto event = CGEventCreateMouseEvent(nullptr, type, target, mouse::move_event_button(type)); | ||
| if (!event) { | ||
| return false; | ||
| } | ||
|
|
||
| // Describe the movement once, with the location and the device deltas agreeing: a fresh | ||
| // event leaves the deltas at zero, so apps reading them instead of the cursor location | ||
| // would see no movement at all. | ||
| CGEventSetDoubleValueField(event, kCGMouseEventDeltaX, target.x - base->x); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Preserve relative deltas when the predicted cursor reaches an edge These fields are derived from the clamped target rather than the received relative motion. On a 1920-point display, starting at x=1900 and sending three +10 packets produces deltas 10, 9, then 0. Once |
||
| CGEventSetDoubleValueField(event, kCGMouseEventDeltaY, target.y - base->y); | ||
|
|
||
| // Modifiers are reported per event, so a Shift/Option/Ctrl held during a drag has to travel | ||
| // with the motion. The button press comes from the virtual HID backend, which stamps the | ||
| // same live state onto its mouse events; without this the press carries the modifier and | ||
| // every drag after it loses it. | ||
| CGEventSetFlags(event, CGEventSourceFlagsState(kCGEventSourceStateHIDSystemState)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Carry the remote keyboard's modifier state into motion events The pinned libvirtualhid backend creates its keyboard source with |
||
|
|
||
| // Exactly one pointer update: no CGWarpMouseCursorPosition() on top of this event. | ||
| CGEventPost(kCGHIDEventTap, event); | ||
| CFRelease(event); | ||
|
|
||
| predicted = target; | ||
| last_packet = now; | ||
| return true; | ||
| } | ||
| } // namespace macos | ||
|
|
||
| platform_caps::caps_t get_capabilities() { | ||
| platform_caps::caps_t caps = 0; | ||
| const auto runtime = virtualhid::create_runtime(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| /** | ||
| * @file src/platform/macos/mouse_utils.h | ||
| * @brief Declarations for macOS relative pointer math helpers. | ||
| */ | ||
| #pragma once | ||
|
|
||
| // system includes | ||
| #include <CoreGraphics/CoreGraphics.h> | ||
|
|
||
| // standard includes | ||
| #include <algorithm> | ||
| #include <optional> | ||
| #include <vector> | ||
|
|
||
| namespace platf { | ||
| namespace macos { | ||
| namespace mouse { | ||
|
|
||
| /** | ||
| * @brief A display rectangle in global CoreGraphics point space. | ||
| */ | ||
| struct rect_t { | ||
| double x; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Document the new struct fields so the documentation build succeeds The Sunshine documentation build for this head fails with eight Doxygen errors: |
||
| double y; | ||
| double width; | ||
| double height; | ||
| }; | ||
|
|
||
| /** | ||
| * @brief The outer bounds of the entire display layout. | ||
| */ | ||
| struct bounds_t { | ||
| double min_x; | ||
| double min_y; | ||
| double max_x; | ||
| double max_y; | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Compute the union of every active display. | ||
| * @param rects The active display rectangles. | ||
| * @return The layout bounds, or an empty optional when there are no displays. | ||
| */ | ||
| inline std::optional<bounds_t> | ||
| layout_bounds(const std::vector<rect_t> &rects) { | ||
| if (rects.empty()) { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| bounds_t bounds {rects.front().x, rects.front().y, rects.front().x + rects.front().width, rects.front().y + rects.front().height}; | ||
| for (const auto &rect : rects) { | ||
| bounds.min_x = std::min(bounds.min_x, rect.x); | ||
| bounds.min_y = std::min(bounds.min_y, rect.y); | ||
| bounds.max_x = std::max(bounds.max_x, rect.x + rect.width); | ||
| bounds.max_y = std::max(bounds.max_y, rect.y + rect.height); | ||
| } | ||
|
|
||
| return bounds; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Clamp a point into the display layout. | ||
| * | ||
| * Secondary displays commonly have negative origins, so the whole layout is clamped | ||
| * instead of the main display rectangle. | ||
| * | ||
| * @param point The point to clamp. | ||
| * @param bounds The bounds to clamp to. | ||
| * @return The clamped point. | ||
| */ | ||
| inline CGPoint | ||
| clamp_to_layout(CGPoint point, const bounds_t &bounds) { | ||
| // Keep one point inside the layout so a drag near the edge still has a hit target. | ||
| const double max_x = bounds.max_x - 1.0; | ||
| const double max_y = bounds.max_y - 1.0; | ||
|
|
||
| return { | ||
| std::min(std::max(point.x, bounds.min_x), max_x), | ||
|
Comment on lines
+77
to
+78
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Clamp against actual display rectangles, including layout gaps The outer bounding rectangle is not the union of the displays for staggered layouts. Using the rectangles already in |
||
| std::min(std::max(point.y, bounds.min_y), max_y) | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Resolve the move event type for the currently held mouse buttons. | ||
| * @param left True when the left button is held. | ||
| * @param right True when the right button is held. | ||
| * @param center True when the center button is held. | ||
| * @return The CoreGraphics event type describing the move. | ||
| */ | ||
| inline CGEventType | ||
| move_event_type(bool left, bool right, bool center) { | ||
| if (left) { | ||
| return kCGEventLeftMouseDragged; | ||
| } | ||
| if (right) { | ||
| return kCGEventRightMouseDragged; | ||
| } | ||
| if (center) { | ||
| return kCGEventOtherMouseDragged; | ||
| } | ||
|
|
||
| return kCGEventMouseMoved; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Resolve the button reported alongside a move event. | ||
| * @param type The event type returned by `move_event_type()`. | ||
| * @return The mouse button for `CGEventCreateMouseEvent()`. | ||
| */ | ||
| inline CGMouseButton | ||
| move_event_button(CGEventType type) { | ||
| switch (type) { | ||
| case kCGEventRightMouseDragged: | ||
| return kCGMouseButtonRight; | ||
| case kCGEventOtherMouseDragged: | ||
| return kCGMouseButtonCenter; | ||
| default: | ||
| return kCGMouseButtonLeft; | ||
| } | ||
| } | ||
|
|
||
| } // namespace mouse | ||
| } // namespace macos | ||
|
|
||
| namespace macos { | ||
| /** | ||
| * @brief Inject a relative pointer movement through CoreGraphics. | ||
| * | ||
| * A single pointer update is emitted per movement so that the delta cannot be applied | ||
| * twice, or fought over by two mechanisms. | ||
| * | ||
| * @param delta_x The horizontal movement in pointer units. | ||
| * @param delta_y The vertical movement in pointer units. | ||
| * @return True when the movement was injected, false when CoreGraphics cannot be used. | ||
| */ | ||
| bool move_mouse_relative(int delta_x, int delta_y); | ||
| } // namespace macos | ||
|
|
||
| } // namespace platf | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,9 @@ | |
| // local includes | ||
| #include "src/config.h" | ||
| #include "src/logging.h" | ||
| #ifdef __APPLE__ | ||
| #include "src/platform/macos/mouse_utils.h" | ||
| #endif | ||
| #include "virtualhid_input.h" | ||
|
|
||
| using namespace std::literals; | ||
|
|
@@ -794,6 +797,23 @@ namespace platf::virtualhid { | |
| } | ||
|
|
||
| void move_mouse(input_context_t &context, int delta_x, int delta_y) { | ||
| #if defined(__APPLE__) && !defined(SUNSHINE_TESTS) | ||
| // libvirtualhid updates the pointer twice per relative packet: it posts a kCGHIDEventTap | ||
| // event carrying an absolute location *and* kCGMouseEventDeltaX/Y, then warps the cursor to | ||
| // the clamped location. Those two updates disagree - the delta is fed through the system | ||
| // pointer-acceleration curve and is computed from the unclamped position - so the cursor is | ||
| // nudged forward and snapped back on every packet. Relative motion therefore reads as the | ||
| // pointer barely moving or jittering in place, while button and scroll input, which never | ||
| // warps, keep working. Emit a single CoreGraphics update instead, and only fall back to the | ||
| // virtual HID report when CoreGraphics cannot inject events at all. | ||
| // | ||
| // Excluded from the test build: these tests assert the submitted virtual HID report, and | ||
| // must not move the pointer of whoever is running the suite. | ||
| if (platf::macos::move_mouse_relative(delta_x, delta_y)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Keep button events consistent with the new multi-display motion path This early return lets relative motion reach a secondary display, but |
||
| return; | ||
| } | ||
| #endif // __APPLE__ && !SUNSHINE_TESTS | ||
|
|
||
| if (context.mouse) { | ||
| log_failure("submit libvirtualhid mouse movement"sv, context.mouse->move_relative(delta_x, delta_y)); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Replace the C-style array to satisfy the required Sonar quality gate
The Sonar quality gate for this head fails on this newly added array. Its annotation explicitly requires
std::arrayfor a fixed-size buffer orstd::vectorfor a dynamically sized one. Usestd::array<CGDirectDisplayID, max_displays>and pass.data()toCGGetActiveDisplayList, or size a vector to the queried display count, to remove the confirmed new issue.