Browse Source

:arrow_up: Upgrade core module

Bindings Package
:recycle: Tests on Bindings where failing due to recent changes on When constructs. Since a Bindings may need to add/register a listener more than once on a same observable avoid using When constructs

Controls Package
:bug: BoundLabel: fixed text node detection algorithm

Observables Package
:memo: When: mention in documentation that only a When construct per Observable can be active at anytime
:sparkles: When: Added method to force the activation/listening by disposing any other When on the given Observable (if one exists already)
:sparkles: When: added method to check if a construct has been disposed before
:recycle: OnChanged, OnInvalidated: also set the Observable to null on disposal

Utils Package
:recycle: LayoutUtils: optimize some of the algorithms by not computing the snapped margins if they are EMPTY
:recycle: LayoutUtils: snap the computed sizes by default before any other calculation as this could lead to wrong pixel positions

:white_check_mark: Added some tests for When constructs

Signed-off-by: Alessadro Parisi <alessandro.parisi406@gmail.com>
Alessadro Parisi 2 years ago
parent
commit
c32c49754d

+ 12 - 0
modules/core/CHANGELOG.md

@@ -26,6 +26,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   overhead
 - Added a new method responsible for registering the When construct. This is responsible for avoiding duplicates in the
   map (subsequent put) that would probably lead to a memory leak and unexpected behavior
+- When: mention in documentation that only a When construct per Observable can be active at anytime
+- When: Added method to force the activation/listening by disposing any other When on the given Observable (if one
+  exists already)
+- When: added method to check if a construct has been disposed before
+- Added some tests for When constructs
 
 ## Changed
 
@@ -33,10 +38,17 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   and the Behavior
 - Moved executeNow(...) methods to the base class for a better fluent API
 - When: added non-static method for disposal
+- Tests on Bindings where failing due to recent changes on When constructs. Since a Bindings may need to add/register a
+  listener more than once on a same observable avoid using When constructs
+- OnChanged, OnInvalidated: also set the Observable to null on disposal
+- LayoutUtils: optimize some of the algorithms by not computing the snapped margins if they are EMPTY
+- LayoutUtils: snap the computed sizes by default before any other calculation as this could lead to wrong pixel
+  positions
 
 ## Fixed
 
 - When: Implemented fix from #212
+- BoundLabel: fixed text node detection algorithm
 
 ## [11.2.4] - 09-02-2023
 

+ 6 - 2
modules/core/src/main/java/io/github/palexdev/mfxcore/base/bindings/AbstractSource.java

@@ -19,13 +19,14 @@
 package io.github.palexdev.mfxcore.base.bindings;
 
 import io.github.palexdev.mfxcore.base.bindings.base.ISource;
