LottieAnimationView remove animation, but LottieAnimationLayer.rootAnimationLayer not remove

[skytoup]

Which Version of Lottie are you using?

Lottie 4.3.3

Expected Behavior

remove animation

let view = LottieAnimationView()
view.animation = nil

https://github.com/airbnb/lottie-ios/blob/45517c3cfec9469bbdd4f86e32393c28ae9df0bc/Sources/Public/Animation/LottieAnimationLayer.swift#L1149C3-L1149C3

Why not set the LottieAnimationLayer.rootAnimationLayer to nil? The rootAnimationLayer will continue reloadImages

[calda]

From reading the code, calling lottieAnimationView.animation = nil will then call lottieAnimationLayer.animation = nil.

Was there a bug or regression introduced by the architecture changes that added LottieAnimationLayer? If so please reopen this issue and share more details about the expected behavior and actual behavior

[calda]

I see, in Lottie 4.2.0 the implementation of LottieAnimationView.makeAnimationLayer had:

    if let oldAnimation = animationLayer {
      oldAnimation.removeFromSuperlayer()
      animationLayer = nil
    }

but the current implementation of LottieAnimationLayer.makeAnimationLayer instead has:

    if let oldAnimation = animationLayer {
      oldAnimation.removeFromSuperlayer()
    }

so we are no longer setting rootAnimationLayer = nil after removing that animation layer from the view hierarchy.

@eromanc made this change as a part of #2073. @eromanc -- was this intentional? Is this necessary for a specific reason?

I can't think of any reason why adding back the rootAnimationLayer = nil would be problematic.

[skytoup]

let view = LottieAnimationView()
view.animation = someAnim

view.animation = nil
view.imageProvider = aNewProvider

Setting the new imageProvider will trigger LottieAnimationLayer.reloadImages, because the rootAnimationLayeris not null, so it will reload the animation's image

[eromanc]

@calda perhaps because animationLayer is a get-only property now?

would this work instead?

if let oldAnimation = animationLayer {
       oldAnimation.removeFromSuperlayer()
      self.rootAnimationLayer = nil
     }

[calda]

Yeah, I think adding rootAnimationLayer = nil there would be correct and would match the previous behavior.

[eromanc]

I just tested that on Origami and nothing seems to break, so I think that change would be good on my end.