Fix frames not rendered when user sets refresh rate (#5802)

Fixes https://mapbox.atlassian.net/browse/MAPSAND-2287

The issue was visible in the following scenario:
1. User calls set user refresh rate to 60 (and display refresh rate is
also 60)
2. A few frames are rendered, for example 30
3. User calls set user refresh rate to 30

Observed:
- The `performPacing` method was returning false due to `drawFrameIndex`
having lower values (due to the new refresh ratio) than
`previousDrawnFrameIndex`.

The fix is to reset the values every time the user or display refresh
rate changes.

cc @mapbox/maps-android

GitOrigin-RevId: bdb1556e2603f34bc6ff88c706997a07e46e17e2

Author: jush

CHANGELOG.md

@@ -8,6 +8,7 @@ Mapbox welcomes participation and contributions from everyone.
 
 ## Bug fixes 🐞
 * Fix potential ANR (Application Not Responding) issue when retrieving display refresh rate during map initialization by offloading the system call to a background thread with proper timeout and fallback handling.
+* Fix frames skipped when calling `mapView.setMaximumFps(..)` multiple times.
 
 ## Dependencies
 * Update Mapbox GeoJSON library to [v7.5.0](https://github.com/mapbox/mapbox-java/releases/tag/v7.5.0).

maps-sdk/src/main/java/com/mapbox/maps/renderer/FpsManager.kt

@@ -39,10 +39,7 @@ internal class FpsManager(
     }
     this.screenRefreshRate = screenRefreshRate
     screenRefreshPeriodNs = ONE_SECOND_NS / screenRefreshRate
-    if (userRefreshRate != USER_DEFINED_REFRESH_RATE_NOT_SET) {
-      userToScreenRefreshRateRatio = (userRefreshRate.toDouble() / screenRefreshRate).coerceIn(0.0, 1.0)
-      logI(TAG, "User defined ratio is $userToScreenRefreshRateRatio")
-    }
+    updateUserToScreenRefreshRatio()
     if (LOG_STATISTICS) {
       logI(
         TAG,
@@ -56,13 +53,21 @@ internal class FpsManager(
     if (userRefreshRate != refreshRate) {
       userRefreshRate = refreshRate
       logI(TAG, "User set max FPS to $userRefreshRate")
-      if (screenRefreshRate != SCREEN_METRICS_NOT_DEFINED) {
-        userToScreenRefreshRateRatio = (userRefreshRate.toDouble() / screenRefreshRate).coerceIn(0.0, 1.0)
-        logI(TAG, "User defined ratio is $userToScreenRefreshRateRatio")
-      }
+      updateUserToScreenRefreshRatio()
     }
   }
 
+  private fun updateUserToScreenRefreshRatio() {
+    if (userRefreshRate == USER_DEFINED_REFRESH_RATE_NOT_SET || screenRefreshRate == SCREEN_METRICS_NOT_DEFINED) {
+      logI(TAG, "userToScreenRefreshRateRatio is not set (userRefreshRate=$userRefreshRate, screenRefreshRate=$screenRefreshRate)")
+      return
+    }
+    userToScreenRefreshRateRatio = (userRefreshRate.toDouble() / screenRefreshRate).coerceIn(0.0, 1.0)
+    logI(TAG, "User defined ratio is $userToScreenRefreshRateRatio")
+    // After changing the ratio we need to reset counters
+    calculateFpsAndReset()
+  }
+
   /**
    * Return true if rendering should happen this frame and false otherwise.
    *
@@ -130,7 +135,7 @@ internal class FpsManager(
     previousFrameTimeNs = frameTimeNs
     // we always increase choreographer tick by one + add number of skipped frames for consistent results
     choreographerTicks += skippedNow + 1
-    if (skippedNow > 0 && LOG_STATISTICS) {
+    if (LOG_STATISTICS && skippedNow > 0) {
       logW(
         TAG,
         "Skipped $skippedNow VSYNC pulses since last actual render," +
@@ -173,7 +178,7 @@ internal class FpsManager(
     if (LOG_STATISTICS) {
       logI(
         TAG,
-        "Performing pacing, current index=$drawnFrameIndex," +
+        "Performing pacing, current index=$drawnFrameIndex, choreographerTicks=$choreographerTicks," +
           " previous drawn=$previousDrawnFrameIndex, proceed with rendering=${drawnFrameIndex > previousDrawnFrameIndex}"
       )
     }

maps-sdk/src/test/java/com/mapbox/maps/renderer/FpsManagerTest.kt

@@ -40,6 +40,7 @@ class FpsManagerTest {
     pauseHandler()
     idleHandler(vsyncCount = 10) {
       assert(fpsManager.preRender(it))
+      fpsManager.postRender()
     }
   }
 
@@ -50,6 +51,7 @@ class FpsManagerTest {
     val frameRenderedPattern = mutableListOf<Boolean>()
     idleHandler(vsyncCount = 10) {
       frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
     }
     Assert.assertArrayEquals(
       arrayOf(
@@ -68,6 +70,7 @@ class FpsManagerTest {
     val frameRenderedPattern = mutableListOf<Boolean>()
     idleHandler(vsyncCount = vsyncCount) {
       frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
     }
     Assert.assertArrayEquals(
       Array(vsyncCount) { it == 8 || it == 17 },
@@ -83,6 +86,7 @@ class FpsManagerTest {
     val frameRenderedPattern = mutableListOf<Boolean>()
     idleHandler(vsyncCount = vsyncCount) {
       frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
     }
     Assert.assertArrayEquals(
       // 6 frames should be rendered in total during 2 seconds with 3 FPS
@@ -99,6 +103,7 @@ class FpsManagerTest {
     val frameRenderedPattern = mutableListOf<Boolean>()
     idleHandler(vsyncCount = vsyncCount) {
       frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
     }
     Assert.assertArrayEquals(
       Array(vsyncCount) { it == 59 || it == 119 },
@@ -114,11 +119,97 @@ class FpsManagerTest {
     val frameRenderedPattern = mutableListOf<Boolean>()
     idleHandler(vsyncCount = vsyncCount) {
       frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
+    }
+    Assert.assertArrayEquals(
+      Array(vsyncCount) { true },
+      frameRenderedPattern.toTypedArray()
+    )
+  }
+
+  @Test
+  fun userRefreshRateSameAsScreenRefreshRateTest() {
+    fpsManager.setUserRefreshRate(SCREEN_FPS)
+    pauseHandler()
+    val frameRenderedPattern = mutableListOf<Boolean>()
+    idleHandler(vsyncCount = 5 * SCREEN_FPS) {
+      frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
+    }
+    Assert.assertArrayEquals(
+      Array(5 * SCREEN_FPS) { true },
+      frameRenderedPattern.toTypedArray()
+    )
+  }
+
+  @Test
+  fun `user refresh rate changes mid render with listener`() {
+    val fpsValueArray = mutableListOf<Double>()
+    val fpsChangedListener = OnFpsChangedListener {
+      fpsValueArray.add(it)
+    }
+    fpsManager.fpsChangedListener = fpsChangedListener
+    pauseHandler()
+    val frameRenderedPattern = mutableListOf<Boolean>()
+    val vsyncCount = 90
+
+    // First set user refresh rate to be the same as screen refresh rate
+    fpsManager.setUserRefreshRate(SCREEN_FPS)
+    idleHandler(vsyncCount = vsyncCount) {
+      frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
     }
     Assert.assertArrayEquals(
       Array(vsyncCount) { true },
       frameRenderedPattern.toTypedArray()
     )
+    Assert.assertEquals(1, fpsValueArray.size)
+    Assert.assertEquals(SCREEN_FPS.toDouble(), fpsValueArray.last(), EPS)
+
+    frameRenderedPattern.clear()
+
+    // Now change the user refresh ratio to be half of the screen refresh rate
+    fpsManager.setUserRefreshRate(SCREEN_FPS / 2)
+    // That triggers a new listener with the FPS value for the remaining 30 frames out of the 90 above
+    Assert.assertEquals(2, fpsValueArray.size)
+    Assert.assertEquals(SCREEN_FPS.toDouble(), fpsValueArray.last(), EPS)
+
+    // Now render vsyncCount frames with the new user refresh rate
+    idleHandler(vsyncCount = vsyncCount) {
+      frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
+    }
+    Assert.assertArrayEquals(
+      // We expect every other frame to be rendered (user SCREEN_FPS/2 FPS)
+      Array(vsyncCount) { it % 2 != 0 },
+      frameRenderedPattern.toTypedArray()
+    )
+    Assert.assertEquals(3, fpsValueArray.size)
+    Assert.assertEquals(SCREEN_FPS / 2.0, fpsValueArray.last(), EPS)
+
+    frameRenderedPattern.clear()
+
+    // Reset user refresh rate to be the same as screen one
+    fpsManager.setUserRefreshRate(SCREEN_FPS)
+    // That triggers a new listener with the FPS value for the remaining 30 frames out of the 90 above
+    Assert.assertEquals(4, fpsValueArray.size)
+    Assert.assertEquals(SCREEN_FPS / 2.0, fpsValueArray.last(), EPS)
+
+    idleHandler(vsyncCount = vsyncCount) {
+      frameRenderedPattern.add(fpsManager.preRender(it))
+      fpsManager.postRender()
+    }
+    Assert.assertArrayEquals(
+      Array(vsyncCount) { true },
+      frameRenderedPattern.toTypedArray()
+    )
+    Assert.assertEquals(5, fpsValueArray.size)
+    Assert.assertEquals(SCREEN_FPS.toDouble(), fpsValueArray.last(), EPS)
+
+    fpsManager.onSurfaceDestroyed()
+    // One last FPS value for the remaining 30 frames out of the 90 above
+    Assert.assertEquals(6, fpsValueArray.size)
+    Assert.assertEquals(SCREEN_FPS.toDouble(), fpsValueArray.last(), EPS)
   }
 
   @Test
@@ -184,12 +275,11 @@ class FpsManagerTest {
   private fun pauseHandler() = Shadows.shadowOf(Looper.getMainLooper()).pause()
 
   private fun idleHandler(vsyncCount: Int, actionOnVsync: ((Long) -> Unit)) {
-    for (i in 0 until vsyncCount) {
-      Choreographer.getInstance().postFrameCallback { frameTimeNs ->
-        actionOnVsync.invoke(frameTimeNs)
-      }
-      Shadows.shadowOf(Looper.getMainLooper())
-        .idleFor(CHOREOGRAPHER_POST_FRAME_CALLBACK_DELAY_MS.toLong(), TimeUnit.MILLISECONDS)
+    val choreographer = Choreographer.getInstance()
+    val looper = Shadows.shadowOf(Looper.getMainLooper())
+    repeat(vsyncCount) {
+      choreographer.postFrameCallback(actionOnVsync)
+      looper.idleFor(CHOREOGRAPHER_POST_FRAME_CALLBACK_DELAY_MS.toLong(), TimeUnit.MILLISECONDS)
     }
   }