Fix wrapped text scrollbar flicker

When averageLengthEstimate is updating it briefly has spurious values which causes jitter. Also reduced horizontal scrollbar noise.

Author: Jugen

src/main/java/org/fxmisc/flowless/PausableSuccessionStream.java

@@ -0,0 +1,122 @@
+package org.fxmisc.flowless;
+
+import java.time.Duration;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+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.value.ObservableBooleanValue;
+
+class PausableSuccessionStream<O> extends EventStreamBase<O> implements AwaitingEventStream<O> {
+    private final EventStream<O> input;
+    private final Function<? super O, ? extends O> initial;
+    private final BiFunction<? super O, ? super O, ? extends O> reduction;
+    private final Timer timer;
+
+    private boolean hasEvent = false;
+    private BooleanBinding pending = null;
+    private BooleanProperty successionOff;
+    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 <i>close temporal succession</i> is
+     * defined by {@code timeout}: time gap between two successive events must
+     * be at most {@code timeout}.
+     *
+     * <p><b>Note:</b> This function can be used only when this stream and
+     * the returned stream are used from the JavaFX application thread.</p>
+     *
+     * @param timeout the maximum time difference between two subsequent events
+     * in a <em>close</em> succession.
+     * @param realTime when true immediately emits the next event and sets
+     * realTime back to <em>false</em>.
+     */
+    public PausableSuccessionStream( EventStream<O> input, Duration timeout, BooleanProperty realTime )
+    {
+        this( input, (Function<? super O, ? extends O>) Function.identity(), (a,b) -> b, timeout, realTime );
+    }
+
+    public PausableSuccessionStream(
+            EventStream<O> input,
+            Function<? super O, ? extends O> initial,
+            BiFunction<? super O, ? super O, ? extends O> reduction,
+            Duration timeout,
+            BooleanProperty realTime) {
+
+        this.input = input;
+        this.initial = initial;
+        this.reduction = reduction;
+        this.successionOff = realTime;
+
+        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) {
+        if(successionOff.get())
+        {
+            timer.stop();
+            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();
+        }
+    }
+}

src/main/java/org/fxmisc/flowless/SizeTracker.java

@@ -4,14 +4,17 @@
 import java.util.Optional;
 import java.util.function.Function;
 
+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 +59,15 @@ 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<Double> knownBreadths = this.breadths.memoizedItems();
+
+        this.maxKnownMinBreadth = Val.create(
+                () -> knownBreadths.stream().mapToDouble( Double::doubleValue ).max().orElse(0.0),
+                EventStreams.changesOf( knownBreadths ).filter( c -> c.getList().size() > 0 )
+                // skips spurious events resulting from cell replacement (delete then add again)
+                .successionEnds( Duration.ofMillis( 15 ) )
+        );
+
         this.breadthForCells = Val.combine(
                 maxKnownMinBreadth,
                 viewportBounds,
@@ -71,8 +80,6 @@ public SizeTracker(
         this.lengths = cells.mapDynamic(lengthFn).memoize();
 
         LiveList<Double> knownLengths = this.lengths.memoizedItems();
-        Val<Double> sumOfKnownLengths = knownLengths.reduce((a, b) -> a + b).orElseConst(0.0);
-        Val<Integer> knownLengthCount = knownLengths.sizeProperty();
 
         this.averageLengthEstimate = Val.create(
                 () -> {
@@ -82,12 +89,14 @@ public SizeTracker(
                         lengths.force(j, j + 1);
                     }
 
-                    int count = knownLengthCount.getValue();
-                    return count == 0
-                            ? null
-                            : sumOfKnownLengths.getValue() / count;
+                    return knownLengths.stream()
+                        .mapToDouble( Double::doubleValue )
+                        .average().orElse(0.0);
                 },
-                sumOfKnownLengths, knownLengthCount);
+                EventStreams.changesOf( knownLengths ).filter( c -> c.getList().size() > 0 )
+                // skips spurious events resulting from cell replacement (delete then add again)
+                .successionEnds( Duration.ofMillis( 15 ) )
+        );
 
         this.totalLengthEstimate = Val.combine(
                 averageLengthEstimate, cells.sizeProperty(),
@@ -98,9 +107,6 @@ public SizeTracker(
                 cells, cells.memoizedItems()); // need to observe cells.memoizedItems()
                 // as well, because they may change without a change in cells.
 
-        Val<? extends Cell<?, ?>> firstVisibleCell = cells.memoizedItems()
-                .collapse(visCells -> visCells.isEmpty() ? null : visCells.get(0));
-
         Val<Integer> knownLengthCountBeforeFirstVisibleCell = Val.create(() -> {
             return firstVisibleIndex.getOpt()
                     .map(i -> lengths.getMemoizedCountBefore(Math.min(i, lengths.size())))
@@ -117,17 +123,23 @@ public SizeTracker(
                 averageLengthEstimate,
                 (firstIdx, knownCnt, avgLen) -> (firstIdx - knownCnt) * avgLen);
 
-        Val<Double> firstCellMinY = firstVisibleCell.flatMap(orientation::minYProperty);
+        Val<Double> firstCellMinY = cells.memoizedItems()
+                .collapse(visCells -> visCells.isEmpty() ? null : visCells.get(0))
+                .flatMap(orientation::minYProperty);
 
-        lengthOffsetEstimate = Val.wrap( EventStreams.combine(
+        EventStream<Tuple3<Double, Double, Double>> 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 +148,9 @@ public SizeTracker(
                 lengthOffsetEstimate.pin());
     }
 
+    private SimpleBooleanProperty immediateUpdate = new SimpleBooleanProperty();
+    void updateNextLengthOffsetEstimateImmediately() { immediateUpdate.set( true ); }
+
     private static <T> Val<T> avoidFalseInvalidations(Val<T> src) {
         return new ValBase<T>() {
             @Override

src/main/java/org/fxmisc/flowless/VirtualFlow.java

@@ -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);

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;
     }
 }

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;
@@ -36,9 +37,11 @@ public void idempotentSetLengthOffsetTest() {
         vf.resize(100, 100); // size of VirtualFlow less than that of the cell
         vf.layout();
 
+        WaitForAsyncUtils.waitForFxEvents();
         vf.setLengthOffset(10.0);
         vf.setLengthOffset(10.0);
         vf.layout();
+
         assertEquals(-10.0, rect.getBoundsInParent().getMinY(), 0.01);
     }