Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] worklet-thread enabled scene graph #2511

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

[WIP] worklet-thread enabled scene graph #2511

wants to merge 12 commits into from

Conversation

wcandillon
Copy link
Contributor

@wcandillon wcandillon commented Jun 28, 2024

Currently, our C scene graph (SkiaDOM) is mutable and thread-safe. Benchmarks show that we pay the locking price twice:

  • With Reanimated (via reading values on the JS thread when binding properties)
  • With SkiaDOM (via its thread-safety mechanism).

The current C SkiaDOM assumes that it will receive many concurrent updates at the same time. However, the number of updates made on the JS thread (often a few times over a component's lifetime) is much lower than updates from the worklet thread (often every single frame). We think it is more sensible to make the scene graph immutable (a JS thread update clones/edits the tree like in Fabric) and have animation values supported on the dedicated thread. Thanks to the persistent mode of the reconciler, this makes handling concurrency much easier.

Additionally, the SkiaDOM is very hard to maintain due to its thread-safety paradigm. The C implementation has a few regressions compared to its JS counterpart, which are hard to fix. We also plan to provide extension points for accessibility, gestures, and layouts, which simply cannot be implemented in the current form of the C SkiaDOM.

The goal of this PR is to use the persistent mode of the reconciler (used in Fabric) and have the scene graph run on a worklet thread. The scene graph is immutable (via persistent mode) and supports animation values directly.

This will bring performance improvements:

  • Right now, each property that uses an animation value creates a new Reanimated mapper. Since we execute the graph on the worklet thread, we can reduce this to a simple mapper.
  • Remove the double locking mechanism (Reanimated lock via bindProps in JS thread, and SkiaDOM locks).

This PR will also fix current SkiaDOM regressions (bugs with displacement maps, shadows).

This will also allow us to build the following features:

  • Extension points for accessibility, gesture, and layouts.
  • Enabling dedicated threads for each Canvas (once we offer canvasRef.transferControlToOffscreen()).
  • In the future, we may look at running the reconciler in a worklet thread too.

fixes #2429

@wcandillon wcandillon changed the title [WIP] Worklet enabled screen graph [WIP] worklet-thread enabled scene graph Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App crashes on RNSkia::JsiDomDeclarationNode::invalidateContext()
1 participant