From 4c8453af88fa53e2e0300df4dbca46b655a36920 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Fri, 15 Jul 2022 01:34:55 +0200 Subject: [PATCH 01/16] Fix bug where ObjectBinding returns null when revalidated immediately Introduced with the fluent bindings PR --- .../javafx/beans/binding/ObjectBinding.java | 5 ++++- .../beans/value/LazyObjectBindingTest.java | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java index 84f62a8416c..af3f93d48b9 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java @@ -180,7 +180,10 @@ public final void invalidate() { valid = false; onInvalidating(); ExpressionHelper.fireValueChangedEvent(helper); - value = null; // clear cached value to avoid hard reference to stale data + + if (!valid) { // if still invalid after calling listeners... + value = null; // clear cached value to avoid hard reference to stale data + } } } diff --git a/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java b/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java index 72da81b3c26..23a630c0673 100644 --- a/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java +++ b/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java @@ -52,6 +52,24 @@ void shouldBeInvalidInitially() { assertFalse(binding.isValid()); } + @Test + void invalidationWhichBecomesValidDuringCallbacksShouldReturnCorrectValue() { + LazyObjectBindingStub binding = new LazyObjectBindingStub<>() { + @Override + protected String computeValue() { + return "A"; + } + }; + + binding.addListener(obs -> { + assertEquals("A", binding.get()); + }); + + binding.invalidate(); // becomes valid again immediately + + assertEquals("A", binding.get()); + } + @Nested class WhenObservedWithInvalidationListener { private InvalidationListener invalidationListener = obs -> {}; From 3485656941f80c1eca4a919b11d76abdd3b1ce29 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Fri, 15 Jul 2022 09:15:03 +0200 Subject: [PATCH 02/16] Change explanatory comment to block comment --- .../main/java/javafx/beans/binding/ObjectBinding.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java index af3f93d48b9..131aa4177ee 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java @@ -181,8 +181,13 @@ public final void invalidate() { onInvalidating(); ExpressionHelper.fireValueChangedEvent(helper); - if (!valid) { // if still invalid after calling listeners... - value = null; // clear cached value to avoid hard reference to stale data + /* + * Cached value should be cleared to avoid a strong reference to stale data, + * but only if this binding didn't become valid after firing the event: + */ + + if (!valid) { + value = null; } } } From a70161cbbdfdebbd571802c92fbac40f8b2a1022 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Fri, 15 Jul 2022 11:05:04 +0200 Subject: [PATCH 03/16] Add conditional bindings to ObservableValue --- .../javafx/binding/ConditionalBinding.java | 67 ++++ .../javafx/beans/value/ObservableValue.java | 60 ++++ .../ObservableValueFluentBindingsTest.java | 287 +++++++++++++++++- .../src/main/java/javafx/scene/Node.java | 91 +++++- .../test/java/test/javafx/scene/NodeTest.java | 57 ++++ 5 files changed, 552 insertions(+), 10 deletions(-) create mode 100644 modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java diff --git a/modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java b/modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java new file mode 100644 index 00000000000..693eeaecc9a --- /dev/null +++ b/modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java @@ -0,0 +1,67 @@ +package com.sun.javafx.binding; + +import java.util.Objects; + +import javafx.beans.value.ObservableValue; + +public class ConditionalBinding extends LazyObjectBinding { + + private final ObservableValue source; + private final ObservableValue nonNullCondition; + + private Subscription subscription; + + public ConditionalBinding(ObservableValue source, ObservableValue condition) { + this.source = Objects.requireNonNull(source, "source"); + this.nonNullCondition = Objects.requireNonNull(condition, "condition").orElse(false); + + // condition is always observed and never unsubscribed + Subscription.subscribe(nonNullCondition, current -> { + invalidate(); + + if (!current) { + getValue(); + } + }); + } + + /** + * This binding is valid whenever it is observed, or it is currently inactive. + * When inactive, the binding has the value of its source at the time it became + * inactive. + */ + @Override + protected boolean allowValidation() { + return super.allowValidation() || !isActive(); + } + + @Override + protected T computeValue() { + if (isObserved() && isActive()) { + if(subscription == null) { + subscription = Subscription.subscribeInvalidations(source, this::invalidate); + } + } + else { + unsubscribe(); + } + + return source.getValue(); + } + + @Override + protected Subscription observeSources() { + return this::unsubscribe; + } + + private boolean isActive() { + return nonNullCondition.getValue(); + } + + private void unsubscribe() { + if (subscription != null) { + subscription.unsubscribe(); + subscription = null; + } + } +} diff --git a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java index dba9e23f399..3deb28315a8 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java +++ b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java @@ -27,6 +27,7 @@ import java.util.function.Function; +import com.sun.javafx.binding.ConditionalBinding; import com.sun.javafx.binding.FlatMappedBinding; import com.sun.javafx.binding.MappedBinding; import com.sun.javafx.binding.OrElseBinding; @@ -251,4 +252,63 @@ default ObservableValue orElse(T constant) { default ObservableValue flatMap(Function> mapper) { return new FlatMappedBinding<>(this, mapper); } + + /** + * Returns an {@code ObservableValue} that holds this value whenever the given + * condition evaluates to {@code true}, otherwise holds the last value when + * {@code condition} became {@code false}. The value is updated whenever this + * {@code ObservableValue} changes, unless the condition currently evaluates + * to {@code false}. + *

+ * The returned {@code ObservableValue} only observes this value when the given + * {@code condition} evaluates to {@code true}. This allows this {@code ObservableValue} + * and the conditional {@code ObservableValue} to be garbage collected if neither is + * otherwise strongly referenced when {@code condition} becomes {@code false}. + *

+ * A currently observed binding will observe its source, which means it will not be eligible + * for garbage collection while source isn't. However, using {@code when} this {@code ObservableValue} + * can still be eligible for garbage collection when the condition is {@code false} and the + * conditional itself is also eligible for garbage collection. + *

+ * Returning {@code null} from the given condition is treated the same as + * returning {@code false}. + *

+ * For example: + * 

{@code
+     * ObservableValue condition = new SimpleBooleanProperty(true);
+     * ObservableValue globalProperty = new SimpleStringProperty("A");
+     * ObservableValue whenProperty = property.when(isShowing);
+     *
+     * // observe whenProperty, which will in turn observe globalProperty
+     * whenProperty.addChangeListener((ov, old, current) -> System.out.println(current));
+     *
+     * globalProperty.setValue("B");  // "B" is printed
+     *
+     * condition.setValue(false);
+     *
+     * // After condition becomes false, whenProperty stops observing globalProperty; condition
+     * // and whenProperty may now be eligible for GC despite being observed by the ChangeListener
+     *
+     * globalProperty.setValue("C");  // nothing is printed
+     * globalProperty.setValue("D");  // nothing is printed
+     *
+     * condition.setValue(true);  // globalProperty is observed again, and "D" is printed
+     * }
+ * Another example: + * 
{@code
+     * Label label = ... ;
+     * ObservableValue globalProperty = new SimpleStringProperty("A");
+     *
+     * // bind label's text to a global property only when it is showing:
+     * label.textProperty().bind(globalProperty.when(label::isShowingProperty));
+     * }
+ * @param condition a boolean {@code ObservableValue}, cannot be {@code null} + * @return an {@code ObservableValue} that holds this value whenever the given + * condition evaluates to {@code true}, otherwise holds the last seen value; + * never returns {@code null} + * @since 20 + */ + default ObservableValue when(ObservableValue condition) { + return new ConditionalBinding<>(this, condition); + } } diff --git a/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java b/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java index b6722f7dad5..18c4c8fd1fd 100644 --- a/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java +++ b/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java @@ -39,6 +39,10 @@ import org.junit.jupiter.api.Test; import javafx.beans.InvalidationListener; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleBooleanProperty; +import javafx.beans.property.SimpleObjectProperty; import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; import javafx.beans.value.ChangeListener; @@ -47,11 +51,12 @@ public class ObservableValueFluentBindingsTest { private int invalidations; - private final StringProperty property = new SimpleStringProperty("Initial"); private final List values = new ArrayList<>(); private final ChangeListener changeListener = (obs, old, current) -> values.add(current); private final InvalidationListener invalidationListener = obs -> invalidations++; + private StringProperty property = new SimpleStringProperty("Initial"); + @Nested class When_map_Called { @@ -878,6 +883,284 @@ void shouldNoLongerBeStronglyReferenced() { } } + @Nested + class When_when_Called { + + @Nested + class WithNull { + + @Test + void shouldThrowNullPointerException() { + assertThrows(NullPointerException.class, () -> property.when(null)); + } + } + + @Nested + class WithNotNullAndInitiallyFalseConditionReturns_ObservableValue_Which { + private BooleanProperty condition = new SimpleBooleanProperty(false); + private ObservableValue observableValue = property.when(condition); + + @Test + void shouldNotBeNull() { + assertNotNull(observableValue); + } + + @Test + void shouldNotBeStronglyReferenced() { + ReferenceAsserts.testIfNotStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + + @Nested + class When_getValue_Called { + + @Test + void shouldReturnInitialValueAtTimeOfCreation() { + property.set("Not Initial"); + + assertEquals("Initial", observableValue.getValue()); + } + } + } + + @Nested + class WithNotNullReturns_ObservableValue_Which { + private ObjectProperty condition = new SimpleObjectProperty(true); // using object property here so it can be set to null for testing + private ObservableValue observableValue = property.when(condition); + + @Test + void shouldNotBeNull() { + assertNotNull(observableValue); + } + + @Test + void shouldNotBeStronglyReferenced() { + ReferenceAsserts.testIfNotStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + + @Nested + class When_getValue_Called { + + @Test + void shouldReturnCurrentPropertyValuesWhileConditionIsTrue() { + assertEquals("Initial", observableValue.getValue()); + + property.set(null); + + assertNull(observableValue.getValue()); + + property.set("Left"); + + assertEquals("Left", observableValue.getValue()); + + condition.set(false); + + property.set("Right"); + + assertEquals("Left", observableValue.getValue()); + + property.set("Middle"); + + assertEquals("Left", observableValue.getValue()); + + condition.set(true); + + assertEquals("Middle", observableValue.getValue()); + } + } + + @Nested + class WhenObservedForInvalidations { + { + startObservingInvalidations(observableValue); + } + + @Test + void shouldOnlyInvalidateOnce() { + assertNotInvalidated(); + + property.set("Left"); + + assertInvalidated(); + + property.set("Right"); + + assertNotInvalidated(); + } + + @Test + void shouldOnlyInvalidateWhileConditionIsTrue() { + assertNotInvalidated(); + + property.set("Left"); // trigger invalidation + + assertInvalidated(); + + condition.set(false); + + assertNotInvalidated(); // already invalid, changing condition won't change that + + observableValue.getValue(); // this would normally make the property valid, but not when condition is false + + property.set("Right"); // trigger invalidation + + assertNotInvalidated(); // nothing happened + + condition.setValue(null); // null is false as well, should not change result + + assertNotInvalidated(); // nothing happened + + condition.set(true); + + assertInvalidated(); + + observableValue.getValue(); // make property valid + + assertNotInvalidated(); + + property.set("Middle"); // trigger invalidation + + assertInvalidated(); + } + + @Test + void shouldBeStronglyReferenced() { + ReferenceAsserts.testIfStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + + @Test + void shouldNotBeStronglyReferencedWhenConditionIsFalse() { + condition.set(false); + + ReferenceAsserts.testIfNotStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + + @Nested + class AndWhenUnobserved { + { + stopObservingInvalidations(observableValue); + } + + @Test + void shouldNoLongerBeCalled() { + assertNotInvalidated(); + + property.set("Left"); + property.set("Right"); + + assertNotInvalidated(); + } + + @Test + void shouldNoLongerBeStronglyReferenced() { + ReferenceAsserts.testIfNotStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + } + } + + @Nested + class WhenObservedForChanges { + { + startObservingChanges(observableValue); + } + + @Test + void shouldReceiveCurrentPropertyValues() { + assertNothingIsObserved(); + + property.set("Right"); + + assertObserved("Right"); + } + + @Test + void shouldOnlyReceiveCurrentPropertyValuesWhileConditionIsTrue() { + assertNothingIsObserved(); + + property.set("Right"); + + assertObserved("Right"); + + condition.set(false); + + assertNothingIsObserved(); + + property.set("Left"); + + assertNothingIsObserved(); + + property.set("Middle"); + + assertNothingIsObserved(); + + condition.setValue(null); // null is false as well, should not change result + + assertNothingIsObserved(); + + condition.set(true); + + assertObserved("Middle"); + } + + @Test + void shouldBeStronglyReferenced() { + ReferenceAsserts.testIfStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + + @Test + void shouldNotBeStronglyReferencedWhenConditionIsFalse() { + condition.set(false); + + ReferenceAsserts.testIfNotStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + + @Nested + class AndWhenUnobserved { + { + stopObservingChanges(observableValue); + } + + @Test + void shouldNoLongerBeCalled() { + assertNothingIsObserved(); + + property.set("Right"); + + assertNothingIsObserved(); + } + + @Test + void shouldNoLongerBeStronglyReferenced() { + ReferenceAsserts.testIfNotStronglyReferenced(observableValue, () -> { + observableValue = null; + condition = null; + }); + } + } + } + } + } + /** * Ensures nothing has been observed since the last check. */ @@ -891,7 +1174,7 @@ private void assertNothingIsObserved() { * @param expectedValues an array of expected values */ private void assertObserved(String... expectedValues) { - assertEquals(values, Arrays.asList(expectedValues)); + assertEquals(Arrays.asList(expectedValues), values); values.clear(); } diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index 588e106fda1..c3292cb4fdd 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -49,6 +49,7 @@ import javafx.beans.property.StringProperty; import javafx.beans.property.StringPropertyBase; import javafx.beans.value.ChangeListener; +import javafx.beans.value.ObservableValue; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener.Change; import javafx.collections.ObservableList; @@ -1397,6 +1398,87 @@ public String getName() { return visible; } + /** + * Whether or not this {@code Node} is showing (that is, it is part of an + * open Window on the user's system). The Window might be "showing", yet the user might not + * be able to see it due to the Window being rendered behind another Window + * or due to the Window being positioned off the monitor. + * + *

Note that the {@code Node} does not need to be visible for this property + * to be {@code true}. + * + * @defaultValue false + * @since 20 + */ + private ReadOnlyBooleanProperty showing; + + public final boolean isShowing() { + Scene s = getScene(); + if (s == null) return false; + Window w = s.getWindow(); + return w != null && w.isShowing(); + } + + public final ReadOnlyBooleanProperty showingProperty() { + if (showing == null) { + ObservableValue ov = sceneProperty() + .flatMap(Scene::windowProperty) + .flatMap(Window::showingProperty); + + showing = new ReadOnlyBooleanDelegate(Node.this, "showing", ov); + } + + return showing; + } + + // Candidate to make publicly available or to add as a convience method to ObservableValue + private static class ReadOnlyBooleanDelegate extends ReadOnlyBooleanProperty { + private final ObservableValue delegate; + private final Object bean; + private final String name; + + ReadOnlyBooleanDelegate(Object bean, String name, ObservableValue delegate) { + this.bean = bean; + this.name = name; + this.delegate = delegate.orElse(false); + } + + @Override + public Object getBean() { + return bean; + } + + @Override + public String getName() { + return name; + } + + @Override + public void addListener(ChangeListener listener) { + delegate.addListener(listener); + } + + @Override + public void removeListener(ChangeListener listener) { + delegate.removeListener(listener); + } + + @Override + public void addListener(InvalidationListener listener) { + delegate.addListener(listener); + } + + @Override + public void removeListener(InvalidationListener listener) { + delegate.removeListener(listener); + } + + @Override + public boolean get() { + return delegate.getValue(); // orElse guarantees this is never null + } + } + public final void setCursor(Cursor value) { cursorProperty().set(value); } @@ -8387,15 +8469,8 @@ void markDirtyLayoutBranch() { } - private boolean isWindowShowing() { - Scene s = getScene(); - if (s == null) return false; - Window w = s.getWindow(); - return w != null && w.isShowing(); - } - final boolean isTreeShowing() { - return isTreeVisible() && isWindowShowing(); + return isTreeVisible() && isShowing(); } private void updateTreeVisible(boolean parentChanged) { diff --git a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java index 2752e0bddd5..39dc500f6cd 100644 --- a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java +++ b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java @@ -1255,6 +1255,63 @@ public void testIsTreeVisible() { } + @Test + public void testIsShowingProperty() { + final Group g = new Group(); + final Circle c = new CircleTest.StubCircle(50); + + ParentShim.getChildren(g).add(c); + + Scene s = new Scene(g); + Stage st = new Stage(); + + assertFalse(g.isShowing()); + assertFalse(c.isShowing()); + assertFalse(g.showingProperty().get()); + assertFalse(c.showingProperty().get()); + + st.show(); + st.setScene(s); + SceneShim.scenePulseListener_pulse(s); + + assertTrue(g.isShowing()); + assertTrue(c.isShowing()); + assertTrue(g.showingProperty().get()); + assertTrue(c.showingProperty().get()); + + g.setVisible(false); // irrelevant change for isShowing + SceneShim.scenePulseListener_pulse(s); + + assertTrue(g.isShowing()); + assertTrue(c.isShowing()); + assertTrue(g.showingProperty().get()); + assertTrue(c.showingProperty().get()); + + s.setRoot(new Group()); + SceneShim.scenePulseListener_pulse(s); + + assertFalse(g.isShowing()); + assertFalse(c.isShowing()); + assertFalse(g.showingProperty().get()); + assertFalse(c.showingProperty().get()); + + s.setRoot(g); + SceneShim.scenePulseListener_pulse(s); + + assertTrue(g.isShowing()); + assertTrue(c.isShowing()); + assertTrue(g.showingProperty().get()); + assertTrue(c.showingProperty().get()); + + st.hide(); + SceneShim.scenePulseListener_pulse(s); + + assertFalse(g.isShowing()); + assertFalse(c.isShowing()); + assertFalse(g.showingProperty().get()); + assertFalse(c.showingProperty().get()); + } + @Test public void testSynchronizationOfInvisibleNodes_2() { final Group g = new Group(); From fd19fc366fabd6d6ca93ee55e2004b42c032055d Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Fri, 15 Jul 2022 11:20:22 +0200 Subject: [PATCH 04/16] Rename showing property to shown as it is already used in subclasses --- .../javafx/beans/value/ObservableValue.java | 6 +-- .../src/main/java/javafx/scene/Node.java | 14 +++--- .../test/java/test/javafx/scene/NodeTest.java | 48 +++++++++---------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java index 3deb28315a8..36e291d7457 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java +++ b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java @@ -277,7 +277,7 @@ default ObservableValue flatMap(Function{@code * ObservableValue condition = new SimpleBooleanProperty(true); * ObservableValue globalProperty = new SimpleStringProperty("A"); - * ObservableValue whenProperty = property.when(isShowing); + * ObservableValue whenProperty = property.when(condition); * * // observe whenProperty, which will in turn observe globalProperty * whenProperty.addChangeListener((ov, old, current) -> System.out.println(current)); @@ -299,8 +299,8 @@ default ObservableValue flatMap(Function globalProperty = new SimpleStringProperty("A"); * - * // bind label's text to a global property only when it is showing: - * label.textProperty().bind(globalProperty.when(label::isShowingProperty)); + * // bind label's text to a global property only when it is shown: + * label.textProperty().bind(globalProperty.when(label::isShownProperty)); * } * @param condition a boolean {@code ObservableValue}, cannot be {@code null} * @return an {@code ObservableValue} that holds this value whenever the given diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index c3292cb4fdd..93a5a4ad6dd 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -1410,25 +1410,25 @@ public String getName() { * @defaultValue false * @since 20 */ - private ReadOnlyBooleanProperty showing; + private ReadOnlyBooleanProperty shown; - public final boolean isShowing() { + public final boolean isShown() { Scene s = getScene(); if (s == null) return false; Window w = s.getWindow(); return w != null && w.isShowing(); } - public final ReadOnlyBooleanProperty showingProperty() { - if (showing == null) { + public final ReadOnlyBooleanProperty shownProperty() { + if (shown == null) { ObservableValue ov = sceneProperty() .flatMap(Scene::windowProperty) .flatMap(Window::showingProperty); - showing = new ReadOnlyBooleanDelegate(Node.this, "showing", ov); + shown = new ReadOnlyBooleanDelegate(Node.this, "shown", ov); } - return showing; + return shown; } // Candidate to make publicly available or to add as a convience method to ObservableValue @@ -8470,7 +8470,7 @@ void markDirtyLayoutBranch() { } final boolean isTreeShowing() { - return isTreeVisible() && isShowing(); + return isTreeVisible() && isShown(); } private void updateTreeVisible(boolean parentChanged) { diff --git a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java index 39dc500f6cd..03095fcbb48 100644 --- a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java +++ b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java @@ -1265,51 +1265,51 @@ public void testIsShowingProperty() { Scene s = new Scene(g); Stage st = new Stage(); - assertFalse(g.isShowing()); - assertFalse(c.isShowing()); - assertFalse(g.showingProperty().get()); - assertFalse(c.showingProperty().get()); + assertFalse(g.isShown()); + assertFalse(c.isShown()); + assertFalse(g.shownProperty().get()); + assertFalse(c.shownProperty().get()); st.show(); st.setScene(s); SceneShim.scenePulseListener_pulse(s); - assertTrue(g.isShowing()); - assertTrue(c.isShowing()); - assertTrue(g.showingProperty().get()); - assertTrue(c.showingProperty().get()); + assertTrue(g.isShown()); + assertTrue(c.isShown()); + assertTrue(g.shownProperty().get()); + assertTrue(c.shownProperty().get()); g.setVisible(false); // irrelevant change for isShowing SceneShim.scenePulseListener_pulse(s); - assertTrue(g.isShowing()); - assertTrue(c.isShowing()); - assertTrue(g.showingProperty().get()); - assertTrue(c.showingProperty().get()); + assertTrue(g.isShown()); + assertTrue(c.isShown()); + assertTrue(g.shownProperty().get()); + assertTrue(c.shownProperty().get()); s.setRoot(new Group()); SceneShim.scenePulseListener_pulse(s); - assertFalse(g.isShowing()); - assertFalse(c.isShowing()); - assertFalse(g.showingProperty().get()); - assertFalse(c.showingProperty().get()); + assertFalse(g.isShown()); + assertFalse(c.isShown()); + assertFalse(g.shownProperty().get()); + assertFalse(c.shownProperty().get()); s.setRoot(g); SceneShim.scenePulseListener_pulse(s); - assertTrue(g.isShowing()); - assertTrue(c.isShowing()); - assertTrue(g.showingProperty().get()); - assertTrue(c.showingProperty().get()); + assertTrue(g.isShown()); + assertTrue(c.isShown()); + assertTrue(g.shownProperty().get()); + assertTrue(c.shownProperty().get()); st.hide(); SceneShim.scenePulseListener_pulse(s); - assertFalse(g.isShowing()); - assertFalse(c.isShowing()); - assertFalse(g.showingProperty().get()); - assertFalse(c.showingProperty().get()); + assertFalse(g.isShown()); + assertFalse(c.isShown()); + assertFalse(g.shownProperty().get()); + assertFalse(c.shownProperty().get()); } @Test From 07e9d88aa9b8ba9ce39f92b44754d7532a98f3a3 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Thu, 1 Sep 2022 15:48:09 +0200 Subject: [PATCH 05/16] Update javadoc of "when" with better phrasing --- .../javafx/beans/value/ObservableValue.java | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java index 36e291d7457..3612ad03409 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java +++ b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java @@ -254,53 +254,47 @@ default ObservableValue flatMap(Function - * The returned {@code ObservableValue} only observes this value when the given - * {@code condition} evaluates to {@code true}. This allows this {@code ObservableValue} + * The returned {@code ObservableValue} only observes this value when + * {@code condition} holds {@code true}. This allows this {@code ObservableValue} * and the conditional {@code ObservableValue} to be garbage collected if neither is - * otherwise strongly referenced when {@code condition} becomes {@code false}. + * otherwise strongly referenced when {@code condition} holds {@code false}. *

* A currently observed binding will observe its source, which means it will not be eligible - * for garbage collection while source isn't. However, using {@code when} this {@code ObservableValue} - * can still be eligible for garbage collection when the condition is {@code false} and the - * conditional itself is also eligible for garbage collection. + * for garbage collection while its source isn't. However, when using {@code when} this {@code ObservableValue} + * can still be eligible for garbage collection when {@code condition} holds {@code false} and {@code condition} + * itself is also eligible for garbage collection. *

- * Returning {@code null} from the given condition is treated the same as - * returning {@code false}. + * A {@code condition} holding {@code null} is treated as holding {@code false}. *

* For example: * 

{@code
      * ObservableValue condition = new SimpleBooleanProperty(true);
-     * ObservableValue globalProperty = new SimpleStringProperty("A");
-     * ObservableValue whenProperty = property.when(condition);
+     * ObservableValue longLivedProperty = new SimpleStringProperty("A");
+     * ObservableValue whenProperty = longLivedProperty.when(condition);
      *
-     * // observe whenProperty, which will in turn observe globalProperty
+     * // observe whenProperty, which will in turn observe longLivedProperty
      * whenProperty.addChangeListener((ov, old, current) -> System.out.println(current));
      *
-     * globalProperty.setValue("B");  // "B" is printed
+     * longLivedProperty.setValue("B");  // "B" is printed
      *
      * condition.setValue(false);
      *
-     * // After condition becomes false, whenProperty stops observing globalProperty; condition
+     * // After condition becomes false, whenProperty stops observing longLivedProperty; condition
      * // and whenProperty may now be eligible for GC despite being observed by the ChangeListener
      *
-     * globalProperty.setValue("C");  // nothing is printed
-     * globalProperty.setValue("D");  // nothing is printed
+     * longLivedProperty.setValue("C");  // nothing is printed
+     * longLivedProperty.setValue("D");  // nothing is printed
      *
-     * condition.setValue(true);  // globalProperty is observed again, and "D" is printed
+     * condition.setValue(true);  // longLivedProperty is observed again, and "D" is printed
      * }
- * Another example: + * An example for binding a label's text to a long-lived property only when it is shown: * 
{@code
      * Label label = ... ;
-     * ObservableValue globalProperty = new SimpleStringProperty("A");
-     *
-     * // bind label's text to a global property only when it is shown:
-     * label.textProperty().bind(globalProperty.when(label::isShownProperty));
+     * ObservableValue longLivedProperty = new SimpleStringProperty("A");
+     * label.textProperty().bind(longLivedProperty.when(label::isShownProperty));
      * }
* @param condition a boolean {@code ObservableValue}, cannot be {@code null} * @return an {@code ObservableValue} that holds this value whenever the given From 13c6e39a83278a13a827df0a93c14ebfdda31912 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Sun, 4 Sep 2022 21:42:30 +0200 Subject: [PATCH 06/16] Small wording fixes --- .../main/java/javafx/beans/value/ObservableValue.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java index 3612ad03409..5a25b6fdead 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java +++ b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java @@ -261,11 +261,8 @@ default ObservableValue flatMap(Function - * A currently observed binding will observe its source, which means it will not be eligible - * for garbage collection while its source isn't. However, when using {@code when} this {@code ObservableValue} - * can still be eligible for garbage collection when {@code condition} holds {@code false} and {@code condition} - * itself is also eligible for garbage collection. + * This is in contrast to the general behavior of bindings, where the binding is + * only eligible for garbage collection when not observed itself. *

* A {@code condition} holding {@code null} is treated as holding {@code false}. *

@@ -276,7 +273,7 @@ default ObservableValue flatMap(Function whenProperty = longLivedProperty.when(condition); * * // observe whenProperty, which will in turn observe longLivedProperty - * whenProperty.addChangeListener((ov, old, current) -> System.out.println(current)); + * whenProperty.addListener((ov, old, current) -> System.out.println(current)); * * longLivedProperty.setValue("B"); // "B" is printed * From 23538520f70c8d569ac75662d97874b9f3b643de Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Thu, 22 Sep 2022 12:59:22 +0200 Subject: [PATCH 07/16] Add missing new line --- .../src/main/java/javafx/beans/value/ObservableValue.java | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java index 5a25b6fdead..29beac4eb40 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java +++ b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java @@ -293,6 +293,7 @@ default ObservableValue flatMap(Function longLivedProperty = new SimpleStringProperty("A"); * label.textProperty().bind(longLivedProperty.when(label::isShownProperty)); * } + * * @param condition a boolean {@code ObservableValue}, cannot be {@code null} * @return an {@code ObservableValue} that holds this value whenever the given * condition evaluates to {@code true}, otherwise holds the last seen value; From 44d2ced84b2fa6520fabb7588926c8efc3263cd9 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Thu, 29 Sep 2022 16:51:20 +0200 Subject: [PATCH 08/16] Fix review comments --- .../java/com/sun/javafx/binding/ConditionalBinding.java | 6 +++--- .../java/test/javafx/beans/value/LazyObjectBindingTest.java | 4 +--- .../beans/value/ObservableValueFluentBindingsTest.java | 1 + 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java b/modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java index 693eeaecc9a..c77cd8bac7f 100644 --- a/modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java +++ b/modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java @@ -12,8 +12,8 @@ public class ConditionalBinding extends LazyObjectBinding { private Subscription subscription; public ConditionalBinding(ObservableValue source, ObservableValue condition) { - this.source = Objects.requireNonNull(source, "source"); - this.nonNullCondition = Objects.requireNonNull(condition, "condition").orElse(false); + this.source = Objects.requireNonNull(source, "source cannot be null"); + this.nonNullCondition = Objects.requireNonNull(condition, "condition cannot be null").orElse(false); // condition is always observed and never unsubscribed Subscription.subscribe(nonNullCondition, current -> { @@ -38,7 +38,7 @@ protected boolean allowValidation() { @Override protected T computeValue() { if (isObserved() && isActive()) { - if(subscription == null) { + if (subscription == null) { subscription = Subscription.subscribeInvalidations(source, this::invalidate); } } diff --git a/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java b/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java index 23a630c0673..81e616b3cdf 100644 --- a/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java +++ b/modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java @@ -61,9 +61,7 @@ protected String computeValue() { } }; - binding.addListener(obs -> { - assertEquals("A", binding.get()); - }); + binding.addListener(obs -> assertEquals("A", binding.get())); binding.invalidate(); // becomes valid again immediately diff --git a/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java b/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java index 18c4c8fd1fd..e7e73a3fedf 100644 --- a/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java +++ b/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java @@ -49,6 +49,7 @@ import javafx.beans.value.ObservableValue; public class ObservableValueFluentBindingsTest { + private int invalidations; private final List values = new ArrayList<>(); From ffba692d05ee7427bffa19bb461d66a282d07400 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Wed, 26 Oct 2022 09:18:12 +0200 Subject: [PATCH 09/16] Improve documentation of shown property --- .../src/main/java/javafx/scene/Node.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index 1469fafdd35..44a7887145f 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -1406,15 +1406,18 @@ public String getName() { } /** - * Whether or not this {@code Node} is showing (that is, it is part of an - * open Window on the user's system). The Window might be "showing", yet the user might not - * be able to see it due to the Window being rendered behind another Window - * or due to the Window being positioned off the monitor. - * - *

Note that the {@code Node} does not need to be visible for this property - * to be {@code true}. + * Indicates whether or not this {@code Node} is shown. A node is considered shown if it's + * part of a {@code Scene} which is part of a {@code Window} whose + * {@link shown #Window::showingProperty} is {@code true}. The {@link visibility #visibleProperty} + * of the node or its scene do not affect this property. + *

+ * This property can be used in conjunction with {@link ObservableValue#when} to + * create bindings which are only actively listening to their source when the node is shown. + *

+ * This property can also be used to start animations when the node is shown, and to stop them + * when it is no longer shown. * - * @defaultValue false + * @see ObservableValue#when(ObservableValue) * @since 20 */ private ReadOnlyBooleanProperty shown; @@ -1430,7 +1433,7 @@ public final ReadOnlyBooleanProperty shownProperty() { if (shown == null) { ObservableValue ov = sceneProperty() .flatMap(Scene::windowProperty) - .flatMap(Window::showingProperty); + .flatMap(Window::showingProperty); // note: the null case is handled by the delegate with an orElse(false) shown = new ReadOnlyBooleanDelegate(Node.this, "shown", ov); } From ea5469984e5bb4a7a2e8356312d7611b3df46418 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Wed, 26 Oct 2022 09:18:26 +0200 Subject: [PATCH 10/16] Fix comment in test --- .../src/test/java/test/javafx/scene/NodeTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java index 03095fcbb48..a4176c902cb 100644 --- a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java +++ b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java @@ -1279,7 +1279,7 @@ public void testIsShowingProperty() { assertTrue(g.shownProperty().get()); assertTrue(c.shownProperty().get()); - g.setVisible(false); // irrelevant change for isShowing + g.setVisible(false); // irrelevant change for isShown SceneShim.scenePulseListener_pulse(s); assertTrue(g.isShown()); From 46f206aa2c7479631fc91b079449636429dffacc Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Wed, 26 Oct 2022 09:53:12 +0200 Subject: [PATCH 11/16] Fix javadoc error --- modules/javafx.graphics/src/main/java/javafx/scene/Node.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index 44a7887145f..5a76a5becc6 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -1408,8 +1408,8 @@ public String getName() { /** * Indicates whether or not this {@code Node} is shown. A node is considered shown if it's * part of a {@code Scene} which is part of a {@code Window} whose - * {@link shown #Window::showingProperty} is {@code true}. The {@link visibility #visibleProperty} - * of the node or its scene do not affect this property. + * {@link Window#showingProperty showing property} is {@code true}. The {@link Node#visibleProperty visibility} + * of the node or its scene does not affect this property. *

* This property can be used in conjunction with {@link ObservableValue#when} to * create bindings which are only actively listening to their source when the node is shown. From 5c22fdd04de21a0402fefd3ee3887ea9ee3b3daf Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Thu, 1 Dec 2022 18:34:19 +0100 Subject: [PATCH 12/16] Adjust Node - Fixed javadoc - Added comment for code that avoid eager instantiation - Changed `isShown` to use property if it is available --- .../src/main/java/javafx/scene/Node.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index 5a76a5becc6..7881e485b83 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -1201,6 +1201,7 @@ public final void setId(String value) { * @defaultValue null * @see CSS Reference Guide */ + @Override public final String getId() { return id == null ? null : id.get(); } @@ -1310,6 +1311,7 @@ public final void setStyle(String value) { * an empty String is returned. * @see CSS Reference Guide */ + @Override public final String getStyle() { return style == null ? "" : style.get(); } @@ -1407,7 +1409,7 @@ public String getName() { /** * Indicates whether or not this {@code Node} is shown. A node is considered shown if it's - * part of a {@code Scene} which is part of a {@code Window} whose + * part of a {@code Scene} that is part of a {@code Window} whose * {@link Window#showingProperty showing property} is {@code true}. The {@link Node#visibleProperty visibility} * of the node or its scene does not affect this property. *

@@ -1417,16 +1419,19 @@ public String getName() { * This property can also be used to start animations when the node is shown, and to stop them * when it is no longer shown. * - * @see ObservableValue#when(ObservableValue) * @since 20 */ private ReadOnlyBooleanProperty shown; public final boolean isShown() { - Scene s = getScene(); - if (s == null) return false; - Window w = s.getWindow(); - return w != null && w.isShowing(); + if (shown == null) { // avoid eager instantiation of property + Scene s = getScene(); + if (s == null) return false; + Window w = s.getWindow(); + return w != null && w.isShowing(); + } + + return shown.get(); } public final ReadOnlyBooleanProperty shownProperty() { From 67c277cba289c0c6651f432c1802cdacdc22c400 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Fri, 2 Dec 2022 10:52:52 +0100 Subject: [PATCH 13/16] Improve wording in javadoc and comments --- .../javafx.graphics/src/main/java/javafx/scene/Node.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index 7881e485b83..0e44ce16669 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -1410,14 +1410,14 @@ public String getName() { /** * Indicates whether or not this {@code Node} is shown. A node is considered shown if it's * part of a {@code Scene} that is part of a {@code Window} whose - * {@link Window#showingProperty showing property} is {@code true}. The {@link Node#visibleProperty visibility} + * {@link Window#showingProperty showing} property is {@code true}. The {@link Node#visibleProperty visibility} * of the node or its scene does not affect this property. *

* This property can be used in conjunction with {@link ObservableValue#when} to * create bindings which are only actively listening to their source when the node is shown. *

- * This property can also be used to start animations when the node is shown, and to stop them - * when it is no longer shown. + * This property can also be useful to perform actions when the node is shown or no longer + * shown, like starting and stopping animations * * @since 20 */ @@ -1446,7 +1446,7 @@ public final ReadOnlyBooleanProperty shownProperty() { return shown; } - // Candidate to make publicly available or to add as a convience method to ObservableValue + // Candidate to make publicly available or to add as a convenience method to ObservableValue private static class ReadOnlyBooleanDelegate extends ReadOnlyBooleanProperty { private final ObservableValue delegate; private final Object bean; From 94b50b5d2e9c742c75420560580e0bb9eb62b088 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Sun, 11 Dec 2022 21:24:37 +0100 Subject: [PATCH 14/16] Remove changes to javafx.graphics Node --- .../src/main/java/javafx/scene/Node.java | 99 ++----------------- .../test/java/test/javafx/scene/NodeTest.java | 57 ----------- 2 files changed, 8 insertions(+), 148 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index 0e44ce16669..c25e0df571b 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -50,7 +50,6 @@ import javafx.beans.property.StringProperty; import javafx.beans.property.StringPropertyBase; import javafx.beans.value.ChangeListener; -import javafx.beans.value.ObservableValue; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener.Change; import javafx.collections.ObservableList; @@ -1201,7 +1200,6 @@ public final void setId(String value) { * @defaultValue null * @see CSS Reference Guide */ - @Override public final String getId() { return id == null ? null : id.get(); } @@ -1311,7 +1309,6 @@ public final void setStyle(String value) { * an empty String is returned. * @see CSS Reference Guide */ - @Override public final String getStyle() { return style == null ? "" : style.get(); } @@ -1407,93 +1404,6 @@ public String getName() { return visible; } - /** - * Indicates whether or not this {@code Node} is shown. A node is considered shown if it's - * part of a {@code Scene} that is part of a {@code Window} whose - * {@link Window#showingProperty showing} property is {@code true}. The {@link Node#visibleProperty visibility} - * of the node or its scene does not affect this property. - *

- * This property can be used in conjunction with {@link ObservableValue#when} to - * create bindings which are only actively listening to their source when the node is shown. - *

- * This property can also be useful to perform actions when the node is shown or no longer - * shown, like starting and stopping animations - * - * @since 20 - */ - private ReadOnlyBooleanProperty shown; - - public final boolean isShown() { - if (shown == null) { // avoid eager instantiation of property - Scene s = getScene(); - if (s == null) return false; - Window w = s.getWindow(); - return w != null && w.isShowing(); - } - - return shown.get(); - } - - public final ReadOnlyBooleanProperty shownProperty() { - if (shown == null) { - ObservableValue ov = sceneProperty() - .flatMap(Scene::windowProperty) - .flatMap(Window::showingProperty); // note: the null case is handled by the delegate with an orElse(false) - - shown = new ReadOnlyBooleanDelegate(Node.this, "shown", ov); - } - - return shown; - } - - // Candidate to make publicly available or to add as a convenience method to ObservableValue - private static class ReadOnlyBooleanDelegate extends ReadOnlyBooleanProperty { - private final ObservableValue delegate; - private final Object bean; - private final String name; - - ReadOnlyBooleanDelegate(Object bean, String name, ObservableValue delegate) { - this.bean = bean; - this.name = name; - this.delegate = delegate.orElse(false); - } - - @Override - public Object getBean() { - return bean; - } - - @Override - public String getName() { - return name; - } - - @Override - public void addListener(ChangeListener listener) { - delegate.addListener(listener); - } - - @Override - public void removeListener(ChangeListener listener) { - delegate.removeListener(listener); - } - - @Override - public void addListener(InvalidationListener listener) { - delegate.addListener(listener); - } - - @Override - public void removeListener(InvalidationListener listener) { - delegate.removeListener(listener); - } - - @Override - public boolean get() { - return delegate.getValue(); // orElse guarantees this is never null - } - } - public final void setCursor(Cursor value) { cursorProperty().set(value); } @@ -8603,8 +8513,15 @@ void markDirtyLayoutBranch() { } + private boolean isWindowShowing() { + Scene s = getScene(); + if (s == null) return false; + Window w = s.getWindow(); + return w != null && w.isShowing(); + } + final boolean isTreeShowing() { - return isTreeVisible() && isShown(); + return isTreeVisible() && isWindowShowing(); } private void updateTreeVisible(boolean parentChanged) { diff --git a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java index a4176c902cb..2752e0bddd5 100644 --- a/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java +++ b/modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java @@ -1255,63 +1255,6 @@ public void testIsTreeVisible() { } - @Test - public void testIsShowingProperty() { - final Group g = new Group(); - final Circle c = new CircleTest.StubCircle(50); - - ParentShim.getChildren(g).add(c); - - Scene s = new Scene(g); - Stage st = new Stage(); - - assertFalse(g.isShown()); - assertFalse(c.isShown()); - assertFalse(g.shownProperty().get()); - assertFalse(c.shownProperty().get()); - - st.show(); - st.setScene(s); - SceneShim.scenePulseListener_pulse(s); - - assertTrue(g.isShown()); - assertTrue(c.isShown()); - assertTrue(g.shownProperty().get()); - assertTrue(c.shownProperty().get()); - - g.setVisible(false); // irrelevant change for isShown - SceneShim.scenePulseListener_pulse(s); - - assertTrue(g.isShown()); - assertTrue(c.isShown()); - assertTrue(g.shownProperty().get()); - assertTrue(c.shownProperty().get()); - - s.setRoot(new Group()); - SceneShim.scenePulseListener_pulse(s); - - assertFalse(g.isShown()); - assertFalse(c.isShown()); - assertFalse(g.shownProperty().get()); - assertFalse(c.shownProperty().get()); - - s.setRoot(g); - SceneShim.scenePulseListener_pulse(s); - - assertTrue(g.isShown()); - assertTrue(c.isShown()); - assertTrue(g.shownProperty().get()); - assertTrue(c.shownProperty().get()); - - st.hide(); - SceneShim.scenePulseListener_pulse(s); - - assertFalse(g.isShown()); - assertFalse(c.isShown()); - assertFalse(g.shownProperty().get()); - assertFalse(c.shownProperty().get()); - } - @Test public void testSynchronizationOfInvisibleNodes_2() { final Group g = new Group(); From 75ccca6b03613dfd42c304e014b422c9c2cafb6e Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Sun, 11 Dec 2022 21:26:13 +0100 Subject: [PATCH 15/16] Remove example referencing Node#shownProperty --- .../src/main/java/javafx/beans/value/ObservableValue.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java index fb54e865889..ddaf4acf1e8 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java +++ b/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java @@ -287,12 +287,6 @@ default ObservableValue flatMap(Function - * An example for binding a label's text to a long-lived property only when it is shown: - * 

{@code
-     * Label label = ... ;
-     * ObservableValue longLivedProperty = new SimpleStringProperty("A");
-     * label.textProperty().bind(longLivedProperty.when(label::isShownProperty));
-     * }
* * @param condition a boolean {@code ObservableValue}, cannot be {@code null} * @return an {@code ObservableValue} that holds this value whenever the given From 7318af2b9e23d3facafb82f3e7a54f04e05bd3b5 Mon Sep 17 00:00:00 2001 From: John Hendrikx Date: Mon, 12 Dec 2022 23:36:21 +0100 Subject: [PATCH 16/16] Fix formatting in test --- .../javafx/beans/value/ObservableValueFluentBindingsTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java b/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java index 5e347fa398b..2ab9504e4b5 100644 --- a/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java +++ b/modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java @@ -928,7 +928,8 @@ void shouldReturnInitialValueAtTimeOfCreation() { @Nested class WithNotNullReturns_ObservableValue_Which { - private ObjectProperty condition = new SimpleObjectProperty(true); // using object property here so it can be set to null for testing + // using object property here so it can be set to null for testing + private ObjectProperty condition = new SimpleObjectProperty<>(true); private ObservableValue observableValue = property.when(condition); @Test