From df4f002c7efcc433b0f58ad239d03fa76ba1c4d0 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 12 Apr 2018 11:40:47 -0700 Subject: [PATCH 1/4] rewrite offsets to the end of the rendered content as offsets to the start --- .../scrolling/auto-size-virtual-scroll.ts | 2 +- .../scrolling/virtual-scroll-viewport.ts | 50 ++++++++++++------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts index 47b49604780b..a6313d892eed 100644 --- a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts +++ b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts @@ -247,7 +247,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); diff --git a/src/cdk-experimental/scrolling/virtual-scroll-viewport.ts b/src/cdk-experimental/scrolling/virtual-scroll-viewport.ts index f31679ca9c47..f01f82ca6ba4 100644 --- a/src/cdk-experimental/scrolling/virtual-scroll-viewport.ts +++ b/src/cdk-experimental/scrolling/virtual-scroll-viewport.ts @@ -86,6 +86,15 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy { /** the currently attached CdkVirtualForOf. */ private _forOf: CdkVirtualForOf | 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) {} @@ -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. @@ -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. From b98402bae4cd7c1b62da65a99c51321bfc336d07 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 12 Apr 2018 13:42:15 -0700 Subject: [PATCH 2/4] add some more autosize demos for testing --- .../virtual-scroll/virtual-scroll-demo.html | 18 ++++++++++++++++++ .../virtual-scroll/virtual-scroll-demo.ts | 2 ++ 2 files changed, 20 insertions(+) diff --git a/src/demo-app/virtual-scroll/virtual-scroll-demo.html b/src/demo-app/virtual-scroll/virtual-scroll-demo.html index c77bbd2b5cfb..b42439967b1c 100644 --- a/src/demo-app/virtual-scroll/virtual-scroll-demo.html +++ b/src/demo-app/virtual-scroll/virtual-scroll-demo.html @@ -1,5 +1,6 @@

Autosize

+

Uniform size

@@ -7,6 +8,23 @@

Autosize

 +

Increasing size

+ +
+ Item #{{i}} - ({{size}}px) +
+ + +

Decreasing size

+ +
+ Item #{{i}} - ({{size}}px) +
+ + +

Random size

diff --git a/src/demo-app/virtual-scroll/virtual-scroll-demo.ts b/src/demo-app/virtual-scroll/virtual-scroll-demo.ts index 2f09db639df6..5a63a65caf02 100644 --- a/src/demo-app/virtual-scroll/virtual-scroll-demo.ts +++ b/src/demo-app/virtual-scroll/virtual-scroll-demo.ts @@ -17,5 +17,7 @@ import {Component, ViewEncapsulation} from '@angular/core'; }) export class VirtualScrollDemo { fixedSizeData = Array(10000).fill(50); + increasingSizeData = Array(10000).fill(0).map((_, i) => i / 10000 * 300); + decreasingSizeData = Array(10000).fill(0).map((_, i) => (10000 - i) / 10000 * 300); randomData = Array(10000).fill(0).map(() => Math.round(Math.random() * 100)); } From 461a3a93684e8e01977d1a9fb966ca4fe01e6675 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 12 Apr 2018 15:07:38 -0700 Subject: [PATCH 3/4] make sure not to remove too many items --- .../scrolling/auto-size-virtual-scroll.ts | 43 +++++++++++++++---- .../virtual-scroll/virtual-scroll-demo.ts | 5 ++- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts index a6313d892eed..dfa7065df43d 100644 --- a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts +++ b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts @@ -85,6 +85,9 @@ 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. */ + 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. @@ -182,6 +185,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. @@ -192,11 +197,12 @@ 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 removeItems = Math.min(renderedRange.end - renderedRange.start, Math.max(0, + Math.floor( + overscan / this._averager.getAverageItemSize() / (this._removalFailures + 1)))); - // 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. @@ -215,19 +221,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'; } @@ -267,6 +293,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy { viewport.setScrollOffset(scrollOffset); } this._lastScrollOffset = scrollOffset; + this._removalFailures = 0; const itemSize = this._averager.getAverageItemSize(); const firstVisibleIndex = diff --git a/src/demo-app/virtual-scroll/virtual-scroll-demo.ts b/src/demo-app/virtual-scroll/virtual-scroll-demo.ts index 5a63a65caf02..5132f0f29d12 100644 --- a/src/demo-app/virtual-scroll/virtual-scroll-demo.ts +++ b/src/demo-app/virtual-scroll/virtual-scroll-demo.ts @@ -17,7 +17,8 @@ import {Component, ViewEncapsulation} from '@angular/core'; }) export class VirtualScrollDemo { fixedSizeData = Array(10000).fill(50); - increasingSizeData = Array(10000).fill(0).map((_, i) => i / 10000 * 300); - decreasingSizeData = Array(10000).fill(0).map((_, i) => (10000 - i) / 10000 * 300); + 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)); } From f2a3e51d82cb4ac96c901d5c30bf6af3b2fd00c4 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 27 Apr 2018 09:57:34 -0700 Subject: [PATCH 4/4] address comments --- .../scrolling/auto-size-virtual-scroll.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts index dfa7065df43d..a26d6d1af704 100644 --- a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts +++ b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts @@ -85,7 +85,11 @@ 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. */ + /** + * 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; /** @@ -199,9 +203,10 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy { // The number of currently rendered items to remove on the side the user is scrolling away // from. If removal has failed in recent cycles we are less aggressive in how much we try to // remove. - const removeItems = Math.min(renderedRange.end - renderedRange.start, Math.max(0, - Math.floor( - overscan / this._averager.getAverageItemSize() / (this._removalFailures + 1)))); + const unboundedRemoveItems = Math.floor( + overscan / this._averager.getAverageItemSize() / (this._removalFailures + 1)); + const removeItems = + Math.min(renderedRange.end - renderedRange.start, Math.max(0, unboundedRemoveItems)); // 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