diff --git a/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImpl.kt b/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImpl.kt index 76b9bafbf9..0ad24e6556 100644 --- a/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImpl.kt +++ b/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImpl.kt @@ -1,8 +1,11 @@ package com.mapbox.maps.plugin.animation import android.animation.Animator +import android.animation.AnimatorListenerAdapter import android.animation.AnimatorSet import android.animation.ValueAnimator +import android.os.Handler +import android.os.Looper import androidx.annotation.VisibleForTesting import androidx.annotation.VisibleForTesting.PRIVATE import com.mapbox.common.Logger @@ -42,8 +45,6 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { * Consists of set owner + animator set itself. */ private var highLevelAnimatorSet: HighLevelAnimatorSet? = null - @VisibleForTesting(otherwise = PRIVATE) - internal var highLevelListener: Animator.AnimatorListener? = null private val centerListeners = CopyOnWriteArraySet>() private val zoomListeners = CopyOnWriteArraySet>() @@ -55,6 +56,16 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { private val lifecycleListeners = CopyOnWriteArraySet() + private val handler = Handler(Looper.getMainLooper()) + private var commitScheduled = false + private val commitChangesRunnable = Runnable { + performMapJump(cameraOptionsBuilder.anchor(anchor).build()) + + // reset values + cameraOptionsBuilder = CameraOptions.Builder() + commitScheduled = false + } + /** * If debug mode is enabled extra logs will be written about animation lifecycle and * some other events that may be useful for debugging. @@ -163,6 +174,7 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { paddingListeners.clear() lifecycleListeners.clear() animators.clear() + handler.removeCallbacks(commitChangesRunnable) } private fun performMapJump(cameraOptions: CameraOptions) { @@ -238,21 +250,6 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { } } - private fun correctInitialCamera( - currentCameraBuilder: CameraOptions.Builder, - cameraAnimator: CameraAnimator<*> - ): CameraOptions { - return when (cameraAnimator) { - is CameraCenterAnimator -> currentCameraBuilder.center(cameraAnimator.animatedValue as? Point).build() - is CameraZoomAnimator -> currentCameraBuilder.zoom(cameraAnimator.animatedValue as? Double).build() - is CameraAnchorAnimator -> currentCameraBuilder.anchor(cameraAnimator.animatedValue as? ScreenCoordinate).build() - is CameraPaddingAnimator -> currentCameraBuilder.padding(cameraAnimator.animatedValue as? EdgeInsets).build() - is CameraBearingAnimator -> currentCameraBuilder.bearing(cameraAnimator.animatedValue as? Double).build() - is CameraPitchAnimator -> currentCameraBuilder.pitch(cameraAnimator.animatedValue as? Double).build() - else -> throw RuntimeException("Unsupported animator type!") - } - } - private fun registerInternalListener(animator: CameraAnimator<*>) { animator.addInternalListener(object : Animator.AnimatorListener { @@ -266,10 +263,6 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { lifecycleListeners.forEach { it.onAnimatorStarting(type, this, owner) } - if (runningAnimatorsQueue.isEmpty()) { - highLevelAnimatorSet?.started = true - highLevelListener?.onAnimationStart(animation) - } mapTransformDelegate.setUserAnimationInProgress(true) // check if such animation is not running already // if it is - then cancel it @@ -326,14 +319,6 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { unregisterAnimators(this, cancelAnimators = false) } if (runningAnimatorsQueue.isEmpty()) { - cameraOptionsBuilder.build().let { options -> - if (options.hasChanges) { - // cameraOptionsBuilder might have some accumulated, but not applied changes. - performMapJump(options) - cameraOptionsBuilder = CameraOptions.Builder() - } - } - mapTransformDelegate.setUserAnimationInProgress(false) } lifecycleListeners.forEach { @@ -342,17 +327,8 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { AnimationFinishStatus.ENDED -> it.onAnimatorEnding(type, this, owner) } } - // it could happen that some animators are cancelled inside onAnimatorStarting callback - // while high-level animation is starting and highLevelListener != null already, - // so we check on `started` flag to validate that high-level animation truly started - if (runningAnimatorsQueue.isEmpty() && highLevelAnimatorSet?.started == true) { - when (finishStatus) { - AnimationFinishStatus.CANCELED -> { - highLevelListener?.onAnimationCancel(animation) - highLevelListener?.onAnimationEnd(animation) - } - AnimationFinishStatus.ENDED -> highLevelListener?.onAnimationEnd(animation) - } + if (runningAnimatorsQueue.isEmpty()) { + commitChanges() } } ?: throw RuntimeException( "Could not finish animation in CameraManager! " + @@ -364,46 +340,38 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { private fun registerInternalUpdateListener(animator: CameraAnimator<*>) { animator.addInternalUpdateListener { + // add current animator to queue-set if was not present + runningAnimatorsQueue.add(animator) + // set current animator value in any case updateCameraValue(animator) - // main idea here is not to update map on each option change - // we perform jump based on update tick of first (oldest) animation - val firstAnimator = if (runningAnimatorsQueue.iterator().hasNext()) { - runningAnimatorsQueue.iterator().next() - } else { - null - } + if (animator.type == CameraAnimatorType.ANCHOR) { anchor = it.animatedValue as ScreenCoordinate } - val cameraOptions = when { - // if no running animators in queue - get current map camera but apply first updated value - firstAnimator == null -> { - correctInitialCamera( - mapCameraManagerDelegate.cameraState.toCameraOptions(anchor).toBuilder(), - animator - ) - } - // if update is triggered for first (oldest) animator - build options and jump - it == firstAnimator -> { - cameraOptionsBuilder.anchor(anchor).build() - } - // do not perform jump if update is triggered for not first (oldest) animator - else -> { - null + + if (animator.hasUserListeners) { + // If the animator have third-party listeners camera changes must be applied immediately + // to be seen from the listeners. + commitChanges() + } else { + // main idea here is not to update map on each option change. + // the runnable posted here will be executed right after all the animators are applied. + if (!commitScheduled) { + handler.postAtFrontOfQueue(commitChangesRunnable) + commitScheduled = true } } - cameraOptions?.let { camera -> - // move map camera - performMapJump(camera) - // reset values - cameraOptionsBuilder = CameraOptions.Builder() - } - // add current animator to queue-set if was not present - runningAnimatorsQueue.add(it) } } + private fun commitChanges() { + if (commitScheduled) { + handler.removeCallbacks(commitChangesRunnable) + } + commitChangesRunnable.run() + } + private fun cancelAnimatorSet() { highLevelAnimatorSet?.let { it.animatorSet.cancel() @@ -857,16 +825,16 @@ class CameraAnimationsPluginImpl : CameraAnimationsPlugin { interpolator = it } animationOptions?.animatorListener?.let { - // duration == 0L does not behave the same as > 0 and actually - // our custom `highLevelListener` designed for proper notifying of lifecycle - // based on `runningAnimatorsQueue` is not working because animators end immediately. - // We use standard listener in that corner case. - highLevelListener = if (animationOptions.duration == 0L) { - addListener(it) - null - } else { - it - } + addListener(object : AnimatorListenerAdapter() { + override fun onAnimationEnd(animation: Animator?) { + commitChanges() + + if (highLevelAnimatorSet?.animatorSet === animation) { + highLevelAnimatorSet = null + } + } + }) + addListener(it) } playTogether(*animators) } diff --git a/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/animator/CameraAnimator.kt b/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/animator/CameraAnimator.kt index e29689d04e..fd17374e57 100644 --- a/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/animator/CameraAnimator.kt +++ b/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/animator/CameraAnimator.kt @@ -174,6 +174,12 @@ abstract class CameraAnimator ( super.cancel() } + /** + * true if CameraAnimator have any external listeners registered. + */ + internal val hasUserListeners: Boolean + get() = userUpdateListeners.isNotEmpty() + internal fun addInternalUpdateListener(listener: AnimatorUpdateListener) { super.removeAllUpdateListeners() internalUpdateListener = listener diff --git a/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsChangeListenersTest.kt b/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsChangeListenersTest.kt index 6197b1e36d..29d777fc1f 100644 --- a/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsChangeListenersTest.kt +++ b/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsChangeListenersTest.kt @@ -6,8 +6,11 @@ import com.mapbox.maps.ScreenCoordinate import com.mapbox.maps.plugin.animation.CameraAnimationsPluginImplTest.Companion.cameraState import org.junit.Assert import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner import kotlin.math.pow +@RunWith(RobolectricTestRunner::class) class CameraAnimationsChangeListenersTest { @Test diff --git a/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImplTest.kt b/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImplTest.kt index 227552c58f..c4e539fa56 100644 --- a/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImplTest.kt +++ b/plugin-animation/src/test/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImplTest.kt @@ -140,6 +140,9 @@ class CameraAnimationsPluginImplTest { playTogether(*animators) start() } + + shadowOf(getMainLooper()).idle() + verify { mapCameraManagerDelegate.setCamera(any()) } } @@ -827,26 +830,6 @@ class CameraAnimationsPluginImplTest { assertEquals(true, listenerTwo.interrupting) } - @Test - fun easeToTestNonZeroDurationWithListener() { - val options = mapAnimationOptions { - duration(1000) - animatorListener(object : AnimatorListenerAdapter() {}) - } - cameraAnimationsPluginImpl.easeTo(cameraState.toCameraOptions(), options) - assert(cameraAnimationsPluginImpl.highLevelListener != null) - } - - @Test - fun easeToTestZeroDurationWithListener() { - val options = mapAnimationOptions { - duration(0) - animatorListener(object : AnimatorListenerAdapter() {}) - } - cameraAnimationsPluginImpl.easeTo(cameraState.toCameraOptions(), options) - assert(cameraAnimationsPluginImpl.highLevelListener == null) - } - @Test fun registerOnlyCameraAnimatorsTest() { val pitch = createPitchAnimator(15.0, 0, 5) @@ -962,13 +945,9 @@ class CameraAnimationsPluginImplTest { bearingAnimator.start() pitchAnimator.start() shadowOf(getMainLooper()).idle() - // first bearing tick as first animation - - // bearing updated to start value and pitch is not updated - assertEquals(20.0, updateList[0].bearing!!, EPS) - assertEquals(0.0, updateList[0].pitch!!, EPS) - // second bearing tick as first animation - - // pitch is updated to start value - assertEquals(10.0, updateList[1].pitch!!, EPS) + // Both, tick and bearing are updated to the start values at first tick. + assertEquals(20.0, updateList.first().bearing!!, EPS) + assertEquals(10.0, updateList.first().pitch!!, EPS) } @Test diff --git a/sdk/src/androidTest/java/com/mapbox/maps/CameraAnimationsPluginTest.kt b/sdk/src/androidTest/java/com/mapbox/maps/CameraAnimationsPluginTest.kt index 28956f114c..0b262f73a6 100644 --- a/sdk/src/androidTest/java/com/mapbox/maps/CameraAnimationsPluginTest.kt +++ b/sdk/src/androidTest/java/com/mapbox/maps/CameraAnimationsPluginTest.kt @@ -27,6 +27,133 @@ import java.util.concurrent.TimeoutException @LargeTest class CameraAnimationsPluginTest : BaseAnimationMapTest() { + @Test + fun testAnimatorListener() { + val listener = CameraAnimatorListener() + mainHandler.post { + mapView.camera.easeTo( + cameraOptions { + zoom(7.0) + bearing(120.0) + }, + mapAnimationOptions { + duration(200) + animatorListener(listener) + } + ) + } + + if (!listener.latchStart.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + throw TimeoutException() + } + + if (listener.latchEnd.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + assertEquals(7.0, mapView.getMapboxMap().cameraState.zoom, EPS) + assertEquals(120.0, mapView.getMapboxMap().cameraState.bearing, EPS) + } else { + throw TimeoutException() + } + } + + @Test + fun testAnimatorListenerOnImmediateAnimations() { + val listener = CameraAnimatorListener() + mainHandler.post { + mapView.camera.easeTo( + cameraOptions { + zoom(7.0) + bearing(120.0) + }, + mapAnimationOptions { + duration(0) + animatorListener(listener) + } + ) + } + + if (!listener.latchStart.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + throw TimeoutException() + } + + if (listener.latchEnd.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + assertEquals(7.0, mapView.getMapboxMap().cameraState.zoom, EPS) + assertEquals(120.0, mapView.getMapboxMap().cameraState.bearing, EPS) + } else { + throw TimeoutException() + } + } + + @Test + fun testAnimatorListenerOnImmediateAnimationsWithOtherOngoingAnimations() { + mainHandler.post { + val bearingAnimator = createBearingAnimator(cameraAnimationPlugin, 180.0, 0, 90000) + cameraAnimationPlugin.registerAnimators(bearingAnimator) + bearingAnimator.start() + } + + val listener = CameraAnimatorListener() + mainHandler.post { + mapView.camera.easeTo( + cameraOptions { + zoom(7.0) + }, + mapAnimationOptions { + duration(0) + animatorListener(listener) + } + ) + } + + if (!listener.latchStart.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + throw TimeoutException() + } + + if (listener.latchEnd.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + assertEquals(7.0, mapView.getMapboxMap().cameraState.zoom, EPS) + } else { + throw TimeoutException() + } + } + + @Test + fun testAnimatorListenerOnImmediatelyCanceledAnimations() { + val listener = CameraAnimatorListener() + + mainHandler.post { + mapView.getMapboxMap().setCamera( + cameraOptions { + zoom(4.0) + bearing(0.0) + }, + ) + } + + mainHandler.post { + val set = mapView.camera.easeTo( + cameraOptions { + zoom(7.0) + bearing(120.0) + }, + mapAnimationOptions { + duration(100) + animatorListener(listener) + } + ) + set.cancel() + } + + if (!listener.latchStart.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + throw TimeoutException() + } + + if (listener.latchCancel.await(LATCH_MAX_TIME, TimeUnit.MILLISECONDS)) { + assertEquals(4.0, mapView.getMapboxMap().cameraState.zoom, EPS) + assertEquals(0.0, mapView.getMapboxMap().cameraState.bearing, EPS) + } else { + throw TimeoutException() + } + } + @Test fun testAnimatorWithAnimatorDelay() { if (ignoreTestForGivenAbi()) {