On Wed, 5 Aug 2026 03:11:44 GMT, John Hendrikx <[email protected]> wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 522:
>>
>>> 520:
>>> 521: @Override
>>> 522: public RenderScaleContext
>>> getRenderScaleContext(Scene scene) {
>>
>> minor: would it make sense to combine `Snapper` and `RenderScaleContext` ->
>> `RenderContext`?
>
> I think it is best not to (I went back and forth a lot on this while building
> it). The reason I think we're better off this way is that I feel that
> providing a Layout with render scale information is a better fit than
> providing it with a way it should adjust its calculations (layouts should be
> able to determine themselves how they want to deal with device pixels).
>
> Where render scale is a simple fact of the rendering surface (ratio of
> logical to device pixels), snapper is just a potential way to deal with that.
>
> I also think (when this becomes public API) that `RenderScaleContext` or
> `RenderScale` is a nicer API to have users deal with than `Snapper`.
> `Snapper` can still become public API if you feel that we should provide this
> to Layout creators so they don't have to roll their own -- it is just not
> required for a minimal implementation.
this makes sense. I would also like to learn what other people think.
>> modules/javafx.graphics/src/main/java/javafx/scene/layout/StackPaneLayout.java
>> line 164:
>>
>>> 162: @Override
>>> 163: public double minWidth(double height) {
>>> 164: // TODO pre-existing bug, insets not snapped anywhere
>>
>> probably not a bug: these methods should not return snapped values (as they
>> might come from properties). the snapping should be done by the caller.
>
> I think it is a bug; other containers (including the "big" ones like
> HBox/VBox) do snap these.
>
> I think it would lead to subtle issue when mixing snapped/unsnapped content
> as well: for an unsnapped container, you can provide unsnapped sizes, but a
> snapped one should include the space it needs to do correct snapping (and not
> rely on the parent container to also be snapped which is why this problem is
> hidden usually now).
>
> So if a `StackPane` has a child of 50 pixels wide, and insets of 0.6 then
> `minWidth(-1)` should return:
> - 0.6 + 50 + 0.6 = 51.2 (unsnapped)
> - 1 + 50 + 1 = 52.0 (snapped)
>
> When placed inside a snapped container, both will `ceil()` to 52, but if
> `StackPane` is snapped and its container isn't, it would get only 51.2 pixels
> assigned to it, but it will still try to place things at pixel offsets,
> meaning 0.8 pixels of space (or border decoration) would get clipped.
>
> Also I think that if you use a `StackPane` as root for a `Scene` (quite
> common) which doesn't do snapping of its own, and give it insets (not
> uncommon) you may find that the `Window` is one pixel too small (in either or
> both directions). Usually this is unnoticable as it just crops one pixel of
> empty space, but it could show up as a subtle difference between the left/top
> and right/bottom spacing.
makes sense.
so the rule should be - if the value comes from a property it can (and should)
not be snapped (because set == get == property.get), but if there is one or
more entities involved then we better snap each constituent.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2241#discussion_r3723831662
PR Review Comment: https://git.openjdk.org/jfx/pull/2241#discussion_r3723818459
