Skip to content

Commit fcdccd9

Browse files
author
Marius Hanl
committed
8277000: Tree-/TableRowSkin: replace listener to fixedCellSize by live lookup
Reviewed-by: angorya, mstrauss
1 parent 8a61dd2 commit fcdccd9

File tree

5 files changed

+110
-62
lines changed

5 files changed

+110
-62
lines changed

‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ public TableRowSkin(TableRow<T> control) {
9595
setupTableViewListeners();
9696
}
9797

98-
// FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000
9998
private void setupTableViewListeners() {
10099
TableView<T> tableView = getSkinnable().getTableView();
101100
if (tableView == null) {
@@ -104,31 +103,22 @@ private void setupTableViewListeners() {
104103
setupTableViewListeners();
105104
});
106105
} else {
107-
registerChangeListener(tableView.fixedCellSizeProperty(), e -> {
108-
VirtualFlow<TableRow<T>> virtualFlow = getVirtualFlow();
109-
if (virtualFlow != null) {
110-
unregisterChangeListeners(virtualFlow.widthProperty());
111-
}
112-
113-
updateCachedFixedSize();
114-
});
115-
116-
updateCachedFixedSize();
106+
VirtualFlow<TableRow<T>> virtualFlow = getVirtualFlow();
107+
if (virtualFlow != null) {
108+
registerChangeListener(virtualFlow.widthProperty(), _ -> requestLayoutWhenFixedCellSizeSet());
109+
}
117110
}
118111
}
119112

