Skip to content

Commit

Permalink
don't throttle below 16ms
Browse files Browse the repository at this point in the history
Summary: For some reason the scroll events are sometimes generated with highly irregular spacing, some coming less than a millisecond apart. For interactions that must track scrolling exactly, this can cause them to glitch. With a scroll throttle of less than 17 ms, the intention is clear that the UI should be updated in sync with the scroll view so we shouldn't drop any events.

Reviewed By: PeteTheHeat

Differential Revision: D15068841

fbshipit-source-id: 730e7cb29cc3ddae66f37cf7392e02e0cc9d7844
  • Loading branch information
sahrens authored and grabbou committed Apr 30, 2019
1 parent 379874d commit 39776a8
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 8 deletions.
4 changes: 3 additions & 1 deletion Libraries/Animated/src/components/AnimatedScrollView.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ const ScrollView = require('ScrollView');

const createAnimatedComponent = require('createAnimatedComponent');

module.exports = createAnimatedComponent(ScrollView);
module.exports = createAnimatedComponent(ScrollView, {
scrollEventThrottle: 0.0001,
});
15 changes: 10 additions & 5 deletions Libraries/Components/ScrollView/ScrollView.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,16 @@ type IOSProps = $ReadOnly<{|
* (as a time interval in ms). A lower number yields better accuracy for code
* that is tracking the scroll position, but can lead to scroll performance
* problems due to the volume of information being send over the bridge.
* You will not notice a difference between values set between 1-16 as the
* JS run loop is synced to the screen refresh rate. If you do not need precise
* scroll position tracking, set this value higher to limit the information
* being sent across the bridge. The default value is zero, which results in
* the scroll event being sent only once each time the view is scrolled.
*
* Values between 0 and 17ms indicate 60fps updates are needed and throttling
* will be disabled.
*
* If you do not need precise scroll position tracking, set this value higher
* to limit the information being sent across the bridge.
*
* The default value is zero, which results in the scroll event being sent only
* once each time the view is scrolled.
*
* @platform ios
*/
scrollEventThrottle?: ?number,
Expand Down
7 changes: 5 additions & 2 deletions React/Views/ScrollView/RCTScrollView.m
Original file line number Diff line number Diff line change
Expand Up @@ -676,16 +676,19 @@ - (void)removeScrollListener:(NSObject<UIScrollViewDelegate> *)scrollListener

- (void)scrollViewDidScroll:(UIScrollView *)scrollView
{
[self updateClippedSubviews];
NSTimeInterval now = CACurrentMediaTime();
[self updateClippedSubviews];
/**
* TODO: this logic looks wrong, and it may be because it is. Currently, if _scrollEventThrottle
* is set to zero (the default), the "didScroll" event is only sent once per scroll, instead of repeatedly
* while scrolling as expected. However, if you "fix" that bug, ScrollView will generate repeated
* warnings, and behave strangely (ListView works fine however), so don't fix it unless you fix that too!
*
* We limit the delta to 17ms so that small throttles intended to enable 60fps updates will not
* inadvertantly filter out any scroll events.
*/
if (_allowNextScrollNoMatterWhat ||
(_scrollEventThrottle > 0 && _scrollEventThrottle < (now - _lastScrollDispatchTime))) {
(_scrollEventThrottle > 0 && _scrollEventThrottle < MAX(17, now - _lastScrollDispatchTime))) {

if (_DEPRECATED_sendUpdatedChildFrames) {
// Calculate changed frames
Expand Down

0 comments on commit 39776a8

Please sign in to comment.