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

Core Animation rendering engine doesn't support text providers #1722

Closed
lostinstaches opened this issue Aug 18, 2022 · 12 comments · Fixed by #1723
Closed

Core Animation rendering engine doesn't support text providers #1722

lostinstaches opened this issue Aug 18, 2022 · 12 comments · Fixed by #1723

Comments

@lostinstaches
Copy link
Contributor

I am trying to use the Lottie animation with some text providers. I am trying to have TextAnimation.
However, the animations take too much memory (not necceseraliy the one in the example), and it would be great if it would be possible to use the core engine.

Do we know when it will be included? 🙏🏼

However, thank you for constantly improving Lottie; the project is incredible!

Which Version of Lottie are you using?

Lottie 3.4.2

Expected Behavior

I would expect to be able to use the text animations with the new engine because it would improve the performance of the app dramatically.

Actual Behavior

The app gets crashed with this message:
Thread 1: Fatal error: The Core Animation rendering engine currently doesn't support textProviders")

Animation JSON

An example of a text animation:

{"v":"5.9.6","fr":25,"ip":0,"op":125,"w":600,"h":600,"nm":"Position","ddd":0,"assets":[],"fonts":{"list":[{"origin":0,"fPath":"","fClass":"","fFamily":"Blogger Sans","fWeight":"","fStyle":"Bold","fName":"BloggerSans-Bold","ascent":72.8127343747765}]},"layers":[{"ddd":0,"ind":1,"ty":5,"nm":"Animation text layer","sr":1,"ks":{"o":{"a":0,"k":100,"ix":11},"r":{"a":0,"k":0,"ix":10},"p":{"a":1,"k":[{"i":{"x":0,"y":1},"o":{"x":0.167,"y":0.13},"t":0,"s":[297.387,665.7,0],"to":[0,-60.95,0],"ti":[0,60.95,0]},{"i":{"x":0.999,"y":0.999},"o":{"x":0.167,"y":0.167},"t":23,"s":[297.387,300,0],"to":[0,0,0],"ti":[0,0,0]},{"i":{"x":0.999,"y":1},"o":{"x":0.993,"y":0},"t":100,"s":[297.387,300,0],"to":[0,-54.2,0],"ti":[0,54.2,0]},{"t":124,"s":[297.387,-25.2,0]}],"ix":2,"l":2},"a":{"a":0,"k":[106.325,-9.295,0],"ix":1,"l":2},"s":{"a":0,"k":[100,100,100],"ix":6,"l":2}},"ao":0,"t":{"d":{"k":[{"s":{"sz":[601.23486328125,330.854888916016],"ps":[-192.50129699707,-43.9063301086426],"s":33.4000015258789,"f":"BloggerSans-Bold","t":"Animation text layer","ca":0,"j":2,"tr":0,"lh":40.0800018310547,"ls":0,"fc":[0.208,0.482,0.949]},"t":0}]},"p":{},"m":{"g":1,"a":{"a":0,"k":[0,0],"ix":2}},"a":[]},"ip":0,"op":125,"st":0,"ct":1,"bm":0}],"markers":[]}

@calda calda changed the title It is not possible to use TextProvider with new CoreAnimation enginer Core Animation rendering engine doesn't support text provider animations Aug 18, 2022
@lostinstaches
Copy link
Contributor Author

@calda , hey!
Thanks for taking care of my request.
Do you know when approximately it would be possible to use this new engine with text providers? 🙏🏼

@calda
Copy link
Member

calda commented Aug 18, 2022

Probably just needs to be implemented -- I don't have immediate plans to work on this but PRs are always welcome!

@lostinstaches
Copy link
Contributor Author

Okay! I might try it, but maybe you could give some advice on where to start.
I am totally a noob in Lottie underline, but I would love to help!

@calda
Copy link
Member

calda commented Aug 18, 2022

Here's the text layer implementation used by the Core Animation rendering engine: https://github.com/airbnb/lottie-ios/blob/master/Sources/Private/CoreAnimation/Layers/TextLayer.swift#L51

