Skip to content
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ The browser build proxies the sidecar at `/sidecar` (same-origin, no CORS); conf
- `GLOBE_CONTROL_TOGGLE_SELECTOR` (`packages/map/src/globe-control-toggle.ts`) mirrors the class names MapLibre's own `GlobeControl` puts on its toggle button — `maplibregl-ctrl-globe` and `maplibregl-ctrl-globe-enabled`, swapped on every projection change. `MapCanvas` persists a projection change from a **click** on that button rather than from the `projectiontransition` event, because style initialization and project reconciliation emit that event too and a stale one overwrites the projection of a project that has just loaded. The classes are internal and unexported, so whenever `maplibre-gl` is bumped (including Dependabot PRs) run the frontend suite — `tests/globe-control-toggle.test.ts` builds a real `GlobeControl` and fails if the mirror stops matching. Without that check a renamed class silently stops persisting the user's projection, with no build error.
- `BASEMAP_PANEL_SELECTOR` / `BASEMAP_ROW_SELECTOR` / `BASEMAP_ROW_ID_ATTR` (`packages/plugins/src/plugins/basemap-thumbnails.ts`) mirror the DOM `maplibre-gl-basemap-control` renders — `.basemap-control-panel`, `.basemap-control-result`, `data-basemap-id` — which the Basemaps panel's thumbnails hook into to find rows and join each one back to its catalog entry. That package exports only `BasemapControl`/`BasemapDefinition`, so a renamed class fails nothing at build time: the queries stop matching and thumbnails silently stop appearing. Whenever `maplibre-gl-basemap-control` is bumped in `packages/plugins/package.json` — including Dependabot PRs — run the frontend suite; `tests/basemap-thumbnails.test.ts` builds a real control and asserts its rendered panel against the mirror. The same file's `hasUnresolvedPlaceholder` deliberately matches the **complement** of the tile tokens it substitutes rather than mirroring that package's credential placeholders (`{api-key}`, `{aws-region}`), so a new provider's placeholder is skipped instead of being fetched literally — keep it that way rather than enumerating placeholder names.
- **Per-layer blend modes** (`packages/map/src/layer-blend-modes.ts`) wrap three *unexported* `maplibre-gl` internals, because MapLibre renders every layer into one canvas and ships no per-layer blend API (upstream draft: maplibre/maplibre-gl-js#8073). The wrappers are `Painter.prototype.renderLayer` (brackets one layer's draws), `Painter.prototype.useProgram` (tells the layer-opacity composite draw from the draws feeding it), and `Context.prototype.setColorMode` (the single place every draw resolves GL blend state). Fill and line layers additionally get `fill-layer-opacity` / `line-layer-opacity` pinned just under 1 by `style-mapper`, which elects MapLibre 6's render-to-texture composite so a layer blends **as a whole** rather than once per overlapping polygon. `installLayerBlendModes` feature-detects every seam and disables the feature (hiding the Style-panel control) rather than breaking the map, so drift fails *quietly* — which is why `tests/layer-blend-modes.test.ts` asserts the seams and `e2e/blend-modes.spec.ts` asserts real pixels. Run both whenever `maplibre-gl` is bumped, including Dependabot PRs. **Do not add a blend mode without checking it in the browser**: MapLibre's blend state covers the alpha channel too, and it composites a blended layer as one viewport-filling quad, so any mode that does not reduce to "leave the destination alone" at zero source alpha repaints the whole map. That is what disqualified `darken` (a `MIN` equation erased the entire basemap to transparent black) and `subtract` (a reverse subtract left the canvas at `dstA - srcA`, showing the page through the layer); the shipped list is `BLEND_MODES` in `@geolibre/core`, and both the unit test's blend simulator and the e2e spec pin their exclusion. Only `fill` and `line` have a `*-layer-opacity` in the style spec, so only they blend as a **whole layer**; `circle` and `fill-extrusion` blend per symbol and visibly double-darken where symbols overlap on screen (measured under Multiply: `rgb(23, 77, 220)` in the overlap vs `rgb(76, 136, 222)` on a single symbol). That is upstream's limitation, documented in `docs/user-guide/layers.md`; the test "has a layer-level composite for fill and line only" fails if a bump adds one of the missing properties, at which point extend `COMPOSITE_LAYER_TYPES` and `style-mapper` together and drop the caveat. The Style-panel control (`blendModeControl` in `StylePanel.tsx`, rendered in each of its terminal branches) is gated on `!pluginOwnsPaint && !controlRendersLayer`: blending only reaches layers **GeoLibre itself paints**, so anything a control renders or paints (3D Tiles, Gaussian splats, LiDAR, the COG raster control, and Add Vector Layer, which sets `customLayerType` *and* `controlOwnsPaint`) is excluded -- layer-sync never applies `fillPaint`/`linePaint` to those, so the `*-layer-opacity` that elects the composite never lands and a Blend menu there would silently do nothing. Keep `docs/user-guide/layers.md` and `tests/layer-blend-modes.test.ts` ("the layer kinds the Blend control is offered for") in step with that gate; build the test's mocks the way the real controls build their metadata, or they pass on shapes that never occur.
- **The PMTiles control's layer ids** (`pmtilesControlLayerId` / `pmtilesIdsForSourceLayers` / `pmtilesIdNamesSourceLayer`, `packages/map/src/pmtiles-layer.ts`, read from `layer-sync.ts` and `packages/plugins/src/plugins/maplibre-components.ts`) mirror an unexported fact about `maplibre-gl-components`' `PMTilesLayerControl`: it names its MapLibre layers `${sourceId}-${name}-${kind}` from the **raw** source-layer name, where `pmtilesVectorLayerId` percent-encodes it. The two agree for every name needing no encoding, so a store layer carrying the control's own ids — the archive kept whole, or a split part, which keeps the ids naming its own source layer — is recognised under the encoded scheme alone until a name holds a `/`, a space or non-ASCII. Then `layer-sync` decides the source layer has no native layer and adds a **second** fill/line/circle trio on top of the control's: drawn twice, and only the control's copy answers the panel. Both schemes are therefore matched, and only ids naming a source layer the store actually holds are kept. What the user **ticked** is deliberately _not_ inferred from those ids: `selectedSourceLayers` is a documented field of the exported `PMTilesLayerControlState` handed to every handler, so `pmtilesLayerOptions` reads it and the compiler checks it — the rules for a stale selection, and for the archive ids the control reuses across a panel close, are written at that function and at `addPMTilesArchive`. A reused id is the one case GeoLibre cannot repair: two archives then name one MapLibre source, the first to sync wins it and the other draws nothing, so `addPMTilesArchive` warns rather than pretending otherwise — while an archive that takes a layer over outright is drawn correctly, keeping the name, folder and styling of the one it replaced. Whenever `maplibre-gl-components` is bumped in `packages/plugins/package.json` — including Dependabot PRs — run the frontend suite: `tests/pmtiles-control-contract.test.ts` drives a **real** `PMTilesLayerControl` against a real archive, through the real `layeradd` handler into the store, and fails if the id scheme moves or the selection stops reaching the handler.
- `GeoLibreCogRenderEngine` (`packages/plugins/src/types.ts`) mirrors the `RenderEngine` union `maplibre-gl-raster` exports (`maplibre-gl-raster` | `cog-tiler-wasm` | `titiler`). It is hand-written rather than imported because `types.ts` is the public plugin-API surface and importing there would make that package's types a hard dependency of every external plugin. Unlike the mirrors above this one is checked by the **compiler**, not a test: `CogRenderEngineMirrorIsExact` in `packages/plugins/src/plugins/maplibre-raster.ts` asserts both directions of assignability against the real imported type, so a renamed or dropped engine identifier fails `npm run typecheck`. Nothing extra to do on a `maplibre-gl-raster` bump beyond letting the build run; without it a stale identifier would reach `control.setEngine()` as a string the control no longer recognizes, silently leaving the raster unrendered.
- `propertySpecFor` (`packages/core/src/expressions.ts`) fabricates the **unexported** `StylePropertySpecification` shape that `@maplibre/maplibre-gl-style-spec`'s `createExpression` uses for expected-result-type enforcement (the Expression Builder's filter → boolean / color checks). The cast hides any contract change from the compiler, so whenever `@maplibre/maplibre-gl-style-spec` is bumped (including Dependabot PRs) run the frontend suite — the "enforces an expected result type" test in `tests/expressions.test.ts` fails if the shape stops being honored.
- `DISTANCE_SEGMENTS` / `NON_DISTANCE_NAMES` (`apps/geolibre-desktop/src/lib/whitebox-distance-params.ts`) decide, by parameter _name_, which Whitebox parameters are ground distances and so get the Processing dialog's metric unit picker (GeoLibre#1540). The segments are generic (`tolerance`, `radius`, `length`, `resolution`), so a tool can carry a matching name that is not a length — `corridor_tolerance` is a 0-1 fraction. Those are safe today only because the picker is confined to tools whose every dataset input is a vector layer, and the colliding names happen to sit on imagery/LiDAR tools; that is a coincidence, not a guarantee. So whenever `geolibre-wasm` is bumped (in `packages/processing/package.json`) — including Dependabot PRs — scan the new catalog for a `double` matching the rule whose description reads as a fraction, ratio, angle or weight, and add it to `NON_DISTANCE_NAMES`. If one is missed, that tool's field offers metres and silently converts a dimensionless number as if it were a distance, with no build error.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,9 @@ export function BasemapExtractPanel({ open, onClose, mapControllerRef }: Basemap
setBasemapStyleUrl(registerOfflineBasemapStyle(layerId, style));
trackStyledBasemap(layerId, `${layerId}.pmtiles`);
} else {
// One layer, deliberately: an extract is a backdrop to draw over, not the thing being
// inspected, so it stays a single row rather than being split per source layer the way an
// archive added from the PMTiles control or a STAC asset is.
addLayer(
createPMTilesStoreLayer({
id: layerId,
Expand Down
11 changes: 8 additions & 3 deletions packages/map/src/headless.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/
import type { GeoLibreLayer } from "@geolibre/core";
import type * as maplibregl from "maplibre-gl";
import { removeLayerFromMap, syncLayer } from "./layer-sync";
import { externalSourceIdsFor, removeLayerFromMap, syncLayer } from "./layer-sync";
import { installMapTransformCompat as _installMapTransformCompat } from "./map-transform-compat";
export { installMapTransformCompat } from "./map-transform-compat";

Expand All @@ -36,8 +36,13 @@ export function createLayerSync(map: maplibregl.Map): LayerSync {
sync(layers) {
const nextIds = new Set(layers.map((layer) => layer.id));
const previousById = new Map(synced.map((layer) => [layer.id, layer]));
// Every layer in `layers` is on the map when this sync ends — including the ones the reorder
// loop below takes off and puts straight back — so a source any of them draws from stays.
const survivingSourceIds = externalSourceIdsFor(layers);
for (const previous of synced) {
if (!nextIds.has(previous.id)) removeLayerFromMap(map, previous.id, previous);
if (!nextIds.has(previous.id)) {
removeLayerFromMap(map, previous.id, previous, survivingSourceIds);
}
}

// Input order is bottom-to-top: each addLayer without an anchor lands on
Expand All @@ -60,7 +65,7 @@ export function createLayerSync(map: maplibregl.Map): LayerSync {
}
for (let index = rebuildFrom; index < layers.length; index += 1) {
const previous = previousById.get(layers[index].id);
if (previous) removeLayerFromMap(map, layers[index].id, previous);
if (previous) removeLayerFromMap(map, layers[index].id, previous, survivingSourceIds);
}

for (const layer of layers) syncLayer(map, layer);
Expand Down
87 changes: 57 additions & 30 deletions packages/map/src/layer-sync.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
import {
controlRendersLayer,
DEFAULT_LAYER_STYLE,
type GeoLibreLayer,
type ExternalNativePaintBridge,
generatorCircleRadiusValue,
geojsonHasZCoordinates,
getExternalNativePaintBridge,
type LayerStyle,
pluginOwnsPaint,
proportionalRadiusExpression,
ruleBasedVisibilityFilter,
shouldUseTiledRendering,
styleValue,
type ExternalNativePaintBridge,
type GeoLibreLayer,
type LayerStyle,
validateMapExpression,
} from "@geolibre/core";
import { normalizePMTilesUrl, PMTILES_PROTOCOL, pmtilesVectorLayerId } from "./pmtiles-layer";
import {
normalizePMTilesUrl,
PMTILES_PROTOCOL,
pmtilesControlLayerId,
pmtilesIdNamesSourceLayer,
pmtilesLayerKinds,
pmtilesVectorLayerId,
} from "./pmtiles-layer";
import { encodeVectorTileLayerPart } from "./vector-tile-layer-ids";
import { addProtocol, config } from "maplibre-gl";
import type { GeoJSON } from "geojson";
Expand Down Expand Up @@ -912,18 +919,9 @@ function ensurePMTilesExternalLayer(
}

for (const sourceLayer of sourceLayers) {
const fillId = getPMTilesNativeLayerId(
nativeLayerIds,
pmtilesVectorLayerId(sourceId, sourceLayer, "fill"),
);
const lineId = getPMTilesNativeLayerId(
nativeLayerIds,
pmtilesVectorLayerId(sourceId, sourceLayer, "line"),
);
const circleId = getPMTilesNativeLayerId(
nativeLayerIds,
pmtilesVectorLayerId(sourceId, sourceLayer, "circle"),
);
const fillId = getPMTilesNativeLayerId(nativeLayerIds, sourceId, sourceLayer, "fill");
const lineId = getPMTilesNativeLayerId(nativeLayerIds, sourceId, sourceLayer, "line");
const circleId = getPMTilesNativeLayerId(nativeLayerIds, sourceId, sourceLayer, "circle");

ensureLayer(
map,
Expand Down Expand Up @@ -1116,22 +1114,12 @@ function getPMTilesRenderableSourceLayers(
): string[] {
const sourceLayers = getPMTilesSourceLayers(layer);
const savedSourceLayers = sourceLayers.filter((sourceLayer) =>
hasPMTilesNativeSourceLayer(nativeLayerIds, sourceId, sourceLayer),
pmtilesIdNamesSourceLayer(nativeLayerIds, sourceId, sourceLayer),
);

return savedSourceLayers.length > 0 ? savedSourceLayers : sourceLayers;
}

function hasPMTilesNativeSourceLayer(
nativeLayerIds: string[],
sourceId: string,
sourceLayer: string,
): boolean {
return ["fill", "line", "circle"].some((kind) =>
nativeLayerIds.includes(pmtilesVectorLayerId(sourceId, sourceLayer, kind)),
);
}

function getPMTilesSourceLayers(layer: GeoLibreLayer): string[] {
const sourceLayers = layer.source.sourceLayers ?? layer.metadata.sourceLayers;
return Array.isArray(sourceLayers)
Expand All @@ -1142,8 +1130,20 @@ function getPMTilesSourceLayers(layer: GeoLibreLayer): string[] {
: [];
}

function getPMTilesNativeLayerId(nativeLayerIds: string[], fallbackId: string): string {
return nativeLayerIds.find((nativeLayerId) => nativeLayerId === fallbackId) ?? fallbackId;
/**
* The id this source layer is already drawn under, or the one to draw it under. Both schemes are
* consulted — see `pmtilesControlLayerId` — or a control-added layer gets a second set over it.
*/
function getPMTilesNativeLayerId(
nativeLayerIds: string[],
sourceId: string,
sourceLayer: string,
kind: (typeof pmtilesLayerKinds)[number],
): string {
const encoded = pmtilesVectorLayerId(sourceId, sourceLayer, kind);
if (nativeLayerIds.includes(encoded)) return encoded;
const raw = pmtilesControlLayerId(sourceId, sourceLayer, kind);
return nativeLayerIds.includes(raw) ? raw : encoded;
}

function isWaybackExternalRasterLayer(layer: GeoLibreLayer): boolean {
Expand Down Expand Up @@ -3629,10 +3629,16 @@ function moveLayer(map: maplibregl.Map, id: string, beforeId?: string): void {
}
}

/** The external sources a set of layers draws from — what a removal must not pull out from under. */
export function externalSourceIdsFor(layers: readonly GeoLibreLayer[]): Set<string> {
return new Set(layers.flatMap((layer) => getExternalSourceIds(layer)));
}

export function removeLayerFromMap(
map: maplibregl.Map,
layerId: string,
layer?: GeoLibreLayer,
survivingSourceIds?: ReadonlySet<string>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

survivingSourceIds is optional and only ever passed by MapController.syncLayers (via externalSourceIdsFor(layers)). The other two call sites that can remove a shared PMTiles-archive layer — headless.ts's createLayerSync().sync()/.dispose() — never pass it, so those paths lean entirely on the stillDrawn map-style scan below.

That scan does protect a straightforward single removal, but createLayerSync's rebuildFrom reorder loop can call removeLayerFromMap for several siblings of the same split archive back-to-back within one sync() call purely to reposition them (they're re-added moments later via syncLayer). If the last sibling processed in that inner loop is also the last one still drawing from the shared source at that instant, the source gets removed and then immediately recreated once syncLayer re-adds it — a same-tick self-heal, but it can needlessly evict and refetch a shared archive's tiles on a reorder that touches more than one of its layers, which couldn't happen before this PR since every PMTiles layer had its own unshared source.

createPMTilesArchiveLayers and createLayerSync are both public exports (@geolibre/map, @geolibre/map/headless), so an external headless consumer combining them would hit this. Worth threading externalSourceIdsFor(layers) through headless.ts too, mirroring map-controller.ts. Confidence: medium-low — this is a same-tick self-heal (not a stuck/broken state) and isn't covered by tests either way.

): void {
// Drop cached paint-bridge state so a later layer reusing this id never
// skips a fresh opacity/visibility apply against a new bridge.
Expand Down Expand Up @@ -3667,14 +3673,35 @@ export function removeLayerFromMap(
]) {
if (map.getLayer(id)) map.removeLayer(id);
}
// An archive's source layers share one source, so it goes only once nothing draws from it. The
// store half covers a layer that survives this sync; the map half covers its siblings inside one
// — deleting a folder removes its children in a single pass, and MapLibre reports removing a
// source still under a style layer as an error the user can do nothing about.
const stillInUse = survivingSourceIds ?? new Set<string>();
// Only an external source can be shared — the derived ids below are this layer's alone — so the
// map is asked at most once, and only when a shareable source is actually up for removal. Walked
// layer by layer rather than read from `getStyle()`, which serializes the whole document.
const shareable = new Set(getExternalSourceIds(layer));
let drawnSources: Set<string> | undefined;
const stillDrawn = (src: string): boolean => {
drawnSources ??= new Set(
map
.getLayersOrder()
.map((styleLayerId) => map.getLayer(styleLayerId)?.source)
.filter((source): source is string => typeof source === "string"),
);
return drawnSources.has(src);
};
Comment on lines +3681 to +3694

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance: this scan now runs for every removal of any layer with an external source, not just shared PMTiles archives.

shareable is populated from getExternalSourceIds(layer), which is non-empty for any layer kind that sets metadata.sourceId/sourceIds — MBTiles, vector-tile, Esri Wayback, 3D Tiles, etc. (many plugins set this). For all of those, stillDrawn now does an O(current style layers) walk (getLayersOrder() + getLayer() per id) on every single removal, even though most of these layer kinds don't actually share a source with siblings — only the new PMTiles-archive split case does.

Removing many such layers in one pass (e.g. deleting a project full of external-source layers, or dispose() in headless.ts) turns what was an O(1)-per-removal source cleanup into effectively O(n²) for n external-source layers, since each removal re-derives drawnSources from scratch (the drawnSources cache is scoped per removeLayerFromMap call, not shared across the batch).

Confidence: medium — likely fine in practice given typical layer counts, but worth confirming this doesn't show up as a regression on large projects/imports with many external layers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low confidence — minor perf note.

stillDrawn is lazily memoized within one removeLayerFromMap call, but when a whole archive (or any group of layers sharing an external source) is removed together, removeLayerFromMap runs once per sibling in a loop, and each call recomputes map.getLayersOrder() plus a getLayer(...).source lookup for every style layer on the map. For a large map (many layers) and an archive split into many source layers, that's O(siblings × total map layers). The comment above already explains why getStyle() was avoided in favor of this walk, so this is likely an accepted tradeoff — just flagging in case a large archive on a busy map turns out to be noticeably slow to delete.

for (const src of [
...getExternalSourceIds(layer),
sourceId(layerId),
labelSourceId(layerId),
invertedSourceId(layerId),
generatorSourceId(layerId),
]) {
if (src && map.getSource(src)) map.removeSource(src);
if (!src || stillInUse.has(src) || !map.getSource(src)) continue;
if (shareable.has(src) && stillDrawn(src)) continue;
map.removeSource(src);
}
// Drop radius-override tracking for the removed layer's native ids so a
// later layer reusing an id never inherits a stale restore.
Expand Down
5 changes: 4 additions & 1 deletion packages/map/src/map-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
} from "./geojson-loader";
import {
mbtilesStyleLayerIds,
externalSourceIdsFor,
removeLayerFromMap,
styleValuesEqual,
syncLayer,
Expand Down Expand Up @@ -1242,9 +1243,11 @@ export class MapController {
const nextIds = layers.map((l) => l.id);
const nextIdSet = new Set(nextIds);
const previousLayers = new Map(this.syncedLayers.map((layer) => [layer.id, layer]));
// Built once for the whole pass: every removal below asks the same question of the same list.
const survivingSourceIds = externalSourceIdsFor(layers);
for (const id of this.layerIds) {
if (!nextIdSet.has(id)) {
removeLayerFromMap(map, id, previousLayers.get(id));
removeLayerFromMap(map, id, previousLayers.get(id), survivingSourceIds);
}
}

Expand Down
Loading
Loading