On Wed, 30 Apr 2025 21:13:12 GMT, Alisen Chung <ach...@openjdk.org> wrote:
>> Currently on macOS when mouseMove is given an offscreen coordinate to move >> the mouse to, mouseMove will physically clamp to the edge of the screen, but >> if you try to grab the mouse location immediately after by using >> MouseInfo.getPointerInfo().getLocation() you will get the value of the >> offscreen point. >> >> Windows and linux do this clamping and coordinate handling for us, but new >> distributions may not necessarily handle clamping the same way, so Robot >> should be checking for clamping rather than delegating it to native. >> >> This fix updates shared code to cache the screen bounds and adds a check to >> not exceed the bounds in mouseMove. The caching is done in the Robot >> constructor, so if the screen bounds changes the constructor must be called >> again to update to the new bounds. > > Alisen Chung has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 17 commits: > > - Merge branch 'master' of https://github.com/openjdk/jdk into 8345538 > - move clamping code into macos > - use absolute distance to choose correct screen for offscreen clamping > - helper function > - grab screen data on mouseMove > - fix bounds > - peer.mouseMove > - fix implementation > - robot update > - Revert "robot multimonitor fix" > > This reverts commit 5734165881a66dc48d5a9f19e02bf63fac57cdc9. > - ... and 7 more: https://git.openjdk.org/jdk/compare/8b16897b...e0a5c872 Tested the fix on macOS it works as expected and the native reproducer (attached to JBS) highlights the offscreen point issue and the need for clamping. The fix looks good to me, I may require some time to catch up on the previous review comments before approving (there has been a lot of update since I last reviewed). src/java.desktop/macosx/classes/sun/lwawt/macosx/CRobot.java line 102: > 100: private Point calcClosestPoint(int x, int y, Rectangle screenBounds) > { > 101: return new Point(Math.min(Math.max(x, screenBounds.x), > screenBounds.x + screenBounds.width-1), > 102: Math.min(Math.max(y, screenBounds.y), screenBounds.y + > screenBounds.height-1)); Suggestion: return new Point(Math.min(Math.max(x, screenBounds.x), screenBounds.x + screenBounds.width - 1), Math.min(Math.max(y, screenBounds.y), screenBounds.y + screenBounds.height - 1)); test/jdk/java/awt/Robot/MouseMoveOffScreen.java line 80: > 78: } > 79: return p; > 80: } I believe validateOffScreen() was added to prevent edge or boundary case rounding error as observed here - https://github.com/openjdk/jdk/pull/22781/files#r1926210713 ------------- PR Review: https://git.openjdk.org/jdk/pull/22781#pullrequestreview-2862874069 PR Review Comment: https://git.openjdk.org/jdk/pull/22781#discussion_r2103597214 PR Review Comment: https://git.openjdk.org/jdk/pull/22781#discussion_r2103598862
