On Wed, 19 Aug 2026 00:17:46 GMT, Michael Strauß <[email protected]> wrote:
> BorderPane's measurement and layout calculations are not correct when > pixel-snapping is enabled. Here is what's wrong with BorderPane, and what > consequently needs to be fixed: > > 1. Insets are not snapped: > * all measurement methods use raw insets > * `layoutChildren()` also uses raw insets > 3. Content-biased children use the wrong dependent dimension: > * top and bottom children call `prefHeight(adjustedWidth)` before their > final width is established; left and right have the same problem with height > * measurement and layout can disagree because > - constrained measurement passes the pane's total width/height without > first removing its snapped insets > - `getAreaWidth` uses `fillHeight=false`, while left/right/center > layout uses `fillHeight=true` > 5. Arithmetic results are not re-snapped. > > --------- > - [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/main/java/javafx/scene/layout/BorderPane.java line 413: > 411: final double topInset = snappedTopInset(); > 412: final double bottomInset = snappedBottomInset(); > 413: final double insideWidth = width < 0 ? -1 : snapSpaceX(width - > snappedLeftInset() - snappedRightInset()); can the result be negative? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 472: > 470: final double topInset = snappedTopInset(); > 471: final double bottomInset = snappedBottomInset(); > 472: final double insideWidth = width < 0 ? -1 : snapSpaceX(width - > snappedLeftInset() - snappedRightInset()); negative result? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 483: > 481: double leftPrefWidth = getAreaWidth(getLeft(), -1, false); > 482: double rightPrefWidth = getAreaWidth(getRight(), -1, false); > 483: double centerWidth = snapSpaceX(Math.max(0, insideWidth - > leftPrefWidth - rightPrefWidth)); negative result? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 522: > 520: final double insideY = snappedTopInset(); > 521: final double insideWidth = snapSpaceX(width - insideX - > snappedRightInset()); > 522: final double insideHeight = snapSpaceY(height - insideY - > snappedBottomInset()); negative result L521, 522 ? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 550: > 548: Insets bottomMargin = getNodeMargin(b); > 549: double adjustedWidth = > snapSpaceX(adjustWidthByMargin(insideWidth, bottomMargin)); > 550: double remainingHeight = snapSpaceY(insideHeight - > topHeight); negative result? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 558: > 556: bottomHeight = > snapSpaceY(snapSpaceY(bottomMargin.getBottom()) + bottomHeight + > snapSpaceY(bottomMargin.getTop())); > 557: Pos alignment = getAlignment(b); > 558: positionInArea(b, insideX, snapPositionY(insideY + > insideHeight - bottomHeight), possible negative result? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 589: > 587: if (r != null && r.isManaged()) { > 588: Insets rightMargin = getNodeMargin(r); > 589: double remainingWidth = snapSpaceX(insideWidth - leftWidth); possible negative result? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 598: > 596: rightWidth = snapSpaceX(snapSpaceX(rightMargin.getLeft()) + > rightWidth + snapSpaceX(rightMargin.getRight())); > 597: Pos alignment = getAlignment(r); > 598: positionInArea(r, snapPositionX(insideX + insideWidth - > rightWidth), middleY, possible negative result? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 608: > 606: Pos alignment = getAlignment(c); > 607: layoutInArea(c, snapPositionX(insideX + leftWidth), middleY, > 608: snapSpaceX(insideWidth - leftWidth - rightWidth), possible negative result? I wonder if snapSpace* should clamp the return value since the space, unlike position, cannot be negative? modules/javafx.graphics/src/test/java/test/javafx/scene/layout/BorderPaneTest.java line 1205: > 1203: Pane root = new Pane(borderpane); > 1204: stage = new Stage(); > 1205: stage.renderScaleXProperty().bind(new > SimpleDoubleProperty(scaleX)); binding is done with WeakReferences - should these properties be made instance fields instead to avoid them being collected during the test? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856546647 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856552540 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856554210 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856557452 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856561650 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856564125 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856566849 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856568207 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856569035 PR Review Comment: https://git.openjdk.org/jfx/pull/2262#discussion_r3856526282
