On Thu, 27 Aug 2026 11:17:09 GMT, Michael Strauß <[email protected]> wrote:
> This PR is an audit of several snapping-related methods in `Region`, along > with some bug fixes. > > I've added a helper method `snapAligned()`, which does the exact same thing > as `snapSpace()`, but clearly states that the author knows that the value is > already pixel-aligned. In addition, I've renamed several local variables > around a "rawFoo" and "snappedFoo" naming scheme, so as to make it easier to > see what's what. > > This PR should probably be integrated before the other layout container PRs, > because those use the `Region` layout methods. > > --------- > - [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/Region.java line 1999: > 1997: if (prefBaselineComplement != -1) { > 1998: double baseline = child.getBaselineOffset(); > 1999: if (child.isResizable() && baseline == > BASELINE_OFFSET_SAME_AS_HEIGHT) { Here and in other places: The sentinel check unnecessarily required `child.isResizable()`. A non-resizable node could also return `BASELINE_OFFSET_SAME_AS_HEIGHT`; the current regular-baseline branch would then add negative infinity. The check should be based on baseline alone. modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 2006: > 2004: } else { > 2005: // For all other Nodes, it's just their baseline and > the complement. > 2006: // Note that the complement already contain the Node's > preferred (or fixed) height No it doesn't. It contains only the greatest extent below the baseline, derived from a preferred or fixed height. modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 2017: > 2015: double max = child.maxWidth(-1); > 2016: if (max == Double.MAX_VALUE) { > 2017: return max; The method returned immediately when `maxWidth(-1)` is unbounded, before calculating the actual height for a vertically biased child. Such a child may have `maxWidth(-1) == Double.MAX_VALUE` but a finite `maxWidth(actualWidth)`. The fix is to calculate `alt`, then query `maxWidth(alt)`, and only then apply the `Double.MAX_VALUE` shortcut. modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 2637: > 2635: // Everything below the baseline is like an > "inset". The Node with BASELINE_OFFSET_SAME_AS_HEIGHT cannot > 2636: // be resized to this area > 2637: bottom += snapSpace(areaHeight - > areaBaselineOffset, isSnapToPixel, snapScaleY); The old code double-counted `bottom` for a resizable, baseline-aligned child whose baseline is `BASELINE_OFFSET_SAME_AS_HEIGHT`. `areaHeight - areaBaselineOffset` is already the complete space below the baseline. When the baseline comes from `getAreaBaselineOffset()`, that space already includes the child's bottom margin. The regression test for this bug is `RegionTest.testLayoutInAreaBaselineSameAsHeightWithMargins` ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3871207676 PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3871200726 PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3871237434 PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3871269552
