diff --git a/src/marker/StrategyMarkerController.ts b/src/marker/StrategyMarkerController.ts index 883881f..acb7f9d 100644 --- a/src/marker/StrategyMarkerController.ts +++ b/src/marker/StrategyMarkerController.ts @@ -122,16 +122,21 @@ export class StrategyMarkerController } async clear(): Promise { - // Release the renderer's per-marker resources (icon images / sprites) - // before dropping the entities. strategy.clear() only clears the manager's - // bookkeeping and never calls onRemove, so without this the GL providers' - // reference-counted icon images and Azure's sprite images would stay - // registered on the map after an explicit clear() while the map is alive. - const entities = this.strategy.markerManager.allEntities(); - if (entities.length > 0) { - await this.renderer.onRemove(entities); - } - this.strategy.clear(); + // Same order as the other controllers' clear(): onRemove(everything) → empty → onPostProcess. + // onRemove releases the renderer's per-marker resources (the GL providers' reference-counted + // icon images, Azure's sprites); strategy.clear() only drops the bookkeeping. The GL renderers + // draw from the manager in onPostProcess, so without it the cleared markers stayed on the map. + // Under the same lock as add/update so a clear cannot interleave with a strategy pass. + await this.semaphore.withLock(async () => { + const entities = this.strategy.markerManager.allEntities(); + if (entities.length > 0) { + await this.renderer.onRemove(entities); + } + this.strategy.clear(); + // Markers queued before the first camera fix were cleared too; don't add them later. + this.pendingStates = null; + await this.renderer.onPostProcess(); + }); } getEntity(id: string): MarkerEntity | null { diff --git a/test/StrategyMarkerClearOrder.test.mjs b/test/StrategyMarkerClearOrder.test.mjs new file mode 100644 index 0000000..3a99a01 --- /dev/null +++ b/test/StrategyMarkerClearOrder.test.mjs @@ -0,0 +1,90 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import { + MarkerManager, + StrategyMarkerController, + createGeoPoint, + createGeoRectBounds, + createMarkerEntity, + createMarkerState, +} from '../dist/index.mjs'; + +/** + * StrategyMarkerController.clear() must leave nothing drawn, like the other controllers' + * clear() (OverlayClearOrder.test.mjs): onRemove(everything) → empty the manager → + * onPostProcess. It used to call onRemove and empty the manager but never onPostProcess, + * so a GL renderer, which redraws from the manager in onPostProcess, kept showing the + * markers. Same test on android-sdk-core and ios-sdk-core. + */ + +const bounds = createGeoRectBounds({ + southWest: createGeoPoint({ latitude: 34, longitude: 138 }), + northEast: createGeoPoint({ latitude: 36, longitude: 140 }), +}); +const camera = { position: createGeoPoint({ latitude: 35, longitude: 139 }), zoom: 10, visibleRegion: { bounds } }; + +/** GL-style renderer: onRemove touches nothing, onPostProcess redraws the manager. */ +class GlRenderer { + constructor(manager) { this.manager = manager; this.drawn = []; this.removed = []; this.holder = null; } + async onAdd(data) { return data.map((p) => p.state.id); } + async onChange(data) { return data.map((p) => p.current.state.id); } + async onRemove(data) { this.removed.push(...data.map((e) => e.state.id)); } + async onAnimate() {} + async onPostProcess() { this.drawn = this.manager.allEntities().map((e) => e.state.id).sort(); } +} + +/** Draws every marker it is given, like the default strategies. */ +class DrawAllStrategy { + constructor() { this.markerManager = MarkerManager.defaultManager(); this.cleared = 0; } + clear() { this.cleared++; this.markerManager.clear(); } + async onAdd({ data, renderer }) { + const markers = await renderer.onAdd(data.map((state) => ({ state, bitmapIcon: null }))); + data.forEach((state, i) => this.markerManager.registerEntity(createMarkerEntity({ state, marker: markers[i] }))); + await renderer.onPostProcess(); + return true; + } + async onUpdate() { return true; } + async onCameraChanged() {} +} + +function fixture() { + const strategy = new DrawAllStrategy(); + const renderer = new GlRenderer(strategy.markerManager); + const controller = new StrategyMarkerController({ strategy, renderer }); + return { strategy, renderer, controller }; +} + +const markers = () => [ + createMarkerState({ id: 'm1', position: createGeoPoint({ latitude: 35, longitude: 139 }) }), + createMarkerState({ id: 'm2', position: createGeoPoint({ latitude: 35.1, longitude: 139.1 }) }), +]; + +test('clear() removes, empties the strategy, then post-processes, so nothing stays drawn', async () => { + const f = fixture(); + await f.controller.onCameraChanged(camera); + await f.controller.add(markers()); + assert.deepEqual(f.renderer.drawn, ['m1', 'm2']); + + await f.controller.clear(); + assert.deepEqual(f.renderer.removed.sort(), ['m1', 'm2'], 'onRemove gets every entity'); + assert.equal(f.strategy.cleared, 1, "the strategy's own clear() still runs"); + assert.deepEqual(f.renderer.drawn, [], 'nothing is drawn after clear()'); +}); + +test('clear() drops markers still waiting for the first camera', async () => { + const f = fixture(); + await f.controller.add(markers()); // no camera yet: queued + await f.controller.clear(); + await f.controller.onCameraChanged(camera); + assert.deepEqual(f.renderer.drawn, [], 'the queued markers must not appear after clear()'); +}); + +test('markers sent again after clear() are drawn again', async () => { + const f = fixture(); + await f.controller.onCameraChanged(camera); + await f.controller.add(markers()); + await f.controller.clear(); + await f.controller.add(markers()); + assert.deepEqual(f.renderer.drawn, ['m1', 'm2']); +});