On Tue, 28 Jan 2025 23:21:26 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 incrementally with two additional > commits since the last revision: > > - helper function > - grab screen data on mouseMove src/java.desktop/share/classes/java/awt/Robot.java line 229: > 227: > 228: GraphicsEnvironment ge = > GraphicsEnvironment.getLocalGraphicsEnvironment(); > 229: GraphicsDevice[] gs = ge.getScreenDevices(); Suggestion: GraphicsDevice[] gds = ge.getScreenDevices(); I found that `gds` is a more common name in our code and testbase. At least for me, I immediately read `gds` as GraphicsDeviceS, array of `GraphicsDevice`. Same applies for the test. P.S. It's not a call to action, it's more of an invitation for discussion. src/java.desktop/share/classes/java/awt/Robot.java line 258: > 256: > 257: if (leastXDiff > leastYDiff) { > 258: peer.mouseMove(finX2, finY2); Let's say I have the following display configuration on Linux. Since it uses Xinerama, it shares the same coordinate system. java.awt.Rectangle[x=0,y=0,width=3440,height=1440] java.awt.Rectangle[x=3440,y=0,width=1440,height=2560]  When I try to move the mouse to `x=20000,y=200`, I see that it clamps to `x=3439,y=200` (a point between 2 screens), while before it was `x=4879,y=200`(a rightmost point). The old behavior seems more logical to me. src/java.desktop/share/classes/java/awt/Robot.java line 268: > 266: private Point calcClosestPoint(int x, int y, Rectangle screenBounds) > { > 267: return new Point(Math.min(Math.max(x, screenBounds.x), > screenBounds.x + screenBounds.width-1), > 268: 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)); minor spacing issue ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22781#discussion_r1939749017 PR Review Comment: https://git.openjdk.org/jdk/pull/22781#discussion_r1939760680 PR Review Comment: https://git.openjdk.org/jdk/pull/22781#discussion_r1939747548