Prevent animated values from leaving expected bounds

blend-library/src/main/java/com/wealthfront/blend/Blend.kt

@@ -83,7 +83,7 @@ open class Blend {
    */
   open fun stopPulsing(vararg views: View) {
     views
-        .flatMap { view -> AdditiveViewProperties.ALPHA.getAnimationData(view).runningAnimations }
+        .flatMap { view -> AdditiveViewProperties.ALPHA.getAnimationData(view).committedAnimations }
         .mapNotNull { singlePropertyAnimation -> singlePropertyAnimation.animator }
         .filter { animator -> animator.repeatCount != 0 }
         .toSet()

blend-library/src/main/java/com/wealthfront/blend/animator/BlendableAnimator.kt

@@ -104,11 +104,20 @@ open class BlendableAnimator : Animator() {
   open fun commitFutureValuesIfNotCommitted() {
     if (!valuesCommitted) {
       beforeStartActions.forEach { it() }
-      animations.forEach { it.setUpOnAnimationStart(this) }
+      animations.forEach { it.setUpOnAnimationCommitted(this) }
     }
     valuesCommitted = true
   }
 
+  /**
+   * Mark all [animations] as part of a fully-committed set of animations.
+   *
+   * This step is necessary for individual animations in a set to not remove each other when being committing.
+   */
+  open fun markAnimationsAsFullyCommitted() {
+    animations.forEach { it.isPartOfAFullyCommittedSet = true }
+  }
+
   override fun start() {
     if (isRunning) {
       throw IllegalStateException("Cannot start an already-running animation")
@@ -124,12 +133,12 @@ open class BlendableAnimator : Animator() {
       }
     })
 
-    commitFutureValuesIfNotCommitted()
     if (repeatCount > 0) {
       cancelAnimatorOnViewsDetached()
     }
+    commitFutureValuesIfNotCommitted()
     innerAnimator.start()
-    super.start()
+    markAnimationsAsFullyCommitted()
   }
 
   private fun cancelAnimatorOnViewsDetached() {

blend-library/src/main/java/com/wealthfront/blend/animator/SinglePropertyAnimation.kt

@@ -1,5 +1,6 @@
 package com.wealthfront.blend.animator
 
+import androidx.annotation.VisibleForTesting
 import com.wealthfront.blend.properties.AdditiveProperty
 
 /**
@@ -29,17 +30,26 @@ class SinglePropertyAnimation<Subject>(
    * The animator running this animation
    */
   var animator: BlendableAnimator? = null
-    private set
+    @VisibleForTesting internal set
+  /**
+   * Whether this animation has started running. Note that it can be committed (i.e. queued) and not started.
+   */
+  val isStarted: Boolean get() = animator?.isStarted ?: false
+  /**
+   * Whether the set that this animation belongs to is done committing all of its animations. Useful for figuring out
+   * when to cancel unstarted (but committed) animations.
+   */
+  var isPartOfAFullyCommittedSet: Boolean = false
 
   /**
    * Set up the starting values for this animation and commit the [targetValue] as [property]'s future value.
    */
-  fun setUpOnAnimationStart(animator: BlendableAnimator) {
+  fun setUpOnAnimationCommitted(animator: BlendableAnimator) {
     this.animator = animator
-    property.setUpOnAnimationStart(subject)
+    property.setUpOnAnimationCommitted(subject)
     startValue = property.getFutureValue(subject)
     previousValue = startValue
-    property.addRunningAnimation(subject, this)
+    property.addCommittedAnimation(subject, this)
     property.addInterruptableEndActions(subject, *interruptibleEndActions.toTypedArray())
   }
 
@@ -52,12 +62,17 @@ class SinglePropertyAnimation<Subject>(
     previousValue = newValue
   }
 
+  fun markCancelled() {
+    startValue = targetValue
+    previousValue = targetValue
+  }
+
   /**
    * Run the end actions added by [BlendableAnimator.doOnFinishedUnlessLastAnimationInterrupted], if we haven't been
    * interrupted by another animation on [subject].[property].
    */
   fun runEndActions() {
     property.runEndActions(subject, this)
-    property.removeRunningAnimator(subject, this)
+    property.removeCommittedAnimation(subject, this)
   }
 }

blend-library/src/main/java/com/wealthfront/blend/dsl/BuilderTraits.kt

@@ -35,9 +35,9 @@ interface AnimationStarter {
     return animatorSet.asAnimator().apply {
       addListener(object : AnimatorListenerAdapter() {
         override fun onAnimationStart(animation: Animator?) {
-          animatorSet.childAnimators
-            .mapNotNull { it as? BlendableAnimator }
-            .forEach { it.commitFutureValuesIfNotCommitted() }
+          val animators = animatorSet.childAnimators.mapNotNull { it as? BlendableAnimator }
+          animators.forEach { it.commitFutureValuesIfNotCommitted() }
+          animators.forEach { it.markAnimationsAsFullyCommitted() }
         }
       })
     }

blend-library/src/main/java/com/wealthfront/blend/properties/AdditiveProperty.kt

@@ -10,8 +10,8 @@ import kotlin.math.roundToInt
  * To properly blend animations, it includes methods like [getFutureValue], [addInterruptableEndActions], and
  * [getAnimationData].
  *
- * We store data on running animations ([AnimationData]) on each subject, ideally. This systematically prevents leaks of
- * references to [Subject]s. The mechanism for storing is described by the implementation of [getAnimationData]
+ * We store data on committed animations ([AnimationData]) on each subject, ideally. This systematically prevents leaks
+ * of references to [Subject]s. The mechanism for storing is described by the implementation of [getAnimationData]
  */
 interface AdditiveProperty<in Subject> {
 
@@ -52,19 +52,19 @@ interface AdditiveProperty<in Subject> {
   /**
    * Perform any setup tasks when an animation on this property starts.
    */
-  fun setUpOnAnimationStart(subject: Subject) { }
+  fun setUpOnAnimationCommitted(subject: Subject) { }
 
   /**
-   * Add a running animation to the [subject]'s [AnimationData] for this property.
+   * Add a committed animation to the [subject]'s [AnimationData] for this property.
    */
-  fun addRunningAnimation(subject: Subject, animation: SinglePropertyAnimation<*>) =
-      getAnimationData(subject).addRunningAnimation(animation)
+  fun addCommittedAnimation(subject: Subject, animation: SinglePropertyAnimation<*>) =
+      getAnimationData(subject).addCommittedAnimation(animation)
 
   /**
-   * Remove a running animation to the [subject]'s [AnimationData] for this property.
+   * Remove a committed animation from the [subject]'s [AnimationData] for this property.
    */
-  fun removeRunningAnimator(subject: Subject, animation: SinglePropertyAnimation<*>) =
-    getAnimationData(subject).removeRunningAnimation(animation)
+  fun removeCommittedAnimation(subject: Subject, animation: SinglePropertyAnimation<*>) =
+      getAnimationData(subject).removeCommittedAnimation(animation)
 
   /**
    * Add [endActions] to the [subject]'s [AnimationData] for this property so they can be cancelled if another
@@ -79,7 +79,7 @@ interface AdditiveProperty<in Subject> {
    */
   fun runEndActions(subject: Subject, animation: SinglePropertyAnimation<*>) {
     val animationData = getAnimationData(subject)
-    if (animation == animationData.runningAnimations.lastOrNull()) {
+    if (animation == animationData.committedAnimations.lastOrNull()) {
       animationData.interruptableEndActions.forEach { it() }
       animationData.interruptableEndActions.clear()
     }

blend-library/src/main/java/com/wealthfront/blend/properties/AnimationData.kt

@@ -4,52 +4,62 @@ import com.wealthfront.blend.animator.SinglePropertyAnimation
 import kotlin.math.roundToInt
 
 /**
- * A container for storing data about running animations to facilitate blending.
+ * A container for storing data about committed animations to facilitate blending.
  *
  * Associated with a specific subject and a property.
  */
 class AnimationData {
 
   /**
-   * All animations currently running on the associated subject for the associated property.
+   * All animations currently committed to run on the associated subject for the associated property. These animations
+   * may or may not be currently running, however.
    */
-  val runningAnimations: MutableList<SinglePropertyAnimation<*>> = mutableListOf()
+  val committedAnimations: MutableList<SinglePropertyAnimation<*>> = mutableListOf()
   /**
    * End actions that will be cancelled if another animation is started for the associated subject.property.
    */
   val interruptableEndActions: MutableList<() -> Unit> = mutableListOf()
   var isLatestAnimationDone = false
 
   /**
-   * Get the value of this property as if all running animations have finished.
+   * Get the value of this property as if all committed animations have finished.
    */
   val futureValue: Float?
-    get() = runningAnimations.lastOrNull()?.targetValue
+    get() = committedAnimations.lastOrNull { it.isStarted }?.targetValue
 
   /**
-   * Register a new animation that has started.
+   * Register a new animation that has started. This cancels any [interruptableEndActions] that we have, since a new
+   * animation has started.
    *
-   * This also cancels any [interruptableEndActions] that we have, since a new animation has started.
+   * This also cancels all non-started [committedAnimations], in order to avoid animating to any unexpected values.
+   * See [this github issue](https://github.com/wealthfront/blend/issues/4) for more information.
    */
-  fun addRunningAnimation(animation: SinglePropertyAnimation<*>) {
+  fun addCommittedAnimation(animation: SinglePropertyAnimation<*>) {
     if (isLatestAnimationDone) {
-      runningAnimations -= runningAnimations.last()
+      committedAnimations -= committedAnimations.last()
     }
     isLatestAnimationDone = false
-    runningAnimations += animation
+    committedAnimations.filter {
+      !it.isStarted && it.isPartOfAFullyCommittedSet
+    }.forEach {
+      it.markCancelled()
+      committedAnimations.remove(it)
+    }
+    committedAnimations += animation
     interruptableEndActions.clear()
   }
 
   /**
-   * Remove a running animation because it has either finished or been cancelled.
+   * Remove a committed animation because it has either finished or been cancelled.
    */
-  fun removeRunningAnimation(animation: SinglePropertyAnimation<*>) {
-    if (runningAnimations.lastOrNull() == animation && runningAnimations.size > 1) {
+  fun removeCommittedAnimation(animation: SinglePropertyAnimation<*>) {
+    animation.markCancelled()
+    if (committedAnimations.lastOrNull() == animation && committedAnimations.size > 1) {
       isLatestAnimationDone = true
     } else {
-      runningAnimations -= animation
-      if (isLatestAnimationDone && runningAnimations.size == 1) {
-        runningAnimations.clear()
+      committedAnimations -= animation
+      if (isLatestAnimationDone && committedAnimations.size == 1) {
+        committedAnimations.clear()
         isLatestAnimationDone = false
       }
     }

blend-library/src/main/java/com/wealthfront/blend/properties/DimensionViewProperties.kt

@@ -35,7 +35,7 @@ object DimensionViewProperties {
 
     override fun getAnimationData(subject: View): AnimationData = subject.getAnimationData(id)
 
-    override fun setUpOnAnimationStart(subject: View) {
+    override fun setUpOnAnimationCommitted(subject: View) {
       if (subject.visibility == GONE) {
         setValue(subject, 0f)
         subject.visibility = VISIBLE
@@ -67,7 +67,7 @@ object DimensionViewProperties {
 
     override fun getAnimationData(subject: View): AnimationData = subject.getAnimationData(id)
 
-    override fun setUpOnAnimationStart(subject: View) {
+    override fun setUpOnAnimationCommitted(subject: View) {
       if (subject.visibility == GONE) {
         setValue(subject, 0f)
         subject.visibility = VISIBLE

blend-library/src/main/java/com/wealthfront/blend/properties/ViewPropertySupport.kt

@@ -44,7 +44,7 @@ fun <Subject : Any> makeExternalAdditiveViewProperty(
           interpolateAction(startValue, endValue, timeFraction, subject)
         }
 
-    override fun setUpOnAnimationStart(subject: Subject) {
+    override fun setUpOnAnimationCommitted(subject: Subject) {
       setUpAction?.invoke(subject)
     }
   }
@@ -101,7 +101,7 @@ internal fun <Subject : View> makeAdditiveViewProperty(
         interpolateAction(startValue, endValue, timeFraction, subject)
       }
 
-    override fun setUpOnAnimationStart(subject: Subject) {
+    override fun setUpOnAnimationCommitted(subject: Subject) {
       setUpAction?.invoke(subject)
     }
   }
@@ -159,7 +159,7 @@ internal fun wrapViewProperty(
 
     override fun getAnimationData(subject: View): AnimationData = subject.getAnimationData(id)
 
-    override fun setUpOnAnimationStart(subject: View) {
+    override fun setUpOnAnimationCommitted(subject: View) {
       setUpAction?.invoke(subject)
     }
   }

blend-test/src/test/java/com/wealthfront/blend/BlendDslTest.kt

@@ -332,7 +332,7 @@ class BlendDslTest {
       return animationDatas[subject] ?: AnimationData().also { animationDatas[subject] = it }
     }
 
-    override fun setUpOnAnimationStart(subject: TestObject) {
+    override fun setUpOnAnimationCommitted(subject: TestObject) {
       addInterruptableEndActions(subject, {
         animationDatas.remove(subject)
       })

blend-test/src/test/java/com/wealthfront/blend/properties/AnimationDataTest.kt

@@ -2,7 +2,9 @@ package com.wealthfront.blend.properties
 
 import android.view.View
 import com.google.common.truth.Truth.assertThat
+import com.wealthfront.blend.animator.BlendableAnimator
 import com.wealthfront.blend.animator.SinglePropertyAnimation
+import com.wealthfront.whenever
 import org.junit.Before
 import org.junit.Test
 import org.mockito.Mock
@@ -13,11 +15,15 @@ class AnimationDataTest {
   lateinit var animationData: AnimationData
   @Mock lateinit var view: View
   @Mock lateinit var property: AdditiveProperty<View>
+  @Mock lateinit var startedAnimator: BlendableAnimator
+  @Mock lateinit var unstartedAnimator: BlendableAnimator
 
   @Before
   fun setUp() {
     initMocks(this)
     animationData = AnimationData()
+
+    whenever(startedAnimator.isStarted).thenReturn(true)
   }
 
   @Test
@@ -29,29 +35,57 @@ class AnimationDataTest {
   fun getFutureValue_futureValue() {
     val firstAnimation = SinglePropertyAnimation(view, property, 10f)
     val latestAnimation = SinglePropertyAnimation(view, property, 20f)
-    animationData.addRunningAnimation(firstAnimation)
-    animationData.addRunningAnimation(latestAnimation)
+    animationData.addCommittedAnimation(firstAnimation)
+    animationData.addCommittedAnimation(latestAnimation)
     assertThat(animationData.futureValue).isWithin(0.01f).of(20f)
   }
 
   @Test
   fun getFutureValue_futureValue_animationEndedEarly() {
     val firstAnimation = SinglePropertyAnimation(view, property, 10f)
     val latestAnimation = SinglePropertyAnimation(view, property, 20f)
-    animationData.addRunningAnimation(firstAnimation)
-    animationData.addRunningAnimation(latestAnimation)
-    animationData.removeRunningAnimation(latestAnimation)
+    animationData.addCommittedAnimation(firstAnimation)
+    animationData.addCommittedAnimation(latestAnimation)
+    animationData.removeCommittedAnimation(latestAnimation)
     assertThat(animationData.futureValue).isWithin(0.01f).of(20f)
   }
 
   @Test
   fun getFutureValue_futureValue_animationEndedEarly_clear() {
     val firstAnimation = SinglePropertyAnimation(view, property, 10f)
     val latestAnimation = SinglePropertyAnimation(view, property, 20f)
-    animationData.addRunningAnimation(firstAnimation)
-    animationData.addRunningAnimation(latestAnimation)
-    animationData.removeRunningAnimation(latestAnimation)
-    animationData.removeRunningAnimation(firstAnimation)
+    animationData.addCommittedAnimation(firstAnimation)
+    animationData.addCommittedAnimation(latestAnimation)
+    animationData.removeCommittedAnimation(latestAnimation)
+    animationData.removeCommittedAnimation(firstAnimation)
+    assertThat(animationData.futureValue).isNull()
+  }
+
+  @Test
+  fun removeChainedAnimation_newAnimationAdded() {
+    val delayedAnimation = SinglePropertyAnimation(view, property, 10f)
+      .apply {
+        animator = unstartedAnimator
+        isPartOfAFullyCommittedSet = true
+      }
+    val newAnimation = SinglePropertyAnimation(view, property, 20f)
+    animationData.addCommittedAnimation(delayedAnimation)
+    animationData.addCommittedAnimation(newAnimation)
+    animationData.removeCommittedAnimation(newAnimation)
     assertThat(animationData.futureValue).isNull()
   }
+
+  @Test
+  fun doNotRemoveChainedAnimation_newAnimationAdded_chainedAnimationIsPartOfUnfinishedSet() {
+    val delayedAnimation = SinglePropertyAnimation(view, property, 10f)
+        .apply {
+          animator = unstartedAnimator
+          isPartOfAFullyCommittedSet = false
+        }
+    val newAnimation = SinglePropertyAnimation(view, property, 20f)
+    animationData.addCommittedAnimation(delayedAnimation)
+    animationData.addCommittedAnimation(newAnimation)
+    animationData.removeCommittedAnimation(newAnimation)
+    assertThat(animationData.futureValue).isNotNull()
+  }
 }