Stop the styleReady flip-flop; load marker icons through loadImage's callback

api/react-for-mappls.api.d.ts

@@ -185,6 +185,17 @@ declare class MapplsMarkerOverlayRenderer extends AbstractMarkerOverlayRenderer<
     private ensureImages;
     private ensureImage;
     private loadBitmapIcon;
+    /**
+     * `map.loadImage` in the callback form the Mappls SDK has.
+     *
+     * The Mappls map is built on the mapbox-gl v1 line, where `loadImage(url, callback)`
+     * returns nothing and reports through the callback. Awaiting it as maplibre's promise
+     * form got `undefined` (so this fell back to a plain Image and fetched the icon twice),
+     * and the SDK then called the missing callback when its own fetch finished, which threw
+     * "e is not a function" as an unhandled rejection inside the SDK on every page with a
+     * marker. A promise, if a future SDK returns one, is honoured as well.
+     */
+    private loadImageThroughMap;
     private ensureFallbackDefaultIcon;
     private removeUnusedImages;
     private syncIconOffsets;

src/circle/MapplsCircleLayer.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 { MapplsMapViewHolder } from '../MapplsMapViewHolder';
 
 export type MapplsActualCircle = PolygonFeature & { id?: string | number };
@@ -51,7 +51,7 @@ export class MapplsCircleLayer {
     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;

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<object, string>();
+
+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<object, readonly unknown[]>();
+
+/**
+ * `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 {

src/marker/MapplsMarkerOverlayRenderer.ts

@@ -228,8 +228,7 @@ export class MapplsMarkerOverlayRenderer extends AbstractMarkerOverlayRenderer<
     url: string,
   ): Promise<HTMLImageElement | ImageBitmap | ImageData> {
     try {
-      const image = await this.holder.map.loadImage(url);
-      return image.data;
+      return await this.loadImageThroughMap(url);
     } catch {
       const image = new Image();
       image.decoding = 'async';
@@ -242,6 +241,36 @@ export class MapplsMarkerOverlayRenderer extends AbstractMarkerOverlayRenderer<
     }
   }
 
+  /**
+   * `map.loadImage` in the callback form the Mappls SDK has.
+   *
+   * The Mappls map is built on the mapbox-gl v1 line, where `loadImage(url, callback)`
+   * returns nothing and reports through the callback. Awaiting it as maplibre's promise
+   * form got `undefined` (so this fell back to a plain Image and fetched the icon twice),
+   * and the SDK then called the missing callback when its own fetch finished, which threw
+   * "e is not a function" as an unhandled rejection inside the SDK on every page with a
+   * marker. A promise, if a future SDK returns one, is honoured as well.
+   */
+  private loadImageThroughMap(url: string): Promise<HTMLImageElement | ImageBitmap | ImageData> {
+    type Loaded = HTMLImageElement | ImageBitmap | ImageData;
+    const map = this.holder.map as unknown as {
+      loadImage(
+        url: string,
+        callback: (error?: Error | null, image?: Loaded | null) => void,
+      ): void | Promise<{ data: Loaded }>;
+    };
+    return new Promise<Loaded>((resolve, reject) => {
+      const returned = map.loadImage(url, (error, image) => {
+        if (error) reject(error);
+        else if (image) resolve(image);
+        else reject(new Error(`Failed to load marker icon: ${url}`));
+      });
+      if (returned && typeof (returned as Promise<{ data: Loaded }>).then === 'function') {
+        (returned as Promise<{ data: Loaded }>).then((response) => resolve(response.data), reject);
+      }
+    });
+  }
+
   private ensureFallbackDefaultIcon(): boolean {
     if (this.holder.map.hasImage(DEFAULT_ICON_ID)) return true;
     const size = 48;

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 { MapplsMapViewHolder } from '../MapplsMapViewHolder';
 
 export type MapplsActualMarker = 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;

src/polygon/MapplsPolygonLayer.ts

@@ -5,7 +5,7 @@ import type {
   LineFeature,
   PolygonFeature,
 } from '../helpers';
-import { bringMarkerLayersToFront } from '../helpers';
+import { bringMarkerLayersToFront, setSourceFeatures } from '../helpers';
 import { MapplsMapViewHolder } from '../MapplsMapViewHolder';
 
 export interface MapplsActualPolygon {
@@ -69,8 +69,8 @@ export class MapplsPolygonLayer {
       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;

src/polyline/MapplsPolylineLayer.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 { MapplsMapViewHolder } from '../MapplsMapViewHolder';
 
 export type MapplsActualPolyline = LineFeature[];
@@ -45,7 +45,7 @@ export class MapplsPolylineLayer {
     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;

test/MarkerIconLoad.test.mjs

@@ -0,0 +1,55 @@
+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, createMarkerState } from '@mapconductor/js-sdk-core';
+
+// The Mappls map is on the mapbox-gl v1 line: loadImage(url, callback) returns nothing.
+// The renderer awaited it in maplibre's promise form, never passed a callback, and the SDK
+// threw "e is not a function" when its fetch finished (a pageerror on every page with a
+// marker). This fake has exactly that callback-only signature.
+mkdirSync(new URL('../build/', import.meta.url), { recursive: true });
+const outDir = mkdtempSync(new URL('../build/marker-icon-tests-', import.meta.url));
+after(() => rmSync(outDir, { recursive: true, force: true }));
+await build({
+    entry: { renderer: 'src/marker/MapplsMarkerOverlayRenderer.ts' },
+    outDir, format: ['esm'], platform: 'node', splitting: false,
+    dts: false, sourcemap: false, config: false, silent: true,
+});
+const { MapplsMarkerOverlayRenderer } = await import(pathToFileURL(resolve(outDir, 'renderer.mjs')));
+
+function fixture() {
+    const images = new Map();
+    const calls = [];
+    const map = {
+        hasImage: (id) => images.has(id),
+        addImage: (id, image) => { images.set(id, image); },
+        loadImage(url, callback) {
+            calls.push({ url, callbackType: typeof callback });
+            setTimeout(() => {
+                if (typeof callback !== 'function') return; // the real SDK throws here
+                callback(null, { fakeImage: url });
+            }, 0);
+            return undefined;
+        },
+    };
+    const noLayer = { draw() { return true; }, drawSelected() { return true; }, setIconOffsets() {} };
+    const renderer = new MapplsMarkerOverlayRenderer({
+        holder: { map },
+        markerManager: { allEntities: () => [] },
+        markerLayer: noLayer,
+        dragLayer: noLayer,
+    });
+    return { renderer, images, calls };
+}
+
+test('marker icons are loaded through the callback form of map.loadImage', async () => {
+    const f = fixture();
+    const state = createMarkerState({ id: 'M', position: createGeoPoint({ latitude: 35, longitude: 139 }) });
+    await f.renderer.onAdd([{ state, bitmapIcon: state.icon?.toBitmapIcon?.() ?? { url: 'data:image/png;base64,' } }]);
+    assert.ok(f.calls.length > 0, 'loadImage was used');
+    assert.ok(f.calls.every((c) => c.callbackType === 'function'), 'every loadImage call passes a callback');
+    assert.deepEqual(f.images.get('mc-default'), { fakeImage: f.calls[0].url }, 'the loaded image is registered');
+});