Release the map view and detach the controller when MapViewBase leaves

.github/workflows/build.yml

@@ -37,9 +37,21 @@ jobs:
           java-version: '17'
           cache: 'gradle'
 
+      # core と同時に出した変更（同じ名前のブランチ）は、その core と組む。
+      # 既定ブランチの core では、まだ入っていない API を使う PR が必ず落ちる。
+      # core に同じ名前のブランチが無ければ、これまでどおり既定ブランチを使う。
       - name: Build core locally
+        env:
+          HEAD_REF: ${{ github.head_ref }}
         run: |
-          git clone --depth 1 https://github.com/MapConductor/android-sdk-core.git _core
+          CORE=https://github.com/MapConductor/android-sdk-core.git
+          if [ -n "$HEAD_REF" ] && git ls-remote --exit-code --heads "$CORE" "$HEAD_REF" > /dev/null; then
+            echo "core: $HEAD_REF"
+            git clone --depth 1 --branch "$HEAD_REF" "$CORE" _core
+          else
+            echo "core: default branch"
+            git clone --depth 1 "$CORE" _core
+          fi
           cd _core
           ./gradlew --no-daemon publishToMavenLocal
           cd ..

api/android-sdk-compose.api

@@ -61,7 +61,12 @@ public final class com.mapconductor.compose.map.MapAttributionOverlayKt {
   public static final void MapAttributionOverlay(java.util.List<java.lang.String>, androidx.compose.runtime.Composer, int);
 }
 public final class com.mapconductor.compose.map.MapViewBaseKt {
-  public static final <SpecificState extends com.mapconductor.core.map.MapViewStateInterface<?>, SpecificController extends com.mapconductor.core.controller.BaseMapViewController, ActualMapView extends android.view.View, ActualMap, SpecificScope extends com.mapconductor.compose.MapViewScope, SpecificHolder extends com.mapconductor.core.map.MapViewHolderInterface<ActualMapView, ActualMap>> void MapViewBase(SpecificState, androidx.compose.runtime.MutableState<com.mapconductor.core.map.MapCameraPositionInterface>, androidx.compose.ui.Modifier, kotlin.jvm.functions.Function2<? super androidx.compose.runtime.Composer, ? super java.lang.Integer, ? extends ActualMapView>, SpecificScope, com.mapconductor.core.map.MapOverlayRegistry, kotlin.jvm.functions.Function1<? super kotlin.coroutines.Continuation<? super java.lang.Boolean>, ? extends java.lang.Object>, kotlin.jvm.functions.Function2<? super ActualMapView, ? super kotlin.coroutines.Continuation<? super SpecificHolder>, ? extends java.lang.Object>, kotlin.jvm.functions.Function2<? super SpecificHolder, ? super kotlin.coroutines.Continuation<? super SpecificController>, ? extends java.lang.Object>, kotlin.jvm.functions.Function1<? super com.mapconductor.core.map.MapViewStateInterface<?>, kotlin.Unit>, kotlin.jvm.functions.Function4<? super com.mapconductor.core.map.InitState, ? super androidx.compose.ui.node.Ref<SpecificHolder>, ? super androidx.compose.runtime.Composer, ? super java.lang.Integer, kotlin.Unit>, com.mapconductor.core.map.MapViewStyle, kotlin.jvm.functions.Function1<? super java.util.List<java.lang.String>, kotlin.Unit>, kotlin.jvm.functions.Function3<? super SpecificScope, ? super androidx.compose.runtime.Composer, ? super java.lang.Integer, kotlin.Unit>, androidx.compose.runtime.Composer, int, int, int);
+  public static final <SpecificState extends com.mapconductor.core.map.MapViewStateInterface<?>, SpecificController extends com.mapconductor.core.controller.BaseMapViewController, ActualMapView extends android.view.View, ActualMap, SpecificScope extends com.mapconductor.compose.MapViewScope, SpecificHolder extends com.mapconductor.core.map.MapViewHolderInterface<ActualMapView, ActualMap>> void MapViewBase(SpecificState, androidx.compose.runtime.MutableState<com.mapconductor.core.map.MapCameraPositionInterface>, androidx.compose.ui.Modifier, kotlin.jvm.functions.Function2<? super androidx.compose.runtime.Composer, ? super java.lang.Integer, ? extends ActualMapView>, SpecificScope, com.mapconductor.core.map.MapOverlayRegistry, kotlin.jvm.functions.Function1<? super kotlin.coroutines.Continuation<? super java.lang.Boolean>, ? extends java.lang.Object>, kotlin.jvm.functions.Function2<? super ActualMapView, ? super kotlin.coroutines.Continuation<? super SpecificHolder>, ? extends java.lang.Object>, kotlin.jvm.functions.Function2<? super SpecificHolder, ? super kotlin.coroutines.Continuation<? super SpecificController>, ? extends java.lang.Object>, kotlin.jvm.functions.Function1<? super com.mapconductor.core.map.MapViewStateInterface<?>, kotlin.Unit>, kotlin.jvm.functions.Function4<? super com.mapconductor.core.map.InitState, ? super androidx.compose.ui.node.Ref<SpecificHolder>, ? super androidx.compose.runtime.Composer, ? super java.lang.Integer, kotlin.Unit>, com.mapconductor.core.map.MapViewStyle, kotlin.jvm.functions.Function1<? super java.util.List<java.lang.String>, kotlin.Unit>, kotlin.jvm.functions.Function1<? super ActualMapView, kotlin.Unit>, kotlin.jvm.functions.Function3<? super SpecificScope, ? super androidx.compose.runtime.Composer, ? super java.lang.Integer, kotlin.Unit>, androidx.compose.runtime.Composer, int, int, int);
+}
+public final class com.mapconductor.compose.map.MapViewLifecycleOwner implements androidx.lifecycle.LifecycleOwner {
+  public com.mapconductor.compose.map.MapViewLifecycleOwner(androidx.lifecycle.Lifecycle);
+  public androidx.lifecycle.Lifecycle getLifecycle();
+  public final void destroy();
 }
 public final class com.mapconductor.compose.marker.LocalMarkerCollectorKt {
   public static final androidx.compose.runtime.ProvidableCompositionLocal<com.mapconductor.core.OverlayCollectorInterface<com.mapconductor.core.marker.MarkerState>> getLocalMarkerCollector();

src/main/java/com/mapconductor/compose/map/MapViewBase.kt

@@ -16,6 +16,7 @@ import androidx.compose.runtime.key
 import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.runtime.setValue
 import androidx.compose.runtime.snapshotFlow
 import androidx.compose.ui.Alignment
@@ -60,10 +61,12 @@ import com.mapconductor.core.map.MapDesignTypeInterface
 import com.mapconductor.core.map.MapOverlayRegistry
 import com.mapconductor.core.map.MapPaddings
 import com.mapconductor.core.map.MapViewHolderInterface
+import com.mapconductor.core.map.MapViewState
 import com.mapconductor.core.map.MapViewStateInterface
 import com.mapconductor.core.map.MapViewStyle
 import com.mapconductor.core.map.MapViewStyleEffect
 import com.mapconductor.core.map.ScreenProjectionRequirement
+import com.mapconductor.core.map.VectorStyleMutationSupportKey
 import com.mapconductor.core.map.VectorStyleSupportKey
 import com.mapconductor.core.map.resolveMapAttributions
 import com.mapconductor.core.marker.MarkerCapableInterface
@@ -109,11 +112,23 @@ fun <
     customDisposableEffect: (@Composable (InitState, Ref<SpecificHolder>) -> Unit)? = null,
     style: MapViewStyle? = null,
     onStyleDiagnostics: ((List<String>) -> Unit)? = null,
+    /**
+     * Releases the SDK's map view after the map has left the composition.
+     *
+     * Called once, on the main thread, after the controller has been destroyed
+     * and the view has been taken out of its parent. Every provider has to pass
+     * this: a map view nobody destroys keeps its render thread (and through its
+     * listeners the controller with every overlay) alive until the process runs
+     * out of memory. It runs for a configuration change too -- the old Activity's
+     * view is never shown again; what survives a rotation is the state.
+     */
+    onDestroyMapView: ((ActualMapView) -> Unit)? = null,
     content: (@Composable SpecificScope.() -> Unit)? = null,
 ) {
     ResourceProvider.init(LocalContext.current)
     val mapViewRef = remember { Ref<ActualMapView>() }
     val controllerRef = remember { Ref<SpecificController>() }
+    val controllerStateRef = remember { Ref<SpecificState>() }
     val holderRef = remember { Ref<SpecificHolder>() }
     var initState by remember { mutableStateOf<InitState>(InitState.NotStarted) }
     val bubbles by scope.bubbleFlow.collectAsState()
@@ -132,6 +147,9 @@ fun <
         onDispose {
             state.serviceRegistry.remove(MarkerRenderingSupportKey)
             state.serviceRegistry.remove(VectorStyleSupportKey)
+            // Mapbox / MapLibre register a store whose callback holds the map's
+            // holder and controller; left here it keeps the whole old map.
+            state.serviceRegistry.remove(VectorStyleMutationSupportKey)
         }
     }
 
