Only animate CGSize and CGPoint key paths additively if additive is enabled. (#72)

jverkoey · web-flow · commit 524b3cb34d58 · 2017-11-29T15:53:04.000-05:00
Prior to this change, CGSize and CGPoint animations were being animated additively even if the additive property was disabled.

Added tests to catch regressions in the future.
diff --git a/examples/apps/Catalog/MotionAnimatorCatalog.xcodeproj/project.pbxproj b/examples/apps/Catalog/MotionAnimatorCatalog.xcodeproj/project.pbxproj
@@ -12,6 +12,8 @@
 		6625876C1FB4DB9C00BC7DF1 /* InitialVelocityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6625876B1FB4DB9C00BC7DF1 /* InitialVelocityTests.swift */; };
 		664F59941FCCE27E002EC56D /* UIKitBehavioralTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 664F59931FCCE27E002EC56D /* UIKitBehavioralTests.swift */; };
 		664F59961FCDB2E6002EC56D /* QuartzCoreBehavioralTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 664F59951FCDB2E5002EC56D /* QuartzCoreBehavioralTests.swift */; };
+		664F599A1FCE6661002EC56D /* NonAdditiveAnimatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 664F59991FCE6661002EC56D /* NonAdditiveAnimatorTests.swift */; };
+		664F599C1FCE67DB002EC56D /* AdditiveAnimatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 664F599B1FCE67DB002EC56D /* AdditiveAnimatorTests.swift */; };
 		666FAA841D384A6B000363DA /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 666FAA831D384A6B000363DA /* AppDelegate.swift */; };
 		666FAA8B1D384A6B000363DA /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 666FAA8A1D384A6B000363DA /* Assets.xcassets */; };
 		666FAA8E1D384A6B000363DA /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 666FAA8C1D384A6B000363DA /* LaunchScreen.storyboard */; };
