diff --git a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkin.java b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkin.java --- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkin.java +++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkin.java @@ -64,8 +64,8 @@ if ("TABLE_VIEW".equals(p)) { updateTableViewSkin(); - for (int i = 0, max = cells.size(); i < max; i++) { - Node n = cells.get(i); + for (int i = 0, max = getChildren().size(); i < max; i++) { + Node n = getChildren().get(i); if (n instanceof TableCell) { ((TableCell)n).updateTableView(getSkinnable().getTableView()); } @@ -95,6 +95,10 @@ cell.updateTableRow(row); } + @Override protected void updateTableColumn(TableCell cell, TableColumnBase col) { + cell.updateTableColumn((TableColumn)col); + } + @Override protected DoubleProperty fixedCellSizeProperty() { return tableView.fixedCellSizeProperty(); } diff --git a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkinBase.java b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkinBase.java --- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkinBase.java +++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TableRowSkinBase.java @@ -80,10 +80,10 @@ */ static final Map 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; +// // 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; @@ -108,17 +108,19 @@ protected WeakHashMap cellsMap; // This observableArrayList contains the currently visible table cells for this row. - protected final List cells = new ArrayList(); +// protected final List cells = new ArrayList(); - private int fullRefreshCounter = DEFAULT_FULL_REFRESH_COUNTER; +// private int fullRefreshCounter = DEFAULT_FULL_REFRESH_COUNTER; protected boolean isDirty = false; protected boolean updateCells = false; + // code related to fixed cell sizes / horizontal virtualisation private double fixedCellSize; private boolean fixedCellSizeEnabled; + private List pile = new ArrayList<>(); - private int columnCount = 0; +// private int columnCount = 0; @@ -139,9 +141,15 @@ // init isn't a constructor, but it is part of the initialisation routine protected void init(C control) { + if (fixedCellSizeProperty() != null) { + registerChangeListener(fixedCellSizeProperty(), "FIXED_CELL_SIZE"); + fixedCellSize = fixedCellSizeProperty().get(); + fixedCellSizeEnabled = fixedCellSize > 0; + } + getSkinnable().setPickOnBounds(false); - recreateCells(); +// recreateCells(); updateCells(true); // init bindings @@ -152,12 +160,6 @@ // --- end init bindings registerChangeListener(control.itemProperty(), "ITEM"); - - if (fixedCellSizeProperty() != null) { - registerChangeListener(fixedCellSizeProperty(), "FIXED_CELL_SIZE"); - fixedCellSize = fixedCellSizeProperty().get(); - fixedCellSizeEnabled = fixedCellSize > 0; - } } @@ -176,7 +178,7 @@ }; private WeakListChangeListener weakVisibleLeafColumnsListener = - new WeakListChangeListener(visibleLeafColumnsListener); + new WeakListChangeListener<>(visibleLeafColumnsListener); @@ -202,6 +204,8 @@ // cell.updateTableRow(skinnable); (i.e cell.updateTableRow(row)) protected abstract void updateCell(R cell, C row); + protected abstract void updateTableColumn(R cell, TableColumnBase col); + protected abstract DoubleProperty fixedCellSizeProperty(); protected abstract boolean isColumnPartiallyOrFullyVisible(TableColumnBase tc); @@ -231,8 +235,13 @@ } @Override protected void layoutChildren(double x, final double y, final double w, final double h) { - checkState(true); - if (cellsMap.isEmpty()) return; +// checkState(true); + + // by doing this we don't get the vertical lines in empty table rows + if (getSkinnable().isEmpty()) { + return; + } +// if (cellsMap.isEmpty()) return; ObservableList visibleLeafColumns = getVisibleLeafColumns(); if (visibleLeafColumns.isEmpty()) { @@ -288,9 +297,7 @@ /////////////////////////////////////////// // layout the individual column cells - double width; - double height; - + R tableCell = null; final double verticalPadding = snappedTopInset() + snappedBottomInset(); final double horizontalPadding = snappedLeftInset() + snappedRightInset(); final double controlHeight = control.getHeight(); @@ -305,13 +312,11 @@ int index = control.getIndex(); if (index < 0/* || row >= itemsProperty().get().size()*/) return; - for (int column = 0, max = cells.size(); column < max; column++) { - R tableCell = cells.get(column); - TableColumnBase tableColumn = getTableColumnBase(tableCell); + for (int column = 0, max = visibleLeafColumns.size(); column < max; column++) { + TableColumnBase tableColumn = visibleLeafColumns.get(column); - width = snapSize(tableCell.prefWidth(-1)) - snapSize(horizontalPadding); - height = Math.max(controlHeight, tableCell.prefHeight(-1)); - height = snapSize(height) - snapSize(verticalPadding); + double width = snapSize(tableColumn.getWidth()) - snapSize(horizontalPadding); + double height; boolean isVisible = true; if (fixedCellSizeEnabled) { @@ -328,11 +333,11 @@ } if (isVisible) { - if (fixedCellSizeEnabled && tableCell.getParent() == null) { - getChildren().add(tableCell); - } + tableCell = getCell(column, tableColumn); - +// width = snapSize(tableCell.prefWidth(-1)) - snapSize(horizontalPadding); + height = Math.max(controlHeight, tableCell.prefHeight(-1)); + height = snapSize(height) - snapSize(verticalPadding); /////////////////////////////////////////// // further indentation code starts here @@ -378,17 +383,11 @@ tableCell.resize(width, height); tableCell.relocate(x, snappedTopInset()); + System.out.println(x + " " + getTableColumnBase(tableCell).getText()); // Request layout is here as (partial) fix for RT-28684. // This does not appear to impact performance... tableCell.requestLayout(); - } else { - 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 - getChildren().remove(tableCell); - } } x += width; @@ -442,11 +441,46 @@ return visibleLeafColumns.get(column); } + private R getCell(int column, TableColumnBase tableColumn) { + if (fixedCellSizeEnabled) { +// System.out.println("get cell: " + column + ", cellsMap size: " + cellsMap.size() + ", pile size: " + pile.size()); + + // firstly attempt to get cell from the cellsMap cache + R cell = cellsMap == null ? null : cellsMap.get(tableColumn); + if (cell != null) { + return cell; + } + + // if that fails, try to reuse a cell from the pile + if (pile.size() > 0) { + // get a cell from the pile and reuse it + cell = pile.remove(pile.size() - 1); + updateTableColumn(cell, tableColumn); + return cell; + } + + // and if that fails (there are no cells on the pile), we will + // create a new cell + cell = createCell(tableColumn); + + return cell; + } else { + // when not in fixed cell size mode, we create all cells, so we + // simply look up the requested cell in the cells list + return pile.get(column); + } + } + protected void updateCells(boolean resetChildren) { + if (getSkinnable().isEmpty()) { + return; + } +// System.out.println(getSkinnable().getIndex() + " 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 boolean cellsEmpty = pile.isEmpty(); + pile.clear(); final C skinnable = getSkinnable(); final int skinnableIndex = skinnable.getIndex(); @@ -455,24 +489,44 @@ for (int i = 0, max = visibleLeafColumns.size(); i < max; i++) { TableColumnBase col = visibleLeafColumns.get(i); - R cell = cellsMap.get(col); - if (cell == null) { - // if the cell is null it means we don't have it in cache and - // need to create it - cell = createCell(col); + R cell = cellsMap == null ? null : cellsMap.get(col); + + if (fixedCellSizeEnabled) { + // we determine if the cell is visible, and if not we have the + // ability to take it out of the scenegraph to help improve + // performance. However, we only do this when there is a + // fixed cell length specified in the TableView. This is because + // when we have a fixed cell length it is possible to know with + // certainty the height of each TableCell - it is the fixed value + // 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. + if (isColumnPartiallyOrFullyVisible(col)) { + cell = createCell(col); + } + } else { + if (cell == null) { + // if the cell is null it means we don't have it in cache and + // need to create it + cell = createCell(col); + } } - updateCell(cell, skinnable); - cell.updateIndex(skinnableIndex); - cells.add(cell); + if (cell != null) { + updateCell(cell, skinnable); + cell.updateIndex(skinnableIndex); + pile.add(cell); + } } // update children of each row if (!fixedCellSizeEnabled && (resetChildren || cellsEmpty)) { - getChildren().setAll(cells); + getChildren().setAll(pile); } } + private double oldPrefWidth = 0; + @Override protected double computePrefWidth(double height, double topInset, double rightInset, double bottomInset, double leftInset) { double prefWidth = 0.0; @@ -481,6 +535,13 @@ prefWidth += visibleLeafColumns.get(i).getWidth(); } + // the width has increased, we need to update the children list + final boolean updateChildren = prefWidth > oldPrefWidth; + // fix for RT-29080 + checkState(updateChildren); + + oldPrefWidth = prefWidth; + return prefWidth; } @@ -489,8 +550,8 @@ return fixedCellSize; } - // fix for RT-29080 - checkState(false); +// // fix for RT-29080 +// checkState(false); // Support for RT-18467: making it easier to specify a height for // cells via CSS, where the desired height is less than the height @@ -503,9 +564,9 @@ // FIXME according to profiling, this method is slow and should // be optimised double prefHeight = 0.0f; - final int count = cells.size(); + final int count = getChildren().size(); for (int i=0; i visibleLeafColumns = getVisibleLeafColumns(); + for (int column = 0, max = visibleLeafColumns.size(); column < max; column++) { + TableColumnBase tableColumn = visibleLeafColumns.get(column); + + // we determine if the cell is visible, and if not we have the + // ability to take it out of the scenegraph to help improve + // performance. However, we only do this when there is a + // fixed cell length specified in the TableView. This is because + // when we have a fixed cell length it is possible to know with + // certainty the height of each TableCell - it is the fixed value + // 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. + boolean isVisible = isColumnPartiallyOrFullyVisible(tableColumn); + + R tableCell = getCell(column, tableColumn); + if (isVisible) { + if (tableCell.getParent() == null) { + getChildren().add(tableCell); + } + } else { + if (tableCell != null) { + // we only add/remove to the scenegraph if the fixed cell + // length support is enabled - otherwise we keep all + // TableCells in the scenegraph +// boolean removed = getChildren().remove(tableCell); +// if (removed) { +// System.out.println("removed cell for column " + column); +// pile.add(tableCell); +// } + } + } + } + System.out.println("AFTER: " + getChildren().size()); + } } @@ -571,54 +683,19 @@ * * **************************************************************************/ - private void recreateCells() { - // This function is smart in the sense that we don't recreate all - // TableCell instances every time this function is called. Instead we - // only create TableCells for TableColumns we haven't already encountered. - // 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 (cellsMap != null) { - Collection cells = cellsMap.values(); - Iterator cellsIter = cells.iterator(); - while (cellsIter.hasNext()) { - R cell = cellsIter.next(); - cell.updateIndex(-1); - cell.getSkin().dispose(); - } - cellsMap.clear(); - } - - ObservableList*/> columns = getVisibleLeafColumns(); - - if (columns.size() != columnCount || fullRefreshCounter == 0 || cellsMap == null) { - cellsMap = new WeakHashMap(columns.size()); - fullRefreshCounter = DEFAULT_FULL_REFRESH_COUNTER; - getChildren().clear(); - } - columnCount = columns.size(); - fullRefreshCounter--; - - for (TableColumnBase col : columns) { - if (cellsMap.containsKey(col)) { - continue; - } - - // create a TableCell for this column and store it in the cellsMap - // for future use - createCell(col); - } - } - private R createCell(TableColumnBase col) { // we must create a TableCell for this table column R cell = getCell(col); + if (cellsMap == null) { + cellsMap = new WeakHashMap<>(getVisibleLeafColumns().size()); + } + // and store this in our HashMap until needed cellsMap.put(col, cell); + System.out.println(getSkinnable().getIndex() + " size: " + cellsMap.size()); + return cell; } diff --git a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java --- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java +++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java @@ -178,14 +178,14 @@ 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); - } +// 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); +// } } } @@ -258,6 +258,10 @@ cell.updateTreeTableRow(row); } + @Override protected void updateTableColumn(TreeTableCell cell, TableColumnBase col) { + cell.updateTreeTableColumn((TreeTableColumn)col); + } + @Override protected boolean isColumnPartiallyOrFullyVisible(TableColumnBase tc) { return treeTableViewSkin == null ? false : treeTableViewSkin.isColumnPartiallyOrFullyVisible(tc); }