RFR: 8277000: Tree-/TableRowSkin: replace listener to fixedCellSize by live lookup

[Andy Goryachev]

On Fri, 22 Nov 2024 20:31:08 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> This PR improves the `Tree-/TableRowSkin` code by doing a normal live lookup for the `fixedCellSize` instead of adding listener just to update variables(`fixedCellSizeEnabled` and `fixedCellSize`) which can otherwise be also just lookup'd without the hassle of listeners.
> 
> While this is mostly a cleanup, it does improve the state of the `Tree-/TableRow`, as when the `TableRowSkinBase` constructor is called, the variables are not yet set.
> 
> It is also consistent with the other cells, see also [JDK-8246745](https://bugs.openjdk.org/browse/JDK-8246745).
> Helps a bit with [JDK-8185887](https://bugs.openjdk.org/browse/JDK-8185887) (https://github.com/openjdk/jfx/pull/1644), but as written above, not required as there is no (visible) effect.

Tested with the Monkey Tester, both TreeTableView and TableView - looks good, responds to fixed cell size changes.

Left a few minor comments.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java line 323:

> 321: 
> 322:             boolean isVisible = true;
> 323:             if (getFixedCellSize() > 0) {

`getFixedCellSize()` is getting called four times, also inside the `for` loop.
I'd suggest to add

double fixedCellSize = getFixedCellSize();

before L318 (outside of the for loop) and use the local variable instead.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java line 432:

> 430:     }
> 431: 
> 432:     double getFixedCellSize() {

should this method be `abstract`?

-------------

PR Review: https://git.openjdk.org/jfx/pull/1645#pullrequestreview-2479255931
PR Review Comment: https://git.openjdk.org/jfx/pull/1645#discussion_r1869884280
PR Review Comment: https://git.openjdk.org/jfx/pull/1645#discussion_r1869886562