RFR: 8264591: HBox/VBox child widths pixel-snap to wrong value [v7]

[Michael Strauß]

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,...
>
> 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've tested #1111, which solves the problem that this PR addresses.
I've also tested #1118, which is not sufficient to fix the problem (child widths can still exceed container width).
Since the implementation is quite different, I think we should close this PR, and move #1111 forward.

> It's also possible that the integration of https://github.com/openjdk/jfx/pull/1118 already solved some of the problems (but I'm sure it did not solve all of them), and so this PR and https://github.com/openjdk/jfx/pull/1111 may have become less important.

The main problem is still present with #1118, so a solution for that has not become less important.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/445#issuecomment-1618481215