Skip to content

Commit

Permalink
Fix wrapped text scrollbar flicker
Browse files Browse the repository at this point in the history
Changed averageLengthEstimate and totalLengthEstimate calculation process to avoid intermediary updates during cell refresh.

Fixed ScaledVirtualized rounding.
  • Loading branch information
Jugen committed Oct 31, 2022
1 parent b5ef653 commit 39a585a
Show file tree
Hide file tree
Showing 6 changed files with 233 additions and 53 deletions.
138 changes: 138 additions & 0 deletions src/main/java/org/fxmisc/flowless/PausableSuccessionStream.java
@@ -0,0 +1,138 @@
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, (Function<? super O, ? extends O>) Function.identity(), (a,b) -> b, 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, (Function<? super O, ? extends O>) Function.identity(), (a,b) -> b, timeout, new SimpleBooleanProperty(), condition );
}

private PausableSuccessionStream(
EventStream<O> input,
Function<? super O, ? extends O> initial,
BiFunction<? super O, ? super O, ? extends O> reduction,
Duration timeout,
BooleanProperty realTime,
Predicate<O> condition) {

this.input = input;
this.initial = initial;
this.reduction = reduction;
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
3 changes: 2 additions & 1 deletion src/main/java/org/fxmisc/flowless/VirtualFlow.java
Expand Up @@ -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 -> {
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 10 additions & 6 deletions src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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;
}
}

0 comments on commit 39a585a

Please sign in to comment.