On Wed, 4 Dec 2024 16:20:48 GMT, Andy Goryachev <ango...@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. > > 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. Sure, can do that. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1645#discussion_r1870972167