What text providers does your animation use?

The best way to set this up and start experimenting is to:

  1. clone this repo
  2. add your sample json to Tests/Samples/Issues/issue_1722.json
  3. open Lottie.xcworkspace
  4. build / run the Example project
  5. switch the example project to using the Core Animation rendering engine
  6. navigate to your animation in the example app

@lostinstaches
Copy link
Contributor Author

lostinstaches commented Aug 18, 2022

About the text providers:

AnimationTextProvider = DefaultTextProvider()

Just that, if I understand you correctly

@calda
Copy link
Member

calda commented Aug 18, 2022

Oh sorry, I was confused and thought you were talking about text animators. I forgot about custom text providers.

For those, we'd need to add the TextProvider to the LayerContext like we do for imageProvider and fontProvider, and check in TextLayer if the context.textProvider has a custom text value for the layer's key path. You can only get the layer's animation keypath in func setupAnimations(context: LayerAnimationContext) so we'd have to override that method in TextLayer and then set the layer text in that method instead.

We'd also need a test case that exercises this functionality, by hardcoding a custom text provider for a specific test case like we do here for testing image providers.

@calda calda changed the title Core Animation rendering engine doesn't support text provider animations Core Animation rendering engine doesn't support text providers Aug 18, 2022
@lostinstaches
Copy link
Contributor Author

Okay, I will try it out and hopefully open a PR with some ideas soon.
Maybe I can assign you there and ask for some next advice (if needed) over there? 🙏🏼

@lostinstaches
Copy link
Contributor Author

@calda Actually sorry, could you please say a bit more. 🙌🏼

I understood that we need to do the following:

  1. Add let textProvider: LayerTextProvider to the LayerContext in LayerModel+makeAnimationLayer.swift
  2. In TextLayer we want to understand if layer from context has a custom value. *
  3. In TextLayer override the setupAnimations(context:) method where we would set some layer to text? **

*Do I understand correctly that we should do it in the configureRenderLayer(with context:) method?
Something like that:

var hasCustomTextValue: Bool = false 

func configureRenderLayer(with context: LayerContext) throws {
      ... 
    _ = context.textProvider.textLayers.map { [unowned self] layer in
      guard layer.keypathLayer != nil else { return }
      self.hasCustomTextValue = true
    }
   ...
}

**And the part about overriding the setupAnimations is not clear as well, to be honest.
Should we operate with this value?:

override func setupAnimations(context: LayerAnimationContext) throws {
    try super.setupAnimations(context: context)
    context.currentKeypath // <---- THIS VALUE?
  }

And which layer text are we talking about?

ps. Thanks a lot for supporting, really hard to grasp the idea by using this library on a high level before! 🙏🏼

@calda
Copy link
Member

calda commented Aug 18, 2022

yes, something like:

override func setupAnimations(context: LayerAnimationContext) throws {
  try super.setupAnimations(context: context)
    
  if let customText = context.textProvider.text(for: context.currentKeypath) {
    renderLayer.text = customText
  } else {
    // currently in `func configureRenderLayer`, but we have to move it to this method now
    renderLayer.text = try textLayerModel.text.exactlyOneKeyframe(context: context, description: "text layer text").text
  }
}

@lostinstaches
Copy link
Contributor Author

Hm I think I understand the idea with renderLayer, but context of type LayerAnimationContext doesn't have the property textProvided. 🤔
And also I didn't see any extensions of LayerAnimationContext that could have them.

@calda
Copy link
Member

calda commented Aug 18, 2022

I think you'll need to add the textProvider to LayerAnimationContext, and pass it in from the LayerContext when constructing the LayerAnimationContext.

@lostinstaches
Copy link
Contributor Author

Hi @calda 👋🏼
Well, I opened this first attemp with a few questions. 🙌🏼
It surely builds, but some answers would be necceseray to proceed with valid PR.
Link to it here.
For some reason, I can't really add you as a reviewer now 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants