diff --git a/src/layers/streetLevel/index.js b/src/layers/streetLevel/index.js index c7170f05c..6384b9217 100644 --- a/src/layers/streetLevel/index.js +++ b/src/layers/streetLevel/index.js @@ -108,7 +108,9 @@ export function createStreetLevelLayer({ getSurface: () => state.surface, notify, actions: { - openImage: (imageId) => openImage(entry.def.id, imageId), + /** `{frame: false}` opens it without moving the camera. */ + openImage: (imageId, options) => + openImage(entry.def.id, imageId, options), /** This provider's image nearest a point the user picked on the map. */ openNearest: (point) => layer.openNearest(point, { @@ -345,6 +347,8 @@ export function createStreetLevelLayer({ for (const other of state.providers.values()) if (other !== entry && other.on) switchProvider(other, false); switchProvider(entry, next); + // A failed lookup's message was about the source just switched away. + if (!state.street.open) state.street.error = null; notify(); return true; } @@ -414,6 +418,7 @@ export function createStreetLevelLayer({ error: stats.error || null, color: entry.def.colors.coverage, coverageLabel: entry.def.coverageLabel || null, + highlightsSequence: entry.def.highlightsSequence !== false, groundClick: entry.def.groundClick === true, filters: [...filtersOf(entry.def)], }; diff --git a/src/layers/streetLevel/index.test.mjs b/src/layers/streetLevel/index.test.mjs index 5ae19c2d0..ccc8b9dfe 100644 --- a/src/layers/streetLevel/index.test.mjs +++ b/src/layers/streetLevel/index.test.mjs @@ -818,3 +818,23 @@ test('an empty provider composition enables and tears down without acquiring ima layer.enable(viewer); assert.deepEqual(layer.getUIState().providers, []); }); + +test('a failed lookup’s error goes when another source is lit; a provider can open an image without framing', async (t) => { + const failing = fakeProvider({ + nearestImage: async () => { + throw new Error('KartaView did not answer in time — try again'); + }, + }); + const other = fakeProvider({ id: 'other', pickPrefix: 'oth:' }); + const { layer, viewer } = await enabledLayer(t, [failing, other]); + layer.attachViewerHost({}); + await layer.openNearest({ lat: 1, lon: 1 }); + assert.match(layer.getUIState().street.error, /did not answer/); + layer.setProviderEnabled('other', true); + await settle(); + assert.equal(layer.getUIState().street.error, null); + // A provider's own open (KartaView's ‹ ›) can leave the camera alone. + await other.context().actions.openImage('img-1', { frame: false }); + assert.deepEqual(other.calls.open, ['img-1']); + assert.equal(viewer.flights.started, 0); +}); diff --git a/src/layers/streetLevel/providers/kartaview/decode.js b/src/layers/streetLevel/providers/kartaview/decode.js index 2c8687eba..d670f4ca2 100644 --- a/src/layers/streetLevel/providers/kartaview/decode.js +++ b/src/layers/streetLevel/providers/kartaview/decode.js @@ -9,8 +9,11 @@ * @returns {{sequences: Array<{id: string, parts: Array>, capturedAt: number, isPano: boolean}>, overview: Array}} */ export function decodeCoverageTile(payload, { x, y, z }) { + // Not cached as empty: an unreadable tile is asked for again. + if (!Array.isArray(payload?.lines)) + throw new Error('KartaView sent an unreadable coverage tile'); const segments = []; - for (const line of Array.isArray(payload?.lines) ? payload.lines : []) { + for (const line of payload.lines) { const points = []; for (const point of Array.isArray(line?.track) ? line.track : []) { const lon = Number(point?.lng); diff --git a/src/layers/streetLevel/providers/kartaview/decode.test.mjs b/src/layers/streetLevel/providers/kartaview/decode.test.mjs index c1ab57f3a..c84855e06 100644 --- a/src/layers/streetLevel/providers/kartaview/decode.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/decode.test.mjs @@ -71,5 +71,17 @@ test('bad points, repeats and one-point tracks are skipped; an empty tile has no [2, 2], ]); assert.deepEqual(decodeCoverageTile({ lines: [] }, TILE).sequences, []); - assert.deepEqual(decodeCoverageTile(null, TILE).sequences, []); +}); + +test('a tile that is not a line list fails, so it is asked for again rather than kept empty', () => { + for (const payload of [ + null, + {}, + { lines: 'nope' }, + { status: { apiCode: 690 } }, + ]) + assert.throws( + () => decodeCoverageTile(payload, TILE), + /unreadable coverage tile/, + ); }); diff --git a/src/layers/streetLevel/providers/kartaview/index.js b/src/layers/streetLevel/providers/kartaview/index.js index c9da4f7c5..a056771dc 100644 --- a/src/layers/streetLevel/providers/kartaview/index.js +++ b/src/layers/streetLevel/providers/kartaview/index.js @@ -5,6 +5,7 @@ import { decodeCoverageTile } from './decode.js'; import { createKartaViewSource } from './source.js'; import { createKartaViewViewer } from './viewer.js'; import { + ARCHIVED_MESSAGE, COLORS, COVERAGE_MAX_HEIGHT_M, COVERAGE_NOUN, @@ -15,9 +16,12 @@ import { KARTAVIEW_NAME, KARTAVIEW_PROVIDER_ID, LINE_CLICK_HINT, + NEAREST_PROBES, NEAREST_RADIUS_M, + NO_COVERAGE_HINT, PICK_PREFIX, RATE_LIMITED_MESSAGE, + SEQUENCE_PAGE_EDGE, SEQUENCE_PAGE_SIZE, TOO_HIGH_HINT, normalizePhoto, @@ -85,6 +89,8 @@ export function createKartaViewProvider({ defaultOn: false, // Coverage lines carry no date or projection: no filter could thin them. filters: [], + // Its lines are streets, not sequences: a selected drive shows as cones. + highlightsSequence: false, coverageLabel: Object.freeze({ noun: COVERAGE_NOUN, action: LINE_CLICK_HINT, @@ -169,42 +175,94 @@ export function createKartaViewProvider({ return entry; } - /** The page holding `photo` (an earlier one when photos are missing). */ - async function windowOf(photo) { - let page = pageOf(photo.sequenceIndex ?? 0); - for (let tries = 0; tries < 3; tries++) { - const window = await loadPage(photo.sequenceId, page); - const first = window.photos[0]; - if ( - window.photos.some((p) => p.id === photo.id) || - page === 1 || - !first || - (first.sequenceIndex ?? 0) <= (photo.sequenceIndex ?? 0) - ) - return window; - page--; + /** + * The page holding `photo`. Pages follow photo ids, which only roughly + * follow the index, and photos go missing, so the page the index + * suggests is a first guess: step towards the photo until a page holds + * it, or the pages straddle where it would be. + */ + async function homePage(photo) { + let number = pageOf(photo.sequenceIndex ?? 0); + const index = photo.sequenceIndex ?? 0; + const seen = new Set(); + let page = null; + while (!seen.has(number) && seen.size < 6) { + seen.add(number); + page = await loadPage(photo.sequenceId, number); + if (page.photos.some((p) => p.id === photo.id)) return page; + const first = page.photos[0]?.sequenceIndex; + const last = page.photos.at(-1)?.sequenceIndex; + // Past the last page, or before this page's photos: go back. + if ((first === undefined || index < first) && number > 1) number--; + else if (last !== undefined && index > last && page.hasMore) number++; + else break; } - return loadPage(photo.sequenceId, page); + return page; } + /** + * The photos around `photo` in index order: its page, plus a + * neighbour page when it sits near an edge (pages overlap in index). + * @returns {Promise<{home: number, photos: Array}>} + */ + async function windowOf(photo) { + const home = await homePage(photo); + const list = home.photos; + const at = list.findIndex((p) => p.id === photo.id); + const parts = [list]; + if (at !== -1 && at < SEQUENCE_PAGE_EDGE && home.page > 1) + parts.push((await loadPage(photo.sequenceId, home.page - 1)).photos); + if (at !== -1 && at >= list.length - SEQUENCE_PAGE_EDGE && home.hasMore) + parts.push((await loadPage(photo.sequenceId, home.page + 1)).photos); + if (parts.length === 1) return { home: home.page, photos: list }; + const byId = new Map(parts.flat().map((p) => [p.id, p])); + return { + home: home.page, + photos: [...byId.values()].sort( + (a, b) => (a.sequenceIndex ?? 0) - (b.sequenceIndex ?? 0), + ), + }; + } + + /** Photo id → whether its file answers (archived ones do not), per session. */ + const probes = createLru(PHOTO_CACHE_SIZE); + function showable(photo, { signal } = {}) { + if (!photo?.urls?.view) return Promise.resolve(false); + let probe = probes.get(photo.id); + if (!probe) { + probe = source.probeImage(photo.urls.view, { signal }); + probes.set(photo.id, probe); + probe.catch(() => probes.delete(photo.id)); + } + return probe; + } + + /** + * The next showable photo `delta` along the sequence, or null at its + * end. Archived photos are skipped, a few at most. + */ async function step(photo, delta) { if (!photo?.sequenceId) return null; - const window = await windowOf(photo); - const list = window.photos; - let at = list.findIndex((p) => p.id === photo.id); - if (at === -1) - at = list.findIndex( - (p) => (p.sequenceIndex ?? 0) >= (photo.sequenceIndex ?? 0), - ); - const next = at === -1 ? -1 : at + delta; - if (next >= 0 && next < list.length) return list[next].id; - if (next < 0 && window.page > 1) { - const before = await loadPage(photo.sequenceId, window.page - 1); - return before.photos.at(-1)?.id ?? null; - } - if (next >= list.length && window.hasMore) { - const after = await loadPage(photo.sequenceId, window.page + 1); - return after.photos[0]?.id ?? null; + let from = photo; + for (let skipped = 0; skipped <= NEAREST_PROBES; skipped++) { + const { photos: list } = await windowOf(from); + let at = list.findIndex((p) => p.id === from.id); + if (at === -1) { + // Not found anywhere: the nearest index on the right side. + const index = from.sequenceIndex ?? 0; + const later = list.findIndex((p) => (p.sequenceIndex ?? 0) > index); + if (delta > 0 && later === -1) return null; + at = + delta > 0 + ? later - 1 + : later === -1 + ? list.length + : list.findIndex((p) => (p.sequenceIndex ?? 0) >= index); + } + const next = list[at + delta]; + if (!next) return null; + if (await showable(next)) return next.id; + from = next; } return null; } @@ -229,7 +287,7 @@ export function createKartaViewProvider({ if (!anchor) return []; const window = await windowOf(anchor); signal?.throwIfAborted(); - drawnPage = { sequenceId, page: window.page }; + drawnPage = { sequenceId, page: window.home }; return thinImages( window.photos.map((photo) => ({ id: photo.id, @@ -245,16 +303,24 @@ export function createKartaViewProvider({ pickPrefix: PICK_PREFIX.image, spriteId: 'street-level:kartaview-cones', renderKey: 'kartaview-sequence', + // The filters are hidden for KartaView: one left set must not hide cones. + passes: () => true, }); - /** Re-centre the cones when ‹ › walked off the page they show. */ - async function followStep(photoId) { + /** + * ‹ › open through the core (loading, errors, the sequence's cones), + * without moving the camera; the cones re-centre once ‹ › walk off the + * page they were drawn from. + */ + async function openStepped(photoId) { + const opened = await context.actions.openImage(photoId, { + frame: false, + }); const photo = photos.get(photoId); - if (!photo?.sequenceId) return; - anchors.set(photo.sequenceId, photo); + if (!opened || !photo?.sequenceId) return; if (drawnPage?.sequenceId !== photo.sequenceId) return; const window = await windowOf(photo).catch(() => null); - if (window && window.page !== drawnPage.page) + if (window && window.home !== drawnPage.page) cones.select(photo.sequenceId, { force: true }); } @@ -265,7 +331,7 @@ export function createKartaViewProvider({ return photo; }, step, - onStepped: followStep, + onStep: openStepped, render: context.services?.render, ...viewerOptions, }); @@ -316,12 +382,18 @@ export function createKartaViewProvider({ coverageStats() { const { hint } = state.coverage; const count = coverage.sequenceCount(); + const loading = state.coverage.pending.size > 0; + const empty = + state.coverage.zoom !== null && + !loading && + count === 0 && + !state.coverage.lastError; return { count, zoom: state.coverage.zoom, kind: state.coverage.kind, - loading: state.coverage.pending.size > 0, - hint: hint || '', + loading, + hint: hint || (empty ? NO_COVERAGE_HINT : ''), error: state.coverage.lastError, keyRequired: false, keyRejected: false, @@ -353,18 +425,25 @@ export function createKartaViewProvider({ loading: state.sequence.loading, }), - /** The nearest photo within the search radius, or null. */ + /** + * The nearest photo within the search radius that can be shown, or + * null. Many photos are archived (their files answer 409) or have no + * file at all, so the nearest few are tried in turn. + */ async nearestImage({ lat, lon }, { signal } = {}) { const records = await source.nearestPhotos({ lat, lon }, { signal }); - let best = null; + const near = []; for (const record of records) { const photo = remember(record); - if (!photo) continue; + if (!photo?.urls.view) continue; const metres = metresBetween({ lat, lon }, photo); - if (metres <= NEAREST_RADIUS_M && (!best || metres < best.metres)) - best = { photo, metres }; + if (metres <= NEAREST_RADIUS_M) near.push({ photo, metres }); } - return best ? best.photo.id : null; + near.sort((a, b) => a.metres - b.metres); + for (const { photo } of near.slice(0, NEAREST_PROBES)) + if (await showable(photo, { signal })) return photo.id; + if (near.length) throw new Error(ARCHIVED_MESSAGE); + return null; }, viewer, diff --git a/src/layers/streetLevel/providers/kartaview/policy.js b/src/layers/streetLevel/providers/kartaview/policy.js index 84a36d7d0..d2d75b5e2 100644 --- a/src/layers/streetLevel/providers/kartaview/policy.js +++ b/src/layers/streetLevel/providers/kartaview/policy.js @@ -47,6 +47,7 @@ export const COVERAGE_TUNING = Object.freeze({ viewNearM: 1_000, }); export const TOO_HIGH_HINT = 'zoom in to see KartaView coverage'; +export const NO_COVERAGE_HINT = 'no KartaView photos in view'; export const RATE_LIMITED_MESSAGE = 'KartaView is rate-limiting requests — they resume on their own'; @@ -57,10 +58,23 @@ export const RATE_LIMITED_MESSAGE = export const PHOTO_API_CONCURRENCY = 2; export const PHOTO_API_RETRIES = 3; export const PHOTO_API_RETRY_BASE_MS = 800; +/** A Retry-After longer than this is not waited out. */ +export const PHOTO_API_RETRY_MAX_MS = 10_000; +/** The API is HTTP/1.1 (six connections): tiles 3, lookups 2, pages 1. */ +export const TILE_CONCURRENCY = 3; +export const PAGE_CONCURRENCY = 1; export const TILE_TIMEOUT_MS = 20_000; export const PHOTO_API_TIMEOUT_MS = 15_000; +/** A sequence page is about a megabyte of JSON. */ +export const PAGE_TIMEOUT_MS = 30_000; /** A photo download (tens of kilobytes to a few megabytes). */ -export const IMAGE_TIMEOUT_MS = 30_000; +export const IMAGE_TIMEOUT_MS = 15_000; +/** Whether a photo file is there (not archived): headers only. */ +export const IMAGE_PROBE_TIMEOUT_MS = 8_000; +/** Nearest photos tried before giving up on archived ones. */ +export const NEAREST_PROBES = 4; +export const ARCHIVED_MESSAGE = + 'KartaView’s photos here are archived and cannot be shown'; /** Photos asked for around a point; the API returns them nearest first. */ export const NEAREST_LIMIT = 10; @@ -69,9 +83,12 @@ export const NEAREST_SEARCH_ZOOM = 18; /** * Photos of a sequence fetched at once, around the open one. Records are - * ~2 kB each, so a page is ~2 MB; the cones and prev/next use it. + * ~2.4 kB each, so a page is ~1.2 MB; the cones and ‹ › use it. Pages are in + * photo-id order, which only roughly follows the sequence index. */ -export const SEQUENCE_PAGE_SIZE = 1000; +export const SEQUENCE_PAGE_SIZE = 500; +/** Within this many photos of a page's edge, its neighbour page joins it. */ +export const SEQUENCE_PAGE_EDGE = 12; /** Equirectangular panorama sizes: a quick preview, then the sharp one. */ export const PANO_PREVIEW_SIZE = Object.freeze([1024, 512]); @@ -161,6 +178,9 @@ export function normalizePhoto(record) { if (!record || record.id == null) return null; const { lat, lon } = positionOf(record); if (!Number.isFinite(lat) || !Number.isFinite(lon)) return null; + // 0,0 is a missing fix, not a photo in the Gulf of Guinea. + if (lat === 0 && lon === 0) return null; + if (Math.abs(lat) > 90 || Math.abs(lon) > 180) return null; const heading = record.heading === '' || record.heading == null ? NaN @@ -195,9 +215,13 @@ export function normalizePhoto(record) { /** * Where the image link goes. KartaView's photo pages fail for signed-out - * visitors ("Sequence cannot be loaded"), so it is the full-size photo on - * KartaView's own storage. + * visitors ("Sequence cannot be loaded"), so it is the full-size photo + * through KartaView's image proxy, which also serves photos whose storage + * host answers 502 directly. */ export function kartaViewPhotoUrl(photo) { - return photo?.urls?.full || photo?.urls?.view || 'https://kartaview.org'; + const url = photo?.urls?.full || photo?.urls?.view; + return url + ? `${KARTAVIEW_CDN_HOST}/pr:sharp/${base64Url(url)}` + : 'https://kartaview.org'; } diff --git a/src/layers/streetLevel/providers/kartaview/policy.test.mjs b/src/layers/streetLevel/providers/kartaview/policy.test.mjs index b48e1742f..140a3903c 100644 --- a/src/layers/streetLevel/providers/kartaview/policy.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/policy.test.mjs @@ -60,7 +60,14 @@ test('photo files come from the storage host the record names, not the one its U photo.urls.full, 'https://storage2.openstreetcam.org/files/photo/2016/8/24/proc/14010_ba53c_57bd1d23d50b7.jpg', ); - assert.equal(kartaViewPhotoUrl(photo), photo.urls.full); + // The link goes through the image proxy, which serves photos whose + // storage host answers 502 directly. + const link = kartaViewPhotoUrl(photo); + assert.match(link, /^https:\/\/cdn\.kartaview\.org\/pr:sharp\/[\w-]+$/); + assert.equal( + Buffer.from(link.split('/').at(-1), 'base64url').toString(), + photo.urls.full, + ); // An unexpected storage value falls back to the URLs as given. const odd = normalizePhoto(record({ storage: 'https://evil.example' })); assert.match(odd.urls.view, /^https:\/\/storage1\.openstreetcam\.org\//); @@ -101,6 +108,14 @@ test('a 360° photo keeps its equirectangular image; the unmatched position is t test('records without an id or a position are dropped; a missing heading is null', () => { assert.equal(normalizePhoto(null), null); + // 0,0 is a missing fix. + assert.equal( + normalizePhoto( + record({ lat: '0', lng: '0', matchLat: '0', matchLng: '0' }), + ), + null, + ); + assert.equal(normalizePhoto(record({ lat: '95', matchLat: '95' })), null); assert.equal(normalizePhoto(record({ id: undefined })), null); assert.equal( normalizePhoto( diff --git a/src/layers/streetLevel/providers/kartaview/provider.test.mjs b/src/layers/streetLevel/providers/kartaview/provider.test.mjs index ef626fe09..582e32551 100644 --- a/src/layers/streetLevel/providers/kartaview/provider.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/provider.test.mjs @@ -24,11 +24,19 @@ function photoRecord(id, { metres = 0, seq = '14010', index = 0 } = {}) { }; } -/** A source over `sequences` (id → indexes present), recording calls. */ -function fakeSource({ nearest = [], sequences = {} } = {}) { - const calls = { nearest: 0, photo: [], pages: [] }; +/** + * A source over `sequences` (id → indexes present, in page order: KartaView + * pages follow photo ids, which only roughly follow the index), recording + * calls. Photos whose id is in `archived` answer 409. + */ +function fakeSource({ nearest = [], sequences = {}, archived = [] } = {}) { + const calls = { nearest: 0, photo: [], pages: [], probes: [] }; return { calls, + probeImage: async (url) => { + calls.probes.push(url); + return !archived.some((id) => url.includes(`/${id}.jpg`)); + }, getTile: async () => ({ lines: [] }), nearestPhotos: async () => { calls.nearest++; @@ -59,7 +67,7 @@ function fakeSource({ nearest = [], sequences = {} } = {}) { } function fakeContext() { - const actions = { opened: [], nearest: [] }; + const actions = { opened: [], nearest: [], instance: null }; return { actions, context: { @@ -71,7 +79,12 @@ function fakeContext() { getSurface: () => 'draped', notify() {}, actions: { - openImage: (id) => actions.opened.push(id), + // As the core does: the provider's own viewer opens it. + openImage: async (id, options) => { + actions.opened.push({ id, options }); + await actions.instance?.viewer.open(id); + return true; + }, openNearest: (point) => actions.nearest.push(point), reportError() {}, }, @@ -96,6 +109,7 @@ function setup(sourceOptions) { }); const { context, actions } = fakeContext(); const instance = definition.create(context); + actions.instance = instance; return { definition, instance, source, actions, loads }; } @@ -159,8 +173,9 @@ test('the nearest photo is the closest within 50 m, and opening it asks nothing test('a cone opens its photo; a line opens the photo nearest the click', () => { const { instance, actions } = setup(); + actions.instance = null; assert.equal(instance.handlePick('kv:img:123'), true); - assert.deepEqual(actions.opened, ['123']); + assert.deepEqual(actions.opened, [{ id: '123', options: undefined }]); const point = { lon: -83.0458, lat: 42.3314 }; assert.equal( instance.handlePick('kv:line:14/1/2/3', { groundPoint: () => point }), @@ -185,47 +200,124 @@ test('coverage stats report KartaView as configured and key-free', async () => { assert.equal(stats.count, 0); }); -test('› walks across sequence pages; ‹ walks back; the ends stop', async () => { +const press = async (button) => { + button.fire('click'); + for (let i = 0; i < 12; i++) await settle(); +}; + +async function mounted(sourceOptions) { + const env = setup(sourceOptions); + const host = fakeHost(); + await env.instance.viewer.mount(host); + const poses = []; + env.instance.viewer.onPose((pose) => poses.push(pose.imageId)); + const [, , prev, next] = host.children[0].children; + return { ...env, poses, prev, next }; +} + +test('› walks across sequence pages without moving the camera; ‹ walks back; the ends stop', async () => { const size = SEQUENCE_PAGE_SIZE; const indexes = Array.from({ length: size + 5 }, (_, i) => i); - const { instance, source } = setup({ sequences: { s: indexes } }); - const host = fakeHost(); - await instance.viewer.mount(host); - const poses = []; - instance.viewer.onPose((pose) => poses.push(pose.imageId)); - const [, , prev, next] = host.children[0].children; - const press = async (button) => { - button.fire('click'); - for (let i = 0; i < 6; i++) await settle(); - }; + const { instance, actions, source, poses, prev, next } = await mounted({ + sequences: { s: indexes }, + }); await instance.viewer.open(`s-${size - 1}`); await press(next); assert.equal(poses.at(-1), `s-${size}`, 'onto the second page'); - assert.deepEqual(source.calls.pages, ['s:1', 's:2']); + assert.deepEqual(actions.opened.at(-1), { + id: `s-${size}`, + options: { frame: false }, + }); + assert.ok(source.calls.pages.includes('s:2')); await press(prev); assert.equal(poses.at(-1), `s-${size - 1}`); await instance.viewer.open('s-0'); + for (let i = 0; i < 6; i++) await settle(); + assert.equal(prev.disabled, true, 'nothing before the first photo'); await press(prev); - assert.equal(poses.at(-1), 's-0', 'nothing before the first photo'); - assert.equal(prev.disabled, true); + assert.equal(poses.at(-1), 's-0'); }); -test('a page with missing photos is found by stepping back a page', async () => { +test('pages that overlap in index (id order) step one photo at a time across the edge', async () => { const size = SEQUENCE_PAGE_SIZE; - // Index `size + 100` sits on page 1 when 200 earlier photos are gone. - const indexes = Array.from({ length: size + 300 }, (_, i) => i).filter( - (i) => i < 100 || i >= 300, + // As KartaView pages by id: page 1 holds 0…size+2 less two, page 2 starts + // with those two (size-2, size) before the rest. + const first = Array.from({ length: size + 3 }, (_, i) => i).filter( + (i) => i !== size - 2 && i !== size, + ); + const second = [ + size - 2, + size, + ...Array.from({ length: 50 }, (_, i) => size + 3 + i), + ]; + const { instance, poses, next } = await mounted({ + sequences: { s: [...first, ...second] }, + }); + await instance.viewer.open(`s-${size - 5}`); + for (let k = 1; k <= 8; k++) await press(next); + assert.deepEqual( + poses.slice(-9).map((id) => Number(id.slice(2))), + Array.from({ length: 9 }, (_, i) => size - 5 + i), + 'no photo skipped at the page edge', + ); +}); + +test('a photo whose index outruns the page count is found, and its neighbours are real', async () => { + // 1,000 photos with 500–1499 deleted: index 1500 is on page 2, not 4. + const indexes = [ + ...Array.from({ length: 500 }, (_, i) => i), + ...Array.from({ length: 500 }, (_, i) => 1500 + i), + ]; + const { instance, poses, prev, next } = await mounted({ + sequences: { s: indexes }, + }); + await instance.viewer.open('s-1500'); + await press(next); + assert.equal(poses.at(-1), 's-1501'); + await press(prev); + await press(prev); + assert.equal(poses.at(-1), 's-499', 'back across the gap'); + await instance.viewer.open('s-1999'); + for (let i = 0; i < 8; i++) await settle(); + assert.equal(next.disabled, true, 'the last photo'); +}); + +test('archived photos are skipped: by the nearest lookup, and by › and ‹', async () => { + const { instance, poses, next } = await mounted({ + nearest: [ + photoRecord('gone', { metres: 3 }), + photoRecord('kept', { metres: 9 }), + ], + archived: ['gone', 's-2', 's-3'], + sequences: { s: [0, 1, 2, 3, 4] }, + }); + assert.equal( + await instance.nearestImage({ lat: 42.3314, lon: -83.0458 }), + 'kept', + ); + await instance.viewer.open('s-1'); + await press(next); + assert.equal(poses.at(-1), 's-4', 'two archived photos stepped over'); + const allGone = setup({ + nearest: [photoRecord('gone', { metres: 3 })], + archived: ['gone'], + }); + await assert.rejects( + allGone.instance.nearestImage({ lat: 42.3314, lon: -83.0458 }), + /archived and cannot be shown/, + ); + const noFiles = setup({ + nearest: [ + { + ...photoRecord('bare', { metres: 2 }), + filepathLTh: '', + filepathProc: '', + }, + ], + }); + assert.equal( + await noFiles.instance.nearestImage({ lat: 42.3314, lon: -83.0458 }), + null, + 'a record with no file is not a photo', ); - const { instance, source } = setup({ sequences: { s: indexes } }); - const host = fakeHost(); - await instance.viewer.mount(host); - const poses = []; - instance.viewer.onPose((pose) => poses.push(pose.imageId)); - const target = size + 100; - await instance.viewer.open(`s-${target}`); - const [, , , next] = host.children[0].children; - next.fire('click'); - for (let i = 0; i < 8; i++) await settle(); - assert.equal(poses.at(-1), `s-${target + 1}`); - assert.deepEqual(source.calls.pages.slice(0, 2), ['s:2', 's:1']); }); diff --git a/src/layers/streetLevel/providers/kartaview/source.js b/src/layers/streetLevel/providers/kartaview/source.js index 7d68c5f62..244ea38d8 100644 --- a/src/layers/streetLevel/providers/kartaview/source.js +++ b/src/layers/streetLevel/providers/kartaview/source.js @@ -1,11 +1,16 @@ import { + IMAGE_PROBE_TIMEOUT_MS, KARTAVIEW_API_HOST, NEAREST_LIMIT, NEAREST_SEARCH_ZOOM, + PAGE_CONCURRENCY, + PAGE_TIMEOUT_MS, PHOTO_API_CONCURRENCY, PHOTO_API_RETRIES, PHOTO_API_RETRY_BASE_MS, + PHOTO_API_RETRY_MAX_MS, PHOTO_API_TIMEOUT_MS, + TILE_CONCURRENCY, TILE_TIMEOUT_MS, } from './policy.js'; @@ -19,7 +24,7 @@ export class KartaViewSourceError extends Error { } } -const NO_ANSWER = 'KartaView did not answer — check the network and try again'; +const NO_ANSWER = 'KartaView did not answer in time — try again'; /** Resolves after `ms`, or rejects when `signal` aborts first. */ function wait(ms, signal) { @@ -92,8 +97,11 @@ export function createLimiter(limit) { /** * KartaView's public API, browser-side and keyless (it allows any origin). - * Coverage tiles are not rate-limited; photo lookups are, so they queue two - * at a time and retry a 429. + * It speaks HTTP/1.1, so the browser opens six connections to it: tiles take + * at most three, photo lookups two and sequence pages one, so a lookup never + * waits behind slow tiles or pages. The photo API answers 429 to more than + * about three requests at once; a 429 (and a lookup that timed out) is + * retried. * @param {{fetchImpl?: typeof fetch, apiHost?: string, retryBaseMs?: number}} [options] */ export function createKartaViewSource({ @@ -102,6 +110,8 @@ export function createKartaViewSource({ retryBaseMs = PHOTO_API_RETRY_BASE_MS, } = {}) { const limiter = createLimiter(PHOTO_API_CONCURRENCY); + const tileLimiter = createLimiter(TILE_CONCURRENCY); + const pageLimiter = createLimiter(PAGE_CONCURRENCY); /** GET JSON within `timeoutMs`; the caller's abort wins over the timeout. */ async function getJson(url, { signal, timeoutMs }) { @@ -137,32 +147,52 @@ export function createKartaViewSource({ } } - /** A photo-API call through the limiter, retrying 429s with backoff. */ - async function photoApi(path, params, { signal } = {}) { + /** + * A photo-API call through `gate`, retrying 429s with backoff and, once, a + * call that timed out. + */ + async function photoApi( + path, + params, + { signal } = {}, + { gate = limiter, timeoutMs = PHOTO_API_TIMEOUT_MS } = {}, + ) { const query = params ? `?${new URLSearchParams(params)}` : ''; const url = `${apiHost}${path}${query}`; + let timeouts = 0; for (let attempt = 0; ; attempt++) { try { - const payload = await limiter.run( - () => getJson(url, { signal, timeoutMs: PHOTO_API_TIMEOUT_MS }), + const payload = await gate.run( + () => getJson(url, { signal, timeoutMs }), { signal }, ); const code = Number(payload?.status?.apiCode); // 600 ok, 601 empty, 602 partial: anything else is a refusal. if (Number.isFinite(code) && (code < 600 || code > 603)) throw new KartaViewSourceError( - String( - payload?.status?.apiMessage || 'KartaView refused the request', - ).trim(), + `KartaView refused the request (code ${code})`, { status: Number(payload?.status?.httpCode) || 0 }, ); return payload?.result ?? {}; } catch (error) { - if (error?.status !== 429 || attempt >= PHOTO_API_RETRIES) throw error; + if (signal?.aborted) throw error; + const timedOut = error?.message === NO_ANSWER && error.status === 0; + if (timedOut && timeouts++ < 1) continue; + if (error?.status !== 429 || attempt >= PHOTO_API_RETRIES) { + if (error?.status === 429) + throw new KartaViewSourceError( + 'KartaView is rate-limiting requests — try again in a moment', + { status: 429, retryAfterSec: error.retryAfterSec }, + ); + throw error; + } await wait( - Math.max( - retryBaseMs * 2 ** attempt, - (error.retryAfterSec ?? 0) * 1000, + Math.min( + PHOTO_API_RETRY_MAX_MS, + Math.max( + retryBaseMs * 2 ** attempt, + (error.retryAfterSec ?? 0) * 1000, + ), ), signal, ); @@ -173,10 +203,42 @@ export function createKartaViewSource({ return { /** A coverage tile: `{lines: [{track: [{lat, lng}]}]}`. */ async getTile({ x, y, z }, { signal } = {}) { - return getJson(`${apiHost}/2.0/sequence/tiles/${x}/${y}/${z}.json`, { - signal, - timeoutMs: TILE_TIMEOUT_MS, - }); + return tileLimiter.run( + () => + getJson(`${apiHost}/2.0/sequence/tiles/${x}/${y}/${z}.json`, { + signal, + timeoutMs: TILE_TIMEOUT_MS, + }), + { signal }, + ); + }, + + /** + * Whether a photo file can be fetched. Archived photos answer 409 (and + * a HEAD says 200 regardless), so this GETs and stops at the headers. + */ + async probeImage(url, { signal } = {}) { + if (!url) return false; + const controller = new AbortController(); + const onAbort = () => controller.abort(signal.reason); + if (signal?.aborted) throw signal.reason; + signal?.addEventListener('abort', onAbort, { once: true }); + const timer = setTimeout( + () => controller.abort(), + IMAGE_PROBE_TIMEOUT_MS, + ); + try { + const response = await fetchImpl(url, { signal: controller.signal }); + return response.ok; + } catch { + if (signal?.aborted) throw signal.reason; + return false; + } finally { + clearTimeout(timer); + signal?.removeEventListener('abort', onAbort); + // The body is not needed: drop it. + controller.abort(); + } }, /** Photo records around a point, nearest first. */ @@ -216,6 +278,7 @@ export function createKartaViewSource({ `/2.0/sequence/${encodeURIComponent(sequenceId)}/photos`, { page: String(page), itemsPerPage: String(itemsPerPage) }, { signal }, + { gate: pageLimiter, timeoutMs: PAGE_TIMEOUT_MS }, ); return { photos: Array.isArray(result.data) ? result.data : [], diff --git a/src/layers/streetLevel/providers/kartaview/source.test.mjs b/src/layers/streetLevel/providers/kartaview/source.test.mjs index 8d8bca033..73fa3b1bb 100644 --- a/src/layers/streetLevel/providers/kartaview/source.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/source.test.mjs @@ -5,7 +5,11 @@ import { createKartaViewSource, createLimiter, } from './source.js'; -import { PHOTO_API_CONCURRENCY, PHOTO_API_RETRIES } from './policy.js'; +import { + PHOTO_API_CONCURRENCY, + PHOTO_API_RETRIES, + TILE_CONCURRENCY, +} from './policy.js'; const ok = (body) => ({ ok: true, @@ -87,12 +91,13 @@ test('rate-limiting that outlasts the retries is reported with a wait', async () assert.ok(error instanceof KartaViewSourceError); assert.equal(error.status, 429); assert.ok(error.retryAfterSec > 0); + assert.match(error.message, /try again in a moment/); return true; }); assert.equal(calls, PHOTO_API_RETRIES + 1); }); -test('an API refusal inside a 200 is an error with KartaView’s message; 601 is just empty', async () => { +test('an API refusal inside a 200 is a readable error with its code; 601 is just empty', async () => { const refusing = createKartaViewSource({ fetchImpl: async () => ok({ @@ -100,7 +105,10 @@ test('an API refusal inside a 200 is an error with KartaView’s message; 601 is result: {}, }), }); - await assert.rejects(refusing.getPhoto('1'), /out of range/); + await assert.rejects( + refusing.getPhoto('1'), + /KartaView refused the request \(code 612\)/, + ); const empty = createKartaViewSource({ fetchImpl: async () => ok({ status: { apiCode: 601 }, result: { data: [] } }), @@ -190,3 +198,92 @@ test('the limiter runs tasks in order and keeps going after a failure', async () await new Promise((resolve) => setTimeout(resolve, 0)); assert.equal(limiter.pending, 0); }); + +test('tiles, lookups and sequence pages queue separately, so a lookup never waits behind them', async () => { + const running = { tile: 0, photo: 0, page: 0 }; + const peak = { tile: 0, photo: 0, page: 0 }; + const release = []; + const kind = (url) => + url.includes('/tiles/') + ? 'tile' + : url.includes('/photos') + ? 'page' + : 'photo'; + const source = createKartaViewSource({ + fetchImpl: async (url) => { + const k = kind(url); + running[k]++; + peak[k] = Math.max(peak[k], running[k]); + if (k !== 'photo') await new Promise((resolve) => release.push(resolve)); + running[k]--; + return k === 'tile' ? ok({ lines: [] }) : ok(photoPayload([])); + }, + }); + const tiles = Array.from({ length: 9 }, (_, i) => + source.getTile({ x: i, y: 0, z: 14 }), + ); + const pages = [1, 2].map((page) => + source.getSequencePhotos('1', { page, itemsPerPage: 500 }), + ); + // Every slow tile and page is in flight or queued; a lookup still answers. + assert.deepEqual(await source.nearestPhotos({ lat: 1, lon: 1 }), []); + assert.equal(peak.tile, TILE_CONCURRENCY); + assert.equal(peak.page, 1); + while (release.length) { + release.shift()(); + await new Promise((resolve) => setTimeout(resolve, 0)); + } + await Promise.all([...tiles, ...pages]); + assert.equal(peak.tile, TILE_CONCURRENCY); + assert.equal(peak.page, 1); +}); + +test('a lookup that times out is asked once more; a second timeout says to try again', async () => { + let calls = 0; + const source = createKartaViewSource({ + fetchImpl: async () => { + calls++; + throw new TypeError('Failed to fetch'); + }, + }); + await assert.rejects( + source.getPhoto('1'), + /did not answer in time — try again/, + ); + assert.equal(calls, 2); +}); + +test('an image probe reads only the status: an archived photo (409) is not showable', async () => { + const seen = []; + const source = createKartaViewSource({ + fetchImpl: async (url, { signal }) => { + seen.push(signal); + return { + ok: !url.includes('archived'), + status: url.includes('archived') ? 409 : 200, + }; + }, + }); + assert.equal( + await source.probeImage('https://storage13.openstreetcam.org/ok.jpg'), + true, + ); + assert.equal( + await source.probeImage('https://storage13.openstreetcam.org/archived.jpg'), + false, + ); + assert.equal(await source.probeImage(null), false); + assert.ok( + seen.every((signal) => signal.aborted), + 'the body is dropped', + ); + const offline = createKartaViewSource({ + fetchImpl: async () => { + throw new TypeError('Failed to fetch'); + }, + }); + assert.equal( + await offline.probeImage('https://x.openstreetcam.org/a.jpg'), + false, + ); +}); diff --git a/src/layers/streetLevel/providers/kartaview/viewer.js b/src/layers/streetLevel/providers/kartaview/viewer.js index 27c5bee6b..d3fc218e7 100644 --- a/src/layers/streetLevel/providers/kartaview/viewer.js +++ b/src/layers/streetLevel/providers/kartaview/viewer.js @@ -49,7 +49,7 @@ export function loadImage( * @param {object} options * @param {(photoId: string) => Promise} options.getPhoto A normalized photo (policy.js normalizePhoto). * @param {(photo: object, delta: -1|1) => Promise} options.step The photo `delta` along the sequence, or null at its end. - * @param {(imageId: string) => void} [options.onStepped] A photo opened from ‹ ›, after its pose. + * @param {(imageId: string) => void} options.onStep Open the photo ‹ › chose (through the core, so loading and errors show). * @param {object} [options.render] Render governor. * @param {(url: string, options?: object) => Promise} [options.load] * @param {(canvas: HTMLCanvasElement) => object} [options.createRenderer] @@ -58,7 +58,7 @@ export function loadImage( export function createKartaViewViewer({ getPhoto, step, - onStepped = () => {}, + onStep, render, load = loadImage, createRenderer = createPanoramaRenderer, @@ -80,6 +80,12 @@ export function createKartaViewViewer({ let drag = null; let poseFrame = 0; let removeListeners = []; + /** The photo an open is fetching: ‹ › step on from it, not the old one. */ + let openingId = null; + /** ‹ › chose this photo: keep the panorama's look direction for it. */ + let keepViewFor = null; + /** ‹ › presses run one after another, each from where the last one went. */ + let steps = Promise.resolve(); function requestRender() { render?.governorRequestRender?.('kartaview-viewer'); @@ -133,6 +139,21 @@ export function createKartaViewViewer({ const enabled = Boolean(photo?.sequenceId) && showing !== null; if (prevButton) prevButton.disabled = !enabled; if (nextButton) nextButton.disabled = !enabled; + if (enabled) checkEnds(photo); + } + + /** Disable ‹ or › at once where the sequence ends (it also warms the neighbours). */ + function checkEnds(target) { + for (const [delta, button] of [ + [-1, prevButton], + [1, nextButton], + ]) + step(target, delta).then( + (id) => { + if (photo === target && button && !id) button.disabled = true; + }, + () => {}, + ); } function show(kind) { @@ -159,29 +180,33 @@ export function createKartaViewViewer({ return renderer; } - /** Step along the sequence from the open photo. */ - async function stepBy(delta) { - const from = photo; - if (!from?.sequenceId || !showing) return; - const seq = openSeq; - let nextId = null; - try { - nextId = await step(from, delta); - } catch { - nextId = null; - } - if (seq !== openSeq || photo !== from) return; - if (!nextId) { - const button = delta < 0 ? prevButton : nextButton; - if (button) button.disabled = true; - return; - } - try { - await adapter.open(nextId, { keepView: true }); - } catch { - return; - } - if (photo?.id === nextId) onStepped(nextId); + /** + * Step along the sequence from the photo showing, or from the one on its + * way after an earlier press, and open it through the core. + */ + function stepBy(delta) { + steps = steps.then(async () => { + const fromId = openingId ?? photo?.id; + if (!fromId || !root) return; + let from; + let nextId = null; + try { + from = await getPhoto(fromId); + if (from.sequenceId) nextId = await step(from, delta); + } catch { + nextId = null; + } + // Closed, or another photo was opened meanwhile. + if ((openingId ?? photo?.id) !== fromId) return; + if (!nextId) { + const button = delta < 0 ? prevButton : nextButton; + if (button && photo?.id === fromId) button.disabled = true; + return; + } + openingId = nextId; + keepViewFor = nextId; + onStep(nextId); + }); } function onPointerDown(event) { @@ -324,15 +349,12 @@ export function createKartaViewViewer({ } async function showPano(target, seq, keepView) { - const preview = resizedImageUrl(target.urls.pano, PANO_PREVIEW_SIZE); - let image; - try { - image = await load(preview, { crossOrigin: true }); - } catch { - image = await load(resizedImageUrl(target.urls.pano, PANO_FULL_SIZE), { - crossOrigin: true, - }); - } + // Only the proxy can shrink a 13,000 px panorama: if it fails, the + // caller shows the front view rather than waiting on it twice. + const image = await load( + resizedImageUrl(target.urls.pano, PANO_PREVIEW_SIZE), + { crossOrigin: true }, + ); if (seq !== openSeq) return false; if (!keepView || showing !== 'pano') renderer.setView({ yaw: 0, pitch: 0, fov: PANO_FOV_DEFAULT_DEG }); @@ -359,6 +381,24 @@ export function createKartaViewViewer({ return true; } + async function openPhoto(id, seq, keepView) { + const target = await getPhoto(id); + if (seq !== openSeq) return; + let shown = false; + if (target.isPano && target.urls.pano && ensureRenderer()) { + try { + shown = await showPano(target, seq, keepView); + } catch { + // The panorama failed: its front view is better than nothing. + if (seq !== openSeq) return; + shown = await showFlat(target, seq); + } + } else shown = await showFlat(target, seq); + if (!shown) return; + emit(); + upgrade(target, seq, showing); + } + const adapter = { async mount(host) { if (!host) throw new Error('Street Level viewer has no host element'); @@ -368,28 +408,24 @@ export function createKartaViewViewer({ }, /** Resolves once the photo is on screen and its pose was emitted. */ - async open(imageId, { keepView = false } = {}) { + async open(imageId) { if (!root) throw new Error('KartaView viewer is not mounted'); + const id = String(imageId); const seq = ++openSeq; - const target = await getPhoto(String(imageId)); - if (seq !== openSeq) return; - let shown = false; - if (target.isPano && target.urls.pano && ensureRenderer()) { - try { - shown = await showPano(target, seq, keepView); - } catch { - // The panorama failed: its front view is better than nothing. - if (seq !== openSeq) return; - shown = await showFlat(target, seq); - } - } else shown = await showFlat(target, seq); - if (!shown) return; - emit(); - upgrade(target, seq, showing); + const keepView = keepViewFor === id; + keepViewFor = null; + openingId = id; + try { + await openPhoto(id, seq, keepView); + } finally { + if (openingId === id && seq === openSeq) openingId = null; + } }, close() { openSeq++; + openingId = null; + keepViewFor = null; photo = null; show(null); if (picture) picture.removeAttribute('src'); @@ -398,6 +434,8 @@ export function createKartaViewViewer({ unmount() { openSeq++; + openingId = null; + keepViewFor = null; teardown(); }, diff --git a/src/layers/streetLevel/providers/kartaview/viewer.test.mjs b/src/layers/streetLevel/providers/kartaview/viewer.test.mjs index f15c6f3fc..7372dea6c 100644 --- a/src/layers/streetLevel/providers/kartaview/viewer.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/viewer.test.mjs @@ -80,7 +80,11 @@ function setup({ failLoads = [], rendererThrows = false, steps = {} } = {}) { return PHOTOS[id]; }, step: async (photo, delta) => steps[`${photo.id}:${delta}`] ?? null, - onStepped: (id) => stepped.push(id), + // The core opens what ‹ › chose; here the adapter is opened directly. + onStep: (id) => { + stepped.push(id); + viewer.open(id).catch(() => {}); + }, load: async (url, options) => { loads.push({ url, crossOrigin: options?.crossOrigin }); if (failLoads.some((part) => url.includes(part))) @@ -108,7 +112,9 @@ const parts = (host) => { const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); test('a flat photo shows its 1280 px version, then the sharp one, with a pose facing its heading', async () => { - const { viewer, poses, host, loads } = setup(); + const { viewer, poses, host, loads } = setup({ + steps: { 'flat1:1': 'flat2', 'flat1:-1': 'flat0' }, + }); await viewer.mount(host); const { picture, canvas, prev, next } = parts(host); assert.equal(prev.disabled, true, 'nothing to step through yet'); @@ -142,7 +148,7 @@ test('a flat photo shows its 1280 px version, then the sharp one, with a pose fa title: null, }, ); - assert.equal(pose.externalUrl, PHOTOS.flat1.urls.full); + assert.match(pose.externalUrl, /^https:\/\/cdn\.kartaview\.org\/pr:sharp\//); assert.equal(loads[0].url, PHOTOS.flat1.urls.view); assert.equal(loads[0].crossOrigin, false, 'an needs no CORS'); await settle(); @@ -222,26 +228,43 @@ test('without WebGL, or when the panorama fails, a 360° photo shows its front v assert.equal(broken.poses.at(-1).imageId, 'pano1'); }); -test('‹ › and the arrow keys step along the sequence; the end disables the button', async () => { +test('‹ › and the arrow keys step along the sequence; ends disable at once', async () => { const { viewer, poses, host, stepped } = setup({ - steps: { 'flat1:1': 'flat2' }, + steps: { 'flat1:1': 'flat2', 'flat2:-1': 'flat1' }, }); await viewer.mount(host); await viewer.open('flat1'); const { root, prev, next } = parts(host); + await settle(); + assert.equal(prev.disabled, true, 'flat1 is the first photo: known up front'); + assert.equal(next.disabled, false); next.fire('click'); - await settle(); - await settle(); + for (let i = 0; i < 4; i++) await settle(); assert.equal(poses.at(-1).imageId, 'flat2'); - assert.deepEqual(stepped, ['flat2']); - root.fire('keydown', { key: 'ArrowRight' }); - await settle(); - await settle(); + assert.deepEqual(stepped, ['flat2'], 'the core is asked to open it'); assert.equal(next.disabled, true, 'flat2 is the last photo'); + assert.equal(prev.disabled, false); root.fire('keydown', { key: 'ArrowLeft', metaKey: true }); await settle(); assert.equal(poses.at(-1).imageId, 'flat2', 'modified arrows are not ours'); - assert.equal(prev.disabled, false); + root.fire('keydown', { key: 'ArrowLeft' }); + for (let i = 0; i < 4; i++) await settle(); + assert.equal(poses.at(-1).imageId, 'flat1'); +}); + +test('quick presses each step on from the photo the last one chose', async () => { + const chain = { 'flat1:1': 'flat2', 'flat2:1': 'pano1' }; + const { viewer, host, stepped } = setup({ + steps: chain, + rendererThrows: true, + }); + await viewer.mount(host); + await viewer.open('flat1'); + const { next } = parts(host); + next.fire('click'); + next.fire('click'); + for (let i = 0; i < 8; i++) await settle(); + assert.deepEqual(stepped, ['flat2', 'pano1']); }); test('a newer open wins; close hides the photo; unmount tears down the panorama', async () => { diff --git a/src/layers/streetLevel/registry.js b/src/layers/streetLevel/registry.js index 1e83ee7d4..2607ab542 100644 --- a/src/layers/streetLevel/registry.js +++ b/src/layers/streetLevel/registry.js @@ -22,7 +22,8 @@ export function filtersOf(provider) { * @property {boolean} [defaultOn] Default true. * @property {{kind: 'view-cone', color: string}} [marker] Default: GEV's pin. * @property {Array<'pano'|'since'>} [filters] Filters it honours (default both). - * @property {{noun: string, action: string}} [coverageLabel] How the panel counts its lines and says what a click does (default: sequences, "click a line for its photos"). + * @property {{noun: string, action: string}} [coverageLabel] + * @property {boolean} [highlightsSequence] Its lines show the selected sequence (default true; the legend's "Selected" needs one). How the panel counts its lines and says what a click does (default: sequences, "click a line for its photos"). * @property {(context: ProviderContext) => ProviderInstance} create * * @typedef {object} ProviderContext Handed to `create()` once by the core. @@ -33,7 +34,7 @@ export function filtersOf(provider) { * @property {GroundCaster|null} groundCaster Bare-earth heights for terrain mode (groundCast.js); null without a terrain service. * @property {{meshAt: (lon: number, lat: number) => number|undefined, request: (points: Array<[number, number]>) => void, onSampled: (listener: (cells: Array<[number, number]>) => void) => () => void}|null} meshSampler Sampled Google 3D surface heights (meshSampler.js); pass `meshAt` to `castLine`. * @property {() => void} notify Ask the core to publish a new UI snapshot. - * @property {{openImage: (imageId: string) => Promise, openNearest: (point: {lon: number, lat: number}) => Promise, reportError: (message: string|null) => void}} actions + * @property {{openImage: (imageId: string, options?: {frame?: boolean}) => Promise, openNearest: (point: {lon: number, lat: number}) => Promise, reportError: (message: string|null) => void}} actions * * @typedef {object} ProviderInstance * @property {() => Promise<{configured: boolean}>} status diff --git a/src/layers/streetLevel/sequenceCones.js b/src/layers/streetLevel/sequenceCones.js index a2aeca1ce..8eb79b29f 100644 --- a/src/layers/streetLevel/sequenceCones.js +++ b/src/layers/streetLevel/sequenceCones.js @@ -49,6 +49,8 @@ export function thinImages(images, spacingM = IMAGE_CONE_MIN_SPACING_M) { * @param {string} options.renderKey Render-governor reason. * @param {(sequenceId: string, selected: boolean) => void} [options.onHighlight] * Highlight (or stop highlighting) the sequence's coverage line. + * @param {(image: ConeImage, filter: object) => boolean} [options.passes] + * Whether the imagery filter keeps a cone. */ export function createSequenceCones({ state, @@ -58,6 +60,7 @@ export function createSequenceCones({ spriteId, renderKey, onHighlight = () => {}, + passes = passesImageryFilter, }) { const { render, sprites } = state.services; @@ -154,7 +157,7 @@ export function createSequenceCones({ // 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, filter)) return; + if (!passes(image, filter)) return; const height = heights?.[index] ?? null; const billboard = collection.add({ id: `${pickPrefix}${image.id}`, diff --git a/src/layers/streetLevel/uiState.js b/src/layers/streetLevel/uiState.js index 77c6bf173..cab0d12b1 100644 --- a/src/layers/streetLevel/uiState.js +++ b/src/layers/streetLevel/uiState.js @@ -18,6 +18,7 @@ import { COLORS } from './policy.js'; * @property {boolean} [groundClick] * @property {Array<'pano'|'since'>} [filters] * @property {{noun: string, action: string}|null} [coverageLabel] + * @property {boolean} [highlightsSequence] Its lines turn cyan when their sequence is selected. */ /** @@ -95,7 +96,13 @@ export function composeUIState({ label: provider.name, color: provider.color, })); - if (legend.length) + // "Selected" only beside a source whose lines turn cyan when selected. + if ( + active.some( + (provider) => + provider.groundClick !== true && provider.highlightsSequence !== false, + ) + ) legend.push({ key: 'selected', label: 'Selected', color: COLORS.selected }); return { enabled, diff --git a/src/layers/streetLevel/uiState.test.mjs b/src/layers/streetLevel/uiState.test.mjs index fe0b19d82..813213c4a 100644 --- a/src/layers/streetLevel/uiState.test.mjs +++ b/src/layers/streetLevel/uiState.test.mjs @@ -236,3 +236,25 @@ test('a key-gated layer offers the switched-off sources that would draw with a k [], ); }); + +test('a source whose lines never show a selection lists no Selected swatch, and names its own lines', () => { + const kartaView = provider({ + id: 'kartaview', + name: 'KartaView', + color: '#635bff', + highlightsSequence: false, + coverageLabel: { + noun: 'streets', + action: 'click a line for its nearest photo', + }, + }); + const ui = composeUIState({ ...base, providers: [kartaView] }); + assert.deepEqual( + ui.legend.map((entry) => entry.key), + ['kartaview'], + ); + assert.deepEqual(ui.coverage.label, { + noun: 'streets', + action: 'click a line for its nearest photo', + }); +});