On Fri, 5 May 2023 08:08:30 GMT, John Hendrikx <[email protected]> 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 incrementally with three 
> additional commits since the last revision:
> 
>  - Remove seed
>  - Add test cases for negative values
>  - Add copyright headers

LGTM, thanks!

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

Marked as reviewed by angorya (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1118#pullrequestreview-1415236012

Reply via email to