+import javafx.beans.value.ChangeListener;
 import javafx.beans.value.ObservableValue;
 
 /**
  * Base class for all types of {@code Sources}, implements {@link ISource}.
  * <p></p>
- * This class allows to have common properties in one place (such as the source's observable and the target), but also
- * implements common methods and defines methods that need to be used only internally.
+ * This class allows to have common properties in one place (such as the source's observable, the target, and listeners),
+ * but also implements common methods and defines methods that need to be used only internally.
  *
  * @param <S> the type of the source's observable
  * @param <T> the type of the target's observable
@@ -37,6 +38,9 @@ public abstract class AbstractSource<S, T> implements ISource<S, T> {
 	protected ObservableValue<? extends S> observable;
 	protected Target<T> target;
 
+	protected ChangeListener<? super S> obvListener;
+	protected ChangeListener<? super T> tgtListener;
+
 	//================================================================================
 	// Constructors
 	//================================================================================

+ 6 - 7
modules/core/src/main/java/io/github/palexdev/mfxcore/base/bindings/ExternalSource.java

@@ -19,7 +19,6 @@
 package io.github.palexdev.mfxcore.base.bindings;
 
 import io.github.palexdev.mfxcore.base.bindings.base.Updater;
-import io.github.palexdev.mfxcore.observables.When;
 import javafx.beans.value.ObservableValue;
 
 import java.util.Objects;
@@ -75,9 +74,8 @@ public class ExternalSource<S> extends AbstractSource<S, S> {
 	 */
 	@Override
 	protected void listen() {
-		When.onChanged(observable)
-				.then(action::update)
-				.listen();
+		if (obvListener == null) obvListener = (ov, o, n) -> action.update(o, n);
+		observable.addListener(obvListener);
 	}
 
 	/**
@@ -113,13 +111,14 @@ public class ExternalSource<S> extends AbstractSource<S, S> {
 	}
 
 	/**
-	 * Disposes the source by using {@link When#disposeFor(ObservableValue)},
-	 * then sets the observable to null.
+	 * Disposes the source by removing the listener, then sets both the observable
+	 * and listener to null.
 	 */
 	@Override
 	public void dispose() {
-		When.disposeFor(observable);
+		observable.removeListener(obvListener);
 		observable = null;
+		obvListener = null;
 	}
 
 	/**

+ 14 - 15
modules/core/src/main/java/io/github/palexdev/mfxcore/base/bindings/MappingSource.java

@@ -20,7 +20,6 @@ package io.github.palexdev.mfxcore.base.bindings;
 
 import io.github.palexdev.mfxcore.base.bindings.base.Updater;
 import io.github.palexdev.mfxcore.enums.BindingType;
-import io.github.palexdev.mfxcore.observables.When;
 import javafx.beans.value.ObservableValue;
 
 /**
@@ -113,47 +112,47 @@ public class MappingSource<S, T> extends AbstractSource<S, T> {
 	/**
 	 * {@inheritDoc}
 	 * <p>
-	 * For unidirectional bindings. The listener to this source's observable is added by using
-	 * {@link When#onChanged(ObservableValue)} and is responsible for triggering {@link #updateTarget(Object, Object)}.
+	 * For unidirectional bindings. The listener added to this source's observable is responsible for
+	 * triggering {@link #updateTarget(Object, Object)}.
 	 */
 	@Override
 	protected void listen() {
-		When.onChanged(observable)
-				.then(this::updateTarget)
-				.listen();
+		if (obvListener == null) obvListener = (ov, o, n) -> updateTarget(o, n);
+		observable.addListener(obvListener);
 	}
 
 	/**
 	 * {@inheritDoc}
 	 * <p>
 	 * For bidirectional bindings. The source's target is set to the given one.
-	 * Then {@link #listen()} is called. Then a listener to the given target is added by using
-	 * {@link When#onChanged(ObservableValue)} and is responsible for triggering {@link #updateSource(Object, Object)}.
+	 * Then {@link #listen()} is called. Then a listener to the given target is added and is responsible for
+	 * triggering {@link #updateSource(Object, Object)}.
 	 */
 	@Override
 	protected void listen(Target<T> target) {
 		listen();
 
 		this.target = target;
-		When.onChanged(target.getObservable())
-				.then(this::updateSource)
-				.listen();
+		if (tgtListener == null) tgtListener = (ov, o, n) -> updateSource(o, n);
+		target.getObservable().addListener(tgtListener);
 	}
 
 	/**
 	 * {@inheritDoc}
 	 * <p>
-	 * Uses {@link When#disposeFor(ObservableValue)} to remove the listeners added to this source's observable
-	 * and the target's observable. Then sets all its properties to null.
+	 * Removes the listeners added to this source's observable and the target's observable (if not null).
+	 * Then sets all its properties and listeners to null.
 	 */
 	@Override
 	public void dispose() {
-		When.disposeFor(observable);
-		if (target != null) When.disposeFor(target.getObservable());
+		observable.removeListener(obvListener);
+		if (target != null && tgtListener != null) target.getObservable().removeListener(tgtListener);
 		observable = null;
 		target = null;
 		targetUpdater = null;
 		sourceUpdater = null;
+		obvListener = null;
+		tgtListener = null;
 	}
 
 	//================================================================================

+ 14 - 15
modules/core/src/main/java/io/github/palexdev/mfxcore/base/bindings/Source.java

