Let a state drop a destroyed controller; pool tile scratch buffers

api/android-sdk-core.api

@@ -1095,6 +1095,7 @@ public abstract class com.mapconductor.core.map.MapViewState<ActualMapDesignType
   public com.mapconductor.core.map.MapCameraPosition getCameraPosition();
   protected final void attachController(com.mapconductor.core.controller.MapViewControllerInterface, boolean);
   protected final void detachController();
+  public final void detachController(com.mapconductor.core.controller.MapViewControllerInterface);
   protected final void setCameraPositionInternal(com.mapconductor.core.map.MapCameraPosition);
   public void moveCameraTo(com.mapconductor.core.features.GeoPoint, java.lang.Long);
   public void moveCameraTo(com.mapconductor.core.map.MapCameraPosition, java.lang.Long);
@@ -2928,6 +2929,18 @@ public final class com.mapconductor.core.tileserver.LocalTileServer$TileOutcome$
 public final class com.mapconductor.core.tileserver.LocalTileServerKt {
   public static final com.mapconductor.core.tileserver.TileCoordinate parseTileCoordinate(java.lang.String);
 }
+public final class com.mapconductor.core.tileserver.ScratchPool<T> {
+  public static final com.mapconductor.core.tileserver.ScratchPool$Companion Companion;
+  public com.mapconductor.core.tileserver.ScratchPool(int);
+  public com.mapconductor.core.tileserver.ScratchPool(int, int, kotlin.jvm.internal.DefaultConstructorMarker);
+  public final T acquire(kotlin.jvm.functions.Function1<? super T, java.lang.Boolean>);
+  public final boolean release(T);
+  public final int getSize();
+  public com.mapconductor.core.tileserver.ScratchPool();
+}
+public final class com.mapconductor.core.tileserver.ScratchPool$Companion {
+  public final int getDEFAULT_CAPACITY();
+}
 public final class com.mapconductor.core.tileserver.TileCoordinate {
   public com.mapconductor.core.tileserver.TileCoordinate(int, int);
   public final int getY();

src/main/java/com/mapconductor/core/circle/CircleController.kt

@@ -35,7 +35,8 @@ abstract class CircleController<ActualCircle>(
 
             data.forEach { state ->
                 if (previous.contains(state.id)) {
-                    val prevEntity = circleManager.getEntity(state.id)!!
+                    // Gone only if destroy() cleared the manager meanwhile; nothing left to update.
+                    val prevEntity = circleManager.getEntity(state.id) ?: return@withPermit
                     if (state.fingerPrint() == prevEntity.fingerPrint) {
                         // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する
                         // （react-sdk と同じ。composition ごとに全オーバーレイへ onChange を
@@ -158,7 +159,9 @@ abstract class CircleController<ActualCircle>(
     override fun find(position: GeoPointInterface): CircleEntityInterface<ActualCircle>? = circleManager.find(position)
 
     override fun destroy() {
-        // No native resources to clean up for circles
+        // A destroyed controller can still be referenced (by its map controller,
+        // or a state that has not let go yet); without this it keeps every entity.
+        circleManager.clear()
     }
 
     override fun has(id: String): Boolean = circleManager.hasEntity(id)

src/main/java/com/mapconductor/core/groundimage/GroundImageController.kt

@@ -36,7 +36,8 @@ abstract class GroundImageController<ActualGroundImage>(
 
             data.forEach { state ->
                 if (previous.contains(state.id)) {
-                    val prevEntity = groundImageManager.getEntity(state.id)!!
+                    // Gone only if destroy() cleared the manager meanwhile; nothing left to update.
+                    val prevEntity = groundImageManager.getEntity(state.id) ?: return@withPermit
                     if (state.fingerPrint() == prevEntity.fingerPrint) {
                         // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する（react-sdk と同じ）。
                         groundImageManager
@@ -157,7 +158,9 @@ abstract class GroundImageController<ActualGroundImage>(
             .find(position)
 
     override fun destroy() {
-        // No native resources to clean up
+        // A destroyed controller can still be referenced (by its map controller,
+        // or a state that has not let go yet); without this it keeps every entity.
+        groundImageManager.clear()
     }
 
     override fun has(id: String): Boolean = groundImageManager.hasEntity(id)

src/main/java/com/mapconductor/core/map/MapViewState.kt

@@ -148,6 +148,23 @@ abstract class MapViewState<ActualMapDesignType>(
         this.attachedMapController = null
     }
 
+    /**
+     * 破棄した [controller] を手放す。地図が壊されたあとで呼ぶ。
+     *
+     * state は地図より長生きする（アプリが保持し、プロバイダを切り替えても残る）。
+     * ここで手放さないと、state が最後のコントローラ（とその全オーバーレイ）を
+     * 抱えたままになる。Compose では `MapViewBase` が地図をコンポジションから
+     * 外したときに呼ぶので、プロバイダが呼ぶ必要はない。
+     *
+     * その間に別のコントローラが接続されていれば何もしない（新しい地図を
+     * 古い地図の後始末で切り離さないため）。
+     */
+    fun detachController(controller: MapViewControllerInterface) {
+        if (this.attachedMapController === controller) {
+            this.attachedMapController = null
+        }
+    }
+
     /**
      * 地図から通知された現在のカメラを保持する（地図を動かさない）。
      *

src/main/java/com/mapconductor/core/marker/MarkerTileRenderer.kt

@@ -6,6 +6,7 @@ import androidx.core.graphics.createBitmap
 import com.mapconductor.core.ResourceProvider
 import com.mapconductor.core.features.GeoPoint
 import com.mapconductor.core.features.GeoRectBounds
+import com.mapconductor.core.tileserver.ScratchPool
 import com.mapconductor.core.tileserver.TilePngEncoder
 import com.mapconductor.core.tileserver.TileProviderInterface
 import com.mapconductor.core.tileserver.TileRequest
@@ -686,10 +687,10 @@ class MarkerTileRenderer<ActualMarker>(
     // ByteArrayOutputStream starts at 32 bytes and doubles its backing array on every
     // compress() call until it reaches the tile's steady-state size (often hundreds of KB
     // for dense tiles), so a new instance per call re-triggers that regrowth every time.
-    // Keeping one per thread (reset, not recreated, before each use) lets the backing
-    // array settle at its steady-state size and be reused across tiles.
-    private val tileByteStream =
-        ThreadLocal.withInitial { ByteArrayOutputStream(16 * 1024) }
+    // Keeping a few (reset, not recreated, before each use) lets the backing array
+    // settle at its steady-state size and be reused across tiles. A pool, not one per
+    // thread: map SDK threads that outlive their map would keep theirs (ScratchPool).
+    private val tileByteStreams = ScratchPool<ByteArrayOutputStream>()
 
     private fun bitmapToByteArray(bitmap: Bitmap): ByteArray {
         // Rust first: it is 4-6x faster than Bitmap.compress on the tiles this
@@ -698,15 +699,17 @@ class MarkerTileRenderer<ActualMarker>(
         // declined the bitmap, and the platform encoder takes over.
         TilePngEncoder.encode(bitmap)?.let { return it }
 
-        // ThreadLocal.get() is a Java generic method, so Kotlin sees its return type as
-        // nullable even though withInitial() guarantees a value; !! is safe here.
-        val outputStream = tileByteStream.get()!!
-        outputStream.reset()
-        bitmap.compress(Bitmap.CompressFormat.PNG, 100, outputStream)
-        // toByteArray() still copies out a right-sized array: callers cache this array or
-        // hand it to a socket write, both of which need a stable array that isn't mutated
-        // by the next renderTile() call on this thread.
-        return outputStream.toByteArray()
+        val outputStream = tileByteStreams.acquire() ?: ByteArrayOutputStream(16 * 1024)
+        try {
+            outputStream.reset()
+            bitmap.compress(Bitmap.CompressFormat.PNG, 100, outputStream)
+            // toByteArray() still copies out a right-sized array: callers cache this array or
+            // hand it to a socket write, both of which need a stable array that isn't mutated
+            // by the next renderTile() call.
+            return outputStream.toByteArray()
+        } finally {
+            tileByteStreams.release(outputStream)
+        }
     }
 
     private fun acquireBitmap(size: Int): Bitmap {

src/main/java/com/mapconductor/core/polygon/PolygonController.kt

@@ -35,7 +35,8 @@ abstract class PolygonController<ActualPolygon>(
 
             data.forEach { state ->
                 if (previous.contains(state.id)) {
-                    val prevEntity = polygonManager.getEntity(state.id)!!
+                    // Gone only if destroy() cleared the manager meanwhile; nothing left to update.
+                    val prevEntity = polygonManager.getEntity(state.id) ?: return@withPermit
                     if (state.fingerPrint() == prevEntity.fingerPrint) {
                         // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する（react-sdk と同じ）。
                         polygonManager.registerEntity(PolygonEntity(state = state, polygon = prevEntity.polygon))
@@ -162,7 +163,9 @@ abstract class PolygonController<ActualPolygon>(
             .find(position)
 
     override fun destroy() {
-        // No native resources to clean up for polygons
+        // A destroyed controller can still be referenced (by its map controller,
+        // or a state that has not let go yet); without this it keeps every entity.
+        polygonManager.clear()
     }
 
     override fun has(id: String): Boolean = polygonManager.hasEntity(id)

src/main/java/com/mapconductor/core/polyline/PolylineController.kt

@@ -39,7 +39,8 @@ abstract class PolylineController<ActualPolyline>(
 
             data.forEach { state ->
                 if (previous.contains(state.id)) {
-                    val prevEntity = polylineManager.getEntity(state.id)!!
+                    // Gone only if destroy() cleared the manager meanwhile; nothing left to update.
+                    val prevEntity = polylineManager.getEntity(state.id) ?: return@withPermit
                     if (state.fingerPrint() == prevEntity.fingerPrint) {
                         // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する（react-sdk と同じ）。
                         polylineManager.registerEntity(PolylineEntity(state = state, polyline = prevEntity.polyline))
@@ -169,7 +170,9 @@ abstract class PolylineController<ActualPolyline>(
     }
 
     override fun destroy() {
-        // No native resources to clean up for polylines
+        // A destroyed controller can still be referenced (by its map controller,
+        // or a state that has not let go yet); without this it keeps every entity.
+        polylineManager.clear()
     }
 
     override fun has(id: String): Boolean = polylineManager.hasEntity(id)

src/main/java/com/mapconductor/core/raster/RasterLayerController.kt

@@ -282,7 +282,9 @@ abstract class RasterLayerController<ActualLayer : Any>(
     }
 
     override fun destroy() {
-        // No native resources to clean up
+        // A destroyed controller can still be referenced (by its map controller,
+        // or a state that has not let go yet); without this it keeps every entity.
+        rasterLayerManager.clear()
     }
 
     override fun has(id: String): Boolean = rasterLayerManager.hasEntity(id)

src/main/java/com/mapconductor/core/tileserver/ScratchPool.kt

@@ -0,0 +1,49 @@
+package com.mapconductor.core.tileserver
+
+/**
+ * A few reusable scratch objects (pixel buffers, bitmaps, output streams) for
+ * code that renders tiles on threads it does not own.
+ *
+ * Use this instead of a `ThreadLocal`. Tile callbacks run on the map SDK's
+ * threads, and some SDKs start new ones for every map and never stop them:
+ * Google Maps keeps an `androidmapsapi-tileoverlay` pool of three live threads
+ * per map it ever created. A thread-local scratch pins one object on each such
+ * thread for good, so every map switch added a few megabytes that nothing
+ * could free. A pool holds at most [capacity] objects whatever the threads do.
+ *
+ * [acquire] hands out a pooled object or null (the caller makes a fresh one);
+ * [release] takes it back, or drops it when the pool is full.
+ */
+internal class ScratchPool<T : Any>(
+    private val capacity: Int = DEFAULT_CAPACITY,
+) {
+    private val items = ArrayDeque<T>(capacity)
+
+    /** A pooled object that [accept] takes, or null. Others are dropped. */
+    fun acquire(accept: (T) -> Boolean = { true }): T? {
+        synchronized(items) {
+            while (items.isNotEmpty()) {
+                val item = items.removeLast()
+                if (accept(item)) return item
+            }
+        }
+        return null
+    }
+
+    /** Returns [item]. False if the pool was full and did not keep it. */
+    fun release(item: T): Boolean =
+        synchronized(items) {
+            if (items.size >= capacity) return false
+            items.addLast(item)
+            true
+        }
+
+    /** Pooled objects right now. For tests. */
+    val size: Int
+        get() = synchronized(items) { items.size }
+
+    companion object {
+        /** About how many tiles are drawn at the same time. */
+        val DEFAULT_CAPACITY: Int = Runtime.getRuntime().availableProcessors().coerceIn(2, 8)
+    }
+}

src/main/java/com/mapconductor/core/tileserver/TilePngEncoder.kt

@@ -23,13 +23,15 @@ import android.graphics.Bitmap
  */
 object TilePngEncoder {
     /**
-     * Scratch pixel buffer, one per calling thread.
+     * Scratch pixel buffers, shared by the calling threads.
      *
-     * Tile rendering runs concurrently across the tile server's threads, and a
-     * tile at this SDK's densities is around 7 MB of RGBA — allocating that per
-     * call would cost more than the encoding it is meant to speed up.
+     * Tile rendering runs concurrently across several threads, and a tile at
+     * this SDK's densities is around 7 MB of RGBA — allocating that per call
+     * would cost more than the encoding it is meant to speed up. Not one per
+     * thread: the callers include map SDK threads that outlive their map (see
+     * [ScratchPool]), and each would keep its buffer for good.
      */
-    private val scratch = ThreadLocal<ByteBuffer>()
+    private val scratch = ScratchPool<ByteBuffer>()
 
     /** False where the native library is missing for this device's ABI. */
     val isAvailable: Boolean =
@@ -55,16 +57,17 @@ object TilePngEncoder {
         if (width <= 0 || height <= 0) return null
         val needed = width * height * 4
 
-        var buffer = scratch.get()
-        if (buffer == null || buffer.capacity() < needed) {
-            buffer = ByteBuffer.allocateDirect(needed).order(ByteOrder.nativeOrder())
-            scratch.set(buffer)
+        val buffer =
+            scratch.acquire { it.capacity() >= needed }
+                ?: ByteBuffer.allocateDirect(needed).order(ByteOrder.nativeOrder())
+        try {
+            buffer.clear()
+            bitmap.copyPixelsToBuffer(buffer)
+            buffer.rewind()
+            return encode(buffer, width, height, premultiplied = bitmap.isPremultiplied)
+        } finally {
+            scratch.release(buffer)
         }
-        buffer.clear()
-        bitmap.copyPixelsToBuffer(buffer)
-        buffer.rewind()
-
-        return encode(buffer, width, height, premultiplied = bitmap.isPremultiplied)
     }
 
     /**

src/test/java/com/mapconductor/core/map/MapViewStateBaseTest.kt

@@ -258,4 +258,31 @@ class MapViewStateBaseTest {
         assertTrue(controller.moved.isEmpty())
         assertNull(state.getMapViewHolder())
     }
+
+    @Test
+    fun `破棄したコントローラを渡すと手放す`() {
+        // アプリが state を持ち続けても、壊した地図のコントローラ（と全オーバーレイ）を
+        // 抱えたままにしない。
+        val controller = FakeController()
+        val state = TestState().apply { attach(controller) }
+
+        state.detachController(controller)
+
+        assertNull(state.getMapViewHolder())
+    }
+
+    @Test
+    fun `別のコントローラが接続済みなら古いものを渡しても外さない`() {
+        val old = FakeController()
+        val current = FakeController()
+        val state = TestState().apply { attach(old) }
+        state.attach(current)
+
+        state.detachController(old)
+
+        assertSame(current.holder, state.getMapViewHolder())
+        current.moved.clear()
+        state.moveCameraTo(cameraAt(1.0, 2.0), durationMillis = 0)
+        assertEquals(1, current.moved.size)
+    }
 }

src/test/java/com/mapconductor/core/tileserver/ScratchPoolTest.kt

@@ -0,0 +1,42 @@
+package com.mapconductor.core.tileserver
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertSame
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class ScratchPoolTest {
+    @Test
+    fun `holds no more than its capacity, however many threads used it`() {
+        // A thread-local kept one per thread, and map SDK threads that never stop
+        // (Google's tile-overlay pool, three per map) kept theirs for good.
+        val pool = ScratchPool<ByteArray>(capacity = 2)
+        val released = (1..10).map { pool.release(ByteArray(1)) }
+
+        assertEquals(listOf(true, true) + List(8) { false }, released)
+        assertEquals(2, pool.size)
+    }
+
+    @Test
+    fun `hands back what was released`() {
+        val pool = ScratchPool<ByteArray>(capacity = 2)
+        val item = ByteArray(4)
+        pool.release(item)
+
+        assertSame(item, pool.acquire())
+        assertNull(pool.acquire())
+    }
+
+    @Test
+    fun `drops pooled objects the caller cannot use`() {
+        val pool = ScratchPool<ByteArray>(capacity = 2)
+        pool.release(ByteArray(4))
+
+        assertNull(pool.acquire { it.size >= 8 })
+        assertEquals(0, pool.size)
+        assertTrue(pool.release(ByteArray(8)))
+        assertFalse(pool.acquire { it.size >= 8 } == null)
+    }
+}