From c87de765f6a9ebf656c188fa2115a1ba01b7939c Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Wed, 24 Apr 2019 17:01:48 -0700 Subject: [PATCH] don't throttle below 16ms 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 --- .../Animated/src/components/AnimatedScrollView.js | 4 +++- Libraries/Components/ScrollView/ScrollView.js | 15 ++++++++++----- React/Views/ScrollView/RCTScrollView.m | 7 +++++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/Libraries/Animated/src/components/AnimatedScrollView.js b/Libraries/Animated/src/components/AnimatedScrollView.js index aae27469b..eb5cf2752 100644 --- a/Libraries/Animated/src/components/AnimatedScrollView.js +++ b/Libraries/Animated/src/components/AnimatedScrollView.js @@ -14,4 +14,6 @@ const ScrollView = require('ScrollView'); const createAnimatedComponent = require('createAnimatedComponent'); -module.exports = createAnimatedComponent(ScrollView, {scrollEventThrottle: 16}); +module.exports = createAnimatedComponent(ScrollView, { + scrollEventThrottle: 0.0001, +}); diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index b685c94ac..37e951edb 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -206,11 +206,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, diff --git a/React/Views/ScrollView/RCTScrollView.m b/React/Views/ScrollView/RCTScrollView.m index 5f7f0069e..e77ff1d27 100644 --- a/React/Views/ScrollView/RCTScrollView.m +++ b/React/Views/ScrollView/RCTScrollView.m @@ -697,16 +697,19 @@ RCT_SCROLL_EVENT_HANDLER(scrollViewDidScrollToTop, onScrollToTop) - (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