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

Reduce log volume from warning about After Effects expressions #2196

Merged
merged 3 commits into from
Sep 20, 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
4 changes: 4 additions & 0 deletions Lottie.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@
0887347B28F0CCDD00458627 /* LottieAnimationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0887347428F0CCDD00458627 /* LottieAnimationView.swift */; };
0887347C28F0CCDD00458627 /* LottieAnimationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0887347428F0CCDD00458627 /* LottieAnimationView.swift */; };
0887347D28F0CCDD00458627 /* LottieAnimationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0887347428F0CCDD00458627 /* LottieAnimationView.swift */; };
089C50C22ABA0C6D007903D3 /* LoggingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 089C50C12ABA0C6D007903D3 /* LoggingTests.swift */; };
08AB05552A61C20400DE86FD /* ReducedMotionOption.swift in Sources */ = {isa = PBXBuildFile; fileRef = 08AB05542A61C20400DE86FD /* ReducedMotionOption.swift */; };
08AB05562A61C20400DE86FD /* ReducedMotionOption.swift in Sources */ = {isa = PBXBuildFile; fileRef = 08AB05542A61C20400DE86FD /* ReducedMotionOption.swift */; };
08AB05572A61C20400DE86FD /* ReducedMotionOption.swift in Sources */ = {isa = PBXBuildFile; fileRef = 08AB05542A61C20400DE86FD /* ReducedMotionOption.swift */; };
Expand Down Expand Up @@ -1176,6 +1177,7 @@
0887347228F0CCDD00458627 /* LottieAnimationHelpers.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LottieAnimationHelpers.swift; sourceTree = "<group>"; };
0887347328F0CCDD00458627 /* LottieAnimationViewInitializers.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LottieAnimationViewInitializers.swift; sourceTree = "<group>"; };
0887347428F0CCDD00458627 /* LottieAnimationView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LottieAnimationView.swift; sourceTree = "<group>"; };
089C50C12ABA0C6D007903D3 /* LoggingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoggingTests.swift; sourceTree = "<group>"; };
08AB05542A61C20400DE86FD /* ReducedMotionOption.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReducedMotionOption.swift; sourceTree = "<group>"; };
08AB05582A61C5B700DE86FD /* DecodingStrategy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DecodingStrategy.swift; sourceTree = "<group>"; };
08AB055C2A61C5CC00DE86FD /* RenderingEngineOption.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RenderingEngineOption.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1755,6 +1757,7 @@
08CB2680291ED2B700B4F071 /* AnimationViewTests.swift */,
2E70F79E295BB6D30089A0EF /* CompatibleAnimationViewTests.swift */,
080F5FDB2AB1075000ADC32C /* TextProviderTests.swift */,
089C50C12ABA0C6D007903D3 /* LoggingTests.swift */,
);
path = Tests;
sourceTree = "<group>";
Expand Down Expand Up @@ -3233,6 +3236,7 @@
2EAF59A727A076BC00E00531 /* Bundle+Module.swift in Sources */,
2E70F79F295BB6D30089A0EF /* CompatibleAnimationViewTests.swift in Sources */,
2E8044AE27A07347006E74CB /* Snapshotting+presentationLayer.swift in Sources */,
089C50C22ABA0C6D007903D3 /* LoggingTests.swift in Sources */,
36E57EAC28AF7ADF00B7EFDA /* HardcodedTextProvider.swift in Sources */,
2E72128527BB32DB0027BC56 /* PerformanceTests.swift in Sources */,
6DB3BDC328245AA2002A276D /* ParsingTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,14 @@ extension CALayer {
guard !keyframes.isEmpty else { return nil }

// Check if this set of keyframes uses After Effects expressions, which aren't supported.
if let unsupportedAfterEffectsExpression = keyframeGroup.unsupportedAfterEffectsExpression {
// - We only log this once per `CoreAnimationLayer` instance.
if keyframeGroup.unsupportedAfterEffectsExpression != nil, !context.loggingState.hasLoggedAfterEffectsExpressionsWarning {
context.loggingState.hasLoggedAfterEffectsExpressionsWarning = true
context.logger.info("""
`\(property.caLayerKeypath)` animation for "\(context.currentKeypath.fullPath)" \
includes an After Effects expression (https://helpx.adobe.com/after-effects/using/expression-language.html), \
which is not supported by lottie-ios (expressions are only supported by lottie-web). \
This animation may not play correctly.

\(unsupportedAfterEffectsExpression.replacingOccurrences(of: "\n", with: "\n "))

""")
}

Expand Down
2 changes: 2 additions & 0 deletions Sources/Private/CoreAnimation/CoreAnimationLayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ final class CoreAnimationLayer: BaseAnimationLayer {
private let valueProviderStore: ValueProviderStore
private let compatibilityTracker: CompatibilityTracker
private let logger: LottieLogger
private let loggingState = LoggingState()

/// The current playback state of the animation that is displayed in this layer
private var currentPlaybackState: PlaybackState? {
Expand Down Expand Up @@ -265,6 +266,7 @@ final class CoreAnimationLayer: BaseAnimationLayer {
valueProviderStore: valueProviderStore,
compatibilityTracker: compatibilityTracker,
logger: logger,
loggingState: loggingState,
currentKeypath: AnimationKeypath(keys: []),
textProvider: textProvider,
recordHierarchyKeypath: configuration.recordHierarchyKeypath)
Expand Down
19 changes: 19 additions & 0 deletions Sources/Private/CoreAnimation/Layers/AnimationLayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ struct LayerAnimationContext {
/// The logger that should be used for assertions and warnings
let logger: LottieLogger

/// Mutable state related to log events, stored on the `CoreAnimationLayer`.
let loggingState: LoggingState

/// The AnimationKeypath represented by the current layer
var currentKeypath: AnimationKeypath

Expand Down Expand Up @@ -84,3 +87,19 @@ struct LayerAnimationContext {
return copy
}
}

// MARK: - LoggingState

/// Mutable state related to log events, stored on the `CoreAnimationLayer`.
final class LoggingState {

// MARK: Lifecycle

init() { }

// MARK: Internal

/// Whether or not the warning about unsupported After Effects expressions
/// has been logged yet for this layer.
var hasLoggedAfterEffectsExpressionsWarning = false
}
106 changes: 106 additions & 0 deletions Tests/LoggingTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Created by Cal Stephens on 9/19/23.
// Copyright © 2023 Airbnb Inc. All rights reserved.

import SnapshotTesting
import UIKit
import XCTest

@testable import Lottie

// MARK: - LoggingTests

@MainActor
final class LoggingTests: XCTestCase {

// MARK: Internal

func testAnimationWithNoIssues() async {
await snapshotLoggedMessages(
animationName: "LottieLogo1",
configuration: LottieConfiguration(renderingEngine: .automatic))
}

func testAutomaticFallbackToMainThreadRenderingEngine() async {
// This animation is not supported by the Core Animation rendering engine
// because it uses time remapping
await snapshotLoggedMessages(
animationName: "Boat_Loader",
configuration: LottieConfiguration(renderingEngine: .automatic))
}

func testCoreAnimationRenderingEngineUnsupportedAnimation() async {
// This animation is not supported by the Core Animation rendering engine
// because it uses time remapping
await snapshotLoggedMessages(
animationName: "Boat_Loader",
configuration: LottieConfiguration(renderingEngine: .coreAnimation))
}

func testExplicitMainThreadRenderingEngine() async {
// This animation is not supported by the Core Animation rendering engine
// because it uses time remapping. Manually specifying the Main Thread
// rendering engine should silence the log messages.
await snapshotLoggedMessages(
animationName: "Boat_Loader",
configuration: LottieConfiguration(renderingEngine: .mainThread))
}

func testUnsupportedAfterEffectsExpressionsWarning() async {
// This animation has unsupported After Effects expressions, which triggers a log message
await snapshotLoggedMessages(
animationName: "LottieFiles/growth",
configuration: LottieConfiguration(renderingEngine: .automatic))
}

// MARK: Private

private func snapshotLoggedMessages(
animationName: String,
configuration: LottieConfiguration,
function: String = #function,
line: UInt = #line)
async
{
let loggedMessages = await loggedMessages(for: animationName, configuration: configuration)

assertSnapshot(
matching: loggedMessages.joined(separator: "\n"),
as: .description,
named: animationName,
testName: function,
line: line)
}

private func loggedMessages(for animationName: String, configuration: LottieConfiguration) async -> [String] {
var logMessages = [String]()

let logger = LottieLogger(
assert: { condition, message, _, _ in
if !condition() {
logMessages.append("[assertionFailure] \(message())")
}
},
assertionFailure: { message, _, _ in
logMessages.append("[assertionFailure] \(message())")
},
warn: { message, _, _ in
logMessages.append("[warning] \(message())")
},
info: { message in
logMessages.append("[info] \(message())")
})

let animationView = await SnapshotConfiguration.makeAnimationView(
for: animationName,
configuration: configuration,
logger: logger)!

animationView.forceDisplayUpdate()

if logMessages.isEmpty {
return ["Animation setup did not emit any logs"]
}

return logMessages
}
}
8 changes: 4 additions & 4 deletions Tests/SnapshotTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ class SnapshotTests: XCTestCase {
#if os(iOS)
for sampleAnimationName in Samples.sampleAnimationNames {
for percent in progressPercentagesToSnapshot(for: SnapshotConfiguration.forSample(named: sampleAnimationName)) {
guard SnapshotConfiguration.forSample(named: sampleAnimationName).shouldSnapshot(using: configuration) else {
continue
}

guard
let animationView = await SnapshotConfiguration.makeAnimationView(
for: sampleAnimationName,
Expand Down Expand Up @@ -282,10 +286,6 @@ extension SnapshotConfiguration {
{
let snapshotConfiguration = customSnapshotConfiguration ?? SnapshotConfiguration.forSample(named: sampleAnimationName)

guard snapshotConfiguration.shouldSnapshot(using: configuration) else {
return nil
}

let animationView: LottieAnimationView
if let animation = Samples.animation(named: sampleAnimationName) {
animationView = LottieAnimationView(
Expand Down
1 change: 1 addition & 0 deletions Tests/ValueProvidersTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ final class ValueProvidersTests: XCTestCase {
valueProviderStore: store,
compatibilityTracker: .init(mode: .track, logger: .printToConsole),
logger: .printToConsole,
loggingState: LoggingState(),
currentKeypath: .init(keys: []),
textProvider: DictionaryTextProvider([:]))

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Animation setup did not emit any logs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[warning] Encountered Core Animation compatibility issue while setting up animation:
[Chest] The Core Animation rendering engine partially supports time remapping keyframes, but this is somewhat experimental and has some known issues. Since it doesn't work in all cases, we have to fall back to using the main thread engine when using `RenderingEngineOption.automatic`.
This animation may have additional compatibility issues, but animation setup was cancelled early to avoid wasted work.

Automatically falling back to Main Thread rendering engine. This fallback comes with some additional performance
overhead, which can be reduced by manually specifying that this animation should always use the Main Thread engine.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[assertionFailure] Encountered Core Animation compatibility issues while setting up animation:
[Chest] The Core Animation rendering engine partially supports time remapping keyframes, but this is somewhat experimental and has some known issues. Since it doesn't work in all cases, we have to fall back to using the main thread engine when using `RenderingEngineOption.automatic`.

This animation cannot be rendered correctly by the Core Animation engine.
To resolve this issue, you can use `RenderingEngineOption.automatic`, which automatically falls back
to the Main Thread rendering engine when necessary, or just use `RenderingEngineOption.mainThread`.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Animation setup did not emit any logs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[info] `transform.rotation.z` animation for "men.man 1.hand 2.Transform" includes an After Effects expression (https://helpx.adobe.com/after-effects/using/expression-language.html), which is not supported by lottie-ios (expressions are only supported by lottie-web). This animation may not play correctly.
Loading