On Fri, 21 Apr 2023 13:52:55 GMT, Karthik P K <[email protected]> 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