diff --git a/src/circle/MapboxCircleLayer.ts b/src/circle/MapboxCircleLayer.ts index 5aa321a..7f5464f 100644 --- a/src/circle/MapboxCircleLayer.ts +++ b/src/circle/MapboxCircleLayer.ts @@ -3,7 +3,7 @@ import type { LayerSpecification, } from 'mapbox-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 { MapboxMapViewHolder } from '../MapboxMapViewHolder'; export type MapboxActualCircle = PolygonFeature & { id?: string | number }; @@ -51,7 +51,7 @@ export class MapboxCircleLayer { 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 7190874..f4db4a6 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -41,14 +41,84 @@ export function removeSourceIfExists(map: MapboxMap, sourceId: string): void { 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: MapboxMap): 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: MapboxMap): 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 447e5bd..d1c4e1c 100644 --- a/src/marker/MarkerLayer.ts +++ b/src/marker/MarkerLayer.ts @@ -4,7 +4,7 @@ import type { LayerSpecification, } from 'mapbox-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 { MapboxMapViewHolder } from '../MapboxMapViewHolder'; export type MapboxActualMarker = 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/MapboxPolygonLayer.ts b/src/polygon/MapboxPolygonLayer.ts index f7eb137..c5aba25 100644 --- a/src/polygon/MapboxPolygonLayer.ts +++ b/src/polygon/MapboxPolygonLayer.ts @@ -5,7 +5,7 @@ import type { LineFeature, PolygonFeature, } from '../helpers'; -import { bringMarkerLayersToFront } from '../helpers'; +import { bringMarkerLayersToFront, setSourceFeatures } from '../helpers'; import { MapboxMapViewHolder } from '../MapboxMapViewHolder'; export interface MapboxActualPolygon { @@ -69,8 +69,8 @@ export class MapboxPolygonLayer { 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/MapboxPolylineLayer.ts b/src/polyline/MapboxPolylineLayer.ts index d4ca048..ef88633 100644 --- a/src/polyline/MapboxPolylineLayer.ts +++ b/src/polyline/MapboxPolylineLayer.ts @@ -1,6 +1,6 @@ import type { GeoJSONSource, LayerSpecification } from 'mapbox-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 { MapboxMapViewHolder } from '../MapboxMapViewHolder'; export type MapboxActualPolyline = LineFeature[]; @@ -45,7 +45,7 @@ export class MapboxPolylineLayer { 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..98df5ca --- /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/MapboxPolylineController.ts', + renderer: 'src/polyline/MapboxPolylineOverlayRenderer.ts', + layer: 'src/polyline/MapboxPolylineLayer.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 { MapboxPolylineController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs'))); +const { MapboxPolylineOverlayRenderer } = await import(pathToFileURL(resolve(outDir, 'renderer.mjs'))); +const { MapboxPolylineLayer } = 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 MapboxPolylineLayer({ holder, canEditStyle: () => true }); + const renderer = new MapboxPolylineOverlayRenderer({ layer, polylineManager: new PolylineManager(), holder }); + const controller = new MapboxPolylineController(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'); +});