onScroll events and performance #160
Comments
|
hello :) here a very nice article about this features I hope it helps |
|
Thanks @F4b1n0u! I am actually using a FlatList in the real code of the project, but the headers don't do all the things I want them to do in the future. I guess my real question is if there will always be a bit of a delay when tracking things using reanimated. And if the choppy rendering I'm seeing is actually normal. Or if I did something wrong? |
|
Same issue with ScrollView |
|
Hi everyone, |
|
Hey I just stumbled upon this issue as well. Did you make some progress or have any idea why that happens? I also especially notice it during iOS Momentum scroll. I was wondering first if it was related to |
|
@osdnk Hi, any updates about this issue =)? Or can you say where to start research this issue(some place in code) ?) Thank you! |
|
I'm also seeing this issue - any progress? |
|
When adding more content into a |
|
Did some digging into this and it seems to be related to iOS optimization and the reanimated scheduler. Due to most battery optimizations not being active in the simulator this problem is most pronounced on a physical device. The major difference between the RN Core animated implementation and reanimated is that RN will update the nodes for each scroll event, and avoids too frequent updates by throttling the event itself. Reanimated on the other hand will accept and queue all events and ask the OS for an animation frame. That means reanimated is always at least one frame behind, which can be illustrated by scrolling up and down quickly where the other view seems to lag behind a bit. The other factor is that the OS decides how often it call the animation frame callback. It's supposed to do so as often as the frame rate if not specified, although in my findings it seems to also depend on when the OS thinks it's needed. So for example during direct manipulation with the finger, the refresh rate is high (60 FPS), but when the finger is off it's suddenly lowered (to 30FPS). I'll continue to investigate whether there are some workarounds, but my assumption is that it's going to be hard to get the same no-lag behaviour without also changing the way the scheduling works. |
osdnk
pushed a commit
that referenced
this issue
Jul 7, 2019
This PR is a proof of concept on how to address issues like #160 #309. As outlined in my comment, the problem with the current batched updates are that reanimated lags behind event processing by at least one event, in some cases up to 3-4. If using something like react-native-gesture-handler this is generally not noticeable as reanimated will be responsible for positioning all elements, but when deriving an animated value from a scroll event this would cause the elements positioned by reanimated to be out of sync with the scroll view. To address the lag, this PR introduces a concept of "direct events", that will bypass the queue and apply its effects immediately. Due to my unfamiliarity with the codebase I didn't really know how to do this in a nice way without also doing a quite big refactor. This is a low invasive change to illustrate how it can be addressed, I'm happy to receive feedback on how this can be done in a cleaner way. Test plan Using @lindesvard's reproduction of the issue https://github.com/lindesvard/reanimated-scroll-perf on a physical device, applying these changes should remove the lag and stuttering. Adverse effects Since the direct effects will bypass the queue, it will not retain the same order. Given the declarative nature of reanimated I think this should be OK in most cases, but still worth mentioning. A possible workaround is to flush the queue before applying direct events. The current way to determine direct events are by event name which is not particularly specific. In the odd case of naming collision, I don't see it having profound negative effects however.
|
Hi, it seems it is resolved in iOS, but, what about Android? any plan for this? |
osdnk
pushed a commit
that referenced
this issue
Nov 11, 2019
* Add CI integration (#278) Motivation In React Native Gesture Handler repo we have integration with travisCI to test if our Example apps are building and some detox e2e tests on iOS. I thought that it is not much work to add CI here as we already have config and project config is similar so it would most likely work out of the box with little tweaks. Changes Added YAML config file and Travis CI integration * Clarify set node description (#285) * Add a mock implementation for test runners (#276) I was recently using reanimated in react-navigation where I needed to mock it to be able to run it in Jest. It'll be great if the mock was in the library so we don't have to duplicate it in every project. I looked through the typescript definitions and added a mock function for almost everything. I didn't use anything specific to a test runner such as Jest, so it should be possible to use the mock with any test runner. * Create ReanimatedModule.web.js (#242) Prevents crashing when referenced in libs like react-navigation. * Fix example app's metro config on windows machines (#266) glob-to-regexp does not escape backslashes as of version 0.4 which causes issues on windows machines. replacing backslashes with slashes only on windows fixes the issue. * TVos support (#291) * Add typing for concat#0 concat#1 (#293) concat#1 can be useful to convert a numerical animated value to a string. * Update README.md (#301) * Remove react/react-native peerDependencies (#298) * 💄Refine TypeScript type for boolean functions (#305) * Get rid of Bezier's arguments check (#307) * Bump version -> 1.1.0 * Add Math.log() operator (#320) I had a need for Math.log in an Animation calculation. This code all seemed to work and I think I found all the touch points, but I'm not well versed on the native side of things so please let me know if anything needs to change. * 💄 Refine TS type (#314) * Fix android CI setup (#313) * fix(🐛): Add TS support for animated transforms (#323) * Replace forEach with for loop (#329) Since `__addChild` etc. are called so many times, using a for loop makes it faster by around 10ms at least (on iOS simulator, on device it might be even more). * Bypass animation queue for onScroll events (#312) This PR is a proof of concept on how to address issues like #160 #309. As outlined in my comment, the problem with the current batched updates are that reanimated lags behind event processing by at least one event, in some cases up to 3-4. If using something like react-native-gesture-handler this is generally not noticeable as reanimated will be responsible for positioning all elements, but when deriving an animated value from a scroll event this would cause the elements positioned by reanimated to be out of sync with the scroll view. To address the lag, this PR introduces a concept of "direct events", that will bypass the queue and apply its effects immediately. Due to my unfamiliarity with the codebase I didn't really know how to do this in a nice way without also doing a quite big refactor. This is a low invasive change to illustrate how it can be addressed, I'm happy to receive feedback on how this can be done in a cleaner way. Test plan Using @lindesvard's reproduction of the issue https://github.com/lindesvard/reanimated-scroll-perf on a physical device, applying these changes should remove the lag and stuttering. Adverse effects Since the direct effects will bypass the queue, it will not retain the same order. Given the declarative nature of reanimated I think this should be OK in most cases, but still worth mentioning. A possible workaround is to flush the queue before applying direct events. The current way to determine direct events are by event name which is not particularly specific. In the odd case of naming collision, I don't see it having profound negative effects however. * (react-native-reanimated.d.ts) fix TransformsStyle (#339) This fixes typescript error `Cannot find name 'TransformStyle'.`. * target iOS 9.0 (#346) * Make `ReanimatedModule.NAME` public (#356) This allow using the constant directly with `TurboReactPackage`. * Fix: Support optional string style types (#352) * Add support for nodes inside proc (#354) Motivation When creating multiple dynamic animations with react-native-reanimated the number of nodes that needs to be transported over the bridge is a performance challenge. One of the limitations of the current primitives offered by reanimated is the lack of function nodes. Being able to define node trees that can be reused will offer better performance. A previous attempt has been done in this pull request: #42 Implementation The suggested implementation tries to limit changes to the current source code by adding new nodes without changing the existing implementation too much. Three new nodes have been added: ParamNode CallFunctionNode FunctionNode A few changes to the NodeManager to be able to force reevaluation when calling functions have been added. The implementation uses a simple proc function in Javascript to create function nodes. Following is an example of how a function is defined and used. // Defining the function const mulXY = proc((x, y) => multipy(x, y)); // Using the function const myX = new Animated.Value(12); const myY = new Animated.Value(14); const myResult = mulXY(myX, myY); Technical The solution I have used in this implementation is to implement the ParamNode as a node that holds a reference to other nodes - returning the referenced node's value when evaluated and setting the referenced node's value when changed. Calling the function node is done using the CallFuncNode that is created when calling the function (on the JS side). When the CallFuncNode is evaluated, it performs the following steps: Start a new context Copy current argument nodes to the param nodes (using a Stack in the param node) Evaluate the function node's expression Pop the current argument nodes from the param nodes End context I have also changed NodeManager's invalidate strategy so that it will force a reevaluation of nodes when we are in a function call to avoid returning the same value for multiple functions calls with different parameter references. Using a stack in the ParamNode ensures that we can create recursive functions. Memory A single proc node should be created as a global constant in javascript since they are immediately attached and will never be removed. Platforms Both Android and iOS platform-specific code has been added in addition to the required JS code. Further plans Rewriting functionality in reanimated called often should be considered. Moving complex spring functions to functions could also be considered but is not necessarily part of this PR. Testing Convert built-in derived functionality to use proc nodes and verify that all examples work nicely. I have already done this for interpolate and easings and it seems to be working fine. Verify that no memory leaks exist. I believe that the current implementation supports caching of function evaluation - would be happy if someone else could confirm this. Co-authored-by: osdnk <micosa97@gmail.com> * 💄Refine typing for cond() (#359) * Fix: jest mock should use Animated.ScrollView (#344) Ran into the issue here: satya164/react-native-tab-view#845 This is because `ScrollView` doesn't have `.getNode()` but `Animated.ScrollView` does. This change fixes the error. I think this is the correct fix. cc @simonbuerger * doc: fix broken code block (#365) replaced backticks(?) to "`" * Remove deprecated componentWillMount (#342) Refactors the create animated component removing the deprecated componentWillMount lifecycle and moving the attachProps to the constructor. * Bump version -> 1.2.0 * Update README.md (#384) Fix typo * FIX javaCompileProvider (#393) FIX for #315 * Remove prevPosition in SpringState (#386) Creates a type name that is non-specific to one animation function. This is useful when an animation state is shared between animation functions (spring + decay for instance). * add event mapping type (#388) * Add createAnimatedComponent to mock (#389) * Specify component typings as React.Component classes (#373) *Rationale* Currently `Animated.View`, `Animated.Text`, `Animated.Image`, `Animated.ScrollView` are exported as `const`ants with a `React.ComponentClass` type. This means they cannot be used as generics in (for example) `React.Ref<Animated.View>`, and that the useful `getNode()` method is not typed. PR addresses this by changing the constants to `React.Component` class exports, decorated with the `getNode()` method * migrate to AndroidX (#395) Migrated source code to use AndroidX, and Example to use React Native 0.60.5. Also removed unnecessary files like flow and buck configurations, Gradle wrapper in android, etc... Removed Geolocation module from Example on iOS. I can compile for Android and iOS versions of the example app, but metro fails to resolve react-native-reanimation from '../src/Animated' via babel configuration. **HELP**: Help make metro work for example app. * Update Example app to use RN 0.61 (#401) * Optimize constant nodes creation and cleanup (#403) This PR optimizes the way we create and cleanup value nodes. When adapting constants (e.g. when doing interpolate({ inputRange: [0,1]}) we generate new nodes for start and end of the range) we've been creating new animated value nodes. This kind of nodes can never be updated on the native side and hence there is no need to send its final value back to JS when they get detached. On top of there there are several constant values that are frequently used across the library and product code with reanimated. Those can only be instantiated once and reused in all the places. That list includes 0, 1 and -1 for now. These two changes reduce the number of nodes instantiated by one third on colors/index.js example (from 88 down to 57) and reduce the number of nodes we call getValue for by 94% (from 88 down to 5). * Update iOS Example project to build with RN 0.61 (#402) * Update android Example project build.gradle to build with RN 0.61 (#407) * fix(🐛): Fix Android bug when dealing with string animated values (#311) This PR takes @msand and @osdnk from #176 into consideration. fixes #300 * Bump version -> 1.3.0 * Added web support (#390) # Why - [Preview](https://twitter.com/Baconbrix/status/1173825701192929280?s=20) - [Demo](https://reanimated.netlify.com) # How - Enabled JS runner - Added JS to broken runner nodes: `CallFunc`, `Param` - Added support for web gesture-handler - Added expo-web to Example - Added `format` node for web color transform # Test Plan - [Example](https://reanimated.netlify.com) should run - Transitions API is not supported yet * [iOS] Fix floor() implementation (#362) fixes #316 * Fix invalid url in readme.md (example in event handling) (#418) * fixed EventArgFunc typings (#422) * 💄Export AnimateProps & AnimateStyle (#429) Exporting these types can be useful for component definition. For instance: ```ts interface FlexibleCardProps extends CardProps { style?: Animated.AnimateStyle<ImageStyle>; } export const FlexibleCard = ({ card, style }: FlexibleCardProps) => ( <Animated.Image style={[styles.flexibleContainer, style]} source={card.source} /> ); ``` ``` * bump -> 1.3.1 * Fix web support (#445) * bump -> 1.3.2 * Add tvos podspec support (#440) * 💄Refine debug() typing (#434) Non-node value crash with debug() * Bump morgan from 1.9.0 to 1.9.1 in /Example (#444) Bumps [morgan](https://github.com/expressjs/morgan) from 1.9.0 to 1.9.1. - [Release notes](https://github.com/expressjs/morgan/releases) - [Changelog](https://github.com/expressjs/morgan/blob/master/HISTORY.md) - [Commits](expressjs/morgan@1.9.0...1.9.1) Signed-off-by: dependabot[bot] <support@github.com> * Bump extend from 3.0.1 to 3.0.2 in /Example (#442) Bumps [extend](https://github.com/justmoon/node-extend) from 3.0.1 to 3.0.2. - [Release notes](https://github.com/justmoon/node-extend/releases) - [Changelog](https://github.com/justmoon/node-extend/blob/master/CHANGELOG.md) - [Commits](justmoon/node-extend@v3.0.1...v3.0.2) Signed-off-by: dependabot[bot] <support@github.com> * Bump js-yaml from 3.12.0 to 3.13.1 in /Example (#441) Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.12.0 to 3.13.1. - [Release notes](https://github.com/nodeca/js-yaml/releases) - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@3.12.0...3.13.1) Signed-off-by: dependabot[bot] <support@github.com> * Bump mixin-deep from 1.3.1 to 1.3.2 in /Example (#443) Bumps [mixin-deep](https://github.com/jonschlinkert/mixin-deep) from 1.3.1 to 1.3.2. - [Release notes](https://github.com/jonschlinkert/mixin-deep/releases) - [Commits](jonschlinkert/mixin-deep@1.3.1...1.3.2) Signed-off-by: dependabot[bot] <support@github.com> * fix: mock.js NOOP to allow for "new" constructor call (#439) i'm getting errors in my tests from this mock as the new Value(0) call throws for arrow functions, this seems to work when i updated my mock.js in node_modules. * 🎣Allow to pass a callback as parameter of useCode() (#408) fixes #343 This can enable us to save some unnecessary node creation but more importantly, it will enable us to benefit from the Exhaustive Deps eslint rule. This rule expect a callback as first parameter: https://github.com/facebook/react/blob/master/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js#L64 This rule is incredibly useful to avoid bugs when using hooks that have dependencies. * Add Jest doc to the README Following #355 (comment) it's true that there is no easily accessible documentation about how to set up Jest tests with reanimated. * Fix node dirty marking when new connection between nodes is made (#450) This change fixes the problem where a parent node were marked as dirty while adding connection between nodes instead of the children node. We should be only marking children node as if we mark parent node there is a chance that this would trigger updates in leave nodes (final nodes) that should not be affected as the children added to parent node cannot impact its value. On top of that the marking change relealed a bug where we initialized loop ID inproperly. We should be initializing it with a value that'd allow the node to be evaluated at least once. It's been working before as every node would be dirty marked. With the above change the root nodes were no longer being marked but we'd still require them to be evaluated. The invalid initial setting for last loop ID was making us skip evaluation of those parent nodes. In order to test this change I created the following sample app: https://gist.github.com/7fce6b4a891f0284baa17d0d63455495 In this app there is a button that creates a new view that hooks into an existing animated value. Before this change when the view was added we'd reevaluate all the nodes that depend on that value and hence we'd get a consol.warn being shown twice (first on the initial render and then second time when we add the view). WIth this change in the `call` node should not be evaluated more than once and only on the initial render. * Bump -> 1.4.0 * Fix ESLint dev setup and errors (#421) * Fix ESLint setup and devDependencies - Fix version of types/react-native - Update husky and lint-staged * Fix lint errors * Rewrite README.md * Format and update 04.transitions.md * Format and update 05.value.md * Format and update 07.block.md * Format and update 09.code.md * Format and update 10.event.md * Format and update 11.baseNodes/set.md * Add 11.baseNodes/log.md * Format and update 11.baseNodes/floor.md * Format and update 11.baseNodes/ceil.md * Update min, max docs * Add proc node docs * Format pages * Update README.md with latest version * Remove unnecessary assets folders * Update README.md * Remove meme :c * Fix links and move 14.backward.md to 13.declarative.md * Remove 1st person narrative in about page * Remove assets/ from component-docs config We don't need static assets right now
|
Hey @alikazemkhanloo It seem like this exact problem applied only to iOS and the fix from @oblador resolved the issue. What I think is happening in your case is that you may be expiriencing issue with some layout property and not translateY as provided in the repro scenario here. I tried the repro case on an Android phone and simulator and didn't expirience any lagging when translate is used. If this is the case for you feel free to reopen this issue. If you have problem with some other properties (e.g. layout props as described in #324) please respond on the relevant issues or open a new one. |
|
I'm experiencing same issue on Android, using my own personal phone. on iOS I don't see any problems. |
|
@papigers Hey I am facing the same issue, how did you address it ? |
|
Would like an update on this too :) Implemented a screen with a large header and sticky tabs. The tabs "stick" using a translateY and reanimated. On android, when you lift your finger of the screen, the sticky tabs "lag" behind a bit. |
|
Me too |
|
Android fling animation lag also present on emulator |
|
I've created new separated issue for this case #1554 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi! Thanks for an interesting library!
I have been playing around with using reanimated to build an header that tracks a scrollview (or flatlist maybe) and have encountered some performance issues I can't figure out.
An example of this can be seen in this snack: https://snack.expo.io/rkT2_e3Z4
On device (iPhone X, iOS 12.1.2) I can see the following:
list.
As I add more things to the header these issues become more apparent and it just does not look good. Using the Animated library from react-native gets rid of these issues, but then there are some nice things I can't do that reanimated could solve :) Is there anything wrong in my code, or any way of getting rid of these issues?
Thanks!
Micke
The text was updated successfully, but these errors were encountered: