airbnb / lottie-ios

An iOS library to natively render After Effects vector animations
http://airbnb.io/lottie/
Apache License 2.0
25.76k stars 3.75k forks source link

Switching an in-flight animation from `LottieLoopMode.loop` to `.playOnce` restarts the animation. #2064

Closed jasdev closed 1 year ago

jasdev commented 1 year ago

Hi! We used to rely on a trick to gracefully finish out an animation when we need to pause it by switching the loop mode from .loop to .playOnce. Bisected it down and think this PR regressed that. If that wasn’t the recommended way to go about this (we also tried play(toProgress: 1, loopMode: .playOnce), but that also does the same), please let us know! 🙏🏽

Which Version of Lottie are you using?

Lottie 4.2.0

Expected Behavior

Switching an in-flight animation’s loop mode from .loop to .playOnce gracefully continues from the current moment to completion and stops.

Actual Behavior

Demo of the bug — notice the hard-cut restart of the animation at :05 when we switch loop modes.

https://github.com/airbnb/lottie-ios/assets/1276296/7e415157-c8cd-4670-aada-cc51a49c405e

calda commented 1 year ago

Can you share a sample project that demonstrates this issue? When I test this in the example app in this repo it still works as I expect:

calda commented 1 year ago

of if you can reproduce this in this repo's example app, repro steps there would work! (you can build and run the example app by downloading the repo and opening Lottie.xcworkspace)

jasdev commented 1 year ago

Pinned down the culprit and there’s an extra precondition I missed in the original report —

We were switching loop modes in an NSViewRepresentable.updateNSView call and noticed we also call into setValueProvider, specifically,

setValueProvider(
    ColorValueProvider(.init(r: …, g: …, …)),
    keypath: AnimationKeypath(keypath: "**.Fill 1.Color")
)

right before switching loop modes. If I swap the order and switch loop modes before calling into setValueProvider, everything works as expected. Swapping the order is easy enough on our end, but flagging in case that sequencing requirement is a regression. 🙏🏽

calda commented 1 year ago

What happens if you force your animation view to use the main thread rendering engine (via configuration: LottieConfiguration(renderingEngine: .mainThread))?

If the behavior is different using LottieConfiguration(renderingEngine: .mainThread) vs LottieConfiguration(renderingEngine: .coreAnimation) then this is a bug we should fix. If the behavior is the same then we'd leave it as-is.

Thanks!

jasdev commented 1 year ago

RenderingEngineOption.mainThread works with the original ordering, alas! We can work around this for now, so no frets if this is understandably lower pri. relative to other issues — appreciate the quick reply @calda, we’re Lottie Fans over here 🩵

Screenshot 2023-05-15 at 4 42 45 PM
calda commented 1 year ago

From the issue description it sounds like you're using a custom SwiftUI implementation. We recently added a SwiftUI LottieView on master. Could you check if this issue still occurs when using the first-party LottieView? If so, please share sample code that reproduces this issue. Thanks!

github-actions[bot] commented 1 year ago

This issue is stale because it is marked "can't reproduce" and has had no activity in the past week. Please comment with additional information, or this issue will be closed due to inactivity in one week.