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

Fix issue where Core Animation rendering engine couldn't display last frame of animation when paused #2254

Merged
merged 8 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ extension CAAnimation {
// 0% 100%
//
let baseAnimation = self
baseAnimation.duration = context.animation.duration
baseAnimation.duration = context.animationDuration
baseAnimation.speed = (context.endFrame < context.startFrame) ? -1 : 1

// To select the subrange of the `baseAnimation` that should be played,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ extension CALayer {
animation.keyTimes = [
NSNumber(value: 0.0),
NSNumber(value: max(Double(context.progressTime(for: inFrame)), 0)),
NSNumber(value: min(Double(context.progressTime(for: outFrame)), 1)),
// Anything visible during the last frame should stay visible until the absolute end of the animation.
// - This matches the behavior of the main thread rendering engine.
context.timeRemapping(outFrame) == context.animation.endFrame
? NSNumber(value: Double(1.0))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, some animations (like LottieLogo1.json) would unexpectedly render a blank frame as the last frame.

This matches the behavior of the main thread rendering engine as far as I can tell, but causes bugs. e.g. this change is why the "1" and "9/10" overlap in this screenshot:

For now let's prioritize parity with the main thread rendering engine.

: NSNumber(value: min(Double(context.progressTime(for: outFrame)), 1)),
NSNumber(value: 1.0),
]

Expand Down
5 changes: 4 additions & 1 deletion Sources/Private/CoreAnimation/CoreAnimationLayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,10 @@ extension CoreAnimationLayer: RootAnimationLayer {
let requiredAnimationConfiguration = AnimationConfiguration(
animationContext: AnimationContext(
playFrom: animation.startFrame,
playTo: animation.endFrame,
// Normal animation playback (like when looping) skips the last frame.
// However when the animation is paused, we need to be able to render the final frame.
// To allow this we have to extend the length of the animation by one frame.
playTo: animation.endFrame + 1,
closure: nil),
timingConfiguration: CAMediaTimingConfiguration(speed: 0))

Expand Down
18 changes: 17 additions & 1 deletion Sources/Private/CoreAnimation/Layers/AnimationLayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,21 @@ struct LayerAnimationContext {
/// in the animation's overall global time
private(set) var timeRemapping: ((AnimationFrameTime) -> AnimationFrameTime) = { $0 }

/// The duration of the animation
var animationDuration: AnimationFrameTime {
// Normal animation playback (like when looping) skips the last frame.
// However when the animation is paused, we need to be able to render the final frame.
// To allow this we have to extend the length of the animation by one frame.
let animationEndFrame: AnimationFrameTime
if timingConfiguration.speed == 0 {
animationEndFrame = animation.endFrame + 1
} else {
animationEndFrame = animation.endFrame
}

return Double(animationEndFrame - animation.startFrame) / animation.framerate
}

/// Adds the given component string to the `AnimationKeypath` stored
/// that describes the current path being configured by this context value
func addingKeypathComponent(_ component: String) -> LayerAnimationContext {
Expand All @@ -66,7 +81,8 @@ struct LayerAnimationContext {
/// The `AnimationProgressTime` for the given `AnimationFrameTime` within this layer,
/// accounting for the `timeRemapping` applied to this layer
func progressTime(for frame: AnimationFrameTime) -> AnimationProgressTime {
animation.progressTime(forFrame: timeRemapping(frame), clamped: false)
let animationFrameCount = animationDuration * animation.framerate
return (timeRemapping(frame) - animation.startFrame) / animationFrameCount
}

/// The real-time `TimeInterval` for the given `AnimationFrameTime` within this layer,
Expand Down
2 changes: 1 addition & 1 deletion Sources/Public/Animation/LottieView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ public struct LottieView<Placeholder: View>: UIViewConfiguringSwiftUIView {
/// a state update every frame.
/// - If the binding is `nil`, the `TimelineView` will be paused and no updates will occur to the binding.
@available(iOS 15.0, tvOS 15.0, macOS 12.0, *)
public func getRealtimeAnimationFrame(_ realtimeAnimationFrame: Binding<AnimationProgressTime>?) -> some View {
public func getRealtimeAnimationFrame(_ realtimeAnimationFrame: Binding<AnimationFrameTime>?) -> some View {
TimelineView(.animation(paused: realtimeAnimationFrame == nil)) { _ in
configure { view in
if let realtimeAnimationFrame = realtimeAnimationFrame {
Expand Down
1 change: 1 addition & 0 deletions Tests/Samples/Issues/issue_2209.json

Large diffs are not rendered by default.

20 changes: 19 additions & 1 deletion Tests/SnapshotConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ struct SnapshotConfiguration {
/// the code supporting the automatic engine.
var testWithAutomaticEngine = false

/// Whether or not this snapshot doesn't animate, so only needs to be snapshot once.
/// Custom progress values (from 0 to 1) that should be screenshot
var customProgressValuesToSnapshot: [Double]?

/// Custom frame values that should be screenshot
var customFramesToSnapshot: [Double]?

/// The maximum size to allow for the resulting snapshot image
var maxSnapshotDimension: CGFloat = 500
}
Expand Down Expand Up @@ -156,6 +159,14 @@ extension SnapshotConfiguration {
]))
.progressValuesToSnapshot([0.3, 0.75]),

"Issues/issue_2209": SnapshotConfiguration.default
.framesToSnapshot([
4.999, // Should show frame 4
5.0, // Should show frame 5
9.9999999, // Should show frame 9
10, // Should show frame 10
]),

// Test cases for `AnimationFontProvider`
"Nonanimating/Text_Glyph": .customFontProvider(HardcodedFontProvider(font: UIFont(name: "Chalkduster", size: 36)!)),

Expand Down Expand Up @@ -255,6 +266,13 @@ extension SnapshotConfiguration {
return copy
}

/// A copy of this `SnapshotConfiguration` with `customFramesToSnapshot` set to the given value
func framesToSnapshot(_ framesToSnapshot: [Double]) -> SnapshotConfiguration {
var copy = self
copy.customFramesToSnapshot = framesToSnapshot
return copy
}

/// A copy of this `SnapshotConfiguration` with `maxSnapshotDimension` set to the given value
func maxSnapshotDimension(_ maxSnapshotDimension: CGFloat) -> SnapshotConfiguration {
var copy = self
Expand Down
50 changes: 42 additions & 8 deletions Tests/SnapshotTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ class SnapshotTests: XCTestCase {
with: "")
}

for frame in knownFrameValues {
animationName = animationName.replacingOccurrences(
of: "-Frame-\(Int(frame)).png",
with: "")
}

animationName = animationName.replacingOccurrences(of: "-", with: "/")

XCTAssert(
Expand Down Expand Up @@ -98,22 +104,36 @@ class SnapshotTests: XCTestCase {

// MARK: Private

/// The progress percentage values that are snapshot by default
private static let defaultProgressPercentageValues: [Double] = [0, 0.25, 0.5, 0.75, 1.0]

/// All of the `progressPercentagesToSnapshot` values used in the snapshot tests
private let knownProgressPercentageValues = Set([0, 0.25, 0.3, 0.5, 0.75, 1.0])
private let knownProgressPercentageValues: Set<Double> = Set(Samples.sampleAnimationNames.flatMap {
SnapshotConfiguration.forSample(named: $0).customProgressValuesToSnapshot ?? defaultProgressPercentageValues
})

/// All of the `customFramesToSnapshot` values used in the snapshot tests
private let knownFrameValues: Set<Double> = Set(Samples.sampleAnimationNames.flatMap {
SnapshotConfiguration.forSample(named: $0).customFramesToSnapshot ?? []
})

/// Progress values or frames that should be snapshot in `compareSampleSnapshots`
private func pausedStatesToSnapshot(for snapshotConfiguration: SnapshotConfiguration) -> [LottiePlaybackMode.PausedState] {
if let customFramesToSnapshot = snapshotConfiguration.customFramesToSnapshot {
return customFramesToSnapshot.map { .frame($0) }
}

/// `currentProgress` percentages that should be snapshot in `compareSampleSnapshots`
private func progressPercentagesToSnapshot(for snapshotConfiguration: SnapshotConfiguration) -> [Double] {
if let customProgressValuesToSnapshot = snapshotConfiguration.customProgressValuesToSnapshot {
for customProgressValue in customProgressValuesToSnapshot {
assert(
knownProgressPercentageValues.contains(customProgressValue),
"All progress values being used must be listed in `knownProgressPercentageValues`")
}

return customProgressValuesToSnapshot
return customProgressValuesToSnapshot.map { .progress($0) }
}

return [0, 0.25, 0.5, 0.75, 1.0]
return SnapshotTests.defaultProgressPercentageValues.map { .progress($0) }
}

/// Captures snapshots of `sampleAnimationURLs` and compares them to the snapshot images stored on disk
Expand All @@ -126,7 +146,7 @@ class SnapshotTests: XCTestCase {

#if os(iOS)
for sampleAnimationName in Samples.sampleAnimationNames {
for percent in progressPercentagesToSnapshot(for: SnapshotConfiguration.forSample(named: sampleAnimationName)) {
for pauseState in pausedStatesToSnapshot(for: SnapshotConfiguration.forSample(named: sampleAnimationName)) {
guard SnapshotConfiguration.forSample(named: sampleAnimationName).shouldSnapshot(using: configuration) else {
continue
}
Expand All @@ -137,14 +157,28 @@ class SnapshotTests: XCTestCase {
configuration: configuration)
else { continue }

animationView.currentProgress = CGFloat(percent)
animationView.setPlaybackMode(.paused(at: pauseState))

let pauseStateDescription: String
switch pauseState {
case .progress(let percent):
pauseStateDescription = "\(Int(percent * 100))%"
case .frame(let frame):
pauseStateDescription = "Frame \(Int(frame))"
case .time(let time):
pauseStateDescription = "Time \(time))"
case .marker(let markerName, position: _):
pauseStateDescription = markerName
case .currentFrame:
pauseStateDescription = "Current Frame"
}

assertSnapshot(
matching: animationView,
as: .imageOfPresentationLayer(
precision: SnapshotConfiguration.forSample(named: sampleAnimationName).precision,
perceptualPrecision: 0.97),
named: "\(sampleAnimationName) (\(Int(percent * 100))%)",
named: "\(sampleAnimationName) (\(pauseStateDescription))",
testName: testName)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Supports Core Animation engine
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading