On Mon, 10 Aug 2026 16:36:03 GMT, John Hendrikx <[email protected]> wrote:
>> There was a discussion about snapping only final values or also intermediate >> values, but I can't recall where. Maybe it was in >> https://github.com/openjdk/jfx/pull/445. > > I don't mind doing a 2nd pass fixing these small things. I can mark them > with a TODO and then fix in another PR while adjusting the small test > deviations that are likely to occur then. Things I'm already aware of: > > - StackPane (and BorderPane) not snapping the insets (major, adding snapped + > unsnapped) > - The compute methods not doing a final snap when adding several snapped > values together (minor) > - Some compute methods taking shortcuts assuming unbiased calculations > (medium) > - Use of `round` instead of `rint` which doesn't do well with values beyond > the `long` range (minor) > - Use of `Math.down` or `Math.ulp` in `ScaledMath.ceil` -- this should be a > fixed epsilon (like 1 millionth of a pixel) (minor) > > I can also consolidate all the `computeChildMin/Pref/MaxAreaWidth/Height` > methods into two methods (I did this for my own layout) with the same > semantics. It looks something like this: > > > 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))); > } > > > Basically a single function (`computeSpans`) that does what the > `computeChild*` methods do. I'm not entirely sure it will fit well in JavaFX > or if it is w... > There was a discussion about snapping only final values or also intermediate > values, but I can't recall where. Maybe it was in #445. Intermediate values don't need snapping, and, if you use `snapSize` (which uses `ceil`) this can even be detrimental as a tiny float errors can then get rounded up multiple times (10.00000000000001 -> 11). The snap functions which do rounding are a lot less dangerous and can basically be applied as often as you want (although best only for initial and final values). ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2241#discussion_r3751496125
