On Fri, 18 Aug 2023 17:48:16 GMT, Andy Goryachev <[email protected]> wrote:
>> You might be right.
>>
>> In this case, `topPrefHeight` comes from `getAreaHeight()`, that calls
>> `computeChildPrefAreaHeight()` that ultimately uses `snapSpaceY()`.
>>
>> However, this would also mean that the returned value should use snapped
>> insets as well?
>>
>> return insets.getLeft() +
>> Math.max(leftPrefWidth + centerMinWidth + rightPrefWidth,
>> Math.max(topMinWidth,bottomMinWidth)) +
>> insets.getRight();
>>
>> should change into
>>
>> return snappedLeftInset() +
>> Math.max(leftPrefWidth + centerMinWidth + rightPrefWidth,
>> Math.max(topMinWidth, bottomMinWidth)) +
>> snappedRightInset();
>>
>>
>> Checking `AnchorPane`, it does use the latter, but not to get the pref size:
>>
>> private double computeHeight(final boolean minimum, final double width) {
>> double max = 0;
>> double contentWidth = width != -1 ? width - getInsets().getLeft()-
>> getInsets().getRight() : -1;
>> ...
>> return snappedTopInset() + max + snappedBottomInset();
>> }
>
> Following the great insight @hjohn vocalized in one of the earlier PRs, at
> the end we are always dealing with integer pixel coordinates.
>
> so, to be correct, any computation that returns pixel coordinates must use
> snapped values. (it also means that any computation that does not result in
> pixel coordinates, might use unsnapped values, such as when we try to
> distribute a single pixel among many columns or nodes).
Please, see my edited comment. If we are to change to snapped values
_everywhere_ (not only in the proposed fix of this PR), that would be a broader
change...
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1203#discussion_r1298718488
