On Sat, 25 Oct 2025 09:26:33 GMT, John Hendrikx <[email protected]> wrote:

> This PR will snap the values returned by `ScrollPaneSkin`s compute methods, 
> and `Control`s `layoutChildren` method to prevent scroll bars from showing up 
> (when using the `AS_NEEDED` policy) due to subtle rounding errors.  The 
> scroll pane internally will compare its own height (minus insets) with its 
> content height, and determine if vertical (or horizontal) scroll bar needs to 
> be shown.  If the values being compared are even slightly off (by a few ulp's 
> in floating point terms) then it may conclude a scroll bar should be shown 
> when it should not be.
> 
> See the sample program and video in the related JBS ticket.
> 
> Note: in many places we forget to resnap values after seemingly innocent 
> floating point calculations (even when all values involved are snapped 
> already, errors can be introduced).  It is out of scope for this PR to fix 
> all of these.
> 
> Note 2: since we only want to fix a rounding error, `snapSpace` is used and 
> not `snapSize` as the latter will ceil values which would make tiny errors 
> worse if they are just one ulp above the desired value.

Thank you, @hjohn !

I think this is the right approach; the chance of regression is likely to be 
low, despite the change impacting all the controls.

A side note: as a rule, we could probably snap just the final result, even 
considering small floating point errors that accumulate in the intermediate 
computation.  For example, let's estimate a rather unlikely case of a table 
with 1000 columns of 1000 pixels each.  `Math.ulp(1000.0) = 
1.1368683772161603E-13`, so the possible error multiplied by 1000 is still < 
`2E-10`.  As long as we snap the result (or when a comparison is performed), we 
should be ok.

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

Marked as reviewed by angorya (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1948#pullrequestreview-3385585071

Reply via email to