From ae2b556b8e3eac1ef90ae02bdefd452ac9b64da5 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-longdo.api.d.ts | 2 -- src/circle/LongdoCircleController.ts | 5 ----- src/polyline/LongdoPolylineController.ts | 5 ----- 3 files changed, 12 deletions(-) diff --git a/api/react-for-longdo.api.d.ts b/api/react-for-longdo.api.d.ts index 7e4d232..186ec13 100644 --- a/api/react-for-longdo.api.d.ts +++ b/api/react-for-longdo.api.d.ts @@ -332,7 +332,6 @@ declare class LongdoCircleController 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 @@ -387,7 +386,6 @@ declare class LongdoPolylineController 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/LongdoCircleController.ts b/src/circle/LongdoCircleController.ts index c72ff7e..b14e252 100644 --- a/src/circle/LongdoCircleController.ts +++ b/src/circle/LongdoCircleController.ts @@ -24,11 +24,6 @@ export class LongdoCircleController 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/LongdoPolylineController.ts b/src/polyline/LongdoPolylineController.ts index 5253d3f..58b0f2c 100644 --- a/src/polyline/LongdoPolylineController.ts +++ b/src/polyline/LongdoPolylineController.ts @@ -19,11 +19,6 @@ export class LongdoPolylineController 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 34819ff5a668d42f274b2df6c546087bdba40a4b 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-longdo.api.d.ts | 2 + .../LongdoGroundImageController.ts | 2 + .../LongdoGroundImageOverlayRenderer.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-longdo.api.d.ts b/api/react-for-longdo.api.d.ts index 186ec13..56d72ca 100644 --- a/api/react-for-longdo.api.d.ts +++ b/api/react-for-longdo.api.d.ts @@ -483,6 +483,7 @@ declare class LongdoGroundImageOverlayRenderer 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: LongdoMapViewHolder; canEditStyle: () => boolean; @@ -498,6 +499,7 @@ declare class LongdoGroundImageOverlayRenderer extends AbstractGroundImageOverla /** Sync an already-created image source+layer to the current state (diffed). */ private applyToExisting; removeGroundImage(entity: GroundImageEntity): Promise; + private removeFromStyle; } declare class LongdoGroundImageController implements SlottedOverlayController { diff --git a/src/groundimage/LongdoGroundImageController.ts b/src/groundimage/LongdoGroundImageController.ts index 5713f08..1475d3f 100644 --- a/src/groundimage/LongdoGroundImageController.ts +++ b/src/groundimage/LongdoGroundImageController.ts @@ -68,6 +68,8 @@ export class LongdoGroundImageController 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/LongdoGroundImageOverlayRenderer.ts b/src/groundimage/LongdoGroundImageOverlayRenderer.ts index 38a42dd..223b85c 100644 --- a/src/groundimage/LongdoGroundImageOverlayRenderer.ts +++ b/src/groundimage/LongdoGroundImageOverlayRenderer.ts @@ -18,6 +18,13 @@ export class LongdoGroundImageOverlayRenderer 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 LongdoGroundImageOverlayRenderer 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 LongdoGroundImageOverlayRenderer 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..43f3016 --- /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/LongdoGroundImageController.ts', + renderer: 'src/groundimage/LongdoGroundImageOverlayRenderer.ts', + }, + outDir, format: ['esm'], platform: 'node', splitting: false, + dts: false, sourcemap: false, config: false, silent: true, +}); +const { LongdoGroundImageController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs'))); +const { LongdoGroundImageOverlayRenderer } = 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 LongdoGroundImageOverlayRenderer({ holder: { map }, canEditStyle: () => ready }); + const controller = new LongdoGroundImageController(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); +});