From 12ea540091faf3d21f371e44d9934067dece6952 Mon Sep 17 00:00:00 2001 From: Masashi Katsumata Date: Sat, 7 Nov 2026 21:56:48 -0800 Subject: [PATCH 1/2] Stop resync from rewriting sources and layers that did not change styleReady flipped between true and false about every 0.3 s on a map that was doing nothing (26-52 flips in 10 s on the clearOverlays test page, with the map re-rendering ~100 times in that window). The loop: a styledata arriving while a GeoJSON source is still loading sets styleReady to false, the idle that follows sets it back and resyncs every overlay, and each resync called setData on every source and moveLayer on the marker layers. moveLayer marks the style changed even when the layer stays where it was, so another styledata came while the just-rewritten sources were loading, and so on. Stopping any single controller's resync did not end it, because they all wrote. bringMarkerLayersToFront now moves nothing when the marker layers are already on top (or the order is still the one it last produced), and the layers write through setSourceFeatures, which skips setData when the features are the same objects as the last write to that same source. Renderers build a new feature for every change, so identity means "unchanged"; a style swap creates new source objects, so a lost source is always written again and design changes still restore the overlays. ResyncWritesNothing.test.mjs pins both, plus "a resync with nothing to do touches nothing" and "after a style swap it rebuilds". --- src/circle/TomTomCircleLayer.ts | 4 +- src/helpers.ts | 72 +++++++++++++++- src/marker/MarkerLayer.ts | 4 +- src/polygon/TomTomPolygonLayer.ts | 6 +- src/polyline/TomTomPolylineLayer.ts | 4 +- test/ResyncWritesNothing.test.mjs | 129 ++++++++++++++++++++++++++++ 6 files changed, 209 insertions(+), 10 deletions(-) create mode 100644 test/ResyncWritesNothing.test.mjs diff --git a/src/circle/TomTomCircleLayer.ts b/src/circle/TomTomCircleLayer.ts index 8599a61..c1a3a6d 100644 --- a/src/circle/TomTomCircleLayer.ts +++ b/src/circle/TomTomCircleLayer.ts @@ -3,7 +3,7 @@ import type { LayerSpecification, } from 'maplibre-gl'; import { type CircleEntity } from '@mapconductor/js-sdk-core'; -import { bringMarkerLayersToFront, type FeatureCollection, type PolygonFeature } from '../helpers'; +import { bringMarkerLayersToFront, setSourceFeatures, type FeatureCollection, type PolygonFeature } from '../helpers'; import { TomTomMapViewHolder } from '../TomTomMapViewHolder'; export type TomTomActualCircle = PolygonFeature & { id?: string | number }; @@ -51,7 +51,7 @@ export class TomTomCircleLayer { try { const source = this.holder.map.getSource(this.sourceId) as GeoJSONSource | undefined; if (!source) return false; - source.setData(data); + setSourceFeatures(source, data.features); return true; } catch { return false; diff --git a/src/helpers.ts b/src/helpers.ts index 15e60fe..48cd857 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -42,14 +42,84 @@ export function removeSourceIfExists(map: maplibregl.Map, sourceId: string): voi if (map.getSource(sourceId)) map.removeSource(sourceId); } +const MARKER_LAYER_IDS = ['mc-raster-lyr-mc-marker-tiles', 'mc-marker-layer', 'mc-marker-drag-layer']; + +// The layer order this module last produced, per map. Lets bringMarkerLayersToFront tell +// "nothing moved since I arranged it" apart from "someone added a layer on top". +const arrangedOrders = new WeakMap(); + +function currentLayerOrder(map: maplibregl.Map): string[] | null { + try { + const m = map as unknown as { + getLayersOrder?: () => string[]; + style?: { order?: string[] }; + getStyle?: () => { layers?: Array<{ id: string }> } | undefined; + }; + if (typeof m.getLayersOrder === 'function') return m.getLayersOrder(); + if (Array.isArray(m.style?.order)) return [...m.style!.order!]; + const layers = m.getStyle?.()?.layers; + return layers ? layers.map((layer) => layer.id) : null; + } catch { + return null; + } +} + +/** + * Keep the marker layers above every other layer. + * + * **Moves nothing when they already are.** `moveLayer` marks the style as changed even when + * the layer stays where it was, and every change fires `styledata`. The provider treats a + * `styledata` that arrives while sources are still loading as "style not ready", and the + * `idle` that follows resyncs every overlay — which called this again. The style flipped + * between ready and not ready about every 0.3 s, forever, with nothing on the map changing. + */ export function bringMarkerLayersToFront(map: maplibregl.Map): void { - for (const layerId of ['mc-raster-lyr-mc-marker-tiles', 'mc-marker-layer', 'mc-marker-drag-layer']) { + const order = currentLayerOrder(map); + if (order) { + const present = MARKER_LAYER_IDS.filter((layerId) => order.includes(layerId)); + if (present.length === 0) return; + const tail = order.slice(order.length - present.length); + if (tail.every((layerId, i) => layerId === present[i])) return; + if (arrangedOrders.get(map) === order.join('\n')) return; + } + for (const layerId of MARKER_LAYER_IDS) { try { if (map.getLayer(layerId)) map.moveLayer(layerId); } catch { continue; } } + const arranged = currentLayerOrder(map); + if (arranged) arrangedOrders.set(map, arranged.join('\n')); +} + +// What each GeoJSON source was last given, keyed by the source object itself. A style swap +// replaces the source objects, so a lost source never matches and is written again. +const writtenFeatures = new WeakMap(); + +/** + * `source.setData` that skips the write when the features are the same objects, in the same + * order, as the last write to this very source. + * + * Renderers build a new feature object whenever an overlay changes, so identity is enough to + * say "nothing changed". Skipping matters for the same reason as in + * [bringMarkerLayersToFront]: a redundant setData puts the source back into "loading", the + * next `styledata` then reads as "style not ready", and the resync on `idle` wrote it again. + */ +export function setSourceFeatures( + source: { setData(data: GeoJSONSourceData): unknown }, + features: FeatureCollection['features'], +): void { + const previous = writtenFeatures.get(source); + if ( + previous && + previous.length === features.length && + previous.every((feature, i) => feature === features[i]) + ) { + return; + } + source.setData({ type: 'FeatureCollection', features } as GeoJSONSourceData); + writtenFeatures.set(source, [...features]); } export function metersToPixels(meters: number, latitude: number): number { diff --git a/src/marker/MarkerLayer.ts b/src/marker/MarkerLayer.ts index b9084f4..a9eca0c 100644 --- a/src/marker/MarkerLayer.ts +++ b/src/marker/MarkerLayer.ts @@ -4,7 +4,7 @@ import type { LayerSpecification, } from 'maplibre-gl'; import type { MarkerEntity } from '@mapconductor/js-sdk-core'; -import { bringMarkerLayersToFront, type FeatureCollection, type PointFeature } from '../helpers'; +import { bringMarkerLayersToFront, setSourceFeatures, type FeatureCollection, type PointFeature } from '../helpers'; import { TomTomMapViewHolder } from '../TomTomMapViewHolder'; export type TomTomActualMarker = PointFeature; @@ -93,7 +93,7 @@ export class MarkerLayer { try { const source = this.holder.map.getSource(this.sourceId) as GeoJSONSource | undefined; if (!source) return false; - source.setData(data); + setSourceFeatures(source, data.features); return true; } catch { return false; diff --git a/src/polygon/TomTomPolygonLayer.ts b/src/polygon/TomTomPolygonLayer.ts index 872fdfd..fb2b911 100644 --- a/src/polygon/TomTomPolygonLayer.ts +++ b/src/polygon/TomTomPolygonLayer.ts @@ -5,7 +5,7 @@ import type { LineFeature, PolygonFeature, } from '../helpers'; -import { bringMarkerLayersToFront } from '../helpers'; +import { bringMarkerLayersToFront, setSourceFeatures } from '../helpers'; import { TomTomMapViewHolder } from '../TomTomMapViewHolder'; export interface TomTomActualPolygon { @@ -69,8 +69,8 @@ export class TomTomPolygonLayer { const outlineSource = this.holder.map.getSource(this.outlineSourceId) as GeoJSONSource | undefined; if (!fillSource || !outlineSource) return false; - fillSource.setData(fillData); - outlineSource.setData(outlineData); + setSourceFeatures(fillSource, fillData.features); + setSourceFeatures(outlineSource, outlineData.features); return true; } catch { return false; diff --git a/src/polyline/TomTomPolylineLayer.ts b/src/polyline/TomTomPolylineLayer.ts index 422c1dd..3d3b1a0 100644 --- a/src/polyline/TomTomPolylineLayer.ts +++ b/src/polyline/TomTomPolylineLayer.ts @@ -1,6 +1,6 @@ import type { GeoJSONSource, LayerSpecification } from 'maplibre-gl'; import type { PolylineEntity } from '@mapconductor/js-sdk-core'; -import { bringMarkerLayersToFront, type FeatureCollection, type LineFeature } from '../helpers'; +import { bringMarkerLayersToFront, setSourceFeatures, type FeatureCollection, type LineFeature } from '../helpers'; import { TomTomMapViewHolder } from '../TomTomMapViewHolder'; export type TomTomActualPolyline = LineFeature[]; @@ -45,7 +45,7 @@ export class TomTomPolylineLayer { try { const source = this.holder.map.getSource(this.sourceId) as GeoJSONSource | undefined; if (!source) return false; - source.setData(data); + setSourceFeatures(source, data.features); return true; } catch { return false; diff --git a/test/ResyncWritesNothing.test.mjs b/test/ResyncWritesNothing.test.mjs new file mode 100644 index 0000000..1d10b49 --- /dev/null +++ b/test/ResyncWritesNothing.test.mjs @@ -0,0 +1,129 @@ +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, createPolylineState, PolylineManager } from '@mapconductor/js-sdk-core'; + +// The provider marks the style "not ready" on a styledata that arrives while a source is +// still loading, and resyncs every overlay on the idle that follows. A resync that rewrote +// unchanged sources (setData) or re-moved layers that were already on top (moveLayer) fired +// another styledata, so the style flipped between ready and not ready every ~0.3 s forever. +// These tests pin "a resync with nothing to do writes nothing", and that a lost style is +// still rebuilt. +mkdirSync(new URL('../build/', import.meta.url), { recursive: true }); +const outDir = mkdtempSync(new URL('../build/resync-tests-', import.meta.url)); +after(() => rmSync(outDir, { recursive: true, force: true })); +await build({ + entry: { + helpers: 'src/helpers.ts', + controller: 'src/polyline/TomTomPolylineController.ts', + renderer: 'src/polyline/TomTomPolylineOverlayRenderer.ts', + layer: 'src/polyline/TomTomPolylineLayer.ts', + }, + outDir, format: ['esm'], platform: 'node', splitting: false, + dts: false, sourcemap: false, config: false, silent: true, +}); +const { bringMarkerLayersToFront, setSourceFeatures } = await import(pathToFileURL(resolve(outDir, 'helpers.mjs'))); +const { TomTomPolylineController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs'))); +const { TomTomPolylineOverlayRenderer } = await import(pathToFileURL(resolve(outDir, 'renderer.mjs'))); +const { TomTomPolylineLayer } = await import(pathToFileURL(resolve(outDir, 'layer.mjs'))); + +function fakeMap(initialLayers = ['background', 'road-label']) { + const writes = { setData: 0, moveLayer: 0, addSource: 0, addLayer: 0 }; + let sources = new Map(); + let order = [...initialLayers]; + const layers = new Map(initialLayers.map((id) => [id, { id }])); + const map = { + writes, + get order() { return [...order]; }, + getLayersOrder: () => [...order], + style: { get order() { return [...order]; } }, + getStyle: () => ({ layers: order.map((id) => layers.get(id)) }), + getSource: (id) => sources.get(id), + getLayer: (id) => layers.get(id), + addSource(id) { + writes.addSource++; + sources.set(id, { setData() { writes.setData++; } }); + }, + addLayer(layer, before) { + writes.addLayer++; + layers.set(layer.id, layer); + const at = before ? order.indexOf(before) : -1; + if (at >= 0) order.splice(at, 0, layer.id); else order.push(layer.id); + }, + moveLayer(id, before) { + writes.moveLayer++; + order = order.filter((x) => x !== id); + const at = before ? order.indexOf(before) : -1; + if (at >= 0) order.splice(at, 0, id); else order.push(id); + }, + // A style swap drops every source and layer the SDK added. + reloadStyle() { + sources = new Map(); + order = order.filter((id) => !id.startsWith('mc-') && !id.startsWith('polyline')); + for (const id of [...layers.keys()]) if (!order.includes(id)) layers.delete(id); + }, + }; + return map; +} + +test('bringMarkerLayersToFront moves nothing when the marker layers are already on top', () => { + const map = fakeMap(['background', 'mc-marker-layer', 'mc-marker-drag-layer']); + bringMarkerLayersToFront(map); + assert.equal(map.writes.moveLayer, 0); +}); + +test('bringMarkerLayersToFront moves the marker layers once, then leaves them alone', () => { + const map = fakeMap(['background', 'mc-marker-layer', 'mc-marker-drag-layer']); + map.addLayer({ id: 'polyline-layer' }); + bringMarkerLayersToFront(map); + assert.deepEqual(map.order.slice(-2), ['mc-marker-layer', 'mc-marker-drag-layer']); + const moved = map.writes.moveLayer; + assert.ok(moved > 0); + bringMarkerLayersToFront(map); + bringMarkerLayersToFront(map); + assert.equal(map.writes.moveLayer, moved); +}); + +test('setSourceFeatures skips identical features but writes to a replaced source', () => { + let calls = 0; + const source = { setData() { calls++; } }; + const a = { type: 'Feature' }; + const b = { type: 'Feature' }; + setSourceFeatures(source, [a, b]); + setSourceFeatures(source, [a, b]); + assert.equal(calls, 1); + setSourceFeatures(source, [b, a]); + assert.equal(calls, 2, 'a different order is a change'); + setSourceFeatures(source, [b]); + assert.equal(calls, 3, 'a removed feature is a change'); + const replaced = { setData() { calls++; } }; + setSourceFeatures(replaced, [b]); + assert.equal(calls, 4, 'a new source object (style swap) is always written'); +}); + +test('resync with nothing changed writes nothing; after a style swap it rebuilds', async () => { + const map = fakeMap(['background', 'road-label', 'mc-marker-layer', 'mc-marker-drag-layer']); + const holder = { map }; + const layer = new TomTomPolylineLayer({ holder, canEditStyle: () => true }); + const renderer = new TomTomPolylineOverlayRenderer({ layer, polylineManager: new PolylineManager(), holder }); + const controller = new TomTomPolylineController(renderer); + await controller.add([createPolylineState({ + id: 'PL', + points: [createGeoPoint({ latitude: 35, longitude: 139 }), createGeoPoint({ latitude: 36, longitude: 140 })], + })]); + const afterAdd = { ...map.writes }; + assert.ok(afterAdd.setData > 0); + + await controller.resync(); + await controller.resync(); + assert.deepEqual(map.writes, afterAdd, 'a resync with nothing to do must not touch the style'); + + map.reloadStyle(); + await controller.resync(); + assert.ok(map.getSource('polyline-source'), 'the lost source is added again'); + assert.ok(map.writes.setData > afterAdd.setData, 'and its data written again'); + assert.ok(map.order.includes('polyline-layer'), 'and its layer'); +}); From 11bf75706de82532831be608158d44f1ee2a4539 Mon Sep 17 00:00:00 2001 From: Masashi Katsumata Date: Sat, 7 Nov 2026 21:56:58 -0800 Subject: [PATCH 2/2] Put raster overlays under the basemap's labels, as the other GL providers do After clearOverlays() and sending the same overlays again, the raster was drawn over the polygons. clearOverlays() leaves the polygon, polyline and circle layers in the style with empty sources, and this renderer appended a new raster layer at the top of the style, so the raster now came after them. The same happened on any update that replaced the raster's source object (remove + add), which a re-render does. maplibre, mapbox, mappls, maptiler and longdo got "insert below the style's first label layer" in "Put raster overlays under the basemap's labels"; TomTom was left out. This brings the same rule here, which also puts the raster under the basemap's labels as on the others. RasterLayerOrder.test.mjs covers the add-after-clear and the source-replacement cases. The API snapshot gains the private member. --- api/react-for-tomtom.api.d.ts | 13 +++ .../TomTomRasterLayerOverlayRenderer.ts | 39 ++++++-- test/RasterLayerOrder.test.mjs | 88 +++++++++++++++++++ 3 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 test/RasterLayerOrder.test.mjs diff --git a/api/react-for-tomtom.api.d.ts b/api/react-for-tomtom.api.d.ts index e05eda0..1cb085a 100644 --- a/api/react-for-tomtom.api.d.ts +++ b/api/react-for-tomtom.api.d.ts @@ -479,6 +479,19 @@ declare class TomTomRasterLayerOverlayRenderer implements RasterLayerOverlayRend onCameraChanged(_mapCameraPosition: MapCameraPosition): Promise; onPostProcess(): Promise; private addLayer; + /** + * The style's first label layer, so a raster overlay can go under it. + * + * Appended at the top of the style instead, a raster overlay covers the + * place names, road names and shields the style draws -- a vector tile + * layer's own roads run straight through them, which is what "the labels + * are under the lines" looks like. Every raster overlay has the same + * problem, so the rule lives here rather than in each of them. + * + * Our own layers are skipped: markers are a symbol layer too, and anchoring + * to them would put the raster back above the labels. + */ + private firstBasemapLabelLayerId; private updateLayer; /** * スタイル再読込中に頼まれた削除の保留分。 diff --git a/src/raster/TomTomRasterLayerOverlayRenderer.ts b/src/raster/TomTomRasterLayerOverlayRenderer.ts index 9b09aa1..7133732 100644 --- a/src/raster/TomTomRasterLayerOverlayRenderer.ts +++ b/src/raster/TomTomRasterLayerOverlayRenderer.ts @@ -91,18 +91,40 @@ export class TomTomRasterLayerOverlayRenderer } const opacity = state.visible ? state.opacity : 0; if (!this.holder.map.getLayer(handle.layerId)) { - this.holder.map.addLayer({ - id: handle.layerId, - type: 'raster', - source: handle.sourceId, - paint: { 'raster-opacity': opacity }, - } as LayerSpecification); + this.holder.map.addLayer( + { + id: handle.layerId, + type: 'raster', + source: handle.sourceId, + paint: { 'raster-opacity': opacity }, + } as LayerSpecification, + this.firstBasemapLabelLayerId(), + ); } else { this.holder.map.setPaintProperty(handle.layerId, 'raster-opacity', opacity); } return handle; } + /** + * The style's first label layer, so a raster overlay can go under it. + * + * Appended at the top of the style instead, a raster overlay covers the + * place names, road names and shields the style draws -- a vector tile + * layer's own roads run straight through them, which is what "the labels + * are under the lines" looks like. Every raster overlay has the same + * problem, so the rule lives here rather than in each of them. + * + * Our own layers are skipped: markers are a symbol layer too, and anchoring + * to them would put the raster back above the labels. + */ + private firstBasemapLabelLayerId(): string | undefined { + const layers = this.holder.map.getStyle?.()?.layers; + if (!layers) return undefined; + const anchor = layers.find((layer) => layer.type === 'symbol' && !isOurLayerId(layer.id)); + return anchor?.id; + } + private updateLayer(handle: TomTomRasterLayerHandle, state: RasterLayerState): void { if (!this.canEditStyle()) return; if (!this.holder.map.getLayer(handle.layerId)) return; @@ -137,3 +159,8 @@ export class TomTomRasterLayerOverlayRenderer removeSourceIfExists(this.holder.map, handle.sourceId); } } + +/** True for layers this SDK adds, as opposed to the style's own. */ +function isOurLayerId(id: string): boolean { + return id.startsWith('mc-'); +} diff --git a/test/RasterLayerOrder.test.mjs b/test/RasterLayerOrder.test.mjs new file mode 100644 index 0000000..8004383 --- /dev/null +++ b/test/RasterLayerOrder.test.mjs @@ -0,0 +1,88 @@ +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 { createRasterLayerState, RasterLayerSource } from '@mapconductor/js-sdk-core'; + +// clearOverlays() leaves the polygon / polyline / circle layers in the style (their sources +// are just emptied). A raster added after that was appended at the top of the style, so +// sending the same overlays again drew the raster over the polygons. The same happened on any +// raster update that replaced its source. The other GL providers insert a raster below the +// basemap's first label layer; TomTom now does the same. +mkdirSync(new URL('../build/', import.meta.url), { recursive: true }); +const outDir = mkdtempSync(new URL('../build/raster-order-tests-', import.meta.url)); +after(() => rmSync(outDir, { recursive: true, force: true })); +await build({ + entry: { + controller: 'src/raster/TomTomRasterLayerController.ts', + renderer: 'src/raster/TomTomRasterLayerOverlayRenderer.ts', + }, + outDir, format: ['esm'], platform: 'node', splitting: false, + dts: false, sourcemap: false, config: false, silent: true, +}); +const { TomTomRasterLayerController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs'))); +const { TomTomRasterLayerOverlayRenderer } = await import(pathToFileURL(resolve(outDir, 'renderer.mjs'))); + +function fixture() { + const sources = new Map(); + const layers = new Map(); + let order = []; + const add = (layer, before) => { + layers.set(layer.id, layer); + const at = before ? order.indexOf(before) : -1; + if (at >= 0) order.splice(at, 0, layer.id); else order.push(layer.id); + }; + // Basemap: a fill, then its first label layer. + add({ id: 'land', type: 'fill' }); + add({ id: 'place-label', type: 'symbol' }); + // What clearOverlays() leaves behind: our vector overlay layers, still in the style. + add({ id: 'polygon-fill-layer', type: 'fill' }); + add({ id: 'mc-marker-layer', type: 'symbol' }); + const map = { + order: () => [...order], + getStyle: () => ({ layers: order.map((id) => layers.get(id)) }), + getLayersOrder: () => [...order], + getSource: (id) => sources.get(id), + getLayer: (id) => layers.get(id), + addSource: (id, source) => { sources.set(id, source); }, + addLayer: (layer, before) => add(layer, before), + removeLayer: (id) => { layers.delete(id); order = order.filter((x) => x !== id); }, + removeSource: (id) => { sources.delete(id); }, + setPaintProperty: (id, property, value) => { layers.get(id).paint[property] = value; }, + moveLayer: (id, before) => { + order = order.filter((x) => x !== id); + const at = before ? order.indexOf(before) : -1; + if (at >= 0) order.splice(at, 0, id); else order.push(id); + }, + }; + const renderer = new TomTomRasterLayerOverlayRenderer({ map }, () => true); + const controller = new TomTomRasterLayerController(renderer); + return { map, controller }; +} + +function relief(opacity) { + return createRasterLayerState({ + id: 'RL', + opacity, + source: RasterLayerSource.UrlTemplate({ template: 'https://example.test/{z}/{x}/{y}.png', tileSize: 256 }), + }); +} + +test('a raster added after the vector overlays goes under them and under the basemap labels', async () => { + const f = fixture(); + await f.controller.composition([relief(0.5)]); + const order = f.map.order(); + assert.ok(order.indexOf('mc-raster-lyr-RL') < order.indexOf('place-label'), order.join(' > ')); + assert.ok(order.indexOf('mc-raster-lyr-RL') < order.indexOf('polygon-fill-layer'), order.join(' > ')); +}); + +test('replacing the raster source keeps it under the vector overlays', async () => { + const f = fixture(); + await f.controller.composition([relief(0.5)]); + // A new source object (as a re-render creates) is a source replacement: remove + add. + await f.controller.composition([relief(0.3)]); + const order = f.map.order(); + assert.ok(order.indexOf('mc-raster-lyr-RL') < order.indexOf('polygon-fill-layer'), order.join(' > ')); +});