@@ -20,7 +20,6 @@ package io.github.palexdev.mfxcore.base.bindings;
 
 import io.github.palexdev.mfxcore.base.bindings.base.Updater;
 import io.github.palexdev.mfxcore.enums.BindingType;
-import io.github.palexdev.mfxcore.observables.When;
 import javafx.beans.value.ObservableValue;
 
 /**
@@ -110,47 +109,47 @@ public class Source<S> extends AbstractSource<S, S> {
 	/**
 	 * {@inheritDoc}
 	 * <p>
-	 * For unidirectional bindings. The listener to this source's observable is added by using
-	 * {@link When#onChanged(ObservableValue)} and is responsible for triggering {@link #updateTarget(Object, Object)}.
+	 * For unidirectional bindings. The listener added to this source's observable is responsible for
+	 * triggering {@link #updateTarget(Object, Object)}.
 	 */
 	@Override
 	protected void listen() {
-		When.onChanged(observable)
-				.then(this::updateTarget)
-				.listen();
+		if (obvListener == null) obvListener = (ov, o, n) -> updateTarget(o, n);
+		observable.addListener(obvListener);
 	}
 
 	/**
 	 * {@inheritDoc}
 	 * <p>
 	 * For bidirectional bindings. The source's target is set to the given one.
-	 * Then {@link #listen()} is called. Then a listener to the given target is added by using
-	 * {@link When#onChanged(ObservableValue)} and is responsible for triggering {@link #updateSource(Object, Object)}.
+	 * Then {@link #listen()} is called. Then a listener to the given target is added and is responsible for
+	 * triggering {@link #updateSource(Object, Object)}.
 	 */
 	@Override
 	protected void listen(Target<S> target) {
 		listen();
 
 		this.target = target;
-		When.onChanged(target.getObservable())
-				.then(this::updateSource)
-				.listen();
+		if (tgtListener == null) tgtListener = (ov, o, n) -> updateSource(o, n);
+		target.getObservable().addListener(tgtListener);
 	}
 
 	/**
 	 * {@inheritDoc}
 	 * <p>
-	 * Uses {@link When#disposeFor(ObservableValue)} to remove the listeners added to this source's observable
-	 * and the target's observable. Then sets all its properties to null.
+	 * Removes the listeners added to this source's observable and the target's observable (if not null).
+	 * Then sets all its properties and listeners to null.
 	 */
 	@Override
 	public void dispose() {
-		When.disposeFor(observable);
-		if (target != null) When.disposeFor(target.getObservable());
+		observable.removeListener(obvListener);
+		if (target != null && tgtListener != null) target.getObservable().removeListener(tgtListener);
 		observable = null;
 		target = null;
 		targetUpdater = null;
 		sourceUpdater = null;
+		obvListener = null;
+		tgtListener = null;
 	}
 
 	//================================================================================

+ 7 - 1
modules/core/src/main/java/io/github/palexdev/mfxcore/controls/BoundLabel.java

@@ -127,7 +127,13 @@ public class BoundLabel extends Label {
 			@Override
 			protected void updateChildren() {
 				super.updateChildren();
-				if (textNode == null && getChildren().size() > 1) setTextNode(getChildren().get(1));
+				if (textNode != null) return;
+
+				if (getChildren().size() == 1 && getGraphic() == null) {
+					setTextNode(getChildren().get(0));
+				} else if (getChildren().size() > 1) {
+					setTextNode(getChildren().get(1));
+				}
 			}
 		};
 	}

+ 1 - 0
modules/core/src/main/java/io/github/palexdev/mfxcore/observables/OnChanged.java

@@ -219,6 +219,7 @@ public class OnChanged<T> extends When<T> {
 			observableValue.removeListener(listener);
 			listener = null;
 			whens.remove(observableValue);
+			observableValue = null;
 		}
 	}
 }

+ 1 - 0
modules/core/src/main/java/io/github/palexdev/mfxcore/observables/OnInvalidated.java

@@ -221,6 +221,7 @@ public class OnInvalidated<T> extends When<T> {
 			observableValue.removeListener(listener);
 			listener = null;
 			whens.remove(observableValue);
+			observableValue = null;
 		}
 	}
 }

