feat(virtual-scroll): change fixed strategy to use min & max buffer (#12557)

* feat(virtual-scroll): change fixed strategy to use min & max buffer

* address comments

Author: mmalerba

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

@@ -83,7 +83,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   private _minBufferPx: number;
 
   /** The number of buffer items to render beyond the edge of the viewport (in pixels). */
-  private _addBufferPx: number;
+  private _maxBufferPx: number;
 
   /** The estimator used to estimate the size of unseen items. */
   private _averager: ItemSizeAverager;
@@ -107,14 +107,14 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   /**
    * @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.
-   * @param addBufferPx The number of pixels worth of buffer to shoot for when rendering new items.
+   * @param maxBufferPx The number of pixels worth of buffer to shoot for when rendering new items.
    *     If the actual amount turns out to be less it will not necessarily trigger an additional
    *     rendering cycle (as long as the amount of buffer is still greater than `minBufferPx`).
    * @param averager The averager used to estimate the size of unseen items.
    */
-  constructor(minBufferPx: number, addBufferPx: number, averager = new ItemSizeAverager()) {
+  constructor(minBufferPx: number, maxBufferPx: number, averager = new ItemSizeAverager()) {
     this._minBufferPx = minBufferPx;
-    this._addBufferPx = addBufferPx;
+    this._maxBufferPx = maxBufferPx;
     this._averager = averager;
   }
 
@@ -172,12 +172,15 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   /**
    * Update the buffer parameters.
    * @param minBufferPx The minimum amount of buffer rendered beyond the viewport (in pixels).
-   * @param addBufferPx The number of buffer items to render beyond the edge of the viewport (in
+   * @param maxBufferPx The number of buffer items to render beyond the edge of the viewport (in
    *     pixels).
    */
-  updateBufferSize(minBufferPx: number, addBufferPx: number) {
+  updateBufferSize(minBufferPx: number, maxBufferPx: number) {
+    if (maxBufferPx < minBufferPx) {
+      throw('CDK virtual scroll: maxBufferPx must be greater than or equal to minBufferPx');
+    }
     this._minBufferPx = minBufferPx;
-    this._addBufferPx = addBufferPx;
+    this._maxBufferPx = maxBufferPx;
   }
 
   /** Update the rendered content after the user scrolls. */
@@ -242,8 +245,8 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
       } else {
         // 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.
-        const addItems = Math.max(0, Math.ceil((underscan - this._minBufferPx + this._addBufferPx) /
+        // `maxBufferPx`. This gives us a little wiggle room in case our item size estimate is off.
+        const addItems = Math.max(0, Math.ceil((underscan - this._minBufferPx + this._maxBufferPx) /
             this._averager.getAverageItemSize()));
         // The amount of filled space beyond what is necessary on the side the user is scrolling
         // away from.
@@ -361,7 +364,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
     const itemSize = this._averager.getAverageItemSize();
     const firstVisibleIndex =
         Math.min(viewport.getDataLength() - 1, Math.floor(scrollOffset / itemSize));
-    const bufferSize = Math.ceil(this._addBufferPx / itemSize);
+    const bufferSize = Math.ceil(this._maxBufferPx / itemSize);
     const range = this._expandRange(
         this._getVisibleRangeForIndex(firstVisibleIndex), bufferSize, bufferSize);
 
@@ -456,14 +459,14 @@ export class CdkAutoSizeVirtualScroll implements OnChanges {
    * Defaults to 200px.
    */
   @Input()
-  get addBufferPx(): number { return this._addBufferPx; }
-  set addBufferPx(value: number) { this._addBufferPx = coerceNumberProperty(value); }
-  _addBufferPx = 200;
+  get maxBufferPx(): number { return this._maxBufferPx; }
+  set maxBufferPx(value: number) { this._maxBufferPx = coerceNumberProperty(value); }
+  _maxBufferPx = 200;
 
   /** The scroll strategy used by this directive. */
-  _scrollStrategy = new AutoSizeVirtualScrollStrategy(this.minBufferPx, this.addBufferPx);
+  _scrollStrategy = new AutoSizeVirtualScrollStrategy(this.minBufferPx, this.maxBufferPx);
 
   ngOnChanges() {
-    this._scrollStrategy.updateBufferSize(this.minBufferPx, this.addBufferPx);
+    this._scrollStrategy.updateBufferSize(this.minBufferPx, this.maxBufferPx);
   }
 }

src/cdk-experimental/scrolling/scrolling.md

@@ -11,19 +11,19 @@ This can be added to your viewport by using the `autosize` directive.
 </cdk-virtual-scroll-viewport>
 ```
 
-The `autosize` strategy is configured through two inputs: `minBufferPx` and `addBufferPx`.
+The `autosize` strategy is configured through two inputs: `minBufferPx` and `maxBufferPx`.
 
 **`minBufferPx`** determines the minimum space outside virtual scrolling viewport that will be
 filled with content. Increasing this will increase the amount of content a user will see before more
 content must be rendered. However, too large a value will cause more content to be rendered than is
 necessary.
 
-**`addBufferPx`** determines the amount of content that will be added incrementally as the viewport
+**`maxBufferPx`** determines the amount of content that will be added incrementally as the viewport
 is scrolled. This should be greater than the size of `minBufferPx` so that one "render" is needed at
 a time.
 
 ```html
-<cdk-virtual-scroll-viewport autosize minBufferPx="50" addBufferPx="100">
+<cdk-virtual-scroll-viewport autosize minBufferPx="50" maxBufferPx="100">
   ...
 </cdk-virtual-scroll-viewport>
 ```

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

@@ -40,6 +40,12 @@ describe('CdkVirtualScrollViewport', () => {
           'should render 4 items to fill 200px space based on 50px estimate from first item');
     }));
 
+    it('should throw if maxBufferPx is less than minBufferPx', fakeAsync(() => {
+      testComponent.minBufferPx = 100;
+      testComponent.maxBufferPx = 99;
+      expect(() => finishInit(fixture)).toThrow();
+    }));
+
     // TODO(mmalerba): Add test that it corrects the initial render if it didn't render enough,
     // once it actually does that.
   });
@@ -61,7 +67,7 @@ function finishInit(fixture: ComponentFixture<any>) {
 @Component({
   template: `
     <cdk-virtual-scroll-viewport
-        autosize [minBufferPx]="minBufferSize" [addBufferPx]="addBufferSize"
+        autosize [minBufferPx]="minBufferPx" [maxBufferPx]="maxBufferPx"
         [orientation]="orientation" [style.height.px]="viewportHeight"
         [style.width.px]="viewportWidth">
       <div class="item" *cdkVirtualFor="let size of items; let i = index" [style.height.px]="size"
@@ -88,8 +94,8 @@ class AutoSizeVirtualScroll {
   @Input() orientation = 'vertical';
   @Input() viewportSize = 200;
   @Input() viewportCrossSize = 100;
-  @Input() minBufferSize = 0;
-  @Input() addBufferSize = 0;
+  @Input() minBufferPx = 0;
+  @Input() maxBufferPx = 0;
   @Input() items = Array(10).fill(50);
 
   get viewportWidth() {

src/cdk/scrolling/fixed-size-virtual-scroll.ts

@@ -7,7 +7,6 @@
  */
 
 import {coerceNumberProperty} from '@angular/cdk/coercion';
-import {ListRange} from '@angular/cdk/collections';
 import {Directive, forwardRef, Input, OnChanges} from '@angular/core';
 import {Observable, Subject} from 'rxjs';
 import {distinctUntilChanged} from 'rxjs/operators';
@@ -28,16 +27,21 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   /** The size of the items in the virtually scrolling list. */
   private _itemSize: number;
 
-  /** The number of buffer items to render beyond the edge of the viewport. */
-  private _bufferSize: number;
+  /** The minimum amount of buffer rendered beyond the viewport (in pixels). */
+  private _minBufferPx: number;
+
+  /** The number of buffer items to render beyond the edge of the viewport (in pixels). */
+  private _maxBufferPx: number;
 
   /**
    * @param itemSize The size of the items in the virtually scrolling list.
-   * @param bufferSize The number of buffer items to render beyond the edge of the viewport.
+   * @param minBufferPx The minimum amount of buffer (in pixels) before needing to render more
+   * @param maxBufferPx The amount of buffer (in pixels) to render when rendering more.
    */
-  constructor(itemSize: number, bufferSize: number) {
+  constructor(itemSize: number, minBufferPx: number, maxBufferPx: number) {
     this._itemSize = itemSize;
-    this._bufferSize = bufferSize;
+    this._minBufferPx = minBufferPx;
+    this._maxBufferPx = maxBufferPx;
   }
 
   /**
@@ -59,11 +63,16 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   /**
    * Update the item size and buffer size.
    * @param itemSize The size of the items in the virtually scrolling list.
-   * @param bufferSize he number of buffer items to render beyond the edge of the viewport.
+   * @param minBufferPx The minimum amount of buffer (in pixels) before needing to render more
+   * @param maxBufferPx The amount of buffer (in pixels) to render when rendering more.
    */
-  updateItemAndBufferSize(itemSize: number, bufferSize: number) {
+  updateItemAndBufferSize(itemSize: number, minBufferPx: number, maxBufferPx: number) {
+    if (maxBufferPx < minBufferPx) {
+      throw Error('CDK virtual scroll: maxBufferPx must be greater than or equal to minBufferPx');
+    }
     this._itemSize = itemSize;
-    this._bufferSize = bufferSize;
+    this._minBufferPx = minBufferPx;
+    this._maxBufferPx = maxBufferPx;
     this._updateTotalContentSize();
     this._updateRenderedRange();
   }
@@ -112,34 +121,33 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
     }
 
     const scrollOffset = this._viewport.measureScrollOffset();
-    const firstVisibleIndex = Math.floor(scrollOffset / this._itemSize);
-    const firstItemRemainder = scrollOffset % this._itemSize;
-    const range = this._expandRange(
-        {start: firstVisibleIndex, end: firstVisibleIndex},
-        this._bufferSize,
-        Math.ceil((this._viewport.getViewportSize() + firstItemRemainder) / this._itemSize) +
-            this._bufferSize);
-    this._viewport.setRenderedRange(range);
-    this._viewport.setRenderedContentOffset(this._itemSize * range.start);
-
-    this._scrolledIndexChange.next(firstVisibleIndex);
-  }
-
-  /**
-   * Expand the given range by the given amount in either direction.
-   * @param range The range to expand
-   * @param expandStart The number of items to expand the start of the range by.
-   * @param expandEnd The number of items to expand the end of the range by.
-   * @return The expanded range.
-   */
-  private _expandRange(range: ListRange, expandStart: number, expandEnd: number): ListRange {
-    if (!this._viewport) {
-      return {...range};
+    const firstVisibleIndex = scrollOffset / this._itemSize;
+    const renderedRange = this._viewport.getRenderedRange();
+    const newRange = {start: renderedRange.start, end: renderedRange.end};
+    const viewportSize = this._viewport.getViewportSize();
+    const dataLength = this._viewport.getDataLength();
+
+    const startBuffer = scrollOffset - newRange.start * this._itemSize;
+    if (startBuffer < this._minBufferPx && newRange.start != 0) {
+      const expandStart = Math.ceil((this._maxBufferPx - startBuffer) / this._itemSize);
+      newRange.start = Math.max(0, newRange.start - expandStart);
+      newRange.end = Math.min(dataLength,
+          Math.ceil(firstVisibleIndex + (viewportSize + this._minBufferPx) / this._itemSize));
+    } else {
+      const endBuffer = newRange.end * this._itemSize - (scrollOffset + viewportSize);
+      if (endBuffer < this._minBufferPx && newRange.end != dataLength) {
+        const expandEnd = Math.ceil((this._maxBufferPx - endBuffer) / this._itemSize);
+        if (expandEnd > 0) {
+          newRange.end = Math.min(dataLength, newRange.end + expandEnd);
+          newRange.start = Math.max(0,
+              Math.floor(firstVisibleIndex - this._minBufferPx / this._itemSize));
+        }
+      }
     }
 
-    const start = Math.max(0, range.start - expandStart);
-    const end = Math.min(this._viewport.getDataLength(), range.end + expandEnd);
-    return {start, end};
+    this._viewport.setRenderedRange(newRange);
+    this._viewport.setRenderedContentOffset(this._itemSize * newRange.start);
+    this._scrolledIndexChange.next(Math.floor(firstVisibleIndex));
   }
 }
 
@@ -172,18 +180,27 @@ export class CdkFixedSizeVirtualScroll implements OnChanges {
   _itemSize = 20;
 
   /**
-   * The number of extra elements to render on either side of the scrolling viewport.
-   * Defaults to 5 elements.
+   * 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. Defaults to 100px.
+   */
+  @Input()
+  get minBufferPx(): number { return this._minBufferPx; }
+  set minBufferPx(value: number) { this._minBufferPx = coerceNumberProperty(value); }
+  _minBufferPx = 100;
+
+  /**
+   * The number of pixels worth of buffer to render for when rendering new items. Defaults to 200px.
    */
   @Input()
-  get bufferSize(): number { return this._bufferSize; }
-  set bufferSize(value: number) { this._bufferSize = coerceNumberProperty(value); }
-  _bufferSize = 5;
+  get maxBufferPx(): number { return this._maxBufferPx; }
+  set maxBufferPx(value: number) { this._maxBufferPx = coerceNumberProperty(value); }
+  _maxBufferPx = 200;
 
   /** The scroll strategy used by this directive. */
-  _scrollStrategy = new FixedSizeVirtualScrollStrategy(this.itemSize, this.bufferSize);
+  _scrollStrategy =
+      new FixedSizeVirtualScrollStrategy(this.itemSize, this.minBufferPx, this.maxBufferPx);
 
   ngOnChanges() {
-    this._scrollStrategy.updateItemAndBufferSize(this.itemSize, this.bufferSize);
+    this._scrollStrategy.updateItemAndBufferSize(this.itemSize, this.minBufferPx, this.maxBufferPx);
   }
 }

src/cdk/scrolling/scrolling.md

@@ -94,12 +94,22 @@ that it allows for better performance, since items do not need to be measured as
 </cdk-virtual-scroll-viewport>
 ```
 
-The fixed size strategy also supports setting the buffer size, i.e. the number of items rendered
-beyond the edge of the viewport. This can be adjusted by setting the `bufferSize` input. If
-`bufferSize` is not specified it defaults to 5 items.
+The fixed size strategy also supports setting a couple of buffer parameters that determine how much
+extra content is rendered beyond what is visible in the viewport. The first of these parameters is
+`minBufferPx`. The `minBufferPx` is the minimum amount of content buffer (in pixels) that the
+viewport must render. If the viewport ever detects that there is less buffered content it will
+immediately render more. The second buffer parameter is `maxBufferPx`. This tells the viewport how 
+much buffer space to render back up to when it detects that more buffer is required.
+
+The interaction of these two buffer parameters can be best illustrated with an example. Supposed 
+that we have the following parameters: `itemSize = 50`, `minBufferPx = 100`, `maxBufferPx = 250`. As
+the user is scrolling through the content the viewport detects that there is only `90px` of buffer
+remaining. Since this is below `minBufferPx` the viewport must render more buffer. It must render at
+least enough buffer to get back to `maxBufferPx`. In this case, it renders 4 items (an additional
+`200px`) to bring the total buffer size to `290px`, back above `maxBufferPx`.
 
 ```html
-<cdk-virtual-scroll-viewport itemSize="50" bufferSize="1">
+<cdk-virtual-scroll-viewport itemSize="50" minBufferPx="100" maxBufferPx="250">
   ...
 </cdk-virtual-scroll-viewport>
 ```