Merge pull request #478 from bvaughn/overscan-predictive-shift

Changed predictive overscan cell shifting to improve perf

CHANGELOG.md

@@ -1,6 +1,10 @@
 Changelog
 ------------
 
+##### 8.5.3
+Changed overscan rows/cols behavior as described [here](https://github.com/bvaughn/react-virtualized/pull/478).
+This change targets performance improvements only and should have no other noticeable impact.
+
 ##### 8.5.2
 Added guard against potential `null` return value from `getComputedStyle` for hidden elements (see PR #465).
 

package.json

@@ -3,7 +3,7 @@
   "description": "React components for efficiently rendering large, scrollable lists and tabular data",
   "author": "Brian Vaughn <brian.david.vaughn@gmail.com>",
   "user": "bvaughn",
-  "version": "8.5.2",
+  "version": "8.5.3",
   "homepage": "https://github.com/bvaughn/react-virtualized",
   "main": "dist/commonjs/index.js",
   "module": "dist/es/index.js",

source/Grid/Grid.js

@@ -4,7 +4,7 @@ import cn from 'classnames'
 import calculateSizeAndPositionDataAndUpdateScrollOffset from './utils/calculateSizeAndPositionDataAndUpdateScrollOffset'
 import ScalingCellSizeAndPositionManager from './utils/ScalingCellSizeAndPositionManager'
 import createCallbackMemoizer from '../utils/createCallbackMemoizer'
-import getOverscanIndices, { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FIXED, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices'
+import getOverscanIndices, { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices'
 import getScrollbarSize from 'dom-helpers/util/scrollbarSize'
 import shallowCompare from 'react-addons-shallow-compare'
 import updateScrollIndexHelper from './utils/updateScrollIndexHelper'
@@ -210,8 +210,8 @@ export default class Grid extends Component {
 
     this.state = {
       isScrolling: false,
-      scrollDirectionHorizontal: SCROLL_DIRECTION_FIXED,
-      scrollDirectionVertical: SCROLL_DIRECTION_FIXED,
+      scrollDirectionHorizontal: SCROLL_DIRECTION_FORWARD,
+      scrollDirectionVertical: SCROLL_DIRECTION_FORWARD,
       scrollLeft: 0,
       scrollTop: 0
     }
@@ -705,9 +705,7 @@ export default class Grid extends Component {
     this._cellCache = {}
 
     this.setState({
-      isScrolling: false,
-      scrollDirectionHorizontal: SCROLL_DIRECTION_FIXED,
-      scrollDirectionVertical: SCROLL_DIRECTION_FIXED
+      isScrolling: false
     })
   }
 

source/Grid/Grid.test.js

@@ -4,7 +4,7 @@ import { findDOMNode } from 'react-dom'
 import { Simulate } from 'react-addons-test-utils'
 import { render } from '../TestUtils'
 import Grid, { DEFAULT_SCROLLING_RESET_TIME_INTERVAL } from './Grid'
-import { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FIXED, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices'
+import { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices'
 
 const DEFAULT_COLUMN_WIDTH = 50
 const DEFAULT_HEIGHT = 100
@@ -800,11 +800,11 @@ describe('Grid', () => {
         scrollToColumn: 25,
         scrollToRow: 50
       }))
-      expect(helper.columnOverscanStartIndex()).toEqual(20)
+      expect(helper.columnOverscanStartIndex()).toEqual(22)
       expect(helper.columnOverscanStopIndex()).toEqual(27)
       expect(helper.columnStartIndex()).toEqual(22)
       expect(helper.columnStopIndex()).toEqual(25)
-      expect(helper.rowOverscanStartIndex()).toEqual(41)
+      expect(helper.rowOverscanStartIndex()).toEqual(46)
       expect(helper.rowOverscanStopIndex()).toEqual(55)
       expect(helper.rowStartIndex()).toEqual(46)
       expect(helper.rowStopIndex()).toEqual(50)
@@ -835,15 +835,6 @@ describe('Grid', () => {
         scrollTop: 50
       }))
 
-      expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_FIXED)
-      expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_FIXED)
-
-      simulateScroll({
-        grid,
-        scrollLeft: 100,
-        scrollTop: 100
-      })
-
       expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_FORWARD)
       expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_FORWARD)
 
@@ -855,9 +846,18 @@ describe('Grid', () => {
 
       expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_BACKWARD)
       expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_BACKWARD)
+
+      simulateScroll({
+        grid,
+        scrollLeft: 100,
+        scrollTop: 100
+      })
+
+      expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_FORWARD)
+      expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_FORWARD)
     })
 
-    it('should overscan more in the direction being scrolled', async (done) => {
+    it('should overscan in the direction being scrolled', async (done) => {
       const helper = createHelper()
 
       let onSectionRenderedResolve
@@ -891,26 +891,36 @@ describe('Grid', () => {
 
       // It should overscan in the direction being scrolled while scroll is in progress
       expect(helper.columnOverscanStartIndex()).toEqual(4)
-      expect(helper.columnOverscanStopIndex()).toEqual(11)
+      expect(helper.columnOverscanStopIndex()).toEqual(9)
       expect(helper.columnStartIndex()).toEqual(4)
       expect(helper.columnStopIndex()).toEqual(7)
       expect(helper.rowOverscanStartIndex()).toEqual(10)
-      expect(helper.rowOverscanStopIndex()).toEqual(24)
+      expect(helper.rowOverscanStopIndex()).toEqual(19)
       expect(helper.rowStartIndex()).toEqual(10)
       expect(helper.rowStopIndex()).toEqual(14)
 
-      // Allow scrolling timeout to complete so that cell cache is reset
-      await new Promise(resolve => setTimeout(resolve, 500))
+      // Wait until the onSectionRendered handler / debouncer has processed
+      onSectionRenderedPromise = new Promise(resolve => {
+        onSectionRenderedResolve = resolve
+      })
+
+      simulateScroll({
+        grid,
+        scrollLeft: 100,
+        scrollTop: 100
+      })
+
+      await onSectionRenderedPromise
 
       // It reset overscan once scrolling has finished
-      expect(helper.columnOverscanStartIndex()).toEqual(2)
-      expect(helper.columnOverscanStopIndex()).toEqual(9)
-      expect(helper.columnStartIndex()).toEqual(4)
-      expect(helper.columnStopIndex()).toEqual(7)
-      expect(helper.rowOverscanStartIndex()).toEqual(5)
-      expect(helper.rowOverscanStopIndex()).toEqual(19)
-      expect(helper.rowStartIndex()).toEqual(10)
-      expect(helper.rowStopIndex()).toEqual(14)
+      expect(helper.columnOverscanStartIndex()).toEqual(0)
+      expect(helper.columnOverscanStopIndex()).toEqual(5)
+      expect(helper.columnStartIndex()).toEqual(2)
+      expect(helper.columnStopIndex()).toEqual(5)
+      expect(helper.rowOverscanStartIndex()).toEqual(0)
+      expect(helper.rowOverscanStopIndex()).toEqual(9)
+      expect(helper.rowStartIndex()).toEqual(5)
+      expect(helper.rowStopIndex()).toEqual(9)
 
       done()
     })

source/Grid/utils/getOverscanIndices.js

@@ -1,5 +1,4 @@
 export const SCROLL_DIRECTION_BACKWARD = -1
-export const SCROLL_DIRECTION_FIXED = 0
 export const SCROLL_DIRECTION_FORWARD = 1
 
 /**
@@ -16,15 +15,15 @@ export default function getOverscanIndices ({ cellCount, overscanCellsCount, scr
   let overscanStartIndex
   let overscanStopIndex
 
-  if (scrollDirection === SCROLL_DIRECTION_FORWARD) {
-    overscanStartIndex = startIndex
-    overscanStopIndex = stopIndex + overscanCellsCount * 2
-  } else if (scrollDirection === SCROLL_DIRECTION_BACKWARD) {
-    overscanStartIndex = startIndex - overscanCellsCount * 2
-    overscanStopIndex = stopIndex
-  } else {
-    overscanStartIndex = startIndex - overscanCellsCount
-    overscanStopIndex = stopIndex + overscanCellsCount
+  switch (scrollDirection) {
+    case SCROLL_DIRECTION_FORWARD:
+      overscanStartIndex = startIndex
+      overscanStopIndex = stopIndex + overscanCellsCount
+      break
+    case SCROLL_DIRECTION_BACKWARD:
+      overscanStartIndex = startIndex - overscanCellsCount
+      overscanStopIndex = stopIndex
+      break
   }
 
   return {

source/Grid/utils/getOverscanIndices.test.js

@@ -1,11 +1,16 @@
 import getOverscanIndices, {
   SCROLL_DIRECTION_BACKWARD,
-  SCROLL_DIRECTION_FIXED,
   SCROLL_DIRECTION_FORWARD
 } from './getOverscanIndices'
 
 describe('getOverscanIndices', () => {
-  function testHelper (cellCount, startIndex, stopIndex, overscanCellsCount, scrollDirection = SCROLL_DIRECTION_FIXED) {
+  function testHelper ({
+    cellCount,
+    startIndex,
+    stopIndex,
+    overscanCellsCount,
+    scrollDirection
+  }) {
     return getOverscanIndices({
       cellCount,
       overscanCellsCount,
@@ -16,41 +21,89 @@ describe('getOverscanIndices', () => {
   }
 
   it('should not overscan if :overscanCellsCount is 0', () => {
-    expect(testHelper(100, 10, 20, 0)).toEqual({
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 10,
+        stopIndex: 20,
+        overscanCellsCount: 0,
+        scrollDirection: SCROLL_DIRECTION_BACKWARD
+      })
+    ).toEqual({
       overscanStartIndex: 10,
       overscanStopIndex: 20
     })
-  })
 
-  it('should overscan by the specified :overscanCellsCount', () => {
-    expect(testHelper(100, 10, 20, 10)).toEqual({
-      overscanStartIndex: 0,
-      overscanStopIndex: 30
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 10,
+        stopIndex: 20,
+        overscanCellsCount: 0,
+        scrollDirection: SCROLL_DIRECTION_FORWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 10,
+      overscanStopIndex: 20
     })
   })
 
-  it('should double the overscan in the direction being scrolled', () => {
-    expect(testHelper(100, 20, 30, 10, SCROLL_DIRECTION_FORWARD)).toEqual({
+  it('should overscan forward', () => {
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 20,
+        stopIndex: 30,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_FORWARD
+      })
+    ).toEqual({
       overscanStartIndex: 20,
-      overscanStopIndex: 50
+      overscanStopIndex: 40
     })
+  })
 
-    expect(testHelper(100, 20, 30, 10, SCROLL_DIRECTION_BACKWARD)).toEqual({
-      overscanStartIndex: 0,
+  it('should overscan backward', () => {
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 20,
+        stopIndex: 30,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_BACKWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 10,
       overscanStopIndex: 30
     })
   })
 
   it('should not overscan beyond the start of the list', () => {
-    expect(testHelper(100, 5, 15, 10)).toEqual({
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 5,
+        stopIndex: 15,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_BACKWARD
+      })
+    ).toEqual({
       overscanStartIndex: 0,
-      overscanStopIndex: 25
+      overscanStopIndex: 15
     })
   })
 
   it('should not overscan beyond the end of the list', () => {
-    expect(testHelper(25, 10, 20, 10)).toEqual({
-      overscanStartIndex: 0,
+    expect(
+      testHelper({
+        cellCount: 25,
+        startIndex: 10,
+        stopIndex: 20,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_FORWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 10,
       overscanStopIndex: 24
     })
   })

source/List/List.test.js

@@ -296,7 +296,7 @@ describe('List', () => {
         overscanRowCount: 10,
         scrollToIndex: 30
       }))
-      expect(overscanStartIndex).toEqual(11)
+      expect(overscanStartIndex).toEqual(21)
       expect(startIndex).toEqual(21)
       expect(stopIndex).toEqual(30)
       expect(overscanStopIndex).toEqual(40)

source/Table/Table.test.js

@@ -778,7 +778,7 @@ describe('Table', () => {
         overscanRowCount: 10,
         scrollToIndex: 30
       }))
-      expect(overscanStartIndex).toEqual(13)
+      expect(overscanStartIndex).toEqual(23)
       expect(startIndex).toEqual(23)
       expect(stopIndex).toEqual(30)
       expect(overscanStopIndex).toEqual(40)