8293119: Additional constrained resize policies for Tree/TableView

[andy-goryachev-oracle]

The current CONSTRAINED_RESIZE_POLICY has a number of issues as explained in JDK-8292810.

We propose to address all these issues by replacing the old column resize algorithm with a different one, which not only honors all the constraints when resizing, but also provides 4 different resize modes similar to JTable's. The new implementation brings changes to the public API for Tree/TableView and ResizeFeaturesBase classes. Specifically:

• create a public abstract javafx.scene.control.ConstrainedColumnResizeBase class
• provide an out-of-the box implementation via javafx.scene.control.ConstrainedColumnResize class, offeting 4 resize modes: AUTO_RESIZE_NEXT_COLUMN, AUTO_RESIZE_SUBSEQUENT_COLUMNS, AUTO_RESIZE_LAST_COLUMN, AUTO_RESIZE_ALL_COLUMNS
• add corresponding public static constants to Tree/TableView
• make Tree/TableView.CONSTRAINED_RESIZE_POLICY an alias to AUTO_RESIZE_SUBSEQUENT_COLUMNS (a slight behavioral change - discuss)
• add getContentWidth() and setColumnWidth(TableColumnBase<S,?> col, double width) methods to ResizeFeatureBase
• suppress the horizontal scroll bar when resize policy is instanceof ConstrainedColumnResizeBase
• update javadoc

Notes

1. The current resize policies' toString() methods return "unconstrained-resize" and "constrained-resize", used by the skin base to set a pseudostate. All constrained policies that extend ConstrainedColumnResizeBase will return "constrained-resize" value.
2. The reason an abstract class ( ConstrainedColumnResizeBase) was chosen instead of a marker interface is exactly for its toString() method which supplies "constrained-resize" value. The implementors might choose to use a different value, however they must ensure the stylesheet contains the same adjustments for the new policy as those made in modena.css for "constrained-resize" value.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires a CSR request to be approved
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

• JDK-8293119: Additional constrained resize policies for Tree/TableView
• JDK-8294398: Additional constrained resize policies for Tree/TableView (CSR)

Reviewers

• Ajit Ghaisas (@aghaisas - Reviewer)
• Kevin Rushforth (@kevinrushforth - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx pull/897/head:pull/897
$ git checkout pull/897

Update a local copy of the PR:
$ git checkout pull/897
$ git pull https://git.openjdk.org/jfx pull/897/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 897

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/897.diff

[bridgekeeper]

👋 Welcome back angorya! 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.

[openjdk]

@andy-goryachev-oracle 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 8293119.constrained
git fetch https://git.openjdk.org/jfx master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[kevinrushforth]

OK, that makes complete sense to me, and is, I agree, a better choice.

We might consider a follow-up doc bug to clarify this, but it isn't critical.

I also don't think the remaining HiDPI issue is critical, so that also could be a follow-up. I'll continue testing it, and also review the code.

[karthikpandelu]

I'm also seeing the mouse cursor tracking issue mentioned by Kevin while dragging the column divider.

I wanted to check about following behavior.
When AUTO_RESIZE_FLEX_LAST_COLUMN policy and pref only data are selected, column divider between C1 and C2 is dragged to the right. C4 size will be reduced first and reaches minimum. Then C3 and C2 column size reduces.
After all the 3 columns reach minimum width, if same divider is dragged to the left , C4 gets expanded first, then C3 and C2.
Where as in AUTO_RESIZE_ALL_COLUMNS policy, the column whose width was reduced last gets expanded first. Should we keep the behavior consistent across all policies?
The behavior in CONSTRAINED_RESIZE_POLICY is same as FLEX policies.

I ran these test in Windows 11 system. I will try to do some more testing on the same machine.

[andy-goryachev-oracle]

Where as in AUTO_RESIZE_ALL_COLUMNS policy, the column whose width was reduced last gets expanded first. Should we keep the behavior consistent across all policies?

thank you for testing, @karthikpandelu !

not really, the idea of providing several different policies with different behavior is that the app developer can choose the one that fits the requirements best.

AUTO_RESIZE_ALL_COLUMNS tries to allocate extra space proportionally to the preferred width of each column, so the behavior you are describing is expected.

[karthikpandelu]

AUTO_RESIZE_ALL_COLUMNS tries to allocate extra space proportionally to the preferred width of each column, so the behavior you are describing is expected.

Understood. Thanks for the clarification.

[andy-goryachev-oracle]

I realized that, in presence of snapping and fractional scale, the implemented algorithm would never work correctly. The reason is that the distance between snapped positions is not the same. As a result, changing a single column width requires shifting of subsequent or all the columns, which in turn might require a change in their widths, and so on.

This makes me think that a different algorithm might be needed, a simulated annealing perhaps.

[kevinrushforth]

I realized that, in presence of snapping and fractional scale, the implemented algorithm would never work correctly. The reason is that the distance between snapped positions is not the same.

Can you file a follow-on bug so we don't lose track of it? It doesn't need to be fixed for JavaFX 20, but it would be good to fix it when we can.

[andy-goryachev-oracle]

Logged two follow-up tickets:

JDK-8299753 Tree/TableView: Column Resizing With Fractional Scale

JDK-8299755 Tree/TableView: Cursor Decouples From Divider When Resizing With Fractional Scale

[kevinrushforth]

I did some additional testing, and other than the minor issues already identified with fractional screen scales (and tracked by follow-up bugs), everything looks good.

I left a few mostly minor comments inline. The only thing I'd definitely like to see is at least some testing of TreeTableView.

[kevinrushforth]

Looks good with one minor formatting comment inline. I'll reapprove if you fix it.

[andy-goryachev-oracle]

@aghaisas could you take a final look at this please?

[aghaisas]

I have tested on macOS. This looks good!

[openjdk]

@andy-goryachev-oracle This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8293119: Additional constrained resize policies for Tree/TableView

Reviewed-by: aghaisas, kcr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the master branch:

• 357cd85: 8282386: JavaFX media stubs rely on libav.org
• bca1bfc: 8243115: Spurious invalidations due to bug in IntegerBinding and other classes

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[andy-goryachev-oracle]

/integrate

[openjdk]

Going to push as commit 1ac97fc.
Since your change was applied there have been 3 commits pushed to the master branch:

• 4a27801: 8217853: Cleanup in the D3D native pipeline
• 357cd85: 8282386: JavaFX media stubs rely on libav.org
• bca1bfc: 8243115: Spurious invalidations due to bug in IntegerBinding and other classes

Your commit was automatically rebased without conflicts.

[openjdk]

@andy-goryachev-oracle Pushed as commit 1ac97fc.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.