Skip to content

virtual-scroll: rewrite offset in terms of "to-top" and fix a bug where items were removed too soon #10986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
/** The last measured size of the rendered content in the viewport. */
private _lastRenderedContentOffset: number;

/**
* The number of consecutive cycles where removing extra items has failed. Failure here means that
* we estimated how many items we could safely remove, but our estimate turned out to be too much
* and it wasn't safe to remove that many elements.
*/
private _removalFailures = 0;

/**
* @param minBufferPx The minimum amount of buffer rendered beyond the viewport (in pixels).
* If the amount of buffer dips below this number, more items will be rendered.
Expand Down Expand Up @@ -182,6 +189,8 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
if (scrollMagnitude >= viewport.getViewportSize()) {
this._setScrollOffset();
} else {
// The currently rendered range.
const renderedRange = viewport.getRenderedRange();
// The number of new items to render on the side the user is scrolling towards. Rather than
// just filling the underscan space, we actually fill enough to have a buffer size of
// `addBufferPx`. This gives us a little wiggle room in case our item size estimate is off.
Expand All @@ -192,11 +201,13 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
const overscan = (scrollDelta < 0 ? endBuffer : startBuffer) - this._minBufferPx +
scrollMagnitude;
// The number of currently rendered items to remove on the side the user is scrolling away
// from.
const removeItems = Math.max(0, Math.floor(overscan / this._averager.getAverageItemSize()));
// from. If removal has failed in recent cycles we are less aggressive in how much we try to
// remove.
const unboundedRemoveItems = Math.floor(
overscan / this._averager.getAverageItemSize() / (this._removalFailures + 1));
const removeItems =
Math.min(renderedRange.end - renderedRange.start, Math.max(0, unboundedRemoveItems));

// The currently rendered range.
const renderedRange = viewport.getRenderedRange();
// The new range we will tell the viewport to render. We first expand it to include the new
// items we want rendered, we then contract the opposite side to remove items we no longer
// want rendered.
Expand All @@ -215,19 +226,39 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
let contentOffset: number;
let contentOffsetTo: 'to-start' | 'to-end';
if (scrollDelta < 0) {
const removedSize = viewport.measureRangeSize({
let removedSize = viewport.measureRangeSize({
start: range.end,
end: renderedRange.end,
});
contentOffset =
this._lastRenderedContentOffset + this._lastRenderedContentSize - removedSize;
// Check that we're not removing too much.
if (removedSize <= overscan) {
contentOffset =
this._lastRenderedContentOffset + this._lastRenderedContentSize - removedSize;
this._removalFailures = 0;
} else {
// If the removal is more than the overscan can absorb just undo it and record the fact
// that the removal failed so we can be less aggressive next time.
range.end = renderedRange.end;
contentOffset = this._lastRenderedContentOffset + this._lastRenderedContentSize;
this._removalFailures++;
}
contentOffsetTo = 'to-end';
} else {
const removedSize = viewport.measureRangeSize({
start: renderedRange.start,
end: range.start,
});
contentOffset = this._lastRenderedContentOffset + removedSize;
// Check that we're not removing too much.
if (removedSize <= overscan) {
contentOffset = this._lastRenderedContentOffset + removedSize;
this._removalFailures = 0;
} else {
// If the removal is more than the overscan can absorb just undo it and record the fact
// that the removal failed so we can be less aggressive next time.
range.start = renderedRange.start;
contentOffset = this._lastRenderedContentOffset;
this._removalFailures++;
}
contentOffsetTo = 'to-start';
}

Expand All @@ -247,7 +278,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
*/
private _checkRenderedContentSize() {
const viewport = this._viewport!;
this._lastRenderedContentOffset = viewport.measureRenderedContentOffset();
this._lastRenderedContentOffset = viewport.getOffsetToRenderedContentStart()!;
this._lastRenderedContentSize = viewport.measureRenderedContentSize();
this._averager.addSample(viewport.getRenderedRange(), this._lastRenderedContentSize);
this._updateTotalContentSize(this._lastRenderedContentSize);
Expand All @@ -267,6 +298,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
viewport.setScrollOffset(scrollOffset);
}
this._lastScrollOffset = scrollOffset;
this._removalFailures = 0;

