Fix wrapped text scrollbar flicker

Changed averageLengthEstimate and totalLengthEstimate calculation process to avoid intermediary updates during cell refresh.

Fixed ScaledVirtualized rounding.

Author: Jugen

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<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 Predicate<O> 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 <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 <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, timeout, realTime, a -> realTime.get() );
+    }
+
+    /**
+    * @param timeout the maximum time difference between two subsequent events
+    * in <em>close</em> succession.
+    * @param condition when true immediately emits the event, otherwise
+    * waits for <em>timeout</em> before emitting the last received event.
+    */
+    public PausableSuccessionStream( EventStream<O> input, Duration timeout, Predicate<O> condition )
+    {
+        this( input, timeout, new SimpleBooleanProperty(), condition );
+    }
+
+    private PausableSuccessionStream(
+            EventStream<O> input,
+            java.time.Duration timeout,
+            BooleanProperty realTime,
+            Predicate<O> 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();
+        }
+    }
+}

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

@@ -17,8 +17,8 @@
  *     VirtualizedScrollPane<ScaledVirtualized> 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);
  *     }
  * </pre>
  *
@@ -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());

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<Double> 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<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(
-                () -> {
-                    // 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<Double> 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<double[/*average,total*/]> 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<double[/*average,total*/]> 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<Integer> 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<? 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 +141,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 +166,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