[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: RFR: 8252935: Add treeShowing listener only when needed [v3]
From: Kevin Rushforth <kcr () openjdk ! java ! net>
Date: 2021-01-28 13:40:47
Message-ID: H6WKIfeEJPycsvO3jYts2nD0uCYkWFl36pUhgIkHNV8=.827e7083-76b4-433d-baa0-34427126d316 () github ! com
[Download RAW message or body]
On Sun, 24 Jan 2021 15:25:06 GMT, John Hendrikx <jhendrikx@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.
>
> John Hendrikx has updated the pull request incrementally with two additional \
> commits since the last revision:
> - Add performance test
> - Use registerChangeListener in ProgressIndicatorSkin
The updated fix using `registerChangeListener` and the test look good. I left a few \
mostly-minor comments.
tests/system/src/test/java/test/javafx/scene/NodeTreeShowingTest.java line 2:
> 1: /*
> 2: * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
2021
tests/system/src/test/java/test/javafx/scene/NodeTreeShowingTest.java line 125:
> 123: });
> 124:
> 125: for(int j = 0; j < 5; j++) {
MInor: add space after `for`
tests/system/src/test/java/test/javafx/scene/NodeTreeShowingTest.java line 49:
> 47: /**
> 48: * TODO fix
> 49: * This test is based on the test case reported in JDK-8209830
Comment block needs to be updated to describe this bug.
modules/javafx.graphics/src/main/java/com/sun/javafx/scene/TreeShowingExpression.java \
line 2:
> 1: /*
> 2: * Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
change 2020 to 2021
-------------
PR: https://git.openjdk.java.net/jfx/pull/185
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic