On Tue, 27 Oct 2020 10:38:34 GMT, Jose Pereda <jper...@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.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update JBS reference in RegionTest

modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 303:

> 301:     }
> 302: 
> 303:     private static double scaledCeil(double value, double scale) {

Can you explicitly add here that this method guarantees 
`scaledCeil(scaledCeil(val, scale), scale) == scaledCeil (val, scale)` 

It seems other code already assumed this, but it should not be taken for 
granted, so now that your fix assures this is the case, it can be taken for 
granted.

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

PR: https://git.openjdk.java.net/jfx/pull/336

Reply via email to