const itemSize = this._averager.getAverageItemSize();
const firstVisibleIndex =
Expand Down
50 changes: 33 additions & 17 deletions src/cdk-experimental/scrolling/virtual-scroll-viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,15 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy {
/** the currently attached CdkVirtualForOf. */
private _forOf: CdkVirtualForOf<any> | null;

/** The last rendered content offset that was set. */
private _renderedContentOffset = 0;

/**
* Whether the last rendered content offset was to the end of the content (and therefore needs to
* be rewritten as an offset to the start of the content).
*/
private _renderedContentOffsetNeedsRewrite = false;

constructor(public elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef,
private _ngZone: NgZone, private _sanitizer: DomSanitizer,
@Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy) {}
Expand Down Expand Up @@ -204,21 +213,43 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy {
// viewport.setRenderedContentOffset(...);
//
// The call to `onContentRendered` will happen after all of the updates have been applied.
Promise.resolve().then(() => this._scrollStrategy.onContentRendered());
Promise.resolve().then(() => {
// If the rendered content offset was specified as an offset to the end of the content,
// rewrite it as an offset to the start of the content.
if (this._renderedContentOffsetNeedsRewrite) {
this._renderedContentOffset -= this.measureRenderedContentSize();
this._renderedContentOffsetNeedsRewrite = false;
this.setRenderedContentOffset(this._renderedContentOffset);
}

this._scrollStrategy.onContentRendered();
});
}));
});
}
}

/** Sets the offset of the rendered portion of the data from the start (in pixels). */
/**
* Gets the offset from the start of the viewport to the start of the rendered data (in pixels).
*/
getOffsetToRenderedContentStart(): number | null {
return this._renderedContentOffsetNeedsRewrite ? null: this._renderedContentOffset;
}

/**
* Sets the offset from the start of the viewport to either the start or end of the rendered data
* (in pixels).
*/
setRenderedContentOffset(offset: number, to: 'to-start' | 'to-end' = 'to-start') {
const axis = this.orientation === 'horizontal' ? 'X' : 'Y';
let transform = `translate${axis}(${Number(offset)}px)`;
this._renderedContentOffset = offset;
if (to === 'to-end') {
// TODO(mmalerba): The viewport should rewrite this as a `to-start` offset on the next render
// cycle. Otherwise elements will appear to expand in the wrong direction (e.g.
// `mat-expansion-panel` would expand upward).
transform += ` translate${axis}(-100%)`;
this._renderedContentOffsetNeedsRewrite = true;
}
if (this._renderedContentTransform != transform) {
// Re-enter the Angular zone so we can mark for change detection.
Expand Down Expand Up @@ -253,21 +284,6 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy {
return this.orientation === 'horizontal' ? contentEl.offsetWidth : contentEl.offsetHeight;
}

// TODO(mmalerba): Try to do this in a way that's less bad for performance. (The bad part here is
// that we have to measure the viewport which is not absolutely positioned.)
/** Measure the offset from the start of the viewport to the start of the rendered content. */
measureRenderedContentOffset(): number {
const viewportEl = this.elementRef.nativeElement;
const contentEl = this._contentWrapper.nativeElement;
if (this.orientation === 'horizontal') {
return contentEl.getBoundingClientRect().left + viewportEl.scrollLeft -
viewportEl.getBoundingClientRect().left - viewportEl.clientLeft;
} else {
return contentEl.getBoundingClientRect().top + viewportEl.scrollTop -
viewportEl.getBoundingClientRect().top - viewportEl.clientTop;
}
}

/**
* Measure the total combined size of the given range. Throws if the range includes items that are
* not rendered.
Expand Down
18 changes: 18 additions & 0 deletions src/demo-app/virtual-scroll/virtual-scroll-demo.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
<h2>Autosize</h2>

<h3>Uniform size</h3>
<cdk-virtual-scroll-viewport class="demo-viewport" autosize>
<div *cdkVirtualFor="let size of fixedSizeData; let i = index" class="demo-item"
[style.height.px]="size">
Item #{{i}} - ({{size}}px)
</div>
</cdk-virtual-scroll-viewport>

<h3>Increasing size</h3>
<cdk-virtual-scroll-viewport class="demo-viewport" autosize>
<div *cdkVirtualFor="let size of increasingSizeData; let i = index" class="demo-item"
[style.height.px]="size">
Item #{{i}} - ({{size}}px)
</div>
</cdk-virtual-scroll-viewport>

<h3>Decreasing size</h3>
<cdk-virtual-scroll-viewport class="demo-viewport" autosize>
<div *cdkVirtualFor="let size of decreasingSizeData; let i = index" class="demo-item"
[style.height.px]="size">
Item #{{i}} - ({{size}}px)
</div>
</cdk-virtual-scroll-viewport>

<h3>Random size</h3>
<cdk-virtual-scroll-viewport class="demo-viewport" autosize>
<div *cdkVirtualFor="let size of randomData; let i = index" class="demo-item"
[style.height.px]="size">
Expand Down
3 changes: 3 additions & 0 deletions src/demo-app/virtual-scroll/virtual-scroll-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ import {Component, ViewEncapsulation} from '@angular/core';
})
export class VirtualScrollDemo {
fixedSizeData = Array(10000).fill(50);
increasingSizeData = Array(10000).fill(0).map((_, i) => (1 + Math.floor(i / 1000)) * 20);
decreasingSizeData = Array(10000).fill(0)
.map((_, i) => (1 + Math.floor((10000 - i) / 1000)) * 20);
randomData = Array(10000).fill(0).map(() => Math.round(Math.random() * 100));
}