[camera] Fix immediate camera animation on API level 23 or below - MAPSAND-614

CHANGELOG.md

@@ -3,6 +3,7 @@
 Mapbox welcomes participation and contributions from everyone.
 
 # main
+* Fix immediate camera animation on API level 23 or below. ([1842](https://github.com/mapbox/mapbox-maps-android/pull/1842))
 
 # 10.10.0-rc.1
 ## Bug fixes 🐞

app/src/androidTest/java/com/mapbox/maps/testapp/viewport/ViewportPluginTest.kt

@@ -1,6 +1,5 @@
 package com.mapbox.maps.testapp.viewport
 
-import android.os.Build
 import android.os.Handler
 import android.os.Looper
 import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -16,7 +15,6 @@ import com.mapbox.maps.plugin.viewport.viewport
 import com.mapbox.maps.testapp.BaseMapTest
 import org.junit.After
 import org.junit.Assert.assertEquals
-import org.junit.Assume.assumeTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -70,52 +68,135 @@ class ViewportPluginTest : BaseMapTest() {
 
   @Test
   fun transitionToDefaultTransition() {
-    assumeTrue(
-      "Can only run on API Level 24 or newer because of difference of animator behaviour.",
-      Build.VERSION.SDK_INT > 23
-    )
+    val latch = CountDownLatch(1)
     handler.post {
-      viewportPlugin.transitionTo(followPuckViewportState)
       assertEquals(0.0, mapView.getMapboxMap().cameraState.bearing, EPS)
       mapView.getMapboxMap().cameraState.center.assertEquals(NULL_ISLAND)
-      locationProvider.locationConsumers.forEach { it.onLocationUpdated(TEST_POINT) }
-      locationProvider.locationConsumers.forEach { it.onBearingUpdated(TEST_BEARING) }
+      // immediate update location puck to test location.
+      locationProvider.locationConsumers.forEach {
+        it.onLocationUpdated(
+          TEST_POINT,
+          options = { duration = 0 }
+        )
+      }
+      locationProvider.locationConsumers.forEach {
+        it.onBearingUpdated(
+          TEST_BEARING,
+          options = { duration = 0 }
+        )
+      }
+      // transition to the followPuckViewportState with default transition
+      viewportPlugin.transitionTo(followPuckViewportState) {
+        latch.countDown()
+      }
     }
 
-    val latch = CountDownLatch(1)
-    latch.await(4000, TimeUnit.MILLISECONDS)
+    // Wait for 5 seconds since the default transition time is 3.5 seconds
+    latch.await(5000, TimeUnit.SECONDS)
     handler.post {
-      mapView.getMapboxMap().cameraState.center.assertEquals(TEST_POINT)
-      assertEquals(TEST_BEARING, mapView.getMapboxMap().cameraState.bearing, EPS)
+      val cameraState = mapView.getMapboxMap().cameraState
+      cameraState.center.assertEquals(TEST_POINT)
+      assertEquals(TEST_BEARING, cameraState.bearing, EPS)
     }
   }
 
   @Test
   fun transitionToImmediateTransition() {
+    val latch = CountDownLatch(1)
     handler.post {
-      viewportPlugin.transitionTo(followPuckViewportState, immediateViewportTransition)
       assertEquals(0.0, mapView.getMapboxMap().cameraState.bearing, EPS)
       mapView.getMapboxMap().cameraState.center.assertEquals(NULL_ISLAND)
-      locationProvider.locationConsumers.forEach { it.onLocationUpdated(TEST_POINT) }
-      locationProvider.locationConsumers.forEach { it.onBearingUpdated(TEST_BEARING) }
+      // immediate update location puck to test location.
+      locationProvider.locationConsumers.forEach {
+        it.onLocationUpdated(
+          TEST_POINT,
+          options = { duration = 0 }
+        )
+      }
+      locationProvider.locationConsumers.forEach {
+        it.onBearingUpdated(
+          TEST_BEARING,
+          options = { duration = 0 }
+        )
+      }
+      // immediately transition to the followPuckViewportState
+      viewportPlugin.transitionTo(followPuckViewportState, immediateViewportTransition) {
+        latch.countDown()
+      }
     }
-    val latch = CountDownLatch(1)
-    latch.await(200, TimeUnit.MILLISECONDS)
+
+    if (!latch.await(2, TimeUnit.SECONDS)) {
+      throw TimeoutException()
+    }
+
     handler.post {
       val cameraState = mapView.getMapboxMap().cameraState
       cameraState.center.assertEquals(TEST_POINT)
+      assertEquals(TEST_BEARING, cameraState.bearing, EPS)
+    }
+  }
+
+  @Test
+  fun testFollowPuckViewportState() {
+    val latch = CountDownLatch(1)
+    handler.post {
+      // immediate update location puck to test location.
+      locationProvider.locationConsumers.forEach {
+        it.onLocationUpdated(
+          TEST_POINT,
+          options = { duration = 0 }
+        )
+      }
+      locationProvider.locationConsumers.forEach {
+        it.onBearingUpdated(
+          TEST_BEARING,
+          options = { duration = 0 }
+        )
+      }
+      // immediately transition to the followPuckViewportState
+      viewportPlugin.transitionTo(followPuckViewportState, immediateViewportTransition) {
+        // now we are in the follow puck viewport state
+        val cameraState = mapView.getMapboxMap().cameraState
+        cameraState.center.assertEquals(TEST_POINT)
+        assertEquals(TEST_BEARING, cameraState.bearing, EPS)
+        // emit new bearing and location updates, location component plugin should be driving the animation.
+        // and viewport plugin should do animation with 0 duration on each animated location puck position
+        locationProvider.locationConsumers.forEach {
+          it.onBearingUpdated(
+            TEST_BEARING + 90.0,
+            options = { duration = 1000 }
+          )
+        }
+        locationProvider.locationConsumers.forEach {
+          it.onLocationUpdated(
+            TEST_POINT_MOVED,
+            options = { duration = 1000 }
+          )
+        }
+      }
+    }
+
+    // The location update will be animated with 1 second duration, we wait for 2 seconds for the animation to finish
+    latch.await(2, TimeUnit.SECONDS)
+
+    // validate the camera is at the moved location
+    handler.post {
+      val cameraState = mapView.getMapboxMap().cameraState
+      assertEquals(TEST_BEARING + 90.0, cameraState.bearing, EPS)
+      cameraState.center.assertEquals(TEST_POINT_MOVED)
     }
   }
 
   private fun Point.assertEquals(other: Point) {
-    assertEquals(this.longitude(), other.longitude(), EPS)
-    assertEquals(this.latitude(), other.latitude(), EPS)
-    assertEquals(this.altitude(), other.altitude(), EPS)
+    assertEquals(other.longitude(), this.longitude(), EPS)
+    assertEquals(other.latitude(), this.latitude(), EPS)
+    assertEquals(other.altitude(), this.altitude(), EPS)
   }
 
   private companion object {
-    val TEST_POINT = Point.fromLngLat(24.9384, 60.1699)
-    val NULL_ISLAND = Point.fromLngLat(0.0, 0.0)
+    val TEST_POINT: Point = Point.fromLngLat(24.9384, 60.1699)
+    val TEST_POINT_MOVED: Point = Point.fromLngLat(24.94284, 60.1699)
+    val NULL_ISLAND: Point = Point.fromLngLat(0.0, 0.0)
     const val EPS = 0.000001
     const val TEST_BEARING = 45.0
   }

plugin-animation/api/plugin-animation.api

@@ -97,6 +97,7 @@ public abstract class com/mapbox/maps/plugin/animation/animator/CameraAnimator :
 	public final fun addListener (Landroid/animation/Animator$AnimatorListener;)V
 	public final fun addUpdateListener (Landroid/animation/ValueAnimator$AnimatorUpdateListener;)V
 	public final fun cancel ()V
+	public fun getAnimatedValue ()Ljava/lang/Object;
 	public final fun getOwner ()Ljava/lang/String;
 	public final fun getStartValue ()Ljava/lang/Object;
 	public final fun getTargets ()[Ljava/lang/Object;

plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/animator/CameraAnimator.kt

@@ -3,11 +3,13 @@ package com.mapbox.maps.plugin.animation.animator
 import android.animation.TypeEvaluator
 import android.animation.ValueAnimator
 import android.annotation.SuppressLint
+import android.os.Build
 import com.mapbox.maps.CameraOptions
 import com.mapbox.maps.logW
 import com.mapbox.maps.plugin.animation.CameraAnimatorOptions
 import com.mapbox.maps.plugin.animation.CameraAnimatorType
 import com.mapbox.maps.threading.AnimationThreadController.postOnAnimatorThread
+import java.util.concurrent.CopyOnWriteArraySet
 
 /**
  * Base generic class for all camera animators.
@@ -42,8 +44,8 @@ abstract class CameraAnimator<out T> (
   private var internalUpdateListener: AnimatorUpdateListener? = null
   private var internalListener: AnimatorListener? = null
 
-  private val userUpdateListeners = mutableListOf<AnimatorUpdateListener?>()
-  private val userListeners = mutableListOf<AnimatorListener?>()
+  private val userUpdateListeners = CopyOnWriteArraySet<AnimatorUpdateListener?>()
+  private val userListeners = CopyOnWriteArraySet<AnimatorListener?>()
 
   internal var canceled = false
   internal var isInternal = false
@@ -83,6 +85,9 @@ abstract class CameraAnimator<out T> (
     postOnAnimatorThread {
       if (registered) {
         canceled = false
+        if (handleImmediateAnimationOnAPI23OrBelow()) {
+          return@postOnAnimatorThread
+        }
         super.start()
       } else {
         logW(
@@ -93,6 +98,59 @@ abstract class CameraAnimator<out T> (
     }
   }
 
+  /**
+   * The most recent value calculated by this <code>ValueAnimator</code> when there is just one
+   * property being animated. This value is only sensible while the animation is running. The main
+   * purpose for this read-only property is to retrieve the value from the <code>ValueAnimator</code>
+   * during a call to {@link AnimatorUpdateListener#onAnimationUpdate(ValueAnimator)}, which
+   * is called during each animation frame, immediately after the value is calculated.
+   *
+   * @return animatedValue The value most recently calculated by this <code>ValueAnimator</code> for
+   * the single property being animated. If there are several properties being animated
+   * (specified by several PropertyValuesHolder objects in the constructor), this function
+   * returns the animated value for the first of those objects.
+   */
+  override fun getAnimatedValue(): Any {
+    // For immediate animations on API <= 23, as we introduced the bypass logic in handleImmediateAnimationOnAPI23OrBelow(),
+    // the returned ValueAnimator.getAnimatedValue() will be null, in this case, we should return the
+    // last configured target value, so we immediately jump to the target value.
+    if (Build.VERSION.SDK_INT <= 23) {
+      if (duration == 0L && startDelay == 0L) {
+        return super.getAnimatedValue() ?: targets.last() as Any
+      }
+    }
+    return super.getAnimatedValue()
+  }
+
+  /**
+   * Handle immediate animation(when duration and startDelay of the animation is 0) on devices running
+   * API 23 or below, by sending updates to animator listeners directly without triggering [ValueAnimator]'s start().
+   *
+   * @return true if the animation is handled, false otherwise
+   */
+  private fun handleImmediateAnimationOnAPI23OrBelow(): Boolean {
+    // Devices with API <= 23 animating with duration = 0 will send initial value from Looper with some small delay as a result.
+    // Hence multiple immediate animations sent close to each other may cancel previous ones.
+    // For these cases, we bypass the ValueAnimator and emit the user registered AnimatorListeners and AnimatorUpdateListeners immediately.
+    if (Build.VERSION.SDK_INT <= 23) {
+      if (duration == 0L && startDelay == 0L) {
+        val tmpListeners = listeners.toList()
+        tmpListeners.forEach {
+          it.onAnimationStart(this)
+        }
+        internalUpdateListener?.onAnimationUpdate(this)
+        userUpdateListeners.forEach {
+          it?.onAnimationUpdate(this)
+        }
+        tmpListeners.forEach {
+          it.onAnimationEnd(this)
+        }
+        return true
+      }
+    }
+    return false
+  }
+
   /**
    * Add an animator listener
    *

plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsListenersTest.kt

@@ -273,7 +273,7 @@ class CameraAnimationsListenersTest {
     bearingAnimator.start()
     Shadows.shadowOf(Looper.getMainLooper()).idle()
 
-    Assert.assertEquals(5, valuesList.size)
+    Assert.assertEquals(3, valuesList.size)
     Assert.assertArrayEquals(intArrayOf(0, 1, 1), valuesList.slice(0..2).toIntArray())
   }
 

sdk/src/androidTest/java/com/mapbox/maps/CameraAnimationsPluginTest.kt

@@ -185,9 +185,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testAnimatorWithAnimatorDelay() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val duration = 2000L
     val delay = 1000L
     val animatorListener = CameraAnimatorListener()
@@ -230,9 +227,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testCancelAnimatorWithDelayed() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val duration = 2000L
     val delay = 500L
     val animatorListener = CameraAnimatorListener()
@@ -276,9 +270,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testAnimatorWithHandlerPostDelay() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val duration = 1000L
     val delay = 2000L
     val animatorListener = CameraAnimatorListener()
@@ -325,9 +316,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testCameraUpdateFrequency() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val duration = 3000L
 
     // Considering min update frequency is not less than 20 ms assuming running on x86 emulator on CI
@@ -358,9 +346,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testRegisterExistedAnimatorTypeAndStart() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val duration = 2000L
 
     val animatorListener1 = CameraAnimatorListener()
@@ -446,9 +431,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testEaseToSingleDurationShort() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val targetBearing = 5.0
     val cameraOptions = CameraOptions.Builder().bearing(targetBearing).build()
     val expectedValues = mutableSetOf(-0.0, targetBearing)
@@ -478,7 +460,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testEaseToSequenceDurationZero() {
-
     val targetBearing1 = 5.0
     val targetBearing2 = 10.0
     val targetBearing3 = 15.0
@@ -632,9 +613,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testEaseToSequenceDurationShort() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val targetBearing1 = 5.0
     val targetBearing2 = 10.0
     val targetBearing3 = 15.0
@@ -685,9 +663,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testEaseToAnimatorSequenceLessThenFrameUpdate() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val targetBearing1 = 5.0
     val targetBearing2 = 10.0
     val targetBearing3 = 15.0
@@ -766,9 +741,6 @@ class CameraAnimationsPluginTest : BaseAnimationMapTest() {
 
   @Test
   fun testPostDelayedAndStartDelayedAnimators() {
-    if (ignoreTestForGivenAbi()) {
-      return
-    }
     val pitchAnimatorOne = createPitchAnimator(cameraAnimationPlugin, 10.0, 0, 1000L)
     val pitchListenerOne = CameraAnimatorListener()
     pitchAnimatorOne.addListener(pitchListenerOne)