120-
private void updateCachedFixedSize() {
121-
TableView<T> tableView = getSkinnable().getTableView();
122-
if (tableView != null) {
123-
fixedCellSize = tableView.getFixedCellSize();
124-
fixedCellSizeEnabled = fixedCellSize > 0;
125-
126-
if (fixedCellSizeEnabled) {
127-
VirtualFlow<TableRow<T>> virtualFlow = getVirtualFlow();
128-
if (virtualFlow != null) {
129-
registerChangeListener(virtualFlow.widthProperty(), e -> getSkinnable().requestLayout());
130-
}
131-
}
113+
/**
114+
* When we have a fixed cell size set, we must request layout when the width of the virtual flow changed,
115+
* because we might need to add or remove cells that are now visible or not anymore.
116+
* <br>
117+
* See also: JDK-8144500 and JDK-8185887.
118+
*/
119+
private void requestLayoutWhenFixedCellSizeSet() {
120+
if (getFixedCellSize() > 0) {
121+
getSkinnable().requestLayout();
132122
}
133123
}
134124

@@ -237,6 +227,12 @@ private TableView<T> getTableView() {
237227
return getSkinnable().getTableView();
238228
}
239229

230+
@Override
231+
double getFixedCellSize() {
232+
TableView<T> tableView = getTableView();
233+
return tableView != null ? tableView.getFixedCellSize() : super.getFixedCellSize();
234+
}
235+
240236
// test-only
241237
TableViewSkin<T> getTableViewSkin() {
242238
TableView<T> tableView = getSkinnable().getTableView();

‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import javafx.scene.Node;
4141
import javafx.scene.Parent;
4242
import javafx.scene.control.*;
43+
import javafx.scene.layout.Region;
4344
import javafx.util.Duration;
4445

4546
import com.sun.javafx.tk.Toolkit;
@@ -115,10 +116,6 @@ public abstract class TableRowSkinBase<T,
115116

116117
boolean isDirty = false;
117118

118-
// FIXME: replace cached values with direct lookup - JDK-8277000
119-
double fixedCellSize;
120-
boolean fixedCellSizeEnabled;
121-
122119

123120
/* *************************************************************************
124121
* *
@@ -304,14 +301,15 @@ protected ObjectProperty<Node> graphicProperty() {
304301
if (index < 0/* || row >= itemsProperty().get().size()*/) return;
305302

306303
VirtualFlow<C> virtualFlow = getVirtualFlow();
304+
double fixedCellSize = getFixedCellSize();
307305
for (int column = 0, max = cells.size(); column < max; column++) {
308306
R tableCell = cells.get(column);
309307
TableColumnBase<T, ?> tableColumn = getTableColumn(tableCell);
310308

311309
width = snapSizeX(tableColumn.getWidth());
312310

313311
boolean isVisible = true;
314-
if (fixedCellSizeEnabled) {
312+
if (fixedCellSize > 0) {
315313
// we determine if the cell is visible, and if not we have the
316314
// ability to take it out of the scenegraph to help improve
317315
// performance. However, we only do this when there is a
@@ -413,6 +411,10 @@ protected ObjectProperty<Node> graphicProperty() {
413411
}
414412
}
415413

414+
double getFixedCellSize() {
415+
return Region.USE_COMPUTED_SIZE;
416+
}
417+
416418
int getIndentationLevel(C control) {
417419
return 0;
418420
}
@@ -512,7 +514,8 @@ VirtualFlow<C> getVirtualFlow() {
512514

513515
/** {@inheritDoc} */
514516
@Override protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
515-
if (fixedCellSizeEnabled) {
517+
double fixedCellSize = getFixedCellSize();
518+
if (fixedCellSize > 0) {
516519
return fixedCellSize;
517520
}
518521

@@ -545,7 +548,8 @@ VirtualFlow<C> getVirtualFlow() {
545548

546549
/** {@inheritDoc} */
547550
@Override protected double computeMinHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
548-
if (fixedCellSizeEnabled) {
551+
double fixedCellSize = getFixedCellSize();
552+
if (fixedCellSize > 0) {
549553
return fixedCellSize;
550554
}
551555

@@ -575,7 +579,8 @@ VirtualFlow<C> getVirtualFlow() {
575579

576580
/** {@inheritDoc} */
577581
@Override protected double computeMaxHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
578-
if (fixedCellSizeEnabled) {
582+
double fixedCellSize = getFixedCellSize();
583+
if (fixedCellSize > 0) {
579584
return fixedCellSize;
580585
}
581586
return super.computeMaxHeight(width, topInset, rightInset, bottomInset, leftInset);

‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ public TreeTableRowSkin(TreeTableRow<T> control) {
106106
setupTreeTableViewListeners();
107107
}
108108

109-
// FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000
110109
private void setupTreeTableViewListeners() {
111110
TreeTableView<T> treeTableView = getSkinnable().getTreeTableView();
112111
if (treeTableView == null) {
@@ -119,32 +118,22 @@ private void setupTreeTableViewListeners() {
119118
updateLeafColumns();
120119
});
121120

122-
registerChangeListener(getTreeTableView().fixedCellSizeProperty(), e -> {
123-
VirtualFlow<TreeTableRow<T>> virtualFlow = getVirtualFlow();
124-
if (virtualFlow != null) {
125-
unregisterChangeListeners(virtualFlow.widthProperty());
126-
}
127-
128-
updateCachedFixedSize();
129-
});
130-
updateCachedFixedSize();
121+
VirtualFlow<TreeTableRow<T>> virtualFlow = getVirtualFlow();
122+
if (virtualFlow != null) {
123+
registerChangeListener(virtualFlow.widthProperty(), _ -> requestLayoutWhenFixedCellSizeSet());
124+
}
131125
}
132126
}
133127

134-
private void updateCachedFixedSize() {
135-
if (getSkinnable() != null) {
136-
TreeTableView<T> t = getSkinnable().getTreeTableView();
137-
if (t != null) {
138-
fixedCellSize = t.getFixedCellSize();
139-
fixedCellSizeEnabled = fixedCellSize > 0.0;
140-
141-
if (fixedCellSizeEnabled) {
142-
VirtualFlow<TreeTableRow<T>> virtualFlow = getTableViewSkin().getVirtualFlow();
143-
if (virtualFlow != null) {
144-
registerChangeListener(virtualFlow.widthProperty(), ev -> getSkinnable().requestLayout());
145-
}
146-
}
147-
}
128+
/**
129+
* When we have a fixed cell size set, we must request layout when the width of the virtual flow changed,
130+
* because we might need to add or remove cells that are now visible or not anymore.
131+
* <br>
132+
* See also: JDK-8144500 and JDK-8185887.
133+
*/
134+
private void requestLayoutWhenFixedCellSizeSet() {
135+
if (getFixedCellSize() > 0) {
136+
getSkinnable().requestLayout();
148137
}
149138
}
150139

@@ -319,6 +308,12 @@ private TreeTableView<T> getTreeTableView() {
319308
return getSkinnable().getTreeTableView();
320309
}
321310

311+
@Override
312+
double getFixedCellSize() {
313+
TreeTableView<T> treeTableView = getTreeTableView();
314+
return treeTableView != null ? treeTableView.getFixedCellSize() : super.getFixedCellSize();
315+
}
316+
322317
private void updateDisclosureNodeAndGraphic() {
323318
disclosureNodeDirty = false;
324319

‎modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableSkinShim.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -117,13 +117,11 @@ public static VirtualFlow<?> getVirtualFlow(TreeTableView<?> table) {
117117
//----------------- Tree/TableRowSkin state
118118

119119
public static <T> boolean isFixedCellSizeEnabled(TableRow<T> tableRow) {
120-
TableRowSkin<T> skin = (TableRowSkin<T>) tableRow.getSkin();
121-
return skin.fixedCellSizeEnabled;
120+
return tableRow.getTableView().getFixedCellSize() > 0;
122121
}
123122

124123
public static <T> boolean isFixedCellSizeEnabled(TreeTableRow<T> tableRow) {
125-
TreeTableRowSkin<T> skin = (TreeTableRowSkin<T>) tableRow.getSkin();
126-
return skin.fixedCellSizeEnabled;
124+
return tableRow.getTreeTableView().getFixedCellSize() > 0;
127125
}
128126

129127
public static <T> boolean isDirty(TableRow<T> tableRow) {

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ public class SkinCleanupTest {
121121
/**
122122
* Test access to fixedCellSize via lookup (not listener)
123123
*/
124-
@Disabled("JDK-8277000")
125124
@Test
126125
public void testTreeTableRowFixedCellSizeListener() {
127126
TreeTableView<Person> tableView = createPersonTreeTable(false);
@@ -271,6 +270,37 @@ public void testTreeTableRowFixedCellSizeEnabled() {
271270
assertTrue(isFixedCellSizeEnabled(tableRow), "fixed cell size enabled");
272271
}
273272

273+
@Test
274+
public void testTreeTableRowVirtualFlowWidthListenerReplaceSkin() {
275+
TreeTableView<Person> tableView = createPersonTreeTable(false);
276+
tableView.setFixedCellSize(24);
277+
showControl(tableView, true);
278+
VirtualFlow<?> flow = getVirtualFlow(tableView);
279+
TreeTableRow<?> tableRow = (TreeTableRow<?>) getCell(tableView, 1);
280+
replaceSkin(tableRow);
281+
Toolkit.getToolkit().firePulse();
282+
TreeTableRowSkin<?> rowSkin = (TreeTableRowSkin<?>) tableRow.getSkin();
283+
assertNotNull(
284+
unregisterChangeListeners(rowSkin, flow.widthProperty()),
285+
"row skin must have listener to virtualFlow width");
286+
}
287+
288+
/**
289+
* Sanity test: listener to flow's width is registered.
290+
*/
291+
@Test
292+
public void testTreeTableRowVirtualFlowWidthListener() {
293+
TreeTableView<Person> tableView = createPersonTreeTable(false);
294+
tableView.setFixedCellSize(24);
295+
showControl(tableView, true);
296+
VirtualFlow<?> flow = getVirtualFlow(tableView);
297+
TreeTableRow<?> tableRow = (TreeTableRow<?>) getCell(tableView, 1);
298+
TreeTableRowSkin<?> rowSkin = (TreeTableRowSkin<?>) tableRow.getSkin();
299+
assertNotNull(
300+
unregisterChangeListeners(rowSkin, flow.widthProperty()),
301+
"row skin must have listener to virtualFlow width");
302+
}
303+
274304
@Test
275305
public void testTreeTableRowTracksVirtualFlowReplaceSkin() {
276306
TreeTableView<Person> tableView = createPersonTreeTable(false);
@@ -550,7 +580,6 @@ private TreeTableView<Person> createPersonTreeTable(boolean installSkin) {
550580
/**
551581
* Test access to fixedCellSize via lookup (not listener)
552582
*/
553-
@Disabled("JDK-8277000")
554583
@Test
555584
public void testTableRowFixedCellSizeListener() {
556585
TableView<Person> tableView = createPersonTable(false);
@@ -672,6 +701,31 @@ public void testTableRowVirtualFlowWidthListener() {
672701
"row skin must have listener to virtualFlow width");
673702
}
674703

704+
@Test
705+
public void testTableRowTracksVirtualFlowReplaceSkin() {
706+
TableView<Person> tableView = createPersonTable(false);
707+
showControl(tableView, true);
708+
VirtualFlow<?> flow = getVirtualFlow(tableView);
709+
TableRow<?> tableRow = (TableRow<?>) getCell(tableView, 1);
710+
replaceSkin(tableRow);
711+
Toolkit.getToolkit().firePulse();
712+
TableRowSkin<?> rowSkin = (TableRowSkin<?>) tableRow.getSkin();
713+
checkFollowsWidth(flow, (Region) rowSkin.getNode());
714+
}
715+
716+
/**
717+
* Sanity test checks that tree table row skin tracks the virtual flow width.
718+
*/
719+
@Test
720+
public void testTableRowTracksVirtualFlowWidth() {
721+
TableView<Person> tableView = createPersonTable(false);
722+
showControl(tableView, true);
723+
VirtualFlow<?> flow = getVirtualFlow(tableView);
724+
TableRow<?> tableRow = (TableRow<?>) getCell(tableView, 1);
725+
TableRowSkin<?> rowSkin = (TableRowSkin<?>) tableRow.getSkin();
726+
checkFollowsWidth(flow, (Region) rowSkin.getNode());
727+
}
728+
675729
/**
676730
* Sanity: children don't pile up with fixed cell size.
677731
*/

0 commit comments

Comments
 (0)