From 5b51b02fa72b3af17eb4977da9f5c26d4c166f1f Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 28 Apr 2026 16:46:16 +0100 Subject: [PATCH 1/6] IM-245 removed unused exports from dataset patternImages --- plugins/beta/datasets/src/adapters/maplibre/patternImages.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/beta/datasets/src/adapters/maplibre/patternImages.js b/plugins/beta/datasets/src/adapters/maplibre/patternImages.js index 4948c0bb..abe9155f 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/patternImages.js +++ b/plugins/beta/datasets/src/adapters/maplibre/patternImages.js @@ -2,7 +2,7 @@ import { hasPattern } from '../../../../../../src/utils/patternUtils.js' import { mergeSublayer } from '../../utils/mergeSublayer.js' -export { hasPattern, getPatternInnerContent, getPatternImageId, getKeyPatternPaths, injectColors } from '../../../../../../src/utils/patternUtils.js' +export { hasPattern, getPatternImageId, getKeyPatternPaths } from '../../../../../../src/utils/patternUtils.js' /** * Returns a flat list of datasets and merged sublayers that require pattern images. From 028d745b385c73e7a81414098a03f9869b0d20e5 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 29 Apr 2026 08:24:08 +0100 Subject: [PATCH 2/6] IM-245 moved getPatternInnerContent to patternRegistry --- providers/maplibre/src/utils/patternImages.js | 4 ++-- src/services/patternRegistry.js | 18 +++++++++++++++ src/utils/patternUtils.js | 22 ++----------------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/providers/maplibre/src/utils/patternImages.js b/providers/maplibre/src/utils/patternImages.js index 490224e2..162e27ca 100644 --- a/providers/maplibre/src/utils/patternImages.js +++ b/providers/maplibre/src/utils/patternImages.js @@ -1,4 +1,4 @@ -import { getPatternInnerContent, getPatternImageId, injectColors, PATTERN_MIN_PIXEL_RATIO } from '../../../../src/utils/patternUtils.js' +import { getPatternImageId, injectColors, PATTERN_MIN_PIXEL_RATIO } from '../../../../src/utils/patternUtils.js' import { getValueForStyle } from '../../../../src/utils/getValueForStyle.js' import { rasteriseToImageData } from './rasteriseToImageData.js' @@ -16,7 +16,7 @@ const imageDataCache = new Map() * @returns {Promise<{ imageId: string, imageData: ImageData }|null>} */ const rasterisePattern = async (dataset, mapStyleId, patternRegistry, pixelRatio) => { - const innerContent = getPatternInnerContent(dataset, patternRegistry) + const innerContent = patternRegistry.getPatternInnerContent(dataset) if (!innerContent) { return null } diff --git a/src/services/patternRegistry.js b/src/services/patternRegistry.js index 4ffc05db..1d260de3 100644 --- a/src/services/patternRegistry.js +++ b/src/services/patternRegistry.js @@ -31,7 +31,25 @@ export const patternRegistry = { */ list () { return [...patterns.values()] + }, + + /** + * Returns the raw (un-coloured) inner SVG content for a style's pattern. + * Precedence: inline fillPatternSvgContent → named fillPattern from registry. + * + * @param {Object} style + * @returns {string|null} + */ + getPatternInnerContent (style) { + if (style.fillPatternSvgContent) { + return style.fillPatternSvgContent + } + if (style.fillPattern) { + return this.get(style.fillPattern)?.svgContent ?? null + } + return null } + } // Seed built-in patterns diff --git a/src/utils/patternUtils.js b/src/utils/patternUtils.js index 31a758be..4e18111d 100644 --- a/src/utils/patternUtils.js +++ b/src/utils/patternUtils.js @@ -33,24 +33,6 @@ export const injectColors = (content, foregroundColor, backgroundColor) => */ export const hasPattern = (dataset) => !!(dataset.fillPattern || dataset.fillPatternSvgContent) -/** - * Returns the raw (un-coloured) inner SVG content for a dataset's pattern. - * Precedence: inline fillPatternSvgContent → named fillPattern from registry. - * - * @param {Object} dataset - * @param {Object} patternRegistry - * @returns {string|null} - */ -export const getPatternInnerContent = (dataset, patternRegistry) => { - if (dataset.fillPatternSvgContent) { - return dataset.fillPatternSvgContent - } - if (dataset.fillPattern) { - return patternRegistry?.get(dataset.fillPattern)?.svgContent ?? null - } - return null -} - /** * Returns a deterministic image ID for a pattern + resolved colour + pixel ratio combination. * @@ -64,7 +46,7 @@ export const getPatternInnerContent = (dataset, patternRegistry) => { export const PATTERN_MIN_PIXEL_RATIO = 2 export const getPatternImageId = (dataset, mapStyleId, patternRegistry, pixelRatio = 1) => { - const innerContent = getPatternInnerContent(dataset, patternRegistry) + const innerContent = patternRegistry.getPatternInnerContent(dataset) if (!innerContent) { return null } @@ -85,7 +67,7 @@ export const getPatternImageId = (dataset, mapStyleId, patternRegistry, pixelRat * @returns {{ border: string, content: string }|null} */ export const getKeyPatternPaths = (dataset, mapStyleId, patternRegistry) => { - const innerContent = getPatternInnerContent(dataset, patternRegistry) + const innerContent = patternRegistry.getPatternInnerContent(dataset) if (!innerContent) { return null } From 354e43ba8220c4e358270dc7a4baa3e91cc08f6e Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 29 Apr 2026 09:06:25 +0100 Subject: [PATCH 3/6] IM-245 moved more methods to patternRegistry --- .../src/adapters/maplibre/layerBuilders.js | 4 +- .../adapters/maplibre/maplibreLayerAdapter.js | 6 +-- .../src/adapters/maplibre/patternImages.js | 2 +- .../datasets/src/components/KeySvgPattern.jsx | 3 +- providers/maplibre/src/utils/patternImages.js | 6 +-- src/services/patternRegistry.js | 46 ++++++++++++++++- src/utils/patternUtils.js | 51 +------------------ 7 files changed, 57 insertions(+), 61 deletions(-) diff --git a/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js index bdd99ed6..71efa378 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js +++ b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js @@ -1,5 +1,5 @@ import { getValueForStyle } from '../../../../../../src/utils/getValueForStyle.js' -import { hasPattern, getPatternImageId } from './patternImages.js' +import { hasPattern } from './patternImages.js' import { mergeSublayer } from '../../utils/mergeSublayer.js' import { getSourceId, getLayerIds, getSublayerLayerIds, isDynamicSource, MAX_TILE_ZOOM } from './layerIds.js' import { hasSymbol, getSymbolDef, getSymbolAnchor, anchorToMaplibre, getSymbolImageId } from './symbolImages.js' @@ -36,7 +36,7 @@ export const addFillLayer = (map, config, layerId, sourceId, sourceLayer, visibi if (!config.fill && !hasPattern(config)) { return } - const patternImageId = hasPattern(config) ? getPatternImageId(config, mapStyleId, patternRegistry, pixelRatio) : null + const patternImageId = hasPattern(config) ? patternRegistry.getPatternImageId(config, mapStyleId, pixelRatio) : null const paint = patternImageId ? { 'fill-pattern': patternImageId, 'fill-opacity': config.opacity || 1 } : { 'fill-color': getValueForStyle(config.fill, mapStyleId), 'fill-opacity': config.opacity || 1 } diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index dd1e4f8e..6e36acbb 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -1,7 +1,7 @@ import { applyExclusionFilter } from '../../utils/filters.js' import { getSourceId, getLayerIds, getSublayerLayerIds, getAllLayerIds } from './layerIds.js' import { addDatasetLayers, addSublayerLayers } from './layerBuilders.js' -import { getPatternConfigs, hasPattern, getPatternImageId } from './patternImages.js' +import { getPatternConfigs, hasPattern } from './patternImages.js' import { getSymbolConfigs, getSymbolImageId } from './symbolImages.js' import { mergeSublayer } from '../../utils/mergeSublayer.js' import { scaleFactor } from '../../../../../../src/config/appConfig.js' @@ -135,7 +135,7 @@ export default class MaplibreLayerAdapter { if (hasPattern(dataset)) { const { fillLayerId } = getLayerIds(dataset) if (this._map.getLayer(fillLayerId)) { - const imageId = getPatternImageId(dataset, mapStyle.id, this._patternRegistry, pixelRatio) + const imageId = this._patternRegistry.getPatternImageId(dataset, mapStyle.id, pixelRatio) if (imageId) { this._map.setPaintProperty(fillLayerId, 'fill-pattern', imageId) } @@ -151,7 +151,7 @@ export default class MaplibreLayerAdapter { } } if (hasPattern(merged) && this._map.getLayer(fillLayerId)) { - const imageId = getPatternImageId(merged, mapStyle.id, this._patternRegistry, pixelRatio) + const imageId = this._patternRegistry.getPatternImageId(merged, mapStyle.id, pixelRatio) if (imageId) { this._map.setPaintProperty(fillLayerId, 'fill-pattern', imageId) } diff --git a/plugins/beta/datasets/src/adapters/maplibre/patternImages.js b/plugins/beta/datasets/src/adapters/maplibre/patternImages.js index abe9155f..cdb47034 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/patternImages.js +++ b/plugins/beta/datasets/src/adapters/maplibre/patternImages.js @@ -2,7 +2,7 @@ import { hasPattern } from '../../../../../../src/utils/patternUtils.js' import { mergeSublayer } from '../../utils/mergeSublayer.js' -export { hasPattern, getPatternImageId, getKeyPatternPaths } from '../../../../../../src/utils/patternUtils.js' +export { hasPattern } from '../../../../../../src/utils/patternUtils.js' /** * Returns a flat list of datasets and merged sublayers that require pattern images. diff --git a/plugins/beta/datasets/src/components/KeySvgPattern.jsx b/plugins/beta/datasets/src/components/KeySvgPattern.jsx index f985ba74..9fdb4d93 100644 --- a/plugins/beta/datasets/src/components/KeySvgPattern.jsx +++ b/plugins/beta/datasets/src/components/KeySvgPattern.jsx @@ -1,11 +1,10 @@ -import { getKeyPatternPaths } from '../../../../../src/utils/patternUtils.js' import { svgProps } from './svgProperties.js' const PATTERN_INSET = 2 export const KeySvgPattern = (props) => { const { patternRegistry, mapStyle } = props - const paths = getKeyPatternPaths(props, mapStyle.id, patternRegistry) + const paths = patternRegistry.getKeyPatternPaths(props, mapStyle.id) return ( diff --git a/providers/maplibre/src/utils/patternImages.js b/providers/maplibre/src/utils/patternImages.js index 162e27ca..686dfbb7 100644 --- a/providers/maplibre/src/utils/patternImages.js +++ b/providers/maplibre/src/utils/patternImages.js @@ -1,4 +1,4 @@ -import { getPatternImageId, injectColors, PATTERN_MIN_PIXEL_RATIO } from '../../../../src/utils/patternUtils.js' +import { injectColors, PATTERN_MIN_PIXEL_RATIO } from '../../../../src/utils/patternUtils.js' import { getValueForStyle } from '../../../../src/utils/getValueForStyle.js' import { rasteriseToImageData } from './rasteriseToImageData.js' @@ -21,7 +21,7 @@ const rasterisePattern = async (dataset, mapStyleId, patternRegistry, pixelRatio return null } - const imageId = getPatternImageId(dataset, mapStyleId, patternRegistry, pixelRatio) + const imageId = patternRegistry.getPatternImageId(dataset, mapStyleId, pixelRatio) if (!imageId) { return null } @@ -64,7 +64,7 @@ export const addPatternsToMap = async (map, styleArray, mapStyleId, patternRegis const effectiveRatio = Math.max(PATTERN_MIN_PIXEL_RATIO, pixelRatio) await Promise.all(styleArray.map(async (config) => { - const imageId = getPatternImageId(config, mapStyleId, patternRegistry, pixelRatio) + const imageId = patternRegistry.getPatternImageId(config, mapStyleId, pixelRatio) if (!imageId || map.hasImage(imageId)) { return } diff --git a/src/services/patternRegistry.js b/src/services/patternRegistry.js index 1d260de3..117e3080 100644 --- a/src/services/patternRegistry.js +++ b/src/services/patternRegistry.js @@ -1,5 +1,6 @@ import { BUILT_IN_PATTERNS } from '../config/patternConfig.js' - +import { getValueForStyle } from '../utils/getValueForStyle.js' +import { KEY_BORDER_PATH, PATTERN_MIN_PIXEL_RATIO, injectColors, hashString } from '../utils/patternUtils.js' const patterns = new Map() export const patternRegistry = { @@ -48,6 +49,49 @@ export const patternRegistry = { return this.get(style.fillPattern)?.svgContent ?? null } return null + }, + + /** + * Returns colour-injected SVG path content for use in Key panel pattern symbols. + * Returns { border, content } where border is the rounded-rect outline and content + * is the pattern fill. Returns null if the style has no pattern. + * + * @param {Object} style + * @param {string} mapStyleId + * @returns {{ border: string, content: string }|null} + */ + getKeyPatternPaths (style, mapStyleId) { + const innerContent = this.getPatternInnerContent(style) + if (!innerContent) { + return null + } + const fg = getValueForStyle(style.fillPatternForegroundColor, mapStyleId) || 'black' + const bg = getValueForStyle(style.fillPatternBackgroundColor, mapStyleId) || 'transparent' + const borderStroke = getValueForStyle(style.stroke, mapStyleId) || fg + const keyPatternPaths = { + border: injectColors(KEY_BORDER_PATH, borderStroke, bg), + content: injectColors(innerContent, fg, bg) + } + return keyPatternPaths + }, + + /** + * Returns a deterministic image ID for a pattern + resolved colour + pixel ratio combination. + * + * @param {Object} dataset + * @param {string} mapStyleId + * @param {number} [pixelRatio=1] + * @returns {string|null} + */ + getPatternImageId (dataset, mapStyleId, pixelRatio = 1) { + const innerContent = this.getPatternInnerContent(dataset) + if (!innerContent) { + return null + } + const fg = getValueForStyle(dataset.fillPatternForegroundColor, mapStyleId) || 'black' + const bg = getValueForStyle(dataset.fillPatternBackgroundColor, mapStyleId) || 'transparent' + const effectiveRatio = Math.max(PATTERN_MIN_PIXEL_RATIO, pixelRatio) + return `pattern-${hashString(innerContent + fg + bg)}-${effectiveRatio}x` } } diff --git a/src/utils/patternUtils.js b/src/utils/patternUtils.js index 4e18111d..055ca1c4 100644 --- a/src/utils/patternUtils.js +++ b/src/utils/patternUtils.js @@ -1,7 +1,7 @@ -import { getValueForStyle } from '../utils/getValueForStyle.js' - // Border path rendered behind the pattern content in Key panel symbols (20×20 coordinate space). export const KEY_BORDER_PATH = '' +// Minimum oversampling — keeps 16×16 physical pixels as the floor so patterns remain crisp. +export const PATTERN_MIN_PIXEL_RATIO = 2 export const hashString = (str) => { let hash = 0 @@ -32,50 +32,3 @@ export const injectColors = (content, foregroundColor, backgroundColor) => * @returns {boolean} */ export const hasPattern = (dataset) => !!(dataset.fillPattern || dataset.fillPatternSvgContent) - -/** - * Returns a deterministic image ID for a pattern + resolved colour + pixel ratio combination. - * - * @param {Object} dataset - * @param {string} mapStyleId - * @param {Object} patternRegistry - * @param {number} [pixelRatio=1] - * @returns {string|null} - */ -// Minimum oversampling — keeps 16×16 physical pixels as the floor so patterns remain crisp. -export const PATTERN_MIN_PIXEL_RATIO = 2 - -export const getPatternImageId = (dataset, mapStyleId, patternRegistry, pixelRatio = 1) => { - const innerContent = patternRegistry.getPatternInnerContent(dataset) - if (!innerContent) { - return null - } - const fg = getValueForStyle(dataset.fillPatternForegroundColor, mapStyleId) || 'black' - const bg = getValueForStyle(dataset.fillPatternBackgroundColor, mapStyleId) || 'transparent' - const effectiveRatio = Math.max(PATTERN_MIN_PIXEL_RATIO, pixelRatio) - return `pattern-${hashString(innerContent + fg + bg)}-${effectiveRatio}x` -} - -/** - * Returns colour-injected SVG path content for use in Key panel pattern symbols. - * Returns { border, content } where border is the rounded-rect outline and content - * is the pattern fill. Returns null if the dataset has no pattern. - * - * @param {Object} dataset - * @param {string} mapStyleId - * @param {Object} patternRegistry - * @returns {{ border: string, content: string }|null} - */ -export const getKeyPatternPaths = (dataset, mapStyleId, patternRegistry) => { - const innerContent = patternRegistry.getPatternInnerContent(dataset) - if (!innerContent) { - return null - } - const fg = getValueForStyle(dataset.fillPatternForegroundColor, mapStyleId) || 'black' - const bg = getValueForStyle(dataset.fillPatternBackgroundColor, mapStyleId) || 'transparent' - const borderStroke = getValueForStyle(dataset.stroke, mapStyleId) || fg - return { - border: injectColors(KEY_BORDER_PATH, borderStroke, bg), - content: injectColors(innerContent, fg, bg) - } -} From e36504e67176f5f7058f420ffc1ceed252e45d7f Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 29 Apr 2026 14:13:08 +0100 Subject: [PATCH 4/6] IM-245 fixed patternImages.test.js --- .../maplibre/src/utils/patternImages.test.js | 308 ++++++++---------- src/services/patternRegistry.js | 8 +- 2 files changed, 146 insertions(+), 170 deletions(-) diff --git a/providers/maplibre/src/utils/patternImages.test.js b/providers/maplibre/src/utils/patternImages.test.js index d816272a..82f28bfe 100644 --- a/providers/maplibre/src/utils/patternImages.test.js +++ b/providers/maplibre/src/utils/patternImages.test.js @@ -1,4 +1,5 @@ import { addPatternsToMap } from './patternImages.js' +import { patternRegistry } from '../../../../src/services/patternRegistry.js' const OUTDOOR = 'outdoor' @@ -30,182 +31,151 @@ const makeMap = (existingIds = []) => ({ addImage: jest.fn() }) -const makePatternRegistry = (id = 'stripes', content = SVG_CONTENT) => ({ - get: jest.fn((name) => name === id ? { svgContent: content } : undefined) -}) - // ─── addPatternsToMap ───────────────────────────────────────────────────────── - -describe('addPatternsToMap — registration', () => { - it('returns early and does not touch map for empty configs', async () => { - const map = makeMap() - const registry = makePatternRegistry() - await addPatternsToMap(map, [], OUTDOOR, registry) - expect(map.hasImage).not.toHaveBeenCalled() - expect(map.addImage).not.toHaveBeenCalled() - }) - - it('calls addImage for an inline fillPatternSvgContent config', async () => { - const map = makeMap() - const registry = makePatternRegistry() - const config = { fillPatternSvgContent: SVG_CONTENT } - await addPatternsToMap(map, [config], OUTDOOR, registry) - expect(map.addImage).toHaveBeenCalledTimes(1) +describe('addPatternsToMap', () => { + beforeEach(() => { + // Clear the registry and re-register a known pattern to ensure consistent test conditions + patternRegistry.clear() + patternRegistry.register('stripes', SVG_CONTENT) }) - - it('skips addImage when image is already registered', async () => { - const registry = makePatternRegistry() - const config = { fillPattern: 'stripes' } - const pixelRatio = 2 - const { getPatternImageId } = await import('../../../../src/utils/patternUtils.js') - const existingId = getPatternImageId(config, OUTDOOR, registry, pixelRatio) - const map = makeMap([existingId]) - await addPatternsToMap(map, [config], OUTDOOR, registry, pixelRatio) - expect(map.addImage).not.toHaveBeenCalled() + describe('addPatternsToMap — registration', () => { + it('returns early and does not touch map for empty configs', async () => { + const map = makeMap() + await addPatternsToMap(map, [], OUTDOOR, patternRegistry) + expect(map.hasImage).not.toHaveBeenCalled() + expect(map.addImage).not.toHaveBeenCalled() + }) + + it('calls addImage for an inline fillPatternSvgContent config', async () => { + const map = makeMap() + const config = { fillPatternSvgContent: SVG_CONTENT } + await addPatternsToMap(map, [config], OUTDOOR, patternRegistry) + expect(map.addImage).toHaveBeenCalledTimes(1) + }) + + it('skips addImage when image is already registered', async () => { + const style = { fillPattern: 'stripes' } + const pixelRatio = 2 + const map = makeMap(['pattern-mpxwil-2x']) + await addPatternsToMap(map, [style], OUTDOOR, patternRegistry, pixelRatio) + expect(map.addImage).not.toHaveBeenCalled() + }) + + it('skips config when pattern has no inner content', async () => { + const map = makeMap() + patternRegistry.clear() + await addPatternsToMap(map, [{ fillPattern: 'unknown' }], OUTDOOR, patternRegistry) + expect(map.addImage).not.toHaveBeenCalled() + }) + + it('skips config when neither fillPattern nor fillPatternSvgContent is set', async () => { + const map = makeMap() + await addPatternsToMap(map, [{ fillColor: '#ff0000' }], OUTDOOR, patternRegistry) + expect(map.addImage).not.toHaveBeenCalled() + }) + + it('processes multiple configs in parallel', async () => { + const map = makeMap() + patternRegistry.register('dots', SVG_CONTENT) + await addPatternsToMap(map, [{ fillPattern: 'stripes' }, { fillPattern: 'dots' }], OUTDOOR, patternRegistry) + expect(map.addImage).toHaveBeenCalledTimes(2) + }) }) - it('skips config when pattern has no inner content', async () => { - const map = makeMap() - const emptyRegistry = { get: jest.fn(() => undefined) } - await addPatternsToMap(map, [{ fillPattern: 'unknown' }], OUTDOOR, emptyRegistry) - expect(map.addImage).not.toHaveBeenCalled() + describe('addPatternsToMap — pixel ratio', () => { + it('encodes effectiveRatio in the image ID and passes it to addImage', async () => { + const map = makeMap() + const config = { fillPattern: 'stripes' } + await addPatternsToMap(map, [config], OUTDOOR, patternRegistry, 2) + expect(map.addImage).toHaveBeenCalledTimes(1) + expect(map.addImage).toHaveBeenCalledWith( + expect.stringMatching(/^pattern-[a-z0-9]+-2x$/), + expect.any(Object), + { pixelRatio: 2 } + ) + }) + + it('floors effectiveRatio at 2 so low-DPI patterns stay crisp', async () => { + const map = makeMap() + const config = { fillPattern: 'stripes' } + await addPatternsToMap(map, [config], OUTDOOR, patternRegistry, 1) + expect(map.addImage).toHaveBeenCalledWith( + expect.stringMatching(/-2x$/), + expect.any(Object), + { pixelRatio: 2 } + ) + }) + + it('produces different image IDs for ratios above the floor', async () => { + const map1 = makeMap() + const map2 = makeMap() + const config = { fillPattern: 'stripes' } + const hiDpi = 3 + await addPatternsToMap(map1, [config], OUTDOOR, patternRegistry, 2) + await addPatternsToMap(map2, [config], OUTDOOR, patternRegistry, hiDpi) + const [id2x] = map1.addImage.mock.calls[0] + const [id3x] = map2.addImage.mock.calls[0] + expect(id2x).not.toBe(id3x) + expect(id2x).toMatch(/-2x$/) + expect(id3x).toMatch(/-3x$/) + }) }) - it('skips config when neither fillPattern nor fillPatternSvgContent is set', async () => { - const map = makeMap() - const registry = makePatternRegistry() - await addPatternsToMap(map, [{ fillColor: '#ff0000' }], OUTDOOR, registry) - expect(map.addImage).not.toHaveBeenCalled() + describe('addPatternsToMap — color resolution and caching', () => { + it('applies foreground and background colors when resolving the SVG', async () => { + const map = makeMap() + const getContextSpy = HTMLCanvasElement.prototype.getContext + await addPatternsToMap( + map, + [{ fillPattern: 'stripes', fillPatternForegroundColor: '#aabbcc', fillPatternBackgroundColor: '#112233' }], + OUTDOOR, + patternRegistry + ) + expect(map.addImage).toHaveBeenCalledTimes(1) + expect(getContextSpy).toHaveBeenCalled() + }) + + it('resolves style-keyed foreground color for the given mapStyleId', async () => { + const map = makeMap() + const config = { + fillPattern: 'stripes', + fillPatternForegroundColor: { outdoor: '#aabbcc', dark: '#112233' } + } + await addPatternsToMap(map, [config], OUTDOOR, patternRegistry) + const map2 = makeMap() + await addPatternsToMap(map2, [config], 'dark', patternRegistry) + const [idOutdoor] = map.addImage.mock.calls[0] + const [idDark] = map2.addImage.mock.calls[0] + expect(idOutdoor).not.toBe(idDark) + }) + + it('uses cached ImageData on second call with identical config', async () => { + const map = makeMap() + const config = { fillPattern: 'stripes', fillPatternForegroundColor: '#unique2' } + const pixelRatio = 2 + await addPatternsToMap(map, [config], OUTDOOR, patternRegistry, pixelRatio) + const imageId = patternRegistry.getPatternImageId(config, OUTDOOR, pixelRatio) + const map2 = makeMap([imageId]) + await addPatternsToMap(map2, [config], OUTDOOR, patternRegistry, pixelRatio) + expect(map2.addImage).not.toHaveBeenCalled() + }) }) - it('processes multiple configs in parallel', async () => { - const map = makeMap() - const registry = { - get: jest.fn((name) => { - if (name === 'stripes') { return { svgContent: '' } } - if (name === 'dots') { return { svgContent: '' } } - return undefined - }) - } - await addPatternsToMap(map, [{ fillPattern: 'stripes' }, { fillPattern: 'dots' }], OUTDOOR, registry) - expect(map.addImage).toHaveBeenCalledTimes(2) - }) -}) - -describe('addPatternsToMap — pixel ratio', () => { - it('encodes effectiveRatio in the image ID and passes it to addImage', async () => { - const map = makeMap() - const registry = makePatternRegistry() - const config = { fillPattern: 'stripes' } - await addPatternsToMap(map, [config], OUTDOOR, registry, 2) - expect(map.addImage).toHaveBeenCalledTimes(1) - expect(map.addImage).toHaveBeenCalledWith( - expect.stringMatching(/^pattern-[a-z0-9]+-2x$/), - expect.any(Object), - { pixelRatio: 2 } - ) - }) - - it('floors effectiveRatio at 2 so low-DPI patterns stay crisp', async () => { - const map = makeMap() - const registry = makePatternRegistry() - const config = { fillPattern: 'stripes' } - await addPatternsToMap(map, [config], OUTDOOR, registry, 1) - expect(map.addImage).toHaveBeenCalledWith( - expect.stringMatching(/-2x$/), - expect.any(Object), - { pixelRatio: 2 } - ) - }) - - it('produces different image IDs for ratios above the floor', async () => { - const map1 = makeMap() - const map2 = makeMap() - const registry = makePatternRegistry() - const config = { fillPattern: 'stripes' } - const hiDpi = 3 - await addPatternsToMap(map1, [config], OUTDOOR, registry, 2) - await addPatternsToMap(map2, [config], OUTDOOR, registry, hiDpi) - const [id2x] = map1.addImage.mock.calls[0] - const [id3x] = map2.addImage.mock.calls[0] - expect(id2x).not.toBe(id3x) - expect(id2x).toMatch(/-2x$/) - expect(id3x).toMatch(/-3x$/) - }) -}) - -describe('addPatternsToMap — color resolution and caching', () => { - it('applies foreground and background colors when resolving the SVG', async () => { - const map = makeMap() - const registry = makePatternRegistry() - const getContextSpy = HTMLCanvasElement.prototype.getContext - await addPatternsToMap( - map, - [{ fillPattern: 'stripes', fillPatternForegroundColor: '#aabbcc', fillPatternBackgroundColor: '#112233' }], - OUTDOOR, - registry - ) - expect(map.addImage).toHaveBeenCalledTimes(1) - expect(getContextSpy).toHaveBeenCalled() - }) - - it('resolves style-keyed foreground color for the given mapStyleId', async () => { - const map = makeMap() - const registry = makePatternRegistry() - const config = { - fillPattern: 'stripes', - fillPatternForegroundColor: { outdoor: '#aabbcc', dark: '#112233' } - } - await addPatternsToMap(map, [config], OUTDOOR, registry) - const map2 = makeMap() - await addPatternsToMap(map2, [config], 'dark', registry) - const [idOutdoor] = map.addImage.mock.calls[0] - const [idDark] = map2.addImage.mock.calls[0] - expect(idOutdoor).not.toBe(idDark) - }) - - it('uses cached ImageData on second call with identical config', async () => { - const map = makeMap() - const registry = makePatternRegistry() - const config = { fillPattern: 'stripes', fillPatternForegroundColor: '#unique2' } - const pixelRatio = 2 - await addPatternsToMap(map, [config], OUTDOOR, registry, pixelRatio) - const { getPatternImageId } = await import('../../../../src/utils/patternUtils.js') - const imageId = getPatternImageId(config, OUTDOOR, registry, pixelRatio) - const map2 = makeMap([imageId]) - await addPatternsToMap(map2, [config], OUTDOOR, registry, pixelRatio) - expect(map2.addImage).not.toHaveBeenCalled() - }) -}) - -describe('addPatternsToMap — null results', () => { - it('does not call addImage when innerContent becomes unavailable inside rasterisePattern', async () => { - // registry.get returns content on the first call (for getPatternImageId in addPatternsToMap) - // but undefined on the second call (for getPatternInnerContent inside rasterisePattern) - const registry = { - get: jest.fn() - .mockReturnValueOnce({ svgContent: SVG_CONTENT }) - .mockReturnValueOnce(undefined) - } - const map = makeMap() - await addPatternsToMap(map, [{ fillPattern: 'stripes' }], OUTDOOR, registry) - expect(map.addImage).not.toHaveBeenCalled() - }) - - it('does not call addImage when imageId becomes unavailable inside rasterisePattern', async () => { - // Three consecutive calls to registry.get: - // 1. getPatternImageId in addPatternsToMap → returns content → imageId is truthy - // 2. getPatternInnerContent directly in rasterisePattern → returns content → passes innerContent guard - // 3. getPatternInnerContent inside getPatternImageId in rasterisePattern → returns undefined - // → getPatternImageId returns null → hits the imageId null guard - const registry = { - get: jest.fn() - .mockReturnValueOnce({ svgContent: SVG_CONTENT }) - .mockReturnValueOnce({ svgContent: SVG_CONTENT }) - .mockReturnValueOnce(undefined) - } - const map = makeMap() - await addPatternsToMap(map, [{ fillPattern: 'stripes' }], OUTDOOR, registry) - expect(map.addImage).not.toHaveBeenCalled() + describe('addPatternsToMap — null results', () => { + it('does not call addImage when innerContent becomes unavailable inside rasterisePattern', async () => { + const getPatternInnerContent = jest.spyOn(patternRegistry, 'getPatternInnerContent').mockReturnValueOnce(undefined) + const map = makeMap() + await addPatternsToMap(map, [{ fillPattern: 'stripes' }], OUTDOOR, patternRegistry) + expect(map.addImage).not.toHaveBeenCalled() + getPatternInnerContent.mockRestore() + }) + + it('does not call addImage when imageId becomes unavailable inside rasterisePattern', async () => { + const getPatternImageId = jest.spyOn(patternRegistry, 'getPatternImageId').mockReturnValue(undefined) + const map = makeMap() + await addPatternsToMap(map, [{ fillPattern: 'stripes' }], OUTDOOR, patternRegistry) + expect(map.addImage).not.toHaveBeenCalled() + getPatternImageId.mockRestore() + }) }) }) diff --git a/src/services/patternRegistry.js b/src/services/patternRegistry.js index 117e3080..c90d4a99 100644 --- a/src/services/patternRegistry.js +++ b/src/services/patternRegistry.js @@ -34,6 +34,13 @@ export const patternRegistry = { return [...patterns.values()] }, + /** + * Clears all registered patterns (including built-ins). Mainly for testing purposes. + */ + clear () { + patterns.clear() + }, + /** * Returns the raw (un-coloured) inner SVG content for a style's pattern. * Precedence: inline fillPatternSvgContent → named fillPattern from registry. @@ -93,7 +100,6 @@ export const patternRegistry = { const effectiveRatio = Math.max(PATTERN_MIN_PIXEL_RATIO, pixelRatio) return `pattern-${hashString(innerContent + fg + bg)}-${effectiveRatio}x` } - } // Seed built-in patterns From 90948d5d82a4979a203a8988d8ab528f11377ab8 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 29 Apr 2026 14:26:41 +0100 Subject: [PATCH 5/6] IM-245 patternRegistry has tests that were in patternUtils --- src/services/patternRegistry.test.js | 115 ++++++++++++++++++++++++- src/utils/patternUtils.test.js | 120 +-------------------------- 2 files changed, 115 insertions(+), 120 deletions(-) diff --git a/src/services/patternRegistry.test.js b/src/services/patternRegistry.test.js index eb79c9a1..6a00fcdc 100644 --- a/src/services/patternRegistry.test.js +++ b/src/services/patternRegistry.test.js @@ -1,5 +1,5 @@ import { patternRegistry } from './patternRegistry.js' - +import { KEY_BORDER_PATH } from '../utils/patternUtils.js' describe('patternRegistry', () => { describe('built-in patterns', () => { test.each([ @@ -45,4 +45,117 @@ describe('patternRegistry', () => { expect(all.every(p => p.id && p.svgContent)).toBe(true) }) }) + + describe('getPatternInnerContent', () => { + test('returns fillPatternSvgContent when set (inline SVG takes precedence)', () => { + const style = { fillPatternSvgContent: '', fillPattern: 'dot' } + expect(patternRegistry.getPatternInnerContent(style)).toBe('') + }) + + test('returns svgContent from registry for a named fillPattern', () => { + const style = { fillPattern: 'dot' } + patternRegistry.register('dot', '') + expect(patternRegistry.getPatternInnerContent(style)).toBe('') + }) + + test('returns null for an unregistered fillPattern name', () => { + const style = { fillPattern: 'unknown-pattern' } + expect(patternRegistry.getPatternInnerContent(style)).toBeNull() + }) + + test('returns null when no pattern is configured', () => { + expect(patternRegistry.getPatternInnerContent({})).toBeNull() + }) + }) + + describe('getKeyPatternPaths', () => { + test('returns border and content strings with colours injected', () => { + const dataset = { + fillPattern: 'dot', + fillPatternForegroundColor: 'red', + fillPatternBackgroundColor: 'white', + stroke: 'black' + } + const result = patternRegistry.getKeyPatternPaths(dataset, 'style-a') + expect(result).not.toBeNull() + expect(result.border).toContain('black') // stroke colour + expect(result.border).toContain('white') // background colour + expect(result.content).toContain('red') // foreground colour + expect(result.border).not.toContain('{{foregroundColor}}') + expect(result.content).not.toContain('{{foregroundColor}}') + }) + + test('returns null when no pattern content is found', () => { + expect(patternRegistry.getKeyPatternPaths({ fillPattern: 'unknown' }, 'style-a')).toBeNull() + }) + + test('falls back to "black" fg and "transparent" bg when colour properties are absent', () => { + const result = patternRegistry.getKeyPatternPaths({ fillPattern: 'dot' }, 'style-a') + expect(result).not.toBeNull() + expect(result.content).toContain('black') + expect(result.border).toContain('transparent') + }) + + test('border stroke falls back to foreground colour when stroke is absent', () => { + const result = patternRegistry.getKeyPatternPaths( + { fillPattern: 'dot', fillPatternForegroundColor: 'green' }, + 'style-a' + ) + expect(result).not.toBeNull() + // borderStroke falls back to fg ('green'), so the border uses green for both stroke and background + expect(result.border).toContain('green') + }) + + test('KEY_BORDER_PATH contains foregroundColor and backgroundColor tokens', () => { + expect(KEY_BORDER_PATH).toContain('{{foregroundColor}}') + expect(KEY_BORDER_PATH).toContain('{{backgroundColor}}') + }) + }) + + describe('patternRegistry.getPatternImageId', () => { + test('returns a deterministic string id', () => { + const dataset = { fillPattern: 'dot', fillPatternForegroundColor: 'red', fillPatternBackgroundColor: 'blue' } + const id = patternRegistry.getPatternImageId(dataset, 'style-a') + expect(typeof id).toBe('string') + expect(id).toMatch(/^pattern-/) + expect(id).toBe(patternRegistry.getPatternImageId(dataset, 'style-a')) + }) + + test('returns null when no pattern content is found', () => { + expect(patternRegistry.getPatternImageId({ fillPattern: 'unknown' }, 'style-a')).toBeNull() + }) + + test('produces different ids for different colours', () => { + const base = { fillPattern: 'dot' } + const idA = patternRegistry.getPatternImageId({ ...base, fillPatternForegroundColor: 'red' }, 'style-a') + const idB = patternRegistry.getPatternImageId({ ...base, fillPatternForegroundColor: 'blue' }, 'style-a') + expect(idA).not.toBe(idB) + }) + + test('floors effective ratio at PATTERN_MIN_PIXEL_RATIO so low-DPI ids match 2x', () => { + const dataset = { fillPattern: 'dot' } + const id1x = patternRegistry.getPatternImageId(dataset, 'style-a', 1) + const id2x = patternRegistry.getPatternImageId(dataset, 'style-a', 2) + expect(id1x).toBe(id2x) + expect(id1x).toMatch(/-2x$/) + }) + + test('produces different ids for pixelRatios above the floor', () => { + const dataset = { fillPattern: 'dot' } + const id2x = patternRegistry.getPatternImageId(dataset, 'style-a', 2) + const id3x = patternRegistry.getPatternImageId(dataset, 'style-a', 3) + expect(id2x).not.toBe(id3x) + expect(id2x).toMatch(/-2x$/) + expect(id3x).toMatch(/-3x$/) + }) + + test('falls back to "black" foreground and "transparent" background when colours are absent', () => { + const id = patternRegistry.getPatternImageId({ fillPattern: 'dot' }, 'style-a') + const idExplicit = patternRegistry.getPatternImageId( + { fillPattern: 'dot', fillPatternForegroundColor: 'black', fillPatternBackgroundColor: 'transparent' }, + 'style-a' + ) + expect(id).toBe(idExplicit) + }) + }) }) diff --git a/src/utils/patternUtils.test.js b/src/utils/patternUtils.test.js index a522016a..d5402800 100644 --- a/src/utils/patternUtils.test.js +++ b/src/utils/patternUtils.test.js @@ -1,11 +1,7 @@ import { hashString, injectColors, - hasPattern, - getPatternInnerContent, - getPatternImageId, - getKeyPatternPaths, - KEY_BORDER_PATH + hasPattern } from './patternUtils.js' const mockRegistry = { @@ -61,117 +57,3 @@ describe('hasPattern', () => { expect(hasPattern({ fill: 'red' })).toBe(false) }) }) - -describe('getPatternInnerContent', () => { - test('returns fillPatternSvgContent when set (inline SVG takes precedence)', () => { - const dataset = { fillPatternSvgContent: '', fillPattern: 'dot' } - expect(getPatternInnerContent(dataset, mockRegistry)).toBe('') - }) - - test('returns svgContent from registry for a named fillPattern', () => { - const dataset = { fillPattern: 'dot' } - expect(getPatternInnerContent(dataset, mockRegistry)).toBe('') - }) - - test('returns null for an unregistered fillPattern name', () => { - const dataset = { fillPattern: 'unknown-pattern' } - expect(getPatternInnerContent(dataset, mockRegistry)).toBeNull() - }) - - test('returns null when no pattern is configured', () => { - expect(getPatternInnerContent({}, mockRegistry)).toBeNull() - }) -}) - -describe('getPatternImageId', () => { - test('returns a deterministic string id', () => { - const dataset = { fillPattern: 'dot', fillPatternForegroundColor: 'red', fillPatternBackgroundColor: 'blue' } - const id = getPatternImageId(dataset, 'style-a', mockRegistry) - expect(typeof id).toBe('string') - expect(id).toMatch(/^pattern-/) - expect(id).toBe(getPatternImageId(dataset, 'style-a', mockRegistry)) - }) - - test('returns null when no pattern content is found', () => { - expect(getPatternImageId({ fillPattern: 'unknown' }, 'style-a', mockRegistry)).toBeNull() - }) - - test('produces different ids for different colours', () => { - const base = { fillPattern: 'dot' } - const idA = getPatternImageId({ ...base, fillPatternForegroundColor: 'red' }, 'style-a', mockRegistry) - const idB = getPatternImageId({ ...base, fillPatternForegroundColor: 'blue' }, 'style-a', mockRegistry) - expect(idA).not.toBe(idB) - }) - - test('floors effective ratio at PATTERN_MIN_PIXEL_RATIO so low-DPI ids match 2x', () => { - const dataset = { fillPattern: 'dot' } - const id1x = getPatternImageId(dataset, 'style-a', mockRegistry, 1) - const id2x = getPatternImageId(dataset, 'style-a', mockRegistry, 2) - expect(id1x).toBe(id2x) - expect(id1x).toMatch(/-2x$/) - }) - - test('produces different ids for pixelRatios above the floor', () => { - const dataset = { fillPattern: 'dot' } - const id2x = getPatternImageId(dataset, 'style-a', mockRegistry, 2) - const id3x = getPatternImageId(dataset, 'style-a', mockRegistry, 3) - expect(id2x).not.toBe(id3x) - expect(id2x).toMatch(/-2x$/) - expect(id3x).toMatch(/-3x$/) - }) - - test('falls back to "black" foreground and "transparent" background when colours are absent', () => { - const id = getPatternImageId({ fillPattern: 'dot' }, 'style-a', mockRegistry) - const idExplicit = getPatternImageId( - { fillPattern: 'dot', fillPatternForegroundColor: 'black', fillPatternBackgroundColor: 'transparent' }, - 'style-a', - mockRegistry - ) - expect(id).toBe(idExplicit) - }) -}) - -describe('getKeyPatternPaths', () => { - test('returns border and content strings with colours injected', () => { - const dataset = { - fillPattern: 'dot', - fillPatternForegroundColor: 'red', - fillPatternBackgroundColor: 'white', - stroke: 'black' - } - const result = getKeyPatternPaths(dataset, 'style-a', mockRegistry) - expect(result).not.toBeNull() - expect(result.border).toContain('black') // stroke colour - expect(result.border).toContain('white') // background colour - expect(result.content).toContain('red') // foreground colour - expect(result.border).not.toContain('{{foregroundColor}}') - expect(result.content).not.toContain('{{foregroundColor}}') - }) - - test('returns null when no pattern content is found', () => { - expect(getKeyPatternPaths({ fillPattern: 'unknown' }, 'style-a', mockRegistry)).toBeNull() - }) - - test('falls back to "black" fg and "transparent" bg when colour properties are absent', () => { - const result = getKeyPatternPaths({ fillPattern: 'dot' }, 'style-a', mockRegistry) - expect(result).not.toBeNull() - expect(result.content).toContain('black') - expect(result.border).toContain('transparent') - }) - - test('border stroke falls back to foreground colour when stroke is absent', () => { - const result = getKeyPatternPaths( - { fillPattern: 'dot', fillPatternForegroundColor: 'green' }, - 'style-a', - mockRegistry - ) - expect(result).not.toBeNull() - // borderStroke falls back to fg ('green'), so the border uses green for both stroke and background - expect(result.border).toContain('green') - }) - - test('KEY_BORDER_PATH contains foregroundColor and backgroundColor tokens', () => { - expect(KEY_BORDER_PATH).toContain('{{foregroundColor}}') - expect(KEY_BORDER_PATH).toContain('{{backgroundColor}}') - }) -}) From a7d5f62c97281f64a9f5a89af1312adc633b739f Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 29 Apr 2026 14:41:54 +0100 Subject: [PATCH 6/6] IM-245 fully covered patternImages.js --- providers/maplibre/src/utils/patternImages.test.js | 12 ++++++++++-- src/utils/patternUtils.test.js | 4 ---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/providers/maplibre/src/utils/patternImages.test.js b/providers/maplibre/src/utils/patternImages.test.js index 82f28bfe..534ef6d4 100644 --- a/providers/maplibre/src/utils/patternImages.test.js +++ b/providers/maplibre/src/utils/patternImages.test.js @@ -163,19 +163,27 @@ describe('addPatternsToMap', () => { describe('addPatternsToMap — null results', () => { it('does not call addImage when innerContent becomes unavailable inside rasterisePattern', async () => { - const getPatternInnerContent = jest.spyOn(patternRegistry, 'getPatternInnerContent').mockReturnValueOnce(undefined) + const getPatternInnerContent = jest.spyOn(patternRegistry, 'getPatternInnerContent').mockReturnValue(undefined) + const getPatternImageId = jest.spyOn(patternRegistry, 'getPatternImageId').mockReturnValue('test-id') + const map = makeMap() await addPatternsToMap(map, [{ fillPattern: 'stripes' }], OUTDOOR, patternRegistry) expect(map.addImage).not.toHaveBeenCalled() + + getPatternImageId.mockRestore() getPatternInnerContent.mockRestore() }) it('does not call addImage when imageId becomes unavailable inside rasterisePattern', async () => { - const getPatternImageId = jest.spyOn(patternRegistry, 'getPatternImageId').mockReturnValue(undefined) + const getPatternInnerContent = jest.spyOn(patternRegistry, 'getPatternInnerContent').mockReturnValue('CONTENT') + const getPatternImageId = jest.spyOn(patternRegistry, 'getPatternImageId') + .mockReturnValueOnce('test-id') + .mockReturnValueOnce(undefined) const map = makeMap() await addPatternsToMap(map, [{ fillPattern: 'stripes' }], OUTDOOR, patternRegistry) expect(map.addImage).not.toHaveBeenCalled() getPatternImageId.mockRestore() + getPatternInnerContent.mockRestore() }) }) }) diff --git a/src/utils/patternUtils.test.js b/src/utils/patternUtils.test.js index d5402800..c6ce080e 100644 --- a/src/utils/patternUtils.test.js +++ b/src/utils/patternUtils.test.js @@ -4,10 +4,6 @@ import { hasPattern } from './patternUtils.js' -const mockRegistry = { - get: (id) => id === 'dot' ? { id: 'dot', svgContent: '' } : undefined -} - describe('hashString', () => { test('returns a non-empty string', () => { expect(typeof hashString('hello')).toBe('string')