-
-
Notifications
You must be signed in to change notification settings - Fork 671
feat(layers): show a tiled archive as a group of its source layers #2065
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
3ac6ec9
c60e19c
5cf921a
87ff209
1fc142e
8278159
ee7dee9
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 |
|---|---|---|
| @@ -1,20 +1,25 @@ | ||
| 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, | ||
| pmtilesLayerKinds, | ||
| pmtilesVectorLayerId, | ||
| } from "./pmtiles-layer"; | ||
| import { encodeVectorTileLayerPart } from "./vector-tile-layer-ids"; | ||
| import { addProtocol, config } from "maplibre-gl"; | ||
| import type { GeoJSON } from "geojson"; | ||
|
|
@@ -1127,7 +1132,7 @@ function hasPMTilesNativeSourceLayer( | |
| sourceId: string, | ||
| sourceLayer: string, | ||
| ): boolean { | ||
| return ["fill", "line", "circle"].some((kind) => | ||
| return pmtilesLayerKinds.some((kind) => | ||
| nativeLayerIds.includes(pmtilesVectorLayerId(sourceId, sourceLayer, kind)), | ||
| ); | ||
| } | ||
|
|
@@ -3629,10 +3634,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>, | ||
| ): void { | ||
| // Drop cached paint-bridge state so a later layer reusing this id never | ||
| // skips a fresh opacity/visibility apply against a new bridge. | ||
|
|
@@ -3667,14 +3678,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
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. Performance: this scan now runs for every removal of any layer with an external source, not just shared PMTiles archives.
Removing many such layers in one pass (e.g. deleting a project full of external-source layers, or 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.
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. Low confidence — minor perf note.
|
||
| 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. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,9 @@ export function normalizePMTilesUrl(url: string): string { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return url.startsWith(`${PMTILES_PROTOCOL}://`) ? url : `${PMTILES_PROTOCOL}://${url}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** The MapLibre layers one source layer is drawn with. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const pmtilesLayerKinds = ["fill", "line", "circle"] as const; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function pmtilesVectorLayerId(sourceId: string, sourceLayer: string, kind: string): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `${sourceId}-${encodeVectorTileLayerPart(sourceLayer)}-${kind}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -39,7 +42,7 @@ export function pmtilesNativeLayerIds( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [`${sourceId}-raster`]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return sourceLayers.flatMap((sourceLayer) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ["fill", "line", "circle"].map((kind) => pmtilesVectorLayerId(sourceId, sourceLayer, kind)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pmtilesLayerKinds.map((kind) => pmtilesVectorLayerId(sourceId, sourceLayer, kind)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -61,6 +64,8 @@ export interface PMTilesStoreLayerOptions { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceLayerColors?: Record<string, string>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** The MapLibre ids a control created itself; derived from the naming scheme otherwise. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeLayerIds?: readonly string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** The MapLibre source to draw from, when it is not this layer's own — a shared archive. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceId?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -70,6 +75,7 @@ export interface PMTilesStoreLayerOptions { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function createPMTilesStoreLayer(options: PMTilesStoreLayerOptions): GeoLibreLayer { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { id, name, tileType } = options; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sourceId = options.sourceId ?? id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sourceLayers = [...options.sourceLayers]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const url = normalizePMTilesUrl(options.url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const fillColor = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -81,7 +87,7 @@ export function createPMTilesStoreLayer(options: PMTilesStoreLayerOptions): GeoL | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: "pmtiles", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceId: id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceLayers, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tileType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: tileType === "raster" ? "raster" : "vector", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -104,18 +110,67 @@ export function createPMTilesStoreLayer(options: PMTilesStoreLayerOptions): GeoL | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| metadata: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| externalNativeLayer: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeLayerIds: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(options.nativeLayerIds ?? pmtilesNativeLayerIds(id, tileType, sourceLayers)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(options.nativeLayerIds ?? pmtilesNativeLayerIds(sourceId, tileType, sourceLayers)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pickable: options.pickable ?? true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceId: id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceKind: "pmtiles-url", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(options.sourceLayerColors ? { sourceLayerColors: options.sourceLayerColors } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceLayers, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tileType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
156
to
166
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. Low confidence / quality note: this drops |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
156
to
167
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 (medium confidence): this hunk drops the The PR description says the "archive stays one layer" paths (STAC panel, offline basemap extract) "paint each [source layer] in the colour the archive assigned ( Neither seems to hold as shipped: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * One layer per source layer in a vector archive, so the Layers panel can show, reorder, style and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * hide them with the machinery it already has. Raster, or a single source layer, stays one layer. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * All of them name the archive's one MapLibre source, so removing one must not remove it — | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `removeLayerFromMap` refcounts it against the survivors. That id doubles as the refcount key; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * anything needing the two to differ needs its own field rather than a third reader of this one. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function createPMTilesArchiveLayers(options: PMTilesStoreLayerOptions): GeoLibreLayer[] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Keyed by the id each source layer would take, not by its name: `encodeVectorTileLayerPart` is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // not injective (`a/b` and `a_2Fb` both encode to `a_2Fb`), and an archive's metadata can repeat | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // a name outright. Either way a second layer would carry the first one's id. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.tileType === "raster") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Raster tiles never split, so the id math below means nothing for them. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [createPMTilesStoreLayer(options)]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const parts = new Map<string, string>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const sourceLayer of options.sourceLayers) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const id = `${options.id}-${encodeVectorTileLayerPart(sourceLayer)}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const taken = parts.get(id); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (taken === undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parts.set(id, sourceLayer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (taken !== sourceLayer) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Two different names, one id: the second's features would draw nowhere. Reported rather | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // than dropped in silence — the Diagnostics panel is where a user can see it. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.warn( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. Minor/quality (low confidence): this warns on every call to |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `PMTiles archive "${options.id}": source layer "${sourceLayer}" collides with "${taken}" and is not drawn.`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+191
to
+214
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. Minor: the collision-detection loop (and its |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (parts.size < 2) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Deduped here too, so one layer never carries the same source layer twice. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| createPMTilesStoreLayer({ ...options, sourceLayers: [...new Set(options.sourceLayers)] }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+215
to
+229
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. Bug (medium confidence): when an archive's source layers collide down to a single id ( Example: Using the already-deduped map instead would keep this path consistent with the split path:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+191
to
+230
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. Minor inconsistency: the collision-safe Today that's harmless ( |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [...parts].map(([id, sourceLayer]) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| createPMTilesStoreLayer({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...options, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: sourceLayer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceLayers: [sourceLayer], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // The archive's source, and so the archive's ids: a layer deriving its own would name ids | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // nothing on the map answers to. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceId: options.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeLayerIds: undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+178
to
+248
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. Bug (medium confidence):
Consider deduping (e.g.
Suggested change
Comment on lines
+178
to
+248
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 (low confidence, minor): when two source layer names collide under |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Facts about a PMTiles archive needed to build a GeoLibre layer for it. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface PMTilesArchiveInfo { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tileType: "vector" | "raster"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
survivingSourceIdsis optional and only ever passed byMapController.syncLayers(viaexternalSourceIdsFor(layers)). The other two call sites that can remove a shared PMTiles-archive layer —headless.ts'screateLayerSync().sync()/.dispose()— never pass it, so those paths lean entirely on thestillDrawnmap-style scan below.That scan does protect a straightforward single removal, but
createLayerSync'srebuildFromreorder loop can callremoveLayerFromMapfor several siblings of the same split archive back-to-back within onesync()call purely to reposition them (they're re-added moments later viasyncLayer). 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 oncesyncLayerre-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.createPMTilesArchiveLayersandcreateLayerSyncare both public exports (@geolibre/map,@geolibre/map/headless), so an external headless consumer combining them would hit this. Worth threadingexternalSourceIdsFor(layers)throughheadless.tstoo, mirroringmap-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.