On Wed, 17 Feb 2021 22:52:01 GMT, John Hendrikx 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
On Wed, 17 Feb 2021 22:52:01 GMT, John Hendrikx 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
On Tue, 16 Feb 2021 18:59:30 GMT, Kevin Rushforth wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/SubScene.java line 2:
>>
>>> 1: /*
>>> 2: * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
>>
>> The initial year needs to be preserved. Please restore as
On Tue, 16 Feb 2021 17:55:45 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add missed null checks to fix test failure
>
> modules/javafx.graphics/src/main/java/javafx/scene/SubScene.java
> 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
On Wed, 10 Feb 2021 21:56:01 GMT, John Hendrikx 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
On Wed, 10 Feb 2021 21:56:01 GMT, John Hendrikx 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
On Wed, 10 Feb 2021 15:24:07 GMT, Jeanette Winzenburg
wrote:
>> thanks for the info :)
>
>> I could extract the methods instead if you donot like me passing in `null`
>> for the `ObservableValue`, it would then look like:
>>
>> ```
>> sceneChanged(null, node.getScene()); // register
> 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
On Wed, 10 Feb 2021 14:53:20 GMT, Jeanette Winzenburg
wrote:
>> Sadly, there is no way to view just the failures -- at least not right now.
>> What I do is open the raw log and look for "FAILED" (all caps).
>>
>> I'll file an RFE to see whether we can produce a summary of the test
>>
On Wed, 10 Feb 2021 14:29:09 GMT, Kevin Rushforth wrote:
>> somehow missed that the red cross on the checks below might be related to
>> test failures (vs. pre-checking compile issues or such only ;) Any way to
>> see the failing tests only? Scrolling through the complete log is a bit ..
>>
On Wed, 10 Feb 2021 14:24:31 GMT, Jeanette Winzenburg
wrote:
>> good ;)
>
> somehow missed that the red cross on the checks below might be related to
> test failures (vs. pre-checking compile issues or such only ;) Any way to see
> the failing tests only? Scrolling through the complete log is
On Wed, 10 Feb 2021 14:13:23 GMT, Jeanette Winzenburg
wrote:
>>> any of old/new scene can be null (or what am I missing?) If that's not
>>> covered by the tests .. ;)
>>
>> Yep. And if you look at the checks run by GitHub actions, there are failing
>> tests. For example:
>>
>>
On Wed, 10 Feb 2021 13:12:17 GMT, Kevin Rushforth wrote:
>> not going for a full review - just a comment: agree with Kevin, the delegate
>> methods are the way out, basically look good
>>
>> `No further changes in testing required as it is all covered` - a bold
>> statement .. looks like
On Wed, 10 Feb 2021 10:49:52 GMT, Jeanette Winzenburg
wrote:
> any of old/new scene can be null (or what am I missing?) If that's not
> covered by the tests .. ;)
Yep. And if you look at the checks run by GitHub actions, there are failing
tests. For example:
2021-02-09T20:57:51.7835652Z
On Tue, 9 Feb 2021 23:44:56 GMT, Kevin Rushforth wrote:
>> hmm ... might appear convenient (in very controlled contexts) but looks like
>> a precondition violation: the sender of the change must not be null
>> (concededly not explicitly spec'ed but logically implied, IMO)
>>
>> so would tend
On Tue, 9 Feb 2021 10:31:57 GMT, Jeanette Winzenburg
wrote:
>> My concern is about having a similar way of doing something. It would keep
>> the code uniform. We have been following the earlier pattern under a cleanup
>> task [JDK-8241364](https://bugs.openjdk.java.net/browse/JDK-8241364).
On Tue, 9 Feb 2021 20:43:08 GMT, John Hendrikx wrote:
>> hmm ... might appear convenient (in very controlled contexts) but looks like
>> a precondition violation: the sender of the change must not be null
>> (concededly not explicitly spec'ed but logically implied, IMO)
>>
>> so would tend to
> 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
On Tue, 9 Feb 2021 10:31:57 GMT, Jeanette Winzenburg
wrote:
>> My concern is about having a similar way of doing something. It would keep
>> the code uniform. We have been following the earlier pattern under a cleanup
>> task [JDK-8241364](https://bugs.openjdk.java.net/browse/JDK-8241364).
On Mon, 8 Feb 2021 13:42:52 GMT, Ambarish Rapte wrote:
>> Isn't it quite error prone to repeat this logic again (especially with all
>> the null cases), not to mention that you would need to test the code for the
>> initial case (with/without Scene, with/without Window), the "in use" case
>>
On Sat, 6 Feb 2021 18:17:40 GMT, John Hendrikx wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/scene/TreeShowingExpression.java
>> line 89:
>>
>>> 87:
>>> NodeHelper.treeVisibleProperty(node).addListener(windowShowingChangedListener);
>>> 88:
>>> 89:
On Fri, 5 Feb 2021 14:43:25 GMT, Ambarish Rapte wrote:
>> John Hendrikx has updated the pull request incrementally with four
>> additional commits since the last revision:
>>
>> - Add missing TreeShowingExpressionTest which also tests SubScene
>> - Also dispose listeners on Window/Showing in
On Fri, 5 Feb 2021 14:46:49 GMT, Ambarish Rapte wrote:
>> John Hendrikx has updated the pull request incrementally with four
>> additional commits since the last revision:
>>
>> - Add missing TreeShowingExpressionTest which also tests SubScene
>> - Also dispose listeners on Window/Showing in
On Mon, 1 Feb 2021 04:44:03 GMT, John Hendrikx 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
On Mon, 1 Feb 2021 04:41:20 GMT, John Hendrikx wrote:
>> The updated fix using `registerChangeListener` and the test look good. I
>> left a few mostly-minor comments.
>
> I think this change is complete now, including an integration test and tests
> on all new code. Please let me know if
On Fri, 22 Jan 2021 21:36:06 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request incrementally with four
>> additional commits since the last revision:
>>
>> - Add missing TreeShowingExpressionTest which also tests SubScene
>> - Also dispose listeners on Window/Showing
> 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
On Thu, 28 Jan 2021 13:37:59 GMT, Kevin Rushforth wrote:
>> 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
On Sun, 24 Jan 2021 15:25:06 GMT, John Hendrikx 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
On Sun, 24 Jan 2021 09:02:17 GMT, John Hendrikx wrote:
>> I left a few inline comments below.
>
> Sorry about the force push, merging over a year of changes from master did
> not seem right to me. It was only for getting up to date, I will do the
> other commits normally.
> 1. Merge in the
> 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
On Sun, 24 Jan 2021 09:24:56 GMT, John Hendrikx wrote:
>> 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
On Fri, 22 Jan 2021 21:31:42 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains one commit:
>>
>> WIP: Moved treeShowingProperty into its own class
>
>
On Fri, 22 Jan 2021 21:32:28 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains one commit:
>>
>> WIP: Moved treeShowingProperty into its own class
>
>
On Fri, 22 Jan 2021 21:36:53 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains one commit:
>>
>> WIP: Moved treeShowingProperty into its own class
>
> I left a few inline comments
> 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
On Fri, 17 Apr 2020 08:06:23 GMT, John Hendrikx 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)
On Wed, 20 Jan 2021 08:46:13 GMT, John Hendrikx wrote:
>> @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:
>>
>>
On Tue, 8 Sep 2020 20:16:20 GMT, Kevin Rushforth wrote:
>> @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
On Fri, 17 Apr 2020 23:47:42 GMT, John Hendrikx wrote:
>> @kevinrushforth I don't propose to remove them, only to move them to where
>> they are needed.
>>
>> Currently they are part of Node, which means all nodes, whether they need to
>> know the state of the Window or not are
>> registering
On Fri, 17 Apr 2020 17:18:00 GMT, John Hendrikx wrote:
>> These listeners exist for a good reason. Removing them will almost certainly
>> cause regressions in behavior. See
>> [JDK-8090322](https://bugs.openjdk.java.net/browse/JDK-8090322) as well as
>> the follow-on fixes (since that was a
On Tue, 21 Apr 2020 14:22:46 GMT, John Hendrikx 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
On Fri, 17 Apr 2020 16:59:29 GMT, Kevin Rushforth 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
On Fri, 17 Apr 2020 08:06:23 GMT, John Hendrikx 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
45 matches
Mail list logo
