diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java index 03dcce9e46e..a55ec20ff83 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java @@ -32,7 +32,6 @@ import com.sun.javafx.scene.control.behavior.BehaviorBase; import com.sun.javafx.scene.control.behavior.TableRowBehavior; -import javafx.beans.property.DoubleProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.scene.AccessibleAttribute; @@ -93,34 +92,41 @@ public TableRowSkin(TableRow control) { } }); - setupTreeTableViewListeners(); + setupTableViewListeners(); } // FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000 - private void setupTreeTableViewListeners() { + private void setupTableViewListeners() { TableView tableView = getSkinnable().getTableView(); if (tableView == null) { registerInvalidationListener(getSkinnable().tableViewProperty(), e -> { unregisterInvalidationListeners(getSkinnable().tableViewProperty()); - setupTreeTableViewListeners(); + setupTableViewListeners(); }); } else { - DoubleProperty fixedCellSizeProperty = tableView.fixedCellSizeProperty(); - if (fixedCellSizeProperty != null) { - registerChangeListener(fixedCellSizeProperty, e -> { - fixedCellSize = fixedCellSizeProperty.get(); - fixedCellSizeEnabled = fixedCellSize > 0; - }); - fixedCellSize = fixedCellSizeProperty.get(); - fixedCellSizeEnabled = fixedCellSize > 0; - - // JDK-8144500: - // When in fixed cell size mode, we must listen to the width of the virtual flow, so - // that when it changes, we can appropriately add / remove cells that may or may not - // be required (because we remove all cells that are not visible). + registerChangeListener(tableView.fixedCellSizeProperty(), e -> { VirtualFlow> virtualFlow = getVirtualFlow(); if (virtualFlow != null) { - registerChangeListener(virtualFlow.widthProperty(), e -> tableView.requestLayout()); + unregisterChangeListeners(virtualFlow.widthProperty()); + } + + updateCachedFixedSize(); + }); + + updateCachedFixedSize(); + } + } + + private void updateCachedFixedSize() { + TableView tableView = getSkinnable().getTableView(); + if (tableView != null) { + fixedCellSize = tableView.getFixedCellSize(); + fixedCellSizeEnabled = fixedCellSize > 0; + + if (fixedCellSizeEnabled) { + VirtualFlow> virtualFlow = getVirtualFlow(); + if (virtualFlow != null) { + registerChangeListener(virtualFlow.widthProperty(), e -> getSkinnable().requestLayout()); } } } diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java index b2474e72e60..355c37bf6b5 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java @@ -36,7 +36,6 @@ import javafx.collections.ObservableList; import javafx.css.StyleOrigin; import javafx.css.StyleableObjectProperty; -import javafx.geometry.Insets; import javafx.geometry.Pos; import javafx.scene.Node; import javafx.scene.Parent; @@ -95,11 +94,6 @@ public abstract class TableRowSkinBase, Double> maxDisclosureWidthMap = new WeakHashMap<>(); - // Specifies the number of times we will call 'recreateCells()' before we blow - // out the cellsMap structure and rebuild all cells. This helps to prevent - // against memory leaks in certain extreme circumstances. - private static final int DEFAULT_FULL_REFRESH_COUNTER = 100; - /* ************************************************************************* * * @@ -113,21 +107,13 @@ public abstract class TableRowSkinBase> cellsMap; // This observableArrayList contains the currently visible table cells for this row. final List cells = new ArrayList<>(); - private int fullRefreshCounter = DEFAULT_FULL_REFRESH_COUNTER; - boolean isDirty = false; - boolean updateCells = false; // FIXME: replace cached values with direct lookup - JDK-8277000 double fixedCellSize; @@ -152,7 +138,7 @@ public TableRowSkinBase(C control) { getSkinnable().setPickOnBounds(false); recreateCells(); - updateCells(true); + updateCells(); // init bindings // watches for any change in the leaf columns observableArrayList - this will indicate @@ -176,7 +162,7 @@ public TableRowSkinBase(C control) { * * **************************************************************************/ - private void updateLeafColumns() { + void updateLeafColumns() { isDirty = true; getSkinnable().requestLayout(); } @@ -289,7 +275,6 @@ protected ObjectProperty graphicProperty() { // earlier rows to update themselves to take into account // this increased indentation. final VirtualFlow flow = getVirtualFlow(); - final int thisIndex = getSkinnable().getIndex(); for (int i = 0; i < flow.cells.size(); i++) { C cell = flow.cells.get(i); if (cell == null || cell.isEmpty()) continue; @@ -318,10 +303,13 @@ protected ObjectProperty graphicProperty() { int index = control.getIndex(); if (index < 0/* || row >= itemsProperty().get().size()*/) return; + VirtualFlow virtualFlow = getVirtualFlow(); for (int column = 0, max = cells.size(); column < max; column++) { R tableCell = cells.get(column); TableColumnBase tableColumn = getTableColumn(tableCell); + width = snapSizeX(tableColumn.getWidth()); + boolean isVisible = true; if (fixedCellSizeEnabled) { // we determine if the cell is visible, and if not we have the @@ -333,7 +321,7 @@ protected ObjectProperty graphicProperty() { // provided by the developer, and this means that we do not have // to concern ourselves with the possibility that the height // may be variable and / or dynamic. - isVisible = isColumnPartiallyOrFullyVisible(tableColumn); + isVisible = isColumnPartiallyOrFullyVisible(x, width, virtualFlow); y = 0; height = fixedCellSize; @@ -342,12 +330,9 @@ protected ObjectProperty graphicProperty() { } if (isVisible) { - if (fixedCellSizeEnabled && tableCell.getParent() == null) { + if (tableCell.getParent() == null) { getChildren().add(tableCell); } - // Note: prefWidth() has to be called only after the tableCell is added to the tableRow, if it wasn't - // already. Otherwise, it might not have its skin yet, and its pref width is therefore 0. - width = tableCell.prefWidth(height); // Added for JDK-8115536, and then updated for JDK-8122708. // We change the alignment from CENTER_LEFT to TOP_LEFT if the @@ -419,11 +404,7 @@ protected ObjectProperty graphicProperty() { // This does not appear to impact performance... tableCell.requestLayout(); } else { - width = tableCell.prefWidth(height); - if (fixedCellSizeEnabled) { - // we only add/remove to the scenegraph if the fixed cell - // length support is enabled - otherwise we keep all - // TableCells in the scenegraph + if (tableCell.getParent() != null) { getChildren().remove(tableCell); } } @@ -473,21 +454,9 @@ boolean isShowRoot() { return true; } - void updateCells(boolean resetChildren) { - // To avoid a potential memory leak (when the TableColumns in the - // TableView are created/inserted/removed/deleted, we have a 'refresh - // counter' that when we reach 0 will delete all cells in this row - // and recreate all of them. - if (resetChildren) { - if (fullRefreshCounter == 0) { - recreateCells(); - } - fullRefreshCounter--; - } - + void updateCells() { // if clear isn't called first, we can run into situations where the // cells aren't updated properly. - final boolean cellsEmpty = cells.isEmpty(); cells.clear(); final C skinnable = getSkinnable(); @@ -518,23 +487,7 @@ void updateCells(boolean resetChildren) { cells.add(cell); } - // update children of each row - if (fixedCellSizeEnabled) { - // we leave the adding / removing up to the layoutChildren method mostly, but here we remove any children - // cells that refer to columns that are removed or not visible. - List toRemove = new ArrayList<>(); - for (Node cell : getChildren()) { - if (!(cell instanceof IndexedCell)) continue; - TableColumnBase tableColumn = getTableColumn((R) cell); - if (!getVisibleLeafColumns().contains(tableColumn)) { - toRemove.add(cell); - } - } - getChildren().removeAll(toRemove); - } - if (resetChildren || cellsEmpty) { - getChildren().setAll(cells); - } + getChildren().setAll(cells); } VirtualFlow getVirtualFlow() { @@ -630,12 +583,8 @@ VirtualFlow getVirtualFlow() { final void checkState() { if (isDirty) { - updateCells(true); + updateCells(); isDirty = false; - updateCells = false; - } else if (updateCells) { - updateCells(false); - updateCells = false; } } @@ -655,33 +604,18 @@ void setDirty(boolean dirty) { * * **************************************************************************/ - private boolean isColumnPartiallyOrFullyVisible(TableColumnBase col) { - if (col == null || !col.isVisible()) return false; + private boolean isColumnPartiallyOrFullyVisible(double start, double width, VirtualFlow virtualFlow) { + double end = start + width; - final VirtualFlow virtualFlow = getVirtualFlow(); double scrollX = virtualFlow == null ? 0.0 : virtualFlow.getHbar().getValue(); + double headerWidth = virtualFlow == null ? 0.0 : virtualFlow.getViewportBreadth(); + double virtualFlowWidth = headerWidth + scrollX; - // work out where this column header is, and it's width (start -> end) - double start = 0; - final ObservableList visibleLeafColumns = getVisibleLeafColumns(); - for (int i = 0, max = visibleLeafColumns.size(); i < max; i++) { - TableColumnBase c = visibleLeafColumns.get(i); - if (c.equals(col)) break; - start += c.getWidth(); - } - double end = start + col.getWidth(); - - // determine the width of the table - final Insets padding = getSkinnable().getPadding(); - double headerWidth = getSkinnable().getWidth() - padding.getLeft() + padding.getRight(); - - return (start >= scrollX || end > scrollX) && (start < (headerWidth + scrollX) || end <= (headerWidth + scrollX)); + return (start >= scrollX || end > scrollX) && (start < virtualFlowWidth || end <= virtualFlowWidth); } private void requestCellUpdate() { - updateCells = true; getSkinnable().requestLayout(); - // update the index of all children cells (JDK-8119094). // Note that we do this after the TableRow item has been updated, // rather than when the TableRow index has changed (as this will be @@ -713,7 +647,6 @@ private void recreateCells() { ObservableList*/> columns = getVisibleLeafColumns(); cellsMap = new WeakHashMap<>(columns.size()); - fullRefreshCounter = DEFAULT_FULL_REFRESH_COUNTER; getChildren().clear(); for (TableColumnBase col : columns) { diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java index 5c1a0aa2c96..aef5a7f4000 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java @@ -72,7 +72,6 @@ public class TreeTableRowSkin extends TableRowSkinBase, TreeTable private boolean disclosureNodeDirty = true; private Node graphic; private final BehaviorBase> behavior; - private boolean childrenDirty = false; /* ************************************************************************* * * @@ -97,10 +96,6 @@ public TreeTableRowSkin(TreeTableRow control) { ListenerHelper lh = ListenerHelper.get(this); - lh.addChangeListener(control.indexProperty(), (ev) -> { - updateCells = true; - }); - lh.addChangeListener(control.treeItemProperty(), (ev) -> { updateTreeItem(); // There used to be an isDirty = true statement here, but this was @@ -121,36 +116,18 @@ private void setupTreeTableViewListeners() { }); } else { registerChangeListener(treeTableView.treeColumnProperty(), (x) -> { - // Fix for JDK-8124861: Need to set isDirty to true, rather than the - // cheaper updateCells, as otherwise the text indentation will not - // be recalculated in TreeTableCellSkin.calculateIndentation() - isDirty = true; - if (getSkinnable() != null) { - getSkinnable().requestLayout(); - } + updateLeafColumns(); }); - DoubleProperty fixedCellSizeProperty = getTreeTableView().fixedCellSizeProperty(); - if (fixedCellSizeProperty != null) { - registerChangeListener(fixedCellSizeProperty, (x) -> { - updateCachedFixedSize(); - }); - updateCachedFixedSize(); - - // JDK-8144500: - // When in fixed cell size mode, we must listen to the width of the virtual flow, so - // that when it changes, we can appropriately add / remove cells that may or may not - // be required (because we remove all cells that are not visible). + registerChangeListener(getTreeTableView().fixedCellSizeProperty(), e -> { VirtualFlow> virtualFlow = getVirtualFlow(); if (virtualFlow != null) { - registerChangeListener(getVirtualFlow().widthProperty(), (x) -> { - if (getSkinnable() != null) { - TreeTableView t = getSkinnable().getTreeTableView(); - t.requestLayout(); - } - }); + unregisterChangeListeners(virtualFlow.widthProperty()); } - } + + updateCachedFixedSize(); + }); + updateCachedFixedSize(); } } @@ -160,6 +137,13 @@ private void updateCachedFixedSize() { if (t != null) { fixedCellSize = t.getFixedCellSize(); fixedCellSizeEnabled = fixedCellSize > 0.0; + + if (fixedCellSizeEnabled) { + VirtualFlow> virtualFlow = getTableViewSkin().getVirtualFlow(); + if (virtualFlow != null) { + registerChangeListener(virtualFlow.widthProperty(), ev -> getSkinnable().requestLayout()); + } + } } } } @@ -229,29 +213,16 @@ public final DoubleProperty indentProperty() { super.updateChildren(); updateDisclosureNodeAndGraphic(); - - if (childrenDirty) { - childrenDirty = false; - if (cells.isEmpty()) { - getChildren().clear(); - } else { - // TODO we can optimise this by only showing cells that are - // visible based on the table width and the amount of horizontal - // scrolling. - getChildren().addAll(cells); - } - } } /** {@inheritDoc} */ @Override protected void layoutChildren(double x, double y, double w, double h) { - if (disclosureNodeDirty) { - updateDisclosureNodeAndGraphic(); - disclosureNodeDirty = false; + Node disclosureNode = getDisclosureNode(); + if (disclosureNode != null && disclosureNode.getParent() == null) { + disclosureNodeDirty = true; } - Node disclosureNode = getDisclosureNode(); - if (disclosureNode != null && disclosureNode.getScene() == null) { + if (disclosureNodeDirty) { updateDisclosureNodeAndGraphic(); } @@ -277,13 +248,10 @@ public final DoubleProperty indentProperty() { } /** {@inheritDoc} */ - @Override void updateCells(boolean resetChildren) { - super.updateCells(resetChildren); + @Override void updateCells() { + super.updateCells(); - if (resetChildren) { - childrenDirty = true; - updateChildren(); - } + updateDisclosureNodeAndGraphic(); } /** {@inheritDoc} */ @@ -352,6 +320,8 @@ private TreeTableView getTreeTableView() { } private void updateDisclosureNodeAndGraphic() { + disclosureNodeDirty = false; + if (getSkinnable().isEmpty()) { getChildren().remove(graphic); return; @@ -375,14 +345,13 @@ private void updateDisclosureNodeAndGraphic() { // check disclosure node Node disclosureNode = getSkinnable().getDisclosureNode(); if (disclosureNode != null) { - boolean disclosureVisible = treeItem != null && ! treeItem.isLeaf(); + boolean disclosureVisible = isDisclosureNodeVisible(); disclosureNode.setVisible(disclosureVisible); - if (! disclosureVisible) { + if (!disclosureVisible) { getChildren().remove(disclosureNode); } else if (disclosureNode.getParent() == null) { getChildren().add(disclosureNode); - disclosureNode.toFront(); } else { disclosureNode.toBack(); } diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java index e620de33034..3b590668900 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java @@ -1081,7 +1081,7 @@ void adjustPosition() { lastWidth = -1; lastHeight = -1; releaseCell(accumCell); - sheet.getChildren().clear(); + sheetChildren.clear(); resetIndex(cells); resetIndex(pile); @@ -1138,6 +1138,13 @@ void adjustPosition() { cell.requestLayout(); } } + + // Also request layout for cells in the pile. As soon as those are reused (and therefore added), + // they will do their layout. + for (T cell : pile) { + cell.requestLayout(); + } + needsCellsLayout = false; // yes, we return here - if needsCellsLayout was set to true, we // only did it to do the above - not rerun the entire layout. @@ -1916,7 +1923,7 @@ final double getMaxPrefBreadth() { private final void setViewportBreadth(double value) { this.viewportBreadth = value; } - private final double getViewportBreadth() { + final double getViewportBreadth() { return viewportBreadth; } @@ -2843,6 +2850,11 @@ private void cleanPile() { cell.setVisible(false); } + // Remove all cells that are in the pile and therefore not relevant anymore. + if (sheetChildren.size() != cells.size()) { + sheetChildren.removeAll(pile); + } + // Fix for JDK-8095710: Rather than have the cells do weird things with // focus (in particular, have focus jump between cells), we return focus // to the VirtualFlow itself. diff --git a/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java b/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java index a00bedd4cb9..e70c8d077a9 100644 --- a/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java +++ b/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java @@ -209,11 +209,6 @@ public static void assertTableCellTextEquals(final Control control, final int ro int _column = column; - // we need to account for TreeTableView having LabeledText node in the TreeTableRow - if (indexedCell instanceof TreeTableRow) { - _column++; - } - IndexedCell cell = (IndexedCell) indexedCell.getChildrenUnmodifiable().get(_column); assertEquals(expected, cell.getText()); return null; @@ -336,14 +331,10 @@ public static void assertTableHeaderColumnExists(final Control control, final Ta } } - public static boolean BLOCK_STAGE_LOADER_DISPOSE = false; - public static VirtualFlow getVirtualFlow(Control control) { StageLoader sl = null; - boolean stageLoaderCreated = false; if (control.getScene() == null) { sl = new StageLoader(control); - stageLoaderCreated = true; } VirtualFlow flow; @@ -355,7 +346,7 @@ public static VirtualFlow getVirtualFlow(Control control) { flow = (VirtualFlow)control.lookup("#virtual-flow"); - if (stageLoaderCreated && sl != null && ! BLOCK_STAGE_LOADER_DISPOSE) { + if (sl != null) { sl.dispose(); } diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java index 6725c395869..5ec33c4e368 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java @@ -2196,9 +2196,8 @@ private void test_rt_34685(boolean useMouseToInitiateEdit, boolean cellSelection table.getColumns().addAll(first); // get the cell at (0,0) - VirtualFlowTestUtils.BLOCK_STAGE_LOADER_DISPOSE = true; + stageLoader = new StageLoader(table); TableCell cell = (TableCell) VirtualFlowTestUtils.getCell(table, 0, 0); - VirtualFlowTestUtils.BLOCK_STAGE_LOADER_DISPOSE = false; assertTrue(cell.getSkin() instanceof TableCellSkin); assertNull(cell.getGraphic()); assertEquals("John", cell.getText()); diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewVirtualizationTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewVirtualizationTest.java new file mode 100644 index 00000000000..7eda6eb7af9 --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewVirtualizationTest.java @@ -0,0 +1,549 @@ +/* + * Copyright (c) 2024, 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; + +import com.sun.javafx.scene.control.VirtualScrollBar; +import com.sun.javafx.tk.Toolkit; +import javafx.beans.property.SimpleStringProperty; +import javafx.collections.FXCollections; +import javafx.scene.Node; +import javafx.scene.control.IndexedCell; +import javafx.scene.control.TableCell; +import javafx.scene.control.TableColumn; +import javafx.scene.control.TableView; +import javafx.scene.control.skin.VirtualFlow; +import javafx.scene.control.skin.VirtualFlowShim; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import test.com.sun.javafx.scene.control.infrastructure.StageLoader; +import test.com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +class TableViewVirtualizationTest { + + private StageLoader stageLoader; + private TableView tableView; + + @BeforeEach + void setUp() { + tableView = new TableView<>(); + tableView.setFixedCellSize(24); + tableView.setPrefWidth(300); + tableView.setPrefHeight(300); + tableView.setItems(FXCollections.observableArrayList("1", "2", "3", "4")); + + for (int index = 0; index < 5; index++) { + TableColumn tableColumn = new TableColumn<>(String.valueOf(index)); + tableColumn.setPrefWidth(100); + tableView.getColumns().add(tableColumn); + } + + stageLoader = new StageLoader(tableView); + + Toolkit.getToolkit().firePulse(); + } + + @AfterEach + void cleanUp() { + if (stageLoader != null) { + stageLoader.dispose(); + } + } + + @Test + void testHorizontalVirtualizationInitial() { + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationClearColumns() { + tableView.getColumns().clear(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 0); + } + } + + @Test + void testHorizontalVirtualizationAddColumnsOneByOne() { + tableView.getColumns().clear(); + Toolkit.getToolkit().firePulse(); + + TableColumn tableColumn = new TableColumn<>("Column"); + tableColumn.setPrefWidth(100); + tableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 1); + } + + tableColumn = new TableColumn<>("Column"); + tableColumn.setPrefWidth(100); + tableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 2); + } + + tableColumn = new TableColumn<>("Column"); + tableColumn.setPrefWidth(100); + tableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + + tableColumn = new TableColumn<>("Column"); + tableColumn.setPrefWidth(100); + tableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + + tableColumn = new TableColumn<>("Column"); + tableColumn.setPrefWidth(100); + tableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationRemoveFirstColumn() { + tableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationRemoveLastColumn() { + tableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationAddFirstColumn() { + TableColumn tableColumn = new TableColumn<>("NEW"); + tableColumn.setPrefWidth(50); + tableView.getColumns().addFirst(tableColumn); + + // Needs a double pulse so that the viewport breadth is correctly calculated. + Toolkit.getToolkit().firePulse(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 4); + } + + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(tableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationAddLastColumn() { + TableColumn tableColumn = new TableColumn<>("NEW"); + tableColumn.setPrefWidth(50); + tableView.getColumns().addLast(tableColumn); + + // Needs a double pulse so that the viewport breadth is correctly calculated. + Toolkit.getToolkit().firePulse(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(tableView); + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 4); + } + } + + @Test + void testHorizontalVirtualizationRemoveAllColumnsFromLast() { + tableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + + tableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 3); + } + + tableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 2); + } + + tableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 1); + } + + tableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + assertCellCountInRow(index, 0); + } + } + + @Test + void testHorizontalVirtualizationRemoveAllColumnsFromFirst() { + tableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, getCellCount(row)); + } + + tableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, getCellCount(row)); + } + + tableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(2, getCellCount(row)); + } + + tableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(1, getCellCount(row)); + } + + tableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(0, getCellCount(row)); + } + } + + /** + * This test does the same as JavaFX is doing when reordering a column (via dragging). + * We expect that the cells match with the table columns. + */ + @Test + void testHorizontalVirtualizationMoveColumn() { + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, row.getChildrenUnmodifiable().size()); + for (int cellIndex = 0; cellIndex < row.getChildrenUnmodifiable().size(); cellIndex++) { + Node cell = row.getChildrenUnmodifiable().get(cellIndex); + if (cell instanceof TableCell tableCell) { + assertSame(tableView.getColumns().get(cellIndex), tableCell.getTableColumn()); + } + } + } + + TableColumn columnToMove = tableView.getColumns().getLast(); + + List> allColumns = new ArrayList<>(tableView.getColumns()); + allColumns.remove(columnToMove); + allColumns.addFirst(columnToMove); + + tableView.getColumns().setAll(allColumns); + + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, row.getChildrenUnmodifiable().size()); + for (int cellIndex = 0; cellIndex < row.getChildrenUnmodifiable().size(); cellIndex++) { + Node cell = row.getChildrenUnmodifiable().get(cellIndex); + if (cell instanceof TableCell tableCell) { + assertSame(tableView.getColumns().get(cellIndex), tableCell.getTableColumn()); + } + } + } + } + + @Test + void testHorizontalVirtualizationInitialChangeFixedCellSize() { + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, row.getChildrenUnmodifiable().size()); + } + + tableView.setFixedCellSize(-1); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(5, row.getChildrenUnmodifiable().size()); + } + + tableView.setFixedCellSize(24); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, row.getChildrenUnmodifiable().size()); + } + } + + @Test + void testHorizontalVirtualizationScrolledToEnd() { + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(tableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, row.getChildrenUnmodifiable().size()); + } + } + + @Test + void testHorizontalVirtualizationScrolledToEndAndStart() { + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(tableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + scrollBar.setValue(0); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, row.getChildrenUnmodifiable().size()); + } + } + + @Test + void testHorizontalVirtualizationScrolledToEndAndNearStartAndStart() { + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(tableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + scrollBar.setValue(10); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(4, getCellCount(row)); + } + + scrollBar.setValue(0); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(3, getCellCount(row)); + } + } + + @Test + void testHorizontalVirtualizationIncreaseTableWidth() { + tableView.setPrefWidth(400); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(4, row.getChildrenUnmodifiable().size()); + } + } + + @Test + void testHorizontalVirtualizationDecreaseTableWidth() { + tableView.setPrefWidth(200); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(2, row.getChildrenUnmodifiable().size()); + } + } + + @Test + void testHorizontalVirtualizationIncreaseColumnSize() { + tableView.getColumns().getFirst().setPrefWidth(200); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(2, row.getChildrenUnmodifiable().size()); + } + } + + @Test + void testHorizontalVirtualizationDecreaseColumnSize() { + tableView.getColumns().getFirst().setPrefWidth(50); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < tableView.getItems().size(); index++) { + IndexedCell row = getRow(index); + + assertEquals(4, row.getChildrenUnmodifiable().size()); + } + } + + /** + * More complex test scenario where we: + *
    + *
  • 1. Scroll to the bottom right first
    • + *
  • 2. Resize the table
    • + *
  • 3. Scroll to the left and a little bit up
    • + *
  • 4. Check, that the cells of the rendered rows do not contain outdated items
    • + *
