Address review feedback

- markerImageValue: route a flat resolved color through imageFor instead of
  returning the base sprite. Rule-based mode with no drawable rules returns the
  else rule's color, which need not equal the layer's markerColor, so the marker
  was baked in the wrong color while the circle paint used the else color.
- colorizedSvgSource: drop a failed remote-SVG fetch from svgSourceCache after
  the await, so a transient error no longer disables QGIS color-parameter
  resolution for that URL for the rest of the session; in-flight dedup is kept.
- layer-sync: remove the redundant prepareMarker call. markerImageValue resolves
  the same base marker internally, so markerImageId was non-null exactly when
  markerImage was — the `?? markerImageId ?? ""` fallback was dead code.
- markers: hoist the match/step/case head set to a module constant instead of
  allocating it on every (recursive) imageFor call.
- tests: cover the else-rule-only color bake and a failed-then-successful remote
  SVG fetch.

Author: giswqs

packages/map/src/layer-sync.ts

@@ -63,7 +63,6 @@ import {
   markerImageValue,
   markerIconSizeValue,
   prepareKmlFeatureIcons,
-  prepareMarker,
 } from "./markers";
 import { isPlaceholderLayer } from "./placeholders";
 import {
@@ -1921,9 +1920,10 @@ function applyVectorDataRenderLayers(
   // a generated image id.
   ensureGeneratedImageHandler(map);
   const fillPatternId = prepareFillPattern(layer.style);
-  const markerImageId = prepareMarker(layer.style);
+  // markerImageValue resolves the same base marker internally, so it is null
+  // exactly when no marker applies — no separate prepareMarker call is needed.
   const markerImage = markerImageValue(layer.style);
-  const kmlIconImage = prepareKmlFeatureIcons(layer.geojson!, markerImage ?? markerImageId ?? "");
+  const kmlIconImage = prepareKmlFeatureIcons(layer.geojson!, markerImage ?? "");
   // Derived companion symbology (inverted mask, geometry generator, dedup
   // labels) is built from the raw features, so no MapLibre filter applies to
   // it. While a Time Slider window or a rule-based visibility filter is
@@ -2241,7 +2241,7 @@ function applyVectorDataRenderLayers(
         },
         beforeId,
       );
-      if (kmlIconImage && !markerImageId) {
+      if (kmlIconImage && !markerImage) {
         // Features without a KML icon still use the ordinary circle renderer.
         ensureLayer(
           map,

packages/map/src/markers.ts

@@ -22,6 +22,8 @@ const MAX_MARKER_SIZE = 96;
 const MAX_SVG_SOURCE_CACHE = 64;
 export const KML_ICON_URL_PROPERTY = "__geolibre_kml_icon_url";
 const svgSourceCache = new Map<string, Promise<string | null>>();
+// The expression heads whose outputs markerImageValue rewrites into sprite ids.
+const COLOR_BRANCH_HEADS: ReadonlySet<string> = new Set(["match", "step", "case"]);
 
 const BUILTIN_SHAPES: ReadonlySet<MarkerShape> = new Set([
   "circle",
@@ -118,6 +120,12 @@ async function colorizedSvgSource(markup: string, color: string): Promise<string
     if (fetched !== null) {
       sourceMarkup = fetched;
     } else {
+      // Do not keep a failed fetch cached: a transient network error would
+      // otherwise block every later color variant of the same source (and any
+      // styleimagemissing retry) until the entry is evicted. Dropping it only
+      // after the await still lets concurrent callers share the in-flight
+      // promise.
+      if (svgSourceCache.get(markup) === pending) svgSourceCache.delete(markup);
       // Preserve the original source when a remote host blocks CORS. The
       // marker still renders, although its QGIS color parameters cannot be
       // resolved without access to the SVG text.
@@ -268,9 +276,6 @@ export function markerImageValue(style: LayerStyle): string | unknown[] | null {
   const baseId = prepareMarker(style, fallback);
   if (!baseId) return null;
 
-  const colorValue = vectorColorExpression(style, fallback);
-  if (!Array.isArray(colorValue)) return baseId;
-
   const imageFor = (value: unknown): unknown => {
     if (typeof value === "string") {
       return normalizeHexColor(value) ? (prepareMarker(style, value) ?? baseId) : baseId;
@@ -279,7 +284,7 @@ export function markerImageValue(style: LayerStyle): string | unknown[] | null {
 
     const expression = [...value];
     const firstOutput = expression[0] === "match" ? 3 : 2;
-    if (!new Set(["match", "step", "case"]).has(String(expression[0]))) return baseId;
+    if (!COLOR_BRANCH_HEADS.has(String(expression[0]))) return baseId;
     for (let index = firstOutput; index < expression.length; index += 2) {
       expression[index] = imageFor(expression[index]);
     }
@@ -288,7 +293,10 @@ export function markerImageValue(style: LayerStyle): string | unknown[] | null {
     }
     return expression;
   };
-  return imageFor(colorValue) as unknown[];
+  // A flat resolved color still goes through imageFor: rule-based mode with no
+  // drawable rules returns the else rule's color, which need not equal the
+  // layer's markerColor that baseId was baked from.
+  return imageFor(vectorColorExpression(style, fallback)) as string | unknown[];
 }
 
 function loadRasterMarker(url: string): Promise<GeneratedImageResult | null> {

tests/marker-categories.test.ts

@@ -1,10 +1,12 @@
 import assert from "node:assert/strict";
 import { describe, it } from "node:test";
 import { DEFAULT_LAYER_STYLE, type LayerStyle } from "@geolibre/core";
+import { ensureGeneratedImageHandler } from "../packages/map/src/generated-images";
 import {
   KML_ICON_URL_PROPERTY,
   markerImageValue,
   prepareKmlFeatureIcons,
+  prepareMarker,
 } from "../packages/map/src/markers";
 
 function categorizedMarker(patch: Partial<LayerStyle> = {}): LayerStyle {
@@ -107,6 +109,26 @@ describe("markerImageValue", () => {
     ]);
   });
 
+  it("bakes the else-rule color when no rule is drawable", () => {
+    const value = markerImageValue(
+      categorizedMarker({
+        vectorStyleMode: "rule-based",
+        vectorRules: [
+          {
+            id: "else",
+            label: "Other",
+            filter: "",
+            color: "#fde725",
+            enabled: true,
+            isElse: true,
+          },
+        ],
+      }),
+    );
+
+    assert.equal(value, "geolibre-marker-circle-fde725-18");
+  });
+
   it("keeps categorized marker fallback inside a mixed KML icon expression", () => {
     const markerImage = markerImageValue(categorizedMarker());
     const value = prepareKmlFeatureIcons(
@@ -132,3 +154,62 @@ describe("markerImageValue", () => {
     assert.deepEqual(value[value.length - 1], markerImage);
   });
 });
+
+describe("custom SVG marker fetches", () => {
+  it("retries a remote SVG whose first fetch failed", async () => {
+    // Run the registered sprite factory the way styleimagemissing does.
+    let missing: ((event: { id: string }) => void) | undefined;
+    const map = {
+      on: (_event: string, handler: (event: { id: string }) => void) => {
+        missing = handler;
+      },
+      hasImage: () => false,
+      addImage: () => {},
+    };
+    ensureGeneratedImageHandler(map as never);
+
+    // The sprite is rasterized through an Image; erroring out keeps the test
+    // off the canvas APIs while still exercising the fetch path.
+    class StubImage {
+      decoding = "";
+      crossOrigin = "";
+      onload: (() => void) | null = null;
+      onerror: (() => void) | null = null;
+      set src(_value: string) {
+        queueMicrotask(() => this.onerror?.());
+      }
+    }
+    const previousImage = globalThis.Image;
+    const previousFetch = globalThis.fetch;
+    let calls = 0;
+    globalThis.Image = StubImage as never;
+    globalThis.fetch = (() => {
+      calls += 1;
+      return calls === 1
+        ? Promise.reject(new Error("offline"))
+        : Promise.resolve({ ok: true, text: () => Promise.resolve("<svg/>") });
+    }) as never;
+
+    try {
+      const id = prepareMarker(
+        categorizedMarker({
+          markerShape: "custom",
+          markerSvg: "https://example.com/retry.svg",
+          vectorStyleMode: "single",
+        }),
+      );
+      assert.ok(id);
+      missing?.({ id });
+      await new Promise((resolve) => setTimeout(resolve, 0));
+      missing?.({ id });
+      await new Promise((resolve) => setTimeout(resolve, 0));
+    } finally {
+      globalThis.Image = previousImage;
+      globalThis.fetch = previousFetch;
+    }
+
+    // A failed fetch must not be cached, or the marker stays uncolorized for
+    // the rest of the session.
+    assert.equal(calls, 2);
+  });
+});