From f3e475cc2cd5431e1269b7080c09198604efd49d Mon Sep 17 00:00:00 2001 From: Masashi Katsumata Date: Sat, 7 Nov 2026 19:07:15 -0800 Subject: [PATCH 1/2] Drop the clear() overrides on polyline and circle They redrew after super.clear() because js-sdk-core's clear() called onPostProcess before emptying the manager (polyline) or not at all (circle), so this provider's redraw-from-manager renderer kept drawing what clear() removed. js-sdk-core now runs onRemove, manager.clear(), onPostProcess, which redraws an empty layer. clear() is still there, inherited from the core controller with the same signature; the API snapshot only stops listing the override. Requires the js-sdk-core release with that change (MapConductor/js-sdk-core fix/overlay-clear-order). Against an older core, clearOverlays() leaves polylines and circles drawn until something else redraws them. --- api/react-for-mappls.api.d.ts | 2 -- src/circle/MapplsCircleController.ts | 5 ----- src/polyline/MapplsPolylineController.ts | 5 ----- 3 files changed, 12 deletions(-) diff --git a/api/react-for-mappls.api.d.ts b/api/react-for-mappls.api.d.ts index 7ccea3b..830f852 100644 --- a/api/react-for-mappls.api.d.ts +++ b/api/react-for-mappls.api.d.ts @@ -297,7 +297,6 @@ declare class MapplsCircleController extends CircleController; resync(): Promise; - clear(): Promise; /** * Hit-test a map click (its lat/lng) against the circles geometrically (inside * the fill radius) and dispatch the click on the matching circle. Does NOT use @@ -352,7 +351,6 @@ declare class MapplsPolylineController extends PolylineController; - clear(): Promise; /** * Hit-test a map click (its lat/lng) against the polylines geometrically and, * if the click lands within the tap tolerance of a line, dispatch the click on diff --git a/src/circle/MapplsCircleController.ts b/src/circle/MapplsCircleController.ts index 78cc1cc..0ffb7e1 100644 --- a/src/circle/MapplsCircleController.ts +++ b/src/circle/MapplsCircleController.ts @@ -24,11 +24,6 @@ export class MapplsCircleController extends CircleController await this.renderer.redraw(); } - override async clear(): Promise { - await super.clear(); - await this.renderer.redraw(); - } - /** * Hit-test a map click (its lat/lng) against the circles geometrically (inside * the fill radius) and dispatch the click on the matching circle. Does NOT use diff --git a/src/polyline/MapplsPolylineController.ts b/src/polyline/MapplsPolylineController.ts index d475be6..b9bfc69 100644 --- a/src/polyline/MapplsPolylineController.ts +++ b/src/polyline/MapplsPolylineController.ts @@ -19,11 +19,6 @@ export class MapplsPolylineController extends PolylineController { - await super.clear(); - await this.renderer.redraw(); - } - /** * Hit-test a map click (its lat/lng) against the polylines geometrically and, * if the click lands within the tap tolerance of a line, dispatch the click on From 0fd262afb9b97b676840c41d16e111ad6e2ab021 Mon Sep 17 00:00:00 2001 From: Masashi Katsumata Date: Sat, 7 Nov 2026 20:03:39 -0800 Subject: [PATCH 2/2] Hold ground image removals until the style can be edited removeGroundImage returned early while canEditStyle() was false, and the controller had already forgotten the state, so the image stayed on the map for good. canEditStyle follows isStyleLoaded(), which turns false right after other overlays rewrite their sources; clearOverlays() clears those first, so the ground image survived every clearOverlays(). The renderer now keeps such removals and applies them from the controller's resync(), which runs when the style becomes editable again. Adding the same id again before then cancels the held removal. GroundImageRemoval.test.mjs covers both. --- api/react-for-mappls.api.d.ts | 2 + .../MapplsGroundImageController.ts | 2 + .../MapplsGroundImageOverlayRenderer.ts | 33 ++++- test/GroundImageRemoval.test.mjs | 127 ++++++++++++++++++ 4 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 test/GroundImageRemoval.test.mjs diff --git a/api/react-for-mappls.api.d.ts b/api/react-for-mappls.api.d.ts index 830f852..3fcd5f4 100644 --- a/api/react-for-mappls.api.d.ts +++ b/api/react-for-mappls.api.d.ts @@ -448,6 +448,7 @@ declare class MapplsGroundImageOverlayRenderer extends AbstractGroundImageOverla private readonly canEditStyle; /** Last values applied to the map style, keyed by state id. */ private readonly applied; + private readonly pendingRemovals; constructor({ holder, canEditStyle, }: { holder: MapplsMapViewHolder; canEditStyle: () => boolean; @@ -463,6 +464,7 @@ declare class MapplsGroundImageOverlayRenderer extends AbstractGroundImageOverla /** Sync an already-created image source+layer to the current state (diffed). */ private applyToExisting; removeGroundImage(entity: GroundImageEntity): Promise; + private removeFromStyle; } declare class MapplsGroundImageController implements SlottedOverlayController { diff --git a/src/groundimage/MapplsGroundImageController.ts b/src/groundimage/MapplsGroundImageController.ts index bb1bbc2..f579ff7 100644 --- a/src/groundimage/MapplsGroundImageController.ts +++ b/src/groundimage/MapplsGroundImageController.ts @@ -68,6 +68,8 @@ export class MapplsGroundImageController implements SlottedOverlayController { resync(): void { this.cancelPendingUpdates(); + // Called when the style becomes editable again: apply removals held back meanwhile. + this.renderer.flushPendingRemovals(); this.groundImageIds.clear(); for (const state of this.groundImageStates.values()) void this.upsert(state); } diff --git a/src/groundimage/MapplsGroundImageOverlayRenderer.ts b/src/groundimage/MapplsGroundImageOverlayRenderer.ts index 0b390d1..31aa4ba 100644 --- a/src/groundimage/MapplsGroundImageOverlayRenderer.ts +++ b/src/groundimage/MapplsGroundImageOverlayRenderer.ts @@ -18,6 +18,13 @@ export class MapplsGroundImageOverlayRenderer extends AbstractGroundImageOverlay private readonly canEditStyle: () => boolean; /** Last values applied to the map style, keyed by state id. */ private readonly applied = new Map(); + // Removals that arrived while the style could not be edited, applied by + // flushPendingRemovals() once it can. Dropping them instead left the image on + // the map for good: the controller forgets the state as soon as it asks for + // the removal, so nothing would ask again. canEditStyle turns false whenever + // isStyleLoaded() does, which includes the moments right after other overlays + // rewrite their sources, so clearOverlays() hit this every time. + private readonly pendingRemovals = new Set(); constructor({ holder, @@ -34,6 +41,8 @@ export class MapplsGroundImageOverlayRenderer extends AbstractGroundImageOverlay layerId(id: string): string { return `mc-gimg-lyr-${id}`; } async createGroundImage(state: GroundImageState): Promise { + // Added again before a held-back removal ran: the caller wants it on the map. + this.pendingRemovals.delete(state.id); const coordinates = groundImageCoordinates(state); if (!coordinates) return null; @@ -140,8 +149,26 @@ export class MapplsGroundImageOverlayRenderer extends AbstractGroundImageOverlay async removeGroundImage(entity: GroundImageEntity): Promise { this.applied.delete(entity.groundImage); - if (!this.canEditStyle()) return; - removeLayerIfExists(this.holder.map, this.layerId(entity.groundImage)); - removeSourceIfExists(this.holder.map, this.sourceId(entity.groundImage)); + if (!this.canEditStyle()) { + this.pendingRemovals.add(entity.groundImage); + return; + } + this.pendingRemovals.delete(entity.groundImage); + this.removeFromStyle(entity.groundImage); + } + + /** + * Applies the removals held back while the style could not be edited. + * @internal + */ + flushPendingRemovals(): void { + if (this.pendingRemovals.size === 0 || !this.canEditStyle()) return; + for (const id of this.pendingRemovals) this.removeFromStyle(id); + this.pendingRemovals.clear(); + } + + private removeFromStyle(id: string): void { + removeLayerIfExists(this.holder.map, this.layerId(id)); + removeSourceIfExists(this.holder.map, this.sourceId(id)); } } diff --git a/test/GroundImageRemoval.test.mjs b/test/GroundImageRemoval.test.mjs new file mode 100644 index 0000000..b119f4c --- /dev/null +++ b/test/GroundImageRemoval.test.mjs @@ -0,0 +1,127 @@ +import assert from 'node:assert/strict'; +import { mkdirSync, mkdtempSync, rmSync } from 'node:fs'; +import { after, test } from 'node:test'; +import { pathToFileURL } from 'node:url'; +import { resolve } from 'node:path'; +import { build } from 'tsup'; +import { createGeoPoint, createGeoRectBounds, createGroundImageState } from '@mapconductor/js-sdk-core'; + +// Exercise the actual internal classes without exporting them as public API. +// Only the map is replaced; no DOM, WebGL, or network is needed. +mkdirSync(new URL('../build/', import.meta.url), { recursive: true }); +const outDir = mkdtempSync(new URL('../build/groundimage-tests-', import.meta.url)); +after(() => rmSync(outDir, { recursive: true, force: true })); +await build({ + entry: { + controller: 'src/groundimage/MapplsGroundImageController.ts', + renderer: 'src/groundimage/MapplsGroundImageOverlayRenderer.ts', + }, + outDir, format: ['esm'], platform: 'node', splitting: false, + dts: false, sourcemap: false, config: false, silent: true, +}); +const { MapplsGroundImageController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs'))); +const { MapplsGroundImageOverlayRenderer } = await import(pathToFileURL(resolve(outDir, 'renderer.mjs'))); + +const sourceId = 'mc-gimg-src-GI'; +const layerId = 'mc-gimg-lyr-GI'; + +function state(opacity = 0.9) { + return createGroundImageState({ + id: 'GI', + bounds: createGeoRectBounds({ + southWest: createGeoPoint({ latitude: 35.64, longitude: 139.74 }), + northEast: createGeoPoint({ latitude: 35.665, longitude: 139.78 }), + }), + imageUrl: '/image.png', + opacity, + }); +} + +function fixture() { + const sources = new Map(); + const layers = new Map(); + let ready = true; + const map = { + getSource: (id) => sources.get(id), + getLayer: (id) => layers.get(id), + addSource(id, source) { + assert.ok(ready, 'cannot add a source while the style is unavailable'); + assert.ok(!sources.has(id)); + sources.set(id, { ...source, updateImage() {}, setCoordinates() {} }); + }, + addLayer(layer) { + assert.ok(ready, 'cannot add a layer while the style is unavailable'); + assert.ok(sources.has(layer.source)); + assert.ok(!layers.has(layer.id)); + layers.set(layer.id, layer); + }, + removeLayer(id) { + assert.ok(ready, 'cannot remove a layer while the style is unavailable'); + layers.delete(id); + }, + removeSource(id) { + assert.ok(ready, 'cannot remove a source while the style is unavailable'); + assert.ok(![...layers.values()].some((layer) => layer.source === id)); + sources.delete(id); + }, + setPaintProperty(id, property, value) { + layers.get(id).paint[property] = value; + }, + moveLayer() {}, + }; + const renderer = new MapplsGroundImageOverlayRenderer({ holder: { map }, canEditStyle: () => ready }); + const controller = new MapplsGroundImageController(renderer); + return { + controller, + setReady(value) { ready = value; }, + onMap: () => sources.has(sourceId) || layers.has(layerId), + }; +} + +const settle = () => new Promise((r) => setTimeout(r, 0)); + +test('a removal while the style is not editable is applied once it is', async () => { + const f = fixture(); + f.controller.composition([state()]); + await settle(); + assert.ok(f.onMap()); + + // clearOverlays() runs right after other overlays rewrote their sources, + // when isStyleLoaded() -- and so canEditStyle -- is briefly false. + f.setReady(false); + f.controller.clear(); + await settle(); + assert.equal(f.controller.hasId('GI'), false); + + f.setReady(true); + f.controller.resync(); + await settle(); + assert.equal(f.onMap(), false, 'the cleared ground image is still on the map'); +}); + +test('adding the same id again before the style is editable keeps it', async () => { + const f = fixture(); + f.controller.composition([state()]); + await settle(); + + f.setReady(false); + f.controller.clear(); + await settle(); + f.controller.composition([state(0.4)]); + await settle(); + + f.setReady(true); + f.controller.resync(); + await settle(); + assert.ok(f.onMap(), 'the re-added ground image was removed'); + assert.equal(f.controller.hasId('GI'), true); +}); + +test('a removal while the style is editable is applied at once', async () => { + const f = fixture(); + f.controller.composition([state()]); + await settle(); + f.controller.composition([]); + await settle(); + assert.equal(f.onMap(), false); +});