On Fri, 21 Apr 2023 13:52:55 GMT, Karthik P K <k...@openjdk.org> wrote:

>> Usage of `getBounds()` method instead `getVisualBounds()` was giving 
>> unreliable screen bound values and color value was read very close to the 
>> edge of the window.
>> 
>> Updated the code to use `getVisualBounds()` instead of `getBounds()` and 
>> moved the coordinates inside the window from where the color value is read.
>> 
>> Ran the tests individually and along with all system tests in following 
>> systems. No failure found after the fix.
>> Mac M1 with Ventura 13.3
>> Window 11
>
> Karthik P K has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated visual bounds value for x y coordinates to pick color and changed 
> offset from edge to 10

Looks good. I left a couple minor comments, and will re-approve if you make the 
changes.

tests/system/src/test/java/test/robot/helloworld/CustomSecurityManagerTest.java 
line 172:

> 170:                 }
> 171:                 testStage1.setX(((int)screenBounds.getWidth() - WIDTH) / 
> 2);
> 172:                 testStage1.setY(((int)screenBounds.getHeight() - HEIGHT) 
> / 2);

Minor: the cast to `(int)` is unnecessary here.

tests/system/src/test/java/test/robot/helloworld/CustomSecurityManagerTest.java 
line 205:

> 203:                 } else {
> 204:                     y = (int)screenBounds.getMaxY() - offset - 1;
> 205:                 }

The comment is now incorrect. Maybe you can delete the comment and replace this 
block with similar logic as used for `x` below?

-------------

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1103#pullrequestreview-1396020833
PR Review Comment: https://git.openjdk.org/jfx/pull/1103#discussion_r1173934008
PR Review Comment: https://git.openjdk.org/jfx/pull/1103#discussion_r1173934836

Reply via email to