diff --git a/src/main/java/org/fxmisc/flowless/PausableSuccessionStream.java b/src/main/java/org/fxmisc/flowless/PausableSuccessionStream.java new file mode 100644 index 0000000..3be0e97 --- /dev/null +++ b/src/main/java/org/fxmisc/flowless/PausableSuccessionStream.java @@ -0,0 +1,136 @@ +package org.fxmisc.flowless; + +import java.time.Duration; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.Predicate; + +import org.reactfx.AwaitingEventStream; +import org.reactfx.EventStream; +import org.reactfx.EventStreamBase; +import org.reactfx.Subscription; +import org.reactfx.util.FxTimer; +import org.reactfx.util.Timer; + +import javafx.beans.binding.BooleanBinding; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; +import javafx.beans.value.ObservableBooleanValue; + +class PausableSuccessionStream extends EventStreamBase implements AwaitingEventStream { + private final EventStream input; + private final Function initial; + private final BiFunction reduction; + private final Timer timer; + + private boolean hasEvent = false; + private BooleanBinding pending = null; + private BooleanProperty successionOff; + private Predicate successionOffCond; + private O event = null; + + /** + * Returns an event stream that, when events are emitted from this stream + * in close temporal succession, emits only the last event of the + * succession. What is considered a close temporal succession is + * defined by {@code timeout}: time gap between two successive events must + * be at most {@code timeout}. + * + *

Note: This function can be used only when this stream and + * the returned stream are used from the JavaFX application thread.

+ * + * @param timeout the maximum time difference between two subsequent events + * in close succession. + * @param realTime when true immediately emits the next event and sets + * realTime back to false. + */ + public PausableSuccessionStream( EventStream input, Duration timeout, BooleanProperty realTime ) + { + this( input, timeout, realTime, a -> realTime.get() ); + } + + /** + * @param timeout the maximum time difference between two subsequent events + * in close succession. + * @param condition when true immediately emits the event, otherwise + * waits for timeout before emitting the last received event. + */ + public PausableSuccessionStream( EventStream input, Duration timeout, Predicate condition ) + { + this( input, timeout, new SimpleBooleanProperty(), condition ); + } + + private PausableSuccessionStream( + EventStream input, + java.time.Duration timeout, + BooleanProperty realTime, + Predicate condition) { + + this.input = input; + this.initial = Function.identity(); + this.reduction = (a,b) -> b; + this.successionOff = realTime; + this.successionOffCond = condition; + + this.timer = FxTimer.create(timeout, this::handleTimeout); + } + + @Override + public ObservableBooleanValue pendingProperty() { + if(pending == null) { + pending = new BooleanBinding() { + @Override + protected boolean computeValue() { + return hasEvent; + } + }; + } + return pending; + } + + @Override + public boolean isPending() { + return pending != null ? pending.get() : hasEvent; + } + + @Override + protected final Subscription observeInputs() { + return input.subscribe(this::handleEvent); + } + + private void handleEvent(O i) { + timer.stop(); + if(successionOffCond.test(i)) + { + hasEvent = false; + event = null; + emit(i); + successionOff.setValue(false); + } + else + { + if(hasEvent) { + event = reduction.apply(event, i); + } else { + event = initial.apply(i); + hasEvent = true; + invalidatePending(); + } + timer.restart(); + } + } + + private void handleTimeout() { + hasEvent = false; + O toEmit = event; + event = null; + emit(toEmit); + invalidatePending(); + } + + private void invalidatePending() { + if(pending != null) { + pending.invalidate(); + } + } +} \ No newline at end of file diff --git a/src/main/java/org/fxmisc/flowless/ScaledVirtualized.java b/src/main/java/org/fxmisc/flowless/ScaledVirtualized.java index bc606b5..c8efd81 100644 --- a/src/main/java/org/fxmisc/flowless/ScaledVirtualized.java +++ b/src/main/java/org/fxmisc/flowless/ScaledVirtualized.java @@ -17,8 +17,8 @@ * VirtualizedScrollPane vsPane = new VirtualizedScrollPane(wrapper); * * // To scale actualContent without also scaling vsPane's scrollbars: - * wrapper.scaleProperty().setY(3); - * wrapper.scaleProperty().setX(2); + * wrapper.getZoom().setY(3); + * wrapper.getZoom().setX(2); * } * * @@ -51,13 +51,13 @@ public ScaledVirtualized(V content) { ); estScrollX = Var.mapBidirectional( content.estimatedScrollXProperty(), - scrollX -> scrollX * zoom.getX(), - scrollX -> scrollX / zoom.getX() + scrollX -> (double) Math.round( scrollX * zoom.getX() ), + scrollX -> (double) Math.round( scrollX / zoom.getX() ) ); estScrollY = Var.mapBidirectional( content.estimatedScrollYProperty(), - scrollY -> scrollY * zoom.getY(), - scrollY -> scrollY / zoom.getY() + scrollY -> (double) Math.round( scrollY * zoom.getY() ), + scrollY -> (double) Math.round( scrollY / zoom.getY() ) ); zoom.xProperty() .addListener((obs, ov, nv) -> requestLayout()); diff --git a/src/main/java/org/fxmisc/flowless/SizeTracker.java b/src/main/java/org/fxmisc/flowless/SizeTracker.java index 1896124..2f5529e 100644 --- a/src/main/java/org/fxmisc/flowless/SizeTracker.java +++ b/src/main/java/org/fxmisc/flowless/SizeTracker.java @@ -3,15 +3,19 @@ import java.time.Duration; import java.util.Optional; import java.util.function.Function; +import java.util.function.Supplier; +import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.value.ObservableObjectValue; import javafx.geometry.Bounds; import javafx.scene.control.IndexRange; +import org.reactfx.EventStream; import org.reactfx.EventStreams; import org.reactfx.Subscription; import org.reactfx.collection.LiveList; import org.reactfx.collection.MemoizationList; +import org.reactfx.util.Tuple3; import org.reactfx.value.Val; import org.reactfx.value.ValBase; @@ -56,9 +60,14 @@ public SizeTracker( this.viewportBounds = viewportBounds; this.cells = lazyCells; this.breadths = lazyCells.map(orientation::minBreadth).memoize(); - this.maxKnownMinBreadth = breadths.memoizedItems() - .reduce(Math::max) - .orElseConst(0.0); + LiveList knownBreadths = this.breadths.memoizedItems(); + + this.maxKnownMinBreadth = Val.create( + () -> knownBreadths.stream().mapToDouble( Double::doubleValue ).max().orElse(0.0), + // skips spurious events resulting from cell replacement (delete then add again) + knownBreadths.changes().successionEnds( Duration.ofMillis( 15 ) ) + ); + this.breadthForCells = Val.combine( maxKnownMinBreadth, viewportBounds, @@ -69,38 +78,53 @@ public SizeTracker( .map(breadth -> cell -> orientation.prefLength(cell, breadth)); this.lengths = cells.mapDynamic(lengthFn).memoize(); - LiveList knownLengths = this.lengths.memoizedItems(); - Val sumOfKnownLengths = knownLengths.reduce((a, b) -> a + b).orElseConst(0.0); - Val knownLengthCount = knownLengths.sizeProperty(); - - this.averageLengthEstimate = Val.create( - () -> { - // make sure to use pref lengths of all present cells - for(int i = 0; i < cells.getMemoizedCount(); ++i) { - int j = cells.indexOfMemoizedItem(i); - lengths.force(j, j + 1); - } - - int count = knownLengthCount.getValue(); - return count == 0 - ? null - : sumOfKnownLengths.getValue() / count; - }, - sumOfKnownLengths, knownLengthCount); - - this.totalLengthEstimate = Val.combine( - averageLengthEstimate, cells.sizeProperty(), - (avg, n) -> n * avg); + + Supplier averageKnownLengths = () -> { + // make sure to use pref lengths of all present cells + for(int i = 0; i < cells.getMemoizedCount(); ++i) { + int j = cells.indexOfMemoizedItem(i); + lengths.force(j, j + 1); + } + + return knownLengths.stream() + .mapToDouble( Double::doubleValue ) + .sorted().average() + .orElse( 0.0 ); + }; + + final int AVERAGE_LENGTH = 0, TOTAL_LENGTH = 1; + Val lengthStats = Val.wrap( + knownLengths.changes().or( cells.sizeProperty().values() ) + .successionEnds( Duration.ofMillis( 15 ) ) // reduce noise + .map( e -> { + double averageLength = averageKnownLengths.get(); + int cellCount = e.isRight() ? e.getRight() : cells.size(); + return new double[] { averageLength, cellCount * averageLength }; + } ).toBinding( new double[] { 0.0, 0.0 } ) + ); + + EventStream filteredLengthStats; + // briefly hold back changes that may be from spurious events coming from cell refreshes, these + // are identified as those where the estimated total length is less than the previous event. + filteredLengthStats = new PausableSuccessionStream<>( lengthStats.changes(), Duration.ofMillis(1000), chg -> { + double[/*average,total*/] oldStats = chg.getOldValue(); + double[/*average,total*/] newStats = chg.getNewValue(); + if ( newStats[TOTAL_LENGTH] < oldStats[TOTAL_LENGTH] ) { + return false; // don't emit yet, first wait & prefer newer values + } + return true; + } ) + .map( chg -> chg.getNewValue() ); + + this.averageLengthEstimate = Val.wrap( filteredLengthStats.map( stats -> stats[AVERAGE_LENGTH] ).toBinding( 0.0 ) ); + this.totalLengthEstimate = Val.wrap( filteredLengthStats.map( stats -> stats[TOTAL_LENGTH] ).toBinding( 0.0 ) ); Val firstVisibleIndex = Val.create( () -> cells.getMemoizedCount() == 0 ? null : cells.indexOfMemoizedItem(0), cells, cells.memoizedItems()); // need to observe cells.memoizedItems() // as well, because they may change without a change in cells. - Val> firstVisibleCell = cells.memoizedItems() - .collapse(visCells -> visCells.isEmpty() ? null : visCells.get(0)); - Val knownLengthCountBeforeFirstVisibleCell = Val.create(() -> { return firstVisibleIndex.getOpt() .map(i -> lengths.getMemoizedCountBefore(Math.min(i, lengths.size()))) @@ -117,17 +141,23 @@ public SizeTracker( averageLengthEstimate, (firstIdx, knownCnt, avgLen) -> (firstIdx - knownCnt) * avgLen); - Val firstCellMinY = firstVisibleCell.flatMap(orientation::minYProperty); + Val firstCellMinY = cells.memoizedItems() + .collapse(visCells -> visCells.isEmpty() ? null : visCells.get(0)) + .flatMap(orientation::minYProperty); - lengthOffsetEstimate = Val.wrap( EventStreams.combine( + EventStream> lengthOffsetStream = EventStreams.combine( totalKnownLengthBeforeFirstVisibleCell.values(), unknownLengthEstimateBeforeFirstVisibleCell.values(), firstCellMinY.values() - ) - .filter( t3 -> t3.test( (a,b,minY) -> a != null && b != null && minY != null ) ) - .thenRetainLatestFor( Duration.ofMillis( 1 ) ) - .map( t3 -> t3.map( (a,b,minY) -> Double.valueOf( a + b - minY ) ) ) - .toBinding( 0.0 ) ); + ); + + lengthOffsetEstimate = Val.wrap( + // skip spurious events resulting from cell replacement (delete then add again), except + // when immediateUpdate is true: activated via updateNextLengthOffsetEstimateImmediately() + new PausableSuccessionStream<>( lengthOffsetStream, Duration.ofMillis(15), immediateUpdate ) + .filter( t3 -> t3.test( (a,b,minY) -> a != null && b != null && minY != null ) ) + .map( t3 -> t3.map( (a,b,minY) -> Double.valueOf( Math.round( a + b - minY ) ) ) ) + .toBinding( 0.0 ) ); // pinning totalLengthEstimate and lengthOffsetEstimate // binds it all together and enables memoization @@ -136,6 +166,9 @@ public SizeTracker( lengthOffsetEstimate.pin()); } + private SimpleBooleanProperty immediateUpdate = new SimpleBooleanProperty(); + void updateNextLengthOffsetEstimateImmediately() { immediateUpdate.set( true ); } + private static Val avoidFalseInvalidations(Val src) { return new ValBase() { @Override diff --git a/src/main/java/org/fxmisc/flowless/StableBidirectionalVar.java b/src/main/java/org/fxmisc/flowless/StableBidirectionalVar.java deleted file mode 100644 index 65abb09..0000000 --- a/src/main/java/org/fxmisc/flowless/StableBidirectionalVar.java +++ /dev/null @@ -1,95 +0,0 @@ -package org.fxmisc.flowless; - -import java.util.function.Consumer; - -import org.reactfx.Subscription; -import org.reactfx.value.ProxyVal; -import org.reactfx.value.Val; -import org.reactfx.value.Var; - -import javafx.application.Platform; -import javafx.beans.property.Property; -import javafx.beans.value.ChangeListener; -import javafx.beans.value.ObservableValue; -/** - * This class overrides the Var.bindBidirectional method implementing a mechanism to prevent looping. - *
By default bindBidirectional delegates to Bindings.bindBidirectional - * which isn't always stable for the Val -> Var paradigm, sometimes producing a continuous feedback loop.
- */ -class StableBidirectionalVar extends ProxyVal implements Var -{ - private final Consumer setval; - private Subscription binding = null; - private ChangeListener left, right; - private T last = null; - - StableBidirectionalVar( Val underlying, Consumer setter ) - { - super( underlying ); - setval = setter; - } - - @Override - public T getValue() - { - return getUnderlyingObservable().getValue(); - } - - @Override - protected Consumer adaptObserver( Consumer observer ) - { - return observer; // no adaptation needed - } - - @Override - public void bind( ObservableValue observable ) - { - unbind(); - binding = Val.observeChanges( observable, (ob,ov,nv) -> setValue( nv ) ); - setValue( observable.getValue() ); - } - - @Override - public boolean isBound() - { - return binding != null; - } - - @Override - public void unbind() - { - if( binding != null ) binding.unsubscribe(); - binding = null; - } - - @Override - public void setValue( T newVal ) - { - setval.accept( newVal ); - } - - @Override - public void unbindBidirectional( Property prop ) - { - if ( right != null ) prop.removeListener( right ); - if ( left != null ) removeListener( left ); - left = null; right = null; - } - - @Override - public void bindBidirectional( Property prop ) - { - unbindBidirectional( prop ); - prop.addListener( right = (ob,ov,nv) -> adjustOther( this, nv ) ); - addListener( left = (ob,ov,nv) -> adjustOther( prop, nv ) ); - } - - private void adjustOther( Property other, T nv ) - { - if ( ! nv.equals( last ) ) - { - Platform.runLater( () -> other.setValue( nv ) ); - last = nv; - } - } -} diff --git a/src/main/java/org/fxmisc/flowless/VirtualFlow.java b/src/main/java/org/fxmisc/flowless/VirtualFlow.java index b49957a..2845765 100644 --- a/src/main/java/org/fxmisc/flowless/VirtualFlow.java +++ b/src/main/java/org/fxmisc/flowless/VirtualFlow.java @@ -180,7 +180,7 @@ private VirtualFlow( layoutBoundsProperty(), b -> new Rectangle(b.getWidth(), b.getHeight()))); - lengthOffsetEstimate = new StableBidirectionalVar<>( sizeTracker.lengthOffsetEstimateProperty(), this::setLengthOffset ); + lengthOffsetEstimate = sizeTracker.lengthOffsetEstimateProperty().asVar(this::setLengthOffset); // scroll content by mouse scroll this.addEventHandler(ScrollEvent.ANY, se -> { @@ -532,6 +532,7 @@ void setLengthOffset(double pixels) { if(diff == 0) { // do nothing } else if(Math.abs(diff) <= length) { // distance less than one screen + sizeTracker.updateNextLengthOffsetEstimateImmediately(); navigator.scrollCurrentPositionBy(diff); } else { jumpToAbsolutePosition(pixels); diff --git a/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java b/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java index 574304e..38d7984 100644 --- a/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java +++ b/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java @@ -327,7 +327,9 @@ private void setHPosition(double pos) { content.getLayoutBounds().getWidth(), padding.getLeft() + padding.getRight(), content.totalWidthEstimateProperty().getValue()); - content.estimatedScrollXProperty().setValue((double) Math.round(offset)); + if ( content.estimatedScrollXProperty().getValue() != offset ) { + content.estimatedScrollXProperty().setValue(offset); + } } private void setVPosition(double pos) { @@ -337,9 +339,9 @@ private void setVPosition(double pos) { content.getLayoutBounds().getHeight(), padding.getTop() + padding.getBottom(), content.totalHeightEstimateProperty().getValue()); - // offset needs rounding otherwise thin lines appear between cells, - // usually only visible when cells have dark backgrounds/borders. - content.estimatedScrollYProperty().setValue((double) Math.round(offset)); + if ( content.estimatedScrollYProperty().getValue() != offset ) { + content.estimatedScrollYProperty().setValue(offset); + } } private static void setupUnitIncrement(ScrollBar bar) { @@ -360,14 +362,16 @@ protected double computeValue() { private static double offsetToScrollbarPosition( double contentOffset, double viewportSize, double padding, double contentSize) { return contentSize > viewportSize - ? contentOffset / (contentSize - viewportSize + padding) * contentSize + // rounding otherwise thin lines appear between cells, only visible with dark backgrounds/borders + ? (double) Math.round( contentOffset / (contentSize - viewportSize + padding) * contentSize ) : 0; } private static double scrollbarPositionToOffset( double scrollbarPos, double viewportSize, double padding, double contentSize) { return contentSize > viewportSize - ? scrollbarPos / contentSize * (contentSize - viewportSize + padding) + // rounding otherwise thin lines appear between cells, only visible with dark backgrounds/borders + ? (double) Math.round( scrollbarPos / contentSize * (contentSize - viewportSize + padding) ) : 0; } } diff --git a/src/test/java/org/fxmisc/flowless/VirtualFlowTest.java b/src/test/java/org/fxmisc/flowless/VirtualFlowTest.java index 9627558..d185ffd 100644 --- a/src/test/java/org/fxmisc/flowless/VirtualFlowTest.java +++ b/src/test/java/org/fxmisc/flowless/VirtualFlowTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertTrue; import org.junit.Test; +import org.testfx.util.WaitForAsyncUtils; import javafx.collections.FXCollections; import javafx.collections.ObservableList; @@ -21,9 +22,9 @@ public void idempotentShowTest() { vf.resize(100, 100); // size of VirtualFlow less than that of the cell vf.layout(); - vf.show(110.0); vf.show(110.0); vf.layout(); + assertEquals(-10.0, rect.getBoundsInParent().getMinY(), 0.01); } @@ -36,9 +37,10 @@ public void idempotentSetLengthOffsetTest() { vf.resize(100, 100); // size of VirtualFlow less than that of the cell vf.layout(); - vf.setLengthOffset(10.0); - vf.setLengthOffset(10.0); + WaitForAsyncUtils.waitForFxEvents(); + vf.setLengthOffset( 10 ); vf.layout(); + assertEquals(-10.0, rect.getBoundsInParent().getMinY(), 0.01); } @@ -65,12 +67,14 @@ public void fastVisibleIndexTest() { vf.layout(); assertSame(visibleCells.get(0), vf.getCell(vf.getFirstVisibleIndex())); assertSame(visibleCells.get(visibleCells.size() - 1), vf.getCell(vf.getLastVisibleIndex())); - assertTrue(vf.getFirstVisibleIndex() <= 50 && 50 <= vf.getLastVisibleIndex()); + assertTrue(vf.getFirstVisibleIndex() > 40 && vf.getFirstVisibleIndex() <= 50); + assertTrue(vf.getLastVisibleIndex() >= 50); vf.show(99); vf.layout(); assertSame(visibleCells.get(0), vf.getCell(vf.getFirstVisibleIndex())); assertSame(visibleCells.get(visibleCells.size() - 1), vf.getCell(vf.getLastVisibleIndex())); - assertTrue(vf.getFirstVisibleIndex() <= 99 && 99 <= vf.getLastVisibleIndex()); + assertTrue(vf.getFirstVisibleIndex() > 50 && vf.getFirstVisibleIndex() <= 99); + assertTrue(vf.getLastVisibleIndex() >= 99); } }