RFR: 8350149: VBox ignores bias of child controls when fillWidth is set to false [v2]

[John Hendrikx]

On Mon, 24 Feb 2025 17:19:58 GMT, Andy Goryachev <angorya at 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