From 93ea6c6c8b02ecaad781a4fe59d48f09f74b1ded Mon Sep 17 00:00:00 2001 From: Roland Date: Sun, 2 Aug 2026 13:33:51 -0600 Subject: [PATCH] ui: the monochrome themes draw a black basemap, not a dark one Roland asked why the red overlay sits on the DARK basemap rather than greyscale or black. Measuring it, black is the better answer and the reason is not brightness: flavor distinct colours hued earth luminance dark 19 4 0.014 black 13 0 0.007 Both media filters are the same pure luminance projection -- `0.2126 0.7152 0.0722` -- so they keep brightness and throw hue away. `black` is already achromatic, so filtering it is a hue rotation and nothing else: all thirteen greys were authored as a monochrome hierarchy and every one arrives intact. `dark` has more colours, but four of them are hued (water, parks, up to 25 chroma) and those distinctions do not survive at all -- the features land at whatever brightness their hue happened to carry rather than one anybody chose. Black also emits half the light on the earth fill, which is the point of a night-vision theme. `grayscale` is not an option despite the name: it is a LIGHT theme (earth L0.604), and a luminance filter turns a light map into a bright red one. What made black look impossible in the first commit was the sprite, and that was my mistake: `sprite` and the layer flavor are separate style properties that merely shared one variable. We ship `light` and `dark` sprite sets, and a `black` basemap with `dark` icons is an ordinary style. They are now separate arguments, with `mapSprite()` beside `mapFlavor()`. Fixed in passing: the contour labels branched on `flavor === "dark"` and would have taken the LIGHT branch for `black` -- dark brown text on a black map. Now branches on `light`, so every dark flavor gets the pale label. The new test is the one worth having: the monochrome themes' flavor must paint with greys only, paired with an assertion that `dark` is NOT achromatic. Without that pair the first would pass on any flavor at all and prove nothing about the choice. 1096 jest / 61 suites, 4 e2e themes, clean build. Co-Authored-By: Claude Opus 5 (1M context) --- app/src/components/MapViewer.js | 18 ++++++---- app/src/themes/map-flavor.test.js | 55 ++++++++++++++++++++++++++++--- app/src/themes/names.ts | 36 +++++++++++++++----- 3 files changed, 89 insertions(+), 20 deletions(-) diff --git a/app/src/components/MapViewer.js b/app/src/components/MapViewer.js index f7ce1970..92799f30 100644 --- a/app/src/components/MapViewer.js +++ b/app/src/components/MapViewer.js @@ -9,7 +9,7 @@ import {addMapPin, deleteMapPin, getMapFiles, getMapPins, searchMap, setMapDefau import {MAP_VIEWER_URI} from "./Vars"; import {Button, Checkbox, Header, Icon, Panel, TextInput, Toggle} from "./ui"; import {SettingsContext, ThemeContext} from "../contexts/contexts"; -import {mapFlavor} from "../themes/names"; +import {mapFlavor, mapSprite} from "../themes/names"; // Terrain DEM file prefix for hillshade and contours. const TERRAIN_PREFIX = "terrain-"; @@ -69,7 +69,10 @@ function initContourSource(filename) { // sources: [{name, url}] where name is used as the MapLibre source ID and url is the pmtiles:// URL. // hasTerrain: whether to include hillshade and contour layers. // scaleUnit: "metric" or "imperial" — affects contour intervals and labels. -function buildStyle(sources, flavor = "light", hasTerrain = false, scaleUnit = "metric") { +// `sprite` is deliberately separate from `flavor`: we ship two sprite sets (light, dark) +// and the monochrome themes draw a `black` basemap, which has no sprites of its own. +function buildStyle(sources, flavor = "light", hasTerrain = false, scaleUnit = "metric", + sprite = "light") { const styleSources = {}; const allLayers = []; @@ -182,8 +185,8 @@ function buildStyle(sources, flavor = "light", hasTerrain = false, scaleUnit = " "text-size": 10, }, paint: { - "text-color": flavor === "dark" ? "rgba(210,180,140,0.9)" : "rgba(150,100,50,0.8)", - "text-halo-color": flavor === "dark" ? "rgba(0,0,0,0.8)" : "rgba(255,255,255,0.8)", + "text-color": flavor === "light" ? "rgba(150,100,50,0.8)" : "rgba(210,180,140,0.9)", + "text-halo-color": flavor === "light" ? "rgba(255,255,255,0.8)" : "rgba(0,0,0,0.8)", "text-halo-width": 1, }, }, @@ -193,7 +196,7 @@ function buildStyle(sources, flavor = "light", hasTerrain = false, scaleUnit = " return { version: 8, glyphs: "/map-assets/fonts/{fontstack}/{range}.pbf", - sprite: `${window.location.origin}/map-assets/sprites/${flavor}`, + sprite: `${window.location.origin}/map-assets/sprites/${sprite}`, sources: styleSources, layers: allLayers, }; @@ -612,7 +615,7 @@ export default function MapViewer() { try { map = new maplibregl.Map({ container: mapContainer.current, - style: buildStyle(mapSources, mapFlavor(theme), hasTerrain, scaleUnit), + style: buildStyle(mapSources, mapFlavor(theme), hasTerrain, scaleUnit, mapSprite(theme)), center: [initialLon, initialLat], zoom: initialZoom, attributionControl: true, @@ -758,7 +761,8 @@ export default function MapViewer() { const center = map.getCenter(); const zoom = map.getZoom(); - map.setStyle(buildStyle(activeFilesRef.current, mapFlavor(theme), hasTerrainRef.current, scaleUnit)); + map.setStyle(buildStyle(activeFilesRef.current, mapFlavor(theme), hasTerrainRef.current, + scaleUnit, mapSprite(theme))); map.once("style.load", () => { map.setCenter(center); map.setZoom(zoom); diff --git a/app/src/themes/map-flavor.test.js b/app/src/themes/map-flavor.test.js index 23cc7454..dc93484b 100644 --- a/app/src/themes/map-flavor.test.js +++ b/app/src/themes/map-flavor.test.js @@ -1,7 +1,7 @@ import fs from 'fs'; import path from 'path'; import layers from 'protomaps-themes-base'; -import {isDarkTheme, mapFlavor, themeNames} from './names'; +import {isDarkTheme, isMonochromeTheme, mapFlavor, mapSprite, themeNames} from './names'; /* * The map viewer draws its basemap with protomaps-themes-base, which takes a "flavor". @@ -41,10 +41,13 @@ describe('the map basemap flavor', () => { expect(built.length).toBeGreaterThan(0); }); - it.each(themeNames)('maps %s to a flavor we ship sprites for', (theme) => { - // The flavor is interpolated into `/map-assets/sprites/`, so a name protomaps - // accepts is not enough -- `black` and `grayscale` are real flavors with no sprites. - expect(spriteFlavors()).toContain(mapFlavor(theme)); + it.each(themeNames)('gives %s a sprite set we actually ship', (theme) => { + /* + * The sprite is a SEPARATE style property from the flavor; they only happened to + * share a variable, which is what made a `black` basemap look impossible. We ship + * two sets, so this is the narrower constraint and it is checked against disk. + */ + expect(spriteFlavors()).toContain(mapSprite(theme)); }); it.each(themeNames)('keeps %s on a basemap of its own brightness', (theme) => { @@ -56,3 +59,45 @@ describe('the map basemap flavor', () => { expect(mapFlavor(theme) === 'light').toBe(!isDarkTheme(theme)); }); }); + +describe('the monochrome themes get an achromatic basemap', () => { + /* + * The reason night and amber take `black` rather than `dark`, and the thing that would + * silently regress if someone "simplified" the mapping to `isDarkTheme ? dark : light`. + * + * Both media filters are a pure luminance projection -- the same `0.2126 0.7152 0.0722` + * row -- so they keep brightness and discard hue. A basemap that encodes anything in + * hue loses it: `dark` marks water and parks with colour, and after filtering those + * features sit at whatever brightness their hue happened to carry. An achromatic + * basemap has nothing to lose, so the designer's hierarchy arrives intact. + */ + const chroma = (hex) => { + const value = hex.replace('#', ''); + const [r, g, b] = [0, 2, 4].map(i => parseInt(value.slice(i, i + 2), 16)); + return Math.max(r, g, b) - Math.min(r, g, b); + }; + + /** Every literal hex a flavor paints with. */ + const paletteOf = (flavor) => layers('src', flavor).flatMap(layer => + ['background-color', 'fill-color', 'line-color'] + .map(key => layer.paint && layer.paint[key]) + .filter(value => typeof value === 'string' && /^#[0-9a-f]{6}$/i.test(value))); + + themeNames.filter(isMonochromeTheme).forEach((theme) => { + it(`paints ${theme} with greys only`, () => { + const palette = paletteOf(mapFlavor(theme)); + + expect(palette.length).toBeGreaterThan(20); + const hued = [...new Set(palette)].filter(hex => chroma(hex) > 0); + expect(hued).toEqual([]); + }); + }); + + it('is a real constraint: the plain dark flavor is NOT achromatic', () => { + // Without this the test above passes on any flavor at all and proves nothing about + // the choice -- `black` would look unremarkable rather than deliberate. + const hued = [...new Set(paletteOf('dark'))].filter(hex => chroma(hex) > 0); + + expect(hued.length).toBeGreaterThan(0); + }); +}); diff --git a/app/src/themes/names.ts b/app/src/themes/names.ts index 32f0a537..f166587c 100644 --- a/app/src/themes/names.ts +++ b/app/src/themes/names.ts @@ -51,17 +51,37 @@ export const isDarkTheme = (theme: ThemeName): boolean => darkThemes.includes(th * `TypeError: Cannot read properties of undefined (reading 'background')` thrown before the * first tile -- which is what /map did in night and amber once those themes existed. * - * Only `light` and `dark` are available: the flavor is also interpolated into - * `/map-assets/sprites/`, and those are the two sprite sets we ship. protomaps' - * `black` and `grayscale` would load a style and then fail to find any icons. + * The monochrome themes take `black`, which is protomaps' only ACHROMATIC dark flavor, and + * that is the whole reason to prefer it. Both media filters are a pure luminance + * projection -- the same `0.2126 0.7152 0.0722` row -- so they keep brightness and throw + * hue away. Filtering `black` is therefore a hue rotation and nothing else: all thirteen + * of its greys were authored as a monochrome hierarchy and every one survives. `dark` has + * nineteen colours but four of them are hued (water, parks, up to 25 chroma), and those + * distinctions do not survive at all -- the features land at whatever brightness their hue + * happened to carry rather than one anybody chose. `black` is also half the light: + * earth L0.007 against dark's L0.014, which is the point of a night-vision theme. * - * Night and amber take `dark` and let the media filter tint the canvas from there. Handing - * them `light` would not work even with the filter on: a filter shifts hue, so a white - * basemap becomes a red one, which is the opposite of what a night-vision theme is for. + * `grayscale` is not an option despite the name -- it is a LIGHT theme (earth L0.604), and + * a luminance filter turns a light map into a bright red one. */ -export type MapFlavor = 'light' | 'dark'; +export type MapFlavor = 'light' | 'dark' | 'black'; -export const mapFlavor = (theme: ThemeName): MapFlavor => +export const mapFlavor = (theme: ThemeName): MapFlavor => { + if (isMonochromeTheme(theme)) return 'black'; + return isDarkTheme(theme) ? 'dark' : 'light'; +}; + +/* + * The sprite set for a theme, which is NOT the flavor. + * + * They are separate style properties and only happened to share a variable, which is what + * made `black` look impossible: we ship two sprite sets, `light` and `dark`, so a style + * naming `/map-assets/sprites/black` would load and then find no icons. A `black` basemap + * with `dark` icons is a perfectly ordinary style. + */ +export type MapSprite = 'light' | 'dark'; + +export const mapSprite = (theme: ThemeName): MapSprite => isDarkTheme(theme) ? 'dark' : 'light'; export const isThemeName = (value: unknown): value is ThemeName =>