RFR: JDK-8306990: The guarantees given by Region's floor and ceiling functions should work for larger values [v2]
Andy Goryachev
angorya at openjdk.org
Thu Apr 27 19:16:24 UTC 2023
On Thu, 27 Apr 2023 19:04:39 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> Region has floor and ceiling functions that ensure that calling them twice in a row will yield the same result:
>>
>> ceil(x) = ceil(ceil(x))
>>
>> However, due to use of a constant `EPSILON` which is added/subtracted before doing the rounding, this only works for small numbers (in the range of 0-50 approximately). For larger values and scales, rounding errors can easily occur. This is visible as artifacts on screen where controls are a pixel wider than they should be.
>>
>> The use of the `EPSILON` constant is incorrect, as its value depends on the magnitude of the value in question (as magnitude increases, the fractional precision decreases).
>>
>> The Math class offers the function `ulp` that should be used here. It represents the smallest possible change in value for a given double.
>>
>> Extending the existing test case `snappingASnappedValueGivesTheSameValueTest` to use larger magnitude numbers exposes the problems.
>
> John Hendrikx has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>
> - Print seed if test fails
> - Merge branch 'master' into feature/ceil-fix
> - Fix Region ceil/floor function guarantees to be more accurate
modules/javafx.graphics/src/test/java/test/javafx/scene/layout/RegionTest.java line 1281:
> 1279: }
> 1280:
> 1281: random.setSeed(seed);
it might be better to set the seed only once at the beginning. otherwise the next 2 tests use the same sequence of doubles due to the seed being the same.
or is this intentional?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1118#discussion_r1179576437
More information about the openjfx-dev
mailing list