On Thu, 10 Sep 2026 23:41:10 GMT, Michael Strauß <[email protected]> wrote:
>> This PR formalizes render scale and snapping policy as an inheritable layout >> context. I've chosen the term "layout context" because it establishes >> additional inputs to the layout algorithm that need to be accounted for, but >> are not part of the geometric definition of the nodes in the scene graph >> themselves. >> >> For example, consider a `VBox` with a spacing of 0.6: the gaps snap to 1.0 >> at render scale 1, but to 0.5 at render scale 2. Even though the size of the >> `VBox` and all of its inputs remain exactly the same, its children still >> need to move a little bit. This is especially relevant for layout containers >> that cache measurements or layout results (of which there are some), and >> makes it necessary that we have a way to invalidate those containers when >> the render scale or effective snapping policy changes. >> >> For this purpose, the following new APIs are added: >> 1. `Parent.isSnappedToPixel()` returns the effective pixel-snapping policy, >> which is only `true` if the node itself and all of its ancestors have their >> `snapToPixel` property set. All snapping methods use this method (instead of >> the `snapToPixel` property) to determine whether values should be snapped. >> 2. `Parent.layoutContextInvalidated()` will be called whenever the render >> scale or the effective pixel-snapping policy changes. This method can be >> overridden by subclasses to clear their cached measurements. >> >> Since we now have a way to invalidate the layout context, we can also cache >> render scales in each node instead of chasing pointers back to `Window` for >> every individual snapping operation. Interestingly, `Region` already cached >> the render scales, but in a useless way that accomplished almost nothing. >> >> Additionally, the snapping methods and the `snapToPixel` property are moved >> from `Region` to `Parent`, as I think having them on `Region` was always >> incorrect. Layout is a mechanism introduced by `Parent` (the >> `layoutChildren()` method is defined on `Parent`), and pixel-snapping >> intrinsically belongs to layout. Think about it: there can be subclasses of >> `Parent` that can lay out nodes, so they must also have access to >> pixel-snapping APIs. One such example is `Group`, which resizes its children >> to their preferred sizes, but is itself not a `Region`. >> >> Moving methods to a superclass is a binary- and source-compatible change. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > revert: move deprecated methods back to Region modules/javafx.controls/src/main/java/javafx/scene/control/skin/ScrollPaneSkin.java line 691: > 689: } > 690: > 691: @Override protected void layoutContextInvalidated() { please move `@Override` annotation to its own line. modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 1126: > 1124: * to the closest value on the pixel grid. > 1125: * > 1126: * @param value the vertical value horizontal value modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 1237: > 1235: if (changed) { > 1236: if (node instanceof Parent parent) { > 1237: parent.requestLayout(); my buddy tells me that this code causes an issue in `Axis`, where requestLayout() is a no-op. if the chart is already laid out, calling `axis.setSnapToPixel(false)` marks the axis subtree `NEEDS_LAYOUT`, but not the chart and the pulse is never requested, so the layout turns stale. In the master, Region:669 calls requestParentLayout() which goes around `Axis` no-op. modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 1318: > 1316: // on the old tileHeight. We need to clear any such transient > results before notifying needsLayout. > 1317: clearSizeCache(); > 1318: setLayoutFlag(LayoutFlags.NEEDS_LAYOUT); setting `NEEDS_LAYOUT` directly avoids each descendant requestLayout(). this will break RichTextArea because it marks its internal cache dirty in VFlow:1238 inside its `requestLayout()`. The fix for `RichTextArea` is to override `layoutContextInvalidated()`, but it also represents a fairly opaque compatibility risk for applications that invalidate caches from `requestLayout()`. modules/javafx.graphics/src/main/java/javafx/scene/layout/FlowPane.java line 653: > 651: } > 652: > 653: @Override protected void layoutContextInvalidated() { please put the annotation on its own line modules/javafx.graphics/src/main/java/javafx/scene/layout/GridPane.java line 1712: > 1710: } > 1711: > 1712: @Override protected void layoutContextInvalidated() { please put the annotation on its own line modules/javafx.graphics/src/main/java/javafx/scene/layout/HBox.java line 569: > 567: > 568: @Override public void requestLayout() { > 569: clearMeasurements(); own line here and L573 modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 272: > 270: } > 271: > 272: return width - snapSpaceX(margin.getLeft()) - > snapSpaceX(margin.getRight()); should the result be snapped here? modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 1788: > 1786: * @since 9 > 1787: */ > 1788: public double snapSpaceX(double value) { moving these method to the superclass introduces a minor compatibility risk (custom class extends `Parent`, declares a method with the same signature) but more importantly, the final keyword prevents overriding `Region.snapSpaceXX()` which was legal before this PR. I don't know why the application would want to override, but it will fail to compile. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4019522618 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4019974850 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4020419332 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4020461458 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4019560198 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4019562628 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4019571266 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4019720018 PR Review Comment: https://git.openjdk.org/jfx/pull/2309#discussion_r4019959123
