From 83f76846aa4c1f99e4e2a54e8731a882191bef78 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 13:46:10 -0500 Subject: [PATCH 01/22] Add refreshLayer and a per-layer refresher hook to both engines --- .../MapEngines/Adapters/DeckGLAdapter.ts | 52 ++++++- .../MapEngines/Adapters/LeafletAdapter.ts | 43 ++++++ src/essence/Basics/MapEngines/IMapEngine.ts | 38 ++++- src/essence/Basics/MapEngines/types/layers.ts | 12 ++ tests/unit/engineRefreshLayer.spec.js | 138 ++++++++++++++++++ 5 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 tests/unit/engineRefreshLayer.spec.js diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index 456192158..66cdbb2b5 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -41,7 +41,8 @@ import type { MapInitOptions, BasemapOptions, } from '../types/view' -import type { LayerOptions, OverlayOptions } from '../types/layers' +import type { LayerOptions, OverlayOptions, RefreshContext } from '../types/layers' +import { compileTileUrl } from '../../Layers_/tileUrlUtils' import type { MapEventHandler, MapEventOptions, @@ -266,6 +267,8 @@ export class DeckGLAdapter implements IMapEngine { private _maxBounds: BoundsLike | null = null private _layers = new Map() + /** Per-layer refresh hooks, keyed by layer id. */ + private _refreshers = new Map Layer | void>() private _layerZIndices = new Map() private _layerIdCounter = 0 @@ -415,6 +418,7 @@ export class DeckGLAdapter implements IMapEngine { this._overlays.clear() this._layers.clear() + this._refreshers.clear() this._layerZIndices.clear() this._eventListeners.clear() this._featureClickHandler = null @@ -918,6 +922,7 @@ export class DeckGLAdapter implements IMapEngine { const id = resolveLayerId(layer) this._layers.delete(id) this._layerZIndices.delete(id) + this._refreshers.delete(id) this._syncLayers() } @@ -939,6 +944,51 @@ export class DeckGLAdapter implements IMapEngine { return updated } + registerLayer(id: string, layer: Layer): void { + // For deck.gl, holding a layer is rendering it — there is no + // "registered but not on the map" state. Keyed by the caller's id + // rather than layer.id so the two can never drift apart. + this._layers.set(id, layer) + this._syncLayers() + } + + setLayerRefresher( + id: string, + refresh: ((layer: Layer, ctx: RefreshContext) => Layer | void) | null + ): void { + if (refresh == null) this._refreshers.delete(id) + else this._refreshers.set(id, refresh) + } + + refreshLayer(id: string, ctx: RefreshContext = {}): boolean { + const existing = this._layers.get(id) + if (!existing) return false + + const refresh = this._refreshers.get(id) + let next: Layer | void + if (refresh) { + next = refresh(existing, { + url: ctx.url, + tileOptions: ctx.tileOptions, + force: ctx.force, + }) + } else { + // Plain tile layers take one static URL, so the per-tile params + // Leaflet would add are baked in here. + if (ctx.url == null) return false + const compiled = compileTileUrl(ctx.url, ctx.tileOptions ?? {}) + // A layer with no resolvable service URL compiles to nothing; + // handing that to deck would blank it. + if (!compiled) return false + next = existing.clone({ data: compiled }) as Layer + } + + // A refresher that mutated in place returns nothing; keep what we hold. + if (next) this._layers.set(id, next) + this._syncLayers() + return true + } + /** * Assign a logical z-index. deck.gl renders layers in array order (index 0 = bottom), * so this re-sorts the internal map by ascending z-index. diff --git a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts index ab3a033d2..cf8c92fdd 100644 --- a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts @@ -29,6 +29,7 @@ import { TileLayerOptions, MarkerOptions, OverlayOptions, + RefreshContext, } from '../types/layers' import { IMapEngineMarkers } from '../IMapEngineMarkers' import { @@ -87,6 +88,9 @@ export default class LeafletAdapter implements IMapEngine, IMapEn */ private _layers: Map = new Map() + /** Per-layer refresh hooks, keyed the same way as {@link _layers}. */ + private _refreshers: Map any> = new Map() + /** * Registry of markers by ID */ @@ -328,6 +332,7 @@ export default class LeafletAdapter implements IMapEngine, IMapEn this._detachFeatureHoverListeners() this._layers.clear() + this._refreshers.clear() this._markers.clear() this._map.remove() @@ -697,6 +702,7 @@ export default class LeafletAdapter implements IMapEngine, IMapEn if (leafletLayer) { this._map.removeLayer(leafletLayer) this._layers.delete(layer) + this._refreshers.delete(layer) } } else { this._map.removeLayer(layer) @@ -754,6 +760,43 @@ export default class LeafletAdapter implements IMapEngine, IMapEn return leafletLayer } + registerLayer(id: string, layer: any): void { + // resolveLeafletLayerId reads _mmgisId, so stamp it: the layer must be + // findable by object as well as by id. + if (layer != null && typeof layer === 'object') layer._mmgisId = id + this._layers.set(id, layer) + } + + setLayerRefresher( + id: string, + refresh: ((layer: any, ctx: RefreshContext) => any) | null + ): void { + if (refresh == null) this._refreshers.delete(id) + else this._refreshers.set(id, refresh) + } + + refreshLayer(id: string, ctx: RefreshContext = {}): boolean { + const layer = this._layers.get(id) + if (!layer) return false + + const refresh = this._refreshers.get(id) + if (refresh) { + refresh(layer, { + url: ctx.url, + tileOptions: ctx.tileOptions, + force: ctx.force, + }) + return true + } + + // A Leaflet tile layer recompiles its URL per tile from this.options, + // which is what refresh() merges tileOptions into — that is why Leaflet + // keeps its tile cache where deck.gl cannot. + if (typeof layer.refresh !== 'function') return false + layer.refresh(ctx.url, ctx.force === true, ctx.tileOptions) + return true + } + setLayerZIndex(layer: any | string, zIndex: number): void { const leafletLayer = typeof layer === 'string' ? this._layers.get(layer) : layer if (leafletLayer && typeof leafletLayer.setZIndex === 'function') { diff --git a/src/essence/Basics/MapEngines/IMapEngine.ts b/src/essence/Basics/MapEngines/IMapEngine.ts index 2951ac57d..de07ca87e 100644 --- a/src/essence/Basics/MapEngines/IMapEngine.ts +++ b/src/essence/Basics/MapEngines/IMapEngine.ts @@ -6,7 +6,7 @@ import { FitBoundsOptions, MapInitOptions, } from './types/view' -import { LayerOptions, OverlayOptions } from './types/layers' +import { LayerOptions, OverlayOptions, RefreshContext } from './types/layers' import { MapEventHandler, MapEventOptions, @@ -200,6 +200,42 @@ export interface IMapEngine< */ updateLayer(layer: TLayer | string, options: Partial): TLayer + /** + * Take ownership of an externally-built native layer under `id`, so + * id-addressed methods can find it. Does not change what is on the map — + * `addLayer` still does that. + * + * Leaflet needs this because MMGIS builds its tile layers itself and hands + * them to `addLayer` as native objects, which carry no id. deck.gl layers + * already carry `id`, so its implementation delegates to `addLayer`. + */ + registerLayer(id: string, layer: TLayer): void + + /** + * Register how one layer recomputes itself, or pass null to clear. + * + * Called by the module that owns the layer kind, at creation — never by an + * adapter, which stays layer-type-agnostic. The engine invokes it with the + * live instance; return a replacement (deck.gl) or mutate in place and + * return nothing (Leaflet). The engine reconciles either way and remains + * the owner, so the function must not retain the instance. + */ + setLayerRefresher( + id: string, + refresh: ((layer: TLayer, ctx: RefreshContext) => TLayer | void) | null + ): void + + /** + * Re-render a layer from its current configuration. + * + * The single update entry point for time changes, colormap/rescale changes + * and any other "your config moved, redraw" event. Callers never branch on + * the active engine or renderer. + * + * @returns Whether the engine had a layer to refresh. + */ + refreshLayer(id: string, ctx?: RefreshContext): boolean + /** * Set the z index of a layer to control draw order. */ diff --git a/src/essence/Basics/MapEngines/types/layers.ts b/src/essence/Basics/MapEngines/types/layers.ts index 5c0642dec..5de149de1 100644 --- a/src/essence/Basics/MapEngines/types/layers.ts +++ b/src/essence/Basics/MapEngines/types/layers.ts @@ -165,3 +165,15 @@ export interface OverlayOptions { latlng: LatLngLike mount: (node: HTMLElement) => (() => void) | void } + +/** + * What a caller knows about a refresh, independent of engine. `url` is the + * *uncompiled* tile source URL — Leaflet recompiles per tile from + * `tileOptions`, deck.gl bakes them in with `compileTileUrl`. A refresher that + * derives its own URL (client-side COG) ignores both. + */ +export type RefreshContext = { + url?: string + tileOptions?: Record + force?: boolean +} diff --git a/tests/unit/engineRefreshLayer.spec.js b/tests/unit/engineRefreshLayer.spec.js new file mode 100644 index 000000000..bcf2a63fb --- /dev/null +++ b/tests/unit/engineRefreshLayer.spec.js @@ -0,0 +1,138 @@ +import { describe, test, expect, vi } from 'vitest' +import LeafletAdapter from '../../src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts' +import DeckGLAdapter from '../../src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts' + +/** + * refreshLayer is the single update entry point both engines expose (#274). + * The registered refresher decides WHAT changes; the adapter decides HOW to + * apply it — Leaflet mutates in place, deck.gl clones and re-syncs. + */ + +// A Leaflet tile layer: mutates in place, exposes the native refresh(). +const makeLeafletTileLayer = () => ({ + refreshed: null, + refresh(url, force, tileOptions) { + this.refreshed = { url, force, tileOptions } + }, +}) + +// A deck.gl layer: immutable, clone() returns a new instance carrying the patch. +const makeDeckLayer = (id, props = {}) => ({ + id, + props: { id, ...props }, + clone(patch) { + return makeDeckLayer(id, { ...props, ...patch }) + }, +}) + +describe('LeafletAdapter.refreshLayer', () => { + test('calls the native refresh() when no refresher is registered', () => { + const adapter = new LeafletAdapter() + const layer = makeLeafletTileLayer() + adapter.registerLayer('l1', layer) + + expect( + adapter.refreshLayer('l1', { + url: 'https://x/{z}/{x}/{y}.png', + tileOptions: { colormap: 'viridis' }, + force: true, + }) + ).toBe(true) + expect(layer.refreshed).toEqual({ + url: 'https://x/{z}/{x}/{y}.png', + force: true, + tileOptions: { colormap: 'viridis' }, + }) + }) + + test('prefers a registered refresher over the native refresh()', () => { + const adapter = new LeafletAdapter() + const layer = makeLeafletTileLayer() + adapter.registerLayer('l1', layer) + const refresh = vi.fn() + adapter.setLayerRefresher('l1', refresh) + + expect(adapter.refreshLayer('l1', { url: 'u' })).toBe(true) + expect(refresh).toHaveBeenCalledWith(layer, { + url: 'u', + tileOptions: undefined, + force: undefined, + }) + expect(layer.refreshed).toBe(null) + }) + + test('returns false for an unregistered id', () => { + expect(new LeafletAdapter().refreshLayer('nope')).toBe(false) + }) + + test('returns false for a layer with neither a refresher nor refresh()', () => { + const adapter = new LeafletAdapter() + adapter.registerLayer('l1', {}) + expect(adapter.refreshLayer('l1')).toBe(false) + }) + + test('setLayerRefresher(id, null) clears the hook', () => { + const adapter = new LeafletAdapter() + const layer = makeLeafletTileLayer() + adapter.registerLayer('l1', layer) + adapter.setLayerRefresher('l1', vi.fn()) + adapter.setLayerRefresher('l1', null) + + adapter.refreshLayer('l1', { url: 'u' }) + expect(layer.refreshed).not.toBe(null) + }) +}) + +describe('DeckGLAdapter.refreshLayer', () => { + test('adopts the instance a refresher returns', () => { + const adapter = new DeckGLAdapter() + const original = makeDeckLayer('l1', { geotiff: 'a.tif' }) + adapter.addLayer(original) + adapter.setLayerRefresher('l1', (layer) => + layer.clone({ geotiff: 'b.tif' }) + ) + + expect(adapter.refreshLayer('l1')).toBe(true) + const held = adapter.getLayers().find((l) => l.id === 'l1') + expect(held).not.toBe(original) + expect(held.props.geotiff).toBe('b.tif') + }) + + test('keeps the existing instance when the refresher returns nothing', () => { + const adapter = new DeckGLAdapter() + const original = makeDeckLayer('l1') + adapter.addLayer(original) + adapter.setLayerRefresher('l1', () => {}) + + expect(adapter.refreshLayer('l1')).toBe(true) + expect(adapter.getLayers().find((l) => l.id === 'l1')).toBe(original) + }) + + test('falls back to cloning with a compiled tile URL', () => { + const adapter = new DeckGLAdapter() + adapter.addLayer(makeDeckLayer('l1', { data: 'old' })) + + expect( + adapter.refreshLayer('l1', { + url: 'https://x/{z}/{x}/{y}.png', + tileOptions: {}, + }) + ).toBe(true) + expect( + adapter.getLayers().find((l) => l.id === 'l1').props.data + ).toBe('https://x/{z}/{x}/{y}.png') + }) + + test('returns false for an unregistered id', () => { + expect(new DeckGLAdapter().refreshLayer('nope')).toBe(false) + }) + + test('does not clone when the fallback has no url to compile', () => { + const adapter = new DeckGLAdapter() + const original = makeDeckLayer('l1', { data: 'old' }) + adapter.addLayer(original) + + expect(adapter.refreshLayer('l1')).toBe(false) + expect(adapter.getLayers().find((l) => l.id === 'l1')).toBe(original) + }) +}) From a73fea1edf04bbbe7abb53e978aa8bfeaf6d6c98 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 13:52:25 -0500 Subject: [PATCH 02/22] Derive COG layer props through one function for creation and refresh --- .../MapEngines/Adapters/DeckCOGLayer.ts | 49 ++++++++++++------- tests/unit/buildDeckCOGLayer.spec.js | 34 +++++++++++++ 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts b/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts index 94f3d832f..909045cae 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts @@ -395,36 +395,27 @@ function makeRenderTile(opts: { // --------------------------------------------------------------------------- /** - * Build the client-side COG layer for the deck.gl engine's makeTileLayer path. - * - * A plain `COGLayer` — the colormap/rescale behaviour rides on the - * `getTileData` and `renderTile` props rather than a subclass, so there is no - * layer-owned GPU state to keep in sync (see `colormapTextures`). - * - * @param id - Layer id (layer name from layerObj). - * @param options - Raw COG file URL + layer config. `rawCogUrl` is the bare - * `.tif` URL with no TiTiler host or query params - * (resolveTileLayerSource's `fileUrl`). + * The complete prop bag for a client-side COG layer, derived from current + * config. Creation and every refresh go through this one function so a + * colormap default can never be computed two different ways. */ -export function buildDeckCOGLayer( +export function deckCOGProps( id: string, options: { rawCogUrl: string layerObj: Record opacity?: number } -): Layer { +): Record { const l = options.layerObj const colormapName = (l.currentCogColormap ?? l.cogColormap ?? 'viridis') as string const rescaleMin = Number(l.currentCogMin ?? l.cogMin ?? 0) const rescaleMax = Number(l.currentCogMax ?? l.cogMax ?? 1) const nodata = l.cogNoData != null ? Number(l.cogNoData) : null - // Derived here rather than passed by callers so every rebuild path - // (creation, colormap/rescale refresh, time reload) keeps the same limits. const minZoom = parseInt(l.minZoom) const maxZoom = parseInt(l.maxZoom) - return new COGLayer({ + return { id, geotiff: options.rawCogUrl, opacity: options.opacity ?? 1, @@ -433,8 +424,7 @@ export function buildDeckCOGLayer( // Supplying getTileData + renderTile together makes COGLayer._parseGeoTIFF // skip its default inferRenderPipeline, which throws for float COGs // ('non-unsigned integers not yet supported'). - // The config nodata (if any) overrides the file's GDAL_NODATA. - getTileData: (image, opts) => + getTileData: (image: any, opts: any) => cogGetTileData(image, { ...opts, noDataOverride: nodata }), renderTile: makeRenderTile({ colormapName, rescaleMin, rescaleMax }), updateTriggers: { @@ -443,5 +433,28 @@ export function buildDeckCOGLayer( // comes from the file's GDAL_NODATA tag, fixed for a given URL. renderTile: [colormapName, rescaleMin, rescaleMax], }, - }) as unknown as Layer + } +} + +/** + * Build the client-side COG layer for the deck.gl engine's makeTileLayer path. + * + * A plain `COGLayer` — the colormap/rescale behaviour rides on the + * `getTileData` and `renderTile` props rather than a subclass, so there is no + * layer-owned GPU state to keep in sync (see `colormapTextures`). + * + * @param id - Layer id (layer name from layerObj). + * @param options - Raw COG file URL + layer config. `rawCogUrl` is the bare + * `.tif` URL with no TiTiler host or query params + * (resolveTileLayerSource's `fileUrl`). + */ +export function buildDeckCOGLayer( + id: string, + options: { + rawCogUrl: string + layerObj: Record + opacity?: number + } +): Layer { + return new COGLayer(deckCOGProps(id, options) as any) as unknown as Layer } diff --git a/tests/unit/buildDeckCOGLayer.spec.js b/tests/unit/buildDeckCOGLayer.spec.js index f1bc641ca..005b52293 100644 --- a/tests/unit/buildDeckCOGLayer.spec.js +++ b/tests/unit/buildDeckCOGLayer.spec.js @@ -1,6 +1,7 @@ import { test, expect, describe } from 'vitest' import { buildDeckCOGLayer, + deckCOGProps, resolveNoDataValue, resolveRgbTextureFormat, } from '../../src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts' @@ -121,3 +122,36 @@ describe('resolveRgbTextureFormat', () => { expect(resolveRgbTextureFormat({ count: 4 }, tags(16))).toContain('unorm') }) }) + +describe('deckCOGProps', () => { + const options = { + rawCogUrl: 'https://example.com/a.tif', + layerObj: { minZoom: '3', maxZoom: '12', cogColormap: 'plasma', cogMin: 0, cogMax: 10 }, + opacity: 0.4, + } + + test('produces exactly the props buildDeckCOGLayer constructs with', () => { + const props = deckCOGProps('l1', options) + const layer = buildDeckCOGLayer('l1', options) + Object.keys(props).forEach((key) => { + // Closures differ by identity; compare what is comparable. + if (typeof props[key] === 'function') { + expect(typeof layer.props[key]).toBe('function') + } else { + expect(layer.props[key]).toEqual(props[key]) + } + }) + }) + + test('carries the id, url and opacity a refresh has to preserve', () => { + const props = deckCOGProps('l1', options) + expect(props.id).toBe('l1') + expect(props.geotiff).toBe('https://example.com/a.tif') + expect(props.opacity).toBe(0.4) + }) + + test('defaults opacity to 1 but keeps an explicit 0', () => { + expect(deckCOGProps('l1', { ...options, opacity: undefined }).opacity).toBe(1) + expect(deckCOGProps('l1', { ...options, opacity: 0 }).opacity).toBe(0) + }) +}) From 28e218bbcd5c4a219e49e7721587fd7fe19b742f Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 13:55:59 -0500 Subject: [PATCH 03/22] Restore missing explanatory comments in deckCOGProps --- src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts b/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts index 909045cae..040bff827 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts @@ -412,6 +412,8 @@ export function deckCOGProps( const rescaleMin = Number(l.currentCogMin ?? l.cogMin ?? 0) const rescaleMax = Number(l.currentCogMax ?? l.cogMax ?? 1) const nodata = l.cogNoData != null ? Number(l.cogNoData) : null + // Derived here rather than passed by callers so every rebuild path + // (creation, colormap/rescale refresh, time reload) keeps the same limits. const minZoom = parseInt(l.minZoom) const maxZoom = parseInt(l.maxZoom) @@ -424,6 +426,7 @@ export function deckCOGProps( // Supplying getTileData + renderTile together makes COGLayer._parseGeoTIFF // skip its default inferRenderPipeline, which throws for float COGs // ('non-unsigned integers not yet supported'). + // The config nodata (if any) overrides the file's GDAL_NODATA. getTileData: (image: any, opts: any) => cogGetTileData(image, { ...opts, noDataOverride: nodata }), renderTile: makeRenderTile({ colormapName, rescaleMin, rescaleMax }), From 4695ff3091e2d548baead337978aefebbe9ef22e Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 14:03:29 -0500 Subject: [PATCH 04/22] Register each layer's refresher and engine id at creation Wires setLayerRefresher for deck.gl COG layers and registerLayer for Leaflet tile layers into makeTileLayer, so the engine can find and refresh them later. Both new calls are guarded to ctx.default === true: Map_.engine is always the main map's engine, and a non-default ctx targets a different map's own registry, so registering into Map_.engine there would collide with the main map's entry under the same uuid. Verified no other layer-creation path (velocity, rain, vectorGrid, tileLayer.gl, GeoRasterLayer, videoOverlay) produces a layer with a refresh() method, so the Leaflet tile layer registered here is the complete set reached by the old typeof tileLayer.refresh === 'function' guard. --- .../Basics/Layers_/deckCOGRefresher.js | 32 ++++++++++ src/essence/Basics/Map_/Map_.js | 29 ++++++++++ tests/unit/deckCOGRefresher.spec.js | 58 +++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 src/essence/Basics/Layers_/deckCOGRefresher.js create mode 100644 tests/unit/deckCOGRefresher.spec.js diff --git a/src/essence/Basics/Layers_/deckCOGRefresher.js b/src/essence/Basics/Layers_/deckCOGRefresher.js new file mode 100644 index 000000000..b3879c976 --- /dev/null +++ b/src/essence/Basics/Layers_/deckCOGRefresher.js @@ -0,0 +1,32 @@ +import { deckCOGProps } from '../MapEngines/Adapters/DeckCOGLayer' +import { resolveDeckCOGFileUrl } from './tileLayerSource' +import L_ from './Layers_' + +/** + * How a client-side COG layer recomputes itself, for + * `IMapEngine.setLayerRefresher`. + * + * Lives on the domain side, not in the adapter: it reads mission config and + * the opacity registry, which adapters must not know about. The engine only + * knows it has a function to call. + * + * Everything is re-derived per call — colormap, rescale, opacity and the + * time-substituted file URL — so a colormap change, a rescale change and a + * time change all flow through this one path. The returned instance keeps the + * layer's id, so deck.gl diffs it against the old one and cached tiles survive. + * + * @param {string} uuid - Layer UUID, also the engine-side layer id. + * @param {object} layerObj - The live `L_.layers.data` entry, read on each call. + * @returns {(layer: object) => object} A refresher returning the replacement. + */ +export function makeDeckCOGRefresher(uuid, layerObj) { + return (layer) => + layer.clone( + deckCOGProps(uuid, { + rawCogUrl: resolveDeckCOGFileUrl(layerObj), + layerObj, + // ?? not ||: an opacity of 0 is a real value, not "default to 1" + opacity: L_.layers.opacity[uuid] ?? 1, + }) + ) +} diff --git a/src/essence/Basics/Map_/Map_.js b/src/essence/Basics/Map_/Map_.js index 901bd66c2..a6308f68e 100644 --- a/src/essence/Basics/Map_/Map_.js +++ b/src/essence/Basics/Map_/Map_.js @@ -26,6 +26,7 @@ import { resolveDeckCOGFileUrl, syncTileFormatToConfig, } from '../Layers_/tileLayerSource' +import { makeDeckCOGRefresher } from '../Layers_/deckCOGRefresher' import { Kinds } from '../../../pre/tools' import DataShaders from '../../Ancillary/DataShaders' import calls from '../../../pre/calls' @@ -1669,6 +1670,21 @@ async function makeTileLayer(layerObj, mapContext = null) { // ?? not ||: an opacity of 0 is a real value, not "default to 1" opacity: ctx.layerRegistry.opacity[layerObj.name] ?? 1, }) + // Map_.engine is always the MAIN map's engine. A non-default ctx + // targets a different map with its own registry, so registering + // into Map_.engine here would collide with the main map's entry + // under the same uuid. Guard to the main path only; the deckRaster + // classification above and the buildDeckCOGLayer call are not + // similarly guarded today (pre-existing, out of scope here). + if (ctx.default === true) { + // The layer kind supplies how it rebuilds; the engine executes it. + // Registered here because this is where the deckRaster + // classification already happened. + Map_.engine.setLayerRefresher( + layerObj.name, + makeDeckCOGRefresher(layerObj.name, layerObj) + ) + } L_._layersLoaded[L_._layersOrdered.indexOf(layerObj.name)] = true allLayersLoaded() return @@ -1742,6 +1758,19 @@ async function makeTileLayer(layerObj, mapContext = null) { variables: layerObj.variables || {}, }) + // The engine addresses layers by id; a Leaflet layer MMGIS built itself + // carries none until it is registered. Without this, refreshLayer cannot + // find it and time reload silently stops working. + // Guarded to the main map: Map_.engine is always the MAIN map's engine, so + // registering a layer built for a secondary ctx (its own map/registry) + // would collide with the main map's entry under the same uuid. + if (ctx.default === true) { + Map_.engine.registerLayer( + layerObj.name, + ctx.layerRegistry.layer[layerObj.name] + ) + } + // Add to map if (ctx.default != true) { ctx.layerRegistry.layer[layerObj.name].addTo(ctx.map) diff --git a/tests/unit/deckCOGRefresher.spec.js b/tests/unit/deckCOGRefresher.spec.js new file mode 100644 index 000000000..11b4f9e13 --- /dev/null +++ b/tests/unit/deckCOGRefresher.spec.js @@ -0,0 +1,58 @@ +import { describe, test, expect, vi } from 'vitest' + +vi.mock('../../src/essence/Basics/Map_/Map_', () => ({ default: {} })) + +// L_.getUrl dereferences window.mmgisglobal on the tile branch; jsdom has none. +window.mmgisglobal = window.mmgisglobal || {} + +const { default: L_ } = await import( + '../../src/essence/Basics/Layers_/Layers_.js' +) +const { makeDeckCOGRefresher } = await import( + '../../src/essence/Basics/Layers_/deckCOGRefresher.js' +) + +/** + * The COG refresher is the layer kind's own answer to "your config moved" — + * it re-derives props from CURRENT config every time it is called, so a + * colormap change, a rescale change and a time change all flow through it. + */ +const makeDeckLayer = (id, props = {}) => ({ + id, + props: { id, ...props }, + clone(patch) { + return makeDeckLayer(id, { ...props, ...patch }) + }, +}) + +describe('makeDeckCOGRefresher', () => { + test('re-reads the colormap from config on each call', () => { + const layerObj = { name: 'l1', url: 'COG:a.tif', cogColormap: 'viridis' } + L_.layers.opacity.l1 = 1 + const refresh = makeDeckCOGRefresher('l1', layerObj) + + const first = refresh(makeDeckLayer('l1')) + expect(first.props.updateTriggers.renderTile[0]).toBe('viridis') + + layerObj.currentCogColormap = 'plasma' + const second = refresh(first) + expect(second.props.updateTriggers.renderTile[0]).toBe('plasma') + }) + + test('re-reads opacity from the registry rather than capturing it', () => { + const layerObj = { name: 'l2', url: 'COG:a.tif' } + L_.layers.opacity.l2 = 1 + const refresh = makeDeckCOGRefresher('l2', layerObj) + + L_.layers.opacity.l2 = 0.25 + expect(refresh(makeDeckLayer('l2')).props.opacity).toBe(0.25) + }) + + test('returns a new instance rather than mutating the old one', () => { + const layerObj = { name: 'l3', url: 'COG:a.tif' } + L_.layers.opacity.l3 = 1 + const original = makeDeckLayer('l3') + + expect(makeDeckCOGRefresher('l3', layerObj)(original)).not.toBe(original) + }) +}) From 9f9863bc0828ba9b7c279fe40522fde486f89072 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 14:23:29 -0500 Subject: [PATCH 05/22] Route time and colormap updates through one engine call TimeControl.reloadLayer's raster-tile branch, the layers:refresh provider, and refreshEngineFacadeTileLayer each used to decide separately whether to mutate a Leaflet layer, clone a deck.gl layer, or rebuild a client-side COG layer. All three now hand off to Map_.engine.refreshLayer(id, ctx) and let the engine (and each layer's registered refresher) decide how; the call sites only resolve the source URL and tile options. Deletes L_.rebuildDeckCOGLayer and the deckRaster/Leaflet-refresh branches it and the two other call sites carried. Renames refreshEngineFacadeTileLayer to refreshTileLayer since it no longer only serves the facade path. --- src/essence/Basics/Layers_/Layers_.js | 86 ++------------ .../Basics/TimeControl_/TimeControl.js | 62 ++-------- tests/unit/layersRefreshProvider.spec.js | 112 +++++++++--------- tests/unit/timeControlReloadLayer.spec.js | 89 +++++++------- 4 files changed, 125 insertions(+), 224 deletions(-) diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index 7ef8d83cf..7dad67f68 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -1,5 +1,4 @@ // Holds all layer data -import { buildDeckCOGLayer } from '../MapEngines/Adapters/DeckGLHelpers' import F_ from '../Formulae_/Formulae_' import Description from '../../Ancillary/Description' import Search from '../../Ancillary/Search' @@ -15,7 +14,6 @@ import { } from './tileLayerSource' import { buildTileUrlOptions, - compileTileUrl, cogSourceType, hasCogColormap, shouldUseDeckRaster, @@ -186,12 +184,7 @@ function layerBoundsFor(uuid) { } /** - * Rebuilds a facade-managed raster tile layer around a freshly compiled URL. - * - * A Leaflet tile layer recompiles its URL per tile from `this.options`, so its - * `refresh()` only has to merge the caller's overrides into those options. A - * facade-managed layer wraps one static URL instead, so the overrides are - * compiled in here. + * Refreshes a raster tile layer through the map engine's per-layer refresher. * * Resolution order — source, then time replacements, then tile-URL options — * is the one layer creation and time-driven reloads use, so all three agree on @@ -204,9 +197,9 @@ function layerBoundsFor(uuid) { * @param {string} uuid - Layer UUID, already resolved. * @param {object} [updateOptions] - Tile-URL option overrides, the keys * buildTileUrlOptions produces. These win over the layer config. - * @returns {Promise} Whether the engine took a new URL. + * @returns {Promise} Whether the engine had a layer to refresh. */ -async function refreshEngineFacadeTileLayer(uuid, updateOptions) { +async function refreshTileLayer(uuid, updateOptions) { const layerObj = L_.layers.data[uuid] // Only raster tiles carry a compiled tile URL. The other facade-managed // types (vector, vectortile, pointcloud) reload through their own paths. @@ -228,17 +221,13 @@ async function refreshEngineFacadeTileLayer(uuid, updateOptions) { ...(updateOptions || {}), } - // A layer with no resolvable service URL compiles to nothing. Handing - // that to the engine would blank it, so leave the existing one alone. - const nextUrl = compileTileUrl(sourceUrl, tileOptions) - if (!nextUrl) return false - - const updated = L_.Map_.engine.updateLayer(uuid, { url: nextUrl }) - // deck.gl layers are immutable, so the registry adopts the replacement. - // The engine returns nothing for a layer it does not hold. - if (updated == null) return false - L_.layers.layer[uuid] = updated - return true + // Leaflet recompiles per tile from tileOptions; deck.gl bakes them in. + // Neither is this caller's business. + return L_.Map_.engine.refreshLayer(uuid, { + url: sourceUrl, + tileOptions, + force: false, + }) } catch (err) { console.error(`layers:refresh failed for "${uuid}"`, err) return false @@ -433,36 +422,7 @@ const L_ = { }), window.mmgisAPI.provide('layers:refresh', async ({ layerUUID, options }) => { const uuid = L_.asLayerUUID(layerUUID) - const layerObj = L_.layers.data[uuid] - // Deck.gl deckRaster COG branch: rebuild the layer with updated - // current* values and re-register it so deck.gl diffs in place. - if ( - L_.Map_ && - L_.Map_.engine && - L_.Map_.engine.engineType === 'deckgl' && - layerObj && - layerObj.cogRendererMode === 'deckRaster' - ) { - // The existing layer's geotiff prop is the already - // resolved (and time-substituted) file URL; fall back - // to getUrl only if the instance is unavailable. - const existing = L_.layers.layer[uuid] - const rawCogUrl = - (existing && existing.props && existing.props.geotiff) || - L_.getUrl(layerObj.type, layerObj.url, layerObj) - L_.rebuildDeckCOGLayer(layerObj, rawCogUrl) - return true - } - // Leaflet fallback — unchanged existing path. - const tileLayer = L_.layers.layer[uuid] - if (tileLayer && typeof tileLayer.refresh === 'function') { - tileLayer.refresh(null, false, options || {}) - return true - } - // A facade-managed layer has no Leaflet refresh() to call. - if (requiresEngineFacade(tileLayer)) - return refreshEngineFacadeTileLayer(uuid, options) - return false + return refreshTileLayer(uuid, options) }), window.mmgisAPI.provide('layers:updateConfig', ({ layerUUID, updates }) => { const uuid = L_.asLayerUUID(layerUUID) @@ -738,30 +698,6 @@ const L_ = { return `${baseUrl}/collections/${collectionName}/preview?assets=asset${bandsParam}${resamplingParam}` } }, - /** - * The single build-and-register path for every client-side deck COG - * update (colormap/rescale refresh, time reload). Rebuilds the layer - * from its current config and swaps it in by id — deck.gl diffs the new - * instance against the old one, so cached tiles are kept and only what - * updateTriggers name is recomputed. - * @param {object} layerObj - Layer config (L_.layers.data entry). - * @param {string} rawCogUrl - Bare, time-substituted .tif URL - * (resolveDeckCOGFileUrl, or the existing - * layer's geotiff prop). - */ - rebuildDeckCOGLayer: function (layerObj, rawCogUrl) { - const uuid = L_.asLayerUUID(layerObj.name) - const rebuilt = buildDeckCOGLayer(uuid, { - rawCogUrl, - layerObj, - opacity: L_.layers.opacity[uuid] ?? 1, - }) - L_.layers.layer[uuid] = rebuilt - // addLayer registers by id in the adapter's registry then syncs, so - // deck.gl diffs and re-renders in place. - L_.Map_.engine.addLayer(rebuilt) - return rebuilt - }, getUrl: function (type, url, layerData) { let wasCOG = false diff --git a/src/essence/Basics/TimeControl_/TimeControl.js b/src/essence/Basics/TimeControl_/TimeControl.js index 2b743ef8e..7b80f7075 100644 --- a/src/essence/Basics/TimeControl_/TimeControl.js +++ b/src/essence/Basics/TimeControl_/TimeControl.js @@ -6,17 +6,9 @@ import F_ from '../Formulae_/Formulae_' import L_ from '../Layers_/Layers_' import Map_ from '../Map_/Map_' import { parseTimeWithOffset, parseTimeToSeconds } from './timeUtils' -import { - compileTileUrl, - formatLayerTime, - buildTileUrlOptions, - shouldUseDeckRaster, -} from '../Layers_/tileUrlUtils' -import { - resolveTileLayerSource, - resolveDeckCOGFileUrl, -} from '../Layers_/tileLayerSource' -import { MAP_ENGINE, isRasterTileLayerType } from '../MapEngines/types/engine' +import { formatLayerTime, buildTileUrlOptions } from '../Layers_/tileUrlUtils' +import { resolveTileLayerSource } from '../Layers_/tileLayerSource' +import { isRasterTileLayerType } from '../MapEngines/types/engine' import './TimeControl.css' @@ -397,7 +389,6 @@ var TimeControl = { TimeControl.setLayerWmsParams(layer) } if (evenIfControlled === true || layer.controlled !== true) { - const tileLayer = L_.layers.layer[layer.name] if (L_.layers.on[layer.name] || evenIfOff) { // resolveTileLayerSource is what layer creation uses, so // the refreshed URL keeps the layer's active tile level and @@ -415,45 +406,14 @@ var TimeControl = { tileFormat ) - // TODO: Refactor this to push URL compilation and refreshing - // into the map engine adapters so TimeControl doesn't branch - // on Map_.engine.engineType - // https://github.com/NASA-IMPACT/MMGIS/issues/212 tracks this - if ( - shouldUseDeckRaster( - Map_.engine?.engineType, - splitColonType, - layer - ) - ) { - // The client-side COG renderer reads the file directly — - // the compiled TiTiler tiles URL above is meaningless to - // it, and updateLayer({url}) would be silently ignored - // (COGLayer is keyed on its `geotiff` prop). Rebuild - // from the time-substituted file URL so deck.gl swaps - // the layer in place by id. - L_.rebuildDeckCOGLayer( - layer, - resolveDeckCOGFileUrl(layer, tileSource) - ) - } else if ( - tileLayer && - typeof tileLayer.refresh === 'function' - ) { - // refresh() copies every key onto this.options, which the - // per-tile getTileUrl then reads. Safe to pass whole: - // buildTileUrlOptions returns only tile-URL keys. - // It also re-applies the creation-time URL normalization - // ({t} rewriting, the WMS base/params split). - tileLayer.refresh( - resolvedUrl, - forceRequery === true, - tileOptions - ) - } else if (Map_.engine?.engineType === MAP_ENGINE.DECKGL) { - const newUrl = compileTileUrl(resolvedUrl, tileOptions) - Map_.engine.updateLayer(layer.name, { url: newUrl }) - } + // The engine decides whether this means mutating, + // cloning or rebuilding; the layer's registered refresher + // supplies the how for kinds that need one. + Map_.engine.refreshLayer(layer.name, { + url: resolvedUrl, + tileOptions, + force: forceRequery === true, + }) } } } else if (layer.type == 'velocity') { diff --git a/tests/unit/layersRefreshProvider.spec.js b/tests/unit/layersRefreshProvider.spec.js index c53314c60..f8f0e7fb2 100644 --- a/tests/unit/layersRefreshProvider.spec.js +++ b/tests/unit/layersRefreshProvider.spec.js @@ -13,12 +13,19 @@ const { default: L_ } = await import( /** * The `layers:refresh` provider re-renders a raster tile layer after a COG - * setting changes (LayerManager's colormap and rescale controls), down either - * the Leaflet or the facade-managed path. + * setting changes (LayerManager's colormap and rescale controls). It no + * longer branches on engine or renderer — it resolves the layer's source and + * hands the uncompiled URL plus tile-URL options to + * `Map_.engine.refreshLayer`, which is the single place that now decides how + * (mutate in place, clone, or run a registered refresher). URL compiling and + * registry adoption on refresh are the engine's job and are covered by + * tests/unit/engineRefreshLayer.spec.js, not here. */ const TITILER_URL = 'titiler-url:https://example.com/titiler/cog/tiles/WebMercatorQuad/{z}/{x}/{y}.png?url=s3://bucket/scene.tif' +const RESOLVED_URL = + 'https://example.com/titiler/cog/tiles/WebMercatorQuad/{z}/{x}/{y}.png?url=s3://bucket/scene.tif' const makeCogLayer = (type) => ({ name: 'Displacement', @@ -30,12 +37,14 @@ const makeCogLayer = (type) => ({ cogMax: 0.2, }) -// A deck.gl layer carries deck's `props` and never Leaflet's `options`. +// A deck.gl layer carries deck's `props` and never Leaflet's `options`. Used +// only as an arbitrary registry-entry fixture here — the provider never reads +// or writes it, which several tests below assert directly. const makeDeckLayer = (id, url) => ({ id, props: { data: url } }) let providers -let updateLayer -// The ids the engine holds, mirroring DeckGLAdapter's own layer map. +let refreshLayer +// The ids the engine holds, mirroring both adapters' own layer maps. let engineLayerIds let timeUrlReplacements @@ -48,16 +57,19 @@ const registerProviders = () => { }, } engineLayerIds = new Set() - // DeckGLAdapter.updateLayer clones the layer it holds and returns the - // replacement, and returns undefined for an id it does not hold. - updateLayer = vi.fn((id, options) => - engineLayerIds.has(id) ? makeDeckLayer(id, options.url) : undefined - ) + // Mirrors the two documented ways a real adapter's refreshLayer says "no": + // an id it does not hold, or (DeckGLAdapter specifically) a null url with + // no registered refresher to fall back on. + refreshLayer = vi.fn((id, ctx) => { + if (!engineLayerIds.has(id)) return false + if (ctx.url == null) return false + return true + }) timeUrlReplacements = vi.fn(async (url) => url) L_.fina( null, { - engine: { engineType: MAP_ENGINE.DECKGL, updateLayer }, + engine: { engineType: MAP_ENGINE.DECKGL, refreshLayer }, nativeLayer: (layer) => layer && layer._deckLayer != null ? layer._deckLayer : layer, }, @@ -85,10 +97,11 @@ describe('layers:refresh provider', () => { registerProviders() }) - // All three canonicalize to `tile`. What routes a layer down the - // facade-managed branch is the shape of its registry entry, not this type. + // All three canonicalize to `tile`. The provider no longer branches on + // this type — Map_.engine.refreshLayer does, and that's tested on its own + // in engineRefreshLayer.spec.js. test.each([['TileLayer'], ['BitmapLayer'], ['tile']])( - 'compiles a colormap override into a new URL for a %s layer', + 'passes a colormap override through to the engine for a %s layer', async (type) => { const layer = makeCogLayer(type) register(layer, makeDeckLayer(layer.name, 'stale')) @@ -99,11 +112,14 @@ describe('layers:refresh provider', () => { }) expect(result).toBe(true) - expect(updateLayer).toHaveBeenCalledTimes(1) - const [id, options] = updateLayer.mock.calls[0] + expect(refreshLayer).toHaveBeenCalledTimes(1) + const [id, ctx] = refreshLayer.mock.calls[0] expect(id).toBe('Displacement') - expect(options.url).toContain('colormap_name=plasma') - expect(options.url).toContain('rescale=-0.1%2C0.2') + expect(ctx.url).toBe(RESOLVED_URL) + expect(ctx.force).toBe(false) + expect(ctx.tileOptions.currentCogColormap).toBe('plasma') + expect(ctx.tileOptions.cogMin).toBe(-0.1) + expect(ctx.tileOptions.cogMax).toBe(0.2) } ) @@ -119,12 +135,11 @@ describe('layers:refresh provider', () => { options: { currentCogColormap: 'plasma' }, }) - const [, options] = updateLayer.mock.calls[0] - expect(options.url).toContain('colormap_name=plasma') - expect(options.url).not.toContain('magma') + const [, ctx] = refreshLayer.mock.calls[0] + expect(ctx.tileOptions.currentCogColormap).toBe('plasma') }) - test('compiles a rescale override into a new URL', async () => { + test('passes a rescale override through to the engine', async () => { const layer = makeCogLayer('TileLayer') register(layer, makeDeckLayer(layer.name, 'stale')) @@ -133,11 +148,15 @@ describe('layers:refresh provider', () => { options: { currentCogMin: 0, currentCogMax: 5 }, }) - const [, options] = updateLayer.mock.calls[0] - expect(options.url).toContain('rescale=0%2C5') + const [, ctx] = refreshLayer.mock.calls[0] + expect(ctx.tileOptions.currentCogMin).toBe(0) + expect(ctx.tileOptions.currentCogMax).toBe(5) }) - test('adopts the replacement instance the engine returns', async () => { + // The write-back a facade-managed refresh used to need (adopting the + // clone the engine returned) is gone — the engine's own registry is + // authoritative now, and refreshLayer returns a boolean, not an instance. + test('does not touch the layer registry — the engine owns that now', async () => { const layer = makeCogLayer('TileLayer') const stale = makeDeckLayer(layer.name, 'stale') register(layer, stale) @@ -147,10 +166,7 @@ describe('layers:refresh provider', () => { options: { currentCogColormap: 'plasma' }, }) - expect(L_.layers.layer['Displacement']).not.toBe(stale) - expect(L_.layers.layer['Displacement'].props.data).toContain( - 'colormap_name=plasma' - ) + expect(L_.layers.layer['Displacement']).toBe(stale) }) test('leaves the layer config URL unmutated so the next refresh re-resolves', async () => { @@ -165,25 +181,6 @@ describe('layers:refresh provider', () => { expect(layer.url).toBe(TITILER_URL) }) - // The provider hands the overrides to Leaflet's refresh() untouched — the - // same `currentCogColormap` key the facade-managed branch compiles itself. - test('a Leaflet tile layer still takes its own refresh()', async () => { - const layer = makeCogLayer('tile') - const refresh = vi.fn() - register(layer, { options: {}, refresh }) - - const result = await providers['layers:refresh']({ - layerUUID: 'Displacement', - options: { currentCogColormap: 'plasma' }, - }) - - expect(result).toBe(true) - expect(refresh).toHaveBeenCalledWith(null, false, { - currentCogColormap: 'plasma', - }) - expect(updateLayer).not.toHaveBeenCalled() - }) - test('reports failure for a facade-managed layer that is not a raster tile', async () => { const layer = { name: 'Roads', type: 'MVTLayer', url: 'x/{z}/{x}/{y}.mvt' } register(layer, makeDeckLayer(layer.name, 'stale')) @@ -194,11 +191,11 @@ describe('layers:refresh provider', () => { }) expect(result).toBe(false) - expect(updateLayer).not.toHaveBeenCalled() + expect(refreshLayer).not.toHaveBeenCalled() }) // A layer the engine does not hold — one toggled off, say — has nothing to - // clone, so the registry has to keep the instance it already has. + // refresh, so the call reports false. test('keeps the registry entry when the engine holds no such layer', async () => { const layer = makeCogLayer('TileLayer') const stale = makeDeckLayer(layer.name, 'stale') @@ -222,12 +219,14 @@ describe('layers:refresh provider', () => { }) expect(result).toBe(false) - expect(updateLayer).not.toHaveBeenCalled() + expect(refreshLayer).not.toHaveBeenCalled() }) // A `COG:` layer needs a TiTiler service to build a tile URL against, and - // resolves to nothing without one. - test('leaves the layer alone when the source resolves to no URL', async () => { + // resolves to no URL without one. The call site no longer guards this + // itself — it hands the null url to the engine, which is the one that + // declines rather than blanking the layer (see engineRefreshLayer.spec.js). + test('hands the engine a null url when the source resolves to none', async () => { const layer = { name: 'Displacement', type: 'TileLayer', @@ -243,7 +242,10 @@ describe('layers:refresh provider', () => { }) expect(result).toBe(false) - expect(updateLayer).not.toHaveBeenCalled() + expect(refreshLayer).toHaveBeenCalledWith( + 'Displacement', + expect.objectContaining({ url: null }) + ) expect(L_.layers.layer['Displacement']).toBe(stale) }) @@ -264,7 +266,7 @@ describe('layers:refresh provider', () => { }) expect(result).toBe(false) - expect(updateLayer).not.toHaveBeenCalled() + expect(refreshLayer).not.toHaveBeenCalled() expect(consoleError).toHaveBeenCalled() consoleError.mockRestore() }) diff --git a/tests/unit/timeControlReloadLayer.spec.js b/tests/unit/timeControlReloadLayer.spec.js index a7f9668a5..9b096f5db 100644 --- a/tests/unit/timeControlReloadLayer.spec.js +++ b/tests/unit/timeControlReloadLayer.spec.js @@ -10,15 +10,16 @@ import { MAP_ENGINE } from '../../src/essence/Basics/MapEngines/types/engine.ts' * makeTileLayer too but stay on the refresh path — see issue #230. */ -const updateLayer = vi.fn() -const rebuildDeckCOGLayer = vi.fn() +const refreshLayer = vi.fn(() => true) vi.mock('../../src/essence/Basics/Map_/Map_', () => ({ default: { engine: { engineType: MAP_ENGINE.DECKGL, - updateLayer: (...args) => updateLayer(...args), + refreshLayer: (...args) => refreshLayer(...args), }, + // A distinct, pre-existing Map_-level path (not Map_.engine.refreshLayer) + // used by the non-raster fallback below. Out of scope for this task. refreshLayer: vi.fn(async () => true), }, })) @@ -35,7 +36,6 @@ vi.mock('../../src/essence/Basics/Layers_/Layers_', () => ({ getUrl: (type, url) => (url.startsWith('COG:') ? url.slice(4) : url), transformStacUrl: (url) => url, timeFilterVectorLayer: vi.fn(), - rebuildDeckCOGLayer: (...args) => rebuildDeckCOGLayer(...args), }, })) @@ -73,8 +73,7 @@ describe('TimeControl.reloadLayer with the deck.gl engine', () => { // layer registry start clean for each case rather than carrying state. beforeEach(async () => { vi.resetModules() - updateLayer.mockClear() - rebuildDeckCOGLayer.mockClear() + refreshLayer.mockClear() TimeControl = (await import('../../src/essence/Basics/TimeControl_/TimeControl')) .default Map_ = (await import('../../src/essence/Basics/Map_/Map_')).default @@ -88,19 +87,24 @@ describe('TimeControl.reloadLayer with the deck.gl engine', () => { L_.layers.on[layer.name] = true } + // The call site hands the engine the uncompiled tile source URL plus + // tileOptions — the {time} substitution is the engine's job now (deck.gl + // bakes it in via compileTileUrl; Leaflet recompiles it per tile), so + // what's asserted here is that the *formatted* time reaches ctx via + // tileOptions, not that ctx.url already has it substituted. test.each([['tile'], ['TileLayer'], ['BitmapLayer']])( - 'substitutes {time} into the tile URL for a %s layer', + 'passes the formatted time for a %s layer without pre-compiling the URL', async (type) => { const layer = makeNO2Layer(type) registerDeckLayer(layer) await TimeControl.reloadLayer(layer) - expect(updateLayer).toHaveBeenCalledTimes(1) - const [name, options] = updateLayer.mock.calls[0] + expect(refreshLayer).toHaveBeenCalledTimes(1) + const [name, ctx] = refreshLayer.mock.calls[0] expect(name).toBe('NO2 Monthly') - expect(options.url).toContain('OMI_trno2_0.10x0.10_202206_Col3_V4.nc') - expect(options.url).not.toContain('{time}') + expect(ctx.url).toContain('{time}') + expect(ctx.tileOptions.time).toBe('202206') } ) @@ -116,14 +120,16 @@ describe('TimeControl.reloadLayer with the deck.gl engine', () => { await TimeControl.reloadLayer(layer) - expect(updateLayer).not.toHaveBeenCalled() + expect(refreshLayer).not.toHaveBeenCalled() expect(Map_.refreshLayer).toHaveBeenCalled() } ) // A colormap picked in the Layer Manager lives on the config as // `currentCogColormap`. A time change recompiles the URL from the config - // alone, so the pick has to survive that recompile. + // alone, so the pick has to survive that recompile. The call site no + // longer bakes tileOptions into the URL itself — the engine's registered + // refresher does that — so the pick travels through ctx.tileOptions. test('keeps a user-picked colormap when the time changes', async () => { const layer = { name: 'CO2 Concentration', @@ -140,10 +146,10 @@ describe('TimeControl.reloadLayer with the deck.gl engine', () => { await TimeControl.reloadLayer(layer) - expect(updateLayer).toHaveBeenCalledTimes(1) - const [, options] = updateLayer.mock.calls[0] - expect(options.url).toContain('colormap_name=magma') - expect(options.url).not.toContain('viridis') + expect(refreshLayer).toHaveBeenCalledTimes(1) + const [, ctx] = refreshLayer.mock.calls[0] + expect(ctx.tileOptions.currentCogColormap).toBe('magma') + expect(ctx.url).not.toContain('colormap_name') }) test('leaves the layer config URL unmutated so the next reload re-substitutes', async () => { @@ -156,31 +162,28 @@ describe('TimeControl.reloadLayer with the deck.gl engine', () => { expect(layer.url).toBe(originalUrl) }) - test('rebuilds a deckRaster COG layer from its time-substituted file URL', async () => { - const layer = { - name: 'CO2 COG', - type: 'tile', - url: 'COG:https://example.com/cogs/co2_{time}.tif', - cogRendererMode: 'deckRaster', - cogTransform: true, - minZoom: '2', - maxZoom: '10', - controlled: false, - time: { ...timeConfig }, - } - registerDeckLayer(layer) - - await TimeControl.reloadLayer(layer) - - // The TiTiler tiles URL is meaningless to the client-side renderer — - // a clone({url}) is silently ignored by COGLayer (it reads `geotiff`). - // The rebuild itself is L_.rebuildDeckCOGLayer's job (the single - // build-and-register path); TimeControl supplies the substituted URL. - expect(updateLayer).not.toHaveBeenCalled() - expect(rebuildDeckCOGLayer).toHaveBeenCalledTimes(1) - const [rebuiltLayerObj, rawCogUrl] = rebuildDeckCOGLayer.mock.calls[0] - expect(rebuiltLayerObj).toBe(layer) - expect(rawCogUrl).toBe('https://example.com/cogs/co2_202206.tif') + // The point of this test: a deckRaster COG config and a plain tile config + // differ only in `cogRendererMode`. If the call site still branched on + // that (or on engine/renderer type) to decide how to update the layer, + // the two calls would differ in shape. They must not — the registered + // refresher, not the call site, owns "how". + test('a deckRaster layer takes the same call as a plain tile layer', async () => { + const plain = makeNO2Layer('tile') + registerDeckLayer(plain) + await TimeControl.reloadLayer(plain) + const plainCall = refreshLayer.mock.calls[0] + + refreshLayer.mockClear() + + // Differs only in how it renders — the call site must not notice. + const cog = { ...makeNO2Layer('tile'), cogRendererMode: 'deckRaster' } + registerDeckLayer(cog) + await TimeControl.reloadLayer(cog) + + expect(refreshLayer).toHaveBeenCalledTimes(1) + expect(Object.keys(refreshLayer.mock.calls[0][1]).sort()).toEqual( + Object.keys(plainCall[1]).sort() + ) }) test('a vector tile layer takes the refresh path, not the tile pipeline', async () => { @@ -195,7 +198,7 @@ describe('TimeControl.reloadLayer with the deck.gl engine', () => { await TimeControl.reloadLayer(layer) - expect(updateLayer).not.toHaveBeenCalled() + expect(refreshLayer).not.toHaveBeenCalled() expect(Map_.refreshLayer).toHaveBeenCalled() }) }) From 3a7556066dcd6ad8596352931993f3449afd8f43 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 14:34:19 -0500 Subject: [PATCH 06/22] Handle vector layers in the Leaflet adapter's opacity surface setLayerOpacity now returns void on both engines: Leaflet mutates the layer in place (setOpacity for tile/image/video, setStyle for vectors, with fillOpacity an absolute value the caller computed) and deck.gl replaces the instance it holds. The TLayer | void write-back contract is gone, so L_.setLayerOpacity no longer reassigns the registry entry from the engine's return value. Also drops deck.gl's off-the-map clone fallback, now unreachable policy since a layer the engine doesn't hold already picks its opacity up from L_.layers.opacity on (re)creation. Fixes the registerLayer doc comment's inaccurate claim that deck.gl delegates to addLayer, and strengthens the deck refresh fallback test to actually pin compileTileUrl substitution via a {time} placeholder. --- src/essence/Basics/Layers_/Layers_.js | 15 ++- .../MapEngines/Adapters/DeckGLAdapter.ts | 20 ++-- .../MapEngines/Adapters/LeafletAdapter.ts | 19 +++- src/essence/Basics/MapEngines/IMapEngine.ts | 25 +++-- tests/unit/deckGLAdapter.spec.js | 16 ++-- tests/unit/engineRefreshLayer.spec.js | 14 ++- tests/unit/engineSetLayerOpacity.spec.js | 93 +++++++++++++++++++ tests/unit/layersOpacity.spec.js | 22 ----- 8 files changed, 159 insertions(+), 65 deletions(-) create mode 100644 tests/unit/engineSetLayerOpacity.spec.js diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index 7dad67f68..a85f9cf78 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -2230,16 +2230,13 @@ const L_ = { if (L_.Globe_) L_.Globe_.litho.setLayerOpacity(name, newOpacity) let l = L_.layers.layer[name] - // Facade-managed layers go through the IMapEngine facade, which may - // return a replacement instance for the registry to hold (deck.gl - // layers are immutable). They have no attachments and no Leaflet marker - // elements, so the sublayer and CSS passes below do not apply to them. + // Facade-managed layers go through the IMapEngine facade, which owns + // the instance and applies the change itself (mutating it in place, + // or replacing the one it holds). They have no attachments and no + // Leaflet marker elements, so the sublayer and CSS passes below do + // not apply to them. if (requiresEngineFacade(l)) { - const updated = L_.Map_.engine.setLayerOpacity( - L_.Map_.nativeLayer(l), - newOpacity - ) - if (updated) L_.layers.layer[name] = updated + L_.Map_.engine.setLayerOpacity(L_.Map_.nativeLayer(l), newOpacity) } else if (l && l.options) { // Leaflet layers only. A registry entry that is neither // facade-managed nor a Leaflet layer — the load failure sentinel diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index 66cdbb2b5..a2bf8094f 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -1029,20 +1029,18 @@ export class DeckGLAdapter implements IMapEngine { } /** - * Set a layer's opacity and return the instance carrying it. deck.gl layers - * are immutable, so the caller must replace its reference with the result. + * Set a layer's opacity. deck.gl layers are immutable, so this replaces + * the instance the engine holds via {@link updateLayer}, which re-syncs + * the render list. * - * A layer on the map goes through {@link updateLayer}, which re-syncs the - * render list. One that is off the map is cloned instead, so it comes back - * at the requested opacity when it is added. A reference that is neither - * on the map nor a clonable layer instance (an unknown id, or a registry - * value that never became a layer) yields no replacement. + * A layer the engine does not hold (an unknown id, or a registry value + * that never became a layer) is a no-op here — `L_.layers.opacity[name]` + * is always written by the caller, and layer creation reads it, so it + * picks up the opacity when it is built or re-added. */ - setLayerOpacity(layer: Layer | string, opacity: number): Layer | undefined { + setLayerOpacity(layer: Layer | string, opacity: number): void { const id = resolveLayerId(layer) - if (this._layers.has(id)) return this.updateLayer(id, { opacity }) - if (typeof (layer as Layer)?.clone !== 'function') return undefined - return (layer as Layer).clone({ opacity }) as Layer + if (this._layers.has(id)) this.updateLayer(id, { opacity }) } /** diff --git a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts index cf8c92fdd..deba01c0e 100644 --- a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts @@ -818,10 +818,25 @@ export default class LeafletAdapter implements IMapEngine, IMapEn } } - setLayerOpacity(layer: any | string, opacity: number): void { + setLayerOpacity( + layer: any | string, + opacity: number, + options?: { fillOpacity?: number } + ): void { const leafletLayer = typeof layer === 'string' ? this._layers.get(layer) : layer - if (leafletLayer && typeof leafletLayer.setOpacity === 'function') { + if (!leafletLayer) return + + // Tile, image and video layers carry a whole-element opacity; vector + // layers have to be re-styled, and paint stroke and fill separately. + if (typeof leafletLayer.setOpacity === 'function') { leafletLayer.setOpacity(opacity) + return + } + if (typeof leafletLayer.setStyle === 'function') { + leafletLayer.setStyle({ + opacity, + fillOpacity: options?.fillOpacity ?? opacity, + }) } } diff --git a/src/essence/Basics/MapEngines/IMapEngine.ts b/src/essence/Basics/MapEngines/IMapEngine.ts index de07ca87e..4b26e4a7b 100644 --- a/src/essence/Basics/MapEngines/IMapEngine.ts +++ b/src/essence/Basics/MapEngines/IMapEngine.ts @@ -207,7 +207,9 @@ export interface IMapEngine< * * Leaflet needs this because MMGIS builds its tile layers itself and hands * them to `addLayer` as native objects, which carry no id. deck.gl layers - * already carry `id`, so its implementation delegates to `addLayer`. + * already carry `id`, but its implementation still keys its registry by + * this caller-supplied `id` rather than `layer.id`, so the two can never + * drift apart. */ registerLayer(id: string, layer: TLayer): void @@ -252,14 +254,21 @@ export interface IMapEngine< bringToBack(layer: TLayer | string): void /** - * Set the opacity of a layer. + * Set a layer's opacity. * - * Engines with immutable layer objects (deck.gl) return the instance that - * carries the new opacity; callers holding a reference to the layer must - * replace it with the returned one. Engines that mutate in place (Leaflet) - * return nothing. - */ - setLayerOpacity(layer: TLayer | string, opacity: number): TLayer | void + * Both engines return nothing: the engine owns the instance and performs + * whatever the change requires — mutating it (Leaflet) or replacing the one + * it holds (deck.gl). Callers never adopt a replacement. + * + * @param options.fillOpacity - The fill opacity to apply to layers that + * paint one. Scaling policy belongs to the caller, so this is an absolute + * value, never a factor the adapter multiplies. Defaults to `opacity`. + */ + setLayerOpacity( + layer: TLayer | string, + opacity: number, + options?: { fillOpacity?: number } + ): void /** * Subscribe to a map event (click, moveend, zoomend, etc). diff --git a/tests/unit/deckGLAdapter.spec.js b/tests/unit/deckGLAdapter.spec.js index 8f4135b3c..1f276811a 100644 --- a/tests/unit/deckGLAdapter.spec.js +++ b/tests/unit/deckGLAdapter.spec.js @@ -220,13 +220,14 @@ test.describe('DeckGLAdapter', () => { expect(stored.opacity).toBe(0.4) }) - test('setLayerOpacity returns the instance carrying the new opacity', () => { + test('setLayerOpacity replaces the stored layer and returns nothing', () => { const adapter = makeAdapter() const original = makeLayer('opacity-layer') adapter.addLayer(original) - const updated = adapter.setLayerOpacity(original, 0.25) - expect(updated.opacity).toBe(0.25) - expect(updated).not.toBe(original) + expect(adapter.setLayerOpacity(original, 0.25)).toBeUndefined() + const stored = adapter.getLayers().find((l) => l.id === 'opacity-layer') + expect(stored.opacity).toBe(0.25) + expect(stored).not.toBe(original) expect(original.opacity).toBeUndefined() }) @@ -238,11 +239,10 @@ test.describe('DeckGLAdapter', () => { expect(stored.opacity).toBe(0) }) - test('setLayerOpacity clones an unmounted layer without adding it to the map', () => { + test('setLayerOpacity is a no-op for a layer the engine does not hold', () => { const adapter = makeAdapter() const offMap = makeLayer('hidden-layer') - const updated = adapter.setLayerOpacity(offMap, 0.6) - expect(updated.opacity).toBe(0.6) + expect(adapter.setLayerOpacity(offMap, 0.6)).toBeUndefined() expect(adapter.hasLayer('hidden-layer')).toBe(false) }) @@ -251,7 +251,7 @@ test.describe('DeckGLAdapter', () => { expect(adapter.setLayerOpacity('nonexistent', 0.5)).toBeUndefined() }) - test('setLayerOpacity on a value with nothing to clone returns undefined without throwing', () => { + test('setLayerOpacity on a non-layer value returns undefined without throwing', () => { const adapter = makeAdapter() expect(() => adapter.setLayerOpacity(false, 0.5)).not.toThrow() expect(adapter.setLayerOpacity(false, 0.5)).toBeUndefined() diff --git a/tests/unit/engineRefreshLayer.spec.js b/tests/unit/engineRefreshLayer.spec.js index bcf2a63fb..de69f3b33 100644 --- a/tests/unit/engineRefreshLayer.spec.js +++ b/tests/unit/engineRefreshLayer.spec.js @@ -112,15 +112,19 @@ describe('DeckGLAdapter.refreshLayer', () => { const adapter = new DeckGLAdapter() adapter.addLayer(makeDeckLayer('l1', { data: 'old' })) + // The url carries a {time} placeholder and tileOptions substitutes it, + // so this is only observable if the fallback actually calls + // compileTileUrl — a byte-identical url (no placeholder) would pass + // even if compileTileUrl were never invoked. expect( adapter.refreshLayer('l1', { - url: 'https://x/{z}/{x}/{y}.png', - tileOptions: {}, + url: 'https://x/{z}/{x}/{y}.png?time={time}', + tileOptions: { time: '2024-01-01T00:00:00Z' }, }) ).toBe(true) - expect( - adapter.getLayers().find((l) => l.id === 'l1').props.data - ).toBe('https://x/{z}/{x}/{y}.png') + const data = adapter.getLayers().find((l) => l.id === 'l1').props.data + expect(data).not.toContain('{time}') + expect(data).toContain('2024-01-01T00:00:00Z') }) test('returns false for an unregistered id', () => { diff --git a/tests/unit/engineSetLayerOpacity.spec.js b/tests/unit/engineSetLayerOpacity.spec.js new file mode 100644 index 000000000..905eddf8d --- /dev/null +++ b/tests/unit/engineSetLayerOpacity.spec.js @@ -0,0 +1,93 @@ +import { describe, test, expect } from 'vitest' +import LeafletAdapter from '../../src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts' +import DeckGLAdapter from '../../src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts' + +/** + * Opacity POLICY (fill scaling, decoration factors) stays with the caller and + * arrives as options.fillOpacity. The adapter only knows how its own engine + * applies opacity — setOpacity for tile-ish layers, setStyle for vectors. + */ +const makeTileish = () => ({ + opacity: null, + setOpacity(o) { this.opacity = o }, +}) + +const makeVector = () => ({ + style: null, + setStyle(s) { this.style = s }, +}) + +const makeDeckLayer = (id, props = {}) => ({ + id, + props: { id, ...props }, + clone(patch) { return makeDeckLayer(id, { ...props, ...patch }) }, +}) + +describe('LeafletAdapter.setLayerOpacity', () => { + test('uses setOpacity when the layer offers it', () => { + const adapter = new LeafletAdapter() + const layer = makeTileish() + adapter.setLayerOpacity(layer, 0.5) + expect(layer.opacity).toBe(0.5) + }) + + test('falls back to setStyle for a vector layer', () => { + const adapter = new LeafletAdapter() + const layer = makeVector() + adapter.setLayerOpacity(layer, 0.5, { fillOpacity: 0.2 }) + expect(layer.style).toEqual({ opacity: 0.5, fillOpacity: 0.2 }) + }) + + test('defaults fillOpacity to the opacity when the caller supplies none', () => { + const adapter = new LeafletAdapter() + const layer = makeVector() + adapter.setLayerOpacity(layer, 0.5) + expect(layer.style).toEqual({ opacity: 0.5, fillOpacity: 0.5 }) + }) + + test('resolves a string id through the registry', () => { + const adapter = new LeafletAdapter() + const layer = makeTileish() + adapter.registerLayer('l1', layer) + adapter.setLayerOpacity('l1', 0.25) + expect(layer.opacity).toBe(0.25) + }) + + test('an opacity of 0 is applied, not read as unset', () => { + const adapter = new LeafletAdapter() + const layer = makeTileish() + adapter.setLayerOpacity(layer, 0) + expect(layer.opacity).toBe(0) + }) + + test('returns nothing and tolerates a layer answering to neither API', () => { + const adapter = new LeafletAdapter() + expect(adapter.setLayerOpacity({}, 0.5)).toBeUndefined() + }) +}) + +describe('DeckGLAdapter.setLayerOpacity', () => { + test('replaces the held instance and returns nothing', () => { + const adapter = new DeckGLAdapter() + const original = makeDeckLayer('l1', { opacity: 1 }) + adapter.addLayer(original) + + expect(adapter.setLayerOpacity('l1', 0.5)).toBeUndefined() + const held = adapter.getLayers().find((l) => l.id === 'l1') + expect(held).not.toBe(original) + expect(held.props.opacity).toBe(0.5) + }) + + test('an opacity of 0 reaches the layer', () => { + const adapter = new DeckGLAdapter() + adapter.addLayer(makeDeckLayer('l1', { opacity: 1 })) + adapter.setLayerOpacity('l1', 0) + expect(adapter.getLayers().find((l) => l.id === 'l1').props.opacity).toBe(0) + }) + + test('is a no-op for an id the engine does not hold', () => { + const adapter = new DeckGLAdapter() + expect(adapter.setLayerOpacity('nope', 0.5)).toBeUndefined() + expect(adapter.getLayers()).toHaveLength(0) + }) +}) diff --git a/tests/unit/layersOpacity.spec.js b/tests/unit/layersOpacity.spec.js index 69b9450ef..17447c12b 100644 --- a/tests/unit/layersOpacity.spec.js +++ b/tests/unit/layersOpacity.spec.js @@ -67,28 +67,6 @@ describe('L_.setLayerOpacity engine dispatch', () => { expect(setLayerOpacity).toHaveBeenCalledWith(original, 0.4) }) - test('the instance returned by the engine replaces the registry entry', () => { - const replacement = makeEngineLayer('vec', 0.4) - setEngine(MAP_ENGINE.DECKGL, () => replacement) - L_.layers.layer.vec = makeEngineLayer('vec', 1) - - L_.setLayerOpacity('vec', 0.4) - - expect(L_.layers.layer.vec).toBe(replacement) - expect(L_.layers.opacity.vec).toBe(0.4) - }) - - test('the registry entry is kept when the engine returns nothing', () => { - setEngine(MAP_ENGINE.DECKGL, () => undefined) - const original = makeEngineLayer('vec', 1) - L_.layers.layer.vec = original - - L_.setLayerOpacity('vec', 0.4) - - expect(L_.layers.layer.vec).toBe(original) - expect(L_.layers.opacity.vec).toBe(0.4) - }) - test('an opacity of 0 reaches the engine rather than being read as unset', () => { const setLayerOpacity = vi.fn(() => makeEngineLayer('vec', 0)) setEngine(MAP_ENGINE.DECKGL, setLayerOpacity) From 14534a4d80c960f9b645a703f2b6807a8b3e31de Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 14:42:09 -0500 Subject: [PATCH 07/22] Read layer opacity from the registry instead of Leaflet options --- src/essence/Basics/Layers_/Layers_.js | 51 ++++++--------------- tests/unit/layersOpacity.spec.js | 66 ++++++++++----------------- 2 files changed, 39 insertions(+), 78 deletions(-) diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index a85f9cf78..da31aae7a 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -2230,6 +2230,14 @@ const L_ = { if (L_.Globe_) L_.Globe_.litho.setLayerOpacity(name, newOpacity) let l = L_.layers.layer[name] + // The configured fill opacity is the factor the slider scales; read it + // from config rather than caching it on the layer, so there is one + // source of truth. + const configuredFill = + L_.layers.data[name]?.style?.fillOpacity != null + ? parseFloat(L_.layers.data[name].style.fillOpacity) + : 1 + // Facade-managed layers go through the IMapEngine facade, which owns // the instance and applies the change itself (mutating it in place, // or replacing the one it holds). They have no attachments and no @@ -2243,17 +2251,12 @@ const L_ = { // (`false`) or an aggregate array — falls through to the registry // write below, which is the value the engine reads when it builds // or re-adds the layer. - if (l.options.initialFillOpacity == null) - l.options.initialFillOpacity = - L_.layers.data[name]?.style?.fillOpacity != null - ? parseFloat(L_.layers.data[name].style.fillOpacity) - : 1 try { l.setOpacity(newOpacity) } catch (error) { l.setStyle({ opacity: newOpacity, - fillOpacity: newOpacity * l.options.initialFillOpacity, + fillOpacity: newOpacity * configuredFill, }) } $(`.leafletMarkerShape_${F_.getSafeName(name)}`).css({ @@ -2273,8 +2276,7 @@ const L_ = { } catch (error) { try { let opacity = newOpacity - let fillOpacity = - newOpacity * l.options.initialFillOpacity + let fillOpacity = newOpacity * configuredFill if (sub === 'uncertainty_ellipses') { opacity = opacity * 0.8 fillOpacity = fillOpacity * 0.25 @@ -2295,19 +2297,6 @@ const L_ = { } } } - - try { - l.options.fillOpacity = - newOpacity * l.options.initialFillOpacity - l.options.opacity = newOpacity - l.options.style.fillOpacity = - newOpacity * l.options.initialFillOpacity - l.options.style.opacity = newOpacity - } catch (error) { - l.options.fillOpacity = - newOpacity * l.options.initialFillOpacity - l.options.opacity = newOpacity - } } L_.layers.opacity[name] = newOpacity @@ -2316,21 +2305,11 @@ const L_ = { } }, getLayerOpacity: function (name) { - var l = L_.layers.layer[name] - - if (l == null) return 0 - - // Facade-managed layer objects carry no Leaflet `options`; the registry - // is the authority on their opacity. - if (requiresEngineFacade(l)) return L_.layers.opacity[name] ?? 1 - - var opacity - try { - opacity = l.options?.style.opacity - } catch (error) { - opacity = l.options?.opacity - } - return opacity + // A layer that was never built has no opacity to report. Everything + // else reads the registry, which is authoritative for both engines — + // Leaflet layer options are no longer mirrored and can be stale. + if (L_.layers.layer[name] == null) return 0 + return L_.layers.opacity[name] ?? 1 }, setLayerFilter: function (name, filter, value) { // Clear diff --git a/tests/unit/layersOpacity.spec.js b/tests/unit/layersOpacity.spec.js index 17447c12b..db944bb8f 100644 --- a/tests/unit/layersOpacity.spec.js +++ b/tests/unit/layersOpacity.spec.js @@ -134,58 +134,40 @@ describe('L_.setLayerOpacity engine dispatch', () => { }) }) -describe('L_.getLayerOpacity engine dispatch', () => { - beforeEach(() => { - resetRegistry() - L_.Map_ = null - }) - - test('reads a facade-managed layer from the registry', () => { - setEngine(MAP_ENGINE.DECKGL, () => undefined) - L_.layers.layer.vec = makeEngineLayer('vec', 1) - L_.layers.opacity.vec = 0.25 - - expect(L_.getLayerOpacity('vec')).toBe(0.25) - }) - - test('reads 0 from the registry rather than falling back to 1', () => { - setEngine(MAP_ENGINE.DECKGL, () => undefined) - L_.layers.layer.vec = makeEngineLayer('vec', 0) - L_.layers.opacity.vec = 0 +describe('L_.getLayerOpacity reads the registry', () => { + beforeEach(resetRegistry) - expect(L_.getLayerOpacity('vec')).toBe(0) + test('reads a Leaflet layer from the registry, not its options', () => { + setEngine(MAP_ENGINE.LEAFLET, vi.fn()) + const layer = makeLeafletLayer() + layer.options.style.opacity = 0.9 // a stale mirror must not win + L_.layers.layer.a = layer + L_.layers.opacity.a = 0.3 + expect(L_.getLayerOpacity('a')).toBe(0.3) }) - test('defaults to 1 when the registry has no entry for the layer', () => { - setEngine(MAP_ENGINE.DECKGL, () => undefined) - L_.layers.layer.vec = makeEngineLayer('vec', 1) - - expect(L_.getLayerOpacity('vec')).toBe(1) + test('reads an engine-owned layer from the registry', () => { + setEngine(MAP_ENGINE.DECKGL, vi.fn()) + L_.layers.layer.a = makeEngineLayer('a', 1) + L_.layers.opacity.a = 0.25 + expect(L_.getLayerOpacity('a')).toBe(0.25) }) - test('round-trips through setLayerOpacity', () => { - setEngine(MAP_ENGINE.DECKGL, (layer, opacity) => - makeEngineLayer('vec', opacity) - ) - L_.layers.layer.vec = makeEngineLayer('vec', 1) - - L_.setLayerOpacity('vec', 0.6) - - expect(L_.getLayerOpacity('vec')).toBe(0.6) + test('reads 0 rather than falling back to 1', () => { + setEngine(MAP_ENGINE.LEAFLET, vi.fn()) + L_.layers.layer.a = makeLeafletLayer() + L_.layers.opacity.a = 0 + expect(L_.getLayerOpacity('a')).toBe(0) }) - test('reads a Leaflet layer from its own style under a deck.gl engine', () => { - setEngine(MAP_ENGINE.DECKGL, () => undefined) - const velocity = makeLeafletLayer() - velocity.options.style.opacity = 0.7 - L_.layers.layer.wind = velocity - - expect(L_.getLayerOpacity('wind')).toBe(0.7) + test('defaults to 1 when the registry has no entry', () => { + setEngine(MAP_ENGINE.LEAFLET, vi.fn()) + L_.layers.layer.a = makeLeafletLayer() + expect(L_.getLayerOpacity('a')).toBe(1) }) test('returns 0 for a layer that has not been built', () => { - setEngine(MAP_ENGINE.DECKGL, () => undefined) - + setEngine(MAP_ENGINE.LEAFLET, vi.fn()) expect(L_.getLayerOpacity('missing')).toBe(0) }) }) From 624096b9a0eb755022e170a7149b1fab02624014 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 14:53:48 -0500 Subject: [PATCH 08/22] Ask the engine per layer part instead of branching on layer shape setLayerOpacity now hands every registry entry (main layer, then each attachment) to the active engine's setLayerOpacity, once per part, instead of branching on whether the layer is facade-managed or a Leaflet object. Update policy (fill scaling, uncertainty_ellipses dimming, the models skip, the marker CSS pass) stays with the caller; only the mechanics of applying an opacity move to the adapter. Deletes requiresEngineFacade, now unused. layerBoundsFor's deck.gl branch, its other caller, switches to a direct shape read since it genuinely inspects layer data rather than choosing an update path. --- src/essence/Basics/Layers_/Layers_.js | 176 ++++++++++---------------- src/essence/Basics/Map_/Map_.js | 7 +- tests/unit/layersGetBounds.spec.js | 4 +- tests/unit/layersOpacity.spec.js | 135 +++++++++++--------- 4 files changed, 145 insertions(+), 177 deletions(-) diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index da31aae7a..1bf9fd34b 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -6,7 +6,7 @@ import Attributions from '../../Ancillary/Attributions' import ToolController_ from '../../Basics/ToolController_/ToolController_' import LayerGeologic from './LayerGeologic/LayerGeologic' import ServiceUrls from '../ServiceUrls/ServiceUrls' -import { MAP_ENGINE, isRasterTileLayerType } from '../MapEngines/types/engine' +import { isRasterTileLayerType } from '../MapEngines/types/engine' import { getActiveTileLevel, getTileLevelUrl, @@ -75,35 +75,6 @@ function titilerUrlFor(layerConfig) { return window.mmgisglobal?.WITH_TITILER === 'true' ? url : null } -/** - * True when a registry entry has to be mutated through the IMapEngine facade - * rather than Leaflet's methods — it has no Leaflet API to call, and under - * deck.gl a mutation returns a replacement instance for the registry to adopt. - * - * Which layer types the facade builds is per-engine, not universal — see - * ENGINE_LAYER_SUPPORT. Types the active engine has no builder for (velocity, - * model, data, image and video under deck.gl) stay Leaflet-built, so the - * registry always holds a mix. - * - * Facade-managed entries are identified positively by shape: a deck.gl layer - * carries deck's `props` (or arrives as a `_deckLayer` wrapper, which - * `Map_.nativeLayer` unwraps) and never Leaflet's `options`. Anything else in - * the registry — Leaflet layers, aggregate arrays of them, and the - * load-failure sentinel (`false`) — takes the Leaflet path. - * - * @param {object} layer - A registry entry from `L_.layers.layer`. - * @returns {boolean} - */ -function requiresEngineFacade(layer) { - return ( - layer != null && - layer.options == null && - (layer.props != null || layer._deckLayer != null) && - L_.Map_?.engine != null && - L_.Map_.engine.engineType !== MAP_ENGINE.LEAFLET - ) -} - /** * A layer's geographic extent as `[[south, west], [north, east]]` — the * `[LatLngLike, LatLngLike]` pair both map engines normalise — or null when no @@ -141,29 +112,28 @@ function layerBoundsFor(uuid) { } } - // A deck.gl layer keeps its features on `props.data` and exposes no - // measurement method. Only inline GeoJSON can be measured here — when - // `data` is a URL the features live inside deck's loaders, out of reach. - if (requiresEngineFacade(layer)) { - const data = layer.props?.data ?? layer._deckLayer?.props?.data - if (data != null && typeof data === 'object') { - // deck accepts a bare array of features as readily as a GeoJSON - // object; turf measures only the latter. - const geojson = Array.isArray(data) - ? { type: 'FeatureCollection', features: data } - : data - try { - const [west, south, east, north] = bbox(geojson) - // An empty FeatureCollection measures to infinities. - if ([west, south, east, north].every(Number.isFinite)) { - return [ - [south, west], - [north, east], - ] - } - } catch (err) { - // Unmeasurable GeoJSON; try the configured footprint. + // A deck.gl layer keeps its features on props.data and exposes no + // measurement method. + const deckData = layer?.props?.data ?? layer?._deckLayer?.props?.data + if (deckData != null && typeof deckData === 'object') { + // deck accepts a bare array of features as readily as a GeoJSON + // object; turf measures only the latter. Only inline GeoJSON can be + // measured here — when data is a URL the features live inside deck's + // loaders, out of reach. + const geojson = Array.isArray(deckData) + ? { type: 'FeatureCollection', features: deckData } + : deckData + try { + const [west, south, east, north] = bbox(geojson) + // An empty FeatureCollection measures to infinities. + if ([west, south, east, north].every(Number.isFinite)) { + return [ + [south, west], + [north, east], + ] } + } catch (err) { + // Unmeasurable GeoJSON; try the configured footprint. } } @@ -2227,77 +2197,63 @@ const L_ = { }, setLayerOpacity: function (name, newOpacity) { newOpacity = parseFloat(newOpacity) + // LithoSphere is not a map engine; the globe keeps its own path. if (L_.Globe_) L_.Globe_.litho.setLayerOpacity(name, newOpacity) - let l = L_.layers.layer[name] - // The configured fill opacity is the factor the slider scales; read it - // from config rather than caching it on the layer, so there is one - // source of truth. + const l = L_.layers.layer[name] + const engine = L_.Map_?.engine + + // The configured fill opacity is what the slider scales. Read from + // config so there is one source of truth. const configuredFill = L_.layers.data[name]?.style?.fillOpacity != null ? parseFloat(L_.layers.data[name].style.fillOpacity) : 1 - // Facade-managed layers go through the IMapEngine facade, which owns - // the instance and applies the change itself (mutating it in place, - // or replacing the one it holds). They have no attachments and no - // Leaflet marker elements, so the sublayer and CSS passes below do - // not apply to them. - if (requiresEngineFacade(l)) { - L_.Map_.engine.setLayerOpacity(L_.Map_.nativeLayer(l), newOpacity) - } else if (l && l.options) { - // Leaflet layers only. A registry entry that is neither - // facade-managed nor a Leaflet layer — the load failure sentinel - // (`false`) or an aggregate array — falls through to the registry - // write below, which is the value the engine reads when it builds - // or re-adds the layer. - try { - l.setOpacity(newOpacity) - } catch (error) { - l.setStyle({ - opacity: newOpacity, - fillOpacity: newOpacity * configuredFill, - }) - } - $(`.leafletMarkerShape_${F_.getSafeName(name)}`).css({ - opacity: newOpacity, + // An MMGIS layer is a compound — main layer plus attachment + // decorations. The caller iterates the parts and asks the engine once + // per part; the adapter never learns what an attachment is. Skipped + // here: the load-failure sentinel (false) and aggregate arrays, + // neither of which is a layer the engine holds. + if (engine && l && l !== false && !Array.isArray(l)) { + engine.setLayerOpacity(L_.Map_.nativeLayer(l), newOpacity, { + fillOpacity: newOpacity * configuredFill, }) const sublayers = L_.layers.attachments[name] - if (sublayers) { - for (let sub in sublayers) { - if ( - sublayers[sub] !== false && - sublayers[sub].layer != null && - !['models'].includes(sub) - ) { - try { - sublayers[sub].layer.setOpacity(newOpacity) - } catch (error) { - try { - let opacity = newOpacity - let fillOpacity = newOpacity * configuredFill - if (sub === 'uncertainty_ellipses') { - opacity = opacity * 0.8 - fillOpacity = fillOpacity * 0.25 - } - sublayers[sub].layer.setStyle({ - opacity, - fillOpacity, - }) - } catch (error2) { - /* - if (sublayers[sub].layer._layers) - for (let sl in sublayers[sub].layer - ._layers) { - } - */ - } - } + for (const sub in sublayers || {}) { + const attachment = sublayers[sub] + // 'models' render on the globe only — no 2D layer to dim. + if ( + attachment === false || + attachment.layer == null || + ['models'].includes(sub) + ) + continue + + // Decoration dimming factors are product behaviour, not + // engine mechanics, so they are applied here and passed in + // as absolute values. + const isEllipses = sub === 'uncertainty_ellipses' + engine.setLayerOpacity( + attachment.layer, + newOpacity * (isEllipses ? 0.8 : 1), + { + fillOpacity: + newOpacity * + configuredFill * + (isEllipses ? 0.25 : 1), } - } + ) } } + + // MMGIS marker markup, keyed by layer name and created outside any + // engine — product markup, so it stays caller-side. + $(`.leafletMarkerShape_${F_.getSafeName(name)}`).css({ + opacity: newOpacity, + }) + L_.layers.opacity[name] = newOpacity if (L_.activeFeature?.layer && L_.activeFeature.layerName === name) { diff --git a/src/essence/Basics/Map_/Map_.js b/src/essence/Basics/Map_/Map_.js index a6308f68e..1e630c5ff 100644 --- a/src/essence/Basics/Map_/Map_.js +++ b/src/essence/Basics/Map_/Map_.js @@ -1392,9 +1392,10 @@ async function makeVectorLayer( } // Only Leaflet vector layers reach here — the deck.gl branch above - // returns first. Attachments are therefore Leaflet-only, which is - // why L_.setLayerOpacity skips its sublayer pass for facade-managed - // layers. + // returns first. Attachments are therefore Leaflet-only: + // L_.layers.attachments has no entry for a deck.gl-built layer, so + // L_.setLayerOpacity's per-attachment loop has nothing to iterate + // for one. ctx.layerRegistry.attachments[layerObj.name] = vl.sublayers ctx.layerRegistry.layer[layerObj.name] = vl.layer diff --git a/tests/unit/layersGetBounds.spec.js b/tests/unit/layersGetBounds.spec.js index ab83e3c9d..6ca4a3819 100644 --- a/tests/unit/layersGetBounds.spec.js +++ b/tests/unit/layersGetBounds.spec.js @@ -33,8 +33,8 @@ const leafletLayer = (bounds) => ({ getBounds: () => bounds, }) -// requiresEngineFacade identifies a deck.gl layer by shape: deck's `props` and -// never Leaflet's `options`. +// A deck.gl layer is identified by shape: deck's `props` and never Leaflet's +// `options`. const deckLayer = (data) => ({ props: { data } }) const feature = (coordinates) => ({ diff --git a/tests/unit/layersOpacity.spec.js b/tests/unit/layersOpacity.spec.js index db944bb8f..9d16fc5fe 100644 --- a/tests/unit/layersOpacity.spec.js +++ b/tests/unit/layersOpacity.spec.js @@ -12,16 +12,15 @@ const { default: L_ } = await import( ) /** - * L_.setLayerOpacity dispatches on which API the layer object answers to, not - * on the layer's configured type: under a non-Leaflet engine the registry holds - * a mix of facade-managed layers (vector, tile, vectortile) and Leaflet-built - * ones (velocity, model, data, image, video). Facade-managed layers are - * immutable in deck.gl, so the facade returns a replacement the registry must - * adopt. + * L_.setLayerOpacity no longer branches on the layer's shape: every registry + * entry that is not the load-failure sentinel or an aggregate array is handed + * to the active engine's setLayerOpacity, once per compound part (main layer, + * then each attachment). Which engine is active, and what shape its native + * layer objects carry, is the engine's business, not the caller's. */ -// A deck.gl Layer stands in as any facade-managed object: it has `props`, never -// `options`, and cannot be mutated in place. +// A deck.gl Layer stands in as a non-Leaflet native layer: it has `props`, +// never `options`. const makeEngineLayer = (id, opacity) => ({ id, props: { opacity } }) // A Leaflet layer always carries `options` and mutates in place. @@ -50,87 +49,99 @@ const resetRegistry = () => { L_.activeFeature = null } -describe('L_.setLayerOpacity engine dispatch', () => { - beforeEach(() => { - resetRegistry() - L_.Map_ = null - }) +describe('L_.setLayerOpacity asks the engine per part', () => { + beforeEach(resetRegistry) - test('facade-managed layer is routed through the facade', () => { - const setLayerOpacity = vi.fn(() => makeEngineLayer('vec', 0.4)) - setEngine(MAP_ENGINE.DECKGL, setLayerOpacity) - const original = makeEngineLayer('vec', 1) - L_.layers.layer.vec = original + test('every layer goes through the engine, whichever engine is active', () => { + for (const engineType of [MAP_ENGINE.LEAFLET, MAP_ENGINE.DECKGL]) { + const setLayerOpacity = vi.fn() + setEngine(engineType, setLayerOpacity) + L_.layers.layer.a = makeLeafletLayer() + L_.setLayerOpacity('a', 0.5) + expect(setLayerOpacity).toHaveBeenCalledTimes(1) + } + }) - L_.setLayerOpacity('vec', 0.4) + test('passes the configured fill opacity scaled by the new opacity', () => { + const setLayerOpacity = vi.fn() + setEngine(MAP_ENGINE.LEAFLET, setLayerOpacity) + L_.layers.layer.a = makeLeafletLayer() + L_.layers.data.a = { style: { fillOpacity: 0.4 } } - expect(setLayerOpacity).toHaveBeenCalledWith(original, 0.4) + L_.setLayerOpacity('a', 0.5) + expect(setLayerOpacity.mock.calls[0][2]).toEqual({ fillOpacity: 0.2 }) }) test('an opacity of 0 reaches the engine rather than being read as unset', () => { - const setLayerOpacity = vi.fn(() => makeEngineLayer('vec', 0)) - setEngine(MAP_ENGINE.DECKGL, setLayerOpacity) - L_.layers.layer.vec = makeEngineLayer('vec', 1) - - L_.setLayerOpacity('vec', 0) - - expect(setLayerOpacity).toHaveBeenCalledWith(expect.anything(), 0) - expect(L_.layers.opacity.vec).toBe(0) + const setLayerOpacity = vi.fn() + setEngine(MAP_ENGINE.LEAFLET, setLayerOpacity) + L_.layers.layer.a = makeLeafletLayer() + L_.setLayerOpacity('a', 0) + expect(setLayerOpacity.mock.calls[0][1]).toBe(0) + expect(L_.layers.opacity.a).toBe(0) }) - test('a Leaflet-built layer under a deck.gl engine stays on the Leaflet path', () => { + test('asks the engine once per attachment as well as for the main layer', () => { const setLayerOpacity = vi.fn() - setEngine(MAP_ENGINE.DECKGL, setLayerOpacity) - const velocity = makeLeafletLayer() - L_.layers.layer.wind = velocity - - L_.setLayerOpacity('wind', 0.3) + setEngine(MAP_ENGINE.LEAFLET, setLayerOpacity) + L_.layers.layer.a = makeLeafletLayer() + const labels = makeLeafletLayer() + L_.layers.attachments.a = { labels: { type: 'labels', layer: labels } } - expect(setLayerOpacity).not.toHaveBeenCalled() - expect(velocity.options.opacity).toBe(0.3) + L_.setLayerOpacity('a', 0.5) + expect(setLayerOpacity).toHaveBeenCalledTimes(2) + expect(setLayerOpacity.mock.calls[1][0]).toBe(labels) }) - test('every layer stays on the Leaflet path under the Leaflet engine', () => { + test('uncertainty ellipses keep their own dimming factors', () => { const setLayerOpacity = vi.fn() setEngine(MAP_ENGINE.LEAFLET, setLayerOpacity) - const leafletLayer = makeLeafletLayer() - L_.layers.layer.vec = leafletLayer - - L_.setLayerOpacity('vec', 0.3) + L_.layers.layer.a = makeLeafletLayer() + L_.layers.data.a = { style: { fillOpacity: 1 } } + const ellipses = makeLeafletLayer() + L_.layers.attachments.a = { + uncertainty_ellipses: { type: 'uncertainty_ellipses', layer: ellipses }, + } - expect(setLayerOpacity).not.toHaveBeenCalled() - expect(leafletLayer.options.opacity).toBe(0.3) + L_.setLayerOpacity('a', 0.5) + const [, opacity, options] = setLayerOpacity.mock.calls[1] + expect(opacity).toBeCloseTo(0.4) // 0.5 * 0.8 + expect(options.fillOpacity).toBeCloseTo(0.125) // 0.5 * 1 * 0.25 }) - test('a facade-managed layer is left alone before Map_ is initialized', () => { - L_.Map_ = null - L_.layers.layer.vec = makeEngineLayer('vec', 1) + test('skips model attachments, which have no 2D layer', () => { + const setLayerOpacity = vi.fn() + setEngine(MAP_ENGINE.LEAFLET, setLayerOpacity) + L_.layers.layer.a = makeLeafletLayer() + L_.layers.attachments.a = { models: { type: 'model', layer: makeLeafletLayer() } } - expect(() => L_.setLayerOpacity('vec', 0.4)).not.toThrow() - expect(L_.layers.opacity.vec).toBe(0.4) + L_.setLayerOpacity('a', 0.5) + expect(setLayerOpacity).toHaveBeenCalledTimes(1) }) - test('a load-failure sentinel (false) under deck.gl skips the engine and keeps the registry write', () => { + test('a load-failure sentinel (false) skips the engine but still records opacity', () => { const setLayerOpacity = vi.fn() setEngine(MAP_ENGINE.DECKGL, setLayerOpacity) - L_.layers.layer.vec = false - - expect(() => L_.setLayerOpacity('vec', 0.4)).not.toThrow() - + L_.layers.layer.a = false + L_.setLayerOpacity('a', 0.5) expect(setLayerOpacity).not.toHaveBeenCalled() - expect(L_.layers.layer.vec).toBe(false) - expect(L_.layers.opacity.vec).toBe(0.4) + expect(L_.layers.opacity.a).toBe(0.5) }) - test('an aggregate registry entry (array of Leaflet layers) is not routed to the engine', () => { + test('an aggregate registry entry (array of layers) is not routed to the engine', () => { const setLayerOpacity = vi.fn() - setEngine(MAP_ENGINE.DECKGL, setLayerOpacity) - L_.layers.layer.arrows = [makeLeafletLayer(), makeLeafletLayer()] - - expect(() => L_.setLayerOpacity('arrows', 0.4)).not.toThrow() - + setEngine(MAP_ENGINE.LEAFLET, setLayerOpacity) + L_.layers.layer.a = [makeLeafletLayer(), makeLeafletLayer()] + L_.setLayerOpacity('a', 0.5) expect(setLayerOpacity).not.toHaveBeenCalled() - expect(L_.layers.opacity.arrows).toBe(0.4) + expect(L_.layers.opacity.a).toBe(0.5) + }) + + test('is a no-op on the engine before Map_ is initialized', () => { + L_.Map_ = null + L_.layers.layer.a = makeLeafletLayer() + expect(() => L_.setLayerOpacity('a', 0.5)).not.toThrow() + expect(L_.layers.opacity.a).toBe(0.5) }) }) From b2be6b4cbea0a1744d4362c3800c54838ee1f9e2 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:04:43 -0500 Subject: [PATCH 09/22] Match DeckGLAdapter.setLayerOpacity's signature to IMapEngine DeckGLAdapter declared no third parameter, so it silently discarded the fillOpacity a caller computed and passed. TypeScript allows a narrower implementation of an optional trailing parameter, so this never surfaced as a compile error even though the interface, and LeafletAdapter, both already had three. Adds the parameter and documents why deck.gl does not apply it separately: its single opacity prop already scales stroke and fill together, so there is no separate fill channel to target here. Updates IMapEngine's setLayerOpacity doc to state the per-engine difference instead of implying uniform handling, and pins the non-behaviour with a test so a future change that starts honouring fillOpacity separately under deck.gl has to touch it deliberately. --- .../MapEngines/Adapters/DeckGLAdapter.ts | 14 +++++++++++++- src/essence/Basics/MapEngines/IMapEngine.ts | 14 ++++++++++++-- tests/unit/engineSetLayerOpacity.spec.js | 19 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index a2bf8094f..75f035e99 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -1037,8 +1037,20 @@ export class DeckGLAdapter implements IMapEngine { * that never became a layer) is a no-op here — `L_.layers.opacity[name]` * is always written by the caller, and layer creation reads it, so it * picks up the opacity when it is built or re-added. + * + * `options.fillOpacity` is accepted to satisfy {@link IMapEngine}, not + * applied separately: deck.gl's single `opacity` prop already scales a + * layer's stroke and fill together at draw time, so there is no separate + * fill channel to target at this level the way Leaflet's `setStyle` has + * one. The value is subsumed by `opacity` rather than ignored — a caller + * that computed it is not silently overridden, there is just nothing + * further for this adapter to do with it. */ - setLayerOpacity(layer: Layer | string, opacity: number): void { + setLayerOpacity( + layer: Layer | string, + opacity: number, + options?: { fillOpacity?: number } + ): void { const id = resolveLayerId(layer) if (this._layers.has(id)) this.updateLayer(id, { opacity }) } diff --git a/src/essence/Basics/MapEngines/IMapEngine.ts b/src/essence/Basics/MapEngines/IMapEngine.ts index 4b26e4a7b..cac0fca2f 100644 --- a/src/essence/Basics/MapEngines/IMapEngine.ts +++ b/src/essence/Basics/MapEngines/IMapEngine.ts @@ -260,9 +260,19 @@ export interface IMapEngine< * whatever the change requires — mutating it (Leaflet) or replacing the one * it holds (deck.gl). Callers never adopt a replacement. * + * The two engines do not honour `fillOpacity` uniformly — this is a real + * per-engine difference, not an oversight: + * - Leaflet applies it to the fill of layers that paint one separately + * from their stroke (`setStyle`'s `fillOpacity`). + * - deck.gl has no separate fill channel at this level: its single + * `opacity` prop scales stroke and fill together at draw time, so the + * value is accepted (to satisfy this signature) and subsumed by + * `opacity` rather than applied on its own. + * * @param options.fillOpacity - The fill opacity to apply to layers that - * paint one. Scaling policy belongs to the caller, so this is an absolute - * value, never a factor the adapter multiplies. Defaults to `opacity`. + * paint one separately from their stroke. Scaling policy belongs to the + * caller, so this is an absolute value, never a factor the adapter + * multiplies. Defaults to `opacity`. See per-engine note above. */ setLayerOpacity( layer: TLayer | string, diff --git a/tests/unit/engineSetLayerOpacity.spec.js b/tests/unit/engineSetLayerOpacity.spec.js index 905eddf8d..ae65133aa 100644 --- a/tests/unit/engineSetLayerOpacity.spec.js +++ b/tests/unit/engineSetLayerOpacity.spec.js @@ -90,4 +90,23 @@ describe('DeckGLAdapter.setLayerOpacity', () => { expect(adapter.setLayerOpacity('nope', 0.5)).toBeUndefined() expect(adapter.getLayers()).toHaveLength(0) }) + + // deck.gl has no separate fill channel at this level — a single `opacity` + // prop covers stroke and fill together — so options.fillOpacity is + // accepted (matching IMapEngine's signature) but subsumed by `opacity` + // rather than applied on its own. Pinned so a future change that starts + // honouring it separately has to update this test rather than slip + // through silently. + test('accepts a third argument and still applies opacity, not a separate fill value', () => { + const adapter = new DeckGLAdapter() + adapter.addLayer(makeDeckLayer('l1', { opacity: 1 })) + + expect( + adapter.setLayerOpacity('l1', 0.5, { fillOpacity: 0.1 }) + ).toBeUndefined() + + const held = adapter.getLayers().find((l) => l.id === 'l1') + expect(held.props.opacity).toBe(0.5) + expect(held.props.fillOpacity).toBeUndefined() + }) }) From 198bb89fc0413d2f3bac6710cc060c84c188d47e Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:32:12 -0500 Subject: [PATCH 10/22] Compile deck tile URLs on the domain side, not in the adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DeckGLAdapter.refreshLayer fell back to compileTileUrl, which branches on MMGIS service prefixes (stac-collection, COG, titiler-url) and injects COG fields — layer-kind knowledge an adapter must not hold, and a new Adapters -> Layers_ dependency. Map_.makeTileLayer's plain deck tile branch now registers a refresher that does the compilation, the way the deckRaster branch beside it already does. The guard against blanking a layer with an unresolvable URL moves into that refresher: it returns nothing, so the engine keeps the instance it holds. A held layer with no refresher now reports false rather than compiling one itself, matching the Leaflet adapter's answer for a layer it cannot refresh. --- .../MapEngines/Adapters/DeckGLAdapter.ts | 31 ++++------ src/essence/Basics/Map_/Map_.js | 28 ++++++++++ tests/unit/engineRefreshLayer.spec.js | 56 ++++++++++++++++--- 3 files changed, 88 insertions(+), 27 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index 75f035e99..8ed5e0de6 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -42,7 +42,6 @@ import type { BasemapOptions, } from '../types/view' import type { LayerOptions, OverlayOptions, RefreshContext } from '../types/layers' -import { compileTileUrl } from '../../Layers_/tileUrlUtils' import type { MapEventHandler, MapEventOptions, @@ -964,26 +963,20 @@ export class DeckGLAdapter implements IMapEngine { const existing = this._layers.get(id) if (!existing) return false + // No fallback: how a layer recomputes itself is layer-kind knowledge, + // and this adapter has none. The module that owns the kind registers a + // refresher at creation (see Map_.makeTileLayer); a held layer without + // one has no way to refresh, exactly as in the Leaflet adapter. const refresh = this._refreshers.get(id) - let next: Layer | void - if (refresh) { - next = refresh(existing, { - url: ctx.url, - tileOptions: ctx.tileOptions, - force: ctx.force, - }) - } else { - // Plain tile layers take one static URL, so the per-tile params - // Leaflet would add are baked in here. - if (ctx.url == null) return false - const compiled = compileTileUrl(ctx.url, ctx.tileOptions ?? {}) - // A layer with no resolvable service URL compiles to nothing; - // handing that to deck would blank it. - if (!compiled) return false - next = existing.clone({ data: compiled }) as Layer - } + if (!refresh) return false + + const next = refresh(existing, { + url: ctx.url, + tileOptions: ctx.tileOptions, + force: ctx.force, + }) - // A refresher that mutated in place returns nothing; keep what we hold. + // A refresher with nothing to apply returns nothing; keep what we hold. if (next) this._layers.set(id, next) this._syncLayers() return true diff --git a/src/essence/Basics/Map_/Map_.js b/src/essence/Basics/Map_/Map_.js index 1e630c5ff..6f9c9db64 100644 --- a/src/essence/Basics/Map_/Map_.js +++ b/src/essence/Basics/Map_/Map_.js @@ -1734,6 +1734,34 @@ async function makeTileLayer(layerObj, mapContext = null) { ), }, }) + + // A plain deck tile layer takes one static URL, so the per-tile params + // Leaflet adds in getTileUrl have to be baked in on every refresh too. + // Registered here, on the domain side, because compileTileUrl is not + // generic — it branches on MMGIS service prefixes (stac-collection, + // COG, titiler-url) and injects COG fields. An adapter must not know + // any of that; it only knows it has a function to call. + // Guarded to the main map for the same reason registerLayer below is: + // Map_.engine is always the MAIN map's engine, so a non-default ctx + // would collide with the main map's entry under the same uuid. + if (ctx.default === true) { + Map_.engine.setLayerRefresher( + layerObj.name, + (layer, refreshCtx) => { + // No source URL, or one that compiles to nothing: return + // nothing so the engine keeps the instance it holds. + // Handing deck an empty url would blank the layer. + if (refreshCtx.url == null) return + const compiled = compileTileUrl( + refreshCtx.url, + refreshCtx.tileOptions ?? {} + ) + if (!compiled) return + return layer.clone({ data: compiled }) + } + ) + } + L_._layersLoaded[L_._layersOrdered.indexOf(layerObj.name)] = true allLayersLoaded() return diff --git a/tests/unit/engineRefreshLayer.spec.js b/tests/unit/engineRefreshLayer.spec.js index de69f3b33..79ba57c82 100644 --- a/tests/unit/engineRefreshLayer.spec.js +++ b/tests/unit/engineRefreshLayer.spec.js @@ -1,6 +1,7 @@ import { describe, test, expect, vi } from 'vitest' import LeafletAdapter from '../../src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts' import DeckGLAdapter from '../../src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts' +import { compileTileUrl } from '../../src/essence/Basics/Layers_/tileUrlUtils.ts' /** * refreshLayer is the single update entry point both engines expose (#274). @@ -108,12 +109,48 @@ describe('DeckGLAdapter.refreshLayer', () => { expect(adapter.getLayers().find((l) => l.id === 'l1')).toBe(original) }) - test('falls back to cloning with a compiled tile URL', () => { + test('returns false for an unregistered id', () => { + expect(new DeckGLAdapter().refreshLayer('nope')).toBe(false) + }) + + // The adapter has no fallback: compiling a tile URL means knowing about + // MMGIS service prefixes and COG fields, which is layer-kind knowledge the + // adapter must not hold. A held layer with no refresher has no way to + // recompute itself, matching the Leaflet adapter's answer for a layer with + // neither a refresher nor a native refresh(). + test('returns false for a held layer with no refresher registered', () => { + const adapter = new DeckGLAdapter() + const original = makeDeckLayer('l1', { data: 'old' }) + adapter.addLayer(original) + + expect( + adapter.refreshLayer('l1', { url: 'https://x/{z}/{x}/{y}.png' }) + ).toBe(false) + expect(adapter.getLayers().find((l) => l.id === 'l1')).toBe(original) + }) +}) + +/** + * The refresher Map_.makeTileLayer registers for a plain deck tile layer. + * Reproduced here because Map_ cannot be imported under vitest (it pulls in + * JSX viewers Vite will not parse from a .js file); what this pins is the + * compile-and-clone contract the adapter drives, not the registration itself. + */ +const makeTileRefresher = () => (layer, ctx) => { + if (ctx.url == null) return + const compiled = compileTileUrl(ctx.url, ctx.tileOptions ?? {}) + if (!compiled) return + return layer.clone({ data: compiled }) +} + +describe('the plain deck tile refresher', () => { + test('clones with a compiled tile URL', () => { const adapter = new DeckGLAdapter() adapter.addLayer(makeDeckLayer('l1', { data: 'old' })) + adapter.setLayerRefresher('l1', makeTileRefresher()) // The url carries a {time} placeholder and tileOptions substitutes it, - // so this is only observable if the fallback actually calls + // so this is only observable if the refresher actually calls // compileTileUrl — a byte-identical url (no placeholder) would pass // even if compileTileUrl were never invoked. expect( @@ -127,16 +164,19 @@ describe('DeckGLAdapter.refreshLayer', () => { expect(data).toContain('2024-01-01T00:00:00Z') }) - test('returns false for an unregistered id', () => { - expect(new DeckGLAdapter().refreshLayer('nope')).toBe(false) - }) - - test('does not clone when the fallback has no url to compile', () => { + // A `COG:` layer with no TiTiler service behind it resolves to no url. + // Handing that to deck would blank the layer, so the refresher declines + // and the engine keeps the instance it holds. + test('leaves the held layer alone when there is no url to compile', () => { const adapter = new DeckGLAdapter() const original = makeDeckLayer('l1', { data: 'old' }) adapter.addLayer(original) + adapter.setLayerRefresher('l1', makeTileRefresher()) - expect(adapter.refreshLayer('l1')).toBe(false) + adapter.refreshLayer('l1', { url: null }) + expect(adapter.getLayers().find((l) => l.id === 'l1')).toBe(original) + + adapter.refreshLayer('l1', { url: '' }) expect(adapter.getLayers().find((l) => l.id === 'l1')).toBe(original) }) }) From 03574d6b3cbbdd7937df322603f54943b71eee87 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:34:46 -0500 Subject: [PATCH 11/22] Decline a non-clonable layer in the deck adapter instead of throwing Under the deck.gl engine MMGIS still builds data, image, video and velocity layers as native Leaflet objects, and L_.setLayerOpacity hands every registry entry to the active engine unconditionally. Such an object carries no deck id, so it usually falls out of resolveLayerId as undefined; but once one is held under that key, updateLayer reached clone() on it and threw. Both updateLayer and setLayerOpacity now check the held value is clonable, so the outcome is a no-op either way. Covered against a real DeckGLAdapter rather than a mocked engine. --- .../MapEngines/Adapters/DeckGLAdapter.ts | 17 ++++++++- tests/unit/engineSetLayerOpacity.spec.js | 36 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index 8ed5e0de6..dcd393d04 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -928,11 +928,19 @@ export class DeckGLAdapter implements IMapEngine { /** * Clone the existing layer with overridden props. deck.gl detects the same * `id` and updates GPU resources incrementally. + * + * A held value that is not a clonable deck layer is a no-op. Under the + * deck.gl engine MMGIS still builds `data`, `image`, `video` and + * `velocity` layers as native Leaflet objects — ENGINE_LAYER_SUPPORT has + * no deck builder for them — and callers hand every registry entry to the + * active engine. Such an object has no `clone`, so this declines rather + * than throwing `existing.clone is not a function`. */ updateLayer(layer: Layer | string, options: Partial): Layer { const id = resolveLayerId(layer) const existing = this._layers.get(id) if (!existing) return existing as unknown as Layer + if (typeof existing.clone !== 'function') return existing const updated = existing.clone({ ...(options.opacity !== undefined ? { opacity: options.opacity } : {}), ...(options.visible !== undefined ? { visible: options.visible } : {}), @@ -1031,6 +1039,12 @@ export class DeckGLAdapter implements IMapEngine { * is always written by the caller, and layer creation reads it, so it * picks up the opacity when it is built or re-added. * + * So is a native Leaflet layer, which reaches here because MMGIS still + * builds `data`, `image`, `video` and `velocity` layers with Leaflet under + * the deck.gl engine and hands every registry entry to the active engine. + * It carries no `id`, so it is not found; and if it ever were registered, + * {@link updateLayer} declines a value it cannot clone. + * * `options.fillOpacity` is accepted to satisfy {@link IMapEngine}, not * applied separately: deck.gl's single `opacity` prop already scales a * layer's stroke and fill together at draw time, so there is no separate @@ -1045,7 +1059,8 @@ export class DeckGLAdapter implements IMapEngine { options?: { fillOpacity?: number } ): void { const id = resolveLayerId(layer) - if (this._layers.has(id)) this.updateLayer(id, { opacity }) + const existing = this._layers.get(id) + if (typeof existing?.clone === 'function') this.updateLayer(id, { opacity }) } /** diff --git a/tests/unit/engineSetLayerOpacity.spec.js b/tests/unit/engineSetLayerOpacity.spec.js index ae65133aa..e2adcd75a 100644 --- a/tests/unit/engineSetLayerOpacity.spec.js +++ b/tests/unit/engineSetLayerOpacity.spec.js @@ -91,6 +91,42 @@ describe('DeckGLAdapter.setLayerOpacity', () => { expect(adapter.getLayers()).toHaveLength(0) }) + /** + * Under the deck.gl engine MMGIS still builds `data`, `image`, `video` + * and `velocity` layers as native Leaflet objects — ENGINE_LAYER_SUPPORT + * has no deck builder for them — and L_.setLayerOpacity hands every + * registry entry to the active engine without inspecting its shape. So a + * Leaflet object does reach this adapter, and must not blow it up. + */ + describe('given a native Leaflet layer', () => { + test('is a no-op passed by object, and does not throw', () => { + const adapter = new DeckGLAdapter() + const velocity = makeTileish() + + expect(adapter.setLayerOpacity(velocity, 0.3)).toBeUndefined() + // Not dimmed here: a Leaflet object carries no deck `id`, so the + // engine never held it. L_.layers.opacity keeps the value. + expect(velocity.opacity).toBe(null) + expect(adapter.getLayers()).toHaveLength(0) + }) + + test('is a no-op even once the engine holds one, and does not throw', () => { + const adapter = new DeckGLAdapter() + const velocity = makeTileish() + // addLayer keys off layer.id, which a Leaflet object lacks, so the + // entry lands under `undefined`. Reproduced rather than endorsed — + // what this pins is that reaching such an entry cannot throw + // `existing.clone is not a function`. + adapter.addLayer(velocity) + + expect(adapter.setLayerOpacity(velocity, 0.3)).toBeUndefined() + expect(velocity.opacity).toBe(null) + expect(() => + adapter.updateLayer(velocity, { opacity: 0.3 }) + ).not.toThrow() + }) + }) + // deck.gl has no separate fill channel at this level — a single `opacity` // prop covers stroke and fill together — so options.fillOpacity is // accepted (matching IMapEngine's signature) but subsumed by `opacity` From 9f84ec52c2da392dfa6e3af14b435e3137c8d166 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:36:10 -0500 Subject: [PATCH 12/22] Document that a Leaflet refresher mutates in place IMapEngine claimed the engine reconciles either a returned replacement or an in-place mutation, but LeafletAdapter.refreshLayer discards any return. Adopting it into the registry without also swapping the layer on the map would leave the two disagreeing, so narrow the contract instead: Leaflet refreshers mutate in place and their return value is ignored. The adapter's own parameter and refresher map now say so with a void return. --- .../MapEngines/Adapters/LeafletAdapter.ts | 17 ++++++++++++++--- src/essence/Basics/MapEngines/IMapEngine.ts | 18 +++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts index deba01c0e..22ab988eb 100644 --- a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts @@ -88,8 +88,11 @@ export default class LeafletAdapter implements IMapEngine, IMapEn */ private _layers: Map = new Map() - /** Per-layer refresh hooks, keyed the same way as {@link _layers}. */ - private _refreshers: Map any> = new Map() + /** + * Per-layer refresh hooks, keyed the same way as {@link _layers}. They + * mutate in place and return nothing — see {@link setLayerRefresher}. + */ + private _refreshers: Map void> = new Map() /** * Registry of markers by ID @@ -767,9 +770,16 @@ export default class LeafletAdapter implements IMapEngine, IMapEn this._layers.set(id, layer) } + /** + * A Leaflet refresher mutates the layer in place — the instance is already + * on the map, so there is nothing for this adapter to swap. The narrowed + * `void` return says so at the type level: {@link refreshLayer} discards + * whatever a refresher returns rather than half-reconciling it into the + * registry while the map still shows the old instance. + */ setLayerRefresher( id: string, - refresh: ((layer: any, ctx: RefreshContext) => any) | null + refresh: ((layer: any, ctx: RefreshContext) => void) | null ): void { if (refresh == null) this._refreshers.delete(id) else this._refreshers.set(id, refresh) @@ -779,6 +789,7 @@ export default class LeafletAdapter implements IMapEngine, IMapEn const layer = this._layers.get(id) if (!layer) return false + // Return value deliberately ignored — see setLayerRefresher above. const refresh = this._refreshers.get(id) if (refresh) { refresh(layer, { diff --git a/src/essence/Basics/MapEngines/IMapEngine.ts b/src/essence/Basics/MapEngines/IMapEngine.ts index cac0fca2f..3433eba93 100644 --- a/src/essence/Basics/MapEngines/IMapEngine.ts +++ b/src/essence/Basics/MapEngines/IMapEngine.ts @@ -218,9 +218,21 @@ export interface IMapEngine< * * Called by the module that owns the layer kind, at creation — never by an * adapter, which stays layer-type-agnostic. The engine invokes it with the - * live instance; return a replacement (deck.gl) or mutate in place and - * return nothing (Leaflet). The engine reconciles either way and remains - * the owner, so the function must not retain the instance. + * live instance and remains its owner, so the function must not retain it. + * + * What a refresher does with that instance differs per engine, because the + * two engines' layers do: + * - **deck.gl** — layers are immutable, so a refresher returns a + * replacement and the engine adopts it. Returning nothing means "nothing + * to apply" and the engine keeps what it holds. + * - **Leaflet** — layers are mutable and already on the map, so a + * refresher mutates in place. Any value it returns is IGNORED: adopting + * one into the registry without also swapping the layer on the map would + * leave the two disagreeing, so the adapter does not try. + * + * The signature keeps `TLayer | void` because this interface is generic + * over whichever engine is in play; the Leaflet adapter narrows its own + * parameter to a void-returning function. */ setLayerRefresher( id: string, From c5227457cd132fa98fed211eae3ffa4823a2a0c1 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:38:36 -0500 Subject: [PATCH 13/22] Answer hasLayer from the map, not the Leaflet registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit registerLayer changed what _layers means: it now holds every makeTileLayer tile layer whether or not that layer is on the map. hasLayer(string) still read it with the old meaning, so it disagreed with hasLayer(layerObject) and mmgisAPI's public map:hasLayer changed its answer. Both forms now ask the map. removeLayer's object branch still keeps the registration on purpose — Map_.rmNotNull removes by object on toggle-off and TimeControl.reloadLayer's evenIfOff path has to refresh those layers — which is now recorded in a comment and pinned by a test. --- .../MapEngines/Adapters/LeafletAdapter.ts | 22 +++++-- tests/unit/LeafletAdapter.spec.js | 61 +++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts index 22ab988eb..6d6a46e25 100644 --- a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts @@ -633,11 +633,20 @@ export default class LeafletAdapter implements IMapEngine, IMapEn return Array.from(this._layers.values()) } + /** + * Whether the layer is currently on the map. + * + * Both forms ask the map, never the registry. `_layers` stopped being an + * answer on its own once {@link registerLayer} began holding every + * MMGIS-built tile layer whether or not it is on the map — reading it here + * would make `hasLayer(id)` and `hasLayer(layerObject)` disagree, and + * mmgisAPI's `map:hasLayer` exposes this answer publicly. + */ hasLayer(layer: any | string): boolean { - if (typeof layer === 'string') { - return this._layers.has(layer) - } - return this._map.hasLayer(layer) + const leafletLayer = + typeof layer === 'string' ? this._layers.get(layer) : layer + if (!leafletLayer) return false + return this._map?.hasLayer(leafletLayer) === true } /** @@ -708,6 +717,11 @@ export default class LeafletAdapter implements IMapEngine, IMapEn this._refreshers.delete(layer) } } else { + // Deliberately keeps the registration. Map_.rmNotNull removes + // layers by object every time one is toggled off, and a toggled-off + // layer still has to be refreshable — TimeControl.reloadLayer's + // `evenIfOff` path depends on it. Only the id form, which means + // "destroy this layer", drops the entry. this._map.removeLayer(layer) } } diff --git a/tests/unit/LeafletAdapter.spec.js b/tests/unit/LeafletAdapter.spec.js index 95018467b..d8fa4a47d 100644 --- a/tests/unit/LeafletAdapter.spec.js +++ b/tests/unit/LeafletAdapter.spec.js @@ -480,10 +480,71 @@ test.describe('LeafletAdapter - addLayer (backward compatibility)', () => { }) }) +// ─── hasLayer ──────────────────────────────────────────────────────────────── + +test.describe('LeafletAdapter - hasLayer', () => { + + // registerLayer holds every MMGIS-built tile layer whether or not it is on + // the map, so a registry hit is not the answer. Both forms ask the map, and + // must give the same one — mmgisAPI's `map:hasLayer` exposes it publicly. + test('a registered layer that is not on the map reports false either way', () => { + setupWithLayerMocks() + const adapter = new LeafletAdapter() + adapter.init({ containerId: 'map' }) + + const layer = { _leaflet_id: 7 } + adapter.registerLayer('off-map', layer) + + expect(adapter.hasLayer('off-map')).toBe(false) + expect(adapter.hasLayer(layer)).toBe(false) + }) + + test('both forms report true once the layer is on the map', () => { + setupWithLayerMocks() + const adapter = new LeafletAdapter() + adapter.init({ containerId: 'map' }) + + const layer = { _leaflet_id: 7 } + adapter.registerLayer('on-map', layer) + adapter.addLayer(layer) + + expect(adapter.hasLayer('on-map')).toBe(true) + expect(adapter.hasLayer(layer)).toBe(true) + }) + + test('an id the adapter never saw reports false rather than throwing', () => { + setupWithLayerMocks() + const adapter = new LeafletAdapter() + adapter.init({ containerId: 'map' }) + + expect(adapter.hasLayer('never-seen')).toBe(false) + }) +}) + // ─── removeLayer ───────────────────────────────────────────────────────────── test.describe('LeafletAdapter - removeLayer', () => { + // Map_.rmNotNull removes by object on every toggle-off, and a toggled-off + // layer still has to be refreshable — TimeControl.reloadLayer's `evenIfOff` + // path depends on it — so the object form deliberately keeps the entry. + test('removing by object keeps the registration', () => { + const { removedLayers } = setupWithLayerMocks() + const adapter = new LeafletAdapter() + adapter.init({ containerId: 'map' }) + + const layer = { _leaflet_id: 9, refresh: vi.fn() } + adapter.registerLayer('toggled-off', layer) + adapter.addLayer(layer) + + adapter.removeLayer(layer) + + expect(removedLayers).toContain(layer) + expect(adapter.hasLayer('toggled-off')).toBe(false) + expect(adapter.refreshLayer('toggled-off', { url: 'u' })).toBe(true) + expect(layer.refresh).toHaveBeenCalled() + }) + test('removes a layer by string ID and cleans up the registry', () => { const { removedLayers } = setupWithLayerMocks() const adapter = new LeafletAdapter() From 5a79999761eac629e8750c3b4dfe222b2b289265 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:39:57 -0500 Subject: [PATCH 14/22] Read the COG refresher's config from the registry on each call The refresher captured the layerObj reference at factory time while its docs promised the live L_.layers.data entry. It works only because every config writer mutates in place; one that replaces the entry would go unseen. It now looks the entry up per call, falling back to the given object when the registry has none. --- .../Basics/Layers_/deckCOGRefresher.js | 17 ++++++++---- tests/unit/deckCOGRefresher.spec.js | 26 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/essence/Basics/Layers_/deckCOGRefresher.js b/src/essence/Basics/Layers_/deckCOGRefresher.js index b3879c976..f6742cb48 100644 --- a/src/essence/Basics/Layers_/deckCOGRefresher.js +++ b/src/essence/Basics/Layers_/deckCOGRefresher.js @@ -16,17 +16,24 @@ import L_ from './Layers_' * layer's id, so deck.gl diffs it against the old one and cached tiles survive. * * @param {string} uuid - Layer UUID, also the engine-side layer id. - * @param {object} layerObj - The live `L_.layers.data` entry, read on each call. + * @param {object} layerObj - Fallback config, used only when the registry has + * no `L_.layers.data` entry for `uuid` at call time. * @returns {(layer: object) => object} A refresher returning the replacement. */ export function makeDeckCOGRefresher(uuid, layerObj) { - return (layer) => - layer.clone( + return (layer) => { + // Looked up per call rather than captured, so this stays a derivation + // of current config. Every writer mutates the entry in place today, + // which would hide a captured reference right up until one replaces + // the entry instead. + const config = L_.layers.data[uuid] ?? layerObj + return layer.clone( deckCOGProps(uuid, { - rawCogUrl: resolveDeckCOGFileUrl(layerObj), - layerObj, + rawCogUrl: resolveDeckCOGFileUrl(config), + layerObj: config, // ?? not ||: an opacity of 0 is a real value, not "default to 1" opacity: L_.layers.opacity[uuid] ?? 1, }) ) + } } diff --git a/tests/unit/deckCOGRefresher.spec.js b/tests/unit/deckCOGRefresher.spec.js index 11b4f9e13..650821e0d 100644 --- a/tests/unit/deckCOGRefresher.spec.js +++ b/tests/unit/deckCOGRefresher.spec.js @@ -48,6 +48,32 @@ describe('makeDeckCOGRefresher', () => { expect(refresh(makeDeckLayer('l2')).props.opacity).toBe(0.25) }) + // Every config writer mutates the L_.layers.data entry in place today, so + // capturing the reference happens to work. It stops working the moment one + // replaces the entry instead — which is what this simulates. + test('reads the live registry entry, not the object captured at creation', () => { + const captured = { name: 'l4', url: 'COG:a.tif', cogColormap: 'viridis' } + L_.layers.opacity.l4 = 1 + const refresh = makeDeckCOGRefresher('l4', captured) + + L_.layers.data.l4 = { ...captured, cogColormap: 'plasma' } + + expect( + refresh(makeDeckLayer('l4')).props.updateTriggers.renderTile[0] + ).toBe('plasma') + }) + + test('falls back to the given config when the registry has no entry', () => { + const layerObj = { name: 'l5', url: 'COG:a.tif', cogColormap: 'magma' } + L_.layers.opacity.l5 = 1 + delete L_.layers.data.l5 + + expect( + makeDeckCOGRefresher('l5', layerObj)(makeDeckLayer('l5')).props + .updateTriggers.renderTile[0] + ).toBe('magma') + }) + test('returns a new instance rather than mutating the old one', () => { const layerObj = { name: 'l3', url: 'COG:a.tif' } L_.layers.opacity.l3 = 1 From 284bc38ec4585bc0c6753b2f49cde7ff737ae8df Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:41:43 -0500 Subject: [PATCH 15/22] Warn when a time reload finds no layer to refresh refreshLayer returns false to say the engine had nothing to refresh. reloadLayer dropped it, so the time change vanished and stale tiles stayed on screen with nothing to explain them. Name the layer in a console.warn instead. Also restore optional chaining on the two engine dereferences this branch added: TimeControl.reloadLayer replaced code that read Map_.engine?.engineType, and makeTileLayer's Leaflet tail is reached precisely when the deck branch's own `if (Map_.engine && ...)` did not hold. --- src/essence/Basics/Map_/Map_.js | 4 ++- .../Basics/TimeControl_/TimeControl.js | 10 ++++++- tests/unit/timeControlReloadLayer.spec.js | 27 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/essence/Basics/Map_/Map_.js b/src/essence/Basics/Map_/Map_.js index 6f9c9db64..3b755ba5f 100644 --- a/src/essence/Basics/Map_/Map_.js +++ b/src/essence/Basics/Map_/Map_.js @@ -1793,8 +1793,10 @@ async function makeTileLayer(layerObj, mapContext = null) { // Guarded to the main map: Map_.engine is always the MAIN map's engine, so // registering a layer built for a secondary ctx (its own map/registry) // would collide with the main map's entry under the same uuid. + // Optional: the deck branch above is entered only `if (Map_.engine && ...)`, + // so this tail is reached precisely when Map_.engine may be missing. if (ctx.default === true) { - Map_.engine.registerLayer( + Map_.engine?.registerLayer( layerObj.name, ctx.layerRegistry.layer[layerObj.name] ) diff --git a/src/essence/Basics/TimeControl_/TimeControl.js b/src/essence/Basics/TimeControl_/TimeControl.js index 7b80f7075..a36c5d76a 100644 --- a/src/essence/Basics/TimeControl_/TimeControl.js +++ b/src/essence/Basics/TimeControl_/TimeControl.js @@ -409,11 +409,19 @@ var TimeControl = { // The engine decides whether this means mutating, // cloning or rebuilding; the layer's registered refresher // supplies the how for kinds that need one. - Map_.engine.refreshLayer(layer.name, { + const refreshed = Map_.engine?.refreshLayer(layer.name, { url: resolvedUrl, tileOptions, force: forceRequery === true, }) + // false means the engine had nothing to refresh — the + // layer was never registered with it, or it has no way to + // recompute it. The time change is then silently lost, so + // say so rather than leaving stale tiles unexplained. + if (refreshed === false) + console.warn( + `TimeControl.reloadLayer: the map engine had no layer to refresh for '${layer.name}'; its time change was not applied.` + ) } } } else if (layer.type == 'velocity') { diff --git a/tests/unit/timeControlReloadLayer.spec.js b/tests/unit/timeControlReloadLayer.spec.js index 9b096f5db..c60434066 100644 --- a/tests/unit/timeControlReloadLayer.spec.js +++ b/tests/unit/timeControlReloadLayer.spec.js @@ -201,4 +201,31 @@ describe('TimeControl.reloadLayer with the deck.gl engine', () => { expect(refreshLayer).not.toHaveBeenCalled() expect(Map_.refreshLayer).toHaveBeenCalled() }) + + // refreshLayer returns false specifically to say it had no layer to + // refresh. Dropping that leaves the time change silently unapplied and + // stale tiles on screen with nothing to explain them. + test('warns by name when the engine had no layer to refresh', async () => { + refreshLayer.mockReturnValueOnce(false) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const layer = makeNO2Layer('TileLayer') + registerDeckLayer(layer) + + await TimeControl.reloadLayer(layer) + + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0][0]).toContain('NO2 Monthly') + warn.mockRestore() + }) + + test('stays quiet when the engine refreshed the layer', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const layer = makeNO2Layer('TileLayer') + registerDeckLayer(layer) + + await TimeControl.reloadLayer(layer) + + expect(warn).not.toHaveBeenCalled() + warn.mockRestore() + }) }) From ee193732887697160024f608360d1c265e4bd28d Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:43:47 -0500 Subject: [PATCH 16/22] Type deckCOGProps against COGLayer's own props MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single source of COG props returned Record, was cast with `as any` at the constructor, and had degraded getTileData's params to (image: any, opts: any) — so a misspelled prop compiled. It now returns COGLayerProps, both casts are gone, and getTileData takes GeoTIFF | Overview and GetTileDataOptions. The options shape the two exported functions shared is extracted as DeckCOGOptions. --- .../MapEngines/Adapters/DeckCOGLayer.ts | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts b/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts index 040bff827..42ea3591a 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckCOGLayer.ts @@ -24,7 +24,10 @@ import { addAlphaChannel, texture as textureUtils, } from '@developmentseed/deck.gl-geotiff' -import type { GetTileDataOptions } from '@developmentseed/deck.gl-geotiff' +import type { + COGLayerProps, + GetTileDataOptions, +} from '@developmentseed/deck.gl-geotiff' import type { GeoTIFF, Overview } from '@developmentseed/geotiff' import type { MinimalTileData, @@ -394,19 +397,30 @@ function makeRenderTile(opts: { // Factory // --------------------------------------------------------------------------- +/** + * What creation and refresh both need to derive a COG layer's props. + * + * `rawCogUrl` is the bare `.tif` URL with no TiTiler host or query params + * (resolveTileLayerSource's `fileUrl`). `layerObj` is the layer's mission + * config, taken loosely because it is parsed from JSON. + */ +export type DeckCOGOptions = { + rawCogUrl: string + layerObj: Record + opacity?: number +} + /** * The complete prop bag for a client-side COG layer, derived from current * config. Creation and every refresh go through this one function so a - * colormap default can never be computed two different ways. + * colormap default can never be computed two different ways — which also + * makes this the one place a typed return is worth having: it is checked + * against COGLayer's own props, so a misspelled prop fails to compile. */ export function deckCOGProps( id: string, - options: { - rawCogUrl: string - layerObj: Record - opacity?: number - } -): Record { + options: DeckCOGOptions +): COGLayerProps { const l = options.layerObj const colormapName = (l.currentCogColormap ?? l.cogColormap ?? 'viridis') as string const rescaleMin = Number(l.currentCogMin ?? l.cogMin ?? 0) @@ -427,8 +441,10 @@ export function deckCOGProps( // skip its default inferRenderPipeline, which throws for float COGs // ('non-unsigned integers not yet supported'). // The config nodata (if any) overrides the file's GDAL_NODATA. - getTileData: (image: any, opts: any) => - cogGetTileData(image, { ...opts, noDataOverride: nodata }), + getTileData: ( + image: GeoTIFF | Overview, + opts: GetTileDataOptions + ) => cogGetTileData(image, { ...opts, noDataOverride: nodata }), renderTile: makeRenderTile({ colormapName, rescaleMin, rescaleMax }), updateTriggers: { // Re-runs renderTile for already-loaded tiles (no refetch) when the @@ -447,17 +463,10 @@ export function deckCOGProps( * layer-owned GPU state to keep in sync (see `colormapTextures`). * * @param id - Layer id (layer name from layerObj). - * @param options - Raw COG file URL + layer config. `rawCogUrl` is the bare - * `.tif` URL with no TiTiler host or query params - * (resolveTileLayerSource's `fileUrl`). + * @param options - Raw COG file URL + layer config, see {@link DeckCOGOptions}. */ -export function buildDeckCOGLayer( - id: string, - options: { - rawCogUrl: string - layerObj: Record - opacity?: number - } -): Layer { - return new COGLayer(deckCOGProps(id, options) as any) as unknown as Layer +export function buildDeckCOGLayer(id: string, options: DeckCOGOptions): Layer { + return new COGLayer( + deckCOGProps(id, options) + ) as unknown as Layer } From 04561f5b30df8d4dce1ae24c62101a3f8e1065ea Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:46:42 -0500 Subject: [PATCH 17/22] Tidy the smaller findings from the branch review - RefreshContext.url is typed string | null: a source that resolves to nothing yields null, callers pass it through, and both adapters test for it. - L_.setLayerOpacity records the new opacity before the engine calls, so an exception from an attachment cannot skip the registry write that is now the sole source of truth. - Note why the main layer goes through nativeLayer while attachments do not. - Note why makeTileLayer's deckRaster branch registers a refresher but no layer, the opposite of its Leaflet sibling. - Re-title the deck opacity test to describe what it actually pins; arity is not something a JS test can enforce. --- src/essence/Basics/Layers_/Layers_.js | 13 +++++++++++-- src/essence/Basics/MapEngines/types/layers.ts | 7 ++++++- src/essence/Basics/Map_/Map_.js | 5 +++++ tests/unit/engineSetLayerOpacity.spec.js | 10 +++++----- tests/unit/layersOpacity.spec.js | 14 ++++++++++++++ 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index 1bf9fd34b..50bf302c6 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -2210,12 +2210,23 @@ const L_ = { ? parseFloat(L_.layers.data[name].style.fillOpacity) : 1 + // Recorded first: the registry is now the sole source of truth for a + // layer's opacity (getLayerOpacity reads it, and layer creation seeds + // itself from it). If an attachment below threw, a write down here + // would be skipped along with the marker pass and the layer would come + // back at the wrong opacity. + L_.layers.opacity[name] = newOpacity + // An MMGIS layer is a compound — main layer plus attachment // decorations. The caller iterates the parts and asks the engine once // per part; the adapter never learns what an attachment is. Skipped // here: the load-failure sentinel (false) and aggregate arrays, // neither of which is a layer the engine holds. if (engine && l && l !== false && !Array.isArray(l)) { + // nativeLayer unwraps the main layer, whose registry entry may be + // a wrapper carrying `._deckLayer` rather than the engine's own + // layer. Attachments below are passed raw because they are always + // plain Leaflet objects, never wrapped. engine.setLayerOpacity(L_.Map_.nativeLayer(l), newOpacity, { fillOpacity: newOpacity * configuredFill, }) @@ -2254,8 +2265,6 @@ const L_ = { opacity: newOpacity, }) - L_.layers.opacity[name] = newOpacity - if (L_.activeFeature?.layer && L_.activeFeature.layerName === name) { L_.highlight(L_.activeFeature.layer) } diff --git a/src/essence/Basics/MapEngines/types/layers.ts b/src/essence/Basics/MapEngines/types/layers.ts index 5de149de1..77be91dd8 100644 --- a/src/essence/Basics/MapEngines/types/layers.ts +++ b/src/essence/Basics/MapEngines/types/layers.ts @@ -171,9 +171,14 @@ export interface OverlayOptions { * *uncompiled* tile source URL — Leaflet recompiles per tile from * `tileOptions`, deck.gl bakes them in with `compileTileUrl`. A refresher that * derives its own URL (client-side COG) ignores both. + * + * `url` is nullable, not merely optional: a source that resolves to nothing — + * a `COG:` layer with no TiTiler service behind it — yields null, and callers + * pass it through so the refresher, not the call site, decides what to do + * with it. Both adapters test `ctx.url == null`. */ export type RefreshContext = { - url?: string + url?: string | null tileOptions?: Record force?: boolean } diff --git a/src/essence/Basics/Map_/Map_.js b/src/essence/Basics/Map_/Map_.js index 3b755ba5f..efa258d71 100644 --- a/src/essence/Basics/Map_/Map_.js +++ b/src/essence/Basics/Map_/Map_.js @@ -1681,6 +1681,11 @@ async function makeTileLayer(layerObj, mapContext = null) { // The layer kind supplies how it rebuilds; the engine executes it. // Registered here because this is where the deckRaster // classification already happened. + // + // A refresher but no registerLayer, the opposite of the + // Leaflet tail below: a deck layer already carries its own id + // and the engine adopts it when the layer is added, so there + // is nothing to register. Only the refresher is missing. Map_.engine.setLayerRefresher( layerObj.name, makeDeckCOGRefresher(layerObj.name, layerObj) diff --git a/tests/unit/engineSetLayerOpacity.spec.js b/tests/unit/engineSetLayerOpacity.spec.js index e2adcd75a..b1c56f721 100644 --- a/tests/unit/engineSetLayerOpacity.spec.js +++ b/tests/unit/engineSetLayerOpacity.spec.js @@ -129,11 +129,11 @@ describe('DeckGLAdapter.setLayerOpacity', () => { // deck.gl has no separate fill channel at this level — a single `opacity` // prop covers stroke and fill together — so options.fillOpacity is - // accepted (matching IMapEngine's signature) but subsumed by `opacity` - // rather than applied on its own. Pinned so a future change that starts - // honouring it separately has to update this test rather than slip - // through silently. - test('accepts a third argument and still applies opacity, not a separate fill value', () => { + // subsumed by `opacity` rather than applied on its own. Pinned so a future + // change that starts honouring it separately has to update this test + // rather than slip through silently. (Nothing here pins the parameter's + // declaration: JS does not enforce arity and vitest does not typecheck.) + test('subsumes a given fillOpacity into opacity, writing no separate fill prop', () => { const adapter = new DeckGLAdapter() adapter.addLayer(makeDeckLayer('l1', { opacity: 1 })) diff --git a/tests/unit/layersOpacity.spec.js b/tests/unit/layersOpacity.spec.js index 9d16fc5fe..a2cdfcefc 100644 --- a/tests/unit/layersOpacity.spec.js +++ b/tests/unit/layersOpacity.spec.js @@ -143,6 +143,20 @@ describe('L_.setLayerOpacity asks the engine per part', () => { expect(() => L_.setLayerOpacity('a', 0.5)).not.toThrow() expect(L_.layers.opacity.a).toBe(0.5) }) + + // The registry is the sole source of truth for opacity — getLayerOpacity + // reads it and layer creation seeds itself from it — so the write must not + // sit behind calls that can throw, such as an attachment's. + test('records the opacity even when an engine call throws', () => { + const setLayerOpacity = vi.fn(() => { + throw new Error('attachment blew up') + }) + setEngine(MAP_ENGINE.LEAFLET, setLayerOpacity) + L_.layers.layer.a = makeLeafletLayer() + + expect(() => L_.setLayerOpacity('a', 0.5)).toThrow() + expect(L_.layers.opacity.a).toBe(0.5) + }) }) describe('L_.getLayerOpacity reads the registry', () => { From a2b6bf987fc80b1a36b8db7427a590ed39b924d8 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 15:59:45 -0500 Subject: [PATCH 18/22] Fix RefreshContext JSDoc: refresher, not adapters, tests ctx.url --- src/essence/Basics/MapEngines/types/layers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/essence/Basics/MapEngines/types/layers.ts b/src/essence/Basics/MapEngines/types/layers.ts index 77be91dd8..6ffb614c1 100644 --- a/src/essence/Basics/MapEngines/types/layers.ts +++ b/src/essence/Basics/MapEngines/types/layers.ts @@ -175,7 +175,7 @@ export interface OverlayOptions { * `url` is nullable, not merely optional: a source that resolves to nothing — * a `COG:` layer with no TiTiler service behind it — yields null, and callers * pass it through so the refresher, not the call site, decides what to do - * with it. Both adapters test `ctx.url == null`. + * with it. The domain-side refresher registered in `Map_.makeTileLayer` tests `ctx.url == null`. */ export type RefreshContext = { url?: string | null From 95958d37f2fed0cafdcda5e1f8691c723ec74641 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Thu, 27 Aug 2026 16:19:53 -0500 Subject: [PATCH 19/22] Rewrite layer-engine comments to stand on their own Several comments added by the engine-layer-updates refactor were written against the change itself (before/after framing, work-scoping asides, "today"/"pre-existing" qualifiers) instead of documenting the resulting code. Rewrite them as standing facts a reader with no knowledge of the refactor can use, and trim a few for length without losing content. No executable code changed. --- src/essence/Basics/Layers_/Layers_.js | 6 ++-- .../Basics/Layers_/deckCOGRefresher.js | 6 ++-- .../MapEngines/Adapters/DeckGLAdapter.ts | 24 ++++++--------- .../MapEngines/Adapters/LeafletAdapter.ts | 18 ++++++------ src/essence/Basics/Map_/Map_.js | 29 ++++++++----------- 5 files changed, 36 insertions(+), 47 deletions(-) diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index 50bf302c6..5327996e3 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -2210,9 +2210,9 @@ const L_ = { ? parseFloat(L_.layers.data[name].style.fillOpacity) : 1 - // Recorded first: the registry is now the sole source of truth for a + // Recorded first: the registry is the sole source of truth for a // layer's opacity (getLayerOpacity reads it, and layer creation seeds - // itself from it). If an attachment below threw, a write down here + // itself from it). If an attachment below throws, a write down here // would be skipped along with the marker pass and the layer would come // back at the wrong opacity. L_.layers.opacity[name] = newOpacity @@ -2272,7 +2272,7 @@ const L_ = { getLayerOpacity: function (name) { // A layer that was never built has no opacity to report. Everything // else reads the registry, which is authoritative for both engines — - // Leaflet layer options are no longer mirrored and can be stale. + // layer options are not a source of opacity. if (L_.layers.layer[name] == null) return 0 return L_.layers.opacity[name] ?? 1 }, diff --git a/src/essence/Basics/Layers_/deckCOGRefresher.js b/src/essence/Basics/Layers_/deckCOGRefresher.js index f6742cb48..e0a531deb 100644 --- a/src/essence/Basics/Layers_/deckCOGRefresher.js +++ b/src/essence/Basics/Layers_/deckCOGRefresher.js @@ -23,9 +23,9 @@ import L_ from './Layers_' export function makeDeckCOGRefresher(uuid, layerObj) { return (layer) => { // Looked up per call rather than captured, so this stays a derivation - // of current config. Every writer mutates the entry in place today, - // which would hide a captured reference right up until one replaces - // the entry instead. + // of current config: every writer mutates the entry in place, but a + // captured reference would go stale if a future write replaced the + // entry outright instead of mutating it. const config = L_.layers.data[uuid] ?? layerObj return layer.clone( deckCOGProps(uuid, { diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index dcd393d04..611f822a8 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -1034,24 +1034,18 @@ export class DeckGLAdapter implements IMapEngine { * the instance the engine holds via {@link updateLayer}, which re-syncs * the render list. * - * A layer the engine does not hold (an unknown id, or a registry value - * that never became a layer) is a no-op here — `L_.layers.opacity[name]` - * is always written by the caller, and layer creation reads it, so it - * picks up the opacity when it is built or re-added. + * Two cases are a no-op: an id the engine does not hold (an unknown id, + * or a registry value that never became a layer), and a native Leaflet + * layer — MMGIS still builds `data`, `image`, `video` and `velocity` + * layers with Leaflet under the deck.gl engine, and such a layer carries + * no `id` to be found by. Either way `L_.layers.opacity[name]` is written + * by the caller regardless, and layer creation reads it, so the opacity + * is picked up when the layer is next built or re-added. * - * So is a native Leaflet layer, which reaches here because MMGIS still - * builds `data`, `image`, `video` and `velocity` layers with Leaflet under - * the deck.gl engine and hands every registry entry to the active engine. - * It carries no `id`, so it is not found; and if it ever were registered, - * {@link updateLayer} declines a value it cannot clone. - * - * `options.fillOpacity` is accepted to satisfy {@link IMapEngine}, not + * `options.fillOpacity` is accepted to satisfy {@link IMapEngine} but not * applied separately: deck.gl's single `opacity` prop already scales a * layer's stroke and fill together at draw time, so there is no separate - * fill channel to target at this level the way Leaflet's `setStyle` has - * one. The value is subsumed by `opacity` rather than ignored — a caller - * that computed it is not silently overridden, there is just nothing - * further for this adapter to do with it. + * fill channel to target here. */ setLayerOpacity( layer: Layer | string, diff --git a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts index 6d6a46e25..e8416a7f8 100644 --- a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts @@ -636,11 +636,11 @@ export default class LeafletAdapter implements IMapEngine, IMapEn /** * Whether the layer is currently on the map. * - * Both forms ask the map, never the registry. `_layers` stopped being an - * answer on its own once {@link registerLayer} began holding every - * MMGIS-built tile layer whether or not it is on the map — reading it here - * would make `hasLayer(id)` and `hasLayer(layerObject)` disagree, and - * mmgisAPI's `map:hasLayer` exposes this answer publicly. + * Both forms ask the map, never the registry: `_layers` holds every + * MMGIS-built tile layer whether or not it is on the map, so membership + * there does not answer "is it on the map". `hasLayer(id)` and + * `hasLayer(layerObject)` must not disagree, because mmgisAPI's + * `map:hasLayer` exposes this answer publicly. */ hasLayer(layer: any | string): boolean { const leafletLayer = @@ -786,10 +786,10 @@ export default class LeafletAdapter implements IMapEngine, IMapEn /** * A Leaflet refresher mutates the layer in place — the instance is already - * on the map, so there is nothing for this adapter to swap. The narrowed - * `void` return says so at the type level: {@link refreshLayer} discards - * whatever a refresher returns rather than half-reconciling it into the - * registry while the map still shows the old instance. + * on the map, so there is nothing for this adapter to swap. Its `void` + * return type reflects that: {@link refreshLayer} discards whatever a + * refresher returns, because adopting one into the registry without also + * swapping the layer on the map would leave the two disagreeing. */ setLayerRefresher( id: string, diff --git a/src/essence/Basics/Map_/Map_.js b/src/essence/Basics/Map_/Map_.js index efa258d71..c26517651 100644 --- a/src/essence/Basics/Map_/Map_.js +++ b/src/essence/Basics/Map_/Map_.js @@ -1674,18 +1674,15 @@ async function makeTileLayer(layerObj, mapContext = null) { // Map_.engine is always the MAIN map's engine. A non-default ctx // targets a different map with its own registry, so registering // into Map_.engine here would collide with the main map's entry - // under the same uuid. Guard to the main path only; the deckRaster - // classification above and the buildDeckCOGLayer call are not - // similarly guarded today (pre-existing, out of scope here). + // under the same uuid. Guarded to the main path only. if (ctx.default === true) { - // The layer kind supplies how it rebuilds; the engine executes it. - // Registered here because this is where the deckRaster - // classification already happened. + // The layer kind supplies how it rebuilds; the engine executes + // it. Registered here because this is where the deckRaster + // classification happens. // - // A refresher but no registerLayer, the opposite of the - // Leaflet tail below: a deck layer already carries its own id - // and the engine adopts it when the layer is added, so there - // is nothing to register. Only the refresher is missing. + // No registerLayer call here, unlike the Leaflet tail below: + // a deck layer already carries its own id and the engine + // adopts it when added, so only the refresher is missing. Map_.engine.setLayerRefresher( layerObj.name, makeDeckCOGRefresher(layerObj.name, layerObj) @@ -1793,13 +1790,11 @@ async function makeTileLayer(layerObj, mapContext = null) { }) // The engine addresses layers by id; a Leaflet layer MMGIS built itself - // carries none until it is registered. Without this, refreshLayer cannot - // find it and time reload silently stops working. - // Guarded to the main map: Map_.engine is always the MAIN map's engine, so - // registering a layer built for a secondary ctx (its own map/registry) - // would collide with the main map's entry under the same uuid. - // Optional: the deck branch above is entered only `if (Map_.engine && ...)`, - // so this tail is reached precisely when Map_.engine may be missing. + // carries none until registered, so without this refreshLayer cannot find + // it and time reload silently stops working. Guarded to the main map: a + // secondary ctx has its own map/registry, and registering here would + // collide with the main map's entry under the same uuid. The `?.` is + // needed because only the deck branch above assumes Map_.engine is set. if (ctx.default === true) { Map_.engine?.registerLayer( layerObj.name, From 8bd1a8eccd4cc50949f783901a1d63c41d8e9ecb Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Fri, 28 Aug 2026 10:51:03 -0500 Subject: [PATCH 20/22] Explain the marker CSS pass in terms of the code, not the migration --- src/essence/Basics/Layers_/Layers_.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index 5327996e3..40d039979 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -2259,8 +2259,9 @@ const L_ = { } } - // MMGIS marker markup, keyed by layer name and created outside any - // engine — product markup, so it stays caller-side. + // Marker elements carry a class keyed by layer name, assigned when + // the marker is built. No engine holds a handle on them, so their + // opacity is set on the DOM here rather than through an engine call. $(`.leafletMarkerShape_${F_.getSafeName(name)}`).css({ opacity: newOpacity, }) From 47c17572b45014c3df287b13e0421c13805dab01 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Fri, 28 Aug 2026 11:15:36 -0500 Subject: [PATCH 21/22] Warn when the deck engine holds a layer it did not build --- .../MapEngines/Adapters/DeckGLAdapter.ts | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index 611f822a8..75b531f45 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -266,6 +266,29 @@ export class DeckGLAdapter implements IMapEngine { private _maxBounds: BoundsLike | null = null private _layers = new Map() + /** + * Layer ids already reported as not-a-deck-layer, so a slider drag warns + * once rather than on every frame. + */ + private _warnedNonDeckLayers = new Set() + + /** + * Whether the engine holds a real deck.gl layer under `id`. Warns once per + * id when it holds something else — see {@link updateLayer}. + */ + private _holdsDeckLayer(id: string, existing: unknown): boolean { + if (typeof (existing as Layer)?.clone === 'function') return true + if (existing != null && !this._warnedNonDeckLayers.has(id)) { + this._warnedNonDeckLayers.add(id) + console.warn( + `DeckGLAdapter: layer "${id}" is not a deck.gl layer, so it cannot be ` + + `updated. It was built with Leaflet because this engine has no builder ` + + `for its type. The update was skipped.` + ) + } + return false + } + /** Per-layer refresh hooks, keyed by layer id. */ private _refreshers = new Map Layer | void>() private _layerZIndices = new Map() @@ -417,6 +440,7 @@ export class DeckGLAdapter implements IMapEngine { this._overlays.clear() this._layers.clear() + this._warnedNonDeckLayers.clear() this._refreshers.clear() this._layerZIndices.clear() this._eventListeners.clear() @@ -929,18 +953,19 @@ export class DeckGLAdapter implements IMapEngine { * Clone the existing layer with overridden props. deck.gl detects the same * `id` and updates GPU resources incrementally. * - * A held value that is not a clonable deck layer is a no-op. Under the - * deck.gl engine MMGIS still builds `data`, `image`, `video` and - * `velocity` layers as native Leaflet objects — ENGINE_LAYER_SUPPORT has - * no deck builder for them — and callers hand every registry entry to the - * active engine. Such an object has no `clone`, so this declines rather - * than throwing `existing.clone is not a function`. + * A held value that is not a deck.gl layer is declined, with a warning. + * That state is invalid — this engine should only ever hold layers it + * built — but it is reachable today for layer types this engine has no + * builder for, which fall through to being constructed with Leaflet. + * Declining keeps the app running; the warning is there so the underlying + * mis-construction surfaces instead of presenting as an update that + * quietly did nothing. */ updateLayer(layer: Layer | string, options: Partial): Layer { const id = resolveLayerId(layer) const existing = this._layers.get(id) if (!existing) return existing as unknown as Layer - if (typeof existing.clone !== 'function') return existing + if (!this._holdsDeckLayer(id, existing)) return existing const updated = existing.clone({ ...(options.opacity !== undefined ? { opacity: options.opacity } : {}), ...(options.visible !== undefined ? { visible: options.visible } : {}), @@ -1054,7 +1079,7 @@ export class DeckGLAdapter implements IMapEngine { ): void { const id = resolveLayerId(layer) const existing = this._layers.get(id) - if (typeof existing?.clone === 'function') this.updateLayer(id, { opacity }) + if (this._holdsDeckLayer(id, existing)) this.updateLayer(id, { opacity }) } /** From 099e6b8e32a08b2db8e520917f4d81175217b2f5 Mon Sep 17 00:00:00 2001 From: Slesa Adhikari Date: Fri, 28 Aug 2026 11:30:53 -0500 Subject: [PATCH 22/22] Correct registerLayer doc and trim oversized layer-refresh comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit registerLayer's doc claimed both engines leave the map untouched, but deck.gl's registry is its render list, so registering also draws; state the per-engine reality instead. Also compact several JSDoc blocks for setLayerRefresher/setLayerOpacity/updateLayer/registerLayer and the deck COG refresher that restated what the code already shows or repeated the same per-engine explanation across the interface and both adapters — keep it once at the interface and let the implementations point back to it. --- .../Basics/Layers_/deckCOGRefresher.js | 18 +++-- .../MapEngines/Adapters/DeckGLAdapter.ts | 35 +++++----- .../MapEngines/Adapters/LeafletAdapter.ts | 7 +- src/essence/Basics/MapEngines/IMapEngine.ts | 67 ++++++++----------- 4 files changed, 52 insertions(+), 75 deletions(-) diff --git a/src/essence/Basics/Layers_/deckCOGRefresher.js b/src/essence/Basics/Layers_/deckCOGRefresher.js index e0a531deb..ae4e2f862 100644 --- a/src/essence/Basics/Layers_/deckCOGRefresher.js +++ b/src/essence/Basics/Layers_/deckCOGRefresher.js @@ -6,18 +6,16 @@ import L_ from './Layers_' * How a client-side COG layer recomputes itself, for * `IMapEngine.setLayerRefresher`. * - * Lives on the domain side, not in the adapter: it reads mission config and - * the opacity registry, which adapters must not know about. The engine only - * knows it has a function to call. - * - * Everything is re-derived per call — colormap, rescale, opacity and the - * time-substituted file URL — so a colormap change, a rescale change and a - * time change all flow through this one path. The returned instance keeps the - * layer's id, so deck.gl diffs it against the old one and cached tiles survive. + * Lives on the domain side, not the adapter: it reads mission config and the + * opacity registry, which adapters must not know about. Colormap, rescale, + * opacity and the time-substituted file URL are all re-derived per call, so + * one path covers every kind of change. The returned instance keeps the + * layer's id, so deck.gl diffs it against the old one and cached tiles + * survive. * * @param {string} uuid - Layer UUID, also the engine-side layer id. - * @param {object} layerObj - Fallback config, used only when the registry has - * no `L_.layers.data` entry for `uuid` at call time. + * @param {object} layerObj - Fallback config used when the registry has no + * `L_.layers.data` entry for `uuid`. * @returns {(layer: object) => object} A refresher returning the replacement. */ export function makeDeckCOGRefresher(uuid, layerObj) { diff --git a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts index 75b531f45..761be4d70 100644 --- a/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/DeckGLAdapter.ts @@ -950,16 +950,14 @@ export class DeckGLAdapter implements IMapEngine { } /** - * Clone the existing layer with overridden props. deck.gl detects the same - * `id` and updates GPU resources incrementally. + * Clone the existing layer with overridden props. deck.gl detects the + * same `id` and updates GPU resources incrementally. * - * A held value that is not a deck.gl layer is declined, with a warning. - * That state is invalid — this engine should only ever hold layers it - * built — but it is reachable today for layer types this engine has no - * builder for, which fall through to being constructed with Leaflet. - * Declining keeps the app running; the warning is there so the underlying - * mis-construction surfaces instead of presenting as an update that - * quietly did nothing. + * A held value that is not a deck.gl layer is declined, with a warning, + * rather than applied: this engine should only ever hold layers it + * built, but layer types it has no builder for fall through to being + * built with Leaflet instead. The warning exists so that mis-construction + * surfaces, rather than presenting as an update that quietly did nothing. */ updateLayer(layer: Layer | string, options: Partial): Layer { const id = resolveLayerId(layer) @@ -1059,18 +1057,15 @@ export class DeckGLAdapter implements IMapEngine { * the instance the engine holds via {@link updateLayer}, which re-syncs * the render list. * - * Two cases are a no-op: an id the engine does not hold (an unknown id, - * or a registry value that never became a layer), and a native Leaflet - * layer — MMGIS still builds `data`, `image`, `video` and `velocity` - * layers with Leaflet under the deck.gl engine, and such a layer carries - * no `id` to be found by. Either way `L_.layers.opacity[name]` is written - * by the caller regardless, and layer creation reads it, so the opacity - * is picked up when the layer is next built or re-added. + * A no-op when the engine doesn't hold `id`, or holds a native Leaflet + * layer (MMGIS still builds `data`, `image`, `video` and `velocity` + * layers with Leaflet under this engine, and those carry no `id` to be + * found by). Either way the caller has already written + * `L_.layers.opacity[name]`, which layer creation reads, so the opacity + * is picked up next time the layer is built or re-added. * - * `options.fillOpacity` is accepted to satisfy {@link IMapEngine} but not - * applied separately: deck.gl's single `opacity` prop already scales a - * layer's stroke and fill together at draw time, so there is no separate - * fill channel to target here. + * `options.fillOpacity` is accepted but not applied separately — see + * {@link IMapEngine.setLayerOpacity}. */ setLayerOpacity( layer: Layer | string, diff --git a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts index e8416a7f8..ea898e8b6 100644 --- a/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts +++ b/src/essence/Basics/MapEngines/Adapters/LeafletAdapter.ts @@ -785,11 +785,8 @@ export default class LeafletAdapter implements IMapEngine, IMapEn } /** - * A Leaflet refresher mutates the layer in place — the instance is already - * on the map, so there is nothing for this adapter to swap. Its `void` - * return type reflects that: {@link refreshLayer} discards whatever a - * refresher returns, because adopting one into the registry without also - * swapping the layer on the map would leave the two disagreeing. + * Mutates the layer in place; any return value is ignored. See + * {@link IMapEngine.setLayerRefresher}. */ setLayerRefresher( id: string, diff --git a/src/essence/Basics/MapEngines/IMapEngine.ts b/src/essence/Basics/MapEngines/IMapEngine.ts index 3433eba93..9802579e6 100644 --- a/src/essence/Basics/MapEngines/IMapEngine.ts +++ b/src/essence/Basics/MapEngines/IMapEngine.ts @@ -202,37 +202,30 @@ export interface IMapEngine< /** * Take ownership of an externally-built native layer under `id`, so - * id-addressed methods can find it. Does not change what is on the map — - * `addLayer` still does that. + * id-addressed methods can find it. * - * Leaflet needs this because MMGIS builds its tile layers itself and hands - * them to `addLayer` as native objects, which carry no id. deck.gl layers - * already carry `id`, but its implementation still keys its registry by - * this caller-supplied `id` rather than `layer.id`, so the two can never - * drift apart. + * Leaflet holds the layer without changing what is drawn — `addLayer` + * still does that separately. deck.gl has no such split: holding a layer + * *is* drawing it, since `_layers` is simultaneously its registry and its + * render list. That's why callers that want a layer held but not yet + * shown may only rely on this method on the Leaflet path. */ registerLayer(id: string, layer: TLayer): void /** * Register how one layer recomputes itself, or pass null to clear. * - * Called by the module that owns the layer kind, at creation — never by an - * adapter, which stays layer-type-agnostic. The engine invokes it with the - * live instance and remains its owner, so the function must not retain it. + * Called by the module that owns the layer kind, at creation — never by + * an adapter, which stays layer-type-agnostic. The engine invokes it with + * the live instance and remains its owner; the function must not retain + * it. * - * What a refresher does with that instance differs per engine, because the - * two engines' layers do: - * - **deck.gl** — layers are immutable, so a refresher returns a - * replacement and the engine adopts it. Returning nothing means "nothing - * to apply" and the engine keeps what it holds. - * - **Leaflet** — layers are mutable and already on the map, so a - * refresher mutates in place. Any value it returns is IGNORED: adopting - * one into the registry without also swapping the layer on the map would - * leave the two disagreeing, so the adapter does not try. - * - * The signature keeps `TLayer | void` because this interface is generic - * over whichever engine is in play; the Leaflet adapter narrows its own - * parameter to a void-returning function. + * deck.gl layers are immutable, so a refresher returns a replacement and + * the engine adopts it (returning nothing keeps what's held). Leaflet + * layers are mutable and already on the map, so a refresher mutates in + * place and any return value is ignored. The signature stays + * `TLayer | void` for that reason; the Leaflet adapter narrows its own + * parameter to void. */ setLayerRefresher( id: string, @@ -266,25 +259,19 @@ export interface IMapEngine< bringToBack(layer: TLayer | string): void /** - * Set a layer's opacity. - * - * Both engines return nothing: the engine owns the instance and performs - * whatever the change requires — mutating it (Leaflet) or replacing the one - * it holds (deck.gl). Callers never adopt a replacement. + * Set a layer's opacity. Both engines return nothing — each owns its + * instance and applies the change internally: Leaflet mutates in place, + * deck.gl replaces the instance it holds. Callers never adopt a + * replacement. * - * The two engines do not honour `fillOpacity` uniformly — this is a real - * per-engine difference, not an oversight: - * - Leaflet applies it to the fill of layers that paint one separately - * from their stroke (`setStyle`'s `fillOpacity`). - * - deck.gl has no separate fill channel at this level: its single - * `opacity` prop scales stroke and fill together at draw time, so the - * value is accepted (to satisfy this signature) and subsumed by - * `opacity` rather than applied on its own. + * `fillOpacity` is not honoured uniformly, deliberately: Leaflet applies + * it to the fill of layers that paint one separately from their stroke + * (`setStyle`'s `fillOpacity`). deck.gl has no separate fill channel — + * its single `opacity` prop scales stroke and fill together, so the value + * is accepted here to satisfy the signature but subsumed into `opacity`. * - * @param options.fillOpacity - The fill opacity to apply to layers that - * paint one separately from their stroke. Scaling policy belongs to the - * caller, so this is an absolute value, never a factor the adapter - * multiplies. Defaults to `opacity`. See per-engine note above. + * @param options.fillOpacity - Absolute fill opacity, not a multiplier. + * Defaults to `opacity`. See per-engine note above. */ setLayerOpacity( layer: TLayer | string,