JavaFX Layout issue in TreeView

Question

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

Answer 1

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:

• TreeCellSkin internally maintains a static map(maxDisclosureWidthMap) to keep track of the widest disclosure node width in a TreeView.
• 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);
    }
}

Answer 2

As already noted in my comment: it's a bug. The underlying reason seems to be sub-optimal cell layout, in particular the layout of the disclosure node (see Sai's self-answer for details). This bug bubbled up after some optimization of layout in VirtualFlow - which still is correct, IMO, but exposed the mis-behavior of cell layout.

An alternative to constantly checking the layout tree in an AnimationTimer is to implement a collaboration between cell and tree:

• a custom cell skin sets the disclosure node width in the tree's properties
• the tree listens to the property change and forces a layout of the cell's parent

Below is a utility class providing support for both. It relies on the style tree which is not fully specified for TreeView (but is similar to ListView which is specified) and Node lookup, otherwise uses only public api. Choosing the one or other hack is a matter of personal preferences.

To use the utility:

// in application code
DisclosureNodeHack.installListener(tree);

// set the custom treeCell skin as default via css
.tree-cell {
    -fx-skin: "mypackage.DisclosureNodeHack$HackedTreeCellSkin"; 
}
// for visualization when debugging
.tree-cell > .tree-disclosure-node {
    -fx-padding: 4 6 4 50;
    -fx-background-color: yellow;
}

The utility class (beware: not formally tested, and most probably with much leeway to improve):

/**
 * 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";

    /**
     * Custom skin that puts the width of the disclosure node in the
     * Tree's properties.
     */
    public static class HackedTreeCellSkin<T> extends TreeCellSkin<T> {

        public HackedTreeCellSkin(TreeCell<T> control) {
            super(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();
                getSkinnable().getTreeView().getProperties().put(DISCLOSURE_NODE_WIDTH, width);
            }
        }
    }

    /**
     * Utility method to register a listener to the tree's properties and
     * forces a re-layout of the flow's sheet if the disclosure width changes.
     *
     * Note: experiments seem to indicate that layout must be done
     * - after the current layout run is ready, that is in Platform.runlater
     * - on the parent of the cell (== sheet), not on the tree
     */
    public static <T> void installListener(TreeView<T> tree) {
        tree.getProperties().addListener((MapChangeListener<Object,Object>)c -> {
            if (DISCLOSURE_NODE_WIDTH.equals(c.getKey())) {
                Platform.runLater(() -> {
                    Node sheet = tree.lookup(".sheet");
                    if (sheet instanceof Parent)
                        ((Parent) sheet).requestLayout();
                });
            }
        });

    };

    private DisclosureNodeHack() {};
}