Skip to content

Commit ed6ab9a

Browse files
authored
virtual-scroll: rewrite offset in terms of "to-top" and fix a bug where items were removed too soon (#10986)
* rewrite offsets to the end of the rendered content as offsets to the start * add some more autosize demos for testing * make sure not to remove too many items * address comments
1 parent 97d623b commit ed6ab9a

File tree

4 files changed

+95
-26
lines changed

4 files changed

+95
-26
lines changed

src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
8585
/** The last measured size of the rendered content in the viewport. */
8686
private _lastRenderedContentOffset: number;
8787

88+
/**
89+
* The number of consecutive cycles where removing extra items has failed. Failure here means that
90+
* we estimated how many items we could safely remove, but our estimate turned out to be too much
91+
* and it wasn't safe to remove that many elements.
92+
*/
93+
private _removalFailures = 0;
94+
8895
/**
8996
* @param minBufferPx The minimum amount of buffer rendered beyond the viewport (in pixels).
9097
* If the amount of buffer dips below this number, more items will be rendered.
@@ -182,6 +189,8 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
182189
if (scrollMagnitude >= viewport.getViewportSize()) {
183190
this._setScrollOffset();
184191
} else {
192+
// The currently rendered range.
193+
const renderedRange = viewport.getRenderedRange();
185194
// The number of new items to render on the side the user is scrolling towards. Rather than
186195
// just filling the underscan space, we actually fill enough to have a buffer size of
187196
// `addBufferPx`. This gives us a little wiggle room in case our item size estimate is off.
@@ -192,11 +201,13 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
192201
const overscan = (scrollDelta < 0 ? endBuffer : startBuffer) - this._minBufferPx +
193202
scrollMagnitude;
194203
// The number of currently rendered items to remove on the side the user is scrolling away
195-
// from.
196-
const removeItems = Math.max(0, Math.floor(overscan / this._averager.getAverageItemSize()));
204+
// from. If removal has failed in recent cycles we are less aggressive in how much we try to
205+
// remove.
206+
const unboundedRemoveItems = Math.floor(
207+
overscan / this._averager.getAverageItemSize() / (this._removalFailures + 1));
208+
const removeItems =
209+
Math.min(renderedRange.end - renderedRange.start, Math.max(0, unboundedRemoveItems));
197210

198-
// The currently rendered range.
199-
const renderedRange = viewport.getRenderedRange();
200211
// The new range we will tell the viewport to render. We first expand it to include the new
201212
// items we want rendered, we then contract the opposite side to remove items we no longer
202213
// want rendered.
@@ -215,19 +226,39 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
215226
let contentOffset: number;
216227
let contentOffsetTo: 'to-start' | 'to-end';
217228
if (scrollDelta < 0) {
218-
const removedSize = viewport.measureRangeSize({
229+
let removedSize = viewport.measureRangeSize({
219230
start: range.end,
220231
end: renderedRange.end,
221232
});
222-
contentOffset =
223-
this._lastRenderedContentOffset + this._lastRenderedContentSize - removedSize;
233+
// Check that we're not removing too much.
234+
if (removedSize <= overscan) {
235+
contentOffset =
236+
this._lastRenderedContentOffset + this._lastRenderedContentSize - removedSize;
237+
this._removalFailures = 0;
238+
} else {
239+
// If the removal is more than the overscan can absorb just undo it and record the fact
240+
// that the removal failed so we can be less aggressive next time.
241+
range.end = renderedRange.end;
242+
contentOffset = this._lastRenderedContentOffset + this._lastRenderedContentSize;
243+
this._removalFailures++;
244+
}
224245
contentOffsetTo = 'to-end';
225246
} else {
226247
const removedSize = viewport.measureRangeSize({
227248
start: renderedRange.start,
228249
end: range.start,
229250
});
230-
contentOffset = this._lastRenderedContentOffset + removedSize;
251+
// Check that we're not removing too much.
252+
if (removedSize <= overscan) {
253+
contentOffset = this._lastRenderedContentOffset + removedSize;
254+
this._removalFailures = 0;
255+
} else {
256+
// If the removal is more than the overscan can absorb just undo it and record the fact
257+
// that the removal failed so we can be less aggressive next time.
258+
range.start = renderedRange.start;
259+
contentOffset = this._lastRenderedContentOffset;
260+
this._removalFailures++;
261+
}
231262
contentOffsetTo = 'to-start';
232263
}
233264

@@ -247,7 +278,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
247278
*/
248279
private _checkRenderedContentSize() {
249280
const viewport = this._viewport!;
250-
this._lastRenderedContentOffset = viewport.measureRenderedContentOffset();
281+
this._lastRenderedContentOffset = viewport.getOffsetToRenderedContentStart()!;
251282
this._lastRenderedContentSize = viewport.measureRenderedContentSize();
252283
this._averager.addSample(viewport.getRenderedRange(), this._lastRenderedContentSize);
253284
this._updateTotalContentSize(this._lastRenderedContentSize);
@@ -267,6 +298,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
267298
viewport.setScrollOffset(scrollOffset);
268299
}
269300
this._lastScrollOffset = scrollOffset;
301+
this._removalFailures = 0;
270302

271303
const itemSize = this._averager.getAverageItemSize();
272304
const firstVisibleIndex =

src/cdk-experimental/scrolling/virtual-scroll-viewport.ts

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,15 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy {
8686
/** the currently attached CdkVirtualForOf. */
8787
private _forOf: CdkVirtualForOf<any> | null;
8888

89+
/** The last rendered content offset that was set. */
90+
private _renderedContentOffset = 0;
91+
92+
/**
93+
* Whether the last rendered content offset was to the end of the content (and therefore needs to
94+
* be rewritten as an offset to the start of the content).
95+
*/
96+
private _renderedContentOffsetNeedsRewrite = false;
97+
8998
constructor(public elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef,
9099
private _ngZone: NgZone, private _sanitizer: DomSanitizer,
91100
@Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy) {}
@@ -204,21 +213,43 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy {
204213
// viewport.setRenderedContentOffset(...);
205214
//
206215
// The call to `onContentRendered` will happen after all of the updates have been applied.
207-
Promise.resolve().then(() => this._scrollStrategy.onContentRendered());
216+
Promise.resolve().then(() => {
217+
// If the rendered content offset was specified as an offset to the end of the content,
218+
// rewrite it as an offset to the start of the content.
219+
if (this._renderedContentOffsetNeedsRewrite) {
220+
this._renderedContentOffset -= this.measureRenderedContentSize();
221+
this._renderedContentOffsetNeedsRewrite = false;
222+
this.setRenderedContentOffset(this._renderedContentOffset);
223+
}
224+
225+
this._scrollStrategy.onContentRendered();
226+
});
208227
}));
209228
});
210229
}
211230
}
212231

