On Tue, 27 Oct 2020 00:03:18 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> As discussed in the [JBS >> issue](https://bugs.openjdk.java.net/browse/JDK-8199592), when snapping an >> already snapped value (either intentionally or by mistake), the result >> should be the same, otherwise we'll be jumping unnecessary from a valid >> pixel to another pixel. >> >> This PR provides a fix to `snapSizeXX` methods used in `Region`, which >> ultimately use `Math.ceil`, by subtracting an epsilon value to scaled value >> before ceiling, to ensure snapping a snapped value gives the same value. >> >> A test to verify `snapSizeX` and `snapSizeY` with 1000 random values, and 6 >> different UI scales is provided. >> For the 1.0, 1.25, 1.5 and 2.0 UI scales, the current approach works fine. >> Only for 1.75 and the random 1.374562997 value fails (the test fails for >> around 2% of the values with 1.75 and around 10% with 1.374562997). >> With the proposed fix, it doesn't fail at all. > > Looks good. > > I can confirm that this fixes the problem of `snapSizeXY(snapSizeXY(val)) != > snapSizeXY(val)` and that it fixes the test case from > [JDK-8199592](https://bugs.openjdk.java.net/browse/JDK-8199592) when > `sizeToScene` is called (as noted above this is a fix for one of the two > underlying problems). > > I also verified that the new test will catch the bug. wondering about other places where scaled ceiling is used, f.i. snapPortion (which is package private and only used in HBox/VBox) - could they have the same problem? ------------- PR: https://git.openjdk.java.net/jfx/pull/336
