Repository navigation
Conversation
… host Let the native layout route on iOS take a `withTiming` with a duration of zero. `playbackOf` (`NativeAnimationTrack.cpp`) gives the playback form of a track. A track with no duration and a delay holds its start value through the delay, and then the model value shows. A track with no duration and no delay has no physical playback: the host ends it in the operation that admits it. Remove the Debug trace event `FirstFrameSampled` and the next-frame sampler of the Core Animation platform. That sample read the value before the commit of the frame, so its value was not correct. The Debug target sample now gives the playback keys of the view: the Core Animation animations of the host on its layer. Add systrace sections to the layout animation build, to the track conversion, and to the steps of the Core Animation platform.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Change: Feature Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
74380541-e4a0-475e-805a-e6d3bcaac474
📒 Files selected for processing: 14
apps/common-app/runtime-tests/reanimated/tests/layoutAnimations/nativeLayoutStarts.test.tsxdocs/docs-reanimated/docs/guides/feature-flags.mdpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsManager.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/NativeLayoutTracks.cpppackages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationHost.cpppackages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationHost.hpackages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationPlatform.hpackages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationTrace.cpppackages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationTrace.hpackages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationTrack.cpppackages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationTrack.hpackages/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.cpppackages/react-native-reanimated/apple/reanimated/apple/NativeAnimations/REANativeAnimationPlatform.mmpackages/react-native-reanimated/changelog/ios-layout-animations-native-zero-duration.other.md
💤 Files with no reviewable changes: 1
- packages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationTrace.h
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| animation.timingFunction = std::visit(TimingFunctionVisitor{}, track.segments.back().timingFromPrevious); | ||
| if (holdsStartValue) { | ||
| animation.toValue = fromValue; | ||
| animation.duration = delay; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
printf '%s\n' '--- reviewed block ---'
nl -ba packages/react-native-reanimated/apple/reanimated/apple/NativeAnimations/REANativeAnimationPlatform.mm | sed -n '430,510p'
printf '%s\n' '--- PR diff for platform file ---'
git diff 760402d73d2055ba9be6ed05c3c62ff07823fa42 68ed4f1836bb7559cc431839dc96f08442a8edc2 -- packages/react-native-reanimated/apple/reanimated/apple/NativeAnimations/REANativeAnimationPlatform.mm
printf '%s\n' '--- related delay/playback symbols ---'
rg -n -F -e 'HeldThroughDelay' -e 'layerOrigin' -e 'convertTime:' -e 'withDelay' -- packages/react-native-reanimated/apple packages/react-native-reanimated/src packages/react-native-reanimated/Common/cpp || test "$?" -eq 1Repository: software-mansion/react-native-reanimated
Length of output: 14735
🏁 Script executed:
printf '%s\n' '--- withDelay contract and implementation ---'
nl -ba packages/react-native-reanimated/src/animation/delay.ts | sed -n '1,145p'
printf '%s\n' '--- native track playback and delay definitions ---'
nl -ba packages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationTrack.cpp | sed -n '1,115p'
nl -ba packages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations/NativeAnimationTrack.h | sed -n '1,105p'
printf '%s\n' '--- slow-animation time conversions ---'
rg -n -F -e 'calculateMediaTimeFromSlowAnimationsTimestamp' -e 'calculateMediaDurationFromSlowAnimationsDuration' -- packages/react-native-reanimated/apple/reanimatedRepository: software-mansion/react-native-reanimated
Length of output: 12264
🏁 Script executed:
printf '%s\n' '--- delay assignment and request timestamp bindings ---'
rg -n -F -e 'delayMs =' -e 'delayMs{' -e 'originTimestampMs' -- packages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations packages/react-native-reanimated/apple/reanimated/apple/NativeAnimations
printf '%s\n' '--- slow-animation conversion implementation ---'
nl -ba packages/react-native-reanimated/apple/reanimated/apple/REASlowAnimations.mm | sed -n '1,75p'
nl -ba packages/react-native-reanimated/apple/reanimated/apple/REASlowAnimations.h | sed -n '1,45p'
printf '%s\n' '--- native timing data declarations / conversion context ---'
rg -n -F -e 'delayMs' -e 'delay' -- packages/react-native-reanimated/Common/cpp/reanimated/NativeAnimations | head -100Repository: software-mansion/react-native-reanimated
Length of output: 4925
🌐 Web query:
Apple Core Animation CALayer convertTime fromLayer nil local time speed duration beginTime documentation
💡 Result:
<source_evidence>
<source kind="search">
<title>Advanced Animation Tricks</title>
<location>https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CoreAnimation_guide/AdvancedAnimationTricks/AdvancedAnimationTricks.html</location>
<excerpt>citeturn0search1 [wordlim: 200] Published: 11.6 years ago; Crawled: 8 months ago; To assist you in making sure time values are appropriate for a given layer, the `CALayer` class defines the `convertTime:fromLayer:` and `convertTime:toLayer:` methods. ... CFTimeInterval localLayerTime = [myLayer convertTime:CACurrentMediaTime() fromLayer:nil];
## Customizing the Timing of an Animation
Timing is an important part of animations, and with Core Animation you specify precise timing information for your animations through the methods and properties of the `CAMediaTiming` protocol. Two Core Animation classes adopt this protocol. The `CAAnimation` class adopts it so that you can specify timing information in your animation objects. The `CALayer` also adopts it so that you can configure some timing-related features for your implicit animations, although the implicit transaction object that wraps those animations usually provides default timing information that takes precedence.
When thinking about timing and animations, it is important to understand how layer objects work with time. Each layer has its own local time that it uses to manage animation timing. Normally, the local time of two different layers is close enough that you could specify the same time values for each and the user might not notice anything. However, the local time of a layer can be modified by its parent layers or by its own timing parameters. For example, changing the layer’s `speed` property causes the duration of animations on that layer (and its sublayers) to change proportionally.
To assist you in making sure time values are appropriate for a given layer, the `CALayer` class defines the `convertTime:fromLayer:` and `convertTime:toLayer:` methods. You can use these methods to convert a fixed time value to the local time of a layer or to convert time values from one layer to another. The methods take into account the media timing properties that might affect the local time of the layer and return a value that you can use with the other layer. Listing 5-3 shows an example that you should use regularly to get the current local time for a layer. The `CACurrentMediaTime` function is a convenience function that returns the computer’s current clock time, which the method takes and converts to the layer’s local time.
Listing 5-3 Getting a layer’s current local time
CFTimeInterval localLayerTime = [myLayer convertTime:CACurrentMediaTime() fromLayer:nil];
Once you have a time value in the layer’s local time, you can use that value to update the timing-related properties of an animation object or layer. With these timing properties, you can achieve some interesting animation behaviors, including:
* Use the `beginTime` property to set the start time of an animation. Normally, animations begin during the next update cycle. You can use the `beginTime` parameter to delay the animation start time by several seconds. The way to chain two animations together is to set the begin time of one animation to match the end time of the other animation.
If you delay the start of an animation, you might also want to set the `fillMode` property to `kCAFillModeBackwards`. This fill mode causes the layer to display the animation’s start value, even if the layer object in the layer tree contains a different value. Without this fill mode, you would see a jump to the final value before the animation starts executing. Other fill modes are available too.
* The `autoreverses` property causes an animation to execute for the specified duration and then return to the starting value of the animation. You can combine this property with the `repeatCount` property to animate back and forth between the start and end values. Setting the repeat count to a whole number (such as 1.0) for an autoreversing animation causes the animation to stop on its starting value. Adding an extra half step (such as a repeat count of 1.5) causes the animation to stop on its end value.
…[truncated]</excerpt>
</source>
<source kind="search">
<title>speed | Apple Developer Documentation</title>
<location>https://developer.apple.com/documentation/quartzcore/camediatiming/speed?changes=_7&language=objc</location>
<excerpt>citeturn1search0 [wordlim: 200] Crawled: 3 months ago; * CAMediaTiming ... For example, if `speed` is 2.0 local time progresses twice as fast as parent time. ... ### Duration and Speed
* Core Animation
* CAMediaTiming
* speed
Instance Property
# speed
Specifies how time is mapped to receiver’s time space from the parent time space.
`@property float speed;`
Required
## Discussion
For example, if `speed` is 2.0 local time progresses twice as fast as parent time. Defaults to 1.0.
## See Also
### Duration and Speed
`duration`
Specifies the basic duration of the animation, in seconds.</excerpt>
</source>
<source kind="search">
<title>beginTime | Apple Developer Documentation</title>
<location>https://developer.apple.com/documentation/quartzcore/camediatiming/begintime</location>
<excerpt>citeturn1search2 [wordlim: 200] Crawled: 3 months ago; * Core Animation * CAMediaTiming ... `var beginTime: `CFTimeInterval` { get set }` ... Specifies an additional time offset in active local time.
* Core Animation
* CAMediaTiming
* beginTime
Instance Property
# beginTime
Specifies the begin time of the receiver in relation to its parent object, if applicable.
`var beginTime: `CFTimeInterval` { get set }`
Required
## Discussion
Defaults to 0.
## See Also
### Related Documentation
Core Animation Programming Guide
### Animation Start Time
`var timeOffset: CFTimeInterval`
Specifies an additional time offset in active local time.
Required</excerpt>
</source>
<source kind="search">
<title>duration | Apple Developer Documentation</title>
<location>https://developer.apple.com/documentation/quartzcore/camediatiming/duration</location>
<excerpt>citeturn1search3 [wordlim: 200] Crawled: 2 months ago; * CAMediaTiming ... `var duration: `CFTimeInterval` { get set }` ... ### Duration and Speed
* Core Animation
* CAMediaTiming
* duration
Instance Property
# duration
Specifies the basic duration of the animation, in seconds.
`var duration: `CFTimeInterval` { get set }`
Required
## Discussion
Defaults to 0.
## See Also
### Duration and Speed
`var speed: Float`
Specifies how time is mapped to receiver’s time space from the parent time space.
Required</excerpt>
</source>
<source kind="search">
<title>Advanced Animation Tricks</title>
<location>https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CoreAnimation_guide/AdvancedAnimationTricks/AdvancedAnimationTricks.html</location>
<excerpt>citeturn1search4 [wordlim: 200] Published: 11.6 years ago; Crawled: 8 months ago; Each layer has its own local time that it uses to manage animation timing. ... To pause an animation, you can take advantage of the fact that layers adopt the `CAMediaTiming` protocol and set the speed of the layer’s animations to `0.0`. ... Only after you commit the changes for the outermost transaction does Core Animation begin the associated animations.
## Customizing the Timing of an Animation
Timing is an important part of animations, and with Core Animation you specify precise timing information for your animations through the methods and properties of the `CAMediaTiming` protocol. Two Core Animation classes adopt this protocol. The `CAAnimation` class adopts it so that you can specify timing information in your animation objects. The `CALayer` also adopts it so that you can configure some timing-related features for your implicit animations, although the implicit transaction object that wraps those animations usually provides default timing information that takes precedence.
When thinking about timing and animations, it is important to understand how layer objects work with time. Each layer has its own local time that it uses to manage animation timing. Normally, the local time of two different layers is close enough that you could specify the same time values for each and the user might not notice anything. However, the local time of a layer can be modified by its parent layers or by its own timing parameters. For example, changing the layer’s `speed` property causes the duration of animations on that layer (and its sublayers) to change proportionally.
To assist you in making sure time values are appropriate for a given layer, the `CALayer` class defines the `convertTime:fromLayer:` and `convertTime:toLayer:` methods. You can use these methods to convert a fixed time value to the local time of a layer or to convert time values from one layer to another. The methods take into account the media timing properties that might affect the local time of the layer and return a value that you can use with the other layer. Listing 5-3 shows an example that you should use regularly to get the current local time for a layer. The `CACurrentMediaTime` function is a convenience function that returns the computer’s current clock time, which the method takes and converts to the layer’s local time.
Listing 5-3 Getting a layer’s current local time
CFTimeInterval localLayerTime = [myLayer convertTime:CACurrentMediaTime() fromLayer:nil];
Once you have a time value in the layer’s local time, you can use that value to update the timing-related properties of an animation object or layer. With these timing properties, you can achieve some interesting animation behaviors, including:
* Use the `beginTime` property to set the start time of an animation. Normally, animations begin during the next update cycle. You can use the `beginTime` parameter to delay the animation start time by several seconds. The way to chain two animations together is to set the begin time of one animation to match the end time of the other animation.
If you delay the start of an animation, you might also want to set the `fillMode` property to `kCAFillModeBackwards`. This fill mode causes the layer to display the animation’s start value, even if the layer object in the layer tree contains a different value. Without this fill mode, you would see a jump to the final value before the animation starts executing. Other fill modes are available too.
* The `autoreverses` property causes an animation to execute for the specified duration and then return to the starting value of the animation. You can combine this property with the `repeatCount` property to animate back and forth between the start and end values. Setting the repeat count to a whole number (such as 1.0) for an autoreversing animation causes the animation to stop on its starting value. Adding an extr…[truncated]</excerpt>
</source>
<omitted_sources count="46">Additional source records were omitted from this bounded result.</omitted_sources>
</source_evidence>
Citations:
- 1: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CoreAnimation_guide/AdvancedAnimationTricks/AdvancedAnimationTricks.html?utm_source=openai
- 2: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CoreAnimation_guide/AdvancedAnimationTricks/AdvancedAnimationTricks.html?utm_source=openai
- 3: https://developer.apple.com/documentation/quartzcore/camediatiming/speed?changes=_7&language=objc&utm_source=openai
- 4: https://developer.apple.com/documentation/quartzcore/camediatiming/begintime?utm_source=openai
- 5: https://developer.apple.com/documentation/quartzcore/camediatiming/duration?utm_source=openai
- 6: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CoreAnimation_guide/AdvancedAnimationTricks/AdvancedAnimationTricks.html?utm_source=openai
🏁 Script executed:
printf '%s\n' '--- native track constructors and request creation ---'
rg -n -F -e 'AnimationTrack{' -e 'AnimationTrack track' -e 'delayMs' -e 'AnimationRequest' -- packages/react-native-reanimated/Common/cpp/reanimated packages/react-native-reanimated/apple/reanimatedRepository: software-mansion/react-native-reanimated
Length of output: 5077
🏁 Script executed:
nl -ba packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/NativeLayoutTracks.cpp | sed -n '70,115p'
nl -ba packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy.cpp | sed -n '1130,1155p'Repository: software-mansion/react-native-reanimated
Length of output: 3695
Convert the held delay to layer time.
For a zero-duration native layout track with a positive delay, playbackOf selects HeldThroughDelay. Core Animation then uses the media-clock delay as a layer-local duration. If an ancestor layer has speed = 0.5, the held period can take twice as long in wall-clock time. Convert the end of the delay to layer time and use its difference from layerOrigin.
🐛 Suggested fix
--- "a/packages/react-native-reanimated/apple/reanimated/apple/NativeAnimations/REANativeAnimationPlatform.mm"
+++ "b/packages/react-native-reanimated/apple/reanimated/apple/NativeAnimations/REANativeAnimationPlatform.mm"
@@ -473,10 +473,10 @@
CABasicAnimation *animation = [CABasicAnimation animationWithKeyPath:keyPathForTarget(track.target)];
animation.fromValue = fromValue;
if (holdsStartValue) {
animation.toValue = fromValue;
- animation.duration = delay;
+ animation.duration = [layer_ convertTime:origin + delay fromLayer:nil] - layerOrigin;
animation.beginTime = layerOrigin;
} else {
animation.toValue = toValue;
animation.duration = calculateMediaDurationFromSlowAnimationsDuration(track.durationMs / 1000.0);📝 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.
| animation.duration = delay; | |
| animation.duration = [layer_ convertTime:origin + delay fromLayer:nil] - layerOrigin; |
BruceBotterson
left a comment
There was a problem hiding this comment.
Hi, Bruce here.
I reviewed this PR's diff: playbackOf, the move of the end logic to endTrack, the immediate end in admit, the hold form in makeAnimation, and the sampler changes.
The endTrack refactor keeps the old onTrackEnded behavior. The immediate end runs after commands_.emplace and after the admitted delivery, so the client gets onAnimationAdmitted before the track end and the Finished result. The hold form also handles a late start correctly: a beginTime in the past with duration == delay only ends at once.
One point:
- A zero-duration CSS transition with a positive delay plays for the Core Animation default duration in place of a jump at the end of the delay (bug). The cause is in #10276, but the new
TrackPlayback::Interpolatedcomment now documents it, so I put it here.
| } | ||
|
|
||
| TrackPlayback playbackOf(const AnimationTrack &track) { | ||
| if (track.durationMs > 0 || track.endpointPolicy != EndpointPolicy::MountedModelMustMatchEndpoint) { |
There was a problem hiding this comment.
Every track whose policy is not MountedModelMustMatchEndpoint gets Interpolated, also with durationMs == 0. The CSS route can send such a track:
src/css/native/normalization/transition/config.ts:113drops a property only whenduration + delay <= 0, sotransitionDuration: 0, transitionDelay: 300reaches native.CSSPlatformTransitionProxy::applyhas no duration check, andvalidateTrackacceptsdurationMs == 0with one segment.makeAnimationthen setsanimation.duration = 0. Your comment inNativeAnimationTrack.hsays that the platform then uses its default duration (0.25 s in Core Animation).
So on the native route the value interpolates for about 0.25 s after the delay. On the CSS loop it jumps at the end of the delay (TransitionProgressProvider.cpp:126, duration_ == 0). I did not run this on a device. Suggested fix: give zero-duration tracks of the other policies the hold form too. For ExecutorCommitsEndpoint, the model already has the end value, so HeldThroughDelay works as it is. For HoldWithoutCommit, the track needs a final segment that holds toValue with fillMode = both. Another option is to make canRoute (or buildTrack) refuse zero-duration CSS runs, so that the loop plays them. A runtime case with transitionDuration: 0 and a delay in css native transitions would catch this.
Note
This pull request was authored by AI on behalf of @pawicao.
Summary
The native layout route on iOS refused a
withTimingwith a duration of zero, so such a layout animation stayed on the frame driver. WithIOS_LAYOUT_ANIMATIONS_CORE_ANIMATIONon, the route now takes it:playbackOf(NativeAnimationTrack.cpp) gives the playback form of each track. A track with no duration and a delay holds its start value through the delay, and then the model value shows; a track with no duration and no delay has no physical playback, and the host ends it in the operation that admits it. I also removed the Debug trace eventFirstFrameSampled, because its sample read the value before the commit of the frame, and I added systrace sections to the build, the track conversion, and the Core Animation steps. With the flag off, nothing changes.How it works
TrackPlaybackhas three forms:Interpolated: the value moves from the start value to the end value. Each track with a duration, and each track whose endpoint policy is notMountedModelMustMatchEndpoint, has this form.HeldThroughDelay: the start value stays for the delay. Then the model value shows. The model already holds the end value after the mount step, so the platform needs no interpolation.Immediate: no delay and no duration. The mounted model is already the end value, so the host reports the end of the track in the admission operation (NativeAnimationHost::endTrack). The command gets its result with no platform playback.I moved the end logic of a track from
onTrackEndedtoendTrack, so that the platform report and the immediate end use one path.The next-frame sampler of the Core Animation platform read the presentation value before the commit of the frame, so its value was not correct. I removed it and its trace event. The Debug target sample now gives the playback keys of the view: the Core Animation animations of the host on its layer, in the form
reanimated.<owner>.<generation>.<target>. A test can thus see which command plays a target.flowchart TD Track["AnimationTrack from the build"] --> P{"playbackOf(track)"} P -->|"duration > 0, or no MountedModelMustMatchEndpoint"| I["Interpolated: Core Animation playback"] P -->|"duration 0 and delay > 0"| H["HeldThroughDelay: start value for the delay, then the model"] P -->|"duration 0 and no delay"| M["Immediate: host ends the track at admission"] M --> Result["callback of the build gets its result"]Test plan
Jest: this pull request adds or changes no Jest test.
Runtime tests: the suite
native layout starts(apps/common-app/runtime-tests/reanimated/tests/layoutAnimations/nativeLayoutStarts.test.tsx) now checks the playback keys of a running and of an ended start, takes a zero duration natively, and has a Bezier easing that leaves the unit range.CI runs the runtime tests on iOS in
ReleaseRuntimeTestsand on Android inDebugRuntimeTests.The native cases need the native test hooks (the Debug-only trace and sampler on the module proxy) and the iOS route. They skip in the iOS Release build and on Android. They run in an iOS Debug build:
yarn workspace fabric-example runtime-tests --library reanimated --platform ios --udid <udid> --configuration DebugRuntimeTests --only 'native layout starts'.Changelog
yarn changelog:addfor each changed package, or this PR does not changereact-native-reanimatedorreact-native-worklets.