On Thu, 10 Sep 2026 13:54:00 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 refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. The pull request > contains one new commit since the last revision: > > JDK-8392158 > fix how adjustRowHeights / adjustColumnWidths and > CompositeSize.getProportionalMinOrMaxSize compute modules/javafx.graphics/src/main/java/javafx/scene/layout/GridPane.java line 2654: > 2652: for (Interval i : multiSizes.keySet()) { > 2653: if (i.contains(position)) { > 2654: double segment = (multiSizes.get(i) - > computeGaps(i.begin, i.end)) / i.size(); I think we might still have a problem: consider two children with 100 pix minimum size and 1 pixel gap. the formula produces 49.5 which will be wounded up to 50 on L2055 and L2298. change to `gridpane.setVgap(1);` on GridPaneTest:3297 to see the issue modules/javafx.graphics/src/test/java/test/javafx/scene/layout/GridPaneTest.java line 3251: > 3249: fixedArea.assertSize(); > 3250: } > 3251: Do you think we should have this tests parameterized with different scales to account for differences in snapping behavior (100%, 125, 150, 175, 200, 225)? modules/javafx.graphics/src/test/java/test/javafx/scene/layout/GridPaneTest.java line 3297: > 3295: @Test > 3296: public void testCheckShrinkingRowsAccountsVgap() { > 3297: gridpane.setVgap(10); also, perhaps it would make sense to parameterize the gaps as well (0, 0.5, 1.0, 3.333, 5, 10) ? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2312#discussion_r4050554819 PR Review Comment: https://git.openjdk.org/jfx/pull/2312#discussion_r4050565663 PR Review Comment: https://git.openjdk.org/jfx/pull/2312#discussion_r4050596795