+ 35 - 2
modules/core/src/main/java/io/github/palexdev/mfxcore/observables/When.java

@@ -40,16 +40,22 @@ import java.util.function.Supplier;
  * This construct also allows to define one-shot listeners, meaning that the
  * above phrase changes like this: "When condition changes, then do this, then dispose(remove listener)"
  * <p></p>
- * <b>Note:</b>Once the construct is not needed anymore it's highly recommended to dispose it
+ * <b>Note 1: </b>Once the construct is not needed anymore it's highly recommended to dispose it
  * using {@link #disposeFor(ObservableValue)} to avoid memory leaks.
  * When constructs and ObservableValues are stored in a {@link WeakHashMap} for this purpose.
+ * <p></p>
+ * <b>Note 2: </b> Every {@link ObservableValue} can have at max one When construct listening to it, attempts at
+ * registering more than one will result with an exception thrown by {@link #register()}.
+ * <p>
+ * One can also build a construct using {@link #forceListen()}, this will lead to the disposal of the previously built
+ * construct (if any was built).
  */
 public abstract class When<T> {
 	//================================================================================
 	// Properties
 	//================================================================================
 	protected static final WeakHashMap<ObservableValue<?>, WeakReference<When<?>>> whens = new WeakHashMap<>();
-	protected final ObservableValue<T> observableValue;
+	protected ObservableValue<T> observableValue;
 	protected boolean oneShot = false;
 
 	protected final Set<Observable> invalidatingObservables;
@@ -68,12 +74,28 @@ public abstract class When<T> {
 	//================================================================================
 	// Abstract Methods
 	//================================================================================
+
+	/**
+	 * Implementations of this should provide the logic that adds the listener on the given {@link ObservableValue},
+	 * as well as handling cases such {@link #oneShot()} and {@link #invalidating(Observable)} as well as making sure that
+	 * the construct is registered at the end, {@link #register()}.
+	 */
 	public abstract When<T> listen();
 
 	//================================================================================
 	// Methods
 	//================================================================================
 
+	/**
+	 * Forces the creation/activation of this {@code When} construct by first checking if there's already one for the
+	 * current {@link ObservableValue} by disposing it first and then calling {@link #listen()}.
+	 */
+	public When<T> forceListen() {
+		if (whens.containsKey(observableValue))
+			disposeFor(observableValue);
+		return listen();
+	}
+
 	/**
 	 * This is responsible for registering the {@code When} construct in a map that
 	 * keep references to all the built constructs. This is to avoid garbage collection and to
@@ -158,6 +180,17 @@ public abstract class When<T> {
 		disposeFor(observableValue);
 	}
 
+	/**
+	 * @return whether this construct has been disposed before. By default, checks if the given {@link ObservableValue}
+	 * is null, there are no invalidating sources and the invalidation listener is null. A construct is considered to be
+	 * properly disposed only when all these conditions are verified
+	 */
+	public boolean isDisposed() {
+		return observableValue == null &&
+				invalidatingObservables.isEmpty() &&
+				invalidationListener == null;
+	}
+
 	//================================================================================
 	// Static Methods
 	//================================================================================

+ 5 - 5
modules/core/src/main/java/io/github/palexdev/mfxcore/utils/fx/LayoutUtils.java

@@ -41,7 +41,7 @@ public class LayoutUtils {
 	                                       double areaBaselineOffset, Insets margin, HPos hAlignment, VPos vAlignment, boolean snapToPixel, boolean computeSizes) {
 
 		Insets snappedMargin = margin == null ? Insets.EMPTY : margin;
-		if (snapToPixel) {
+		if (snapToPixel && snappedMargin != Insets.EMPTY) {
 			snappedMargin = InsetsBuilder.of(
 					parent.snapSpaceY(snappedMargin.getTop()),
 					parent.snapSpaceX(snappedMargin.getRight()),
@@ -57,7 +57,7 @@ public class LayoutUtils {
 
 	public static double computeXPosition(Region parent, Node child, double areaX, double areaWidth, Insets margin, boolean snapMargin, HPos hAlignment, boolean snapToPixel, boolean computeSizes) {
 		Insets snappedMargin = margin == null ? Insets.EMPTY : margin;
-		if (snapMargin) {
+		if (snapMargin && snappedMargin != Insets.EMPTY) {
 			snappedMargin = InsetsBuilder.of(
 					parent.snapSpaceY(snappedMargin.getTop()),
 					parent.snapSpaceX(snappedMargin.getRight()),
@@ -68,7 +68,7 @@ public class LayoutUtils {
 
 		final double leftMargin = snappedMargin.getLeft();
 		final double rightMargin = snappedMargin.getRight();
-		final double xOffset = leftMargin + computeXOffset(areaWidth - leftMargin - rightMargin, computeSizes ? boundWidth(child) : child.getLayoutBounds().getWidth(), hAlignment);
+		final double xOffset = leftMargin + computeXOffset(areaWidth - leftMargin - rightMargin, computeSizes ? parent.snapSizeX(boundWidth(child)) : child.getLayoutBounds().getWidth(), hAlignment);
 		final double xPosition = areaX + xOffset;
 		return snapToPixel ? parent.snapPositionX(xPosition) : xPosition;
 	}
@@ -90,12 +90,12 @@ public class LayoutUtils {
 		if (vAlignment == VPos.BASELINE) {
 			double bo = child.getBaselineOffset();
 			if (bo == Node.BASELINE_OFFSET_SAME_AS_HEIGHT) {
-				yOffset = areaBaselineOffset - (computeSizes ? boundHeight(child) : child.getLayoutBounds().getHeight());
+				yOffset = areaBaselineOffset - (computeSizes ? parent.snapSizeY(boundHeight(child)) : child.getLayoutBounds().getHeight());
 			} else {
 				yOffset = areaBaselineOffset - bo;
 			}
 		} else {
-			yOffset = topMargin + computeYOffset(areaHeight - topMargin - bottomMargin, computeSizes ? boundHeight(child) : child.getLayoutBounds().getHeight(), vAlignment);
+			yOffset = topMargin + computeYOffset(areaHeight - topMargin - bottomMargin, computeSizes ? parent.snapSizeY(boundHeight(child)) : child.getLayoutBounds().getHeight(), vAlignment);
 		}
 		final double yPosition = areaY + yOffset;
 		return snapToPixel ? parent.snapPositionY(yPosition) : yPosition;

+ 64 - 0
modules/core/src/test/java/unit/observables/WhenTests.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2023 Parisi Alessandro - alessandro.parisi406@gmail.com
+ * This file is part of MaterialFX (https://github.com/palexdev/MaterialFX)
+ *
+ * MaterialFX is free software: you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 3 of the License,
+ * or (at your option) any later version.
+ *
+ * MaterialFX is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with MaterialFX. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package unit.observables;
+
+import io.github.palexdev.mfxcore.observables.OnInvalidated;
+import io.github.palexdev.mfxcore.observables.When;
+import javafx.beans.property.IntegerProperty;
+import javafx.beans.property.SimpleIntegerProperty;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class WhenTests {
+
+	@Test
+	void testMultipleFail() {
+		IntegerProperty ip = new SimpleIntegerProperty();
+
+		When.onInvalidated(ip)
+				.then(System.out::println)
+				.listen();
+
+		assertThrows(
+				IllegalArgumentException.class,
+				() -> When.onInvalidated(ip)
+						.then(n -> System.out.println("This should not happen"))
+						.listen()
+		);
+	}
+
+	@Test
+	void testMultipleForce() {
+		IntegerProperty ip = new SimpleIntegerProperty();
+
+		OnInvalidated<Number> first = When.onInvalidated(ip)
+				.then(System.out::println)
+				.listen();
+
+		assertDoesNotThrow(
+				() -> {
+					When.onInvalidated(ip)
+							.then(n -> System.out.println("This should not happen"))
+							.forceListen();
+				}
+		);
+		assertTrue(first.isDisposed());
+	}
+}