On Wed, 28 Oct 2026 16:25:52 GMT, Florian Kirmaier <[email protected]> 
wrote:

>> Follow-up to JDK-8092379: since then a gap is only rendered before a 
>> row/column in which a child starts, but two places still assumed a gap 
>> between every row/column.
>> 
>> adjustRowHeights / adjustColumnWidths reserved a gap before every row/column 
>> before distributing the percentages. They now reserve only the rendered gaps.
>> 
>> CompositeSize.getProportionalMinOrMaxSize divided the size of a spanning 
>> child by the number of rows/columns without subtracting the gaps inside the 
>> span. It now subtracts them.
>> 
>> Each fix is covered by tests that fail before and pass after the change.
>> A test application can be found in the ticket.
>> 
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Florian Kirmaier has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8392158 Split a span's min among rows that don't exceed their share

modules/javafx.graphics/src/main/java/javafx/scene/layout/GridPane.java line 
1786:

> 1784:                 if (isSnapToPixel()) {
> 1785:                     areaW = ScaledMath.rint(areaW, getSnapScaleX(this));
> 1786:                     areaH = ScaledMath.rint(areaH, getSnapScaleY(this));

`.rint()` is probably incorrect here: size 50.25 at scale=1.0 will be measured 
at 51 pixels when computing the pref size, while the layout allocates 50.  The 
comment applies to both `areaW` and `areaH`.

modules/javafx.graphics/src/main/java/javafx/scene/layout/GridPane.java line 
2057:

> 2055:         while (available != 0 && wasPositive == isPositive && 
> adjusting.size() > 0) {
> 2056:             if (!handleRemainder) {
> 2057:                 final double pixels = isSnapToPixel() ? 
> Math.rint(available / pixelSize) : available;

possible regression:
- scale=1, no gaps
- first row set at 10.5, second grow ALWAYS
- two children, min height 0, pref=20
- resize grid to 32

modules/javafx.graphics/src/main/java/javafx/scene/layout/GridPane.java line 
2686:

> 2684:                             }
> 2685:                         } while (changed && count > 0);
> 2686:                         

trailing whitespace

modules/javafx.graphics/src/main/java/javafx/scene/layout/GridPane.java line 
2692:

> 2690:                                 index += own[j - i.begin] ? 0 : 1;
> 2691:                             }
> 2692:                             double propSize = segment(rest, count, 
> index, pixelSize);

there might be a case where the layout overflows: 
- scale=1.0, no gaps
- three rows, first row set at 20%
- each row contains a child, with pref=200 and minimums set to 0, 80, 0
- spanning child min=100, pref=100

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

PR Review Comment: https://git.openjdk.org/jfx/pull/2312#discussion_r4404010285
PR Review Comment: https://git.openjdk.org/jfx/pull/2312#discussion_r4404329377
PR Review Comment: https://git.openjdk.org/jfx/pull/2312#discussion_r4404211969
PR Review Comment: https://git.openjdk.org/jfx/pull/2312#discussion_r4404239286

Reply via email to