Repository navigation
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAndroid CSS transitions now use numeric property and easing IDs across C++, JNI, and Kotlin. The manager replaces ChangesAndroid CSS transitions
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CSSPlatformTransitions
participant NativeProxy
participant CSSPlatformTransitionsManager
participant Choreographer
participant CSSPropertyWriter
CSSPlatformTransitions->>NativeProxy: send property ID and easing ID
NativeProxy->>CSSPlatformTransitionsManager: enqueue transition command
CSSPlatformTransitionsManager->>Choreographer: request animation frame
Choreographer->>CSSPlatformTransitionsManager: deliver frame timestamp
CSSPlatformTransitionsManager->>CSSPropertyWriter: write interpolated value
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
947a1ce to
3d64faf
Compare
971a02c to
2f1fd9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.cpp`:
- Around line 141-156: Update CSSPlatformTransitions::applyTransition to call
removeTransition(viewTag, propertyName) before returning false when easingId is
unavailable and when animate_ refuses the transition. Preserve the existing
success path so active_ is written only for accepted transitions, preventing
stale entries during platform-to-loop migration.
In
`@packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt`:
- Around line 306-325: Bound the retry loop in
CSSPlatformTransitionsManager.beginWhenMounted for persistent transitions as
well as non-persistent ones: track a fixed maximum number of frame retries, stop
reposting callbacks once that limit is reached, and remove the corresponding
startTokens entry. Preserve token validation and pending-command handling while
ensuring an unresolved view cannot keep Choreographer active indefinitely.
- Around line 166-178: Synchronize the entire read-copy-write-publish sequence
in CSSPlatformTransitionsManager.defineEasing using the existing lock referenced
by the proposed fix. Keep interpolator creation and array growth behavior
unchanged, but ensure concurrent calls cannot publish stale easings arrays that
discard another easing entry.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e0ffb193-9197-4c5a-bd3a-0ef257b385c8
📒 Files selected for processing (9)
packages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.cpppackages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.hpackages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpppackages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.hpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.ktpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionReconciler.ktpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.ktpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPropertyWriter.ktpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/DurationScale.kt
| const auto easingId = easingIdFor(toPlatformEasing(resolvedSettings.easingConfig)); | ||
| if (!easingId) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!animate_( | ||
| static_cast<int>(viewTag), | ||
| propertyName, | ||
| *propertyId, | ||
| *from, | ||
| *to, | ||
| reversing.duration, | ||
| reversing.startTimestamp, | ||
| toPlatformEasing(resolvedSettings.easingConfig), | ||
| *easingId, | ||
| settings == nullptr)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚖️ Poor tradeoff
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect every applyTransition/removeTransition caller and the routing fallback contract.
set -euo pipefail
fd -t f 'CSSPlatformTransitions\.(cpp|h)$' --exec cat -n {}
# Callers of the routed-transition API on the shared C++ side.
rg -nP -C6 '\b(applyTransition|removeTransition|cssApplyTransition|cssRemoveTransition|CSSApplyTransitionFunction|CSSRemoveTransitionFunction)\b' --glob '*.{cpp,h}'Repository: software-mansion/react-native-reanimated
Length of output: 12965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate CSS platform transition files =="
git ls-files | rg 'CSSPlatformTransitions|CSS|Transition' | sed -n '1,200p'
echo
echo "== call sites for CSSPlatformTransitions APIs =="
rg -n -C4 '\b(active_\.|CSSPlatformTransitions|applyTransitionForTag|applyTransition\(|removeTransitionForTag|removeTransition\(|animate_\(|remove_\(`|\)`|defineEasing_\()' --glob '*.{cpp,h,m,mm,kt,kts}' .
echo
echo "== Android CSS PlatformTransition implementation references =="
for f in $(git ls-files | rg 'CSSPlatformTransitions|PlatformTransition|CSSPlatform.*Transition|Transition.*Platform|cssPlatformTransitions|PlatformTransitions'); do
echo "--- $f"
rg -n -C4 '\b(applyTransitionForTag|applyTransition\(|removeTransitionForTag|removeTransition\(|animate_\(|remove_\(|defineEasing_\(|active_)\b' "$f"
doneRepository: software-mansion/react-native-reanimated
Length of output: 50396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== NativeProxy CSS transition functions =="
rg -n -C10 'cssAnimateTransition|cssRemoveTransition|cssDefineEasing|cssPlatformTransitions' \
packages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp
echo
echo "== CSSPlatformTransitionsManager Java/Kotlin implementation around animate/remove =="
fd -t f 'CSSPlatformTransitionsManager\.java$|CSSPlatformTransitionsManager\.kt$'
fd -t f 'CSSPlatformTransitionsManager\.java$|CSSPlatformTransitionsManager\.kt$' --exec sh -c '
echo "--- $1"
wc -l "$1"
rg -n -C16 '\b(animate|remove|transition|RunningTransition|setOpacity|intendedOpacity|active|fromValue|toValue)\b' "$1"
' sh {}Repository: software-mansion/react-native-reanimated
Length of output: 4957
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CSSPlatformTransitionsManager implementation =="
FILE="$(fd -t f 'CSSPlatformTransitionsManager\.kt$' | head -n 1)"
echo "$FILE"
wc -l "$FILE"
sed -n '1,260p' "$FILE" | cat -n
echo
echo "== Android call sites for cssApplyTransition/cssRemoveTransition =="
rg -n -C8 '\b(cssApplyTransition|cssRemoveTransition|removeTransitionForTag|applyTransitionForTag|cssApplyTransitionFunction|cssRemoveTransitionFunction)\b' packages/react-native-reanimated/Common packages/react-native-reanimated/android packages/react-native-reanimated/apple --glob '*.{cpp,h,m,mm,kt,kts,ts,tsx,js}'
echo
echo "== CSS platform transition fallback routing API definition =="
rg -n -C8 'CSSPlatformTransitionProxy|PlatformTransition|cssApplyTransitionFunction|cssRemoveTransitionFunction|canRouteCSSProperty' packages/react-native-reanimated/Common packages/react-native-reanimated/android/cmake packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css --glob '*.{cpp,h,m,mm,kt,kts,ts,tsx}'Repository: software-mansion/react-native-reanimated
Length of output: 50396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CSSPlatformTransitionsManager remaining implementation =="
sed -n '260,410p' packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt | cat -n
echo
echo "== CSSPlatformTransitionProxy implementation and contract =="
sed -n '1,280p' packages/react-native-reanimated/Common/cpp/reanimated/CSS/core/transition/CSSPlatformTransitionProxy.cpp | cat -n
sed -n '1,180p' packages/react-native-reanimated/Common/cpp/reanimated/CSS/core/transition/CSSPlatformTransitionProxy.h | cat -n
echo
echo "== Reconciler implementation =="
cat -n packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionReconciler.ktRepository: software-mansion/react-native-reanimated
Length of output: 19075
Release the platform transition when applyTransition falls back to the loop.
CSSPlatformTransitionProxy treats a false applyTransition result as a platform->loop migration and calls removeTransition for properties already routed on the platform. CSSPlatformTransitions::applyTransition returns false before writing active_ for an exhausted easing table or a refused animate_ call, but the previous active entry remains in active_. A later platform-routed apply can reuse those stale reversed values while the property now goes to the loop. Call removeTransition(viewTag, propertyName) before returning false on both of these paths so only a successfully accepted active transition is used.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.cpp`
around lines 141 - 156, Update CSSPlatformTransitions::applyTransition to call
removeTransition(viewTag, propertyName) before returning false when easingId is
unavailable and when animate_ refuses the transition. Preserve the existing
success path so active_ is written only for accepted transitions, preventing
stale entries during platform-to-loop migration.
| /** PathInterpolator flattens its curve natively on construction, so build once per id. */ | ||
| fun defineEasing( | ||
| easingId: Int, | ||
| easingType: Int, | ||
| easingPointsX: FloatArray, | ||
| easingPointsY: FloatArray, | ||
| ) { | ||
| val interpolator = CSSEasing.interpolator(easingType, easingPointsX, easingPointsY) | ||
| val current = easings | ||
| val grown = if (easingId < current.size) current.copyOf() else current.copyOf(easingId + 1) | ||
| grown[easingId] = interpolator | ||
| easings = grown | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace the call sites and threading of the easing definition path.
set -euo pipefail
rg -nP -C6 '\b(cssDefineEasing|defineEasing|easingIdFor|defineEasing_)\b' --glob '*.{kt,cpp,h}'
# The native caller decides the thread; show the transition apply path.
fd -t f 'CSSPlatformTransitions.cpp' --exec rg -n -C8 'applyTransition|easingIdFor' {}Repository: software-mansion/react-native-reanimated
Length of output: 344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg 'CSSPlatformTransitions|ReactCSSPlatformTransitions|cssDefineEasing|easingIdFor|defineEasing|CSSPlatformTransitionsManager|CSSPlatformTransitions' || true
echo
echo "== search keyword variations =="
rg -n -C4 'defineEasing|cssDefineEasing|easing_id|easingId|easings|`@JniModule`|`@JsModule`|REACT_METHOD|REACT_CLASS|class CSSPlatformTransitionsManager|object CSSPlatformTransitionsManager' --glob '*.{kt,cpp,h,ts,js,tsx,java}' . || trueRepository: software-mansion/react-native-reanimated
Length of output: 45047
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== NativeProxy Android JNI setup =="
rg -n -C6 'NativeProxy|nativeConstructor|NativeProxy::cssPlatformTransitionsManager|ReactBaseModule|ReanimatedModule|ModuleProvider|reactProxy|javaPart_|JNativeProxy|CSSPlatformTransitionsManager' --glob '*.{kt,cpp,h}' .
echo
echo "== CSSPlatformTransitionsManager outline =="
ast-grep outline packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt --view expanded || true
echo
echo "== native proxy construction / init threading =="
rg -n -C8 'shared_from_this|uiScheduler|dispatchOnUI|onUiThread|jsQueue|reactQueue|jsTaskQueue|NativeProxyFactory|NativeProxy\\(' --glob '*.{kt,cpp,h}' packages/react-native-reanimated/android packages/react-native-reanimated/Common packages/react-native-reanimated/android/src/main/cpp/reanimated/android | head -n 240Repository: software-mansion/react-native-reanimated
Length of output: 50397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== focused NativeProxy.kt =="
cat -n packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.kt | sed -n '1,360p'
echo
echo "== focused NativeProxy.cpp NativeProxy constructor =="
cat -n packages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp | sed -n '110,420p'
echo
echo "== JSI/native function wrapper definitions around css methods =="
rg -n -C5 'jsi_utils::installJSIFunction|JValueToJSI|react_callInvoker|NativeProxy::cssDefineEasing|cssDefineEasing|scheduleOnUI|onUiThread|DispatchQueue::async|react::CallInvoker|CallInvoker' packages react-native-reanimated/android/src/main/cpp/reanimated/android packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.kt | head -n 240
echo
echo "== relevant CSSPlatformTransitions and CSSPlatformTransitionsManager snippets =="
cat -n packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt | sed -n '50,220p'
cat -n packages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.cpp | sed -n '1,180p'
echo
echo "== deterministic Java copy/write interleaving probe =="
python3 - <<'PY'
class State:
def __init__(self):
self.easings = [None, None, None]
def define_copy_append(self, easing_id, value):
current = self.easings
grown = list(current)
growth_expected = False
if easing_id >= len(grown):
grown += [None] * (easing_id + 1 - len(grown))
growth_expected = True
grown[easing_id] = value
self.easings = grown
return "grow" if growth_expected else "overwrite"
cases = []
def trace():
s = State()
# Thread A reads current; Thread B reads current; both copy/write on same visible state; B publishes; A publishes.
current_a = s.easings
current_b = s.easings
grown_a = list(current_a)
grown_b = list(current_b)
result_b = "grow" if 1 >= len(grown_b) else "overwrite"
grown_b[1] = "B"
s.easings = grown_b
result_a = "grow" if 2 >= len(grown_a) else "overwrite"
grown_a[2] = "A"
s.easings = grown_a
return s.easings, (result_a, result_b)
print(trace()[0])
PYRepository: software-mansion/react-native-reanimated
Length of output: 50396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== PlatformDepMethodsHolder and scheduler call sites =="
rg -n -C5 'typedef .*PlatformDepMethodsHolder|struct .*PlatformDepMethodsHolder|class .*PlatformDepMethodsHolder|method\s*\(' --glob '*.{h,cpp,kt,java}' packages/react-native-reanimated/Common/cpp packages/react-native-reanimated/Common packages/react-native-reanimated/src packages/react-native-reanimated/android/src/main/java packages/react-native-worklets/src | head -n 320
echo
echo "== ReanimatedModuleProxy CSS transition apply path =="
rg -n -C6 'applyCSSAnimations|runCSSTransition|easingIdFor\(|cssPlatformTransitions\(|cssApplyTransition|synchronousPropsIntBuffer_|synchronousPropsDoubleBuffer_|synchronous' --glob '*.{h,cpp,kt,java,ts}' packages/react-native-reanimated packages/react-native-worklets/src | head -n 360
echo
echo "== JSI bridge install for PlatformDepMethodsHolder methods =="
rg -n 'jsi_utils::installJSIFunction|jsi::valueToNativeFunction|PlatformDepMethodsHolder|jsni\\[.*=.*lambda|installMethods|method\\(' packages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp packages/react-native-reanimated/Common/cpp/reanimated/NativeModules packages/react-native-reanimated/apple/reanimated/apple/native packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated packages/react-native-reanimated/src -g '*.{h,cpp,kt,ts}' | head -n 360
echo
echo "== NativeProxy.kt css methods continuation around initialization =="
cat -n packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.kt | sed -n '96,112p'
echo
echo "== Read-only Java copy/append interleaving scenario =="
python3 - <<'PY'
class State:
def __init__(self):
self.easings = [None, None, None] if False else []
def define(self, easing_id, value):
current = self.easings
grown = list(current)
if easing_id >= len(grown):
grown += [None] * (easing_id + 1 - len(grown))
grown[easing_id] = value
self.easings = grown
s = State()
# Model interleaving where two calls read the same empty array.
# Thread A reads [].
current_a = s.easings
# Thread B reads [].
current_b = s.easings
# Thread B appends at id 0.
grown_b = list(current_b)
grown_b += [None] * (0 + 1 - len(grown_b))
grown_b[0] = "B"
s.easings = grown_b
# Thread A appends at id 1 and publishes, preserving thread B's value but leaving id 0 null.
grown_a = list(current_a)
grown_a += [None] * (1 + 1 - len(grown_a))
grown_a[1] = "A"
s.easings = grown_a
print("final", s.easings)
print("id 0 null:", s.easings[0] is None)
PYRepository: software-mansion/react-native-reanimated
Length of output: 40304
🌐 Web query:
ReactNative Worklets scheduleOnUI calls execute on JS thread PlatformDepMethodsHolder
💡 Result:
The scheduleOnUI function in React Native Worklets does not execute on the JavaScript (JS) thread; its primary purpose is to schedule worklet execution asynchronously on the UI Runtime (the UI thread) [1][2][3]. When you call scheduleOnUI from the JS thread (or any other thread), it queues the provided worklet to be executed on the UI Runtime [1][4][5]. In native environments, this involves the Worklets module processing the queue and eventually triggering the worklet execution on the UI thread [5]. Regarding PlatformDepMethodsHolder: PlatformDepMethodsHolder is a C++ struct used internally by React Native Reanimated to bridge the platform-agnostic core logic with platform-specific native implementations [6]. It defines an interface for operations that require direct access to native capabilities, such as: - requestRender: Scheduling callbacks for the next animation frame [6]. - synchronouslyUpdateUIPropsFunction: Updating view properties directly on the UI thread [6]. - getAnimationTimestamp: Providing high-resolution timing for animations [6]. - Handling sensor registration, keyboard events, and UI update queues [7][6]. PlatformDepMethodsHolder is not where scheduleOnUI executes; rather, it is a set of utilities that enables the Reanimated core (which operates across different runtimes) to perform low-level tasks on the host platform [6]. The scheduling mechanism for worklets is managed by the Worklets module, which orchestrates the movement of tasks between the JS runtime, the UI runtime, and the native thread [5][8].
Citations:
- 1: https://docs.swmansion.com/react-native-worklets/docs/threading/scheduleOnUI/
- 2: https://docs.swmansion.com/react-native-reanimated/docs/guides/worklets/
- 3: https://docs.swmansion.com/react-native-worklets/docs/fundamentals/glossary/
- 4: https://github.com/software-mansion/react-native-reanimated/blob/8e56ae6a/packages/react-native-worklets/src/threads.ts
- 5: https://github.com/software-mansion/react-native-reanimated/blob/8e56ae6a/packages/react-native-worklets/src/threads.native.ts
- 6: https://deepwiki.com/software-mansion/react-native-reanimated/3-native-implementation
- 7: https://github.com/software-mansion/react-native-reanimated/blob/8fd9302d/packages/react-native-reanimated/Common/cpp/reanimated/Tools/PlatformDepMethodsHolder.h
- 8: https://github.com/software-mansion/react-native-reanimated/blob/8fd9302d/packages/react-native-reanimated/apple/reanimated/apple/native/NativeProxy.mm
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== android native proxy css apply implementation =="
rg -n -C12 'getPlatformDependentMethods|PlatformDepMethodsHolder|cssApplyTransition|applyTransition\(' packages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp packages/react-native-reanimated/android/src/main/cpp/reanimated -g '*.{h,cpp}' || rg -n -C12 'getPlatformDependentMethods|PlatformDepMethodsHolder|cssApplyTransition|applyTransition\(' packages/react-native-reanimated/apple packages/react-native-reanimated/Common/cpp/reanimated -g '*.{h,mm,cpp}' | head -n 240
echo
echo "== cssApply call-chain in cpp apply transition =="
rg -n -C12 'cssApplyTransition|CSSPlatformTransitions::applyTransition|CSSPlatformTransitions::easingIdFor|defineEasing_\(' packages/react-native-reanimated/Common/cpp/reanimated/CSS packages/react-native-reanimated/android/src/main/cpp/reanimated/packages/react-native-reanimated/android/src/main/cpp/reanimated -g '*.{h,cpp}' || true
echo
echo "== scheduleOnUI execution path comments =="
cat -n packages/react-native-worklets/src/threads.native.ts | sed -n '1,180p'Repository: software-mansion/react-native-reanimated
Length of output: 50396
Guard defineEasing against stale copy-on-append updates.
defineEasing reads easings, copies the array, writes one easing id, and then publishes the grown array. Overlapping calls can publish a copy taken before another call appended its defining entry, leaving that id null. animateTransition then rejects null entries and transitions fall back to the loop for that id. Synchronize the read-copy-write-publish path.
Use the lock shown in the proposed fix around the existing copy-on-append body.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt`
around lines 166 - 178, Synchronize the entire read-copy-write-publish sequence
in CSSPlatformTransitionsManager.defineEasing using the existing lock referenced
by the proposed fix. Keep interpolator creation and array growth behavior
unchanged, but ensure concurrent calls cannot publish stale easings arrays that
discard another easing entry.
| private fun beginWhenMounted( | ||
| key: Key, | ||
| token: Long, | ||
| endTimestampMs: Double, | ||
| persistent: Boolean, | ||
| begin: () -> Boolean, | ||
| ) { | ||
| if (startTokens[key] != token) return | ||
| if (begin() || animationTimestamp() >= endTimestampMs) { | ||
| if (hasPendingCommand(key)) { | ||
| // A queued command may invalidate this token; let the flush run first. | ||
| Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, persistent, begin) } | ||
| return | ||
| } | ||
| // A persistent value outlives its timeline, so its start never expires. | ||
| if (begin() || (!persistent && animationTimestamp() >= endTimestampMs)) { | ||
| startTokens.remove(key) | ||
| return | ||
| } | ||
| Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, begin) } | ||
| Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, persistent, begin) } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A persistent transition whose view never mounts retries every frame forever.
Line 320 expires the retry only when !persistent. For persistent = true the retry has no end condition. If the view tag never resolves, beginWhenMounted reposts a frame callback on every frame for the life of the process. The Choreographer then never idles, which drains battery and blocks the main thread from going idle.
A view tag can fail to resolve permanently. React can unmount the view before it ever mounts. The only escape today is dropTransition, which needs a Remove command from the native side for that exact key.
Bound the retry. Stop when the tag no longer resolves after a fixed number of frames, and drop the token so the entry does not leak.
🔒️ Proposed fix to bound the mount retry
+ /** A tag that never resolves must not keep the Choreographer awake forever. */
+ private val maxMountRetryFrames = 300
+
private fun beginWhenMounted(
key: Key,
token: Long,
endTimestampMs: Double,
persistent: Boolean,
+ framesLeft: Int = maxMountRetryFrames,
begin: () -> Boolean,
) {
if (startTokens[key] != token) return
if (hasPendingCommand(key)) {
// A queued command may invalidate this token; let the flush run first.
- Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, persistent, begin) }
+ Choreographer.getInstance().postFrameCallback {
+ beginWhenMounted(key, token, endTimestampMs, persistent, framesLeft, begin)
+ }
return
}
- // A persistent value outlives its timeline, so its start never expires.
- if (begin() || (!persistent && animationTimestamp() >= endTimestampMs)) {
+ // A persistent value outlives its timeline, so only the frame budget expires it.
+ if (begin() || framesLeft <= 0 || (!persistent && animationTimestamp() >= endTimestampMs)) {
startTokens.remove(key)
return
}
- Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, persistent, begin) }
+ Choreographer.getInstance().postFrameCallback {
+ beginWhenMounted(key, token, endTimestampMs, persistent, framesLeft - 1, begin)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun beginWhenMounted( | |
| key: Key, | |
| token: Long, | |
| endTimestampMs: Double, | |
| persistent: Boolean, | |
| begin: () -> Boolean, | |
| ) { | |
| if (startTokens[key] != token) return | |
| if (begin() || animationTimestamp() >= endTimestampMs) { | |
| if (hasPendingCommand(key)) { | |
| // A queued command may invalidate this token; let the flush run first. | |
| Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, persistent, begin) } | |
| return | |
| } | |
| // A persistent value outlives its timeline, so its start never expires. | |
| if (begin() || (!persistent && animationTimestamp() >= endTimestampMs)) { | |
| startTokens.remove(key) | |
| return | |
| } | |
| Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, begin) } | |
| Choreographer.getInstance().postFrameCallback { beginWhenMounted(key, token, endTimestampMs, persistent, begin) } | |
| } | |
| /** A tag that never resolves must not keep the Choreographer awake forever. */ | |
| private val maxMountRetryFrames = 300 | |
| private fun beginWhenMounted( | |
| key: Key, | |
| token: Long, | |
| endTimestampMs: Double, | |
| persistent: Boolean, | |
| framesLeft: Int = maxMountRetryFrames, | |
| begin: () -> Boolean, | |
| ) { | |
| if (startTokens[key] != token) return | |
| if (hasPendingCommand(key)) { | |
| // A queued command may invalidate this token; let the flush run first. | |
| Choreographer.getInstance().postFrameCallback { | |
| beginWhenMounted(key, token, endTimestampMs, persistent, framesLeft, begin) | |
| } | |
| return | |
| } | |
| // A persistent value outlives its timeline, so only the frame budget expires it. | |
| if (begin() || framesLeft <= 0 || (!persistent && animationTimestamp() >= endTimestampMs)) { | |
| startTokens.remove(key) | |
| return | |
| } | |
| Choreographer.getInstance().postFrameCallback { | |
| beginWhenMounted(key, token, endTimestampMs, persistent, framesLeft - 1, begin) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt`
around lines 306 - 325, Bound the retry loop in
CSSPlatformTransitionsManager.beginWhenMounted for persistent transitions as
well as non-persistent ones: track a fixed maximum number of frame retries, stop
reposting callbacks once that limit is reached, and remove the corresponding
startTokens entry. Preserve token validation and pending-command handling while
ensuring an unresolved view cannot keep Choreographer active indefinitely.
Replace the per-property ObjectAnimator player with one Choreographer frame callback that walks the active transitions and writes values directly, so cost scales with the number of running animations rather than the number of animator objects.
2f1fd9b to
e2555e6
Compare
3d64faf to
03f6a82
Compare
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one per view. Commands are now queued and drained by a single asynchronous message per burst. The queue and its double buffer are taken from the frame pump prototype in the closed #10170, where they were introduced alongside the player change; they apply to the ObjectAnimator player unchanged.
Starting a transition posted its own message to the UI thread, so a commit touching every view on screen posted one message per view. Commands are now queued and drained by a single asynchronous message per burst. The queue itself comes from the frame pump prototype in the closed #10170, where it was introduced alongside the player change. It is independent of the player, so it applies to the ObjectAnimator engine unchanged.
Replaces the per-property ObjectAnimator player with one Choreographer frame callback that walks the active transitions and writes values directly. Cost scales with the number of running animations rather than the number of animator objects, and delays and late starts resolve from the absolute start timestamp so they need no extra state.
Supersedes #10126 in the restacked series.