Add support to TextProviders for CoreAnimation
#1723
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,23 @@ final class TextLayer: BaseCompositionLayer { | |
| fatalError("init(coder:) has not been implemented") | ||
| } | ||
|
|
||
| override func setupAnimations(context: LayerAnimationContext) throws { | ||
| try super.setupAnimations(context: context) | ||
|
|
||
| _ = try context.currentKeypath.keys.map { key in | ||
|
There was a problem hiding this comment. I don't think we need this There was a problem hiding this comment. Got rid of it in c8a84c2 ✅ |
||
| // Question 2.1: Should we get from somewhere source text? What if the user has custom name? | ||
| let customText = context.textProvider.textFor(keypathName: context.currentKeypath.fullPath, sourceText: "text layer text") | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. But what if the JSON will have a different name for this text layer? |
||
| // Question 2.2: If we don't have this .isEmpty check, then how do we decide when to set our custom text, and when to | ||
| // set the result of the `exactlyOneKeyFrame()`? | ||
|
There was a problem hiding this comment. If you look at the behavior of the main thread rendering engine, it just unconditionally sets There was a problem hiding this comment. Updated the method in c8a84c2 🟡 Then we just set it like this? 🤔 |
||
| if customText.isEmpty { | ||
|
There was a problem hiding this comment. I don't think we need this There was a problem hiding this comment. Updated the method in 5055f10 🟡 Okay, I see. But then I just don't understand how do we actually decide when to set this: and when do this:
There was a problem hiding this comment. we always just do There was a problem hiding this comment. Okay, then it is this way already in the code! 🎉 |
||
| renderLayer.text = customText | ||
| } else { | ||
| renderLayer.text = try textLayerModel.text.exactlyOneKeyframe(context: context, description: "text layer text").text | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| /// Called by CoreAnimation to create a shadow copy of this layer | ||
| /// More details: https://developer.apple.com/documentation/quartzcore/calayer/1410842-init | ||
| override init(layer: Any) { | ||
|
|
@@ -48,13 +65,15 @@ final class TextLayer: BaseCompositionLayer { | |
| // - We may be able to support animating `fillColor` by getting clever with layer blend modes | ||
| // or masks (e.g. use `CoreTextRenderLayer` to draw black glyphs, and then fill them in | ||
| // using a `CAShapeLayer`). | ||
|
|
||
| // Question 3: Should we somehow initialize LayerContextAnimations at this stage | ||
|
There was a problem hiding this comment. I think we can remove the There was a problem hiding this comment. Right! Fixed in 01307aa ✅ |
||
| // and pass it to `self.setupAnimations(context: layerAnimationsContext)` | ||
| if !textLayerModel.animators.isEmpty { | ||
| try context.logCompatibilityIssue(""" | ||
| The Core Animation rendering engine currently doesn't support text animators. | ||
| """) | ||
| } | ||
|
|
||
| renderLayer.font = context.fontProvider.fontFor(family: text.fontFamily, size: CGFloat(text.fontSize)) | ||
|
|
||
| renderLayer.alignment = text.justification.textAlignment | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,9 @@ struct SnapshotConfiguration { | |
| /// A custom `AnimationImageProvider` to use when rendering this animation | ||
| var customImageProvider: AnimationImageProvider? | ||
|
|
||
| /// A custom `AnimationTextProvider` to use when rendering this animatino | ||
| var customTextProvider: AnimationTextProvider? | ||
|
There was a problem hiding this comment. Make sure to actually apply this text provider to the There was a problem hiding this comment. Cool to know, thanks! |
||
|
|
||
| /// A custom `AnimationFontProvider` to use when rendering this animation | ||
| var customFontProvider: AnimationFontProvider? | ||
|
|
||
|
|
@@ -70,6 +73,9 @@ extension SnapshotConfiguration { | |
| // Test cases for `AnimatedImageProvider` | ||
| "Nonanimating/_dog": .customImageProvider(HardcodedImageProvider(imageName: "Samples/Images/dog.png")), | ||
|
|
||
| // Test cases for `AnimatedTextProvider` | ||
| "Issues/issue_1722": .customTextProvider(HardcodedTextProvider(text: "Bounce-bounce")), | ||
|
There was a problem hiding this comment. When you're done with the code changes you can generate the snapshot images for this by running the unit test target on an iPhone 8 simulator (or by running There was a problem hiding this comment. fyi I think the snapshot tests are sensitive to specific Xcode versions -- right now I think they run correctly on Xcode 13.4.1 but not on Xcode 14. If you get a lot of spurious failures when you run the tests let me know and I can pull you branch and run them for you. There was a problem hiding this comment. Hm unfortunately it always return this message during the test run: There was a problem hiding this comment.
Yeah, Cal, that would be helpful, thanks a bunch! 🙏🏼 There was a problem hiding this comment. By the way the name of the text layer in the provided animation is There was a problem hiding this comment.
This is expected the first time you run the tests after you add a new sample JSON file. If you run the tests again this goes away, since now it compares the new snapshot with the previously-recorded one. There was a problem hiding this comment. Hm true. However, the snapshots anyway don't contain the text. Could it be because the text layer has a different name? There was a problem hiding this comment. I think this was because we had to call |
||
|
|
||
| // Test cases for `AnimationFontProvider` | ||
| "Nonanimating/Text_Glyph": .customFontProvider(HardcodedFontProvider(font: UIFont(name: "Chalkduster", size: 36)!)), | ||
|
|
||
|
|
@@ -128,6 +134,15 @@ extension SnapshotConfiguration { | |
| return configuration | ||
| } | ||
|
|
||
| static func customTextProvider( | ||
| _ customTextProvider: AnimationTextProvider) | ||
| -> SnapshotConfiguration | ||
| { | ||
| var configuration = SnapshotConfiguration.default | ||
| configuration.customTextProvider = customTextProvider | ||
| return configuration | ||
| } | ||
|
|
||
| /// A `SnapshotConfiguration` value using the given custom value providers | ||
| static func customFontProvider( | ||
| _ customFontProvider: AnimationFontProvider) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,23 @@ | ||||||||||
| // Created by Igor Katselenbogen on 08/19/22. | ||||||||||
| // Copyright © 2022 Airbnb Inc. All rights reserved. | ||||||||||
|
|
||||||||||
| import Lottie | ||||||||||
| import QuartzCore | ||||||||||
| #if os(iOS) | ||||||||||
| import UIKit | ||||||||||
| #endif | ||||||||||
|
There was a problem hiding this comment. Are these necessary?
Suggested change
|
||||||||||
|
|
||||||||||
| // MARK: - HardcodedImageProvider | ||||||||||
|
|
||||||||||
| /// An `AnimationTextProvider` that always returns a specific hardcoded text | ||||||||||
| class HardcodedTextProvider: AnimationTextProvider { | ||||||||||
| private let text: String | ||||||||||
|
|
||||||||||
| init(text: String) { | ||||||||||
| self.text = text | ||||||||||
| } | ||||||||||
|
|
||||||||||
| func textFor(keypathName: String, sourceText: String) -> String { | ||||||||||
| text | ||||||||||
| } | ||||||||||
| } | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should pass the entire keypath here, e.g.
context.currentKeypath.fullPath