@@ -55,6 +57,8 @@
 		6625876B1FB4DB9C00BC7DF1 /* InitialVelocityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InitialVelocityTests.swift; sourceTree = "<group>"; };
 		664F59931FCCE27E002EC56D /* UIKitBehavioralTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIKitBehavioralTests.swift; sourceTree = "<group>"; };
 		664F59951FCDB2E5002EC56D /* QuartzCoreBehavioralTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = QuartzCoreBehavioralTests.swift; sourceTree = "<group>"; };
+		664F59991FCE6661002EC56D /* NonAdditiveAnimatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NonAdditiveAnimatorTests.swift; sourceTree = "<group>"; };
+		664F599B1FCE67DB002EC56D /* AdditiveAnimatorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AdditiveAnimatorTests.swift; sourceTree = "<group>"; };
 		666FAA801D384A6B000363DA /* MotionAnimatorCatalog.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = MotionAnimatorCatalog.app; sourceTree = BUILT_PRODUCTS_DIR; };
 		666FAA831D384A6B000363DA /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = AppDelegate.swift; path = Catalog/AppDelegate.swift; sourceTree = "<group>"; };
 		666FAA8A1D384A6B000363DA /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = "<group>"; };
@@ -218,13 +222,15 @@
 		66FD99F81EE9FBA000C53A82 /* unit */ = {
 			isa = PBXGroup;
 			children = (
+				664F599B1FCE67DB002EC56D /* AdditiveAnimatorTests.swift */,
 				66A6A6671FBA158000DE54CB /* AnimationRemovalTests.swift */,
 				66EF6F271FC33C4800C83A63 /* HeadlessLayerImplicitAnimationTests.swift */,
 				66BF5A8E1FB0E4CB00E864F6 /* ImplicitAnimationTests.swift */,
 				6625876B1FB4DB9C00BC7DF1 /* InitialVelocityTests.swift */,
 				66EF6F291FC48D6A00C83A63 /* InstantAnimationTests.swift */,
 				66FD99F91EE9FBBE00C53A82 /* MotionAnimatorTests.m */,
 				668726491EF04B4C00113675 /* MotionAnimatorTests.swift */,
+				664F59991FCE6661002EC56D /* NonAdditiveAnimatorTests.swift */,
 				664F59951FCDB2E5002EC56D /* QuartzCoreBehavioralTests.swift */,
 				660636011FACC24300C3DFB8 /* TimeScaleFactorTests.swift */,
 				664F59931FCCE27E002EC56D /* UIKitBehavioralTests.swift */,
@@ -505,11 +511,13 @@
 			files = (
 				6625876C1FB4DB9C00BC7DF1 /* InitialVelocityTests.swift in Sources */,
 				66EF6F281FC33C4800C83A63 /* HeadlessLayerImplicitAnimationTests.swift in Sources */,
+				664F599A1FCE6661002EC56D /* NonAdditiveAnimatorTests.swift in Sources */,
 				664F59941FCCE27E002EC56D /* UIKitBehavioralTests.swift in Sources */,
 				66EF6F2A1FC48D6A00C83A63 /* InstantAnimationTests.swift in Sources */,
 				660636021FACC24300C3DFB8 /* TimeScaleFactorTests.swift in Sources */,
 				66BF5A8F1FB0E4CB00E864F6 /* ImplicitAnimationTests.swift in Sources */,
 				664F59961FCDB2E6002EC56D /* QuartzCoreBehavioralTests.swift in Sources */,
+				664F599C1FCE67DB002EC56D /* AdditiveAnimatorTests.swift in Sources */,
 				66A6A6681FBA158000DE54CB /* AnimationRemovalTests.swift in Sources */,
 				6687264A1EF04B4C00113675 /* MotionAnimatorTests.swift in Sources */,
 				66FD99FA1EE9FBBE00C53A82 /* MotionAnimatorTests.m in Sources */,
diff --git a/src/private/CABasicAnimation+MotionAnimator.m b/src/private/CABasicAnimation+MotionAnimator.m
@@ -170,9 +170,12 @@ void MDMConfigureAnimation(CABasicAnimation *animation,
     CGSize from = [animation.fromValue CGSizeValue];
     CGSize to = [animation.toValue CGSizeValue];
     CGSize additiveDisplacement = CGSizeMake(from.width - to.width, from.height - to.height);
-    animation.fromValue = [NSValue valueWithCGSize:additiveDisplacement];
-    animation.toValue = [NSValue valueWithCGSize:CGSizeZero];
-    animation.additive = true;
+
+    if (wantsAdditive) {
+      animation.fromValue = [NSValue valueWithCGSize:additiveDisplacement];
+      animation.toValue = [NSValue valueWithCGSize:CGSizeZero];
+      animation.additive = true;
+    }
 
 #pragma clang diagnostic push
     // CASpringAnimation is a private API on iOS 8 - we're able to make use of it because we're
@@ -199,9 +202,12 @@ void MDMConfigureAnimation(CABasicAnimation *animation,
     CGPoint from = [animation.fromValue CGPointValue];
     CGPoint to = [animation.toValue CGPointValue];
     CGPoint additiveDisplacement = CGPointMake(from.x - to.x, from.y - to.y);
-    animation.fromValue = [NSValue valueWithCGPoint:additiveDisplacement];
-    animation.toValue = [NSValue valueWithCGPoint:CGPointZero];
-    animation.additive = true;
+
+    if (wantsAdditive) {
+      animation.fromValue = [NSValue valueWithCGPoint:additiveDisplacement];
+      animation.toValue = [NSValue valueWithCGPoint:CGPointZero];
+      animation.additive = true;
+    }
 
 #pragma clang diagnostic push
     // CASpringAnimation is a private API on iOS 8 - we're able to make use of it because we're
diff --git a/tests/unit/AdditiveAnimatorTests.swift b/tests/unit/AdditiveAnimatorTests.swift
@@ -0,0 +1,118 @@
+/*
+ Copyright 2017-present The Material Motion Authors. All Rights Reserved.
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+import XCTest
+#if IS_BAZEL_BUILD
+import _MotionAnimator
+#else
+import MotionAnimator
+#endif
+
+class AdditiveAnimationTests: XCTestCase {
+  var animator: MotionAnimator!
+  var timing: MotionTiming!
+  var view: UIView!
+
+  override func setUp() {
+    super.setUp()
+
+    animator = MotionAnimator()
+
+    animator.additive = true
+
+    timing = MotionTiming(delay: 0,
+                          duration: 1,
+                          curve: MotionCurveMakeBezier(p1x: 0, p1y: 0, p2x: 1, p2y: 1),
+                          repetition: .init(type: .none, amount: 0, autoreverses: false))
+
+    let window = UIWindow()
+    window.makeKeyAndVisible()
+    view = UIView() // Need to animate a view's layer to get implicit animations.
+    window.addSubview(view)
+
+    // Connect our layers to the render server.
+    CATransaction.flush()
+  }
+
+  override func tearDown() {
+    animator = nil
+    timing = nil
+    view = nil
+
+    super.tearDown()
+  }
+
+  func testNumericKeyPathsAnimateAdditively() {
+    animator.animate(with: timing, to: view.layer, withValues: [1, 0], keyPath: .cornerRadius)
+
+    XCTAssertNotNil(view.layer.animationKeys(),
+                    "Expected an animation to be added, but none were found.")
+    guard let animationKeys = view.layer.animationKeys() else {
+      return
+    }
+    XCTAssertEqual(animationKeys.count, 1,
+                   "Expected only one animation to be added, but the following were found: "
+                    + "\(animationKeys).")
+    guard let key = animationKeys.first,
+      let animation = view.layer.animation(forKey: key) as? CABasicAnimation else {
+        return
+    }
+
+    XCTAssertTrue(animation.isAdditive, "Animation is not additive when it should be.")
+  }
+
+  func testCGSizeKeyPathsAnimateAdditively() {
+    animator.animate(with: timing, to: view.layer,
+                     withValues: [CGSize(width: 0, height: 0),
+                                  CGSize(width: 1, height: 2)], keyPath: .shadowOffset)
+
+    XCTAssertNotNil(view.layer.animationKeys(),
+                    "Expected an animation to be added, but none were found.")
+    guard let animationKeys = view.layer.animationKeys() else {
+      return
+    }
+    XCTAssertEqual(animationKeys.count, 1,
+                   "Expected only one animation to be added, but the following were found: "
+                    + "\(animationKeys).")
+    guard let key = animationKeys.first,
+      let animation = view.layer.animation(forKey: key) as? CABasicAnimation else {
+        return
+    }
+
+    XCTAssertTrue(animation.isAdditive, "Animation is not additive when it should be.")
+  }
+
+  func testCGPointKeyPathsAnimateAdditively() {
+    animator.animate(with: timing, to: view.layer,
+                     withValues: [CGPoint(x: 0, y: 0),
+                                  CGPoint(x: 1, y: 2)], keyPath: .position)
+
+    XCTAssertNotNil(view.layer.animationKeys(),
+                    "Expected an animation to be added, but none were found.")
+    guard let animationKeys = view.layer.animationKeys() else {
+      return
+    }
+    XCTAssertEqual(animationKeys.count, 1,
+                   "Expected only one animation to be added, but the following were found: "
+                    + "\(animationKeys).")
+    guard let key = animationKeys.first,
+      let animation = view.layer.animation(forKey: key) as? CABasicAnimation else {
+        return
+    }
+
+    XCTAssertTrue(animation.isAdditive, "Animation is not additive when it should be.")
+  }
+}
diff --git a/tests/unit/NonAdditiveAnimatorTests.swift b/tests/unit/NonAdditiveAnimatorTests.swift
@@ -0,0 +1,139 @@
+/*
+ Copyright 2017-present The Material Motion Authors. All Rights Reserved.
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+import XCTest
+#if IS_BAZEL_BUILD
+import _MotionAnimator
+#else
+import MotionAnimator
+#endif
+
+class NonAdditiveAnimationTests: XCTestCase {
+  var animator: MotionAnimator!
+  var timing: MotionTiming!
+  var view: UIView!
+
+  override func setUp() {
+    super.setUp()
+
+    animator = MotionAnimator()
+
+    animator.additive = false
+
+    timing = MotionTiming(delay: 0,
+                          duration: 1,
+                          curve: MotionCurveMakeBezier(p1x: 0, p1y: 0, p2x: 0, p2y: 0),
+                          repetition: .init(type: .none, amount: 0, autoreverses: false))
+
+    let window = UIWindow()
+    window.makeKeyAndVisible()
+    view = UIView() // Need to animate a view's layer to get implicit animations.
+    window.addSubview(view)
+
+    // Connect our layers to the render server.
+    CATransaction.flush()
+  }
+
+  override func tearDown() {
+    animator = nil
+    timing = nil
+    view = nil
+
+    super.tearDown()
+  }
+
+  func testNumericKeyPathsDontAnimateAdditively() {
+    animator.animate(with: timing, to: view.layer, withValues: [1, 0], keyPath: .cornerRadius)
+
+    XCTAssertNotNil(view.layer.animationKeys(),
+                    "Expected an animation to be added, but none were found.")
+    guard let animationKeys = view.layer.animationKeys() else {
+      return
+    }
+    XCTAssertEqual(animationKeys.count, 1,
+                   "Expected only one animation to be added, but the following were found: "
+                    + "\(animationKeys).")
+    guard let key = animationKeys.first,
+      let animation = view.layer.animation(forKey: key) as? CABasicAnimation else {
+        return
+    }
+
+    XCTAssertFalse(animation.isAdditive, "Animation is additive when it shouldn't be.")
+  }
+
+  func testSizeKeyPathsDontAnimateAdditively() {
+    animator.animate(with: timing, to: view.layer,
+                     withValues: [CGSize(width: 0, height: 0),
+                                  CGSize(width: 1, height: 2)], keyPath: .shadowOffset)
+
+    XCTAssertNotNil(view.layer.animationKeys(),
+                    "Expected an animation to be added, but none were found.")
+    guard let animationKeys = view.layer.animationKeys() else {
+      return
+    }
+    XCTAssertEqual(animationKeys.count, 1,
+                   "Expected only one animation to be added, but the following were found: "
+                    + "\(animationKeys).")
+    guard let key = animationKeys.first,
+      let animation = view.layer.animation(forKey: key) as? CABasicAnimation else {
+        return
+    }
+
+    XCTAssertFalse(animation.isAdditive, "Animation is additive when it shouldn't be.")
+  }
+
+  func testPositionKeyPathsDontAnimateAdditively() {
+    animator.animate(with: timing, to: view.layer,
+                     withValues: [CGPoint(x: 0, y: 0),
+                                  CGPoint(x: 1, y: 2)], keyPath: .position)
+
+    XCTAssertNotNil(view.layer.animationKeys(),
+                    "Expected an animation to be added, but none were found.")
+    guard let animationKeys = view.layer.animationKeys() else {
+      return
+    }
+    XCTAssertEqual(animationKeys.count, 1,
+                   "Expected only one animation to be added, but the following were found: "
+                    + "\(animationKeys).")
+    guard let key = animationKeys.first,
+      let animation = view.layer.animation(forKey: key) as? CABasicAnimation else {
+        return
+    }
+
+    XCTAssertFalse(animation.isAdditive, "Animation is additive when it shouldn't be.")
+  }
+
+  func testRectKeyPathsDontAnimateAdditively() {
+    animator.animate(with: timing, to: view.layer,
+                     withValues: [CGRect(x: 0, y: 0, width: 0, height: 0),
+                                  CGRect(x: 0, y: 0, width: 100, height: 50)], keyPath: .bounds)
+
+    XCTAssertNotNil(view.layer.animationKeys(),
+                    "Expected an animation to be added, but none were found.")
+    guard let animationKeys = view.layer.animationKeys() else {
+      return
+    }
+    XCTAssertEqual(animationKeys.count, 1,
+                   "Expected only one animation to be added, but the following were found: "
+                    + "\(animationKeys).")
+    guard let key = animationKeys.first,
+      let animation = view.layer.animation(forKey: key) as? CABasicAnimation else {
+        return
+    }
+
+    XCTAssertFalse(animation.isAdditive, "Animation is additive when it shouldn't be.")
+  }
+}