213-
/** Sets the offset of the rendered portion of the data from the start (in pixels). */
232+
/**
233+
* Gets the offset from the start of the viewport to the start of the rendered data (in pixels).
234+
*/
235+
getOffsetToRenderedContentStart(): number | null {
236+
return this._renderedContentOffsetNeedsRewrite ? null: this._renderedContentOffset;
237+
}
238+
239+
/**
240+
* Sets the offset from the start of the viewport to either the start or end of the rendered data
241+
* (in pixels).
242+
*/
214243
setRenderedContentOffset(offset: number, to: 'to-start' | 'to-end' = 'to-start') {
215244
const axis = this.orientation === 'horizontal' ? 'X' : 'Y';
216245
let transform = `translate${axis}(${Number(offset)}px)`;
246+
this._renderedContentOffset = offset;
217247
if (to === 'to-end') {
218248
// TODO(mmalerba): The viewport should rewrite this as a `to-start` offset on the next render
219249
// cycle. Otherwise elements will appear to expand in the wrong direction (e.g.
220250
// `mat-expansion-panel` would expand upward).
221251
transform += ` translate${axis}(-100%)`;
252+
this._renderedContentOffsetNeedsRewrite = true;
222253
}
223254
if (this._renderedContentTransform != transform) {
224255
// Re-enter the Angular zone so we can mark for change detection.
@@ -253,21 +284,6 @@ export class CdkVirtualScrollViewport implements DoCheck, OnInit, OnDestroy {
253284
return this.orientation === 'horizontal' ? contentEl.offsetWidth : contentEl.offsetHeight;
254285
}
255286

256-
// TODO(mmalerba): Try to do this in a way that's less bad for performance. (The bad part here is
257-
// that we have to measure the viewport which is not absolutely positioned.)
258-
/** Measure the offset from the start of the viewport to the start of the rendered content. */
259-
measureRenderedContentOffset(): number {
260-
const viewportEl = this.elementRef.nativeElement;
261-
const contentEl = this._contentWrapper.nativeElement;
262-
if (this.orientation === 'horizontal') {
263-
return contentEl.getBoundingClientRect().left + viewportEl.scrollLeft -
264-
viewportEl.getBoundingClientRect().left - viewportEl.clientLeft;
265-
} else {
266-
return contentEl.getBoundingClientRect().top + viewportEl.scrollTop -
267-
viewportEl.getBoundingClientRect().top - viewportEl.clientTop;
268-
}
269-
}
270-
271287
/**
272288
* Measure the total combined size of the given range. Throws if the range includes items that are
273289
* not rendered.

src/demo-app/virtual-scroll/virtual-scroll-demo.html

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,30 @@
11
<h2>Autosize</h2>
22

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

11+
<h3>Increasing size</h3>
12+
<cdk-virtual-scroll-viewport class="demo-viewport" autosize>
13+
<div *cdkVirtualFor="let size of increasingSizeData; let i = index" class="demo-item"
14+
[style.height.px]="size">
15+
Item #{{i}} - ({{size}}px)
16+
</div>
17+
</cdk-virtual-scroll-viewport>
18+
19+
<h3>Decreasing size</h3>
20+
<cdk-virtual-scroll-viewport class="demo-viewport" autosize>
21+
<div *cdkVirtualFor="let size of decreasingSizeData; let i = index" class="demo-item"
22+
[style.height.px]="size">
23+
Item #{{i}} - ({{size}}px)
24+
</div>
25+
</cdk-virtual-scroll-viewport>
26+
27+
<h3>Random size</h3>
1028
<cdk-virtual-scroll-viewport class="demo-viewport" autosize>
1129
<div *cdkVirtualFor="let size of randomData; let i = index" class="demo-item"
1230
[style.height.px]="size">

src/demo-app/virtual-scroll/virtual-scroll-demo.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,8 @@ import {Component, ViewEncapsulation} from '@angular/core';
1717
})
1818
export class VirtualScrollDemo {
1919
fixedSizeData = Array(10000).fill(50);
20+
increasingSizeData = Array(10000).fill(0).map((_, i) => (1 + Math.floor(i / 1000)) * 20);
21+
decreasingSizeData = Array(10000).fill(0)
22+
.map((_, i) => (1 + Math.floor((10000 - i) / 1000)) * 20);
2023
randomData = Array(10000).fill(0).map(() => Math.round(Math.random() * 100));
2124
}

0 commit comments

Comments
 (0)