-
-
Notifications
You must be signed in to change notification settings - Fork 570
Fix categorized styling for marker icons #1782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { | |
| normalizeHexColor, | ||
| proportionalSizeRange, | ||
| styleValue, | ||
| vectorColorExpression, | ||
| type LayerStyle, | ||
| type MarkerShape, | ||
| } from "@geolibre/core"; | ||
|
|
@@ -88,8 +89,36 @@ export function loadMarkerSvgImage(markup: string): Promise<HTMLImageElement | n | |
| }); | ||
| } | ||
|
|
||
| function loadSvgMarker(markup: string, size: number): Promise<GeneratedImageResult | null> { | ||
| const src = resolveSvgSource(markup); | ||
| function replaceSvgColorParameters(markup: string, color: string): string { | ||
| return markup | ||
| .replace(/param\(fill\)/gi, color) | ||
| .replace(/param\(fill-opacity\)/gi, "1") | ||
| .replace(/param\(outline\)/gi, color) | ||
| .replace(/param\(outline-opacity\)/gi, "1") | ||
| .replace(/param\(outline-width\)/gi, "0"); | ||
| } | ||
|
|
||
| async function colorizedSvgSource(markup: string, color: string): Promise<string | null> { | ||
| let sourceMarkup = markup; | ||
| if (/^(?:https?:|data:image\/svg\+xml)/i.test(markup)) { | ||
| try { | ||
| const response = await fetch(markup); | ||
| if (response.ok) sourceMarkup = await response.text(); | ||
| } catch { | ||
| // 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. | ||
| } | ||
| } | ||
| return resolveSvgSource(replaceSvgColorParameters(sourceMarkup, color)); | ||
| } | ||
|
giswqs marked this conversation as resolved.
giswqs marked this conversation as resolved.
Comment on lines
+105
to
+135
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security note (low confidence): this is the first place in The existing |
||
|
|
||
| async function loadSvgMarker( | ||
| markup: string, | ||
| color: string, | ||
| size: number, | ||
| ): Promise<GeneratedImageResult | null> { | ||
| const src = await colorizedSvgSource(markup, color); | ||
| if (!src) return Promise.resolve(null); | ||
| const ratio = MARKER_PIXEL_RATIO; | ||
| const px = size * ratio; | ||
|
|
@@ -193,28 +222,70 @@ export function markerIconSizeValue(style: LayerStyle): number | unknown[] { | |
| * @param style - The layer style. | ||
| * @returns The image id, or `null` when no marker applies. | ||
| */ | ||
| export function prepareMarker(style: LayerStyle): string | null { | ||
| export function prepareMarker(style: LayerStyle, colorOverride?: string): string | null { | ||
| if (!styleValue(style, "markerEnabled")) return null; | ||
| const shape = styleValue(style, "markerShape"); | ||
| const size = markerBakedSize(style); | ||
|
|
||
| if (shape === "custom") { | ||
| const markup = styleValue(style, "markerSvg").trim(); | ||
| if (!markup) return null; | ||
| const id = `geolibre-marker-svg-${hashText(markup)}-${size}`; | ||
| const color = colorOverride ?? markerColor(style); | ||
| const id = `geolibre-marker-svg-${hashText(`${markup}\0${color}`)}-${size}`; | ||
| // Capture the markup in the factory closure so the lazy generator never | ||
| // depends on a separate, evictable cache (which could blank the marker). | ||
| registerGeneratedImage(id, () => loadSvgMarker(markup, size)); | ||
| registerGeneratedImage(id, () => loadSvgMarker(markup, color, size)); | ||
| return id; | ||
| } | ||
|
|
||
| if (!BUILTIN_SHAPES.has(shape)) return null; | ||
| const color = markerColor(style); | ||
| const color = colorOverride ?? markerColor(style); | ||
| const id = `geolibre-marker-${shape}-${color.replace("#", "")}-${size}`; | ||
| registerGeneratedImage(id, () => drawBuiltinMarker(shape, color, size)); | ||
| return id; | ||
| } | ||
|
|
||
| /** | ||
| * Resolve a marker's `icon-image` layout value. Categorized, graduated, and | ||
| * rule-based color expressions select a separately baked sprite per class, | ||
| * because ordinary bitmap sprites cannot be tinted per feature by MapLibre. | ||
| */ | ||
| export function markerImageValue(style: LayerStyle): string | unknown[] | null { | ||
| const fallback = markerColor(style); | ||
| const baseId = prepareMarker(style, fallback); | ||
| if (!baseId) return null; | ||
|
|
||
| const colorValue = vectorColorExpression(style, fallback); | ||
| if (!Array.isArray(colorValue)) return baseId; | ||
|
giswqs marked this conversation as resolved.
Outdated
giswqs marked this conversation as resolved.
Outdated
|
||
|
|
||
| const imageFor = (value: unknown): unknown => { | ||
| if (typeof value !== "string" || !normalizeHexColor(value)) return value; | ||
| return prepareMarker(style, value) ?? baseId; | ||
| }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment on lines
+279
to
+300
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quality / completeness (medium confidence): This is intentional per the "uses the base marker for invalid expression color outputs" test, but the mismatch could be confusing: the same expression correctly colors fill/line/circle layers per-feature (via |
||
| const expression = [...colorValue]; | ||
| switch (expression[0]) { | ||
| case "match": | ||
| for (let index = 3; index < expression.length; index += 2) { | ||
| expression[index] = imageFor(expression[index]); | ||
| } | ||
| expression[expression.length - 1] = imageFor(expression[expression.length - 1]); | ||
| return expression; | ||
| case "step": | ||
| for (let index = 2; index < expression.length; index += 2) { | ||
| expression[index] = imageFor(expression[index]); | ||
| } | ||
| return expression; | ||
| case "case": | ||
| for (let index = 2; index < expression.length; index += 2) { | ||
| expression[index] = imageFor(expression[index]); | ||
| } | ||
| expression[expression.length - 1] = imageFor(expression[expression.length - 1]); | ||
| return expression; | ||
| default: | ||
| return baseId; | ||
| } | ||
| } | ||
|
giswqs marked this conversation as resolved.
|
||
|
|
||
| function loadRasterMarker(url: string): Promise<GeneratedImageResult | null> { | ||
| if (!/^data:image\/(?!svg)[\w.+-]+;base64,/i.test(url)) return Promise.resolve(null); | ||
| return new Promise((resolve) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import assert from "node:assert/strict"; | ||
| import { describe, it } from "node:test"; | ||
| import { DEFAULT_LAYER_STYLE, type LayerStyle } from "@geolibre/core"; | ||
| import { markerImageValue } from "../packages/map/src/markers"; | ||
|
|
||
| function categorizedMarker(patch: Partial<LayerStyle> = {}): LayerStyle { | ||
| return { | ||
| ...DEFAULT_LAYER_STYLE, | ||
| markerEnabled: true, | ||
| markerShape: "circle", | ||
| markerColor: "#3b82f6", | ||
| markerSize: 18, | ||
| vectorStyleMode: "categorized", | ||
| vectorStyleProperty: "status", | ||
| vectorStyleStops: [ | ||
| { value: "good", color: "#339084" }, | ||
| { value: "bad", color: "#fde725" }, | ||
| ], | ||
| ...patch, | ||
| }; | ||
| } | ||
|
|
||
| describe("markerImageValue", () => { | ||
|
giswqs marked this conversation as resolved.
|
||
| it("selects a separately colored built-in marker for each category", () => { | ||
| assert.deepEqual(markerImageValue(categorizedMarker()), [ | ||
| "match", | ||
| ["to-string", ["get", "status"]], | ||
| "good", | ||
| "geolibre-marker-circle-339084-18", | ||
| "bad", | ||
| "geolibre-marker-circle-fde725-18", | ||
| "geolibre-marker-circle-3b82f6-18", | ||
| ]); | ||
| }); | ||
|
|
||
| it("creates distinct parameterized SVG sprites for category colors", () => { | ||
| const value = markerImageValue( | ||
| categorizedMarker({ | ||
| markerShape: "custom", | ||
| markerSvg: | ||
| '<svg xmlns="http://www.w3.org/2000/svg"><path fill="param(fill)" d="M0 0h10v10z"/></svg>', | ||
| }), | ||
| ); | ||
|
|
||
| assert.ok(Array.isArray(value)); | ||
| const imageIds = [value[3], value[5], value[6]]; | ||
| assert.ok( | ||
| imageIds.every((id) => typeof id === "string" && id.startsWith("geolibre-marker-svg-")), | ||
| ); | ||
| assert.equal(new Set(imageIds).size, 3); | ||
| }); | ||
|
|
||
| it("creates distinct category sprites when the SVG is supplied by URL", () => { | ||
| const value = markerImageValue( | ||
| categorizedMarker({ | ||
| markerShape: "custom", | ||
| markerSvg: "https://example.com/tree.svg", | ||
| }), | ||
| ); | ||
|
|
||
| assert.ok(Array.isArray(value)); | ||
| assert.equal(new Set([value[3], value[5], value[6]]).size, 3); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.