Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v3]

2023-04-09 Thread John Hendrikx
On Sun, 9 Apr 2023 21:27:36 GMT, John Hendrikx wrote: >> Description copied from issue: >> >> There are up to two additional invalidations performed that really should be >> avoided, causing downstream fluent bindings to be recomputed with the same >> values. This is very confusing as these

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v3]

2023-04-09 Thread Nir Lisker
On Sun, 9 Apr 2023 21:27:36 GMT, John Hendrikx wrote: >> Description copied from issue: >> >> There are up to two additional invalidations performed that really should be >> avoided, causing downstream fluent bindings to be recomputed with the same >> values. This is very confusing as these

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread John Hendrikx
On Sun, 9 Apr 2023 16:55:50 GMT, Nir Lisker wrote: > About the second bug. I tested that there are no extra invalidation events > after the patch that were occurring without it. Isn't this something there > should be a test for (a counter for invalidation events)? I think I misunderstood this

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v3]

2023-04-09 Thread John Hendrikx
> Description copied from issue: > > There are up to two additional invalidations performed that really should be > avoided, causing downstream fluent bindings to be recomputed with the same > values. This is very confusing as these should only be called when there is > an actual change, and

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread John Hendrikx
On Sun, 9 Apr 2023 20:53:48 GMT, John Hendrikx wrote: >> That's not really useful as there is no way to make such a fluent binding >> valid again (it would invalidate once, and never become valid). The only >> way to make it valid would be to assign the result of `map` to a variable, >> and

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread John Hendrikx
On Sun, 9 Apr 2023 16:46:44 GMT, Nir Lisker wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix review comments > > modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueWhenTest.java > line

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread John Hendrikx
On Sun, 9 Apr 2023 20:48:50 GMT, John Hendrikx wrote: >> modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueWhenTest.java >> line 139: >> >>> 137: property.when(condition) >>> 138: .map(x -> { observedMappings.add(x); return x; }) >>>

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread John Hendrikx
On Sun, 9 Apr 2023 16:42:10 GMT, Nir Lisker wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix review comments > > modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueWhenTest.java > line

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread Nir Lisker
On Sun, 9 Apr 2023 09:04:15 GMT, John Hendrikx wrote: > > Also, I really wish we could make `add/removeListener` throw on `null` :) > > You mean immediately? Because the `null` check is done by `ExpressionHelper` > already. What I wish for is that `removeListener` would throw when removing >

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread Nir Lisker
On Fri, 7 Apr 2023 06:36:50 GMT, John Hendrikx wrote: >> Description copied from issue: >> >> There are up to two additional invalidations performed that really should be >> avoided, causing downstream fluent bindings to be recomputed with the same >> values. This is very confusing as these

Re: RFR: JDK-8304959: Public API in javafx.css.Match should not return private API class PseudoClassState [v6]

2023-04-09 Thread John Hendrikx
> The class `PseudoClassState` is private API, but was exposed erroneously in > the CSS API. Instead, `Set` should have been used. This PR > corrects this. John Hendrikx has updated the pull request incrementally with one additional commit since the last revision: Fix PseudoClassTest

Re: RFR: JDK-8303897 ObservableValue's when binding should only invalidate when strictly needed [v2]

2023-04-09 Thread John Hendrikx
On Sat, 8 Apr 2023 23:56:10 GMT, Nir Lisker wrote: > I reviewed the first bug and the fix looks good. > > The fix adds a `boolean` field to all `ObjectBinding`s, which could be a > slight performance hit. I have a local version of a refactored > `ExpressionHelper` class that uses a singleton

Re: RFR: JDK-8304959: Public API in javafx.css.Match should not return private API class PseudoClassState [v5]

2023-04-09 Thread John Hendrikx
> The class `PseudoClassState` is private API, but was exposed erroneously in > the CSS API. Instead, `Set` should have been used. This PR > corrects this. John Hendrikx has updated the pull request incrementally with one additional commit since the last revision: Add missing null check