RFR: 8264591: HBox/VBox child widths pixel-snap to wrong value [v7]
John Hendrikx
jhendrikx at openjdk.org
Tue Apr 25 07:53:22 UTC 2023
On Tue, 3 Jan 2023 06:31:37 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
>> The children of HBox/VBox don't always pixel-snap to the same value as the container itself when a render scale other than 1 is used. This can lead to a visual glitch where the content bounds don't line up with the container bounds. In this case, the container will extend beyond its content, or vice versa.
>>
>> The following program shows the problem for HBox:
>>
>> Region r1 = new Region();
>> Region r2 = new Region();
>> Region r3 = new Region();
>> Region r4 = new Region();
>> Region r5 = new Region();
>> Region r6 = new Region();
>> r1.setBackground(new Background(new BackgroundFill(Color.GREY, null, null)));
>> r2.setBackground(new Background(new BackgroundFill(Color.DARKGRAY, null, null)));
>> r3.setBackground(new Background(new BackgroundFill(Color.BLACK, null, null)));
>> r4.setBackground(new Background(new BackgroundFill(Color.GREY, null, null)));
>> r5.setBackground(new Background(new BackgroundFill(Color.DARKGRAY, null, null)));
>> r6.setBackground(new Background(new BackgroundFill(Color.BLACK, null, null)));
>> r1.setPrefWidth(25.3);
>> r2.setPrefWidth(25.3);
>> r3.setPrefWidth(25.4);
>> r4.setPrefWidth(25.3);
>> r5.setPrefWidth(25.3);
>> r6.setPrefWidth(25.4);
>> r1.setMaxHeight(30);
>> r2.setMaxHeight(30);
>> r3.setMaxHeight(30);
>> r4.setMaxHeight(30);
>> r5.setMaxHeight(30);
>> r6.setMaxHeight(30);
>> HBox hbox1 = new HBox(r1, r2, r3, r4, r5, r6);
>> hbox1.setBackground(new Background(new BackgroundFill(Color.RED, null, null)));
>> hbox1.setPrefHeight(40);
>>
>> r1 = new Region();
>> r2 = new Region();
>> r3 = new Region();
>> r4 = new Region();
>> r5 = new Region();
>> r6 = new Region();
>> r1.setBackground(new Background(new BackgroundFill(Color.GREY, null, null)));
>> r2.setBackground(new Background(new BackgroundFill(Color.DARKGRAY, null, null)));
>> r3.setBackground(new Background(new BackgroundFill(Color.BLACK, null, null)));
>> r4.setBackground(new Background(new BackgroundFill(Color.GREY, null, null)));
>> r5.setBackground(new Background(new BackgroundFill(Color.DARKGRAY, null, null)));
>> r6.setBackground(new Background(new BackgroundFill(Color.BLACK, null, null)));
>> r1.setPrefWidth(25.3);
>> r2.setPrefWidth(25.3);
>> r3.setPrefWidth(25.4);
>> r4.setPrefWidth(25.3);
>> r5.setPrefWidth(25.3);
>> r6.setPrefWidth(25.4);
>> r1.setMaxHeight(30);
>> r2.setMaxHeight(30);
>> r3.setMaxHeight(30);
>> r4.setMaxHeight(30);
>> r5.setMaxHeight(30);
>> r6.setMaxHeight(30);
>> HBox hbox2 = new HBox(r1, r2, r3, r4, r5, r6);
>> hbox2.setBackground(new Background(new BackgroundFill(Color.RED, null, null)));
>> hbox2.setPrefHeight(40);
>> hbox2.setPrefWidth(152);
>>
>> VBox root = new VBox(new HBox(hbox1), new HBox(hbox2));
>> root.setSpacing(20);
>> Scene scene = new Scene(root, 500, 250);
>>
>> primaryStage.setScene(scene);
>> primaryStage.show();
>>
>>
>> Here's a before-and-after comparison of the program above after applying the fix. Note that 'before', the backgrounds of the container (red) and its content (black) don't align perfectly. The render scale is 175% in this example.
>> 
>>
>> I've filed a bug report and will change the title of the GitHub issue as soon as it's posted.
>
> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
>
> - changes per review
> - Merge branch 'master' into fixes/box-snap-to-pixel
>
> # Conflicts:
> # modules/javafx.graphics/src/test/java/test/javafx/scene/layout/HBoxTest.java
> # modules/javafx.graphics/src/test/java/test/javafx/scene/layout/VBoxTest.java
> - Merge branch 'openjdk:master' into fixes/box-snap-to-pixel
> - revert snappedSum
> - don't call snappedSum in hot loop
> - Improved code documentation
> - Merge branch 'master' into fixes/box-snap-to-pixel
> - changed some method names, make test config a local class
> - added documentation, improved method names
> - Merge branch 'master' into fixes/box-snap-to-pixel
> - ... and 2 more: https://git.openjdk.org/jfx/compare/a35c3bf7...fbbc273a
> > I'm sorry, I don't quite see what you mean here, perhaps this is unique to the TableView layout.
>
> It is not unique. The scenario I am trying to describe is when set[VH]Grow is true, possibly for multiple nodes. With the fractional scale and snapping, the snapped coordinates are not equidistant. So if one node has to move, all of a sudden its width must change, affecting the others. Which means
>
> 1. one cannot scale the constraints as they become position-dependent
> 2. no single pass algorithm is going to work
>
> (the reason I mentioned TableView is because I've tripped over the very same problem, see JDK-8299753)
>
> For reference:
>
> scale=1.5
> x=1 snapped=1.333 step=1.333
> x=2 snapped=2.000 step=0.667
> x=3 snapped=3.333 step=1.333
> x=4 snapped=4.000 step=0.667
Okay, I'm going to explain my understanding of the system, please correct me where I'm going wrong:
We have logical positions and sizes, which are `double`s and can be any value, not just integers. And we have physical positions and sizes which can also be any value, but can be snapped if we want them to correspond to physical pixels. These snapped physical coordinates will be integers, and when converted back to logical ones will become fractional for most render scales.
To go from logical to physical, you multiply the logical value by render scale. To go from physical to logical, you divide the physical value by render scale. To "snap" a logical value, you multiple it by the render scale, round it (depending on type), then divide it by render scale. To "snap" a physical value, you round it to an integer.
|Scale|Logical|Physical|Logical (snapped)|Physical (snapped)|
|---|---|---|---|---|
|1.0|0.0|0.0|0.0|0.0|
|1.0|0.5|0.5|1.0|1.0|
|1.0|1.0|1.0|1.0|1.0|
|1.0|1.5|1.5|2.0|2.0|
|1.5|0.0|0.0|0.0|0.0|
|1.5|0.5|0.75|0.66|1.0|
|1.5|1.0|1.5|1.33|2.0|
|1.5|1.5|2.25|1.33|2.0|
`HBox` will determine positions itself, and is not limited to integer logical coordinates, see example:
Given a scale of 1.5, and HBox has a logical margin of 1, and snapping is on. After snapping, the margin becomes a logical width of 1.33 (2 pixels wide) [calculation: round(1.0 * renderScale) / renderScale]. The first control therefore gets positioned at a logical position of 1.33. It's logical width can take on any multiple of the physical pixel size divided by the render scale, which is 1 / 1.5 = 0.66. So it can span from 1.33 to 2.0 or 1.33 to 2.66 or 1.33 to 3.33, etc.
What am I missing?
Are the column widths in TableView perhaps restricted to integer values?
-------------
PR Comment: https://git.openjdk.org/jfx/pull/445#issuecomment-1521320760
More information about the openjfx-dev
mailing list