Skip to content

Commit

Permalink
Fix wrapped text scrollbar flicker (#113)
Browse files Browse the repository at this point in the history
Changed averageLengthEstimate and totalLengthEstimate calculation process to avoid intermediary updates during cell refresh. Also fixed ScaledVirtualized rounding.
  • Loading branch information
Jugen committed Nov 7, 2022
1 parent b5ef653 commit 80c0fe9
Show file tree
Hide file tree
Showing 7 changed files with 231 additions and 148 deletions.
136 changes: 136 additions & 0 deletions 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();
}
}
}
12 changes: 6 additions & 6 deletions src/main/java/org/fxmisc/flowless/ScaledVirtualized.java
Expand Up @@ -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>
*
Expand Down Expand Up @@ -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());
Expand Down
103 changes: 68 additions & 35 deletions src/main/java/org/fxmisc/flowless/SizeTracker.java
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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())))
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 80c0fe9

Please sign in to comment.