8231644: TreeTableView Regression: Indentation wrong using Label as column content type

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableCellSkin.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -104,8 +104,42 @@ public TreeTableCellSkin(TreeTableCell<S,T> control) {
     }
 
     /** {@inheritDoc} */
-    @Override double leftLabelPadding() {
-        double leftPadding = super.leftLabelPadding();
+    @Override
+    protected double computeMinWidth(double height, double topInset, double rightInset, double bottomInset,
+            double leftInset) {
+        return super.computeMinWidth(height, topInset, rightInset, bottomInset, leftInset) + calculateIndentation();
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    protected void layoutChildren(double x, double y, double w, double h) {
+        double indentation = calculateIndentation();
+        x += indentation;
+        w -= indentation;
+        super.layoutChildren(x, y, w, h);
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    protected double computePrefWidth(double height, double topInset, double rightInset, double bottomInset,
+            double leftInset) {
+        if (isDeferToParentForPrefWidth) {
+            return super.computePrefWidth(height, topInset, rightInset, bottomInset, leftInset) + calculateIndentation();
+        }
+
+        return super.computePrefWidth(height, topInset, rightInset, bottomInset, leftInset);
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset,
+            double leftInset) {
+        width -= calculateIndentation();
+        return super.computePrefHeight(width, topInset, rightInset, bottomInset, leftInset);
+    }
+
+    private double calculateIndentation() {
+        double indentation = 0;
 
         // RT-27167: we must take into account the disclosure node and the
         // indentation (which is not taken into account by the LabeledSkinBase.
@@ -114,43 +148,43 @@ public TreeTableCellSkin(TreeTableCell<S,T> control) {
         TreeTableCell<S,T> cell = getSkinnable();
 
         TreeTableColumn<S,T> tableColumn = cell.getTableColumn();
-        if (tableColumn == null) return leftPadding;
+        if (tableColumn == null) return indentation;
 
         // check if this column is the TreeTableView treeColumn (i.e. the
         // column showing the disclosure node and graphic).
         TreeTableView<S> treeTable = cell.getTreeTableView();
-        if (treeTable == null) return leftPadding;
+        if (treeTable == null) return indentation;
 
         int columnIndex = treeTable.getVisibleLeafIndex(tableColumn);
 
         TreeTableColumn<S,?> treeColumn = treeTable.getTreeColumn();
-        if ((treeColumn == null && columnIndex != 0) || (treeColumn != null && ! tableColumn.equals(treeColumn))) {
-            return leftPadding;
+        if ((treeColumn == null && columnIndex != 0) || (treeColumn != null && !tableColumn.equals(treeColumn))) {
+            return indentation;
         }
 
         TreeTableRow<S> treeTableRow = cell.getTableRow();
-        if (treeTableRow == null) return leftPadding;
+        if (treeTableRow == null) return indentation;
 
         TreeItem<S> treeItem = treeTableRow.getTreeItem();
-        if (treeItem == null) return leftPadding;
+        if (treeItem == null) return indentation;
 
         int nodeLevel = treeTable.getTreeItemLevel(treeItem);
-        if (! treeTable.isShowRoot()) nodeLevel--;
+        if (!treeTable.isShowRoot()) nodeLevel--;
 
         double indentPerLevel = 10;
         if (treeTableRow.getSkin() instanceof TreeTableRowSkin) {
             indentPerLevel = ((TreeTableRowSkin<?>)treeTableRow.getSkin()).getIndentationPerLevel();
         }
-        leftPadding += nodeLevel * indentPerLevel;
+        indentation += nodeLevel * indentPerLevel;
 
         // add in the width of the disclosure node, if one exists
         Map<TableColumnBase<?,?>, Double> mdwp = TableRowSkinBase.maxDisclosureWidthMap;
-        leftPadding += mdwp.containsKey(treeColumn) ? mdwp.get(treeColumn) : 0;
+        indentation += mdwp.containsKey(treeColumn) ? mdwp.get(treeColumn) : 0;
 
         // adding in the width of the graphic on the tree item
         Node graphic = treeItem.getGraphic();
-        leftPadding += graphic == null ? 0 : graphic.prefWidth(height);
+        indentation += graphic == null ? 0 : graphic.prefWidth(height);
 
-        return leftPadding;
+        return indentation;
     }
 }

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableViewIndentationTest.java

@@ -0,0 +1,163 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package test.javafx.scene.control.skin;
+
+import com.sun.javafx.tk.Toolkit;
+import javafx.scene.Node;
+import javafx.scene.control.Label;
+import javafx.scene.control.TreeItem;
+import javafx.scene.control.TreeTableCell;
+import javafx.scene.control.TreeTableColumn;
+import javafx.scene.control.TreeTableRow;
+import javafx.scene.control.TreeTableView;
+import javafx.scene.control.skin.LabeledSkinBase;
+import javafx.scene.control.skin.LabeledSkinBaseShim;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import test.com.sun.javafx.scene.control.infrastructure.StageLoader;
+import test.com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils;
+
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests for the indentation of the {@link javafx.scene.control.TreeTableView}.
+ */
+public class TreeTableViewIndentationTest {
+
+    private TreeTableView<String> treeTableView;
+    private TreeTableColumn<String, String> column;
+    private StageLoader stageLoader;
+
+    @Before
+    public void setup() {
+        treeTableView = new TreeTableView<>();
+
+        column = new TreeTableColumn<>("Column");
+        treeTableView.getColumns().add(column);
+
+        TreeItem<String> root = new TreeItem<>("Root");
+        root.getChildren().addAll(List.of(new TreeItem<>("TreeItem 1"), new TreeItem<>("TreeItem 2")));
+        treeTableView.setRoot(root);
+
+        stageLoader = new StageLoader(treeTableView);
+    }
+
+    @After
+    public void cleanup() {
+        stageLoader.dispose();
+    }
+
+    @Test
+    public void testIndentationOfCell() {
+        column.setCellFactory(col -> new TreeTableCell<>());
+
+        testXCoordinateIsAfterDisclosureNode();
+    }
+
+    @Test
+    public void testIndentationOfCellWithGraphic() {
+        column.setCellFactory(col -> new TreeTableCell<>() {
+
+            @Override
+            protected void updateItem(String item, boolean empty) {
+                super.updateItem(item, empty);
+
+                if (empty) {
+                    setGraphic(null);
+                } else {
+                    setGraphic(new Label("Graphic"));
+                }
+            }
+        });
+
+        testXCoordinateIsAfterDisclosureNode();
+    }
+
+    @Test
+    public void testIndentationOfCellWithText() {
+        column.setCellFactory(col -> new TreeTableCell<>() {
+
+            @Override
+            protected void updateItem(String item, boolean empty) {
+                super.updateItem(item, empty);
+
+                if (empty) {
+                    setText(null);
+                } else {
+                    setText("Text");
+                }
+            }
+        });
+
+        testXCoordinateIsAfterDisclosureNode();
+    }
+
+    @Test
+    public void testIndentationOfCellWithGraphicAndText() {
+        column.setCellFactory(col -> new TreeTableCell<>() {
+
+            @Override
+            protected void updateItem(String item, boolean empty) {
+                super.updateItem(item, empty);
+
+                if (empty) {
+                    setGraphic(null);
+                    setText(null);
+                } else {
+                    setGraphic(new Label("Graphic"));
+                    setText("Text");
+                }
+            }
+        });
+
+        testXCoordinateIsAfterDisclosureNode();
+    }
+
+    private void testXCoordinateIsAfterDisclosureNode() {
+        Toolkit.getToolkit().firePulse();
+
+        TreeTableRow<String> row = (TreeTableRow<String>) VirtualFlowTestUtils.getCell(treeTableView, 0);
+        TreeTableCell<String, String> cell = (TreeTableCell<String, String>) row.getChildrenUnmodifiable().get(1);
+
+        Node graphic = cell.getGraphic();
+        double x;
+        if (graphic != null) {
+            x = graphic.getLayoutX();
+        } else {
+            x = LabeledSkinBaseShim.get_text((LabeledSkinBase<TreeTableCell>) cell.getSkin()).getLayoutX();
+        }
+
+        double leftInset = cell.snappedLeftInset();
+        double disclosureNodeWidth = row.getDisclosureNode().prefWidth(-1);
+        double expectedX = leftInset + disclosureNodeWidth;
+
+        assertEquals(expectedX, x, 0);
+    }
+
+}