[location] Gate Android puck pulsing on layer visibility and lifecycle

CHANGELOG.md

@@ -6,6 +6,9 @@ Mapbox welcomes participation and contributions from everyone.
 
 # main
 
+## Bug fixes 🐞
+* Stop location puck pulse animations while the puck is hidden or the location component is stopped.
+
 ## Features ✨ and improvements 🏁
 * Accessing a getter for a plugin that was intentionally disabled (compass, logo, attribution, scalebar, lifecycle, indoorSelector) because the Compose extension replaces with its own APIs, now throws a descriptive `PluginDisabledException`, instead of an unhelpful `NullPointerException`.
 

plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImpl.kt

@@ -35,6 +35,8 @@ internal class LocationComponentPluginImpl : LocationComponentPlugin, LocationCo
 
   private lateinit var weakContext: WeakReference<Context>
 
+  private var isStarted = false
+
   @VisibleForTesting(otherwise = PRIVATE)
   internal var locationPuckManager: LocationPuckManager? = null
 
@@ -200,11 +202,12 @@ internal class LocationComponentPluginImpl : LocationComponentPlugin, LocationCo
    * Called whenever activity's/fragment's lifecycle is entering a "started" state.
    */
   override fun onStart() {
+    isStarted = true
     activateLocationComponent()
   }
 
   private fun activateLocationComponent() {
-    if (internalSettings.enabled) {
+    if (isStarted && internalSettings.enabled) {
       val style = delegateProvider.mapStyleManagerDelegate
       if (locationPuckManager?.isLayerInitialised() == true && isLocationComponentActivated) {
         return
@@ -238,6 +241,7 @@ internal class LocationComponentPluginImpl : LocationComponentPlugin, LocationCo
    * Called whenever activity's/fragment's lifecycle is entering a "stopped" state.
    */
   override fun onStop() {
+    isStarted = false
     isLocationComponentActivated = false
     locationPuckManager?.onStop()
     locationProvider?.unRegisterLocationConsumer(this)

plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRenderer.kt

@@ -27,6 +27,7 @@ internal class LocationIndicatorLayerRenderer(
 ) : LocationLayerRenderer {
 
   private var style: MapboxStyleManager? = null
+  private var isVisible = false
 
   override fun initializeComponents(style: MapboxStyleManager) {
     this.style = style
@@ -126,6 +127,7 @@ internal class LocationIndicatorLayerRenderer(
   }
 
   private fun setLayerVisibility(visible: Boolean) {
+    isVisible = visible
     layer.visibility(visible)
   }
 
@@ -156,6 +158,7 @@ internal class LocationIndicatorLayerRenderer(
     radius: Float,
     opacity: Float?
   ) {
+    if (!isVisible) return
     val rgbaArray = colorToRgbaArray(pulsingColorInt)
     rgbaArray[3] = opacity ?: 1f
     layer.emphasisCircleRadius(radius.toDouble())

plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManager.kt

@@ -83,6 +83,7 @@ internal class LocationPuckManager(
         onBearingUpdated,
         onAccuracyRadiusUpdated
       )
+      animationManager.onPuckVisibilityChanged(false)
       animationManager.setLocationLayerRenderer(locationLayerRenderer)
       animationManager.applySettings(settings)
       lastLocation?.let {
@@ -236,11 +237,13 @@ internal class LocationPuckManager(
     if (forceUpdate || isHidden) {
       isHidden = false
       locationLayerRenderer.show()
+      animationManager.onPuckVisibilityChanged(true)
     }
   }
 
   fun hide() {
     isHidden = true
+    animationManager.onPuckVisibilityChanged(false)
     locationLayerRenderer.hide()
   }
 

plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManager.kt

@@ -23,6 +23,9 @@ internal class PuckAnimatorManager(
     PuckAccuracyRadiusAnimator(indicatorAccuracyRadiusChangedListener)
   private var pulsingAnimator = PuckPulsingAnimator(pixelRatio)
 
+  private var isStarted = false
+  private var isPuckVisible = false
+
   internal var puckAnimationEnabled: Boolean
     get() = bearingAnimator.enabled
     set(value) {
@@ -69,18 +72,31 @@ internal class PuckAnimatorManager(
   }
 
   fun onStart() {
-    if (pulsingAnimator.enabled) {
-      pulsingAnimator.animateInfinite()
-    }
+    isStarted = true
+    updatePulsingState()
   }
 
   fun onStop() {
+    isStarted = false
     bearingAnimator.cancelRunning()
     positionAnimator.cancelRunning()
     pulsingAnimator.cancelRunning()
     accuracyRadiusAnimator.cancelRunning()
   }
 
+  fun onPuckVisibilityChanged(visible: Boolean) {
+    isPuckVisible = visible
+    updatePulsingState()
+  }
+
+  private fun updatePulsingState() {
+    if (isStarted && isPuckVisible && pulsingAnimator.enabled) {
+      pulsingAnimator.animateInfinite()
+    } else {
+      pulsingAnimator.cancelRunning()
+    }
+  }
+
   fun animateBearing(
     vararg targets: Double,
     options: (ValueAnimator.() -> Unit)?
@@ -110,24 +126,18 @@ internal class PuckAnimatorManager(
       enabled = settings.pulsingEnabled
       if (settings.pulsingEnabled) {
         maxRadius = target
-        animateInfinite()
-      } else {
-        cancelRunning()
       }
     }
+    updatePulsingState()
   }
 
   fun applySettings(settings: LocationComponentSettings) {
     pulsingAnimator.apply {
       enabled = settings.pulsingEnabled
       maxRadius = settings.pulsingMaxRadius.toDouble()
       pulsingColor = settings.pulsingColor
-      if (settings.pulsingEnabled) {
-        animateInfinite()
-      } else {
-        cancelRunning()
-      }
     }
+    updatePulsingState()
     accuracyRadiusAnimator.apply {
       enabled = settings.showAccuracyRing
       accuracyCircleColor = settings.accuracyRingColor

plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimator.kt

@@ -21,6 +21,12 @@ internal class PuckPulsingAnimator(private val pixelRatio: Float = 1.0f) :
     repeatMode = RESTART
     repeatCount = INFINITE
     interpolator = PULSING_DEFAULT_INTERPOLATOR
+    addListener(object : AnimatorListenerAdapter() {
+      override fun onAnimationRepeat(animation: Animator) {
+        super.onAnimationRepeat(animation)
+        setObjectValues(0.0, maxRadius)
+      }
+    })
   }
 
   fun animateInfinite() {
@@ -30,12 +36,6 @@ internal class PuckPulsingAnimator(private val pixelRatio: Float = 1.0f) :
     if (!isRunning) {
       animate(0.0, maxRadius)
     }
-    addListener(object : AnimatorListenerAdapter() {
-      override fun onAnimationRepeat(animation: Animator) {
-        super.onAnimationRepeat(animation)
-        setObjectValues(0.0, maxRadius)
-      }
-    })
   }
 
   override fun updateLayer(fraction: Float, value: Double) {

plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImplTest.kt

@@ -94,6 +94,22 @@ class LocationComponentPluginImplTest {
     locationComponentPlugin = LocationComponentPluginImpl()
   }
 
+  @Test
+  fun settingsWhileStoppedDoNotReactivateLocationComponent() {
+    preparePluginInitialisationWithEnabled()
+    locationComponentPlugin.onStop()
+    io.mockk.clearMocks(locationPuckManager, locationProvider, answers = false)
+
+    locationComponentPlugin.pulsingEnabled = true
+    locationComponentPlugin.pulsingColor = Color.GREEN
+    verify(exactly = 0) { locationPuckManager.onStart() }
+    verify(exactly = 0) { locationProvider.registerLocationConsumer(any()) }
+
+    locationComponentPlugin.onStart()
+    verify(exactly = 1) { locationPuckManager.onStart() }
+    verify(exactly = 1) { locationProvider.registerLocationConsumer(any()) }
+  }
+
   @After
   fun cleanUp() {
     unmockkAll()

plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRendererTest.kt

@@ -70,6 +70,20 @@ class LocationIndicatorLayerRendererTest {
     locationLayerRenderer.initializeComponents(style)
   }
 
+  @Test
+  fun hiddenRendererIgnoresPendingPulseUpdates() {
+    locationLayerRenderer.updatePulsingUi(Color.BLUE, 10f, 0.5f)
+    verify(exactly = 0) { layerWrapper.emphasisCircleRadius(any()) }
+    locationLayerRenderer.show()
+    locationLayerRenderer.updatePulsingUi(Color.BLUE, 10f, 0.5f)
+    verify(exactly = 1) { layerWrapper.emphasisCircleRadius(10.0) }
+    clearMocks(layerWrapper, answers = false)
+    locationLayerRenderer.hide()
+    locationLayerRenderer.updatePulsingUi(Color.BLUE, 15f, 0.4f)
+    verify(exactly = 0) { layerWrapper.emphasisCircleRadius(any()) }
+    verify(exactly = 0) { layerWrapper.emphasisCircleColor(any()) }
+  }
+
   @Test
   fun sanity() {
     Assert.assertNotNull(locationLayerRenderer)

plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManagerTest.kt

@@ -97,6 +97,27 @@ class LocationPuckManagerTest {
     unmockkAll()
   }
 
+  @Test
+  fun visibilityGatesPulsingAcrossLocationAndStyleReload() {
+    locationPuckManager.initialize(style)
+    verify { animationManager.onPuckVisibilityChanged(false) }
+    verify(exactly = 0) { animationManager.onPuckVisibilityChanged(true) }
+
+    locationPuckManager.updateCurrentPosition(Point.fromLngLat(10.0, 20.0))
+    verify { animationManager.onPuckVisibilityChanged(true) }
+    locationPuckManager.hide()
+    assertTrue(locationPuckManager.isHidden)
+
+    io.mockk.clearMocks(animationManager, answers = false)
+    locationPuckManager.lastLocation = Point.fromLngLat(10.0, 20.0)
+    locationPuckManager.updateSettings(settings)
+    verifyOrder {
+      animationManager.onPuckVisibilityChanged(false)
+      animationManager.applySettings(settings)
+      animationManager.onPuckVisibilityChanged(true)
+    }
+  }
+
   @Test
   fun testInitialise() {
     locationPuckManager.initialize(style)

plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManagerTest.kt

@@ -6,6 +6,7 @@ import android.os.Looper
 import com.mapbox.geojson.Point
 import com.mapbox.maps.plugin.locationcomponent.LocationLayerRenderer
 import com.mapbox.maps.plugin.locationcomponent.generated.LocationComponentSettings
+import io.mockk.clearMocks
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
@@ -36,6 +37,9 @@ class PuckAnimatorManagerTest {
 
   @Before
   fun setUp() {
+    var pulsingEnabled = false
+    every { pulsingAnimator.enabled } answers { pulsingEnabled }
+    every { pulsingAnimator.enabled = any() } answers { pulsingEnabled = firstArg() }
     puckAnimatorManager = PuckAnimatorManager(
       mockk(),
       mockk(),
@@ -63,6 +67,7 @@ class PuckAnimatorManagerTest {
   @Test
   fun onStartOne() {
     every { pulsingAnimator.enabled } returns true
+    puckAnimatorManager.onPuckVisibilityChanged(true)
     puckAnimatorManager.onStart()
     verify(exactly = 1) { pulsingAnimator.animateInfinite() }
   }
@@ -162,6 +167,8 @@ class PuckAnimatorManagerTest {
 
   @Test
   fun applyAnimationSettingsOne() {
+    puckAnimatorManager.onPuckVisibilityChanged(true)
+    puckAnimatorManager.onStart()
     val settings = mockk<LocationComponentSettings>(relaxed = true)
     every { settings.pulsingEnabled } returns true
     every { settings.pulsingMaxRadius } returns 20.0f
@@ -222,6 +229,8 @@ class PuckAnimatorManagerTest {
 
   @Test
   fun testUpdatePulsingPuckRadius() {
+    puckAnimatorManager.onPuckVisibilityChanged(true)
+    puckAnimatorManager.onStart()
     val updatedRadius = 50.0
     val settings = mockk<LocationComponentSettings>(relaxed = true)
     every { settings.pulsingEnabled } returns true
@@ -250,6 +259,48 @@ class PuckAnimatorManagerTest {
     verify(exactly = 0) { pulsingAnimator.maxRadius = updatedRadius }
   }
 
+  @Test
+  fun pulsingWaitsForVisibilityAndResumesAfterStop() {
+    val settings = mockk<LocationComponentSettings>(relaxed = true)
+    every { settings.pulsingEnabled } returns true
+    puckAnimatorManager.applySettings(settings)
+    puckAnimatorManager.onStart()
+    verify(exactly = 0) { pulsingAnimator.animateInfinite() }
+
+    puckAnimatorManager.onPuckVisibilityChanged(true)
+    verify(exactly = 1) { pulsingAnimator.animateInfinite() }
+    clearMocks(pulsingAnimator, answers = false)
+
+    puckAnimatorManager.onPuckVisibilityChanged(false)
+    puckAnimatorManager.updatePulsingRadius(50.0, settings)
+    puckAnimatorManager.applySettings(settings)
+    verify(exactly = 0) { pulsingAnimator.animateInfinite() }
+    verify { pulsingAnimator.cancelRunning() }
+
+    puckAnimatorManager.onPuckVisibilityChanged(true)
+    verify(exactly = 1) { pulsingAnimator.animateInfinite() }
+    puckAnimatorManager.onStop()
+    clearMocks(pulsingAnimator, answers = false)
+    puckAnimatorManager.updatePulsingRadius(60.0, settings)
+    puckAnimatorManager.applySettings(settings)
+    verify(exactly = 0) { pulsingAnimator.animateInfinite() }
+    puckAnimatorManager.onStart()
+    verify(exactly = 1) { pulsingAnimator.animateInfinite() }
+  }
+
+  @Test
+  fun visibilityDoesNotOverrideDisabledPulsing() {
+    val settings = mockk<LocationComponentSettings>(relaxed = true)
+    every { settings.pulsingEnabled } returns false
+    puckAnimatorManager.applySettings(settings)
+    puckAnimatorManager.onStart()
+    repeat(3) {
+      puckAnimatorManager.onPuckVisibilityChanged(true)
+      puckAnimatorManager.onPuckVisibilityChanged(false)
+    }
+    verify(exactly = 0) { pulsingAnimator.animateInfinite() }
+  }
+
   companion object {
     private val START_POINT = Point.fromLngLat(-122.4194, 37.7749)
     private val END_POINT = Point.fromLngLat(-77.0369, 38.9072)

plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimatorTest.kt

@@ -23,6 +23,14 @@ class PuckPulsingAnimatorTest {
     pulsingAnimator = spyk(animator)
   }
 
+  @Test
+  fun repeatedStartsDoNotAccumulateRepeatListeners() {
+    every { pulsingAnimator.animate(any(), any()) } just Runs
+    val listenerCount = pulsingAnimator.listeners?.size ?: 0
+    repeat(5) { pulsingAnimator.animateInfinite() }
+    Assert.assertEquals(listenerCount, pulsingAnimator.listeners?.size ?: 0)
+  }
+
   @Test
   fun creation() {
     Assert.assertEquals(PuckPulsingAnimator.PULSING_DEFAULT_DURATION, pulsingAnimator.duration)