Issue
Recently I updated my application from JavaFX 8 to JavaFX 18. After the migration I found some weird issues related to layout of TreeView. If I understand correctly, by the end of a scene pulse, all the nodes (in fact parents) are rendered completely and the isNeedsLayout is turned to false, till the next change occurs.
This is working as expected in JavaFX 8. Whereas in JavaFX 18, for some nodes isNeedsLayout flag is still true even after the pulse is completed. Is this a bug? Or is it deliberately implemented like that?
In the below demo, I tried to print all the nodes (children of TreeView) state after the pulse is completed. And I can clearly see the difference in output between the two JavaFX versions.
Can anyone tell me, how can I ensure that all the nodes are rendered/laid-out correctly.
import javafx.animation.KeyFrame;
import javafx.animation.Timeline;
import javafx.application.Application;
import javafx.geometry.Insets;
import javafx.scene.Node;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.control.TreeCell;
import javafx.scene.control.TreeItem;
import javafx.scene.control.TreeView;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
import javafx.util.Duration;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
public class TreeViewLayoutIssue extends Application {
int k = 1;
@Override
public void start(Stage primaryStage) throws Exception {
final TreeView<String> fxTree = new TreeView<>();
fxTree.setCellFactory(t -> new TreeCell<String>() {
Label lbl = new Label();
@Override
protected void updateItem(String item, boolean empty) {
super.updateItem(item, empty);
setText(null);
if (item != null) {
lbl.setText(item);
setGraphic(lbl);
} else {
setGraphic(null);
}
}
@Override
protected void layoutChildren() {
super.layoutChildren();
if (getItem() != null) {
System.out.println("Layouting ::> " + getItem());
}
}
});
fxTree.setShowRoot(false);
StackPane root = new StackPane(fxTree);
root.setPadding(new Insets(15));
final Scene scene = new Scene(root, 250, 250);
scene.getStylesheets().add(this.getClass().getResource("treeview.css").toExternalForm());
primaryStage.setTitle("TreeView FX18");
primaryStage.setScene(scene);
primaryStage.show();
addData(fxTree);
final Timeline timeline = new Timeline(new KeyFrame(Duration.millis(2000), e -> {
System.out.println("\nIteration #" + k++);
printNeedsLayout(fxTree);
System.out.println("-----------------------------------------------------------------------------");
}));
timeline.setCycleCount(3);
timeline.play();
}
private void printNeedsLayout(final Parent parent) {
System.out.println(" " + parent + " isNeedsLayout: " + parent.isNeedsLayout());
for (final Node n : parent.getChildrenUnmodifiable()) {
if (n instanceof Parent) {
printNeedsLayout((Parent) n);
}
}
}
private void addData(TreeView<String> fxTree) {
final TreeItem<String> rootNode = new TreeItem<>("");
fxTree.setRoot(rootNode);
final TreeItem<String> grp1Node = new TreeItem<>("Group 1");
final TreeItem<String> grp2Node = new TreeItem<>("Group 2");
rootNode.getChildren().addAll(grp1Node, grp2Node);
final TreeItem<String> subNode = new TreeItem<>("Team");
grp1Node.getChildren().addAll(subNode);
final List<TreeItem<String>> groups = Stream.of("Red", "Green", "Yellow", "Blue").map(TreeItem::new).collect(Collectors.toList());
groups.forEach(itm -> subNode.getChildren().add(itm));
grp1Node.setExpanded(true);
grp2Node.setExpanded(true);
subNode.setExpanded(true);
}
}
treeview.css
.virtual-flow .clipped-container .sheet .tree-cell .tree-disclosure-node > .arrow {
-fx-background-color: #77797a;
}
.virtual-flow .clipped-container .sheet .tree-cell .tree-disclosure-node {
-fx-padding: 5px 6px 3px 8px; /* default is 4px 6px 4px 8px */
}
.virtual-flow .clipped-container .sheet .tree-cell:expanded > .tree-disclosure-node {
-fx-padding: 7px 6px 1px 8px; /* default is 4px 6px 4px 8px */
}
.virtual-flow .clipped-container .sheet .tree-cell:selected > .tree-disclosure-node > .arrow {
-fx-background-color: #f7f7f7;
}
And the output is as follows:
When selecting the node for the first time, a slight shift in text is observed in FX18. Whereas there is no issue with FX8.
Output (JavaFX 8): All the node's isNeedsLayout is false.
TreeView@7abc5bb4[styleClass=tree-view] isNeedsLayout: false
VirtualFlow[id=virtual-flow, styleClass=virtual-flow] isNeedsLayout: false
VirtualFlow$ClippedContainer@187e8c80[styleClass=clipped-container] isNeedsLayout: false
Group@57d7468f[styleClass=sheet] isNeedsLayout: false
TreeViewLayoutIssue$1@11a5c9a9[styleClass=cell indexed-cell tree-cell]'Group 1' isNeedsLayout: false
StackPane@7c0429[styleClass=tree-disclosure-node] isNeedsLayout: false
StackPane@731322a7[styleClass=arrow] isNeedsLayout: false
TreeViewLayoutIssue$1@198e401a[styleClass=cell indexed-cell tree-cell]'Team' isNeedsLayout: false
StackPane@e2e9b3[styleClass=tree-disclosure-node] isNeedsLayout: false
StackPane@18615368[styleClass=arrow] isNeedsLayout: false
TreeViewLayoutIssue$1@1632f423[styleClass=cell indexed-cell tree-cell]'Red' isNeedsLayout: false
TreeViewLayoutIssue$1@1f706faa[styleClass=cell indexed-cell tree-cell]'Green' isNeedsLayout: false
TreeViewLayoutIssue$1@2fc143cc[styleClass=cell indexed-cell tree-cell]'Yellow' isNeedsLayout: false
TreeViewLayoutIssue$1@6cd95e88[styleClass=cell indexed-cell tree-cell]'Blue' isNeedsLayout: false
TreeViewLayoutIssue$1@1d2b9016[styleClass=cell indexed-cell tree-cell]'Group 2' isNeedsLayout: false
TreeViewLayoutIssue$1@7ed38c68[styleClass=cell indexed-cell tree-cell]'null' isNeedsLayout: false
TreeViewLayoutIssue$1@4a73e93a[styleClass=cell indexed-cell tree-cell]'null' isNeedsLayout: false
TreeViewLayoutIssue$1@2a65b6dd[styleClass=cell indexed-cell tree-cell]'null' isNeedsLayout: false
Group@2c481877 isNeedsLayout: false
TreeViewLayoutIssue$1@1621d92f[styleClass=cell indexed-cell tree-cell]'null' isNeedsLayout: false
VirtualScrollBar@9a98f02[styleClass=scroll-bar] isNeedsLayout: false
StackPane@343dbe88[styleClass=track-background] isNeedsLayout: false
ScrollBarSkin$2@4727ac31[styleClass=increment-button] isNeedsLayout: false
Region@6d0e770[styleClass=increment-arrow] isNeedsLayout: false
ScrollBarSkin$3@201819d7[styleClass=decrement-button] isNeedsLayout: false
Region@38d26811[styleClass=decrement-arrow] isNeedsLayout: false
StackPane@2e0a5131[styleClass=track] isNeedsLayout: false
ScrollBarSkin$1@771a6386[styleClass=thumb] isNeedsLayout: false
VirtualScrollBar@3177bd8a[styleClass=scroll-bar] isNeedsLayout: false
StackPane@39ae89a6[styleClass=track-background] isNeedsLayout: false
ScrollBarSkin$2@3393f75c[styleClass=increment-button] isNeedsLayout: false
Region@20002045[styleClass=increment-arrow] isNeedsLayout: false
ScrollBarSkin$3@1230eb63[styleClass=decrement-button] isNeedsLayout: false
Region@5dcf6e46[styleClass=decrement-arrow] isNeedsLayout: false
StackPane@342b0a3d[styleClass=track] isNeedsLayout: false
ScrollBarSkin$1@798546a7[styleClass=thumb] isNeedsLayout: false
StackPane@7094a28f[styleClass=corner] isNeedsLayout: false
Output (JavaFX 18): Some node's isNeedsLayout is still true.
TreeView@6d663ddb[styleClass=tree-view] isNeedsLayout: true
VirtualFlow[id=virtual-flow, styleClass=virtual-flow] isNeedsLayout: false
VirtualFlow$ClippedContainer@25755334[styleClass=clipped-container] isNeedsLayout: false
Group@5ec62b29[styleClass=sheet] isNeedsLayout: true
TreeViewLayoutIssue$1@61b62840[styleClass=cell indexed-cell tree-cell]'null' isNeedsLayout: false
TreeViewLayoutIssue$1@1995039d[styleClass=cell indexed-cell tree-cell]'null' isNeedsLayout: false
TreeViewLayoutIssue$1@5401ae6f[styleClass=cell indexed-cell tree-cell]'null' isNeedsLayout: false
TreeViewLayoutIssue$1@2d51271e[styleClass=cell indexed-cell tree-cell]'Group 2' isNeedsLayout: true
TreeViewLayoutIssue$1@500b9275[styleClass=cell indexed-cell tree-cell]'Blue' isNeedsLayout: true
TreeViewLayoutIssue$1@342e3899[styleClass=cell indexed-cell tree-cell]'Yellow' isNeedsLayout: true
TreeViewLayoutIssue$1@68c87749[styleClass=cell indexed-cell tree-cell]'Green' isNeedsLayout: true
TreeViewLayoutIssue$1@18ed5cce[styleClass=cell indexed-cell tree-cell]'Red' isNeedsLayout: true
TreeViewLayoutIssue$1@2f10a091[styleClass=cell indexed-cell tree-cell]'Team' isNeedsLayout: true
StackPane@16cb362[styleClass=tree-disclosure-node] isNeedsLayout: true
StackPane@7b6a2594[styleClass=arrow] isNeedsLayout: false
TreeViewLayoutIssue$1@70c30d4[styleClass=cell indexed-cell tree-cell]'Group 1' isNeedsLayout: true
StackPane@698dbc70[styleClass=tree-disclosure-node] isNeedsLayout: true
StackPane@45cafbcd[styleClass=arrow] isNeedsLayout: false
Group@461a2cd9 isNeedsLayout: false
VirtualScrollBar@13f48448[styleClass=scroll-bar] isNeedsLayout: false
StackPane@1dea4632[styleClass=track-background] isNeedsLayout: false
ScrollBarSkin$2@6acd8e11[styleClass=increment-button] isNeedsLayout: false
Region@5ab4b98c[styleClass=increment-arrow] isNeedsLayout: false
ScrollBarSkin$3@36602364[styleClass=decrement-button] isNeedsLayout: false
Region@178b3dbd[styleClass=decrement-arrow] isNeedsLayout: false
StackPane@d25f45e[styleClass=track] isNeedsLayout: false
ScrollBarSkin$1@4231e81e[styleClass=thumb] isNeedsLayout: false
VirtualScrollBar@6662e7e5[styleClass=scroll-bar] isNeedsLayout: false
StackPane@63f40cf6[styleClass=track-background] isNeedsLayout: false
ScrollBarSkin$2@36c6ae74[styleClass=increment-button] isNeedsLayout: false
Region@261de839[styleClass=increment-arrow] isNeedsLayout: false
ScrollBarSkin$3@7f53c4b5[styleClass=decrement-button] isNeedsLayout: false
Region@6c2c3d1f[styleClass=decrement-arrow] isNeedsLayout: false
StackPane@721aca85[styleClass=track] isNeedsLayout: false
ScrollBarSkin$1@314afb8c[styleClass=thumb] isNeedsLayout: false
StackPane@5183e9a5[styleClass=corner] isNeedsLayout: false
Solution
One observation regarding the shift in text:
If I add the data to treeView after showing the stage, the layout of the cells is in opposite direction to the layout of the cells when adding data before showing the stage.
Case#1: Layout of cells when data is added before showing the stage:
Layouting ::> Group 1
Layouting ::> Team
Layouting ::> Red
Layouting ::> Green
Layouting ::> Yellow
Layouting ::> Blue
Layouting ::> Group 2
Case#2: Layout of cells when data is added after showing the stage:
Layouting ::> Group 2
Layouting ::> Blue
Layouting ::> Yellow
Layouting ::> Green
Layouting ::> Red
Layouting ::> Team
Layouting ::> Group 1
I can see that this is another issue in TreeView. To breifly explain the issue:
TreeCellSkininternally maintains a static map(maxDisclosureWidthMap) to keep track of the widest disclosure node width in aTreeView.- Lets say I customized the disclosure node width (via css) to have width greater than 18px (18px is the hardcoded value in
TreeCellSkin!!). - If the nodes are laid-out from bottom to top (case#2), it uses the default 18px disclosure node width for all the cells till it finds the first arrow (here it is 'Team').
- Once it finds a width greater than the default width, it updates the map. But there is no code to recompute the already computed cells !!.
- When the next layout request comes, now all the cells are computed with the updated disclosure-node width. And that is the reason for the shift in text.
To prove this, if I update my css to have a larger left-padding to the disclosure node (say 50px). In the below gif, you can see that, the cells above "Team" are laid-out correctly. And when I select the treeView(which forces a layout request), then the other cells use the correct disclosure node width and shifts the text.
.virtual-flow .clipped-container .sheet .tree-cell .tree-disclosure-node {
-fx-padding: 5px 6px 3px 50px; /* default is 4px 6px 4px 8px */
}
.virtual-flow .clipped-container .sheet .tree-cell:expanded > .tree-disclosure-node {
-fx-padding: 7px 6px 1px 50px; /* default is 4px 6px 4px 8px */
}
My final solution: [Outdated][see another solution below]
Considering all the above issues (inconsistent isNeedsLayout, text shift.. etc), I came up with the below solution.
The idea is to start an AnimationTimer to keep checking if any of of the child nodes isNeedsLayout is true. If it is true, then it forces to layout by calling the layout() method. Once I add the below code after my treeView initialiation, all the issues are fixed.
// Code to add after initializing TreeView
new AnimationTimer() {
@Override
public void handle(final long now) {
forceLayout(fxTree);
}
}.start();
private void forceLayout(final Parent parent) {
for (final Node n : parent.getChildrenUnmodifiable()) {
if (n instanceof final Parent p) {
forceLayout(p);
}
}
if (parent.isNeedsLayout()) {
parent.layout();
}
}
Now my next biggest fear is: Will it degrade the performance ??
[Update] Alternate Solution:
Though the answer provided by @kleopatra fixes the issue, I can still see a quick layout jump when the window is opened. It is quite obvious behaviour, as we are requesting layout in some unknown time in future(Platform.runLater).
To get rid of this effect, I need to ensure that the layout is corrected with in the same pulse. So I came with a solution to forceLayout (my previous solution) at the end of the pulse whenever there is a change in disclosure node width(@kleopatra's solution)
The new solution is :
- Whenever there is a change in disclosure node width, we register a listener in scene's postLayoutPulseListener to force layout the treeView .
- After the layoutPass in the pulse, this postLayoutPulseListener will force the layout of all children in the treeView.
- Once the force layout is done, we remove this listener to ensure that this will not trigger in next pulses.
Below is the complete working code of the example with the new solution (using the same treeview.css):
import javafx.animation.KeyFrame;
import javafx.animation.Timeline;
import javafx.application.Application;
import javafx.geometry.Insets;
import javafx.scene.Node;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.scene.control.*;
import javafx.scene.control.skin.TreeCellSkin;
import javafx.scene.layout.Region;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
import javafx.util.Duration;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
public class TreeViewLayoutIssue extends Application {
int k = 1;
/**
* Utility class to hack around JDK-8288665: broken layout of
* nested TreeCells.
*/
public class DisclosureNodeHack {
/**
* Key for max disclosure node width
*/
public static final String DISCLOSURE_NODE_WIDTH = "disclosureNodeWidth";
public static class HackedTreeCell<String> extends TreeCell<String> {
Label lbl = new Label();
@Override
protected void updateItem(String item, boolean empty) {
super.updateItem(item, empty);
setText(null);
if (item != null) {
lbl.setText((java.lang.String) item);
setGraphic(lbl);
} else {
setGraphic(null);
}
}
@Override
protected void layoutChildren() {
super.layoutChildren();
if (getItem() != null) {
System.out.println("Laid-out TreeCell ::> " + getItem());
}
}
@Override
protected Skin<?> createDefaultSkin() {
return new DisclosureNodeHack.HackedTreeCellSkin<>(this);
}
public final HackedTreeView<String> getHackedTreeView() {
return (HackedTreeView<String>) getTreeView();
}
}
/**
* Custom skin that puts the width of the disclosure node in the
* Tree's properties.
*/
public static class HackedTreeCellSkin<T> extends TreeCellSkin<T> {
HackedTreeCell<T> cell;
public HackedTreeCellSkin(HackedTreeCell<T> control) {
super(control);
cell = control;
}
@Override
protected void layoutChildren(double x, double y, double w, double h) {
super.layoutChildren(x, y, w, h);
if (getSkinnable().getTreeItem() == null || getSkinnable().getTreeView() == null) return;
Node disclosure = getSkinnable().lookup(".tree-disclosure-node");
if (disclosure instanceof Region) {
double width = ((Region) disclosure).getWidth();
Object prevWidth = getSkinnable().getTreeView().getProperties().get(DISCLOSURE_NODE_WIDTH);
getSkinnable().getTreeView().getProperties().put(DISCLOSURE_NODE_WIDTH, width);
if (prevWidth == null || ((Double) prevWidth).doubleValue() != width) {
cell.getHackedTreeView().installListener();
}
}
}
}
public static class HackedTreeView<T> extends TreeView<T> {
private Runnable listener = new Runnable() {
@Override
public void run() {
System.out.println("------ Forcing Layout ------");
forceLayout(HackedTreeView.this);
getScene().removePostLayoutPulseListener(this);
}
};
public HackedTreeView() {
setCellFactory(t -> new DisclosureNodeHack.HackedTreeCell());
}
private void forceLayout(final Parent parent) {
for (final Node n : parent.getChildrenUnmodifiable()) {
if (n instanceof final Parent p) {
forceLayout(p);
}
}
if (parent.isNeedsLayout()) {
parent.layout();
}
}
public final void installListener() {
getScene().removePostLayoutPulseListener(listener);
getScene().addPostLayoutPulseListener(listener);
}
}
private DisclosureNodeHack() {
}
}
@Override
public void start(Stage primaryStage) throws Exception {
final DisclosureNodeHack.HackedTreeView<String> fxTree = new DisclosureNodeHack.HackedTreeView<>();
fxTree.setShowRoot(false);
StackPane root = new StackPane(fxTree);
root.setPadding(new Insets(15));
final Scene scene = new Scene(root, 250, 250);
scene.getStylesheets().add(this.getClass().getResource("treeview.css").toExternalForm());
primaryStage.setTitle("TreeView FX18");
primaryStage.setScene(scene);
primaryStage.show();
addData(fxTree);
final Timeline timeline = new Timeline(new KeyFrame(Duration.millis(2000), e -> {
System.out.println("\nIteration #" + k++);
printNeedsLayout(fxTree);
System.out.println("-----------------------------------------------------------------------------");
}));
timeline.setCycleCount(1);
timeline.play();
}
private void printNeedsLayout(final Parent parent) {
System.out.println(" " + parent + " isNeedsLayout: " + parent.isNeedsLayout());
for (final Node n : parent.getChildrenUnmodifiable()) {
if (n instanceof Parent) {
printNeedsLayout((Parent) n);
}
}
}
private void addData(TreeView<String> fxTree) {
final TreeItem<String> rootNode = new TreeItem<>("");
fxTree.setRoot(rootNode);
final TreeItem<String> grp1Node = new TreeItem<>("Group 1");
final TreeItem<String> grp2Node = new TreeItem<>("Group 2");
rootNode.getChildren().addAll(grp1Node, grp2Node);
final TreeItem<String> subNode = new TreeItem<>("Team");
grp1Node.getChildren().addAll(subNode);
final List<TreeItem<String>> groups = Stream.of("Red", "Green", "Yellow", "Blue").map(TreeItem::new).collect(Collectors.toList());
groups.forEach(itm -> subNode.getChildren().add(itm));
grp1Node.setExpanded(true);
grp2Node.setExpanded(true);
subNode.setExpanded(true);
}
}
Answered By - Sai Dandem
Answer Checked By - Mary Flores (JavaFixing Volunteer)
