On Thu, 27 Aug 2026 11:23:46 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 > 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. Yes, this is also something I found (I use a consolidated version of all these functions instead of writing 3 or 6 different ones) and I remove this "short cut" to make them all match (as the short-cut is wrong). Consolidated variants look like this, one function for width/height min/pref/max: default double computeSpan(SizeQuery query, Measurable child, double baselineComplement, Insets margin, double extent, boolean fillExtent) { boolean usesBaseline = baselineComplement != -1 && orientation() == Orientation.VERTICAL; if(usesBaseline) { double baseline = child.getBaselineOffset(); if(baseline != BASELINE_OFFSET_SAME_AS_HEIGHT) { return baseline + baselineComplement; } } double dependentExtent = -1; if(extent != -1 && child.getContentBias() == cross().orientation()) { // span depends on cross span double areaExtent = baselineComplement != -1 && orientation() == Orientation.HORIZONTAL && child.getBaselineOffset() == BASELINE_OFFSET_SAME_AS_HEIGHT ? extent - baselineComplement : extent; dependentExtent = cross().computeDependentExtent(child, margin, areaExtent, fillExtent); } return margin(margin) + snapSize(query.compute(this, child, dependentExtent)) + (usesBaseline ? baselineComplement : 0); } default double computeDependentExtent(Measurable child, Insets margin, double areaExtent, boolean fillExtent) { double contentExtent = areaExtent - margin(margin); return fillExtent ? snapSize(boundedSize(min(child, -1), contentExtent, max(child, -1))) : snapSize(boundedSize(min(child, -1), pref(child, -1), Math.min(max(child, -1), contentExtent))); } ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2278#discussion_r3872429758
