Fix artifacts when ContextMode.SHARED is used (#2299)

* Fix artifacts when ContextMode.SHARED is used

* Fix crash on surface recreation, add test

* Handle surface resize more properly

* Fix incorrect widget position and scale when resizing the drawing surface.

Author: kiryldz

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 Mapbox welcomes participation and contributions from everyone.
 
+# 10.16.7
+## Bug fixes 🐞
+* Fix map being pixelated on some devices when `ContextMode.SHARED` is used (e.g. in AndroidAuto extension).
+* Fix incorrect widget position and scale when resizing the drawing surface.
 
 # 10.16.6 March 04, 2024
 ## Bug fixes 🐞

app/src/androidTest/java/com/mapbox/maps/testapp/integration/surface/SurfaceReopenTest.kt

@@ -0,0 +1,43 @@
+package com.mapbox.maps.testapp.integration.surface
+
+import android.view.KeyEvent
+import androidx.test.ext.junit.rules.ActivityScenarioRule
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.filters.LargeTest
+import com.mapbox.maps.testapp.examples.SurfaceActivity
+import com.mapbox.maps.testapp.integration.BaseIntegrationTest
+import com.mapbox.maps.testapp.integration.launchActivity
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/**
+ * Regression test that validates reopening an Activity with a surface using ContextMode.SHARED
+ */
+@RunWith(AndroidJUnit4::class)
+class SurfaceReopenTest : BaseIntegrationTest() {
+
+  @get:Rule
+  var activityRule: ActivityScenarioRule<SurfaceActivity> = ActivityScenarioRule(
+    SurfaceActivity::class.java
+  )
+
+  @Test
+  @LargeTest
+  fun reopenSurfaceActivity() {
+    activityRule.scenario.onActivity {
+      device.launchActivity(it, SurfaceActivity::class.java)
+      device.waitForIdle()
+    }
+    for (i in 0 until testRepeatCount) {
+      device.waitForIdle()
+      device.pressHome()
+      device.waitForIdle()
+      // double click restores last opened application
+      device.pressKeyCode(KeyEvent.KEYCODE_APP_SWITCH)
+      device.pressKeyCode(KeyEvent.KEYCODE_APP_SWITCH)
+      // some time to set things up
+      Thread.sleep(2_000L)
+    }
+  }
+}

sdk/src/main/java/com/mapbox/maps/MapSurface.kt

@@ -49,7 +49,10 @@ class MapSurface : MapPluginProviderDelegate, MapControllable {
     this.context = context
     this.surface = surface
     this.mapInitOptions = mapInitOptions
-    this.renderer = MapboxSurfaceRenderer(mapInitOptions.antialiasingSampleCount)
+    this.renderer = MapboxSurfaceRenderer(
+      mapInitOptions.antialiasingSampleCount,
+      mapInitOptions.mapOptions.contextMode ?: ContextMode.UNIQUE
+    )
     this.mapController = MapController(renderer, mapInitOptions).apply {
       initializePlugins(mapInitOptions)
     }

sdk/src/main/java/com/mapbox/maps/MapView.kt

@@ -105,15 +105,18 @@ open class MapView : FrameLayout, MapPluginProviderDelegate, MapControllable {
     } else {
       SurfaceView(context, attrs)
     }
+    val contextMode = resolvedMapInitOptions.mapOptions.contextMode ?: ContextMode.UNIQUE
     mapController = MapController(
       when (view) {
         is SurfaceView -> MapboxSurfaceHolderRenderer(
           view.holder,
-          resolvedMapInitOptions.antialiasingSampleCount
+          resolvedMapInitOptions.antialiasingSampleCount,
+          contextMode
         )
         is TextureView -> MapboxTextureViewRenderer(
           view,
-          resolvedMapInitOptions.antialiasingSampleCount
+          resolvedMapInitOptions.antialiasingSampleCount,
+          contextMode
         )
         else -> throw IllegalArgumentException("Provided view has to be a texture or a surface.")
       },

sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt

@@ -9,6 +9,7 @@ import android.view.Choreographer
 import android.view.Surface
 import androidx.annotation.*
 import com.mapbox.common.MapboxSDKCommon
+import com.mapbox.maps.ContextMode
 import com.mapbox.maps.logE
 import com.mapbox.maps.logI
 import com.mapbox.maps.logW
@@ -55,6 +56,7 @@ internal class MapboxRenderThread : Choreographer.FrameCallback {
   private val widgetRenderer: MapboxWidgetRenderer
   private var widgetRenderCreated = false
   private val widgetTextureRenderer: TextureRenderer
+  private val contextMode: ContextMode
 
   @Volatile
   @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@@ -133,13 +135,15 @@ internal class MapboxRenderThread : Choreographer.FrameCallback {
     mapboxWidgetRenderer: MapboxWidgetRenderer,
     translucentSurface: Boolean,
     antialiasingSampleCount: Int,
+    contextMode: ContextMode,
   ) {
     this.translucentSurface = translucentSurface
     this.mapboxRenderer = mapboxRenderer
     this.widgetRenderer = mapboxWidgetRenderer
     this.eglCore = EGLCore(translucentSurface, antialiasingSampleCount)
     this.eglSurface = eglCore.eglNoSurface
     this.widgetTextureRenderer = TextureRenderer()
+    this.contextMode = contextMode
     renderHandlerThread = RenderHandlerThread()
     val handler = renderHandlerThread.start()
     fpsManager = FpsManager(handler)
@@ -162,6 +166,7 @@ internal class MapboxRenderThread : Choreographer.FrameCallback {
     this.fpsManager = fpsManager
     this.widgetTextureRenderer = widgetTextureRenderer
     this.eglSurface = eglCore.eglNoSurface
+    this.contextMode = ContextMode.UNIQUE
   }
 
   private fun postPrepareRenderFrame(delayMillis: Long = 0L) {
@@ -315,9 +320,12 @@ internal class MapboxRenderThread : Choreographer.FrameCallback {
       postPrepareRenderFrame()
       return
     }
+    if (contextMode == ContextMode.SHARED) {
+      GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT or GLES20.GL_DEPTH_BUFFER_BIT or GLES20.GL_STENCIL_BUFFER_BIT)
+    }
     if (widgetRenderer.hasWidgets()) {
-      if (widgetRenderer.needTextureUpdate) {
-        widgetRenderer.updateTexture()
+      if (widgetRenderer.needRender) {
+        widgetRenderer.renderToFrameBuffer()
         eglCore.makeCurrent(eglSurface)
       }
 

sdk/src/main/java/com/mapbox/maps/renderer/MapboxSurfaceHolderRenderer.kt

@@ -2,10 +2,11 @@ package com.mapbox.maps.renderer
 
 import android.view.SurfaceHolder
 import androidx.annotation.VisibleForTesting
+import com.mapbox.maps.ContextMode
 
 internal class MapboxSurfaceHolderRenderer : MapboxSurfaceRenderer, SurfaceHolder.Callback {
 
-  constructor(surfaceHolder: SurfaceHolder, antialiasingSampleCount: Int) : super(antialiasingSampleCount) {
+  constructor(surfaceHolder: SurfaceHolder, antialiasingSampleCount: Int, contextMode: ContextMode) : super(antialiasingSampleCount, contextMode) {
     surfaceHolder.addCallback(this)
   }
 

sdk/src/main/java/com/mapbox/maps/renderer/MapboxSurfaceRenderer.kt

@@ -2,14 +2,15 @@ package com.mapbox.maps.renderer
 
 import android.view.Surface
 import androidx.annotation.VisibleForTesting
+import com.mapbox.maps.ContextMode
 
 internal open class MapboxSurfaceRenderer : MapboxRenderer {
 
   private var createSurface = false
 
   override val widgetRenderer: MapboxWidgetRenderer
 
-  constructor(antialiasingSampleCount: Int) {
+  constructor(antialiasingSampleCount: Int, contextMode: ContextMode) {
     widgetRenderer = MapboxWidgetRenderer(
       antialiasingSampleCount = antialiasingSampleCount,
     )
@@ -18,6 +19,7 @@ internal open class MapboxSurfaceRenderer : MapboxRenderer {
       mapboxWidgetRenderer = widgetRenderer,
       translucentSurface = false,
       antialiasingSampleCount = antialiasingSampleCount,
+      contextMode = contextMode
     )
   }
 

sdk/src/main/java/com/mapbox/maps/renderer/MapboxTextureViewRenderer.kt

@@ -4,12 +4,13 @@ import android.graphics.SurfaceTexture
 import android.view.Surface
 import android.view.TextureView
 import androidx.annotation.VisibleForTesting
+import com.mapbox.maps.ContextMode
 
 internal class MapboxTextureViewRenderer : MapboxRenderer, TextureView.SurfaceTextureListener {
 
   override val widgetRenderer: MapboxWidgetRenderer
 
-  constructor(textureView: TextureView, antialiasingSampleCount: Int) {
+  constructor(textureView: TextureView, antialiasingSampleCount: Int, contextMode: ContextMode) {
     val widgetRenderer = MapboxWidgetRenderer(
       antialiasingSampleCount = antialiasingSampleCount,
     )
@@ -19,6 +20,7 @@ internal class MapboxTextureViewRenderer : MapboxRenderer, TextureView.SurfaceTe
       mapboxWidgetRenderer = widgetRenderer,
       translucentSurface = true,
       antialiasingSampleCount = antialiasingSampleCount,
+      contextMode = contextMode,
     )
     textureView.let {
       it.isOpaque = false

sdk/src/main/java/com/mapbox/maps/renderer/MapboxWidgetRenderer.kt

@@ -1,13 +1,15 @@
 package com.mapbox.maps.renderer
 
 import android.opengl.GLES20
+import com.mapbox.maps.MapboxExperimental
 import com.mapbox.maps.logE
 import com.mapbox.maps.renderer.egl.EGLCore
 import com.mapbox.maps.renderer.widget.Widget
 import java.util.concurrent.CopyOnWriteArraySet
 import javax.microedition.khronos.egl.EGLContext
 import javax.microedition.khronos.egl.EGLSurface
 
+@OptIn(MapboxExperimental::class)
 internal class MapboxWidgetRenderer(
   private val antialiasingSampleCount: Int,
 ) {
@@ -24,7 +26,7 @@ internal class MapboxWidgetRenderer(
   private var width = 0
   private var height = 0
 
-  val needTextureUpdate: Boolean
+  val needRender: Boolean
     get() = widgets.any { it.renderer.needRender }
 
   fun hasWidgets() = widgets.isNotEmpty()
@@ -51,12 +53,9 @@ internal class MapboxWidgetRenderer(
     widgets.forEach { it.renderer.onSurfaceChanged(width, height) }
   }
 
-  private fun attachTexture() {
-    if (textures[0] != 0) {
-      GLES20.glDeleteTextures(textures.size, textures, 0)
-    }
+  private fun prepareFrameBufferWithTexture() {
+    GLES20.glGenFramebuffers(1, framebuffers, 0)
     GLES20.glGenTextures(1, textures, 0)
-
     GLES20.glBindTexture(GLES20.GL_TEXTURE_2D, textures[0])
     GLES20.glTexParameterf(
       GLES20.GL_TEXTURE_2D,
@@ -89,23 +88,26 @@ internal class MapboxWidgetRenderer(
       GLES20.GL_UNSIGNED_BYTE,
       null
     )
+    GLES20.glBindTexture(GLES20.GL_TEXTURE_2D, 0)
+    GLES20.glBindFramebuffer(GLES20.GL_FRAMEBUFFER, framebuffers[0])
     GLES20.glFramebufferTexture2D(
       GLES20.GL_FRAMEBUFFER, GLES20.GL_COLOR_ATTACHMENT0,
       GLES20.GL_TEXTURE_2D, textures[0], 0
     )
-    GLES20.glBindTexture(GLES20.GL_TEXTURE_2D, 0)
+    GLES20.glBindFramebuffer(GLES20.GL_FRAMEBUFFER, 0)
   }
 
   fun release() {
+    releaseAll(resizing = false)
+  }
+
+  private fun releaseAll(resizing: Boolean) {
     val eglCore = this.eglCore
     val eglSurface = this.eglSurface
     if (eglCore != null) {
       if (eglSurface != null && eglSurface != eglCore.eglNoSurface) {
         eglCore.makeCurrent(eglSurface)
-
-        GLES20.glDeleteFramebuffers(framebuffers.size, framebuffers, 0)
-        GLES20.glDeleteTextures(textures.size, textures, 0)
-
+        deleteFrameBufferWithTexture()
         // Do not clear widgets here, or they will be lost if Activity is paused/resumed.
         widgets.forEach {
           it.renderer.release()
@@ -117,52 +119,50 @@ internal class MapboxWidgetRenderer(
       eglCore.release()
     }
     this.eglSurface = null
-    this.eglCore = null
+    if (!resizing) {
+      this.eglCore = null
+    }
     this.eglContextCreated = false
   }
 
-  fun updateTexture() {
-    if (needTextureUpdate) {
-      checkSizeChanged()
-      checkEgl()
-      val eglCore = this.eglCore
-      val eglSurface = this.eglSurface
-      if (eglCore != null && eglSurface != null && eglSurface != eglCore.eglNoSurface) {
-        eglCore.makeCurrent(eglSurface)
-        bindFramebuffer()
-        attachTexture()
-        widgets.forEach {
-          it.renderer.render()
-        }
-        unbindFramebuffer()
-      }
+  fun renderToFrameBuffer() {
+    checkSizeChanged()
+    checkEgl()
+    val eglCore = this.eglCore
+    val eglSurface = this.eglSurface
+    if (eglCore == null || eglSurface == null || eglSurface == eglCore.eglNoSurface) {
+      return
     }
+    eglCore.makeCurrent(eglSurface)
+    if (!hasTexture()) {
+      prepareFrameBufferWithTexture()
+    }
+    GLES20.glBindFramebuffer(GLES20.GL_FRAMEBUFFER, framebuffers[0])
+    GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT or GLES20.GL_DEPTH_BUFFER_BIT or GLES20.GL_STENCIL_BUFFER_BIT)
+    widgets.forEach {
+      it.renderer.render()
+    }
+    GLES20.glBindFramebuffer(GLES20.GL_FRAMEBUFFER, 0)
+  }
+
+  private fun deleteFrameBufferWithTexture() {
+    GLES20.glDeleteFramebuffers(framebuffers.size, framebuffers, 0)
+    GLES20.glDeleteTextures(textures.size, textures, 0)
+    framebuffers[0] = 0
+    textures[0] = 0
   }
 
   private fun checkSizeChanged() {
     if (sizeChanged) {
       val eglCore = this.eglCore
       val eglSurface = this.eglSurface
       if (eglCore != null && eglSurface != null && eglSurface != eglCore.eglNoSurface) {
-        eglCore.releaseSurface(eglSurface)
-        this.eglSurface = eglCore.eglNoSurface
+        releaseAll(resizing = true)
       }
-
       sizeChanged = false
     }
   }
 
-  private fun unbindFramebuffer() {
-    GLES20.glBindFramebuffer(GLES20.GL_FRAMEBUFFER, 0)
-  }
-
-  private fun bindFramebuffer() {
-    if (framebuffers[0] == 0) {
-      GLES20.glGenFramebuffers(1, framebuffers, 0)
-    }
-    GLES20.glBindFramebuffer(GLES20.GL_FRAMEBUFFER, framebuffers[0])
-  }
-
   private fun checkEgl() {
     val eglSurface = this.eglSurface
     val eglCore = this.eglCore
@@ -185,7 +185,7 @@ internal class MapboxWidgetRenderer(
 
     if (eglSurface == null || eglSurface == eglCore.eglNoSurface) {
       this.eglSurface = eglCore.createOffscreenSurface(width = width, height = height)
-      if (eglSurface == eglCore.eglNoSurface) {
+      if (this.eglSurface == eglCore.eglNoSurface) {
         logE(TAG, "Widget offscreen surface was not configured, please check logs above.")
         return
       }

sdk/src/main/java/com/mapbox/maps/renderer/egl/EGLCore.kt

@@ -158,6 +158,7 @@ internal class EGLCore(
     if (eglCreatePbufferSurfaceError != null || eglSurface == null) {
       return eglNoSurface
     }
+    logI(TAG, "Created PBuffer, w = $width, h = $height")
     return eglSurface
   }