+ */ + @Test + void testHorizontalVirtualizationDoesNotBreakTableCells() { + tableView.setPrefHeight(500); + tableView.setPrefWidth(500); + + final String cellValue = "NOT VISIBLE"; + + for (int index = 0; index < 15; index++) { + TableColumn tableColumn = new TableColumn<>(String.valueOf(index)); + tableColumn.setPrefWidth(100); + tableColumn.setCellValueFactory(_ -> new SimpleStringProperty(cellValue)); + tableView.getColumns().add(tableColumn); + } + + for (int index = 0; index < 100; index++) { + tableView.getItems().add(String.valueOf(index)); + } + + Toolkit.getToolkit().firePulse(); + + VirtualScrollBar vbar = VirtualFlowTestUtils.getVirtualFlowVerticalScrollbar(tableView); + VirtualScrollBar hbar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(tableView); + + vbar.setValue(vbar.getMax()); + Toolkit.getToolkit().firePulse(); + + hbar.setValue(hbar.getMax()); + Toolkit.getToolkit().firePulse(); + + tableView.setPrefHeight(300); + Toolkit.getToolkit().firePulse(); + + hbar.setValue(0); + Toolkit.getToolkit().firePulse(); + + vbar.setValue(0.85); + Toolkit.getToolkit().firePulse(); + + VirtualFlow> virtualFlow = VirtualFlowShim.getVirtualFlow(tableView.getSkin()); + + List> rows = VirtualFlowShim.getCells(virtualFlow); + for (IndexedCell row : rows) { + for (Node cell : row.getChildrenUnmodifiable()) { + if (cell instanceof TableCell tableCell) { + assertNotEquals(cellValue, tableCell.getItem()); + } + } + } + } + + private IndexedCell getRow(int index) { + return VirtualFlowTestUtils.getVirtualFlow(tableView).getVisibleCell(index); + } + + private int getCellCount(IndexedCell row) { + return row.getChildrenUnmodifiable().size(); + } + + private void assertCellCountInRow(int index, int count) { + IndexedCell row = getRow(index); + assertEquals(count, getCellCount(row)); + } + +} diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java index 2df7ac9c3cf..2e2b25be7c1 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java @@ -3275,10 +3275,10 @@ private void test_rt_34685(boolean useMouseToInitiateEdit, boolean cellSelection table.getColumns().addAll(first); + stageLoader = new StageLoader(table); + // get the cell at (0,0) - we're hiding the root row - VirtualFlowTestUtils.BLOCK_STAGE_LOADER_DISPOSE = true; TreeTableCell cell = (TreeTableCell) VirtualFlowTestUtils.getCell(table, 0, 0); - VirtualFlowTestUtils.BLOCK_STAGE_LOADER_DISPOSE = false; assertTrue(cell.getSkin() instanceof TreeTableCellSkin); assertNull(cell.getGraphic()); assertEquals("John", cell.getText()); diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewVirtualizationTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewVirtualizationTest.java new file mode 100644 index 00000000000..613a2f07028 --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewVirtualizationTest.java @@ -0,0 +1,519 @@ +/* + * Copyright (c) 2024, 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; + +import com.sun.javafx.scene.control.VirtualScrollBar; +import com.sun.javafx.tk.Toolkit; +import javafx.beans.property.SimpleStringProperty; +import javafx.scene.Node; +import javafx.scene.control.IndexedCell; +import javafx.scene.control.TreeItem; +import javafx.scene.control.TreeTableCell; +import javafx.scene.control.TreeTableColumn; +import javafx.scene.control.TreeTableView; +import javafx.scene.control.skin.VirtualFlow; +import javafx.scene.control.skin.VirtualFlowShim; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import test.com.sun.javafx.scene.control.infrastructure.StageLoader; +import test.com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +class TreeTableViewVirtualizationTest { + + private StageLoader stageLoader; + private TreeTableView treeTableView; + + @BeforeEach + void setUp() { + treeTableView = new TreeTableView<>(); + treeTableView.setFixedCellSize(24); + treeTableView.setPrefWidth(300); + treeTableView.setShowRoot(false); + treeTableView.setRoot(new TreeItem<>()); + treeTableView.getRoot().getChildren() + .addAll(new TreeItem<>("1"), new TreeItem<>("2"), new TreeItem<>("3"), new TreeItem<>("4")); + + for (int index = 0; index < 5; index++) { + TreeTableColumn tableColumn = new TreeTableColumn<>(String.valueOf(index)); + tableColumn.setPrefWidth(100); + treeTableView.getColumns().add(tableColumn); + } + + stageLoader = new StageLoader(treeTableView); + + Toolkit.getToolkit().firePulse(); + } + + @AfterEach + void cleanUp() { + if (stageLoader != null) { + stageLoader.dispose(); + } + } + + @Test + void testHorizontalVirtualizationInitial() { + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationClearColumns() { + treeTableView.getColumns().clear(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 0); + } + } + + @Test + void testHorizontalVirtualizationAddColumnsOneByOne() { + treeTableView.getColumns().clear(); + Toolkit.getToolkit().firePulse(); + + TreeTableColumn tableColumn = new TreeTableColumn<>("Column"); + tableColumn.setPrefWidth(100); + treeTableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 1); + } + + tableColumn = new TreeTableColumn<>("Column"); + tableColumn.setPrefWidth(100); + treeTableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 2); + } + + tableColumn = new TreeTableColumn<>("Column"); + tableColumn.setPrefWidth(100); + treeTableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + tableColumn = new TreeTableColumn<>("Column"); + tableColumn.setPrefWidth(100); + treeTableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + tableColumn = new TreeTableColumn<>("Column"); + tableColumn.setPrefWidth(100); + treeTableView.getColumns().add(tableColumn); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationRemoveFirstColumn() { + treeTableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationRemoveLastColumn() { + treeTableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationAddFirstColumn() { + TreeTableColumn tableColumn = new TreeTableColumn<>("NEW"); + tableColumn.setPrefWidth(50); + treeTableView.getColumns().addFirst(tableColumn); + + // Needs a double pulse so that the viewport breadth is correctly calculated. + Toolkit.getToolkit().firePulse(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 4); + } + + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(treeTableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationAddLastColumn() { + TreeTableColumn tableColumn = new TreeTableColumn<>("NEW"); + tableColumn.setPrefWidth(50); + treeTableView.getColumns().addLast(tableColumn); + + // Needs a double pulse so that the viewport breadth is correctly calculated. + Toolkit.getToolkit().firePulse(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(treeTableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 4); + } + } + + @Test + void testHorizontalVirtualizationRemoveAllColumnsFromLast() { + treeTableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + treeTableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + treeTableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 2); + } + + treeTableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 1); + } + + treeTableView.getColumns().removeLast(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 0); + } + } + + @Test + void testHorizontalVirtualizationRemoveAllColumnsFromFirst() { + treeTableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + treeTableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + treeTableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 2); + } + + treeTableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 1); + } + + treeTableView.getColumns().removeFirst(); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 0); + } + } + + /** + * This test does the same as JavaFX is doing when reordering a column (via dragging). + * We expect that the cells match with the table columns. + */ + @Test + void testHorizontalVirtualizationMoveColumn() { + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + + IndexedCell row = getRow(index); + for (int cellIndex = 0; cellIndex < row.getChildrenUnmodifiable().size(); cellIndex++) { + Node cell = row.getChildrenUnmodifiable().get(cellIndex); + if (cell instanceof TreeTableCell tableCell) { + assertSame(treeTableView.getColumns().get(cellIndex), tableCell.getTableColumn()); + } + } + } + + TreeTableColumn columnToMove = treeTableView.getColumns().getLast(); + + List> allColumns = new ArrayList<>(treeTableView.getColumns()); + allColumns.remove(columnToMove); + allColumns.addFirst(columnToMove); + + treeTableView.getColumns().setAll(allColumns); + + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + + IndexedCell row = getRow(index); + for (int cellIndex = 0; cellIndex < row.getChildrenUnmodifiable().size(); cellIndex++) { + Node cell = row.getChildrenUnmodifiable().get(cellIndex); + if (cell instanceof TreeTableCell tableCell) { + assertSame(treeTableView.getColumns().get(cellIndex), tableCell.getTableColumn()); + } + } + } + } + + @Test + void testHorizontalVirtualizationInitialChangeFixedCellSize() { + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + + treeTableView.setFixedCellSize(-1); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 5); + } + + treeTableView.setFixedCellSize(24); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationScrolledToEnd() { + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(treeTableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationScrolledToEndAndNearStart() { + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(treeTableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + scrollBar.setValue(10); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 4); + } + } + + @Test + void testHorizontalVirtualizationScrolledToEndAndNearStartAndStart() { + VirtualScrollBar scrollBar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(treeTableView); + + scrollBar.setValue(scrollBar.getMax()); + Toolkit.getToolkit().firePulse(); + + scrollBar.setValue(10); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 4); + } + + scrollBar.setValue(0); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 3); + } + } + + @Test + void testHorizontalVirtualizationIncreaseTableWidth() { + treeTableView.setPrefWidth(400); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 4); + } + } + + @Test + void testHorizontalVirtualizationDecreaseTableWidth() { + treeTableView.setPrefWidth(200); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 2); + } + } + + @Test + void testHorizontalVirtualizationIncreaseColumnSize() { + treeTableView.getColumns().getFirst().setPrefWidth(200); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 2); + } + } + + @Test + void testHorizontalVirtualizationDecreaseColumnSize() { + treeTableView.getColumns().getFirst().setPrefWidth(50); + Toolkit.getToolkit().firePulse(); + + for (int index = 0; index < treeTableView.getRoot().getChildren().size(); index++) { + assertCellCountInRow(index, 4); + } + } + + /** + * More complex test scenario where we: + *
    + *
  • 1. Scroll to the bottom right first
    • + *
  • 2. Resize the table
    • + *
  • 3. Scroll to the left and a little bit up
    • + *
  • 4. Check, that the cells of the rendered rows do not contain outdated items
    • + *
+ */ + @Test + void testHorizontalVirtualizationDoesNotBreakTableCells() { + treeTableView.setPrefHeight(500); + treeTableView.setPrefWidth(500); + + final String cellValue = "NOT VISIBLE"; + + for (int index = 0; index < 15; index++) { + TreeTableColumn tableColumn = new TreeTableColumn<>(String.valueOf(index)); + tableColumn.setPrefWidth(100); + tableColumn.setCellValueFactory(_ -> new SimpleStringProperty(cellValue)); + treeTableView.getColumns().add(tableColumn); + } + + for (int index = 0; index < 100; index++) { + treeTableView.getRoot().getChildren().add(new TreeItem<>(String.valueOf(index))); + } + + Toolkit.getToolkit().firePulse(); + + VirtualScrollBar vbar = VirtualFlowTestUtils.getVirtualFlowVerticalScrollbar(treeTableView); + VirtualScrollBar hbar = VirtualFlowTestUtils.getVirtualFlowHorizontalScrollbar(treeTableView); + + vbar.setValue(vbar.getMax()); + Toolkit.getToolkit().firePulse(); + + hbar.setValue(hbar.getMax()); + Toolkit.getToolkit().firePulse(); + + treeTableView.setPrefHeight(300); + Toolkit.getToolkit().firePulse(); + + hbar.setValue(0); + Toolkit.getToolkit().firePulse(); + + vbar.setValue(0.85); + Toolkit.getToolkit().firePulse(); + + VirtualFlow> virtualFlow = VirtualFlowShim.getVirtualFlow(treeTableView.getSkin()); + List> rows = VirtualFlowShim.getCells(virtualFlow); + for (IndexedCell row : rows) { + for (Node cell : row.getChildrenUnmodifiable()) { + if (cell instanceof TreeTableCell tableCell) { + assertNotEquals(cellValue, tableCell.getItem()); + } + } + } + } + + private IndexedCell getRow(int index) { + return VirtualFlowTestUtils.getVirtualFlow(treeTableView).getVisibleCell(index); + } + + private int getCellCount(IndexedCell row) { + return row.getChildrenUnmodifiable().size(); + } + + private void assertCellCountInRow(int index, int count) { + IndexedCell row = getRow(index); + assertEquals(count, getCellCount(row)); + } + +} diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java index 444af2fe49c..d5746d8d8ef 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2025, 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 @@ -58,6 +58,7 @@ import javafx.scene.control.Button; import javafx.scene.control.FocusModel; import javafx.scene.control.IndexedCell; +import javafx.scene.control.Label; import javafx.scene.control.MultipleSelectionModel; import javafx.scene.control.SelectionMode; import javafx.scene.control.SelectionModel; @@ -71,8 +72,10 @@ import javafx.scene.control.cell.TextFieldTreeCell; import javafx.scene.control.skin.TextFieldSkin; import javafx.scene.control.skin.TreeCellSkin; +import javafx.scene.control.skin.VirtualFlow; import javafx.scene.image.ImageView; import javafx.scene.input.KeyCode; +import javafx.scene.layout.StackPane; import javafx.scene.layout.VBox; import javafx.scene.paint.Color; import javafx.scene.shape.Circle; @@ -4101,6 +4104,77 @@ public void testCheckPositionAfterCollapsed() { assertTrue(scrolledCell.isVisible()); } + /** + * When the height is changed and a TreeItem is collapsed, the graphics of the cells below that TreeItem + * should still be visible and not disappear. + * + * @see JDK-8346824 + */ + @Test + void testGraphicShouldNotDisappear() { + treeView.setShowRoot(false); + treeView.setRoot(new TreeItem<>("")); + treeView.setFixedCellSize(24); + + for (int rootIndex = 0; rootIndex < 3; rootIndex++) { + TreeItem child = new TreeItem<>("root: " + rootIndex); + child.setExpanded(true); + for (int childIndex = 0; childIndex < 5; childIndex++) { + TreeItem treeItem = new TreeItem<>(); + treeItem.setValue("text: " + rootIndex + "-" + childIndex); + treeItem.setGraphic(new Label("graphic: " + rootIndex + "-" + childIndex)); + child.getChildren().add(treeItem); + } + treeView.getRoot().getChildren().add(child); + } + + stageLoader = new StageLoader(new Scene(new StackPane(treeView), 500, 500)); + VirtualFlow> virtualFlow = + (VirtualFlow>) VirtualFlowTestUtils.getVirtualFlow(treeView); + IndexedCell cell; + + // Check children of first root [1..5] + for (int cellIndex = 1; cellIndex < 6; cellIndex++) { + cell = virtualFlow.getCell(cellIndex); + assertTrue(cell.getChildrenUnmodifiable().contains(cell.getGraphic()), + "Cell does not contain graphic for index: " + cellIndex); + } + + // Check children of second root [7..11] + for (int cellIndex = 1; cellIndex < 6; cellIndex++) { + cell = virtualFlow.getCell(cellIndex); + assertTrue(cell.getChildrenUnmodifiable().contains(cell.getGraphic()), + "Cell does not contain graphic for index: " + cellIndex); + } + + // Check children of third root [13..17] + for (int cellIndex = 13; cellIndex < 18; cellIndex++) { + cell = virtualFlow.getCell(cellIndex); + assertTrue(cell.getChildrenUnmodifiable().contains(cell.getGraphic()), + "Cell does not contain graphic for index: " + cellIndex); + } + + stageLoader.getStage().getScene().getRoot().resize(500, 300); + Toolkit.getToolkit().firePulse(); + + treeView.getRoot().getChildren().get(1).setExpanded(false); + Toolkit.getToolkit().firePulse(); + + // Check children of first root [1..5] + for (int cellIndex = 1; cellIndex < 6; cellIndex++) { + cell = virtualFlow.getCell(cellIndex); + assertTrue(cell.getChildrenUnmodifiable().contains(cell.getGraphic()), + "Cell does not contain graphic for index: " + cellIndex); + } + // Second root is collapsed [6] + // Check children of third root [8..12] + for (int cellIndex = 8; cellIndex < 12; cellIndex++) { + cell = virtualFlow.getCell(cellIndex); + assertTrue(cell.getChildrenUnmodifiable().contains(cell.getGraphic()), + "Cell does not contain graphic for index: " + cellIndex); + } + } + public static class MisbehavingOnCancelTreeCell extends TreeCell { @Override diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java index 2ecd0df38a5..5e1f597426b 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java @@ -644,6 +644,7 @@ public void testTableRowFixedCellSizeEnabled() { @Test public void testTableRowVirtualFlowWidthListenerReplaceSkin() { TableView tableView = createPersonTable(false); + tableView.setFixedCellSize(24); showControl(tableView, true); VirtualFlow flow = getVirtualFlow(tableView); TableRow tableRow = (TableRow) getCell(tableView, 1); @@ -661,6 +662,7 @@ public void testTableRowVirtualFlowWidthListenerReplaceSkin() { @Test public void testTableRowVirtualFlowWidthListener() { TableView tableView = createPersonTable(false); + tableView.setFixedCellSize(24); showControl(tableView, true); VirtualFlow flow = getVirtualFlow(tableView); TableRow tableRow = (TableRow) getCell(tableView, 1); diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java index 85b3c0121e3..fb0c099fbf7 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java @@ -322,6 +322,7 @@ public void invisibleColumnsShouldRemoveCorrespondingCellsInRow() { */ @Test public void cellsShouldBeAddedInRowFixedCellSize() { + tableView.setPrefWidth(800); tableView.setFixedCellSize(24); TableColumn otherColumn = new TableColumn<>("other"); diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java index 911dda7063f..86e8d4421e0 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2024 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 @@ -31,6 +31,8 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import java.lang.ref.WeakReference; + +import javafx.beans.property.SimpleStringProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.geometry.Insets; @@ -328,6 +330,28 @@ public void invisibleColumnsShouldRemoveCorrespondingCellsInRow() { invisibleColumnsShouldRemoveCorrespondingCellsInRowImpl(); } + /** + * The {@link TreeTableRowSkin} should add new cells after new columns are added. + * See: JDK-8321970 + */ + @Test + public void cellsShouldBeAddedInRowFixedCellSize() { + treeTableView.setPrefWidth(800); + treeTableView.setFixedCellSize(24); + + TreeTableColumn otherColumn = new TreeTableColumn<>("other"); + otherColumn.setPrefWidth(100); + otherColumn.setCellValueFactory(value -> new SimpleStringProperty("other")); + treeTableView.getColumns().add(otherColumn); + + Toolkit.getToolkit().firePulse(); + assertEquals(5, treeTableView.getColumns().size()); + + Toolkit.getToolkit().firePulse(); + IndexedCell row = VirtualFlowTestUtils.getCell(treeTableView, 1); + assertEquals(5, row.getChildrenUnmodifiable().stream().filter(TreeTableCell.class::isInstance).count()); + } + /** TreeTableView.refresh() must release all discarded cells JDK-8307538 */ @Test public void cellsMustBeCollectableAfterRefresh() { @@ -419,9 +443,8 @@ private void invisibleColumnsShouldRemoveCorrespondingCellsInRowImpl() { Toolkit.getToolkit().firePulse(); // We set 2 columns to invisible, so the cell count should be decremented by 2 as well. - // Note: TreeTableView has an additional children - the disclosure node - therefore we subtract 1 here. assertEquals(treeTableView.getColumns().size() - 2, - VirtualFlowTestUtils.getCell(treeTableView, 0).getChildrenUnmodifiable().size() - 1); + VirtualFlowTestUtils.getCell(treeTableView, 0).getChildrenUnmodifiable().size()); } private void removedColumnsShouldRemoveCorrespondingCellsInRowImpl() { @@ -431,9 +454,8 @@ private void removedColumnsShouldRemoveCorrespondingCellsInRowImpl() { Toolkit.getToolkit().firePulse(); // We removed 2 columns, so the cell count should be decremented by 2 as well. - // Note: TreeTableView has an additional children - the disclosure node - therefore we subtract 1 here. assertEquals(treeTableView.getColumns().size(), - VirtualFlowTestUtils.getCell(treeTableView, 0).getChildrenUnmodifiable().size() - 1); + VirtualFlowTestUtils.getCell(treeTableView, 0).getChildrenUnmodifiable().size()); } private static class ThrowingTreeTableRowSkin extends TreeTableRowSkin { diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableViewIndentationTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableViewIndentationTest.java index efa87ea4d0d..4cfe4f2099f 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableViewIndentationTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableViewIndentationTest.java @@ -141,7 +141,7 @@ private void testXCoordinateIsAfterDisclosureNode() { Toolkit.getToolkit().firePulse(); TreeTableRow row = (TreeTableRow) VirtualFlowTestUtils.getCell(treeTableView, 0); - TreeTableCell cell = (TreeTableCell) row.getChildrenUnmodifiable().get(1); + TreeTableCell cell = (TreeTableCell) row.getChildrenUnmodifiable().get(0); Node graphic = cell.getGraphic(); double x; diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java index 7a3a8227198..49e403ec122 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java @@ -2092,6 +2092,32 @@ public void testScrollingYIsSnapped() { loader.dispose(); } + @Test + public void testSheetChildrenAreAlwaysTheAmountOfVisibleCells() { + flow = new VirtualFlowShim<>(); + flow.setFixedCellSize(24); + flow.setCellFactory(fw -> new CellStub(flow)); + flow.setCellCount(20); + + flow.resize(250, 240); + pulse(); + + assertEquals(10, flow.sheetChildren.size()); + assertEquals(flow.cells, flow.sheetChildren); + + flow.resize(250, 480); + pulse(); + + assertEquals(20, flow.sheetChildren.size()); + assertEquals(flow.cells, flow.sheetChildren); + + flow.resize(250, 240); + pulse(); + + assertEquals(10, flow.sheetChildren.size()); + assertEquals(flow.cells, flow.sheetChildren); + } + } class GraphicalCellStub extends IndexedCellShim {