[11] JDK-8196031: FX Robot mouseMove fails on Windows 10 1709 with HiDPI
Kevin Rushforth
kevin.rushforth at oracle.com
Fri Jun 8 19:45:21 UTC 2018
The fix works for me, but I have a couple comment on the implementation
and a couple more on the test:
1. You borrowed the following from the Java2D fix:
+ signum((int)fx);
+ signum((int)fy);
This effectively does a rounding in the normalized absolute coordinate
space use by the Windows mouse move request. Given that we already round
the values in the unscaled (FX) space before the conversion, I don't
think this is necessary or desirable. I recommend removing it (and you
can remove the function entirely, since it was added just for this).
2. One other difference between this and the 2D fix is that the FX fix
uses 65535 (which was in my initial prototype), whereas the 2D fix uses
65536. The latter is probably more accurate, so you might want to change
it to 65536.
Test comments:
3. I recommend using Util.runAndWait rather than runLater with your own
latch everywhere. That method also propagates exceptions so you can do
an assertEquals right in that block rather than saving a "mismatch"
flag. You will get a better error message that way since you will see
the actual and expected values in the assertions rather than having to
look at the log (or run with --info) to see them.
4. Since you are using a for loop with increment by 1 in cross and edge,
it might be better to use "int"s for those params after all (sorry for
leading you astray on this one earlier).
5. Minor: we usually use 20 seconds for test timeouts, and this is a
short-running test, so maybe switch to that? (it's up to you)
-- Kevin
On 6/7/2018 12:57 PM, Pankaj Bansal wrote:
> Hi Kevin & Murali,
>
>
>
> Please review this fix,
>
> Webrev: http://cr.openjdk.java.net/~pbansal/8196031/webrev.00/
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8196031
>
>
>
>
>
> Regards,
>
> Pankaj
>
>
More information about the openjfx-dev
mailing list