[camera] Fix immediate camera animation on API level 23 or below - MA…

…PSAND-614 (#1842)

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
 ## Features ✨ and improvements 🏁

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
@@ -10,13 +9,11 @@ import com.mapbox.maps.plugin.locationcomponent.LocationConsumer
 import com.mapbox.maps.plugin.locationcomponent.LocationProvider
 import com.mapbox.maps.plugin.locationcomponent.location
 import com.mapbox.maps.plugin.viewport.ViewportPlugin
-import com.mapbox.maps.plugin.viewport.state.FollowPuckViewportState
 import com.mapbox.maps.plugin.viewport.transition.ViewportTransition
 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
@@ -33,7 +30,6 @@ class ViewportPluginTest : BaseMapTest() {
 
   private val handler = Handler(Looper.getMainLooper())
   private lateinit var viewportPlugin: ViewportPlugin
-  private lateinit var followPuckViewportState: FollowPuckViewportState
   private lateinit var immediateViewportTransition: ViewportTransition
 
   private val locationProvider = object : LocationProvider {
@@ -53,7 +49,6 @@ class ViewportPluginTest : BaseMapTest() {
     rule.scenario.onActivity { activity ->
       activity.runOnUiThread {
         viewportPlugin = mapView.viewport
-        followPuckViewportState = viewportPlugin.makeFollowPuckViewportState()
         immediateViewportTransition = viewportPlugin.makeImmediateViewportTransition()
         mapView.location.apply {
           enabled = true
@@ -70,52 +65,144 @@ 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(viewportPlugin.makeFollowPuckViewportState()) {
+        latch.countDown()
+      }
     }
 
-    val latch = CountDownLatch(1)
-    latch.await(4000, TimeUnit.MILLISECONDS)
+    // Wait for 5 seconds since the default transition time is 3.5 seconds
+    if (!latch.await(5, TimeUnit.SECONDS)) {
+      throw TimeoutException()
+    }
     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(viewportPlugin.makeFollowPuckViewportState(), immediateViewportTransition) {
+        latch.countDown()
+      }
     }
-    val latch = CountDownLatch(1)
-    latch.await(200, TimeUnit.MILLISECONDS)
+
+    if (!latch.await(200, TimeUnit.MILLISECONDS)) {
+      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 {
+      assertEquals(0.0, mapView.getMapboxMap().cameraState.bearing, EPS)
+      mapView.getMapboxMap().cameraState.center.assertEquals(NULL_ISLAND)
+      // 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(viewportPlugin.makeFollowPuckViewportState(), immediateViewportTransition) {
+        latch.countDown()
+      }
+    }
+
+    if (!latch.await(200, TimeUnit.MILLISECONDS)) {
+      throw TimeoutException()
+    }
+
+    val latch2 = CountDownLatch(1)
+    handler.post {
+      // 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
+    latch2.await(2, TimeUnit.SECONDS)
+
+    // validate the camera is at the moved location
+    handler.post {
+      val cameraState = mapView.getMapboxMap().cameraState
+      cameraState.center.assertEquals(TEST_POINT_MOVED)
+      assertEquals(TEST_BEARING + 90.0, cameraState.bearing, EPS)
     }
   }
 
   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())
   }