RFR: 8252935: Add treeShowing listener only when needed

Kevin Rushforth kcr at openjdk.java.net
Fri Jan 22 21:39:46 UTC 2021

On Fri, 17 Apr 2020 08:06:23 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

> This is a PoC for performance testing.  
> It contains commented out code in PopupWindow and ProgressIndicatorSkin and two tests are failing because of that.
> This code avoids registering two listeners (on Scene and on Window) for each and every Node to support the aforementioned classes.  The complete change should update these two classes to add their own listeners instead.

I left a few inline comments below.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java line 137:

> 135: 
> 136:         treeShowingExpression = new TreeShowingExpression(control);
> 137:         treeShowingExpression.addListener((obs, old, current) -> updateAnimation());

Is there a reason not to still use `registerChangeListener` for this?

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java line 239:

> 237:         super.dispose();
> 238: 
> 239:         treeShowingExpression.dispose();

This could be removed if you are able to use `registerChangeListener` to add the listener.

modules/javafx.graphics/src/main/java/javafx/scene/SubScene.java line 312:

> 310:                     _value.setTreeVisible(isTreeVisible());
> 311:                     _value.setDisabled(isDisabled());
> 312:                     _value.setTreeShowing(isTreeShowing());

This looks fine. It might be worth adding a unit test to ensure that `isTreeShowing` works correctly for nodes in a `SubScene`.


PR: https://git.openjdk.java.net/jfx/pull/185

More information about the openjfx-dev mailing list