On Fri, 12 Jun 2026 09:38:42 GMT, Florian Kirmaier <[email protected]> 
wrote:

> This PR fixes the computeSize methods not considering the gap in some cases.
> 
> It also fixes a missed spot, in the previous PR for JDK-8092379.
> 
> Each fix is covered by simple tests which fail before and work after the 
> change.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

modules/javafx.graphics/src/test/java/test/javafx/scene/layout/GridPaneTest.java
 line 3174:

> 3172: 
> 3173:     static class FixedAreaRegion extends Region {
> 3174:         final double AREA = 100*100;

Suggestion:

        final double AREA = 10000;

modules/javafx.graphics/src/test/java/test/javafx/scene/layout/GridPaneTest.java
 line 3215:

> 3213: 
> 3214:         assertEquals(70, fixedArea.getWidth());
> 3215:         fixedArea.checkSize();

I would rather prefer an accurate assertions here and below. This also makes it 
more understandable what the test does IMO.

The you can also remove the `checkSize` method.

Suggestion:

        assertEquals(143, fixedArea.getHeight());


or

Suggestion:

        assertEquals(Math.round(FixedAreaRegion.AREA / 70), 
fixedArea.getHeight());

modules/javafx.graphics/src/test/java/test/javafx/scene/layout/GridPaneTest.java
 line 3232:

> 3230: 
> 3231:         assertEquals(70, fixedArea.getHeight());
> 3232:         fixedArea.checkSize();

Suggestion:

        assertEquals(143, fixedArea.getWidth());

modules/javafx.graphics/src/test/java/test/javafx/scene/layout/GridPaneTest.java
 line 3248:

> 3246: 
> 3247:         assertEquals(100, fixedArea.getWidth());
> 3248:         fixedArea.checkSize();

Suggestion:

        assertEquals(100, fixedArea.getHeight());

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

PR Review Comment: https://git.openjdk.org/jfx/pull/2187#discussion_r3446432386
PR Review Comment: https://git.openjdk.org/jfx/pull/2187#discussion_r3446422003
PR Review Comment: https://git.openjdk.org/jfx/pull/2187#discussion_r3446422662
PR Review Comment: https://git.openjdk.org/jfx/pull/2187#discussion_r3446423269

Reply via email to