On Sun, 16 Aug 2026 00:43:27 GMT, Michael Strauß <[email protected]> wrote:
> `Region.layoutInArea()` calls `Region.boundedNodeSizeWithBias()`, which > derives the dependent value from a potentially unsnapped value, after which > `layoutInArea()` assigns a potentially different, snapped value to the child. > This is wrong, because the dependent value is effectively computed against a > value that might not be assigned to the child. > > For a horizontally biased child, the current sequence is effectively: > > > double rawWidth = boundedSize(...); > double rawHeight = boundedSize( > child.minHeight(rawWidth), > ... child.prefHeight(rawWidth) ..., > child.maxHeight(rawWidth)); > > child.resize( > snapSize(rawWidth), > snapSize(rawHeight)); > > > The correct sequence would be: > > > double width = snapSize(boundedSize(...)); > double height = snapSize(boundedSize( > child.minHeight(width), > ... child.prefHeight(width) ..., > child.maxHeight(width))); > > child.resize(width, height); > > > This snapping bug can only be observed when all of the following conditions > are met: > 1. pixel snapping is enabled > 2. the child is resizable and has a horizontal or vertical content bias > 3. the bounded primary dimension is not already correctly snapped > 4. the dependent size constraints change between the raw and snapped primary > dimensions > > This came out of the "snapping rules" I've compiled for PR #2260, > specifically the rule [Use the same snapped dependent dimension for > measurement and > layout](https://github.com/openjdk/jfx/pull/2260/changes#diff-3fe4ab83269e3ae0ec167ebe622893aff218168e7cab666e5a64c99e6dfbfd4fR464). > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). This is a good change. My only question is whether there are other places which needs to have the same treatment, such as StackPane. modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 530: > 528: double adjustedHeight = adjustHeightByMargin(insideHeight, > topMargin); > 529: Vec2d result = boundedNodeSizeWithBias(t, adjustedWidth, > 530: adjustedHeight, true, false, snapToPixel, > snapScaleX, snapScaleY, TEMP_VEC2D); Does this change impact only these three classes (`BorderPane`, `HeaderBar`, `Region`), or do we have to fix some others? For example, StackPane uses unsnapped width in L339 - would that be a problem? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 569: > 567: false, true, snapToPixel, snapScaleX, snapScaleY, > TEMP_VEC2D); > 568: leftWidth = result.x; > 569: l.resize(leftWidth, result.y); very minor: maybe rename result -> snappedResult for clarity? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 586: > 584: double adjustedHeight = adjustHeightByMargin(insideHeight - > topHeight - bottomHeight, rightMargin); > 585: > 586: Vec2d result = boundedNodeSizeWithBias(r, adjustedWidth, > adjustedHeight, and here? modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 2175: > 2173: * @param snapScaleY the vertical scale to use when snapping > 2174: * @param result Vec2d object for the result or null if new one > should be created > 2175: * @return Vec2d object with width(x parameter) and height (y > parameter) maybe add the word "snapped" to the return value for clarity ------------- PR Review: https://git.openjdk.org/jfx/pull/2261#pullrequestreview-4955185905 PR Review Comment: https://git.openjdk.org/jfx/pull/2261#discussion_r3799477941 PR Review Comment: https://git.openjdk.org/jfx/pull/2261#discussion_r3799339681 PR Review Comment: https://git.openjdk.org/jfx/pull/2261#discussion_r3799341920 PR Review Comment: https://git.openjdk.org/jfx/pull/2261#discussion_r3799344413