@@ -441,6 +459,11 @@ fun <
         mapViewRef.value?.let { mapView ->
             val holder = holderProvider(mapView)
             holderRef.value = holder
+            // The provider attaches the controller to the state it was composed
+            // with now, which need not be the one this effect first saw: an app
+            // can hand the same MapViewBase another state (the example does, for
+            // a frame, between ArcGIS 3D and 2D).
+            controllerStateRef.value = state
             controllerRef.value = controllerProvider(holder)
             initState = InitState.MapCreating
         }
@@ -450,16 +473,60 @@ fun <
     LaunchedEffect(state.uiSettings, initState) {
         controllerRef.value?.applyUISettings(state.uiSettings)
     }
+    val currentOnDestroyMapView by rememberUpdatedState(onDestroyMapView)
+    val currentState by rememberUpdatedState(state)
     DisposableEffect(Unit) {
-        // Setup logic when the composable enters the screen
-
         onDispose {
-            controllerRef.value?.destroy()
+            // Overlays first, while the map they live on is still alive.
+            controllerRef.value?.let { controller ->
+                controller.destroy()
+                // The state outlives the map (apps keep it, e.g. one per provider
+                // while switching); without this it holds the destroyed controller
+                // and with it every overlay of the old map. Each state lets go only
+                // if this controller is the one attached to it.
+                setOf(controllerStateRef.value, currentState, state).forEach {
+                    (it as? MapViewState<*>)?.detachController(controller)
+                }
+            }
+            controllerStateRef.value = null
+            controllerRef.value = null
+            holderRef.value = null
+            val mapView = mapViewRef.value ?: return@onDispose
+            mapViewRef.value = null
+            destroyMapView(
+                mapView = mapView,
+                provider = state::class.java.simpleName,
+                onDestroyMapView = currentOnDestroyMapView,
+            )
         }
     }
     customDisposableEffect?.invoke(initState, holderRef)
 }
 
+private fun <ActualMapView : View> destroyMapView(
+    mapView: ActualMapView,
+    provider: String,
+    onDestroyMapView: ((ActualMapView) -> Unit)?,
+) {
+    if (onDestroyMapView == null) {
+        // Not passing it compiles and the map still draws; only the memory goes.
+        Log.w(
+            "MapConductor",
+            "$provider does not release its map view (MapViewBase onDestroyMapView is null); " +
+                "the old map view and its controller stay in memory",
+        )
+        return
+    }
+    // Detach before destroying, so the view's surface goes away while the SDK
+    // renderer is still there to handle it.
+    (mapView.parent as? ViewGroup)?.removeView(mapView)
+    try {
+        onDestroyMapView(mapView)
+    } catch (e: Exception) {
+        Log.w("MapConductor", "$provider: releasing the map view failed", e)
+    }
+}
+
 @Composable
 private fun BasicMessage(text: String) {
     Box(

src/main/java/com/mapconductor/compose/map/MapViewLifecycleOwner.kt

@@ -0,0 +1,47 @@
+package com.mapconductor.compose.map
+
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LifecycleEventObserver
+import androidx.lifecycle.LifecycleOwner
+import androidx.lifecycle.LifecycleRegistry
+
+/**
+ * A lifecycle for one map view that follows the host's but can end on its own.
+ *
+ * Some SDKs tie what a map view holds to the [LifecycleOwner] it is given or
+ * finds in the view tree -- coroutines in its `lifecycleScope`, observers,
+ * component callbacks -- and let go of it only when that lifecycle is
+ * destroyed. Given the Activity's, a map the user switched away from stays
+ * until the Activity goes. Give the view this instead and call [destroy] from
+ * `onDestroyMapView`.
+ *
+ * Until [destroy] it moves with [host] (it catches up on creation). [destroy]
+ * stops following and moves it down to DESTROYED, which the SDK sees as
+ * ON_PAUSE / ON_STOP / ON_DESTROY as far as it had got. Calling it again, or
+ * after the host was destroyed, does nothing.
+ */
+class MapViewLifecycleOwner(
+    private val host: Lifecycle,
+) : LifecycleOwner {
+    private val registry = LifecycleRegistry(this)
+
+    private val follow =
+        LifecycleEventObserver { _, event ->
+            if (registry.currentState != Lifecycle.State.DESTROYED) registry.handleLifecycleEvent(event)
+        }
+
+    init {
+        host.addObserver(follow)
+    }
+
+    override val lifecycle: Lifecycle
+        get() = registry
+
+    fun destroy() {
+        host.removeObserver(follow)
+        // INITIALIZED cannot go straight to DESTROYED; nothing was started then.
+        if (registry.currentState.isAtLeast(Lifecycle.State.CREATED)) {
+            registry.currentState = Lifecycle.State.DESTROYED
+        }
+    }
+}