Re: RFR: 8264591: HBox/VBox child widths pixel-snap to wrong value

2021-11-10 Thread Michael Strauß
On Wed, 22 Sep 2021 18:48:13 GMT, Marius Hanl  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.
>> ![pixel-glitch](https://user-images.githubusercontent.com/43553916/112891996-146e2d80-90d9-11eb-9d83-97754ae38dc1.png)
>> 
>> I've filed a bug report and will change the title of the GitHub issue as 
>> soon as it's posted.
>
> I had a look at the PR. But it took quite a while to fully understand the 
> changes (also the current implementation 😄). 
> I think it make sense to improve it a bit e.g. by adding javadoc to the new 
> methods, maybe also the existing? Other ideas are also welcome. 
> With that said maybe more people will review it then. I will have a full look 
> soon as well. :)

@Maran23 Would you be interested in reviewing the implementation and the added 
documentation?

-

PR: https://git.openjdk.java.net/jfx/pull/445


Re: RFR: 8264591: HBox/VBox child widths pixel-snap to wrong value

2021-11-02 Thread Michael Strauß
On Wed, 22 Sep 2021 18:48:13 GMT, Marius Hanl  wrote:

> I had a look at the PR. But it took quite a while to fully understand the 
> changes (also the current implementation 😄). I think it make sense to improve 
> it a bit e.g. by adding javadoc to the new methods, maybe also the existing? 
> Other ideas are also welcome. With that said maybe more people will review it 
> then. I will have a full look soon as well. :)

I've added a bit of documentation.

-

PR: https://git.openjdk.java.net/jfx/pull/445


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

2021-11-02 Thread Michael Strauß
> 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.
> ![pixel-glitch](https://user-images.githubusercontent.com/43553916/112891996-146e2d80-90d9-11eb-9d83-97754ae38dc1.png)
> 
> 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 incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains four additional 
commits since the last revision:

 - added documentation, improved method names
 - Merge branch 'master' into fixes/box-snap-to-pixel
 - Fix pixel-snapping glitches in VBox/HBox
 - Failing test

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/445/files
  - new: https://git.openjdk.java.net/jfx/pull/445/files/c218b809..744f19f5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=445&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=445&range=00-01

  Stats: 449813 lines in 8109 files changed: 251439 ins; 126257 del; 72117 mod
  Patch: https://git.openjdk.java.net/jfx/pull/445.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/445/head:pull/445

PR: https://git.openjdk.java.net/jfx/pull/445


Re: RFR: 8264591: HBox/VBox child widths pixel-snap to wrong value

2021-09-22 Thread Marius Hanl
On Mon, 29 Mar 2021 19:57:17 GMT, Michael Strauß  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.
> ![pixel-glitch](https://user-images.githubusercontent.com/43553916/112891996-146e2d80-90d9-11eb-9d83-97754ae38dc1.png)
> 
> I've filed a bug report and will change the title of the GitHub issue as soon 
> as it's posted.

I had a look at the PR. But it took quite a while to fully understand the 
changes (also the current implementation 😄). 
I think it make sense to improve it a bit e.g. by adding javadoc to the new 
methods, maybe also the existing? Other ideas are also welcome. 
With that said maybe more people will review it then. I will have a full look 
soon as well. :)

-

PR: https://git.openjdk.java.net/jfx/pull/445


Re: RFR: 8264591: HBox/VBox child widths pixel-snap to wrong value

2021-04-01 Thread Kevin Rushforth
On Mon, 29 Mar 2021 19:57:17 GMT, mstr2  
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.
> ![pixel-glitch](https://user-images.githubusercontent.com/43553916/112891996-146e2d80-90d9-11eb-9d83-97754ae38dc1.png)
> 
> I've filed a bug report and will change the title of the GitHub issue as soon 
> as it's posted.

The bug is posted here:

[JDK-8264591](https://bugs.openjdk.java.net/browse/JDK-8264591): HBox/VBox 
child widths pixel-snap to wrong value

-

PR: https://git.openjdk.java.net/jfx/pull/445


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

2021-04-01 Thread mstr2
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.
![pixel-glitch](https://user-images.githubusercontent.com/43553916/112891996-146e2d80-90d9-11eb-9d83-97754ae38dc1.png)

I've filed a bug report and will change the title of the GitHub issue as soon 
as it's posted.

-

Commit messages:
 - Fix pixel-snapping glitches in VBox/HBox
 - Failing test

Changes: https://git.openjdk.java.net/jfx/pull/445/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=445&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264591
  Stats: 306 lines in 5 files changed: 215 ins; 19 del; 72 mod
  Patch: https://git.openjdk.java.net/jfx/pull/445.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/445/head:pull/445

PR: https://git.openjdk.java.net/jfx/pull/445