From 62a36b9609aa8d3b0ba2335002a54424a2f84c23 Mon Sep 17 00:00:00 2001 From: manjunath Date: Sat, 31 Oct 2026 02:23:43 +0530 Subject: [PATCH] keep tile outlines honest when the anchor tile fails to load A building or grounds outline from tiles now needs the tile holding the anchor: when that tile fails and only neighbours load, no neighbouring building or area stands in. Nominatim name tags holding several ";"-separated names match each name. Document recovery from a stuck public-Nominatim dispatch marker, correct the phone credit comment, drive the slow-device queue test with a mocked performance clock, and credit GitHub handles in the changelog. --- .env.example | 5 ++ changelog.d/842-voice-outlines.md | 24 +++++---- docs/CURRENT-STATE.md | 2 +- server/providers/regional/outlineSelect.js | 4 +- .../hybridAnnotationRenderer.test.mjs | 19 +++++-- src/annotations/outlineRungs.js | 30 ++++++++--- src/annotations/outlineRungs.test.mjs | 54 +++++++++++++++++++ src/tooling/nominatimOutlineRoute.test.mjs | 12 +++++ src/ui/mobileShell.test.mjs | 4 +- src/ui/styles/mobile-shell.css | 12 ++--- 10 files changed, 134 insertions(+), 32 deletions(-) diff --git a/.env.example b/.env.example index c5d10c220..d4bf6c601 100644 --- a/.env.example +++ b/.env.example @@ -280,6 +280,11 @@ OVERPASS_UPSTREAMS= # Public-instance requests per UTC day, outlines and search together. Not # applied to your own NOMINATIM_URL. # NOMINATIM_DAILY_CAP=50 +# Public-instance pacing is shared through .gev-cache/nominatim/state.json. If +# the server stopped mid-request (or its disk filled), it leaves a +# state.json.dispatch.* marker and refuses public lookups ("unfinished durable +# Nominatim dispatch") rather than risk breaking the spacing. With the server +# stopped, delete those marker files to resume. # PHOTON_URL: keyless place search endpoint used by the browser # (e.g. https://photon.example.org/api/). Unset uses photon.komoot.io; set it # empty to disable. diff --git a/changelog.d/842-voice-outlines.md b/changelog.d/842-voice-outlines.md index 67f17c666..62ebb5eaf 100644 --- a/changelog.d/842-voice-outlines.md +++ b/changelog.d/842-voice-outlines.md @@ -1,23 +1,24 @@ - Add source-backed voice outlines without public Overpass calls: bundled Census and Who's On First boundaries cover places and neighborhoods, OpenFreeMap tiles cover streets, buildings, and grounds, and guarded Nominatim lookup handles - remaining named areas with bounded caching and usage limits (Bilawal Sidhu, + remaining named areas with bounded caching and usage limits (bilawalsidhu, #842). - Preserve standalone server-token maps, project tile outlines only on demand, select building components with their holes independently, preserve non-Latin street-name marks, and enforce public Nominatim spacing at fetch invocation. - Nominatim now requires explicit endpoint configuration (Manjunath, #842). + Nominatim now requires explicit endpoint configuration (manjunath22466, #842). - Keep Census labels in filled geometry, handle local dateline outlines in one coordinate frame, and require whole settlement names or explicit returned - aliases while retaining landmark shorthand (Manjunath, #842). -- Harden and integrate voice outlines with main (Manjunath, #842). + aliases while retaining landmark shorthand (manjunath22466, #842). +- Harden and integrate voice outlines with main (manjunath22466, #842). - Wait for the original building installation before reporting duplicate results, including deferred outline installs, without restarting probes - (Manjunath, #842). -- Carry actual outline provenance to visible geometry and retain inline map credits only while the owning outline is displayed (Manjunath, #842). + (manjunath22466, #842). +- Carry actual outline provenance to visible geometry and retain inline map + credits only while the owning outline is displayed (manjunath22466, #842). - Require a named building's resolved anchor inside its tile footprint, outside courtyard holes, while keeping nearby selection for pointed-at buildings - (Manjunath, #842). + (manjunath22466, #842). - Start each queued building's ground-sampling budget when it reaches the head of the queue, keep Nominatim outlines that cross the antimeridian, let an unnamed school, university, hospital, cemetery, stadium or zoo area answer a @@ -26,10 +27,13 @@ shared before, fold place names in one shared helper, and exclude Who's On First sources whose licence is only listed as CC BY or CC0 "equivalent", including the Ottawa, Saskatoon, Gatineau and Madrid records - (Manjunath, #842). + (manjunath22466, #842). - Skip Nominatim place and outline lookups when the server reports that no Nominatim endpoint is configured, so a default install sends no request it - would refuse (Manjunath, #842). + would refuse (manjunath22466, #842). - Keep the phone credit line on screen when map and outline credits make it longer, wrapping it within the screen instead of running off the edge - (Manjunath, #842). + (manjunath22466, #842). +- Outline nothing from tiles when the tile under the point failed to load, so a + neighbouring building or area never stands in for it, and match each of a + Nominatim name tag's ";"-separated names (manjunath22466, #842). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index fca732e69..f2aaa01c5 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -5403,7 +5403,7 @@ request viewbox and continues searching without that geographic preference. - Runtime entry: `src/main.js` calls `initAnnotations({ viewer, tileset })`, exposes `window.__gevAnnotations`, and passes the engine into the voice action runner. - Engine contract: `src/annotations/annotationEngine.js` owns annotation state, TTL/fade lifecycle, concurrent anchor resolution, duplicate detection keyed on geometry, cancellation on clear/newer generations, and a hard cap of 120 live marks. Deferred outline upgrades drain FIFO at concurrency 2; queued work retains the owning abort controller and is discarded on a generation change before it can fetch. -- Resolver contract: `src/annotations/resolver.js` converts names, coordinates and screen pixels into world anchors and optional geometry. Google Geocode/Places supplies a centroid and scope; route requests use `/api/route`; ambiguous or distant results are rejected or recovered near the current view instead of drawing misleading blobs. Outline resolution keeps this order: bundled admin/place/neighborhood/Natural Earth geometry; OpenFreeMap street or pointed-building geometry where applicable; the configured OSM/Overpass admin/place/footprint/street/enclosing-area ladder; guarded Nominatim for eligible named cities, admin units, landmarks, buildings and grounds; then an OpenFreeMap building fallback that accepts a named building only when its resolved anchor lies in the selected filled footprint. Pointed-at building requests retain nearby selection within 40 m. Tile grounds answer a named grounds ask only from an area of the same name, or, when the ask's head noun names a kind of grounds (school, university/college, hospital, cemetery, stadium, zoo; "campus" alone names none), from the part of the smallest unnamed area of that class that holds the anchor (a class word elsewhere in a name, as in "University Avenue Park", does not count); a differently named area never answers. If no rung can supply honest geometry, the mark remains a point or reports the capability unavailable. A real polygon, definitive miss, configured-ladder approximation, rate limit or transient result keeps its existing finality; Nominatim is only tried after higher-priority configured sources cannot answer. Building tile selection returns the selected component and its holes; a shared vector feature ID does not establish that unrelated components are one building. Other trusted multi-part outlines and holes are retained in `anno.polygons`, with `ring` anchored to the part whose filled area contains the resolved point, outside its holes, when possible. Missing capability never invents a radius outline. Only explicitly country/state/county-scoped asks bypass near-view recovery and proximity gating; state scope requires a leading `state of …`/`the state of …` phrase, while bare names, proper names ending in “State,” and administrative geocode result types alone remain guarded. Overpass throttles remain distinct from normal transients: `Retry-After` is honored for one retry, and a repeated throttle ends only that mark's outline upgrade. +- Resolver contract: `src/annotations/resolver.js` converts names, coordinates and screen pixels into world anchors and optional geometry. Google Geocode/Places supplies a centroid and scope; route requests use `/api/route`; ambiguous or distant results are rejected or recovered near the current view instead of drawing misleading blobs. Outline resolution keeps this order: bundled admin/place/neighborhood/Natural Earth geometry; OpenFreeMap street or pointed-building geometry where applicable; the configured OSM/Overpass admin/place/footprint/street/enclosing-area ladder; guarded Nominatim for eligible named cities, admin units, landmarks, buildings and grounds; then an OpenFreeMap building fallback that accepts a named building only when its resolved anchor lies in the selected filled footprint. Pointed-at building requests retain nearby selection within 40 m. Tile building and grounds answers also need the tile holding the anchor: when that tile fails to load and only neighbours arrive, no neighbouring building or area stands in. Tile grounds answer a named grounds ask only from an area of the same name, or, when the ask's head noun names a kind of grounds (school, university/college, hospital, cemetery, stadium, zoo; "campus" alone names none), from the part of the smallest unnamed area of that class that holds the anchor (a class word elsewhere in a name, as in "University Avenue Park", does not count); a differently named area never answers. If no rung can supply honest geometry, the mark remains a point or reports the capability unavailable. A real polygon, definitive miss, configured-ladder approximation, rate limit or transient result keeps its existing finality; Nominatim is only tried after higher-priority configured sources cannot answer. Building tile selection returns the selected component and its holes; a shared vector feature ID does not establish that unrelated components are one building. Other trusted multi-part outlines and holes are retained in `anno.polygons`, with `ring` anchored to the part whose filled area contains the resolved point, outside its holes, when possible. Missing capability never invents a radius outline. Only explicitly country/state/county-scoped asks bypass near-view recovery and proximity gating; state scope requires a leading `state of …`/`the state of …` phrase, while bare names, proper names ending in “State,” and administrative geocode result types alone remain guarded. Overpass throttles remain distinct from normal transients: `Retry-After` is honored for one retry, and a repeated throttle ends only that mark's outline upgrade. - With no detailed-feature capability, annotations retain their pins and say "Detailed outline unavailable"; location focus reports the same result without retrying. Missing capability never synthesizes an outline from a radius. diff --git a/server/providers/regional/outlineSelect.js b/server/providers/regional/outlineSelect.js index cdaead385..401d9aeb9 100644 --- a/server/providers/regional/outlineSelect.js +++ b/server/providers/regional/outlineSelect.js @@ -276,8 +276,10 @@ function outlineNameMatches(row, query, kind) { }; add(row?.name); add(String(row?.display_name || '').split(',')[0]); + // OSM name tags may hold several names separated by ";" ("York;Eboracum"). if (row?.namedetails && typeof row.namedetails === 'object') - for (const value of Object.values(row.namedetails)) add(value); + for (const value of Object.values(row.namedetails)) + for (const part of String(value ?? '').split(';')) add(part); if (kind === 'city' || kind === 'admin' || kind === 'neighborhood') { const head = normalizePlaceName(String(query ?? '').split(',')[0]); return Boolean(head) && names.has(head); diff --git a/src/annotations/hybridAnnotationRenderer.test.mjs b/src/annotations/hybridAnnotationRenderer.test.mjs index e219f825c..9ccbed205 100644 --- a/src/annotations/hybridAnnotationRenderer.test.mjs +++ b/src/annotations/hybridAnnotationRenderer.test.mjs @@ -723,6 +723,10 @@ test('a building queued behind others gets its full budget on a slow device', as installBrowserGlobals(t); t.mock.timers.enable({ apis: ['setTimeout'] }); t.after(() => t.mock.timers.reset()); + // The renderers' per-frame budget checks read performance.now(); it follows + // the same mocked clock as the timers. + let now = 0; + t.mock.method(globalThis.performance, 'now', () => now); const { viewer, frames } = fakeViewer(-99, 31); const renderer = createHybridAnnotationRenderer(viewer); viewer.scene.clampToHeight = (position) => position; @@ -732,17 +736,22 @@ test('a building queued behind others gets its full budget on a slow device', as pendingVisible: false, workTrace: traces[index], })); - // Eight probes per building at one probe per frame; 1.5 s passes while - // each of the first two buildings samples, so the third waits 3 s in line. + // Eight probes per building at one probe per frame, at about 5 fps: each + // building samples for 1.5 s, so the third waits 3 s in line. Pending + // promise work runs between frames, as it does in a browser. for (let building = 0; building < 3; building += 1) { - for (let frame = 0; frame < 8; frame += 1) frames.emit(); - if (building < 2) t.mock.timers.tick(1500); + for (let frame = 0; frame < 8; frame += 1) { + now += 187.5; + t.mock.timers.tick(187.5); + frames.emit(); + await new Promise((resolve) => setImmediate(resolve)); + } } const results = await Promise.all(completions); assert.deepEqual(results.map((result) => result.status), ['installed', 'installed', 'installed']); assert.deepEqual(traces.map((trace) => trace.samplerStatus), ['installed', 'installed', 'installed']); + assert.ok(traces[2].samplerHeadAt - traces[2].samplerStart >= 2000, 'the third waited longer than one budget'); assert.ok(traces[2].samplerHeadAt >= traces[1].samplerHeadAt); - assert.ok(traces[2].samplerHeadAt >= traces[2].samplerStart); }); test('building cancellation, queued deadline and destroy prevent saved callbacks from committing', async (t) => { diff --git a/src/annotations/outlineRungs.js b/src/annotations/outlineRungs.js index 485486d55..ad80b9f27 100644 --- a/src/annotations/outlineRungs.js +++ b/src/annotations/outlineRungs.js @@ -5,6 +5,7 @@ import { } from '../sources/featureGeometry.js'; import { isUnavailableCapability } from '../sources/capability.js'; import { polygonsContain } from '../data/adminBoundaries.js'; +import { lonLatToTile } from '../data/tomtomTiles.js'; import { BUILDING_MAX_DISTANCE_M, OUTLINE_TILE_ZOOM, @@ -173,13 +174,27 @@ export function createOutlineRungs({ tiles = sharedOutlineTiles(), nominatim = createNominatimOutlineClient(), } = {}) { - /** Decoded tiles around a point, or null when the tiles cannot be read. */ - async function tilesAround(lat, lon, radiusM, signal) { + /** + * Decoded tiles around a point, or null when the tiles cannot be read. With + * `needAnchor`, a partial load that lost the point's own tile is null too: a + * neighbouring tile alone cannot show what is at the point, so a nearby + * building or area would stand in for it. + */ + async function tilesAround(lat, lon, radiusM, signal, needAnchor = false) { try { - const { tiles: decoded } = await tiles.fetchBounds( - boxAround(lat, lon, radiusM), - { zoom: OUTLINE_TILE_ZOOM, signal }, - ); + const { + tiles: decoded, + partial, + loadedTiles, + } = await tiles.fetchBounds(boxAround(lat, lon, radiusM), { + zoom: OUTLINE_TILE_ZOOM, + signal, + }); + if (needAnchor && partial && Array.isArray(loadedTiles)) { + const anchor = lonLatToTile(lon, lat, OUTLINE_TILE_ZOOM); + if (!loadedTiles.some((t) => t.x === anchor.x && t.y === anchor.y)) + return null; + } return decoded; } catch (error) { signal?.throwIfAborted(); @@ -212,6 +227,7 @@ export function createOutlineRungs({ ctx.lon, BUILDING_MAX_DISTANCE_M * 2, ctx.signal, + true, ); const building = decoded && buildingFromTiles(decoded, ctx); if (!building || (requireContainment && building.contains !== true)) @@ -228,7 +244,7 @@ export function createOutlineRungs({ /** The open area enclosing `seed` (the named feature) or the anchor. */ async function groundsRung(ctx, seed = null) { const at = seed || { lat: ctx.lat, lon: ctx.lon }; - const decoded = await tilesAround(at.lat, at.lon, 600, ctx.signal); + const decoded = await tilesAround(at.lat, at.lon, 600, ctx.signal, true); const area = decoded && enclosingAreaFromTiles(decoded, { diff --git a/src/annotations/outlineRungs.test.mjs b/src/annotations/outlineRungs.test.mjs index 945165e2a..820952cb8 100644 --- a/src/annotations/outlineRungs.test.mjs +++ b/src/annotations/outlineRungs.test.mjs @@ -340,6 +340,60 @@ test('named building tile fallbacks require the anchor in filled geometry', asyn assert.equal(pointed.outlineCredit, 'tiles'); }); +test('a partial tile load that lost the point tile lets nothing stand in', async () => { + // The point (z14 tile 8192/8192) is ~28 m from a building in the tile that + // did load; without the point's own tile, that neighbour must not answer. + const anchor = { lat: 0, lon: 0.00035 }; + const school = square(CAPITOL.lat, CAPITOL.lon, 0.002); + const load = (loadedTiles) => ({ + fetchBounds: async () => ({ + tiles: [ + { + buildings: [{ rings: [square(0, 0, 0.0001)], heightM: null }], + areas: [ + { name: null, rings: [school], class: 'school', clipped: false }, + ], + }, + ], + loadedTiles, + partial: true, + }), + }); + const pointAt = (tiles) => + createOutlineRungs({ tiles, nominatim: fakeNominatim(null) }).resolve( + ctx({ + ...anchor, + view: anchor, + scope: 'building', + target: 'this building', + fromName: false, + }), + ladder(UNAVAILABLE), + ); + assert.equal(await pointAt(load([{ z: 14, x: 8191, y: 8192 }])), UNAVAILABLE); + const kept = await pointAt(load([{ z: 14, x: 8192, y: 8192 }])); + assert.equal(kept.footprintKind, 'building', 'the point tile loaded'); + + const grounds = (loaded) => + createOutlineRungs({ + tiles: load(loaded), + nominatim: fakeNominatim(null), + }).resolve( + ctx({ + scope: 'building', + target: 'Lincoln High School grounds', + groundsLike: true, + }), + ladder(UNAVAILABLE), + ); + // The Capitol fixture point lies in tile 3743/6745. + assert.equal(await grounds([{ z: 14, x: 3744, y: 6745 }]), UNAVAILABLE); + assert.equal( + (await grounds([{ z: 14, x: 3743, y: 6745 }])).footprintKind, + 'area', + ); +}); + test('a city with no configured Overpass is outlined by Nominatim', async () => { const ring = square(43.6, 1.44, 0.05); const nominatim = fakeNominatim({ polygons: [[ring]], class: 'boundary' }); diff --git a/src/tooling/nominatimOutlineRoute.test.mjs b/src/tooling/nominatimOutlineRoute.test.mjs index d14abb19c..f00c25ba7 100644 --- a/src/tooling/nominatimOutlineRoute.test.mjs +++ b/src/tooling/nominatimOutlineRoute.test.mjs @@ -884,6 +884,18 @@ test('a result whose name does not answer the ask is passed over', () => { selectOutlineResult([city('City of York', { short_name: 'York' })], york) .outline, ); + // A name tag holding several names answers for each of them, not the joint text. + assert.ok( + selectOutlineResult([city('Eboracum', { alt_name: 'Jorvik;York' })], york) + .outline, + ); + assert.equal( + selectOutlineResult( + [city('Eboracum', { alt_name: 'Jorvik;New York' })], + york, + ).outline, + null, + ); assert.equal( selectOutlineResult([city('新東京')], { kind: 'city', query: '東京' }) .outline, diff --git a/src/ui/mobileShell.test.mjs b/src/ui/mobileShell.test.mjs index 7793fadd4..7cc5662fc 100644 --- a/src/ui/mobileShell.test.mjs +++ b/src/ui/mobileShell.test.mjs @@ -500,8 +500,8 @@ test('the phone shell keeps the required attribution visible', () => { /display:\s*none|visibility:\s*hidden|opacity:\s*0/, ); // A long credit line (map plus outline credits) wraps inside the screen: - // the box needs a real width for Cesium's absolutely positioned credit - // block, set before the 0.8 scale, so it is divided by it. + // it wraps, and the box gets a real width, set before the 0.8 scale, so it + // is divided by it. assert.match(idleCredit, /white-space: normal;/); assert.match( idleCredit, diff --git a/src/ui/styles/mobile-shell.css b/src/ui/styles/mobile-shell.css index e509f9ddf..64c09e678 100644 --- a/src/ui/styles/mobile-shell.css +++ b/src/ui/styles/mobile-shell.css @@ -378,12 +378,12 @@ body.mobile-shell:not(.recording-mode):not(.cockpit-mode):not(.ui-clean-view) right: auto; bottom: calc(var(--mobile-tab-space) + 106px) !important; left: 10px; - /* Cesium's credit block is absolutely positioned inside this box, so the - box needs a real width (a max-width alone leaves it 0 wide) for the - credits to wrap at the screen width rather than one word per line. The - width applies before the 0.8 scale, keeping the scaled line 10px from the - right edge, so every credit and "Data attribution" stay on screen. Like - the offset above, it must beat the dock's width: auto !important. */ + /* With the base rule's nowrap the credits run off the right edge, and a + max-width alone wrapped them into a narrow column: the line needs wrapping + and a real width. The width applies before the 0.8 scale, keeping the + scaled line 10px from the right edge, so every credit and "Data + attribution" stay on screen. Like the offset above, it must beat the + dock's width: auto !important. */ width: calc((100vw - 20px) / 0.8) !important; max-width: none !important; white-space: normal;