Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

@mstr2
Copy link
Collaborator

@mstr2 mstr2 commented Mar 29, 2021

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

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


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

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

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/445/head:pull/445
$ git checkout pull/445

Update a local copy of the PR:
$ git checkout pull/445
$ git pull https://git.openjdk.java.net/jfx pull/445/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 445

View PR using the GUI difftool:
$ git pr show -t 445

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/445.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Mar 29, 2021

👋 Welcome back mstr2! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@mstr2 mstr2 force-pushed the fixes/box-snap-to-pixel branch from 6cb8fef to c218b80 Mar 29, 2021
@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Mar 31, 2021

/reviewers 2

@openjdk
Copy link

@openjdk openjdk bot commented Mar 31, 2021

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Apr 1, 2021

The bug is posted here:

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

@mstr2 mstr2 changed the title Fix pixel-snapping glitches in VBox/HBox 8264591: Fix pixel-snapping glitches in VBox/HBox Apr 1, 2021
@mstr2 mstr2 changed the title 8264591: Fix pixel-snapping glitches in VBox/HBox 8264591: HBox/VBox child widths pixel-snap to wrong value Apr 1, 2021
@openjdk openjdk bot added the rfr label Apr 1, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 1, 2021

Webrevs

@Maran23
Copy link
Member

@Maran23 Maran23 commented Sep 22, 2021

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. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants