From 64cbbe4b354b879a9473d61c24e408324e0ab436 Mon Sep 17 00:00:00 2001 From: Masashi Katsumata Date: Sat, 7 Nov 2026 22:21:12 -0800 Subject: [PATCH] Clear StrategyMarkerController in the same order as the other controllers clear() only called strategy.clear(), which empties the manager. It never called onRemove or onPostProcess, unlike AbstractMarkerController and the five shape controllers. The GL renderers (MapLibre, Mapbox, Mappls, MapTiler) leave the map alone in onRemove and redraw the manager in onPostProcess, so markers drawn through a strategy stayed on the map after clear(). clear() now runs onRemove(everything), strategy.clear() (so the strategy's own reset, such as the cluster state, is kept), then onPostProcess, inside the permit. The permit matters here: this class is not an actor, and pendingStates (a class-holding field) may only be touched while holding it. Markers queued before the first camera are dropped as well; they would otherwise be drawn on the first camera event after clear(). Nothing in the SDK calls this clear() today (clearOverlays() clears the providers' own controllers; HERE's and ArcGIS's strategy controllers are not included). StrategyMarkerClearOrderTests fails on the old clear() and passes now. Same test in android-sdk-core and js-sdk-core. --- .../marker/StrategyMarkerController.swift | 21 +++- .../StrategyMarkerClearOrderTests.swift | 118 ++++++++++++++++++ 2 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 Tests/MapConductorCoreTests/StrategyMarkerClearOrderTests.swift diff --git a/Sources/MapConductorCore/marker/StrategyMarkerController.swift b/Sources/MapConductorCore/marker/StrategyMarkerController.swift index d1653e4..0a7d32c 100644 --- a/Sources/MapConductorCore/marker/StrategyMarkerController.swift +++ b/Sources/MapConductorCore/marker/StrategyMarkerController.swift @@ -138,8 +138,27 @@ where Strategy.ActualMarker == ActualMarker, Renderer.ActualMarker == ActualMark } } + /// 他の 5 種類・``AbstractMarkerController`` の clear() と同じく + /// 「onRemove(全部) → 空にする → onPostProcess」の順に呼ぶ。 + /// + /// 以前は `strategy.clear()` で manager を空にするだけだった。GL 系の描画担当は + /// onRemove では地図に触らず onPostProcess で manager から描き直すので、呼ばないと + /// clear() の後もマーカーが地図に残る。空にするのは `strategy.clear()` のままにして、 + /// ストラテジ固有の後片付け（クラスタの状態など）を保つ。 + /// + /// **permit の中で行う。** `pendingStates` を触るため（上の ★ を参照）と、 + /// add / カメラ変更のストラテジ処理と交錯させないため。 public func clear() async { - strategy.clear() + await semaphore.withPermit { + let entities = markerManager.allEntities() + if !entities.isEmpty { + await renderer.onRemove(data: entities) + } + strategy.clear() + // 最初のカメラ待ちで保留していたものも消す。残すと次のカメラで描かれてしまう。 + pendingStates = nil + await renderer.onPostProcess() + } } public func find(position: GeoPointProtocol) -> MarkerEntity? { diff --git a/Tests/MapConductorCoreTests/StrategyMarkerClearOrderTests.swift b/Tests/MapConductorCoreTests/StrategyMarkerClearOrderTests.swift new file mode 100644 index 0000000..6f88800 --- /dev/null +++ b/Tests/MapConductorCoreTests/StrategyMarkerClearOrderTests.swift @@ -0,0 +1,118 @@ +import XCTest + +@testable import MapConductorCore + +/// ``StrategyMarkerController/clear()`` は他のコントローラの clear() と同じく +/// 「onRemove(全部) → 空にする → onPostProcess」で、何も描かれていない状態で終わること。 +/// +/// 以前は `strategy.clear()` で manager を空にするだけで、onRemove も onPostProcess も +/// 呼ばなかった。GL 系の描画担当は onPostProcess で manager から描き直すので、 +/// clear() の後もマーカーが地図に残る。android-sdk-core / js-sdk-core の同名テストと同じ。 +final class StrategyMarkerClearOrderTests: XCTestCase { + private let camera = MapCameraPosition( + position: GeoPoint(latitude: 35.0, longitude: 139.0), + zoom: 10, + visibleRegion: VisibleRegion(bounds: GeoRectBounds( + southWest: GeoPoint(latitude: 34.0, longitude: 138.0), + northEast: GeoPoint(latitude: 36.0, longitude: 140.0) + )) + ) + + private func markers() -> [MarkerState] { + [ + MarkerState(position: GeoPoint(latitude: 35.0, longitude: 139.0), id: "m1"), + MarkerState(position: GeoPoint(latitude: 35.1, longitude: 139.1), id: "m2"), + ] + } + + /// GL 系の描画担当: onRemove では地図に触らず、onPostProcess で manager の全部を描き直す。 + @MainActor + private final class GlRenderer: MarkerOverlayRendererProtocol { + typealias ActualMarker = String + let manager: MarkerManager + var drawn: [String] = [] + var removed: [String] = [] + var animateStartListener: OnMarkerEventHandler? + var animateEndListener: OnMarkerEventHandler? + init(_ manager: MarkerManager) { self.manager = manager } + func onAdd(data: [MarkerOverlayAddParams]) async -> [String?] { data.map { $0.state.id } } + func onChange(data: [MarkerOverlayChangeParams]) async -> [String?] { data.map { $0.current.state.id } } + func onRemove(data: [MarkerEntity]) async { removed += data.map { $0.state.id } } + func onAnimate(entity: MarkerEntity) async {} + func onPostProcess() async { drawn = manager.allEntities().map { $0.state.id }.sorted() } + } + + /// 渡されたマーカーを全部描くストラテジ。描いた状態は manager へ直接登録する。 + private final class DrawAllStrategy: MarkerRenderingStrategyProtocol { + typealias ActualMarker = String + let markerManager = MarkerManager() + var cleared = 0 + + func clear() { + cleared += 1 + markerManager.clear() + } + + func onAdd( + data: [MarkerState], + viewport _: GeoRectBounds, + renderer: Renderer + ) async -> Bool where Renderer.ActualMarker == String { + for state in data { + markerManager.registerEntity(MarkerEntity(marker: state.id, state: state, isRendered: true)) + } + await renderer.onPostProcess() + return true + } + + func onUpdate( + state _: MarkerState, + viewport _: GeoRectBounds, + renderer _: Renderer + ) async -> Bool where Renderer.ActualMarker == String { true } + + func onCameraChanged( + mapCameraPosition _: MapCameraPosition, + renderer _: Renderer + ) async where Renderer.ActualMarker == String {} + } + + @MainActor + func testClearLeavesNothingDrawnAndKeepsTheStrategyReset() async { + let strategy = DrawAllStrategy() + let renderer = GlRenderer(strategy.markerManager) + let controller = StrategyMarkerController(strategy: strategy, renderer: renderer) + await controller.onCameraChanged(mapCameraPosition: camera) + await controller.add(data: markers()) + XCTAssertEqual(renderer.drawn, ["m1", "m2"]) + + await controller.clear() + + XCTAssertEqual(renderer.removed.sorted(), ["m1", "m2"]) + XCTAssertEqual(strategy.cleared, 1) + XCTAssertEqual(renderer.drawn, []) + } + + @MainActor + func testClearDropsMarkersWaitingForTheFirstCamera() async { + let strategy = DrawAllStrategy() + let renderer = GlRenderer(strategy.markerManager) + let controller = StrategyMarkerController(strategy: strategy, renderer: renderer) + await controller.add(data: markers()) // カメラ前なので保留 + await controller.clear() + await controller.onCameraChanged(mapCameraPosition: camera) + XCTAssertEqual(renderer.drawn, []) + } + + @MainActor + func testMarkersSentAgainAfterClearAreDrawnAgain() async { + let strategy = DrawAllStrategy() + let renderer = GlRenderer(strategy.markerManager) + let controller = StrategyMarkerController(strategy: strategy, renderer: renderer) + await controller.onCameraChanged(mapCameraPosition: camera) + await controller.add(data: markers()) + await controller.clear() + await controller.add(data: markers()) + XCTAssertEqual(renderer.drawn, ["m1", "m2"]) + } +}