From 5f98b871e8ba91de3c06f4ca252edf792c73c5ea Mon Sep 17 00:00:00 2001 From: Said Turksever Date: Mon, 5 Oct 2026 18:35:44 +0100 Subject: [PATCH] fix(street-level): sequence loads, playback and nearest-photo edge cases From gekh's review of #768, each bug with a test that fails before: - P2: a failed sequence load clears the previous sequence's cones, resets the selection and can be retried; a second click while loading starts no second lookup. sequences.js had no tests: sequences.test.mjs now covers failure and retry, a superseded late answer, the cache and clearSelection. - P2: closing a photo stops MapillaryJS sequence playback, which kept the hidden viewer stepping and downloading images. - openNearest without the panel reports it instead of loading forever, and stops if the layer (or its provider) went off during the lookup; the mesh sampler's misses are bounded and forgotten with its samples; the sequence count is the union of ids across tiles; a provider withdraws only its own error, so a sequence error cannot mask KEY REJECTED; hover stays on (throttled) under an orbiting camera and re-picks when it stops; "since N days" follows the clock in a long-open tab. - A prewarm waits for one in flight and then builds if it gave up. - New coverage for the layer core (credits, KEY REJECTED vs REQUIRED, switching a provider off), the refresh wiring of the tilted-view fix and the draped-to-cast swap completion. - Cleanup: loading is derived from pending; freshStreet(); tile ranking computes distances once and shares one path for the date line; shared metresBetween / whenIdle / redrawTile; one NEAREST_RADIUS_M and one source method list; unused contract fields, exports, getImage and the decoder's image branch removed; PROVIDER_COLORS lists only Mapillary. Co-Authored-By: Claude Opus 5.5 --- scripts/format-scope.json | 1 + src/app/constructCatalog.js | 8 +- src/layers/streetLevel/groundCast.js | 11 +- src/layers/streetLevel/groundCast.test.mjs | 16 +- src/layers/streetLevel/index.js | 58 ++-- src/layers/streetLevel/index.test.mjs | 256 +++++++++++++++--- src/layers/streetLevel/meshSampler.js | 67 ++--- src/layers/streetLevel/meshSampler.test.mjs | 60 +++- src/layers/streetLevel/policy.js | 10 +- .../providers/mapillary/coverage.js | 87 +++--- .../providers/mapillary/coverage.test.mjs | 229 ++++++++++++++-- .../streetLevel/providers/mapillary/decode.js | 38 +-- .../providers/mapillary/decode.test.mjs | 2 - .../streetLevel/providers/mapillary/index.js | 47 +--- .../streetLevel/providers/mapillary/policy.js | 15 +- .../providers/mapillary/provider.test.mjs | 29 +- .../providers/mapillary/sequences.js | 30 +- .../providers/mapillary/sequences.test.mjs | 172 ++++++++++++ .../streetLevel/providers/mapillary/source.js | 4 - .../providers/mapillary/source.test.mjs | 4 +- .../streetLevel/providers/mapillary/viewer.js | 36 ++- src/layers/streetLevel/registry.js | 20 +- src/layers/streetLevel/registry.test.mjs | 11 +- src/layers/streetLevel/selection.js | 25 +- src/layers/streetLevel/selection.test.mjs | 56 +++- src/layers/streetLevel/state.js | 45 +-- src/layers/streetLevel/tileMath.js | 144 +++++----- src/layers/streetLevel/tileMath.test.mjs | 9 +- src/layers/streetLevel/view.js | 25 ++ src/layers/streetLevel/viewerHost.js | 43 ++- src/layers/streetLevel/viewerHost.test.mjs | 41 ++- 31 files changed, 1129 insertions(+), 470 deletions(-) create mode 100644 src/layers/streetLevel/providers/mapillary/sequences.test.mjs diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 58ae396c6..ba7f5111b 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -915,6 +915,7 @@ "src/ui/streetLevelControls.test.mjs", "src/ui/panelDock.test.mjs", "src/layers/streetLevel/viewerHost.test.mjs", + "src/layers/streetLevel/providers/mapillary/sequences.test.mjs", "src/layers/streetLevel/selection.test.mjs", "src/layers/streetLevel/index.test.mjs", "scripts/qa-overpass-offload.mjs", diff --git a/src/app/constructCatalog.js b/src/app/constructCatalog.js index dbbd98fd5..762dc3644 100644 --- a/src/app/constructCatalog.js +++ b/src/app/constructCatalog.js @@ -56,13 +56,7 @@ const SOURCE_METHODS = Object.freeze({ earthquakes: ['getSnapshot'], 'fire-perimeters': ['getSnapshot'], cables: ['fetch'], - mapillary: [ - 'getStatus', - 'getTile', - 'getImage', - 'getSequenceImages', - 'nearestImages', - ], + mapillary: ['getStatus', 'getTile', 'getSequenceImages', 'nearestImages'], }); /** diff --git a/src/layers/streetLevel/groundCast.js b/src/layers/streetLevel/groundCast.js index d93683359..e181d59d2 100644 --- a/src/layers/streetLevel/groundCast.js +++ b/src/layers/streetLevel/groundCast.js @@ -90,9 +90,6 @@ export const SURFACE_TERRAIN_ENTER_M = 1400; /** Camera height above which terrain mode switches back to draped. */ export const SURFACE_TERRAIN_EXIT_M = 1800; -/** Surface modes: draped on whatever the globe shows, or cast to bare earth. */ -export const SURFACE_MODES = Object.freeze(['draped', 'terrain']); - /** * The next surface mode for the camera. Terrain mode applies only on Google * 3D and only at street zoom; the gap between the enter and exit heights @@ -245,12 +242,6 @@ export function createGroundCaster({ return south + (north - south) * fy; } - /** Where an overlay goes: bare earth plus the lift; null when unknown. */ - function heightAt(lon, lat) { - const ground = groundAt(lon, lat); - return ground === null ? null : ground + lift; - } - /** * A line as flat [lon, lat, height, ...] degrees and metres, densified; * null when any corner it needs is not cached yet. With `meshAt` (sampled @@ -275,5 +266,5 @@ export function createGroundCaster({ return flat; } - return { prepare, prepareLines, groundAt, heightAt, castLine }; + return { prepare, prepareLines, groundAt, castLine }; } diff --git a/src/layers/streetLevel/groundCast.test.mjs b/src/layers/streetLevel/groundCast.test.mjs index 5eaf5bd84..740f228a1 100644 --- a/src/layers/streetLevel/groundCast.test.mjs +++ b/src/layers/streetLevel/groundCast.test.mjs @@ -86,14 +86,16 @@ test('densifyLine splits long segments and keeps the vertices', () => { test('the caster interpolates cached grid corners and adds the lift', async () => { const terrain = fakeTerrain(); const caster = createGroundCaster({ terrain, step: 0.001 }); - assert.equal(caster.heightAt(10.0005, 50.0005), null); + assert.equal(caster.groundAt(10.0005, 50.0005), null); assert.equal(await caster.prepare([[10.0005, 50.0005]]), true); assert.equal(terrain.calls.length, 1); assert.equal(terrain.calls[0].length, 4); // Centre of the cell: mean of 100, 101, 102, 103. - const height = caster.heightAt(10.0005, 50.0005); - assert.ok(Math.abs(height - (101.5 + GROUND_CAST_LIFT_M)) < 1e-6); assert.ok(Math.abs(caster.groundAt(10.0005, 50.0005) - 101.5) < 1e-6); + // A cast line sits the lift above it. + const point = [10.0005, 50.0005]; + const [, , height] = caster.castLine([point, point]); + assert.ok(Math.abs(height - (101.5 + GROUND_CAST_LIFT_M)) < 1e-6); // Same cell again: nothing to fetch. assert.equal(await caster.prepare([[10.0002, 50.0008]]), true); assert.equal(terrain.calls.length, 1); @@ -120,11 +122,11 @@ test('a geoid fallback or a failing proxy leaves the heights unknown', async () terrain: fakeTerrain({ source: 'geoid-fallback' }), }); assert.equal(await fallback.prepare([[10.0005, 50.0005]]), false); - assert.equal(fallback.heightAt(10.0005, 50.0005), null); + assert.equal(fallback.groundAt(10.0005, 50.0005), null); const down = createGroundCaster({ terrain: fakeTerrain({ fail: true }) }); assert.equal(await down.prepare([[10.0005, 50.0005]]), false); - assert.equal(down.heightAt(10.0005, 50.0005), null); + assert.equal(down.groundAt(10.0005, 50.0005), null); }); test('too many corners are not requested, and aborted prepares do nothing', async () => { @@ -255,9 +257,9 @@ test('a full cache drops its oldest corners, not every corner', async () => { assert.equal(await caster.prepare([cell]), true); // A fourth makes room by dropping the first cell only. assert.equal(await caster.prepare([cells[3]]), true); - assert.equal(caster.heightAt(...cells[0]), null, 'oldest dropped'); + assert.equal(caster.groundAt(...cells[0]), null, 'oldest dropped'); for (const cell of cells.slice(1)) - assert.notEqual(caster.heightAt(...cell), null, 'newer cells kept'); + assert.notEqual(caster.groundAt(...cell), null, 'newer cells kept'); }); test('cancelling a prepare cancels its terrain request', async () => { diff --git a/src/layers/streetLevel/index.js b/src/layers/streetLevel/index.js index 569949a38..8da2e370f 100644 --- a/src/layers/streetLevel/index.js +++ b/src/layers/streetLevel/index.js @@ -9,7 +9,7 @@ import { requiresKeyIdFor, validateProviders } from './registry.js'; import { normalizeFilter, resolveFilter, sameFilter } from './filter.js'; import { decodeParams, encodeParams } from './params.js'; import { composeUIState, summarizeCoverage } from './uiState.js'; -import { cameraHeightAboveGround, viewCentre } from './view.js'; +import { cameraHeightAboveGround, viewCentre, whenIdle } from './view.js'; import { createGroundCaster, nextSurfaceMode } from './groundCast.js'; import { createMeshSampler } from './meshSampler.js'; @@ -63,13 +63,6 @@ export function createStreetLevelLayer({ }; parts.selection = createSelection(context); - /** Run low-priority work when the browser is idle (or soon, headless). */ - function scheduleIdle(task) { - if (typeof globalThis.requestIdleCallback === 'function') - globalThis.requestIdleCallback(() => task(), { timeout: 1500 }); - else setTimeout(task, 200); - } - let notifyQueued = false; function notify() { if (notifyQueued) return; @@ -91,7 +84,7 @@ export function createStreetLevelLayer({ function providerContext(entry) { return Object.freeze({ services: state.services, - getViewer: () => state.viewer, + /** The imagery filter resolved against now; read it when filtering. */ getFilter: () => resolveFilter(state.filter), isActive: () => state.enabled && entry.on, /** Bare-earth heights (see groundCast.js), or null without a terrain service. */ @@ -103,8 +96,21 @@ export function createStreetLevelLayer({ notify, actions: { openImage: (imageId) => openImage(entry.def.id, imageId), + /** + * Show a provider error, or (null) withdraw the one it showed. A + * withdrawal never clears an error the viewer or another provider + * has shown since, and a stale one cannot mask a later coverage + * error such as KEY REJECTED. + */ reportError: (message) => { - state.street.error = message || null; + if (message) { + state.street.error = entry.reportedError = message; + } else { + if (!entry.reportedError) return; + if (state.street.error === entry.reportedError) + state.street.error = null; + entry.reportedError = null; + } notify(); }, }, @@ -112,7 +118,14 @@ export function createStreetLevelLayer({ } for (const def of definitionsFrozen) { - const entry = { def, instance: null, on: true, status: null }; + const entry = { + def, + instance: null, + on: true, + status: null, + /** The error this provider last reported, until it withdraws it. */ + reportedError: null, + }; entry.instance = def.create(providerContext(entry)); state.providers.set(def.id, entry); } @@ -217,7 +230,7 @@ export function createStreetLevelLayer({ entry.instance.activate(state.viewer); parts.credits.show(state.viewer, entry.def); if (!entry.status) refreshStatus(entry); - scheduleIdle(() => parts.viewerHost.prewarm([entry])); + whenIdle(() => parts.viewerHost.prewarm([entry]), 1500); } function deactivate(entry) { @@ -384,7 +397,6 @@ export function createStreetLevelLayer({ else if (coverage.hint && state.enabled) loadingLabel = coverage.hint; return { count: coverage.count, - sequences: coverage.count, loading: coverage.loading, keyRequired: coverage.keyRequired, error: coverage.keyRequired @@ -438,13 +450,11 @@ export function createStreetLevelLayer({ attachViewerHost(element) { parts.viewerHost.attach(element); if (element && state.enabled) - scheduleIdle(() => parts.viewerHost.prewarm(activeEntries())); + whenIdle(() => parts.viewerHost.prewarm(activeEntries()), 1500); }, setProviderEnabled, - isProviderEnabled: (id) => state.providers.get(id)?.on === true, /** Imagery filter for coverage, cones and nearest-image lookups. */ setCoverageFilter, - getCoverageFilter: () => ({ ...state.filter }), openImage, /** Open the nearest image any active provider has around a point. */ async openNearest(point) { @@ -467,13 +477,16 @@ export function createStreetLevelLayer({ lastError = error; continue; } - if (!imageId) continue; - await openImage(entry.def.id, imageId); - return true; + if (!state.enabled) break; + // This provider was switched off while it looked: try the next. + if (!imageId || !entry.on) continue; + return openImage(entry.def.id, imageId); } - state.street.error = - lastError?.message || - `No street-level imagery within ${NEAREST_RADIUS_M} m of the view centre`; + // Switched off while it looked: nothing to open, nothing to report. + if (state.enabled && activeEntries().length) + state.street.error = + lastError?.message || + `No street-level imagery within ${NEAREST_RADIUS_M} m of the view centre`; state.street.loading = false; notify(); return false; @@ -485,7 +498,6 @@ export function createStreetLevelLayer({ }, setViewerRenderMode: (mode) => parts.viewerHost.setRenderMode(mode), setFollow: (enabled) => parts.follow.setFollow(enabled), - lookAtImage: () => parts.follow.lookAtPosition(), resizeViewer: () => parts.viewerHost.resize(), selectSequence( sequenceId, diff --git a/src/layers/streetLevel/index.test.mjs b/src/layers/streetLevel/index.test.mjs index 10bf2e9f6..06ef1f972 100644 --- a/src/layers/streetLevel/index.test.mjs +++ b/src/layers/streetLevel/index.test.mjs @@ -1,57 +1,121 @@ import assert from 'node:assert/strict'; import test from 'node:test'; +import * as Cesium from 'cesium'; import { createStreetLevelLayer } from './index.js'; +import { MAPILLARY_CREDIT_HTML } from './providers/mapillary/policy.js'; -/** The smallest provider the core accepts; nothing here touches a viewer. */ -function fakeProvider() { +/** + * The smallest provider the core accepts. `stats` is what coverageStats() + * answers; `calls` counts what the core asked of the provider and its viewer. + */ +function fakeProvider({ + id = 'mapillary', + pickPrefix = 'mly:', + html = MAPILLARY_CREDIT_HTML, + nearestImage = async () => null, +} = {}) { + const calls = { activate: 0, deactivate: 0, mount: 0, open: [], unmount: 0 }; + const stats = { + count: 0, + zoom: null, + kind: null, + loading: false, + hint: '', + error: null, + keyRequired: false, + }; + let context = null; return { - id: 'mapillary', + calls, + stats, + context: () => context, + id, name: 'Mapillary', label: 'MAPILLARY', requiresKeyId: null, - pickPrefix: 'mly:', + pickPrefix, colors: { coverage: '#05cb63' }, - credit: { key: 'mapillary', html: 'Mapillary' }, - capabilities: { - coverage: 'tiles', - sequences: false, - pano: true, - capturedAt: true, - creator: true, - follow: true, + credit: { html }, + create: (providerContext) => { + context = providerContext; + return { + status: async () => ({ configured: true }), + init() {}, + activate: () => calls.activate++, + deactivate: () => calls.deactivate++, + destroy() {}, + refreshCoverage() {}, + setFilter() {}, + coverageStats: () => ({ ...stats }), + handlePick: () => false, + nearestImage, + viewer: { + mount: async () => calls.mount++, + open: async (imageId) => calls.open.push(imageId), + close() {}, + unmount: () => calls.unmount++, + resize() {}, + onPose: () => () => {}, + }, + }; }, - externalUrl: (id) => `https://example.test/${id}`, - create: () => ({ - status: async () => ({ configured: true }), - init() {}, - activate() {}, - deactivate() {}, - destroy() {}, - refreshCoverage() {}, - setFilter() {}, - coverageStats: () => ({ - count: 0, - zoom: null, - kind: null, - loading: false, - hint: '', - error: null, - keyRequired: false, - }), - handlePick: () => false, - nearestImage: async () => null, - viewer: { - mount: async () => {}, - open: async () => {}, - close() {}, - unmount() {}, - resize() {}, - onPose: () => () => {}, - }, - }), }; } +/** + * A stand-in Cesium viewer the layer can be enabled on: a canvas for the + * click handler, camera events, a primitive list and a credit display. + */ +function fakeViewer() { + const credits = []; + const canvas = Object.assign(new EventTarget(), { + style: {}, + // Keep Cesium's handler on the canvas; there is no real document here. + disableRootEvents: true, + onwheel: null, + }); + return { + credits, + scene: { canvas, primitives: { add: (p) => p, remove() {} } }, + camera: { + changed: new Cesium.Event(), + moveStart: new Cesium.Event(), + moveEnd: new Cesium.Event(), + }, + creditDisplay: { + addStaticCredit: (credit) => credits.push(credit), + removeStaticCredit: (credit) => + credits.splice(credits.indexOf(credit), 1), + }, + }; +} + +/** A layer over `providers`, initialised and enabled on a stand-in viewer. */ +async function enabledLayer(t, providers = [fakeProvider()]) { + const saved = globalThis.document; + globalThis.document = new EventTarget(); + const viewer = fakeViewer(); + const layer = createStreetLevelLayer({ providers }); + layer.init(viewer); + layer.enable(viewer); + t.after(() => { + layer.destroy(); + globalThis.document = saved; + }); + await settle(); + return { layer, viewer }; +} + +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + +function deferred() { + let resolve; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + /** A map stack controller that can be switched between stacks. */ function fakeMapStack(initial) { let active = initial; @@ -110,3 +174,113 @@ test('the layer lets go of the map stack when destroyed or re-attached', () => { layer.destroy(); assert.equal(second.listenerCount(), 0); }); + +test('the CC BY-SA credit shows while a provider is on and goes with it', async (t) => { + const { layer, viewer } = await enabledLayer(t); + const shown = () => viewer.credits.map((credit) => credit.html); + assert.deepEqual(shown(), [MAPILLARY_CREDIT_HTML], 'shown on activate'); + assert.match(shown()[0], /CC BY-SA 4\.0/); + layer.setProviderEnabled('mapillary', false); + assert.deepEqual(shown(), [], 'hidden with the provider'); + layer.setProviderEnabled('mapillary', true); + assert.deepEqual(shown(), [MAPILLARY_CREDIT_HTML]); + layer.disable(); + assert.deepEqual(shown(), [], 'every credit goes with the layer'); +}); + +test('getStats tells a rejected key from a missing one', async (t) => { + const provider = fakeProvider(); + const { layer } = await enabledLayer(t, [provider]); + provider.stats.loading = true; + assert.equal(layer.getStats().loadingLabel, 'loading coverage...'); + assert.equal(layer.getStats().keyRequired, false); + + provider.stats.keyRequired = true; + assert.deepEqual( + [layer.getStats().loadingLabel, layer.getStats().error], + ['KEY REQUIRED', 'KEY REQUIRED'], + ); + + // A rejected key gates the provider exactly like a missing one. + provider.stats.keyRequired = false; + provider.stats.keyRejected = true; + provider.stats.error = 'Mapillary rejected MAPILLARY_CLIENT_TOKEN'; + const stats = layer.getStats(); + assert.equal(stats.keyRequired, true); + assert.equal(stats.loadingLabel, 'KEY REJECTED'); + assert.equal(stats.error, 'Mapillary rejected MAPILLARY_CLIENT_TOKEN'); + const ui = layer.getUIState(); + assert.equal(ui.keyRequired, true); + assert.equal(ui.keyRejected, true); + assert.equal(ui.providers[0].keyRequired, true); +}); + +test('switching a provider off deactivates it and closes the image it shows', async (t) => { + const provider = fakeProvider(); + const { layer } = await enabledLayer(t, [provider]); + layer.attachViewerHost({}); + assert.equal(await layer.openImage('mapillary', 'img1'), true); + assert.deepEqual(provider.calls.open, ['img1']); + assert.equal(layer.getUIState().street.open, true); + layer.setProviderEnabled('mapillary', false); + assert.equal(provider.calls.deactivate, 1); + assert.equal(provider.calls.unmount, 1, 'its viewer is released'); + assert.equal(layer.getUIState().street.open, false); + assert.equal(layer.getUIState().street.providerId, null); +}); + +test('openNearest without the panel reports it instead of loading forever (review P3)', async (t) => { + const provider = fakeProvider({ nearestImage: async () => 'img1' }); + const { layer } = await enabledLayer(t, [provider]); + assert.equal(await layer.openNearest({ lat: 38.58, lon: -121.49 }), false); + const { street } = layer.getUIState(); + assert.equal(street.loading, false); + assert.match(street.error, /Open the Street Level panel/); + assert.equal(provider.calls.mount, 0); +}); + +test('openNearest opens nothing once the layer went off during the lookup (review P3)', async (t) => { + const answer = deferred(); + const provider = fakeProvider({ nearestImage: () => answer.promise }); + const { layer } = await enabledLayer(t, [provider]); + layer.attachViewerHost({}); + const opening = layer.openNearest({ lat: 38.58, lon: -121.49 }); + layer.disable(); + answer.resolve('img1'); + assert.equal(await opening, false); + assert.equal(provider.calls.mount, 0, 'no viewer was stood up'); + const { street } = layer.getUIState(); + assert.equal(street.open, false); + assert.equal(street.loading, false); + assert.equal(street.error, null); +}); + +test('openNearest skips a provider switched off during its lookup (review P3)', async (t) => { + const answer = deferred(); + const provider = fakeProvider({ nearestImage: () => answer.promise }); + const { layer } = await enabledLayer(t, [provider]); + layer.attachViewerHost({}); + const opening = layer.openNearest({ lat: 38.58, lon: -121.49 }); + layer.setProviderEnabled('mapillary', false); + answer.resolve('img1'); + assert.equal(await opening, false); + assert.deepEqual(provider.calls.open, []); + assert.equal(layer.getUIState().street.loading, false); +}); + +test('a provider withdraws only the error it reported (review P3)', async (t) => { + const provider = fakeProvider(); + const { layer } = await enabledLayer(t, [provider]); + const { actions } = provider.context(); + actions.reportError('Sequence images unavailable'); + assert.equal(layer.getUIState().street.error, 'Sequence images unavailable'); + actions.reportError(null); + assert.equal(layer.getUIState().street.error, null, 'withdrawn'); + + // The viewer's own error is not the provider's to clear. + layer.attachViewerHost(null); + actions.reportError('Sequence images unavailable'); + await layer.openImage('mapillary', 'img1'); + actions.reportError(null); + assert.match(layer.getUIState().street.error, /Open the Street Level panel/); +}); diff --git a/src/layers/streetLevel/meshSampler.js b/src/layers/streetLevel/meshSampler.js index a06bc6f43..eeb819ac8 100644 --- a/src/layers/streetLevel/meshSampler.js +++ b/src/layers/streetLevel/meshSampler.js @@ -1,4 +1,5 @@ import * as Cesium from 'cesium'; +import { metresBetween, whenIdle } from './view.js'; /** * Rendered-surface heights for ground casting on Google 3D (see groundCast.js @@ -21,7 +22,7 @@ export const MESH_SAMPLE_RADIUS_M = 900; export const MESH_SAMPLE_BUDGET_MS = 6; /** A cell whose probe missed is tried again after this long. */ export const MESH_MISS_RETRY_MS = 8000; -/** Cached cells before the cache is dropped and refilled. */ +/** Cached cells (and remembered misses) before they are dropped and refilled. */ const MESH_CACHE_MAX = 80_000; /** Listeners hear about new samples at most this often. */ const MESH_NOTIFY_MS = 700; @@ -29,14 +30,6 @@ const MESH_NOTIFY_MS = 700; const cellOf = (value) => Math.round(value / MESH_CELL_DEG); const keyOf = (lon, lat) => `${cellOf(lon)},${cellOf(lat)}`; -function metres([lon1, lat1], [lon2, lat2]) { - const lat = (((lat1 + lat2) / 2) * Math.PI) / 180; - return Math.hypot( - (lon2 - lon1) * 111_320 * Math.cos(lat), - (lat2 - lat1) * 110_540, - ); -} - /** Every top-level primitive that is not a 3D tileset: what a probe skips. */ function overlays(scene) { const out = []; @@ -48,25 +41,24 @@ function overlays(scene) { return out; } -function idle(task) { - if (typeof globalThis.requestIdleCallback === 'function') - globalThis.requestIdleCallback(task, { timeout: 300 }); - else setTimeout(() => task(null), 16); -} - /** - * @param {{getViewer: () => object|null, budgetMs?: number, radiusM?: number}} options + * @param {{getViewer: () => object|null, budgetMs?: number, radiusM?: number, cacheMax?: number}} options */ export function createMeshSampler({ getViewer, budgetMs = MESH_SAMPLE_BUDGET_MS, radiusM = MESH_SAMPLE_RADIUS_M, + cacheMax = MESH_CACHE_MAX, }) { /** cell key → sampled mesh height (ellipsoidal metres). */ const heights = new Map(); - /** cell key → retry time for probes that hit nothing. */ + /** + * cell key → retry time for probes that hit nothing. An entry goes once + * its cell is sampled or asked for after the retry time, and with the + * height cache, so the map stays bounded. + */ const misses = new Map(); - /** cell key → [lon, lat] waiting to be probed. */ + /** cell key → {lon, lat} cell centre waiting to be probed. */ const wanted = new Map(); const listeners = new Set(); let enabled = false; @@ -87,11 +79,15 @@ export function createMeshSampler({ if (!Number.isFinite(lon) || !Number.isFinite(lat)) continue; const key = keyOf(lon, lat); if (heights.has(key) || wanted.has(key)) continue; - if ((misses.get(key) || 0) > now) continue; - wanted.set(key, [ - cellOf(lon) * MESH_CELL_DEG, - cellOf(lat) * MESH_CELL_DEG, - ]); + const retryAt = misses.get(key); + if (retryAt !== undefined) { + if (retryAt > now) continue; + misses.delete(key); + } + wanted.set(key, { + lon: cellOf(lon) * MESH_CELL_DEG, + lat: cellOf(lat) * MESH_CELL_DEG, + }); } schedule(); } @@ -99,7 +95,7 @@ export function createMeshSampler({ function schedule() { if (running || !enabled || !wanted.size) return; running = true; - idle(step); + whenIdle(step, 300); } function emit() { @@ -120,14 +116,14 @@ export function createMeshSampler({ const scene = getViewer()?.scene; const carto = getViewer()?.camera?.positionCartographic; if (!enabled || !scene?.sampleHeightSupported || !carto) return; - const centre = [ - Cesium.Math.toDegrees(carto.longitude), - Cesium.Math.toDegrees(carto.latitude), - ]; + const centre = { + lon: Cesium.Math.toDegrees(carto.longitude), + lat: Cesium.Math.toDegrees(carto.latitude), + }; // Nearest first; cells out of range are dropped (they stay bare earth). const order = []; for (const [key, point] of wanted) { - const distance = metres(point, centre); + const distance = metresBetween(point, centre); if (distance > radiusM) wanted.delete(key); else order.push([distance, key, point]); } @@ -139,7 +135,7 @@ export function createMeshSampler({ const until = performance.now() + Math.min(budgetMs, slice); const exclude = overlays(scene); let probed = 0; - for (const [, key, [lon, lat]] of order) { + for (const [, key, { lon, lat }] of order) { if (probed && performance.now() > until) break; wanted.delete(key); probed++; @@ -153,10 +149,12 @@ export function createMeshSampler({ height = undefined; } if (!Number.isFinite(height)) { + if (misses.size >= cacheMax) misses.clear(); misses.set(key, Date.now() + MESH_MISS_RETRY_MS); continue; } - if (heights.size >= MESH_CACHE_MAX) heights.clear(); + if (heights.size >= cacheMax) forget(); + misses.delete(key); heights.set(key, height); fresh.push([lon, lat]); } @@ -165,6 +163,12 @@ export function createMeshSampler({ schedule(); } + /** Drop every sample and remembered miss. */ + function forget() { + heights.clear(); + misses.clear(); + } + /** Probe only while overlays are cast (Google 3D at street zoom). */ function setEnabled(on) { enabled = on === true; @@ -180,6 +184,7 @@ export function createMeshSampler({ function destroy() { enabled = false; + forget(); wanted.clear(); listeners.clear(); clearTimeout(notifyTimer); diff --git a/src/layers/streetLevel/meshSampler.test.mjs b/src/layers/streetLevel/meshSampler.test.mjs index 55568d74e..e4478767a 100644 --- a/src/layers/streetLevel/meshSampler.test.mjs +++ b/src/layers/streetLevel/meshSampler.test.mjs @@ -1,7 +1,11 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import * as Cesium from 'cesium'; -import { MESH_CELL_DEG, createMeshSampler } from './meshSampler.js'; +import { + MESH_CELL_DEG, + MESH_MISS_RETRY_MS, + createMeshSampler, +} from './meshSampler.js'; const settle = (ms = 60) => new Promise((resolve) => setTimeout(resolve, ms)); const RAD = Math.PI / 180; @@ -78,3 +82,57 @@ test('far cells are skipped, misses are not latched, and nothing runs while disa assert.ok(Math.abs(probes[0].lon - 10.005) < MESH_CELL_DEG); sampler.destroy(); }); + +/** Probes run on a 16 ms timer headless; with mocked timers, run them now. */ +function probeNow(t) { + t.mock.timers.tick(20); +} + +test('a missed cell is probed again after the retry window, not before', (t) => { + t.mock.timers.enable({ apis: ['setTimeout', 'Date'] }); + const { viewer, probes } = fakeViewer({ holes: () => true }); + const sampler = createMeshSampler({ getViewer: () => viewer }); + sampler.setEnabled(true); + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(probes.length, 1, 'probed, and missed'); + t.mock.timers.tick(MESH_MISS_RETRY_MS - 100); + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(probes.length, 1, 'within the window: not probed'); + t.mock.timers.tick(100 + 1); + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(probes.length, 2, 'after the window: probed again'); + sampler.destroy(); +}); + +test('remembered misses are bounded like the samples (review P3)', (t) => { + t.mock.timers.enable({ apis: ['setTimeout', 'Date'] }); + const { viewer, probes } = fakeViewer({ holes: () => true }); + const sampler = createMeshSampler({ getViewer: () => viewer, cacheMax: 2 }); + sampler.setEnabled(true); + for (const lon of [10.001, 10.002, 10.003]) { + sampler.request([[lon, 50]]); + probeNow(t); + } + assert.equal(probes.length, 3); + // The third miss overflowed the list, which was dropped: the first cell + // is asked about again rather than remembered forever. + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(probes.length, 4); + sampler.destroy(); +}); + +test('destroy forgets the samples', (t) => { + t.mock.timers.enable({ apis: ['setTimeout', 'Date'] }); + const { viewer } = fakeViewer(); + const sampler = createMeshSampler({ getViewer: () => viewer }); + sampler.setEnabled(true); + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(sampler.meshAt(10.001, 50), 100); + sampler.destroy(); + assert.equal(sampler.meshAt(10.001, 50), undefined); +}); diff --git a/src/layers/streetLevel/policy.js b/src/layers/streetLevel/policy.js index 3249a038b..4ddd23544 100644 --- a/src/layers/streetLevel/policy.js +++ b/src/layers/streetLevel/policy.js @@ -7,12 +7,11 @@ export const POSITION_PICK_ID = 'sl:pos'; /** * One colour per imagery source, used everywhere that source appears: its * chip, coverage lines, overview points, image cones and legend swatch. - * Registered providers take their entry from here; a new source adds one. + * Registered providers take their entry from here; a new source adds its + * colour together with its provider. */ export const PROVIDER_COLORS = Object.freeze({ mapillary: '#05cb63', - panoramax: '#a66bff', - 'google-street-view': '#4285f4', }); /** Colours every provider shares: the selection highlight and the marker. */ @@ -30,7 +29,10 @@ export const MAX_SINCE_DAYS = 36_500; /** Filter every provider starts with: all imagery, any date. */ export const FILTER_DEFAULT = Object.freeze({ pano: 'all', sinceDays: 0 }); -/** Nearest-image search when the user asks to look at a place. */ +/** + * Nearest-image search when the user asks to look at a place, in metres + * (Mapillary's graph API answers within at most 50 m). + */ export const NEAREST_RADIUS_M = 50; /** diff --git a/src/layers/streetLevel/providers/mapillary/coverage.js b/src/layers/streetLevel/providers/mapillary/coverage.js index edaaf1fa5..8d3fb1980 100644 --- a/src/layers/streetLevel/providers/mapillary/coverage.js +++ b/src/layers/streetLevel/providers/mapillary/coverage.js @@ -1,7 +1,13 @@ import * as Cesium from 'cesium'; import { decodeCoverageTile } from './decode.js'; import { passesImageryFilter } from '../../filter.js'; -import { groundUnderCamera, viewFocus, visibleBbox } from '../../view.js'; +import { + groundUnderCamera, + metresBetween, + viewFocus, + visibleBbox, + whenIdle, +} from '../../view.js'; import { densifyLine, MESH_DENSIFY_DEG } from '../../groundCast.js'; import { MESH_CELL_DEG } from '../../meshSampler.js'; import { @@ -27,7 +33,7 @@ import { } from './policy.js'; /** Draped lines stay at most this long while a tile's cast lines build. */ -const SWAP_MAX_WAIT_MS = 4000; +export const SWAP_MAX_WAIT_MS = 4000; /** Tiles touched by new mesh samples are redrawn at most this often. */ const REMESH_INTERVAL_MS = 1500; /** Gap between redrawing one dirty tile and the next. */ @@ -104,8 +110,9 @@ export function createCoverage({ state, source }) { return `${z}/${x}/${y}`; } + /** The imagery filter, resolved now: "since N days" moves with the clock. */ function filter() { - return state.filter; + return state.context.getFilter(); } function terrainMode() { @@ -122,9 +129,10 @@ export function createCoverage({ state, source }) { */ function drawnSequences(entry) { const drawn = []; + const current = filter(); for (const sequence of entry.sequenceList) { if (drawn.length >= PER_TILE_SEQUENCE_CAP) break; - if (passesImageryFilter(sequence, filter())) drawn.push(sequence); + if (passesImageryFilter(sequence, current)) drawn.push(sequence); } return drawn; } @@ -232,15 +240,22 @@ export function createCoverage({ state, source }) { } // A remesh may have cleared the flag while this was in flight. entry.castRequested = true; - // Keep the draped lines until the cast ones are built, so nothing blinks. - const previous = entry.primitives; - entry.primitives = []; - attachPrimitive(entry); - removeWhenReady(entry, previous); + redrawTile(entry); requestMesh(entry); }); } + /** + * Rebuild a tile's lines (cast, or with new mesh heights), keeping the old + * ones on screen until the new ones are built, so nothing blinks. + */ + function redrawTile(entry) { + const previous = entry.primitives; + entry.primitives = []; + attachPrimitive(entry); + removeWhenReady(entry, previous); + } + /** The mesh cells under a tile's drawn lines, built once per filter. */ function meshCells(entry) { if (entry.meshCells) return entry.meshCells; @@ -302,25 +317,16 @@ export function createCoverage({ state, source }) { if (!entry) return; state.coverage.remeshDirty.delete(entry); if (attached.has(entry)) { - const previous = entry.primitives; - entry.primitives = []; - attachPrimitive(entry); - removeWhenReady(entry, previous); + redrawTile(entry); requestRender(); } if (state.coverage.remeshDirty.size) state.coverage.remeshTimer = setTimeout( - () => idleTask(remesh), + () => whenIdle(remesh, 500), REMESH_STAGGER_MS, ); } - function idleTask(task) { - if (typeof globalThis.requestIdleCallback === 'function') - globalThis.requestIdleCallback(() => task(), { timeout: 500 }); - else task(); - } - state.coverage.remeshDirty = new Set(); state.context.meshSampler?.onSampled(onMeshSampled); @@ -367,8 +373,9 @@ export function createCoverage({ state, source }) { 0.85, ); const drawn = []; + const current = filter(); for (const point of points) { - if (!passesImageryFilter(point, filter())) continue; + if (!passesImageryFilter(point, current)) continue; drawn.push( collection.add({ position: Cesium.Cartesian3.fromDegrees(point.lon, point.lat), @@ -513,7 +520,6 @@ export function createCoverage({ state, source }) { return; const controller = new AbortController(); state.coverage.pending.set(key, controller); - state.coverage.loading++; notify(); try { // Fetch and the one-time terrain-height table load run side by side. @@ -578,10 +584,9 @@ export function createCoverage({ state, source }) { if (error?.retryAfterSec) holdFor(error.retryAfterSec); } finally { // Only the request that still owns the key settles it: a superseded one - // must not drop a newer request's entry or its loading count. + // must not drop a newer request's entry (which is what counts as loading). if (state.coverage.pending.get(key) === controller) { state.coverage.pending.delete(key); - state.coverage.loading = Math.max(0, state.coverage.loading - 1); if (!state.coverage.pending.size) purgeStale(); } notify(); @@ -632,7 +637,6 @@ export function createCoverage({ state, source }) { for (const controller of state.coverage.pending.values()) controller.abort(); state.coverage.pending.clear(); - state.coverage.loading = 0; for (const [key, entry] of state.coverage.tiles) { cancelCast(entry); const previous = state.coverage.stale.get(key); @@ -654,10 +658,7 @@ export function createCoverage({ state, source }) { */ function isTilted({ nadir, ahead }, height) { if (!ahead) return true; - const dLat = (ahead.lat - nadir.lat) * 111_320; - const dLon = - (ahead.lon - nadir.lon) * 111_320 * Math.cos((nadir.lat * Math.PI) / 180); - return Math.hypot(dLat, dLon) > Math.max(1, height); + return metresBetween(nadir, ahead) > Math.max(1, height); } /** Recompute the tile set for the current camera and reconcile primitives. */ @@ -723,10 +724,9 @@ export function createCoverage({ state, source }) { for (const [key, controller] of [...state.coverage.pending]) if (!wanted.has(key)) { // The aborted request no longer owns its key, so its `finally` will - // not settle the count: settle it here. + // not settle it: settle it here. controller.abort(); state.coverage.pending.delete(key); - state.coverage.loading = Math.max(0, state.coverage.loading - 1); } for (const tile of tiles) loadTile(tile, kind); if (!state.coverage.pending.size) purgeStale(); @@ -769,7 +769,6 @@ export function createCoverage({ state, source }) { for (const controller of state.coverage.pending.values()) controller.abort(); state.coverage.pending.clear(); - state.coverage.loading = 0; for (const key of [...state.coverage.tiles.keys()]) removeTile(key); purgeStale(); state.coverage.zoom = null; @@ -807,15 +806,6 @@ export function createCoverage({ state, source }) { refresh(); } - /** Look a sequence up across loaded tiles. */ - function findSequence(id) { - for (const entry of state.coverage.tiles.values()) { - const hit = entry.sequences.get(id); - if (hit) return hit; - } - return null; - } - /** Recolour every part of a sequence in one ready primitive. */ function recolorInstances(primitive, sequence, selected) { const value = sequenceColor({ selected }); @@ -893,10 +883,18 @@ export function createCoverage({ state, source }) { state.coverage.stopSelectionWatch = null; } + /** + * Sequences (or overview points) drawn. A sequence that crosses a tile + * edge is in both tiles' lists, so sequences count once by id. + */ function sequenceCount() { - let count = 0; - for (const entry of state.coverage.tiles.values()) count += entry.count; - return count; + const ids = new Set(); + let points = 0; + for (const entry of state.coverage.tiles.values()) + if (entry.kind === 'sequence') + for (const id of entry.sequences.keys()) ids.add(id); + else points += entry.count; + return ids.size + points; } return { @@ -907,7 +905,6 @@ export function createCoverage({ state, source }) { resetErrors, rebuild, setSurface, - findSequence, recolorSequence, sequenceCount, }; diff --git a/src/layers/streetLevel/providers/mapillary/coverage.test.mjs b/src/layers/streetLevel/providers/mapillary/coverage.test.mjs index 2aefafd19..25099d9b5 100644 --- a/src/layers/streetLevel/providers/mapillary/coverage.test.mjs +++ b/src/layers/streetLevel/providers/mapillary/coverage.test.mjs @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import * as Cesium from 'cesium'; -import { createCoverage } from './coverage.js'; +import { SWAP_MAX_WAIT_MS, createCoverage } from './coverage.js'; import { encodeCoverageTile } from './coverageFixture.mjs'; import { COLORS, @@ -114,9 +114,10 @@ function setup({ const state = { viewer, services: {}, - filter: { pano: 'all', sinceMs: null }, keyRequired: false, context: { + filter: { pano: 'all', sinceMs: null }, + getFilter: () => state.context.filter, isActive: () => true, notify() {}, getSurface: () => surface, @@ -130,7 +131,6 @@ function setup({ stale: new Map(), staleTimer: null, pending: new Map(), - loading: 0, lastError: null, debounceTimer: null, removeCameraListener: null, @@ -188,7 +188,7 @@ test('a superseded tile request cannot strand lines on the globe (review P0 #2)' true, '#2 still owns the key', ); - assert.equal(state.coverage.loading, 1); + assert.equal(state.coverage.pending.size, 1, 'still loading'); coverage.refresh(); assert.equal(forTile().length, 2, 'no duplicate request'); @@ -265,26 +265,24 @@ test('a tile whose lines cannot be cast is not rebuilt for nothing', async () => coverage.clear(); }); -test('panning off a tile that is still loading settles the loading count', async () => { +test('panning off a tile that is still loading stops counting it as loading', async () => { const { viewer, source, state, coverage } = setup(); coverage.refresh(); - assert.equal(state.coverage.loading, 1); + assert.equal(state.coverage.pending.size, 1); // Same zoom, two tiles east: the first request is dropped, a new one starts. viewer.view.lon += 0.05; coverage.refresh(); assert.equal(source.calls.length, 2); assert.equal(source.calls[0].signal.aborted, true); - assert.equal(state.coverage.pending.size, 1); assert.equal( - state.coverage.loading, + state.coverage.pending.size, 1, 'only the live request counts as loading', ); source.calls[0].reject(new DOMException('aborted', 'AbortError')); source.calls[1].resolve(new Uint8Array()); await settle(); - assert.equal(state.coverage.pending.size, 0); - assert.equal(state.coverage.loading, 0, 'LOADING clears'); + assert.equal(state.coverage.pending.size, 0, 'LOADING clears'); coverage.clear(); }); @@ -337,7 +335,7 @@ test('a rate limit keeps the drawn tiles and asks again once the wait is over (r await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); - assert.match(state.coverage.lastError, /rate-limiting/); + assert.match(state.coverage.lastError, /rate-limited/); viewer.view.lon += 0.05; coverage.refresh(); assert.equal(source.calls.length, 1, 'held: nothing is requested'); @@ -494,7 +492,7 @@ test('the per-tile cap applies after the imagery filter, so a dense tile keeps i capturedAt: 1_000_000_000_000 + i, isPano: true, }); - state.filter = { pano: 'pano', sinceMs: null }; + state.context.filter = { pano: 'pano', sinceMs: null }; coverage.refresh(); source.calls[0].resolve(encodeCoverageTile(centre.tile, { sequences })); await settle(); @@ -502,13 +500,14 @@ test('the per-tile cap applies after the imagery filter, so a dense tile keeps i const cap = Math.floor(COVERAGE_MAX_SEQUENCES / COVERAGE_MAX_TILES); assert.equal(entry.count, Math.min(300, cap), 'every 360° line is drawn'); assert.equal(coverage.sequenceCount(), Math.min(300, cap)); - assert.ok(coverage.findSequence('pano-0'), 'drawn lines can be looked up'); - assert.equal(coverage.findSequence('flat-699'), null); - state.filter = { pano: 'all', sinceMs: null }; + // Picking and recolouring look sequences up among the drawn lines. + assert.ok(entry.sequences.has('pano-0'), 'drawn lines can be looked up'); + assert.equal(entry.sequences.has('flat-699'), false); + state.context.filter = { pano: 'all', sinceMs: null }; coverage.rebuild(); assert.equal(entry.count, cap, 'all imagery is capped as before'); - assert.ok(coverage.findSequence('flat-699'), 'the newest flat line is drawn'); - assert.equal(coverage.findSequence('pano-299'), null, 'over the cap'); + assert.ok(entry.sequences.has('flat-699'), 'the newest flat line is drawn'); + assert.equal(entry.sequences.has('pano-299'), false, 'over the cap'); coverage.clear(); }); @@ -558,3 +557,199 @@ test('a selection made or cleared while a tile builds is applied once it is read coverage.clear(); assert.equal(viewer.postRender.size, 0, 'the selection watch is gone'); }); + +test('a sequence that crosses a tile edge is counted once (review P3)', async () => { + const { viewer, source, coverage, centre } = setup(); + // Look straight down on the edge between this tile and its east neighbour. + const { east } = tileBounds(centre.tile.x, centre.tile.y, 14); + viewer.view.lon = east; + coverage.refresh(); + assert.equal(source.calls.length, 2, 'two tiles in view'); + const across = [ + [ + [east - 0.001, centre.lat], + [east + 0.001, centre.lat], + ], + ]; + for (const call of source.calls) { + const [, x, y] = call.key.split('/').map(Number); + call.resolve( + encodeCoverageTile( + { x, y, z: 14 }, + { + sequences: [ + { id: 'across', parts: across }, + { id: `only-${x}`, parts: across }, + ], + }, + ), + ); + } + await settle(); + assert.equal(coverage.sequenceCount(), 3, 'across, plus one per tile'); + coverage.clear(); +}); + +/** + * In terrain mode with a cast tile mid-swap: the draped lines and the cast + * primitive whose readiness the test controls. + */ +async function midSwap() { + // Drawn draped first; castLine only answers once the heights are in. + let heightsReady = false; + const groundCaster = { + prepareLines: async () => (heightsReady = true), + castLine: (coords) => + heightsReady ? coords.flatMap(([lon, lat]) => [lon, lat, 30]) : null, + }; + const context = setup({ surface: 'terrain', groundCaster }); + context.coverage.refresh(); + context.source.calls[0].resolve(context.bytes); + await settle(); + await settle(); + const { groundPrimitives, primitives } = context.viewer.scene; + assert.equal(groundPrimitives.items.size, 1, 'draped lines kept'); + assert.equal(primitives.items.size, 1, 'cast lines building'); + const [cast] = primitives.items; + const control = { ready: false }; + Object.defineProperty(cast, 'ready', { get: () => control.ready }); + return { ...context, control }; +} + +test('the draped lines go once the cast lines are ready', async () => { + const { viewer, coverage, control } = await midSwap(); + frame(viewer.postRender); + assert.equal(viewer.scene.groundPrimitives.items.size, 1, 'still building'); + control.ready = true; + frame(viewer.postRender); + assert.equal(viewer.scene.groundPrimitives.items.size, 0, 'draped removed'); + assert.equal(viewer.scene.primitives.items.size, 1, 'cast lines stay'); + assert.equal(viewer.postRender.size, 0, 'nothing left watching'); + coverage.clear(); +}); + +test('the draped lines go after the wait even if the cast lines never get ready', async (t) => { + t.mock.timers.enable({ apis: ['Date'] }); + const { viewer, coverage } = await midSwap(); + t.mock.timers.tick(SWAP_MAX_WAIT_MS - 1); + frame(viewer.postRender); + assert.equal(viewer.scene.groundPrimitives.items.size, 1, 'still waiting'); + t.mock.timers.tick(2); + frame(viewer.postRender); + assert.equal(viewer.scene.groundPrimitives.items.size, 0, 'draped removed'); + assert.equal(viewer.scene.primitives.items.size, 1); + coverage.clear(); +}); + +/** + * A camera `height` m above WGS84 looking along `heading` with `pitch` (deg), + * a 60°×40° view on a 100×100 canvas, whose screen rays really meet the + * ellipsoid they are given (as Cesium's `pickEllipsoid` does). + */ +function tiltedCamera({ lon, lat, height, heading, pitch }) { + const position = Cesium.Cartesian3.fromDegrees(lon, lat, height); + const frameAt = Cesium.Transforms.eastNorthUpToFixedFrame(position); + function ray(point) { + const h = (heading + (point.x / 100 - 0.5) * 60) * RAD; + const p = (pitch + (0.5 - point.y / 100) * 40) * RAD; + const local = new Cesium.Cartesian3( + Math.sin(h) * Math.cos(p), + Math.cos(h) * Math.cos(p), + Math.sin(p), + ); + const direction = Cesium.Matrix4.multiplyByPointAsVector( + frameAt, + local, + new Cesium.Cartesian3(), + ); + return new Cesium.Ray( + position, + Cesium.Cartesian3.normalize(direction, direction), + ); + } + return { + positionWC: position, + positionCartographic: { + longitude: lon * RAD, + latitude: lat * RAD, + height, + }, + pickEllipsoid(point, ellipsoid) { + const r = ray(point); + const hit = Cesium.IntersectionTests.rayEllipsoid(r, ellipsoid); + return hit ? Cesium.Ray.getPoint(r, hit.start) : undefined; + }, + computeViewRectangle: () => undefined, + }; +} + +/** + * Denver, 300 m above a street 1,600 m above the ellipsoid, at the centre of + * a z14 tile and looking east with the screen centre `aheadM` metres out. + * Google 3D hides the globe, so only the bare-earth caster knows the ground. + */ +function denverStreetView(aheadM) { + const z = 14; + const x = lonToTileX(-104.99, z); + const y = latToTileY(39.74, z); + const tile = tileBounds(x, y, z); + const lon = (tile.west + tile.east) / 2; + const lat = (tile.south + tile.north) / 2; + const ground = 1600; + const above = 300; + const context = setup({ groundCaster: { groundAt: () => ground } }); + const tileWidthM = (tile.east - tile.west) * 111_320 * Math.cos(lat * RAD); + context.viewer.camera = tiltedCamera({ + lon, + lat, + height: ground + above, + heading: 90, + pitch: -Math.atan(above / aheadM(tileWidthM)) / RAD, + }); + context.coverage.refresh(); + const keys = context.source.calls.map((call) => call.key); + /** Requested tiles as [dx, dy] from the tile under the camera. */ + const offsets = keys.map((k) => { + const [, tx, ty] = k.split('/').map(Number); + return [tx - x, ty - y]; + }); + return { + ...context, + keys, + offsets, + key: (dx, dy) => `${z}/${x + dx}/${y + dy}`, + }; +} + +test('a tilted street view over ground 1,600 m up asks for the tiles it looks at first', () => { + // The screen centre meets the ground at the centre of the next tile east. + const { state, coverage, keys, offsets, key } = denverStreetView( + (tileWidthM) => tileWidthM, + ); + assert.equal(state.coverage.zoom, 14, 'height measured from the ground'); + assert.deepEqual( + keys.slice(0, 2), + [key(0, 0), key(1, 0)], + 'the tile under the camera, then the one the screen centre looks at', + ); + assert.ok(keys.includes(key(-1, 0)), 'the ground behind the camera too'); + for (const [dx, dy] of offsets) + assert.ok(dx >= -1 && dx <= 2 && Math.abs(dy) <= 1, `${dx},${dy} in reach`); + coverage.clear(); +}); + +test('a street view toward the horizon ranks only the ground within range', () => { + // The screen centre meets the ground 6 km out, past the 3 km range: the + // tiles are ranked around the camera, not along a line to the horizon. + const { coverage, keys, offsets, key } = denverStreetView(() => 6000); + assert.equal(keys[0], key(0, 0), 'the tile under the camera first'); + // Behind and ahead are equally near the camera (ties keep row order); a + // line of sight to the far ground would rank the tile ahead first. + assert.ok( + keys.indexOf(key(-1, 0)) < keys.indexOf(key(1, 0)), + 'the tile ahead is not pulled forward', + ); + for (const [dx, dy] of offsets) + assert.ok(Math.abs(dx) <= 1 && Math.abs(dy) <= 1, `${dx},${dy} around it`); + coverage.clear(); +}); diff --git a/src/layers/streetLevel/providers/mapillary/decode.js b/src/layers/streetLevel/providers/mapillary/decode.js index d2067098d..57ea34bd2 100644 --- a/src/layers/streetLevel/providers/mapillary/decode.js +++ b/src/layers/streetLevel/providers/mapillary/decode.js @@ -4,15 +4,15 @@ import { tileLocalToLonLat } from '../../tileMath.js'; /** * Decode a Mapillary coverage tile (`mly1_public`) in the browser. - * Sequences are LineStrings with capture metadata; the `image` layer is only - * present at z14 and is skipped unless asked for (it can hold >150k points). + * Sequences are LineStrings with capture metadata; z0–5 tiles carry + * `overview` points instead. The z14 `image` layer (it can hold >150k + * points) is never read. * @param {Uint8Array} bytes * @param {{x:number,y:number,z:number}} address - * @param {{images?: boolean}} [options] - * @returns {{sequences: Array, images: Array}} + * @returns {{sequences: Array, overview: Array}} */ -export function decodeCoverageTile(bytes, address, { images = false } = {}) { - const result = { sequences: [], images: [], overview: [] }; +export function decodeCoverageTile(bytes, address) { + const result = { sequences: [], overview: [] }; if (!bytes || !bytes.length) return result; const tile = new VectorTile(new PbfReader(bytes)); const { x, y, z } = address; @@ -70,31 +70,5 @@ export function decodeCoverageTile(bytes, address, { images = false } = {}) { }); } } - const imageLayer = images ? tile.layers.image : null; - if (imageLayer) { - for (let i = 0; i < imageLayer.length; i++) { - const feature = imageLayer.feature(i); - const props = feature.properties || {}; - const point = feature.loadGeometry()?.[0]?.[0]; - if (!point) continue; - const [lon, lat] = tileLocalToLonLat( - point.x, - point.y, - imageLayer.extent, - x, - y, - z, - ); - result.images.push({ - id: String(props.id ?? feature.id), - lon, - lat, - compassAngle: Number(props.compass_angle) || 0, - capturedAt: Number(props.captured_at) || 0, - isPano: props.is_pano === true, - sequenceId: props.sequence_id ? String(props.sequence_id) : null, - }); - } - } return result; } diff --git a/src/layers/streetLevel/providers/mapillary/decode.test.mjs b/src/layers/streetLevel/providers/mapillary/decode.test.mjs index af522f2e7..eeb1f0261 100644 --- a/src/layers/streetLevel/providers/mapillary/decode.test.mjs +++ b/src/layers/streetLevel/providers/mapillary/decode.test.mjs @@ -113,7 +113,6 @@ test('sequences and overview points decode into lon/lat records', () => { assert.equal(decoded.overview[0].id, '3'); assert.equal(decoded.overview[0].isPano, false); assert.ok(Math.abs(decoded.overview[0].lon - -90) < 1e-6, 'tile centre'); - assert.equal(decoded.images.length, 0, 'image layer skipped by default'); }); test('an empty tile decodes to empty lists', () => { @@ -121,7 +120,6 @@ test('an empty tile decodes to empty lists', () => { decodeCoverageTile(new Uint8Array(0), { x: 0, y: 0, z: 0 }), { sequences: [], - images: [], overview: [], }, ); diff --git a/src/layers/streetLevel/providers/mapillary/index.js b/src/layers/streetLevel/providers/mapillary/index.js index 8c02c017c..da4edafe1 100644 --- a/src/layers/streetLevel/providers/mapillary/index.js +++ b/src/layers/streetLevel/providers/mapillary/index.js @@ -2,6 +2,7 @@ import { createCoverage, sequenceIdFromPick } from './coverage.js'; import { createSequences } from './sequences.js'; import { createMapillaryViewer } from './viewer.js'; import { passesImageryFilter } from '../../filter.js'; +import { metresBetween } from '../../view.js'; import { COLORS, MAPILLARY_CREDIT_HTML, @@ -9,22 +10,14 @@ import { MAPILLARY_LABEL, MAPILLARY_NAME, MAPILLARY_PROVIDER_ID, + MAPILLARY_SOURCE_METHODS, NEAREST_LIMIT, NEAREST_RADIUS_M, PICK_PREFIX, - mapillaryImageUrl, } from './policy.js'; export { MAPILLARY_PROVIDER_ID, mapillaryImageUrl } from './policy.js'; -const SOURCE_METHODS = [ - 'getStatus', - 'getTile', - 'getImage', - 'getSequenceImages', - 'nearestImages', -]; - /** Per-provider mutable state, created once per `create()`. */ function createProviderState(context) { return { @@ -35,7 +28,6 @@ function createProviderState(context) { /** Mapillary refused the configured token: nothing is requested until re-enabled. */ keyRejected: false, status: null, - filter: context.getFilter(), coverage: { zoom: null, /** @type {Map, count: number}>} */ @@ -43,8 +35,8 @@ function createProviderState(context) { /** Tiles from the previous zoom, kept on screen until replacements land. */ stale: new Map(), staleTimer: null, + /** Tile key → its request's controller; any entry means LOADING. */ pending: new Map(), - loading: 0, lastError: null, /** While Mapillary rate-limits: no requests before this time (ms). */ holdUntil: 0, @@ -74,7 +66,11 @@ function createProviderState(context) { * @returns {import('../../registry.js').StreetLevelProvider} */ export function createMapillaryProvider({ source }) { - if (!SOURCE_METHODS.every((method) => typeof source?.[method] === 'function')) + if ( + !MAPILLARY_SOURCE_METHODS.every( + (method) => typeof source?.[method] === 'function', + ) + ) throw new TypeError('A Mapillary source is required'); return Object.freeze({ @@ -84,19 +80,7 @@ export function createMapillaryProvider({ source }) { requiresKeyId: MAPILLARY_KEY_ID, pickPrefix: PICK_PREFIX.root, colors: COLORS, - credit: Object.freeze({ - key: MAPILLARY_PROVIDER_ID, - html: MAPILLARY_CREDIT_HTML, - }), - capabilities: Object.freeze({ - coverage: 'tiles', - sequences: true, - pano: true, - capturedAt: true, - creator: true, - follow: true, - }), - externalUrl: mapillaryImageUrl, + credit: Object.freeze({ html: MAPILLARY_CREDIT_HTML }), create(context) { const state = createProviderState(context); @@ -150,8 +134,8 @@ export function createMapillaryProvider({ source }) { refreshCoverage: () => parts.coverage.refresh(), - setFilter(filter) { - state.filter = filter; + /** The filter changed: redraw. Each draw reads it from the context. */ + setFilter() { parts.coverage.rebuild(); parts.sequences.rerender(); }, @@ -167,7 +151,7 @@ export function createMapillaryProvider({ source }) { count: parts.coverage.sequenceCount(), zoom: state.coverage.zoom, kind: state.coverage.kind, - loading: state.coverage.loading > 0, + loading: state.coverage.pending.size > 0, hint: state.coverage.hint, error: state.coverage.lastError, keyRequired: state.keyRequired, @@ -208,19 +192,18 @@ export function createMapillaryProvider({ source }) { const metres = (record) => { const [lon2, lat2] = (record.computed_geometry || record.geometry) ?.coordinates || [NaN, NaN]; - const dx = (lon2 - lon) * 111_320 * Math.cos((lat * Math.PI) / 180); - const dy = (lat2 - lat) * 110_540; - const d = Math.hypot(dx, dy); + const d = metresBetween({ lon, lat }, { lon: lon2, lat: lat2 }); return Number.isFinite(d) ? d : Infinity; }; const byDistance = [...images].sort((a, b) => metres(a) - metres(b)); + const filter = context.getFilter(); const hit = byDistance.find((record) => passesImageryFilter( { isPano: record.is_pano === true, capturedAt: Number(record.captured_at) || 0, }, - state.filter, + filter, ), ); return hit ? String(hit.id) : null; diff --git a/src/layers/streetLevel/providers/mapillary/policy.js b/src/layers/streetLevel/providers/mapillary/policy.js index cb7f6a1cd..467786791 100644 --- a/src/layers/streetLevel/providers/mapillary/policy.js +++ b/src/layers/streetLevel/providers/mapillary/policy.js @@ -1,5 +1,7 @@ import { COLORS as SHARED_COLORS, PROVIDER_COLORS } from '../../policy.js'; +export { NEAREST_RADIUS_M } from '../../policy.js'; + /** Identity and tuning for the Mapillary street-level provider. */ export const MAPILLARY_PROVIDER_ID = 'mapillary'; export const MAPILLARY_NAME = 'Mapillary'; @@ -7,6 +9,14 @@ export const MAPILLARY_LABEL = 'MAPILLARY'; export const MAPILLARY_KEY_ID = 'mapillary'; export const MAPILLARY_GRAPH_HOST = 'https://graph.mapillary.com'; +/** Methods a source (./source.js or a stand-in) must have for the provider. */ +export const MAPILLARY_SOURCE_METHODS = Object.freeze([ + 'getStatus', + 'getTile', + 'getSequenceImages', + 'nearestImages', +]); + /** Stable id prefixes for picked primitives; every one starts with `mly:`. */ export const PICK_PREFIX = Object.freeze({ root: 'mly:', @@ -33,7 +43,7 @@ export const KEY_REJECTED_MESSAGE = 'Mapillary rejected MAPILLARY_CLIENT_TOKEN — replace it in Provider Settings'; /** Shown while Mapillary rate-limits tile requests (429). */ export const RATE_LIMITED_MESSAGE = - 'Mapillary is rate-limiting coverage requests — it resumes on its own'; + 'Coverage requests are being rate-limited — they resume on their own'; /** * How far from the camera a street-zoom view may reach for coverage: a tilted * view sees to the horizon, but only the ground within this range is ranked @@ -52,8 +62,7 @@ export const SEQUENCE_IMAGES_LIMIT = 2000; export const IMAGE_CONE_SIZE_PX = 26; export const IMAGE_CONE_MIN_SPACING_M = 3; -/** Nearest-image search when the user asks to look at a place. */ -export const NEAREST_RADIUS_M = 50; +/** Images asked for in a nearest-image search (radius: NEAREST_RADIUS_M). */ export const NEAREST_LIMIT = 8; /** diff --git a/src/layers/streetLevel/providers/mapillary/provider.test.mjs b/src/layers/streetLevel/providers/mapillary/provider.test.mjs index de1c01f0c..b5ee92c18 100644 --- a/src/layers/streetLevel/providers/mapillary/provider.test.mjs +++ b/src/layers/streetLevel/providers/mapillary/provider.test.mjs @@ -4,13 +4,13 @@ import { createMapillaryProvider, mapillaryImageUrl } from './index.js'; import { sequenceIdFromPick } from './coverage.js'; import { validateProviders } from '../../registry.js'; import { PROVIDER_COLORS } from '../../policy.js'; +import { resolveFilter } from '../../filter.js'; function fakeSource({ nearest = [] } = {}) { return { hasToken: () => true, getStatus: async () => ({ configured: true }), getTile: async () => new Uint8Array(0), - getImage: async () => ({}), getSequenceImages: async () => [], nearestImages: async () => nearest, }; @@ -21,8 +21,7 @@ function fakeContext(filter = { pano: 'all', sinceMs: null }) { return { opened, services: {}, - getViewer: () => null, - getFilter: () => filter, + getFilter: typeof filter === 'function' ? filter : () => filter, isActive: () => true, notify: () => {}, actions: { @@ -41,7 +40,6 @@ test('the definition satisfies the Street Level provider contract', () => { assert.equal(def.label, 'MAPILLARY'); assert.equal(def.requiresKeyId, 'mapillary'); assert.equal(def.pickPrefix, 'mly:'); - assert.equal(def.capabilities.coverage, 'tiles'); assert.match(def.credit.html, /CC BY-SA 4\.0/); // One colour per source: Mapillary is green everywhere it draws. assert.deepEqual(Object.keys(def.colors).sort(), ['coverage', 'selected']); @@ -51,9 +49,8 @@ test('the definition satisfies the Street Level provider contract', () => { }); test('image deep links match the mapillary.com share format', () => { - const def = createMapillaryProvider({ source: fakeSource() }); assert.equal( - def.externalUrl(1814275685699406), + mapillaryImageUrl(1814275685699406), 'https://www.mapillary.com/app/?pKey=1814275685699406&focus=photo', ); assert.equal(mapillaryImageUrl(' '), 'https://www.mapillary.com/app/'); @@ -118,3 +115,23 @@ test('nearestImage picks the closest image, not the first the API returned', asy const instance = createMapillaryProvider({ source }).create(fakeContext()); assert.equal(await instance.nearestImage({ lat: 1, lon: 2 }), 'near'); }); + +test('a "since N days" window follows the clock in a long-open tab (review P3)', async (t) => { + t.mock.timers.enable({ apis: ['Date'], now: Date.UTC(2026, 9, 1) }); + const day = 86_400_000; + const capturedAt = Date.now() - day / 2; + const source = fakeSource({ + nearest: [{ id: 'recent', is_pano: false, captured_at: capturedAt }], + }); + // The core's context resolves the stored filter whenever it is asked. + const instance = createMapillaryProvider({ source }).create( + fakeContext(() => resolveFilter({ pano: 'all', sinceDays: 1 })), + ); + assert.equal(await instance.nearestImage({ lat: 1, lon: 2 }), 'recent'); + t.mock.timers.tick(2 * day); + assert.equal( + await instance.nearestImage({ lat: 1, lon: 2 }), + null, + 'two days on, the image is older than the window', + ); +}); diff --git a/src/layers/streetLevel/providers/mapillary/sequences.js b/src/layers/streetLevel/providers/mapillary/sequences.js index ea70489a7..17074fc5a 100644 --- a/src/layers/streetLevel/providers/mapillary/sequences.js +++ b/src/layers/streetLevel/providers/mapillary/sequences.js @@ -2,6 +2,7 @@ import * as Cesium from 'cesium'; import { imageConeGlyph } from '../../glyphs.js'; import { passesImageryFilter } from '../../filter.js'; import { refineHeights } from '../../groundCast.js'; +import { metresBetween } from '../../view.js'; import { COLORS, IMAGE_CONE_MIN_SPACING_M, @@ -14,14 +15,6 @@ const SPRITE_ID = 'street-level:mapillary-cones'; /** How many recently viewed sequences keep their image list in memory. */ const SEQUENCE_CACHE_SIZE = 40; -/** Approximate metres between two lon/lat points (small distances). */ -function metresBetween(a, b) { - const lat = ((a.lat + b.lat) / 2) * (Math.PI / 180); - const dx = (b.lon - a.lon) * 111_320 * Math.cos(lat); - const dy = (b.lat - a.lat) * 110_540; - return Math.hypot(dx, dy); -} - /** Normalize a graph image record into the shape the cones use. */ export function normalizeSequenceImage(record) { const coordinates = record?.geometry?.coordinates; @@ -119,8 +112,10 @@ export function createSequences({ state, source, parts }) { color: COLORS.coverage, pano: true, }); + // Resolved now, so a "since N days" window keeps up with the clock. + const filter = state.context.getFilter(); images.forEach((image, index) => { - if (!passesImageryFilter(image, state.filter)) return; + if (!passesImageryFilter(image, filter)) return; const height = heights?.[index] ?? null; collection.add({ id: `${PICK_PREFIX.image}${image.id}`, @@ -158,9 +153,10 @@ export function createSequences({ state, source, parts }) { /** Select a sequence: highlight its line and load its image cones. */ async function select(sequenceId) { if (!sequenceId || !state.viewer) return; + // Already on screen, or already on its way. if ( state.sequence.selectedId === sequenceId && - state.sequence.images.length + (state.sequence.images.length || state.sequence.abort) ) return; if (state.sequence.selectedId && state.sequence.selectedId !== sequenceId) @@ -174,9 +170,14 @@ export function createSequences({ state, source, parts }) { state.sequence.loading = false; state.sequence.images = cached; renderCones(cached); + state.context.actions.reportError(null); notify(); return; } + // The previous sequence's cones must not stay clickable under the new + // highlight while this one loads, nor after its load fails. + clearCones(); + requestRender(); const controller = new AbortController(); state.sequence.abort = controller; state.sequence.loading = true; @@ -195,11 +196,16 @@ export function createSequences({ state, source, parts }) { remember(sequenceId, images); state.sequence.images = images; renderCones(images); + state.context.actions.reportError(null); } catch (error) { - if (!controller.signal.aborted) + if (!controller.signal.aborted) { + // Nothing to show: drop the highlight, so a click asks again. + parts.coverage.recolorSequence(sequenceId, false); + state.sequence.selectedId = null; state.context.actions.reportError( error?.message || 'Sequence images unavailable', ); + } } finally { if (state.sequence.abort === controller) { state.sequence.loading = false; @@ -217,6 +223,8 @@ export function createSequences({ state, source, parts }) { state.sequence.selectedId = null; state.sequence.loading = false; clearCones(); + // An error about the sequence goes with it. + state.context.actions.reportError(null); requestRender(); notify(); } diff --git a/src/layers/streetLevel/providers/mapillary/sequences.test.mjs b/src/layers/streetLevel/providers/mapillary/sequences.test.mjs new file mode 100644 index 000000000..4ea1aeac1 --- /dev/null +++ b/src/layers/streetLevel/providers/mapillary/sequences.test.mjs @@ -0,0 +1,172 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createSequences } from './sequences.js'; + +// The cone glyphs draw on a canvas; Node has none, so any drawing is a no-op. +const noop = () => {}; +const context2d = new Proxy({}, { get: () => noop, set: () => true }); +globalThis.document ??= { + createElement: () => ({ getContext: () => context2d }), +}; + +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + +/** A billboard collection that remembers the cones on the globe. */ +function cones() { + const items = []; + return { + items, + show: true, + add(options) { + items.push(options); + return options; + }, + removeAll() { + items.length = 0; + }, + }; +} + +/** A source whose sequence lookups the test answers, one call at a time. */ +function deferredSource() { + const calls = []; + return { + calls, + getSequenceImages(sequenceId, { signal } = {}) { + return new Promise((resolve, reject) => { + calls.push({ sequenceId, signal, resolve, reject }); + }); + }, + /** The latest lookup for a sequence. */ + last(sequenceId) { + return calls.filter((call) => call.sequenceId === sequenceId).at(-1); + }, + }; +} + +/** Graph image records for a sequence, a few metres apart. */ +function records(sequenceId, count = 2) { + return Array.from({ length: count }, (_, i) => ({ + id: `${sequenceId}-${i}`, + geometry: { coordinates: [-121.49 + i * 0.001, 38.58] }, + compass_angle: 90, + captured_at: 1_700_000_000_000 + i, + })); +} + +function setup() { + const source = deferredSource(); + const colours = []; + const errors = []; + const state = { + services: {}, + viewer: {}, + context: { + notify() {}, + getFilter: () => ({ pano: 'all', sinceMs: null }), + getSurface: () => 'draped', + groundCaster: null, + meshSampler: null, + actions: { reportError: (message) => errors.push(message) }, + }, + sequence: { + selectedId: null, + images: [], + cache: new Map(), + collection: cones(), + loading: false, + abort: null, + }, + }; + const parts = { + coverage: { + recolorSequence: (id, selected) => colours.push([id, selected]), + }, + }; + const sequences = createSequences({ state, source, parts }); + const drawn = () => + state.sequence.collection.items.map((cone) => cone.id.split(':').at(-1)); + return { state, source, sequences, colours, errors, drawn }; +} + +test('a failed sequence load leaves no stale cones and can be retried (review P2-4)', async () => { + const { state, source, sequences, colours, errors, drawn } = setup(); + sequences.select('A'); + source.last('A').resolve(records('A')); + await settle(); + assert.deepEqual(drawn(), ['A-0', 'A-1']); + + sequences.select('B'); + assert.deepEqual(drawn(), [], "A's cones go while B loads"); + source.last('B').reject(new Error('Sequence images unavailable')); + await settle(); + assert.deepEqual(drawn(), [], 'nothing of A is left to click'); + assert.equal(state.sequence.loading, false); + assert.equal(state.sequence.selectedId, null, 'B is not left highlighted'); + assert.deepEqual(colours.at(-1), ['B', false]); + assert.deepEqual(errors, [null, 'Sequence images unavailable']); + + // Clicking B again asks again. + sequences.select('B'); + assert.equal(source.calls.filter((c) => c.sequenceId === 'B').length, 2); + source.last('B').resolve(records('B')); + await settle(); + assert.deepEqual(drawn(), ['B-0', 'B-1']); + assert.equal(state.sequence.selectedId, 'B'); + assert.equal(errors.at(-1), null, 'the error is cleared once B loads'); +}); + +test('a late answer for a superseded sequence neither draws nor is cached', async () => { + const { state, source, sequences, drawn } = setup(); + sequences.select('A'); + sequences.select('B'); + assert.equal(source.last('A').signal.aborted, true, 'A was cancelled'); + source.last('B').resolve(records('B')); + await settle(); + source.last('A').resolve(records('A')); + await settle(); + assert.deepEqual(drawn(), ['B-0', 'B-1']); + assert.equal(state.sequence.selectedId, 'B'); + assert.equal(state.sequence.cache.has('A'), false); + assert.equal(state.sequence.loading, false); +}); + +test('a second click on a loading sequence does not start another lookup', async () => { + const { source, sequences } = setup(); + sequences.select('A'); + sequences.select('A'); + assert.equal(source.calls.length, 1); + assert.equal(source.calls[0].signal.aborted, false); +}); + +test('a cached sequence is re-selected without a lookup', async () => { + const { state, source, sequences, drawn } = setup(); + sequences.select('A'); + source.last('A').resolve(records('A')); + await settle(); + sequences.select('B'); + source.last('B').resolve(records('B')); + await settle(); + sequences.select('A'); + assert.equal(source.calls.length, 2, 'A came from the cache'); + assert.deepEqual(drawn(), ['A-0', 'A-1'], 'drawn at once'); + assert.equal(state.sequence.loading, false); + assert.equal(state.sequence.selectedId, 'A'); +}); + +test('clearSelection cancels the lookup and uncolours the sequence', async () => { + const { state, source, sequences, colours, errors, drawn } = setup(); + sequences.select('A'); + sequences.clearSelection(); + assert.equal(source.last('A').signal.aborted, true); + assert.deepEqual(colours, [ + ['A', true], + ['A', false], + ]); + assert.equal(state.sequence.selectedId, null); + assert.equal(state.sequence.loading, false); + assert.equal(errors.at(-1), null, 'an error about it goes too'); + source.last('A').resolve(records('A')); + await settle(); + assert.deepEqual(drawn(), [], 'the late answer draws nothing'); +}); diff --git a/src/layers/streetLevel/providers/mapillary/source.js b/src/layers/streetLevel/providers/mapillary/source.js index 2ad2d9b20..d140c0487 100644 --- a/src/layers/streetLevel/providers/mapillary/source.js +++ b/src/layers/streetLevel/providers/mapillary/source.js @@ -117,10 +117,6 @@ export function createMapillarySource({ return new Uint8Array(await response.arrayBuffer()); }, - getImage(id, { signal, fields = IMAGE_FIELDS } = {}) { - return graph(String(id), { fields }, { signal }); - }, - async getSequenceImages( sequenceId, { signal, limit = SEQUENCE_IMAGES_LIMIT } = {}, diff --git a/src/layers/streetLevel/providers/mapillary/source.test.mjs b/src/layers/streetLevel/providers/mapillary/source.test.mjs index e9e16ba19..c4ad09cba 100644 --- a/src/layers/streetLevel/providers/mapillary/source.test.mjs +++ b/src/layers/streetLevel/providers/mapillary/source.test.mjs @@ -83,7 +83,7 @@ test('graph calls without a token fail fast as keyRequired without a request', a const { calls, fetchImpl } = fakeFetch(() => jsonResponse({})); const source = createMapillarySource({ token: '', fetchImpl }); await assert.rejects( - () => source.getImage('123'), + () => source.getSequenceImages('123'), (error) => error instanceof MapillarySourceError && error.keyRequired === true, ); @@ -96,7 +96,7 @@ test('graph API errors carry the upstream message and status', async () => { ); const source = createMapillarySource({ token: 'MLY|1|abc', fetchImpl }); await assert.rejects( - () => source.getImage('999'), + () => source.getSequenceImages('999'), (error) => error instanceof MapillarySourceError && error.status === 400 && diff --git a/src/layers/streetLevel/providers/mapillary/viewer.js b/src/layers/streetLevel/providers/mapillary/viewer.js index 41a8f0c1b..543ee2da0 100644 --- a/src/layers/streetLevel/providers/mapillary/viewer.js +++ b/src/layers/streetLevel/providers/mapillary/viewer.js @@ -11,7 +11,8 @@ export function createMapillaryViewer({ source, render } = {}) { let Library = null; let container = null; let pendingOpen = null; - let prewarming = false; + /** The prewarm in flight, if any. */ + let prewarming = null; /** In-flight viewer construction, so pre-warm and open never build two. */ let creating = null; /** Bumped by `unmount`, so a construction it overtook builds nothing. */ @@ -167,6 +168,13 @@ export function createMapillaryViewer({ source, render } = {}) { close() { pendingOpen = null; current = null; + // A playing sequence would keep stepping through (and downloading) + // images in the hidden viewer. + try { + viewer?.getComponent('sequence')?.stop(); + } catch { + /* sequence component not available */ + } }, unmount() { @@ -190,16 +198,22 @@ export function createMapillaryViewer({ source, render } = {}) { * opening one only costs the image download. Safe to call repeatedly. */ async prewarm(host) { - if (prewarming || !host) return; - prewarming = true; - try { - await ensureLibrary(); - if (!viewer) await ensureViewer(host); - } catch { - /* the real open reports errors */ - } finally { - prewarming = false; - } + if (!host) return; + // A prewarm already in flight may give up (the layer went off and on + // while the library downloaded): wait for it, then build if it did not. + if (prewarming) await prewarming; + if (viewer) return; + const run = (async () => { + try { + await ensureLibrary(); + if (!viewer) await ensureViewer(host); + } catch { + /* the real open reports errors */ + } + })(); + prewarming = run; + await run; + if (prewarming === run) prewarming = null; }, /** Show the whole image ('letterbox') or crop it to the frame ('fill'). */ diff --git a/src/layers/streetLevel/registry.js b/src/layers/streetLevel/registry.js index 86d59029b..0f26f8366 100644 --- a/src/layers/streetLevel/registry.js +++ b/src/layers/streetLevel/registry.js @@ -8,15 +8,12 @@ * @property {string|null} requiresKeyId Key-setup id the provider needs, or null when keyless. * @property {string} pickPrefix Every primitive id the provider creates starts with it. * @property {{coverage: string}} colors The source's one colour (policy.js PROVIDER_COLORS): chip, lines, points, cones and legend. - * @property {{key: string, html: string}} credit On-globe attribution while the provider is active. - * @property {{coverage: 'tiles'|'bbox'|'none', sequences: boolean, pano: boolean, capturedAt: boolean, creator: boolean, follow: boolean}} capabilities - * @property {(imageId: string) => string} externalUrl Deep link to the image on the provider's site. + * @property {{html: string}} credit On-globe attribution while the provider is active. * @property {(context: ProviderContext) => ProviderInstance} create * * @typedef {object} ProviderContext Handed to `create()` once by the core. * @property {object} services Scene services: picking, input, render, sprites, ground, terrain. - * @property {() => object|null} getViewer The Cesium viewer once the layer is initialised. - * @property {() => {pano: string, sinceMs: number|null}} getFilter The resolved imagery filter. + * @property {() => {pano: string, sinceMs: number|null}} getFilter The imagery filter resolved against now: read it whenever filtering, so a "since N days" window keeps up with the clock. * @property {() => boolean} isActive Layer enabled and this provider switched on. * @property {() => 'draped'|'terrain'} getSurface 'terrain' on Google 3D at street zoom: draw on the bare earth, not the mesh top. * @property {GroundCaster|null} groundCaster Bare-earth heights for terrain mode (groundCast.js); null without a terrain service. @@ -31,7 +28,7 @@ * @property {() => void} deactivate Detach, clear coverage and selection, hide. * @property {(viewer: object) => void} destroy * @property {() => void} refreshCoverage - * @property {(filter: {pano: string, sinceMs: number|null}) => void} setFilter + * @property {(filter: {pano: string, sinceMs: number|null}) => void} setFilter The filter changed: redraw (reading `getFilter()`). * @property {(mode: 'draped'|'terrain') => void} [setSurface] Redraw coverage and cones for the new surface mode. * @property {() => {count: number, zoom: number|null, kind: string|null, loading: boolean, hint: string, error: string|null, keyRequired: boolean}} coverageStats * @property {(pickId: string) => boolean} handlePick The id carries the provider's own prefix. @@ -45,7 +42,6 @@ * @property {(points: Array<[number, number]>, options?: {signal?: AbortSignal}) => Promise} prepare Fetch the heights around [lon, lat] points; true when all are known. * @property {(lines: Array>, options?: {signal?: AbortSignal}) => Promise} prepareLines * @property {(lon: number, lat: number) => number|null} groundAt Cached bare-earth ellipsoidal height. - * @property {(lon: number, lat: number) => number|null} heightAt groundAt plus the overlay lift. * @property {(coords: Array<[number, number]>) => Array|null} castLine Densified [lon, lat, height, ...] or null. * * @typedef {object} ViewerAdapter @@ -69,7 +65,7 @@ * @property {number|null} capturedAt Epoch milliseconds. * @property {string|null} creator * @property {string|null} sequenceId - * @property {string} externalUrl + * @property {string} externalUrl Deep link to the image on the provider's site. */ const REQUIRED = Object.freeze([ @@ -79,8 +75,6 @@ const REQUIRED = Object.freeze([ 'pickPrefix', 'colors', 'credit', - 'capabilities', - 'externalUrl', 'create', ]); @@ -110,10 +104,6 @@ export function validateProviders(providers) { throw new TypeError( `Street Level provider ${label}: create is not a function`, ); - if (typeof provider.externalUrl !== 'function') - throw new TypeError( - `Street Level provider ${label}: externalUrl is not a function`, - ); if (typeof provider.pickPrefix !== 'string' || !provider.pickPrefix) throw new TypeError(`Street Level provider ${label} needs a pick prefix`); for (const other of prefixes) @@ -124,7 +114,7 @@ export function validateProviders(providers) { throw new TypeError( `Street Level provider ${label}: pick prefix ${provider.pickPrefix} overlaps ${other}`, ); - if (!provider.credit?.key || !provider.credit?.html) + if (!provider.credit?.html) throw new TypeError(`Street Level provider ${label} needs a credit`); if ( typeof provider.colors?.coverage !== 'string' || diff --git a/src/layers/streetLevel/registry.test.mjs b/src/layers/streetLevel/registry.test.mjs index 009b3c576..278bf5ffa 100644 --- a/src/layers/streetLevel/registry.test.mjs +++ b/src/layers/streetLevel/registry.test.mjs @@ -9,9 +9,7 @@ const def = (overrides = {}) => ({ requiresKeyId: 'mapillary', pickPrefix: 'mly:', colors: { coverage: '#05cb63' }, - credit: { key: 'mapillary', html: 'Mapillary' }, - capabilities: { coverage: 'tiles', sequences: true }, - externalUrl: (id) => `https://example.test/${id}`, + credit: { html: 'Mapillary' }, create: () => ({}), ...overrides, }); @@ -42,8 +40,6 @@ test('every required field is checked by name', () => { 'pickPrefix', 'colors', 'credit', - 'capabilities', - 'externalUrl', 'create', ]) assert.throws( @@ -52,10 +48,7 @@ test('every required field is checked by name', () => { key, ); assert.throws(() => validateProviders([def({ create: 1 })]), /create/); - assert.throws( - () => validateProviders([def({ credit: { key: 'x' } })]), - /credit/, - ); + assert.throws(() => validateProviders([def({ credit: {} })]), /credit/); assert.throws( () => validateProviders([def({ colors: { pano: '#f0f' } })]), /coverage colour/, diff --git a/src/layers/streetLevel/selection.js b/src/layers/streetLevel/selection.js index 6273a4c4c..034c76ab5 100644 --- a/src/layers/streetLevel/selection.js +++ b/src/layers/streetLevel/selection.js @@ -52,7 +52,6 @@ export function createSelection({ state, parts }) { let hoverPosition = null; let hoverLastPickAt = -Infinity; let hoverTimer = null; - let cameraMoving = false; let pointerButtons = 0; let removeHoverWatchers = null; @@ -71,8 +70,8 @@ export function createSelection({ state, parts }) { const viewer = state.viewer; // Switched off (or torn down) since the frame was queued. if (!viewer || !state.enabled || !state.clickHandler) return; - // Orbiting, or a drag in progress: the pointer is the camera's. - if (cameraMoving || pointerButtons !== 0) return; + // A drag in progress: the pointer is the camera's. + if (pointerButtons !== 0) return; const canvas = viewer.scene?.canvas; if (!canvas || viewer.isDestroyed?.()) return; hoverLastPickAt = Date.now(); @@ -98,6 +97,11 @@ export function createSelection({ state, parts }) { const viewer = state.viewer; if (!viewer || !state.enabled) return; hoverPosition = movement.endPosition; + queueHoverPick(); + } + + /** Pick `hoverPosition` on a coming frame, within the throttle. */ + function queueHoverPick() { if (hoverQueued) return; hoverQueued = true; const wait = hoverLastPickAt + HOVER_PICK_INTERVAL_MS - Date.now(); @@ -111,7 +115,12 @@ export function createSelection({ state, parts }) { }, wait); } - /** Track what makes a hover pick pointless: camera motion, held buttons. */ + /** + * Track held buttons (a drag picks nothing) and camera stops. A moving + * camera does not stop hover: an orbiting or tracking camera never stops, + * and the throttle already bounds the cost. When the camera comes to rest + * the pointer is picked once more, since what was under it has moved. + */ function watchHover(viewer) { const canvas = viewer.scene.canvas; const camera = viewer.camera; @@ -120,16 +129,12 @@ export function createSelection({ state, parts }) { }; for (const type of ['pointerdown', 'pointermove', 'pointerup']) canvas.addEventListener?.(type, onButtons); - const removeStart = camera?.moveStart?.addEventListener(() => { - cameraMoving = true; - }); const removeEnd = camera?.moveEnd?.addEventListener(() => { - cameraMoving = false; + if (hoverPosition) queueHoverPick(); }); removeHoverWatchers = () => { for (const type of ['pointerdown', 'pointermove', 'pointerup']) canvas.removeEventListener?.(type, onButtons); - removeStart?.(); removeEnd?.(); }; } @@ -140,8 +145,8 @@ export function createSelection({ state, parts }) { clearTimeout(hoverTimer); hoverTimer = null; hoverQueued = false; + hoverPosition = null; hoverLastPickAt = -Infinity; - cameraMoving = false; pointerButtons = 0; } diff --git a/src/layers/streetLevel/selection.test.mjs b/src/layers/streetLevel/selection.test.mjs index eb680cd43..be9d739ce 100644 --- a/src/layers/streetLevel/selection.test.mjs +++ b/src/layers/streetLevel/selection.test.mjs @@ -18,6 +18,8 @@ function harness() { globalThis.requestAnimationFrame = (task) => frames.push(task); globalThis.document = new EventTarget(); const picks = []; + /** What the pointer is over: a line this layer owns, or nothing. */ + const scene = { under: null }; const canvas = Object.assign(new EventTarget(), { style: {}, // Keep Cesium's handler on the canvas; there is no real document here. @@ -30,7 +32,7 @@ function harness() { canvas, pick(position) { picks.push(position); - return null; + return scene.under && { id: scene.under }; }, }, camera, @@ -45,7 +47,7 @@ function harness() { const selection = createSelection({ state, parts: { - router: { ownsPick: () => false, resolve: () => null }, + router: { ownsPick: (id) => id === 'mly:seq:1', resolve: () => null }, hasSelectedSequence: () => false, clearSequences() {}, }, @@ -56,6 +58,7 @@ function harness() { ); return { state, + scene, canvas, camera, picks, @@ -102,26 +105,53 @@ test('hover picks at most every 120 ms through a burst of moves, ending on the l } }); -test('hover does not pick while the camera moves or a mouse button is held', () => { +test('hover does not pick while a mouse button is held', () => { const h = harness(); try { - h.camera.moveStart.raiseEvent(); + h.buttons('pointerdown', 1); h.move(1); h.wait(200); - assert.equal(h.picks.length, 0, 'camera in motion'); - h.camera.moveEnd.raiseEvent(); + assert.equal(h.picks.length, 0, 'a drag in progress'); + h.buttons('pointerup', 0); h.move(2); h.wait(200); - assert.equal(h.picks.length, 1, 'picks again once the camera rests'); + assert.equal(h.picks.length, 1); + } finally { + h.restore(); + } +}); - h.buttons('pointerdown', 1); - h.move(3); +test('a camera that never stops (orbit, tracking) keeps hover throttled, not off (review P3)', () => { + const h = harness(); + try { + // An orbiting or tracking camera raises moveStart and never moveEnd. + h.camera.moveStart.raiseEvent(); + for (let i = 0; i < 60; i++) { + h.move(i); + h.wait(16); + } + assert.ok(h.picks.length >= 7, `${h.picks.length} picks in ~1 s`); + assert.ok(h.picks.length <= 9, `${h.picks.length} picks in ~1 s`); + } finally { + h.restore(); + } +}); + +test('when the camera stops, the pointer is picked again so the cursor cannot stick (review P3)', () => { + const h = harness(); + try { + h.scene.under = 'mly:seq:1'; + h.move(5); h.wait(200); - assert.equal(h.picks.length, 1, 'a drag in progress'); - h.buttons('pointerup', 0); - h.move(4); + assert.equal(h.canvas.style.cursor, 'pointer', 'over a line'); + // The camera moves the line out from under a still pointer. + h.camera.moveStart.raiseEvent(); + h.scene.under = null; + h.camera.moveEnd.raiseEvent(); h.wait(200); - assert.equal(h.picks.length, 2); + assert.equal(h.picks.length, 2, 'one pick when the camera rests'); + assert.deepEqual(h.picks.at(-1), { x: 5, y: 10 }); + assert.equal(h.canvas.style.cursor, '', 'the pointer cursor is gone'); } finally { h.restore(); } diff --git a/src/layers/streetLevel/state.js b/src/layers/streetLevel/state.js index da332cf32..db07e522c 100644 --- a/src/layers/streetLevel/state.js +++ b/src/layers/streetLevel/state.js @@ -1,5 +1,32 @@ import { FILTER_DEFAULT } from './policy.js'; +/** + * The open image's fields of `state.street`, as they are with no image open. + * Closing the viewer resets exactly these; the panel host, the render mode + * and whether following is available outlive any one image. + */ +export function freshStreet() { + return { + open: false, + follow: false, + providerId: null, + providerName: null, + providerLabel: null, + imageId: null, + position: null, + bearing: null, + tilt: null, + altitude: null, + isPano: false, + capturedAt: null, + sequenceId: null, + creator: null, + externalUrl: null, + loading: false, + error: null, + }; +} + /** Mutable core state, created once per layer instance. */ export function createState({ services }) { return { @@ -17,27 +44,11 @@ export function createState({ services }) { street: { host: null, - open: false, - follow: false, /** Whether the active map stack allows following (Google 3D only). */ followAvailable: false, - providerId: null, - providerName: null, - providerLabel: null, - imageId: null, - position: null, - bearing: null, - tilt: null, - altitude: null, - isPano: false, - capturedAt: null, - sequenceId: null, - creator: null, - externalUrl: null, - loading: false, - error: null, /** 'letterbox' shows the whole image; 'fill' crops it to the frame. */ renderMode: 'letterbox', + ...freshStreet(), }, /** 'terrain' on Google 3D at street zoom (overlays on the bare earth), else 'draped'. */ diff --git a/src/layers/streetLevel/tileMath.js b/src/layers/streetLevel/tileMath.js index 02eed1aa3..2267aa21c 100644 --- a/src/layers/streetLevel/tileMath.js +++ b/src/layers/streetLevel/tileMath.js @@ -9,23 +9,32 @@ function clampLat(lat) { return Math.max(-MAX_LAT, Math.min(MAX_LAT, lat)); } +/** Fractional tile column of a longitude in [-180, 180] at zoom z. */ +function tileXAt(lon, z) { + return ((lon + 180) / 360) * 2 ** z; +} + +/** Fractional tile row of a latitude at zoom z. */ +function tileYAt(lat, z) { + const rad = (clampLat(lat) * Math.PI) / 180; + return ( + ((1 - Math.log(Math.tan(rad) + 1 / Math.cos(rad)) / Math.PI) / 2) * 2 ** z + ); +} + /** Tile column for a longitude at zoom z. */ export function lonToTileX(lon, z) { const n = 2 ** z; // 180° is the east edge of the last column, not the west edge of the first. if (lon >= 180) return n - 1; const wrapped = ((((lon + 180) % 360) + 360) % 360) - 180; - return Math.min(n - 1, Math.max(0, Math.floor(((wrapped + 180) / 360) * n))); + return Math.min(n - 1, Math.max(0, Math.floor(tileXAt(wrapped, z)))); } /** Tile row for a latitude at zoom z. */ export function latToTileY(lat, z) { const n = 2 ** z; - const rad = (clampLat(lat) * Math.PI) / 180; - const y = Math.floor( - ((1 - Math.log(Math.tan(rad) + 1 / Math.cos(rad)) / Math.PI) / 2) * n, - ); - return Math.min(n - 1, Math.max(0, y)); + return Math.min(n - 1, Math.max(0, Math.floor(tileYAt(lat, z)))); } /** West longitude of tile column x at zoom z. */ @@ -73,45 +82,18 @@ export function normalizeBbox(input) { return { west, south, east, north }; } -/** Area of a bbox in square degrees (Mapillary's bbox limit unit). */ -export function bboxAreaDeg2(bbox) { - const box = normalizeBbox(bbox); - return box ? (box.east - box.west) * (box.north - box.south) : 0; -} - -/** - * Number of tiles at zoom z covering a bbox, without allocating them. - */ -export function countTilesForBbox(bbox, z) { - const box = normalizeBbox(bbox); - if (!box) return 0; - const x0 = lonToTileX(box.west, z); - const x1 = lonToTileX(box.east, z); - const y0 = latToTileY(box.north, z); - const y1 = latToTileY(box.south, z); - return (x1 - x0 + 1) * (y1 - y0 + 1); -} - /** * Squared distance (in tiles) from tile (x, y)'s centre to the segment * `from` → `to` at zoom z, measured the short way round the date line. */ function segmentDistance2(tile, from, to, z) { const n = 2 ** z; - /** Unfloored tile coordinates of a point. */ - const tileX = (lon) => ((lon + 180) / 360) * n; - const tileY = (lat) => { - const rad = (clampLat(lat) * Math.PI) / 180; - return ( - ((1 - Math.log(Math.tan(rad) + 1 / Math.cos(rad)) / Math.PI) / 2) * n - ); - }; const px = tile.x + 0.5; const py = tile.y + 0.5; - const ax = tileX(from.lon); - const ay = tileY(from.lat); - let bx = to ? tileX(to.lon) : ax; - const by = to ? tileY(to.lat) : ay; + const ax = tileXAt(from.lon, z); + const ay = tileYAt(from.lat, z); + let bx = to ? tileXAt(to.lon, z) : ax; + const by = to ? tileYAt(to.lat, z) : ay; if (bx - ax > n / 2) bx -= n; else if (ax - bx > n / 2) bx += n; let best = Infinity; @@ -145,62 +127,60 @@ function segmentDistance2(tile, from, to, z) { * @returns {{tiles: Array<{x:number,y:number,z:number}>, truncated: boolean, total: number}} */ export function tilesForBbox(bbox, z, { limit = Infinity, focus = null } = {}) { - // A box across the date line comes as west > east: split it at ±180°. + const n = 2 ** z; + const tiles = []; + let centre; if (Array.isArray(bbox) && Number(bbox[0]) > Number(bbox[2])) { + // A box across the date line comes as west > east: split it at ±180°. const [west, south, east, north] = bbox.map(Number); - const n = 2 ** z; - const tiles = [ - ...tilesForBbox([west, south, 180, north], z).tiles, - ...tilesForBbox([-180, south, east, north], z).tiles, - ]; - const x0 = lonToTileX(west, z); - const x1 = lonToTileX(east, z) + n; - const cx = ((x0 + x1) / 2) % n; - const cy = (latToTileY(north, z) + latToTileY(south, z)) / 2; - const dx = (x) => Math.min(Math.abs(x - cx), n - Math.abs(x - cx)); - if (focus?.from) - tiles.sort( - (a, b) => - segmentDistance2(a, focus.from, focus.to, z) - - segmentDistance2(b, focus.from, focus.to, z), - ); - else - tiles.sort( - (a, b) => - dx(a.x) ** 2 + (a.y - cy) ** 2 - (dx(b.x) ** 2 + (b.y - cy) ** 2), - ); - const truncated = tiles.length > limit; - return { - tiles: truncated ? tiles.slice(0, limit) : tiles, - truncated, - total: tiles.length, + for (const half of [ + [west, south, 180, north], + [-180, south, east, north], + ]) { + const box = normalizeBbox(half); + if (box) tiles.push(...tileGrid(box, z).tiles); + } + centre = { + x: ((lonToTileX(west, z) + lonToTileX(east, z) + n) / 2) % n, + y: (latToTileY(north, z) + latToTileY(south, z)) / 2, }; + } else { + const box = normalizeBbox(bbox); + if (!box) return { tiles: [], truncated: false, total: 0 }; + const grid = tileGrid(box, z); + tiles.push(...grid.tiles); + centre = { x: (grid.x0 + grid.x1) / 2, y: (grid.y0 + grid.y1) / 2 }; } - const box = normalizeBbox(bbox); - if (!box) return { tiles: [], truncated: false, total: 0 }; + const distance2 = focus?.from + ? (tile) => segmentDistance2(tile, focus.from, focus.to, z) + : (tile) => { + // Measured the short way round the date line. + const dx = Math.abs(tile.x - centre.x); + return Math.min(dx, n - dx) ** 2 + (tile.y - centre.y) ** 2; + }; + // Each tile's distance is computed once; ties keep their row order. + const ranked = tiles + .map((tile) => ({ tile, d: distance2(tile) })) + .sort((a, b) => a.d - b.d) + .map(({ tile }) => tile); + const truncated = ranked.length > limit; + return { + tiles: truncated ? ranked.slice(0, limit) : ranked, + truncated, + total: ranked.length, + }; +} + +/** The tiles at zoom z in a normalized box, row by row, and their range. */ +function tileGrid(box, z) { const x0 = lonToTileX(box.west, z); const x1 = lonToTileX(box.east, z); const y0 = latToTileY(box.north, z); const y1 = latToTileY(box.south, z); - const total = (x1 - x0 + 1) * (y1 - y0 + 1); - const cx = (x0 + x1) / 2; - const cy = (y0 + y1) / 2; const tiles = []; for (let y = y0; y <= y1; y++) for (let x = x0; x <= x1; x++) tiles.push({ x, y, z }); - if (focus?.from) - tiles.sort( - (a, b) => - segmentDistance2(a, focus.from, focus.to, z) - - segmentDistance2(b, focus.from, focus.to, z), - ); - else - tiles.sort( - (a, b) => - (a.x - cx) ** 2 + (a.y - cy) ** 2 - ((b.x - cx) ** 2 + (b.y - cy) ** 2), - ); - const truncated = tiles.length > limit; - return { tiles: truncated ? tiles.slice(0, limit) : tiles, truncated, total }; + return { tiles, x0, x1, y0, y1 }; } /** diff --git a/src/layers/streetLevel/tileMath.test.mjs b/src/layers/streetLevel/tileMath.test.mjs index 626c147b3..ff367a280 100644 --- a/src/layers/streetLevel/tileMath.test.mjs +++ b/src/layers/streetLevel/tileMath.test.mjs @@ -1,8 +1,6 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { - bboxAreaDeg2, - countTilesForBbox, coverageZoomForHeight, overviewZoomForHeight, latToTileY, @@ -43,12 +41,11 @@ test('normalizeBbox orders and clamps coordinates', () => { assert.equal(normalizeBbox([1, 2, Number.NaN, 3]), null); }); -test('city-scale boxes exceed the Mapillary bbox limit and need tiles', () => { +test('city-scale boxes span many z14 tiles', () => { const sacramento = [-121.56, 38.44, -121.36, 38.69]; - assert.ok(bboxAreaDeg2(sacramento) > 0.01); - assert.equal(countTilesForBbox(sacramento, 14), 160); + assert.equal(tilesForBbox(sacramento, 14).total, 160); const detroit = [-83.29, 42.25, -82.91, 42.45]; - assert.equal(countTilesForBbox(detroit, 14), 234); + assert.equal(tilesForBbox(detroit, 14).total, 234); }); test('tilesForBbox orders from the centre outwards and honours the cap', () => { diff --git a/src/layers/streetLevel/view.js b/src/layers/streetLevel/view.js index 662644940..45af1ecb2 100644 --- a/src/layers/streetLevel/view.js +++ b/src/layers/streetLevel/view.js @@ -1,5 +1,30 @@ import * as Cesium from 'cesium'; +/** + * Approximate metres between two {lon, lat} points: equirectangular, good + * for the street-scale distances the layer measures. + */ +export function metresBetween(a, b) { + const lat = (((a.lat + b.lat) / 2) * Math.PI) / 180; + return Math.hypot( + (b.lon - a.lon) * 111_320 * Math.cos(lat), + (b.lat - a.lat) * 110_540, + ); +} + +/** + * Run low-priority work when the browser is idle, within `timeout` ms; a + * browser without idle callbacks (and Node) runs it a frame later. The task + * gets the idle deadline, or null. + * @param {(deadline: IdleDeadline|null) => void} task + * @param {number} timeout + */ +export function whenIdle(task, timeout) { + if (typeof globalThis.requestIdleCallback === 'function') + globalThis.requestIdleCallback(task, { timeout }); + else setTimeout(() => task(null), 16); +} + /** * Height of the surface under the camera above the ellipsoid, in metres. The * globe's terrain answers when the globe is shown; Google 3D hides the globe, diff --git a/src/layers/streetLevel/viewerHost.js b/src/layers/streetLevel/viewerHost.js index fabe67468..19cf80160 100644 --- a/src/layers/streetLevel/viewerHost.js +++ b/src/layers/streetLevel/viewerHost.js @@ -1,3 +1,5 @@ +import { freshStreet } from './state.js'; + /** * Own the one viewer element in the panel and whichever provider's viewer * adapter is mounted in it. Poses the adapter emits become `state.street`, @@ -107,16 +109,20 @@ export function createViewerHost({ state, parts }) { } /** - * Open an image from one provider. Returns once the first pose is in; - * later poses stream through `state.street` as the user navigates. + * Open an image from one provider. Resolves once the first pose is in + * (true) or the open failed or was overtaken (false); later poses stream + * through `state.street` as the user navigates. + * @returns {Promise} */ async function open(providerId, imageId, { frame = true } = {}) { const entry = providerEntry(providerId); - if (!entry || !imageId) return; + if (!entry || !imageId) return false; if (!state.street.host) { + // A nearest-image lookup may have set it; nothing is loading now. + state.street.loading = false; state.street.error = 'Open the Street Level panel to view imagery'; notify(); - return; + return false; } const seq = ++openSeq; const current = () => seq === openSeq; @@ -131,9 +137,9 @@ export function createViewerHost({ state, parts }) { notify(); try { const adapter = await mount(entry); - if (!current()) return; + if (!current()) return false; await adapter.open(String(imageId)); - if (!current()) return; + if (!current()) return false; if (frame && !state.street.follow) parts.follow.lookAtPosition(); } catch (error) { if (current()) @@ -142,32 +148,16 @@ export function createViewerHost({ state, parts }) { if (current()) state.street.loading = false; notify(); } - if (current() && !state.street.error) selectCurrentSequence(); + if (!current() || state.street.error) return false; + selectCurrentSequence(); + return true; } /** Close the image; the mounted adapter stays warm for the next open. */ function close() { openSeq++; active?.adapter.close(); - Object.assign(state.street, { - open: false, - follow: false, - providerId: null, - providerName: null, - providerLabel: null, - imageId: null, - position: null, - bearing: null, - tilt: null, - altitude: null, - isPano: false, - capturedAt: null, - sequenceId: null, - creator: null, - externalUrl: null, - loading: false, - error: null, - }); + Object.assign(state.street, freshStreet()); parts.marker.clear(); notify(); } @@ -248,6 +238,5 @@ export function createViewerHost({ state, parts }) { setRenderMode, prewarm, unmount, - activeProviderId: () => active?.id || null, }; } diff --git a/src/layers/streetLevel/viewerHost.test.mjs b/src/layers/streetLevel/viewerHost.test.mjs index 61717aca9..f50f02f9c 100644 --- a/src/layers/streetLevel/viewerHost.test.mjs +++ b/src/layers/streetLevel/viewerHost.test.mjs @@ -126,10 +126,13 @@ test('unmounting while a mount is in flight does not leave it active', async () const opening = host.open('mapillary', 'a'); host.unmount(); await opening; - assert.equal(host.activeProviderId(), null); assert.equal(adapter.calls.listeners, 0); assert.equal(adapter.calls.unmount, 1); assert.equal(state.street.open, false); + // Not left active: the next open mounts again. + await host.open('mapillary', 'b'); + assert.equal(adapter.calls.mount, 2); + assert.equal(state.street.imageId, 'b'); }); test('a prewarmed viewer is released when the layer goes off, or at once if it went off mid-load', async () => { @@ -215,12 +218,19 @@ test('switching the layer off and on during a cold first open keeps the shared v /** MapillaryJS, counting live viewers (each holds a WebGL context). */ function fakeLibrary() { const gate = deferred(); - const viewers = { created: 0, live: 0 }; + const viewers = { created: 0, live: 0, instances: [] }; class Viewer { constructor() { viewers.created++; viewers.live++; + viewers.instances.push(this); this.removed = false; + this.playback = { stops: 0 }; + } + /** The sequence component's play/stop API, as MapillaryJS 4 exposes it. */ + getComponent(name) { + if (name !== 'sequence') return undefined; + return { stop: () => this.playback.stops++ }; } on() {} async getPosition() { @@ -281,3 +291,30 @@ test('a Mapillary viewer unmounted mid-download is never built; the next mount b adapter.unmount(); assert.equal(viewers.live, 0, 'nothing left holding a WebGL context'); }); + +test('a prewarm that gave up when the layer went off and on mid-download is followed by one that builds', async () => { + const { gate, viewers } = fakeLibrary(); + const adapter = createMapillaryViewer({ source: { token: 't' } }); + const element = {}; + const first = adapter.prewarm(element); + adapter.unmount(); // layer off while MapillaryJS downloads + const second = adapter.prewarm(element); // layer on again + gate.resolve(); + await Promise.all([first, second]); + assert.equal(viewers.created, 1, 'the second prewarm built the viewer'); + assert.equal(viewers.live, 1); + adapter.unmount(); +}); + +test('closing the photo stops sequence playback in the hidden viewer (review P2-6)', async () => { + const { gate, viewers } = fakeLibrary(); + gate.resolve(); + const adapter = createMapillaryViewer({ source: { token: 't' } }); + const { host } = harness(adapter); + await host.open('mapillary', 'img1'); + const [viewer] = viewers.instances; + assert.equal(viewer.playback.stops, 0); + host.close(); + assert.equal(viewer.playback.stops, 1, 'playback stopped with the photo'); + assert.equal(viewers.live, 1, 'the viewer itself stays warm'); +});