diff --git a/Package.swift b/Package.swift index 075b7515..0cd2783e 100644 --- a/Package.swift +++ b/Package.swift @@ -9,7 +9,7 @@ let package = Package( ], dependencies: [ .package(url: "https://source.skip.tools/skip.git", from: "1.7.8"), - .package(url: "https://source.skip.tools/skip-model.git", from: "1.7.2"), + .package(url: "https://github.com/tifroz/skip-model.git", branch: "withanimation-global-scope-fix"), ], targets: [ .target(name: "SkipUI", dependencies: [.product(name: "SkipModel", package: "skip-model")], plugins: [.plugin(name: "skipstone", package: "skip")]), diff --git a/Sources/SkipUI/SkipUI/Animation/Animation.swift b/Sources/SkipUI/SkipUI/Animation/Animation.swift index ad62415b..21dcf458 100644 --- a/Sources/SkipUI/SkipUI/Animation/Animation.swift +++ b/Sources/SkipUI/SkipUI/Animation/Animation.swift @@ -2,6 +2,7 @@ // SPDX-License-Identifier: MPL-2.0 #if !SKIP_BRIDGE import Foundation +import SkipModel #if SKIP import androidx.compose.animation.Animatable import androidx.compose.animation.core.Animatable @@ -92,6 +93,20 @@ extension View { final class AnimationHolder { var animation: Animation? } + +final class AnimationTransaction: StateMutationTransaction { + let id: Int + let animation: Animation? + + init(id: Int, animation: Animation?) { + self.id = id + self.animation = animation + } +} + +public enum AnimationDebug { + public static var decisionSink: ((String) -> Void)? +} #endif // SKIP @bridge @@ -109,7 +124,13 @@ public struct Animation : Hashable { var isNested = false synchronized (withAnimationLock) { isNested = _withAnimation != nil + nextWithAnimationTransactionID += 1 + let transaction = AnimationTransaction(id: nextWithAnimationTransactionID, animation: animation) + withAnimationTransactions.append(transaction) + StateTracking.currentMutationTransaction = transaction + StateTracking.clearMutationReads() _withAnimation = animation + debugLog("open withAnimation \(debugDescription(for: withAnimationTransactions.last)) nested=\(isNested)") } return isNested #else @@ -120,10 +141,17 @@ public struct Animation : Hashable { // SKIP @bridge public static func postBodyWithAnimation() { #if SKIP + synchronized (withAnimationLock) { + debugLog("clear withAnimation transactions after body") + withAnimationTransactions.removeAll() + StateTracking.currentMutationTransaction = nil + } GlobalScope.async(Dispatchers.Main) { awaitFrame() synchronized (withAnimationLock) { + debugLog("clear global withAnimation after frame") _withAnimation = nil + StateTracking.clearMutationReads() } } #endif @@ -151,6 +179,33 @@ public struct Animation : Hashable { return isAnimating ? rememberedAnimation : nil } + /// The current active animation for normal value animatables. + /// + /// Value animatables should be driven by explicit `.animation(..., value:)` + /// environment state or by a transaction attached to the state write that + /// caused the value change. The legacy global `_withAnimation` frame window + /// remains only for transition surfaces that cannot yet be tied to reads. + @Composable static func currentForAnimatable(isAnimating: Bool) -> Animation? { + let environmentAnimation = EnvironmentValues.shared._animation + let transaction = StateTracking.consumeMutationRead() as? AnimationTransaction + let animation = environmentAnimation ?? transaction?.animation + + let rememberedAnimationHolder = remember { AnimationHolder() } + let rememberedAnimation = rememberedAnimationHolder.animation + if animation != nil { + rememberedAnimationHolder.animation = animation + } else if !isAnimating { + rememberedAnimationHolder.animation = nil + } + + debugDecision("animatable transaction=\(debugDescription(for: transaction)) environmentAnimation=\(environmentAnimation != nil) usesAnimation=\(animation != nil)") + + guard animation == nil else { + return animation + } + return isAnimating ? rememberedAnimation : nil + } + /// Whether we're in a `withAnimation` block. static var isInWithAnimation: Bool { synchronized (withAnimationLock) { @@ -158,10 +213,36 @@ public struct Animation : Hashable { } } + static var currentTransaction: AnimationTransaction? { + synchronized (withAnimationLock) { + return withAnimationTransactions.last + } + } + + static func debugDescription(for transaction: AnimationTransaction?) -> String { + if let transaction { + return "#\(transaction.id) animation=\(transaction.animation != nil)" + } else { + return "none" + } + } + + static func debugLog(_ message: String) { + if debugWithAnimationTransactions { + android.util.Log.d("SkipUI.Animation", message) + } + } + + static func debugDecision(_ message: String) { + AnimationDebug.decisionSink?(message) + debugLog(message) + } + /// Internal implementation of global `withAnimation` SwiftUI function. static func withAnimation(_ animation: Animation? = .default, _ body: () throws -> Result) rethrows -> Result { let isNested = preBodyWithAnimation(animation) defer { + finishBodyWithAnimation() if !isNested { postBodyWithAnimation() } @@ -169,7 +250,19 @@ public struct Animation : Hashable { return body() } + private static func finishBodyWithAnimation() { + synchronized (withAnimationLock) { + if !withAnimationTransactions.isEmpty { + withAnimationTransactions.removeLast() + } + StateTracking.currentMutationTransaction = withAnimationTransactions.last + } + } + private static var _withAnimation: Animation? + private static var withAnimationTransactions: [AnimationTransaction] = [] + private static var nextWithAnimationTransactionID = 0 + private static let debugWithAnimationTransactions = false private static let withAnimationLock: java.lang.Object = java.lang.Object() private let spec: AnimationSpec @@ -458,7 +551,8 @@ public enum AnimationCompletionCriteria : Hashable { let animatable = remember { Animatable(resetValue.value ?? value, converter) } let isAnimating = animatable.isRunning || animatable.value != animatable.targetValue if isAnimating || animatable.value != value { - let animation = Animation.current(isAnimating: isAnimating) + let animation = Animation.currentForAnimatable(isAnimating: isAnimating) + Animation.debugDecision("animatable change value=\(value) isAnimating=\(isAnimating) usesAnimation=\(animation != nil)") LaunchedEffect(value, animation) { if let animation { if animation.isInfinite { diff --git a/Sources/SkipUI/SkipUI/BridgeSupport/StateSupport.swift b/Sources/SkipUI/SkipUI/BridgeSupport/StateSupport.swift index e001d09d..410e6f63 100644 --- a/Sources/SkipUI/SkipUI/BridgeSupport/StateSupport.swift +++ b/Sources/SkipUI/SkipUI/BridgeSupport/StateSupport.swift @@ -41,6 +41,7 @@ public final class StateSupport: StateTracker { // SKIP @bridge public func access() { #if SKIP + StateTracking.recordMutationRead(lastAnimationTransaction) let _ = state?.value #endif } @@ -48,6 +49,8 @@ public final class StateSupport: StateTracker { // SKIP @bridge public func update() { #if SKIP + lastAnimationTransaction = StateTracking.currentMutationTransaction as? AnimationTransaction + Animation.debugLog("bridged StateSupport update transaction=\(Animation.debugDescription(for: lastAnimationTransaction))") state?.value += 1 #endif } @@ -58,6 +61,10 @@ public final class StateSupport: StateTracker { state = mutableStateOf(0) #endif } + + #if SKIP + var lastAnimationTransaction: AnimationTransaction? + #endif } #endif diff --git a/Sources/SkipUI/SkipUI/Properties/State.swift b/Sources/SkipUI/SkipUI/Properties/State.swift index 50b90665..ab5d610e 100644 --- a/Sources/SkipUI/SkipUI/Properties/State.swift +++ b/Sources/SkipUI/SkipUI/Properties/State.swift @@ -22,6 +22,7 @@ public final class State: StateTracker { get { #if SKIP if let _wrappedValueState { + StateTracking.recordMutationRead(lastAnimationTransaction) return _wrappedValueState.value } #endif @@ -30,6 +31,8 @@ public final class State: StateTracker { set { _wrappedValue = newValue #if SKIP + lastAnimationTransaction = StateTracking.currentMutationTransaction as? AnimationTransaction + Animation.debugLog("State write transaction=\(Animation.debugDescription(for: lastAnimationTransaction))") _wrappedValueState?.value = _wrappedValue #endif } @@ -37,6 +40,7 @@ public final class State: StateTracker { private var _wrappedValue: Value #if SKIP private var _wrappedValueState: MutableState? + var lastAnimationTransaction: AnimationTransaction? #endif public var projectedValue: Binding { diff --git a/Tests/SkipUITests/AnimationTransactionTests.swift b/Tests/SkipUITests/AnimationTransactionTests.swift new file mode 100644 index 00000000..b39d90a3 --- /dev/null +++ b/Tests/SkipUITests/AnimationTransactionTests.swift @@ -0,0 +1,141 @@ +// Copyright 2026 Skip +// SPDX-License-Identifier: MPL-2.0 +import SwiftUI +import XCTest + +#if SKIP +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.Saver +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import org.junit.Rule +#endif + +final class AnimationTransactionTests: SkipUITestCase { + // SKIP INSERT: @get:Rule val composeRule = createComposeRule() + + func testOutsideThenInsideWithAnimationScopesOnlyInsideWrite() throws { + #if !SKIP + throw XCTSkip("Animation transaction decisions are Android-only") + #else + let decisions = try runScenario(.outsideThenInside) + XCTAssertDecision(decisions, at: 0, usesAnimation: false) + XCTAssertDecision(decisions, at: 1, usesAnimation: true) + #endif + } + + func testBothInsideWithAnimationBothAnimate() throws { + #if !SKIP + throw XCTSkip("Animation transaction decisions are Android-only") + #else + let decisions = try runScenario(.bothInside) + XCTAssertDecision(decisions, at: 0, usesAnimation: true) + XCTAssertDecision(decisions, at: 1, usesAnimation: true) + #endif + } + + func testWithoutWithAnimationBothSnap() throws { + #if !SKIP + throw XCTSkip("Animation transaction decisions are Android-only") + #else + let decisions = try runScenario(.neitherInside) + XCTAssertDecision(decisions, at: 0, usesAnimation: false) + XCTAssertDecision(decisions, at: 1, usesAnimation: false) + #endif + } + + func testValueAnimationEnvironmentTakesPrecedence() throws { + #if !SKIP + throw XCTSkip("Animation transaction decisions are Android-only") + #else + let decisions = try runScenario(.environmentOverridesTransaction) + XCTAssertDecision(decisions, at: 0, usesAnimation: true) + XCTAssertDecision(decisions, at: 1, usesAnimation: true) + XCTAssertTrue(decisions.contains { $0.contains("environmentAnimation=true") && $0.contains("usesAnimation=true") }, decisions.joined(separator: "\n")) + #endif + } + + #if SKIP + private func runScenario(_ scenario: AnimationTransactionScenario) throws -> [String] { + var decisions: [String] = [] + AnimationDebug.decisionSink = { decisions.append($0) } + defer { AnimationDebug.decisionSink = nil } + + composeRule.setContent { + AnimationTransactionTestView(scenario: scenario).Compose() + } + composeRule.waitForIdle() + + decisions.removeAll() + composeRule.onNodeWithTag("run").performClick() + composeRule.waitForIdle() + + return decisions + } + + private func XCTAssertDecision(_ decisions: [String], at index: Int, usesAnimation: Bool) { + let expected = "usesAnimation=\(usesAnimation)" + let changes = decisions.filter { $0.contains("animatable change value=") } + XCTAssertTrue(changes.count > index, decisions.joined(separator: "\n")) + XCTAssertTrue(changes[index].contains(expected), decisions.joined(separator: "\n")) + } + #endif +} + +#if SKIP +private enum AnimationTransactionScenario { + case outsideThenInside + case bothInside + case neitherInside + case environmentOverridesTransaction +} + +private struct AnimationTransactionTestView: View { + let scenario: AnimationTransactionScenario + + @State private var redOpacity = 1.0 + @State private var greenOpacity = 1.0 + + var body: some View { + VStack { + Color.red + .opacity(redOpacity) + .frame(width: 8.0, height: 8.0) + Color.green + .opacity(greenOpacity) + .frame(width: 8.0, height: 8.0) + Button("Run") { + switch scenario { + case .outsideThenInside: + redOpacity = 0.25 + withAnimation(.linear(duration: 1.5)) { + greenOpacity = 0.5 + } + case .bothInside: + withAnimation(.linear(duration: 1.5)) { + redOpacity = 0.25 + greenOpacity = 0.5 + } + case .neitherInside: + redOpacity = 0.25 + greenOpacity = 0.5 + case .environmentOverridesTransaction: + withAnimation(.linear(duration: 1.5)) { + redOpacity = 0.25 + greenOpacity = 0.5 + } + } + } + .accessibilityIdentifier("run") + .buttonStyle(.bordered) + } + .animation(scenario == .environmentOverridesTransaction ? .easeInOut(duration: 0.25) : nil, value: redOpacity) + .animation(scenario == .environmentOverridesTransaction ? .easeInOut(duration: 0.25) : nil, value: greenOpacity) + } +} +#endif