Add setImmediate implementation to the UI runtime #3970

kmagiera · 2023-01-18T22:58:25Z

Summary

This PR adds an implementation of web's setImmediate API to the reanimated's UI runtime. We utilize this implementation in mappers and event handlers for more accurate animation/gesture tracking as well as unblock further optimizations.

There are several things that this PR changes:

We add event timestamp metadata to touch/scroll/sensor/etc events. This change is in preparation of providing a true system timestamps for such events (we don't do that yet but instead call system timer in order to pass down the current time)
We update performance.now API to use the same reference point for the provided time as requestAnimationFrame API. Before performance.now would use a different reference point which made it impossible to use it as a starting point for animations.
We update event handler callbacks such that they perform a flush for rAF and immediates queues
We populate global.__frameTimestamp for all the code that executes within requestAnimationFrame and event callbacks. We then use it in useAnimatedStyle as a reference point for starting animations and in case it wasn't set, we use performance.now
We add some code in initializers.ts to define setImmediate method and to register method for flushing immediates queue
We introduce set-immediate-based batching for runOnUI calls on the main RN runtime. We then flush all "immediates" after the whole batch is processed on the UI runtime.

Test plan

Check a bunch of examples from the example app: "Use animated style", "Bokeh", "Drag and snap".
For touch based interaction verify that movement doesn't lag behind the event. The way this can be done is by screen recording drag and snap example with touch indicators on, then checking on the recorded clip in between frames whether there is a correct movement aligned with the updates of the touch indicator.
The above needs to be done for all configurations: Android/iOS/Fabric/Paper

… things and need to pull in changes from main

This reverts commit 924a1b1.

jwajgelt · 2023-02-21T12:41:58Z

android/src/main/cpp/NativeProxy.cpp

@@ -323,7 +303,7 @@ void NativeProxy::installJSIBindings(
  std::shared_ptr<UIManager> uiManager =
      binding->getScheduler()->getUIManager();
  module->setUIManager(uiManager);
-  module->setNewestShadowNodesRegistry(newestShadowNodesRegistry_);
+  module->setNewestShadowNodesRegistry(newezstShadowNodesRegistry_);


Suggested change

module->setNewestShadowNodesRegistry(newezstShadowNodesRegistry_);

module->setNewestShadowNodesRegistry(newestShadowNodesRegistry_);

jwajgelt · 2023-02-21T12:45:40Z

Common/cpp/NativeModules/NativeReanimatedModule.cpp

+  std::vector<FrameCallback> callbacks = frameCallbacks;
+  frameCallbacks.clear();


You should be able to just move-construct callbacks from frameCallbacks here, right?

Suggested change

std::vector<FrameCallback> callbacks = frameCallbacks;

frameCallbacks.clear();

std::vector<FrameCallback> callbacks = std::move(frameCallbacks);

This wasn't my code so prefer not to make this change. Is this behavior defined that the original vector gets reset when moved? Typically move constructors does not specify behavior of the moved object as normally the assumption is that the moved object is never used again. Here we still want to use the vector

It should be fine.
C++ standard says that the moved-from object is left in a "valid but unspecified state" (so it should be fine to use the old vector later), and the std::vector spec further specifies that "After the move, other is guaranteed to be empty()." when move-constructing a vector, so it should clear it as well.

In case you don't feel safe using a vector after moving from it, I would still suggest doing something like

Suggested change

std::vector<FrameCallback> callbacks = frameCallbacks;

frameCallbacks.clear();

std::vector<FrameCallback> callbacks;

std::swap(callbacks, frameCallbacks)

since this is unambiguous in what it does, but still avoids copies/allocations.

I'd just like to avoid copying the callbacks here, since they can potentially capture a bunch of things - in this case it's a couple of shared_ptrs, which means unnecessary work on the atomic reference counters inside.

…e mocked when running tests on node 14

piaskowyk · 2023-02-22T09:15:53Z

src/reanimated2/mappers.ts

    if (mappers.size !== sortedMappers.length) {
      updateMappersOrder();
    }
    for (const mapper of sortedMappers) {
      if (mapper.dirty) {
        mapper.dirty = false;
-        mapper.worklet();
+        mapper.worklet(7);


Why this 7 is here?

idk, must've got here by mistake, mapper's worklets don't take arguments

piaskowyk · 2023-02-22T09:32:32Z

src/reanimated2/threads.ts

+    }
+    _runOnUIQueue.push([worklet, args]);
+    if (_runOnUIQueue.length === 1) {
+      setImmediate(() => {


This is Reanimated or react-native setImmediate? If this is setImmediate from react-native, could we add some comment or use different naming to identify two version easily.

It is meant to the same setImmediate. We provide a compatibility layer with the exception that reanimated's version doesn't support cancelling callbacks (yet). So we aim for both setImmediate from RN runtime and UI runtimes to be compatible and have same behavior hence we use the standard naming

piaskowyk · 2023-02-22T09:44:05Z

src/reanimated2/mappers.ts

-      requestAnimationFrame(mapperFrame);
-      frameRequested = true;
+    if (!runRequested) {
+      setImmediate(mapperRun);


Why setImmediate is better than requestAnimationFrame here?

rAF will delay the run of the mappers by one frame – the callback will run happen in the following frame. Whereas setImmediate will execute the callback at the end of the tasks that are currently scheduled on the run loop. The reason we schedule the run in the first place is that there is a chance multiple mappers gets updated in one frame and so we want to run the updates only once as it requires traversing dependency graph.

One issue with this implementation is that it's historically very problematic with Next.js, since setImmediate is not available on the server.

The current behavior leads to #4140

Would this work?

const scheduleUpdates = typeof window != 'undefined' ? global.setImmediate : requestAnimationFrame;

bglgwyng · 2023-02-23T05:18:35Z

@kmagiera

I posted a question #3925 a month ago and haven't gotten any answer yet.

I just saw this PR by chance and I don't know the details and context of this PR at all. However, the quotations sound quite relevant to my previous question.

We populate global.__frameTimestamp for all the code that executes within requestAnimationFrame and event callbacks. We then use it in useAnimatedStyle as a reference point for starting animations and in case it wasn't set, we use performance.now

We introduce set-immediate-based batching for runOnUI calls on the main RN runtime. We then flush all "immediates" after the whole batch is processed on the UI runtime.

Does this change solves the problem discussed in the discussion?

kmagiera · 2023-02-23T06:04:27Z

This change impacts the way we execute updates on the main thread. Now they are all batches and will run in a single transaction. So I'd expect this pr to change the way your example work but the easiest way to find out would be to just try it out.

## Summary Before this change we'd only call `performOperations` for a subset of filtered events (initially introduced in #312). The event's that weren't considered "direct" would require additional animation frame for having their updates flushed. This change makes it so that we call `performOperations` for all types of events therefore matching the behavior [on Android](https://github.com/software-mansion/react-native-reanimated/blob/99b8b3ed56e36ca615cce7164ccaf04d154571b1/android/src/main/java/com/swmansion/reanimated/NodesManager.java#L283) A consequence of this change was that for some event types, the updates reanimated was performing weren't getting flushed onto screen. Namely, we noticed this problem in Pager example where a view pager component with some custom set of events is used and event handlers update shared values. Even though such event would trigger shared value update, and these updates would trigger the style to recalculate and we'd even call updateProps method to apply the updated props, we'd still see no result as in that example the changes require layout run. Without `performOperation` call the layout would not be executed unless react would rerender or other time-based animation would run. The problem became apparent after #3970 where we changed the place where updates are performed from requestAnimationFrame to setImmediate. Before this change, since we were running the updates in "animation frame" the `performOperation` method was being run by the frame scheduler. We, however were getting these updates delayed by one frame because of that. This issue also wasn't noticed prior to shareable rewrite from #3722 because before, we were always starting frame updater for every single update happening to shared value even if it was due to an event. As a result, we were getting the stuff updated on screen but again, with a delay of one frame. ## Test plan Run pager example on iOS.

… flow as other types of events (#4098) ## Summary This diff fixes an issue with animated views not updating when animating along keyboard. This problem regressed after #3970 where we changed the logic of when enqueued callbacks for animation frame and immediates are flushed. The logic has been moved from c++ to js and we added some code to execute animation frame and immediates as a part of event handling process. However, keyboard events have a separate flow of providing event data (we should unify this at some point), and we missed updating it in that place. This PR adds similar logic to [what we use for regular events](https://github.com/software-mansion/react-native-reanimated/blob/535b5d64c0720da3a9a39d99a67e878fe09b5154/src/reanimated2/core.ts#L117) into the code responsible for handling keyboard events. ## Test plan Run animated keyboard example on iOS and Android.

## Summary This PR fixes issue with UI not updating when style changes are connected to events on iOS Fabric. This problem has been introduced in #3970 where we migrated from using `rAF` and started using `setImmediate` instead. As a result we had to manually flush set-immediates queue in certain conditions. Specifically this was already happening after animations frame was run (i.e. in `requestAnimationFrame` callback) but should also be done directly after handling events much like we already do on Android [here](https://github.com/software-mansion/react-native-reanimated/blob/99b8b3ed56e36ca615cce7164ccaf04d154571b1/android/src/main/java/com/swmansion/reanimated/NodesManager.java#L283) and on iOS-paper [here](https://github.com/software-mansion/react-native-reanimated/blob/d9a55c556fc32fcb5db59acc92cbedd6452af9dc/ios/REANodesManager.mm#L334). The reason this stopped working on iOS and not on Android was that Android-fabric implementation still uses old architecture flow for handling events while iOS uses new C++ based `react::EventListener` API (that apparently wasn't working on Android last time we checked). This PR adds flushing operations queue in that new C++ based flow. Note that this problem didn't occur before Shareable Rewrite (#3722) because before all mapper updates would result in us scheduling new animation frames. So the updates were happening eventually, were just delayed by one frame. ## Test plan Run Fabric example on iOS, test Article progress example.

eweilow · 2023-03-01T10:27:22Z

Hi! Can create an issue as well, but I think this PR may have broken running react-native-reanimated on the Web without a Babel plugin.

The issue we are getting is that any use of react-native-reanimated throws runOnUI() can only be used on worklets coming from this line
https://github.com/software-mansion/react-native-reanimated/pull/3970/files#diff-e2f6e662599fdf6a81975be4bbd34818651984e9076077405b709d7ae6354680R96

In the PR that adds support for running without a Babel plugin (#3997), the runOnUI function is changed to add a !shouldBeUseWeb() check before testing the __workletHash:

export function runOnUI<A extends any[], R>(
  worklet: ComplexWorkletFunction<A, R>
): (...args: A) => void {
-  if (__DEV__) {
+  if (__DEV__ && !shouldBeUseWeb()) {

From what I can tell, the same needs to be done for runOnUIImmediately:

export function runOnUIImmediately<A extends any[], R>(
  worklet: ComplexWorkletFunction<A, R>
): (...args: A) => void {
-  if (__DEV__) {
+  if (__DEV__ && !shouldBeUseWeb()) {
    if (worklet.__workletHash === undefined) {

Perhaps the error message runOnUI() can only be used on worklets should also be changed to runOnUIImmediately() can only be used on worklets?

nandorojo · 2023-03-01T17:17:43Z

This PR also added a regression on Next.js, which utilizes requestAnimationFrame rather than setImmediate, which isn't available on the server. I opened an issue here: #4140

Fixes software-mansion#3970 (comment) This logic matches that of `runOnUI`.

nandorojo · 2023-03-01T17:20:56Z

@eweilow Yeah you're right, I opened a PR for that fix here: #4141

Fixes #3970 (comment) This logic matches that of `runOnUI`.  ## Summary  ## Test plan  --------- Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>

) ## Summary This PR adds an implementation of web's setImmediate API to the reanimated's UI runtime. We utilize this implementation in mappers and event handlers for more accurate animation/gesture tracking as well as unblock further optimizations. There are several things that this PR changes: 1) We add event timestamp metadata to touch/scroll/sensor/etc events. This change is in preparation of providing a true system timestamps for such events (we don't do that yet but instead call system timer in order to pass down the current time) 2) We update performance.now API to use the same reference point for the provided time as requestAnimationFrame API. Before performance.now would use a different reference point which made it impossible to use it as a starting point for animations. 3) We update event handler callbacks such that they perform a flush for rAF and immediates queues 4) We populate global.__frameTimestamp for all the code that executes within requestAnimationFrame and event callbacks. We then use it in useAnimatedStyle as a reference point for starting animations and in case it wasn't set, we use performance.now 5) We add some code in initializers.ts to define setImmediate method and to register method for flushing immediates queue 6) We introduce set-immediate-based batching for runOnUI calls on the main RN runtime. We then flush all "immediates" after the whole batch is processed on the UI runtime. ## Test plan Check a bunch of examples from the example app: "Use animated style", "Bokeh", "Drag and snap". For touch based interaction verify that movement doesn't lag behind the event. The way this can be done is by screen recording drag and snap example with touch indicators on, then checking on the recorded clip in between frames whether there is a correct movement aligned with the updates of the touch indicator. The above needs to be done for all configurations: Android/iOS/Fabric/Paper

## Summary Before this change we'd only call `performOperations` for a subset of filtered events (initially introduced in software-mansion#312). The event's that weren't considered "direct" would require additional animation frame for having their updates flushed. This change makes it so that we call `performOperations` for all types of events therefore matching the behavior [on Android](https://github.com/software-mansion/react-native-reanimated/blob/99b8b3ed56e36ca615cce7164ccaf04d154571b1/android/src/main/java/com/swmansion/reanimated/NodesManager.java#L283) A consequence of this change was that for some event types, the updates reanimated was performing weren't getting flushed onto screen. Namely, we noticed this problem in Pager example where a view pager component with some custom set of events is used and event handlers update shared values. Even though such event would trigger shared value update, and these updates would trigger the style to recalculate and we'd even call updateProps method to apply the updated props, we'd still see no result as in that example the changes require layout run. Without `performOperation` call the layout would not be executed unless react would rerender or other time-based animation would run. The problem became apparent after software-mansion#3970 where we changed the place where updates are performed from requestAnimationFrame to setImmediate. Before this change, since we were running the updates in "animation frame" the `performOperation` method was being run by the frame scheduler. We, however were getting these updates delayed by one frame because of that. This issue also wasn't noticed prior to shareable rewrite from software-mansion#3722 because before, we were always starting frame updater for every single update happening to shared value even if it was due to an event. As a result, we were getting the stuff updated on screen but again, with a delay of one frame. ## Test plan Run pager example on iOS.

… flow as other types of events (software-mansion#4098) ## Summary This diff fixes an issue with animated views not updating when animating along keyboard. This problem regressed after software-mansion#3970 where we changed the logic of when enqueued callbacks for animation frame and immediates are flushed. The logic has been moved from c++ to js and we added some code to execute animation frame and immediates as a part of event handling process. However, keyboard events have a separate flow of providing event data (we should unify this at some point), and we missed updating it in that place. This PR adds similar logic to [what we use for regular events](https://github.com/software-mansion/react-native-reanimated/blob/535b5d64c0720da3a9a39d99a67e878fe09b5154/src/reanimated2/core.ts#L117) into the code responsible for handling keyboard events. ## Test plan Run animated keyboard example on iOS and Android.

## Summary This PR fixes issue with UI not updating when style changes are connected to events on iOS Fabric. This problem has been introduced in software-mansion#3970 where we migrated from using `rAF` and started using `setImmediate` instead. As a result we had to manually flush set-immediates queue in certain conditions. Specifically this was already happening after animations frame was run (i.e. in `requestAnimationFrame` callback) but should also be done directly after handling events much like we already do on Android [here](https://github.com/software-mansion/react-native-reanimated/blob/99b8b3ed56e36ca615cce7164ccaf04d154571b1/android/src/main/java/com/swmansion/reanimated/NodesManager.java#L283) and on iOS-paper [here](https://github.com/software-mansion/react-native-reanimated/blob/d9a55c556fc32fcb5db59acc92cbedd6452af9dc/ios/REANodesManager.mm#L334). The reason this stopped working on iOS and not on Android was that Android-fabric implementation still uses old architecture flow for handling events while iOS uses new C++ based `react::EventListener` API (that apparently wasn't working on Android last time we checked). This PR adds flushing operations queue in that new C++ based flow. Note that this problem didn't occur before Shareable Rewrite (software-mansion#3722) because before all mapper updates would result in us scheduling new animation frames. So the updates were happening eventually, were just delayed by one frame. ## Test plan Run Fabric example on iOS, test Article progress example.

Fixes software-mansion#3970 (comment) This logic matches that of `runOnUI`.  ## Summary  ## Test plan  --------- Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>

kmagiera added 14 commits January 11, 2023 23:51

Stashing work

229262b

Reset unrelated changes

651538c

Breaking things, please don't use this commit, just trying differenty…

37fdaf7

… things and need to pull in changes from main

Moar updates

6a1dba4

Even moar updates

2e41569

A bit of cleanup

4208b6d

Fix Android builds

924a1b1

Revert "Fix Android builds"

f1a7860

This reverts commit 924a1b1.

Fix android builds for realz

35fe5dc

Merge branch 'main' into set-immediate

cf7e2b3

Handle events with timestamp

1cadbbe

Minor fixes

e024b5f

Fix cpplint

81a7ef1

Trying to fix tests (not yet passing)

096c34a

tomekzaw mentioned this pull request Feb 21, 2023

Scrolling animation got delayed after upgrading from version 3.0.0-rc.7 to version 3.0.0-rc.8 #4069

Closed

mstach60161 mentioned this pull request Feb 21, 2023

Call scrollTo function when scrollViewOffset changed #4048

Closed

kmagiera added 2 commits February 21, 2023 13:34

Merge remote-tracking branch 'origin/main' into set-immediate

0428f0c

Fix some files after merging main

ba92fbb

jwajgelt reviewed Feb 21, 2023

View reviewed changes

kmagiera added 9 commits February 21, 2023 14:00

Lint/type fixes

e4db047

''Fix'' tests

6f43b36

fix typoz

b445d51

Moar test fixes

616db16

Update comments

77b0151

Moar cleanup

eea48bf

Further cleanups + update to node 16 because performance.now cannot b…

e3dc780

…e mocked when running tests on node 14

Merge remote-tracking branch 'origin/main' into set-immediate

2c29b10

Merge remote-tracking branch 'origin/main' into set-immediate

914fa25

piaskowyk approved these changes Feb 22, 2023

View reviewed changes

Remove timestamp from mapper worklet agrument as it is not being used

bfb4a93

kmagiera added this pull request to the merge queue Feb 22, 2023

Merged via the queue into main with commit 535b5d6 Feb 22, 2023

kmagiera deleted the set-immediate branch February 22, 2023 10:21

kmagiera added a commit that referenced this pull request Feb 23, 2023

Fix flushing issues caused by set-immediate change from #3970

e922b26

This was referenced Feb 23, 2023

Make all events on iOS flush updates immediately #4097

Merged

Flush animation-frame and immediates on keyboard event to follow same flow as other types of events #4098

Merged

kmagiera mentioned this pull request Feb 26, 2023

Flush operations for events on iOS Fabric #4110

Merged

tjzel added a commit that referenced this pull request Feb 27, 2023

included #3970 plugin changes

225c956

nandorojo mentioned this pull request Mar 1, 2023

[v3][Web] setImmediate is not defined #4140

Closed

nandorojo added a commit to nandorojo/react-native-reanimated that referenced this pull request Mar 1, 2023

Fix web on v3 when not using a Babel plugin

3ee06d2

Fixes software-mansion#3970 (comment) This logic matches that of `runOnUI`.

nandorojo mentioned this pull request Mar 1, 2023

Fix web on v3 when not using a Babel plugin #4141

Merged

bglgwyng mentioned this pull request Apr 20, 2023

Animated styles are not properly reactive in V3, but an extra touch triggers rerendering #4382

Closed

bglgwyng mentioned this pull request May 12, 2023

Delay in applying animated styles during decreased app performance #4446

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add setImmediate implementation to the UI runtime #3970

Add setImmediate implementation to the UI runtime #3970

kmagiera commented Jan 18, 2023

jwajgelt Feb 21, 2023

jwajgelt Feb 21, 2023

kmagiera Feb 21, 2023

jwajgelt Feb 22, 2023

jwajgelt Feb 22, 2023

piaskowyk Feb 22, 2023

kmagiera Feb 22, 2023

piaskowyk Feb 22, 2023

kmagiera Feb 22, 2023

piaskowyk Feb 22, 2023

kmagiera Feb 22, 2023

nandorojo Mar 1, 2023 •

edited

Loading

bglgwyng commented Feb 23, 2023 •

edited

Loading

kmagiera commented Feb 23, 2023

eweilow commented Mar 1, 2023

nandorojo commented Mar 1, 2023

nandorojo commented Mar 1, 2023

	module->setNewestShadowNodesRegistry(newezstShadowNodesRegistry_);
	module->setNewestShadowNodesRegistry(newestShadowNodesRegistry_);

		std::vector<FrameCallback> callbacks = frameCallbacks;
		frameCallbacks.clear();

	std::vector<FrameCallback> callbacks = frameCallbacks;
	frameCallbacks.clear();
	std::vector<FrameCallback> callbacks = std::move(frameCallbacks);

Add setImmediate implementation to the UI runtime #3970

Add setImmediate implementation to the UI runtime #3970

Conversation

kmagiera commented Jan 18, 2023

Summary

Test plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nandorojo Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

bglgwyng commented Feb 23, 2023 • edited Loading

kmagiera commented Feb 23, 2023

eweilow commented Mar 1, 2023

nandorojo commented Mar 1, 2023

nandorojo commented Mar 1, 2023

nandorojo Mar 1, 2023 •

edited

Loading

bglgwyng commented Feb 23, 2023 •

edited

Loading