On Mon, 24 Feb 2025 17:19:58 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> 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 two additional
>> commits since the last revision:
>>
>> - Merge branch 'openjdk:master' into feature/vbox-fillwidth-bug-fix
>> - Make computeChildMin/Pref/MaxAreaHeight accept a fillWidth parameter
>
> modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line
> 2020:
>
>> 2018: double baseline = child.getBaselineOffset();
>> 2019: if (child.isResizable() && baseline ==
>> BASELINE_OFFSET_SAME_AS_HEIGHT) {
>> 2020: return top +
>> snapSizeY(boundedSize(child.minHeight(alt), max, Double.MAX_VALUE)) + bottom
>
> here and elsewhere: a wise man once said that the sum of snapped values may
> not come out snapped. Should we start snapping the result of any algebraic
> operation that involves snapped values?
LOL, I think I may have said that :) However, perhaps the problem is not as
bad as I made it out to be. It's definitely true that any operation on a
floating point value may slightly nudge it away from a true snapped value
(somewhere in the 10th decimal place) and that this problem gets worse the more
operations you perform (ie. a HBox with 10.000 children will have a worse error
than one with only a few children).
But there may be some middle ground possible here. As long as our layout
containers are aware that values returned from `computeMinWidth` etc are only
"near" snapped (meaning that if you round them to the nearest pixel, you'll
always get the right result), it should be fine -- one must take care not to
immediately call `ceil` or `floor` on these values. A thing we may want to
look into is how this is rendered on screen by the rendering layer; does the
rendering do additional work when values are near snapped, or do they perform
some rounding already anyway. I certainly never noticed these small errors
resulting in display artifacts.
In any case, this is probably better addressed in a separate effort, and we
should probably write some guidelines first. I'm hoping to do some of this
with the space distributor PR.
> modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line
> 2044:
>
>> 2042: * # content width/heights:
>> 2043: *
>> 2044: * The space allocated to a child, minus its margins. These are
>> never -1.
>
> or always `> 0.0` ?
Yeah, or possibly zero included. What I meant here is that these are **real**
values, and don't have a "special" value `-1` meaning absent/unavailable.
> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/RegionTest.java
> line 1025:
>
>> 1023: */
>> 1024:
>> 1025: assertEquals(12, RegionShim.computeChildMinAreaWidth(pane, c2,
>> -1, new Insets(1), -1, false), 1e-100);
>
> might as well declare a static `computeChildMinAreaWidth()` which delegates
> to `RegionShim.computeChildMinAreaWidth`...
Yeah, I can streamline this a bit more.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1723#discussion_r1968386958
PR Review Comment: https://git.openjdk.org/jfx/pull/1723#discussion_r1968389924
PR Review Comment: https://git.openjdk.org/jfx/pull/1723#discussion_r1968390974
