8276671: Iterative layout algorithm

[mstr2]

This PR fixes a long-standing issue with baseline layout computation in JavaFX as mentioned in JDK-8090261 and makes baseline layouts more consistent and dependable. There may be other filed bugs or enhancement requests that relate to the same fundamental issue.

How the baseline offset is computed

Generally, the value returned from Node.getBaselineOffset() is one of the following:

1. the actual baseline offset
2. the special constant BASELINE_OFFSET_SAME_AS_HEIGHT

The reason for this is that there is a circular dependency between the height of a resizable control and its baseline: in order to know the baseline offset, one must know the size of the control; and in order to know the size of the control, one must know the baseline offset. The special constant BASELINE_OFFSET_SAME_AS_HEIGHT basically means: we don't know the baseline offset at this time, but when it comes to layouting the node, we just use whatever the height of the resizable node turns out to be.

However, some resizable controls (Labeled being the most prominent) must report an actual baseline value in order to work meaningfully, but this value can't be computed consistently for the reasons mentioned (this is the cause of JDK-809261).

Step 1: solving Labeled

Since we can't know the actual baseline offset of a resizable Labeled before the control is laid out, we schedule another layout pass when the layoutY property of the Text node was changed during layout. Repeatedly layouting and measuring the resulting baseline offset will quickly converge to the true baseline offset of the Labeled control. This is the code sample taken from JDK-8090261 before and after the change:

Step 2: making baseline layouts more consistent

Even with Labeled solved, the more general issue is that baseline layouts often yield inconsistent results depending on which layout containers are used to compose the scene graph. In order to fix this, we need to ensure that the baseline offset of any given layout container meaningfully reflects the baseline of the nodes placed within it.

This is achieved by adding Node.isTextBaseline(), which returns true if the node or any of its children contain text (and thus should be the primary source when determining a baseline). Currently, Labeled, Text and TextInputControl return true by default.

Additionally, a new boolean property Node.prefBaselineProperty() is introduced. This flag can be used to override the baseline source selection.

The default behavior of Parent.getBaselineOffset() is changed such that the baseline is derived from the first managed child for which

1. Node.isPrefBaseline() returns true, or if there is no such child,
2. Node.isTextBaseline() returns true, or if there is no such child,
3. Parent.getBaselineOffset() returns a value other than BASELINE_OFFSET_SAME_AS_HEIGHT.

The difference can be seen when comparing the old layout behavior to the new layout behavior:

---

Progress

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

Integration blocker

 ⚠️ The change requires a CSR request to be approved.

Issue

• JDK-8276671: Iterative layout algorithm

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 433

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

Using diff file

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

[bridgekeeper[bot]]

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

[kevinrushforth]

Since this enhancement represents a behavioral change, and adds new API, it will need to be discussed on the openjfx-dev mailing list prior to review. A Draft PR can be helpful to illustrate what you are proposing, but the focus should be first on the problem, and then on the API changes and behavior changes you propose to address that problem.

If as a result of that discussion, there is general agreement about the problem and the approach, then next step would be to focus on the API and specification changes, which will need a CSR.

/reviewers 3 /csr

[openjdk[bot]]

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

[openjdk[bot]]

@kevinrushforth has indicated that a compatibility and specification (CSR) request is needed for this pull request. @mstr2 this pull request must refer to an issue in JBS to be able to link it to a CSR request. To refer this pull request to an issue in JBS, please use the /issue command in a comment in this pull request.

[openjdk[bot]]

@mstr2 this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout feature/baselinelayout
git fetch https://git.openjdk.java.net/jfx master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[mstr2]

I've created a small playground app that shows a few scenarios in which a baseline-aligned layout now works as expected.

Here's a screenshot of the current behavior on the left side, and the new behavior on the right side.

[bridgekeeper[bot]]

@mstr2 This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper[bot]]

@mstr2 This pull request has been inactive for more than 16 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.