On Tue, 21 Apr 2020 14:22:46 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> @kevinrushforth I've updated this PR now with proper listeners added in >> again for PopupWindow and >> ProgressIndicatorSkin. In short, the functionality to support the >> `treeShowingProperty` has been extracted to a >> separate class `TreeShowingExpression` which is now used by the two classes >> mentioned. All tests pass, including the >> memory leak tests that failed before. >> The issue JDK-8090322 you mentioned actually cautioned for not adding such >> listeners for all nodes and seemed to >> propose the kind of solution I constructed here with a separate class for >> the `treeShowingProperty`: >>> This is too expensive to calculate for all nodes by default. So the >>> simplest way to provide it would be a special >>> binding implementation or a util class. Where you create a instance and >>> pass in the node you are interested in. It can >>> then register listeners all the way up the tree and listen to what it needs. > > @kevinrushforth > > I have another working alternative, but won't make a PR for it until it is > more clear which direction the TableView / > TreeTableView performance fixes are going to take. > The alternative would convert the `treeShowing` property in `Node` into a > "lazy" property (something which JavaFX does > not support directly at the moment). A lazy property only binds to its > dependencies when it is observed itself (so in > this specific case, only when PopupWindow or ProgressIndicatorSkin are making > use of it). This means the property > stays a part of `NodeHelper` but will only register its listeners on Window > and Scene when it is observed itself. > Such lazy properties could be of great use in JavaFX in general, not just in > this case. @hjohn Per [this message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-September/027534.html) on the openjfx-dev mailing list, I have filed a new JBS issue for this PR to use. Please change the title to: 8252935: Add treeShowing listener only when needed ------------- PR: https://git.openjdk.java.net/jfx/pull/185
