diff --git a/scripts/qa-street-level.mjs b/scripts/qa-street-level.mjs index 1355cb765..4b60a0695 100644 --- a/scripts/qa-street-level.mjs +++ b/scripts/qa-street-level.mjs @@ -507,6 +507,7 @@ async function assertKeylessKartaView(page) { * Runs in the page. Find a point where a real click picks `prefix` + `id` (any * id when null), searched around `points` ([lon, lat]), the aimed billboards, * or else a grid mid-screen. Only points where the canvas is topmost count. + * Each call picks for at most `budgetMs`; the next call resumes the search. * @returns {{x: number, y: number, id: string}|null} client coordinates */ export function findPickPoint({ @@ -515,6 +516,7 @@ export function findPickPoint({ points, billboardIds, billboardPrefix = null, + budgetMs = 4000, }) { const viewer = window.__godsEyeView.viewer; const scene = viewer.scene; @@ -569,24 +571,54 @@ export function findPickPoint({ centres.push([at.x, at.y]); } } - for (const [cx, cy] of centres) - for (let r = 0; r <= 16; r += 2) + // A pick is a render of the scene (0.5–2 s each under SwiftShader): try + // the centres the canvas is on top of first, nearest the screen centre, + // so the first pick usually hits. + const uncovered = ([x, y]) => + document.elementFromPoint(rect.left + x, rect.top + y) === canvas; + const fromMiddle = ([x, y]) => + (x - rect.width / 2) ** 2 + (y - rect.height / 2) ** 2; + centres.sort( + (a, b) => + Number(uncovered(b)) - Number(uncovered(a)) || + fromMiddle(a) - fromMiddle(b), + ); + const candidates = []; + for (const [cx, cy] of centres) { + candidates.push([cx, cy]); + for (let r = 2; r <= 16; r += 2) for (const [dx, dy] of [ - [0, 0], [r, 0], [-r, 0], [0, r], [0, -r], - ]) { - const hit = tryAt(cx + dx, cy + dy); - if (hit) return hit; - } - if (points || billboardIds) return null; - for (let y = rect.height * 0.25; y < rect.height * 0.8; y += 18) - for (let x = rect.width * 0.25; x < rect.width * 0.7; x += 6) { - const hit = tryAt(x, y); - if (hit) return hit; + ]) + candidates.push([cx + dx, cy + dy]); + } + // Aimed at known places: never fall back to a scan of the whole screen. + if (!points && !billboardIds && billboardPrefix === null) + for (let y = rect.height * 0.25; y < rect.height * 0.8; y += 18) + for (let x = rect.width * 0.25; x < rect.width * 0.7; x += 6) + candidates.push([x, y]); + // One evaluation stays well inside the caller's wait: past the budget it + // returns null, and the next poll carries on from where this one stopped. + const key = JSON.stringify([prefix, id, billboardPrefix]); + const cursors = (window.__qaPickCursors ??= {}); + const deadline = performance.now() + budgetMs; + let at = cursors[key] ?? 0; + if (at >= candidates.length) at = 0; + for (; at < candidates.length; at++) { + const hit = tryAt(...candidates[at]); + if (hit) { + delete cursors[key]; + return hit; } + if (performance.now() > deadline) { + cursors[key] = at + 1; + return null; + } + } + delete cursors[key]; return null; } @@ -1542,22 +1574,28 @@ async function main() { ); if (fixtures) assert.equal(sequence.images, fixture.images.length, 'every photo'); - // The cones are on the globe: one of them picks as an image. + // The cones are on the globe: one of them picks as an image. Aim at + // where each cone billboard is drawn (its ground position at + // height 0 can sit a few pixels off, and every missed pick costs a + // render); the same 30 s as the line pick above. const cone = await page.waitForFunction( findPickPoint, - { timeout: 15_000, polling: 250 }, + { timeout: 30_000, polling: 250 }, { prefix: 'mly:img:', id: null, - points: fixtures - ? fixture.images.map((image) => [image.lon, image.lat]) - : null, + points: null, billboardIds: null, - // Live: the cones' positions are only known from the globe. - billboardPrefix: fixtures ? null : 'mly:img:', + billboardPrefix: 'mly:img:', }, ); - assert.ok(await cone.jsonValue(), 'an image cone is pickable'); + const picked = await cone.jsonValue(); + assert.ok(picked, 'an image cone is pickable'); + if (fixtures) + assert.ok( + fixture.images.some((image) => image.id === picked.id), + `the cone is a photo of the drive: ${picked.id}`, + ); await page.evaluate(() => window.__godsEyeView.dataManager.layers .get('street-level') @@ -1685,7 +1723,9 @@ async function main() { assert.ok(neighbour, 'a neighbour of the other camera type'); const handle = await page.waitForFunction( findPickPoint, - { timeout: 15_000, polling: 250 }, + // A pick is a render; under SwiftShader on a busy machine one + // evaluation can take seconds. + { timeout: 30_000, polling: 250 }, { prefix: 'mly:img:', id: neighbour.id, diff --git a/src/layers/streetLevel/providers/kartaview/index.js b/src/layers/streetLevel/providers/kartaview/index.js index 0435b28d8..da093112b 100644 --- a/src/layers/streetLevel/providers/kartaview/index.js +++ b/src/layers/streetLevel/providers/kartaview/index.js @@ -206,8 +206,13 @@ export function createKartaViewProvider({ ), metresBetween, ); - // The open photo may have just learnt which way it looked. - viewer.refreshPose?.(); + // The open photo may have just learnt which way it looked. The + // page is good whatever the viewer is doing. + try { + viewer.refreshPose?.(); + } catch { + /* the next pose carries the direction */ + } return { page, hasMore, photos: list }; }); pages.set(key, entry); @@ -362,10 +367,17 @@ export function createKartaViewProvider({ const cones = createSequenceCones({ state, async loadImages(sequenceId, { signal }) { - const anchor = anchors.get(sequenceId); + let anchor = anchors.get(sequenceId); if (!anchor) return []; - const window = await windowOf(anchor); + let window = await windowOf(anchor); signal?.throwIfAborted(); + // Another photo of the drive opened while that page loaded (its + // re-centre found nothing drawn yet): centre on the newest one. + while (anchors.get(sequenceId) !== anchor) { + anchor = anchors.get(sequenceId); + window = await windowOf(anchor); + signal?.throwIfAborted(); + } drawnPage = { sequenceId, page: window.home }; countPhotos(sequenceId); return thinImages( diff --git a/src/layers/streetLevel/providers/kartaview/provider.test.mjs b/src/layers/streetLevel/providers/kartaview/provider.test.mjs index 055c3b1a6..ea82db277 100644 --- a/src/layers/streetLevel/providers/kartaview/provider.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/provider.test.mjs @@ -859,3 +859,104 @@ test('the marker is KartaView’s own cone, and it points along the drive, not a `then along the drive: ${bearings}`, ); }); + +/** Hold `seq`'s page `page` until the returned function is called. */ +function holdPage(source, seq, page) { + const getPage = source.getSequencePhotos; + let release; + const gate = new Promise((resolve) => (release = resolve)); + source.getSequencePhotos = async (id, options) => { + if (id === seq && options.page === page) await gate; + return getPage(id, options); + }; + return () => release(); +} + +test('a photo opened while the cones first load is the one they centre on', async (t) => { + const size = SEQUENCE_PAGE_SIZE; + const env = await mounted({ + sequences: { s: Array.from({ length: size * 5 }, (_, i) => i) }, + }); + const cones = await withCones(t, env); + const release = holdPage(env.source, 's', 1); + await env.instance.viewer.open('s-10'); + const selected = env.instance.selectSequence('s'); + // A line click further along the drive while page 1 is still loading: + // nothing is drawn yet for its re-centre to move. + await env.instance.viewer.open(`s-${size * 4 + 10}`); + release(); + await selected; + assert.ok( + await until( + () => !env.instance.sequenceStats().loading && cones.count() > 0, + ), + ); + assert.ok( + cones()[0] <= size * 4 + 10 && cones()[1] >= size * 4 + 10, + `around the newer photo, not the first: ${cones()}`, + ); +}); + +test('a sequence page that lands while a lost GPU context recovers still draws its cones', async (t) => { + const env = setup({ sequences: { p: [0, 1, 2, 3] } }); + const sphere = (record) => ({ + ...record, + projection: 'SPHERE', + fieldOfView: '360', + filepathProc: `2022/10/6/wrapped_proc/${record.id}.jpg`, + }); + const getPhoto = env.source.getPhoto; + env.source.getPhoto = async (id) => sphere(await getPhoto(id)); + const getPage = env.source.getSequencePhotos; + env.source.getSequencePhotos = async (...args) => { + const page = await getPage(...args); + return { ...page, photos: page.photos.map(sphere) }; + }; + const release = holdPage(env.source, 'p', 1); + let holdImages = false; + let releaseImage = null; + const view = { yaw: 0, pitch: 0, fov: 80 }; + const definition = createKartaViewProvider({ + source: env.source, + viewerOptions: { + load: async (url) => { + if (holdImages) await new Promise((r) => (releaseImage = r)); + return { src: url }; + }, + createRenderer: () => ({ + setImage() {}, + setView: (next) => Object.assign(view, next), + getView: () => ({ ...view }), + resize() {}, + clear() {}, + destroy() {}, + }), + }, + }); + const { context, actions } = fakeContext(); + const errors = []; + context.actions.reportError = (message) => errors.push(message); + const instance = definition.create(context); + actions.instance = instance; + const cones = await withCones(t, { instance }); + const host = fakeHost(); + await instance.viewer.mount(host); + await instance.viewer.open('p-0'); + // › to the next panorama; the GPU resets while it loads. + holdImages = true; + const opening = instance.viewer.open('p-1'); + assert.ok(await until(() => releaseImage !== null)); + host.children[0].children[1].fire('webglcontextlost'); + const selected = instance.selectSequence('p'); + release(); + await selected; + assert.ok( + await until(() => !instance.sequenceStats().loading && cones.count() > 0), + 'the page is drawn, not dropped', + ); + releaseImage(); + await opening; + assert.equal(host.children[0].dataset.kind, 'pano', 'p-1 shows'); + // reportError(null) clears; nothing was reported. + assert.deepEqual(errors.filter(Boolean), []); +}); diff --git a/src/layers/streetLevel/providers/kartaview/source.js b/src/layers/streetLevel/providers/kartaview/source.js index 4c1b47ff8..4283cf859 100644 --- a/src/layers/streetLevel/providers/kartaview/source.js +++ b/src/layers/streetLevel/providers/kartaview/source.js @@ -145,8 +145,10 @@ export function createKartaViewSource({ ), timeoutMs, ); + let response = null; + let read = false; try { - const response = await fetchImpl(url, { signal: controller.signal }); + response = await fetchImpl(url, { signal: controller.signal }); if (response.status === 429) throw new KartaViewSourceError('KartaView is rate-limiting requests', { status: 429, @@ -166,6 +168,7 @@ export function createKartaViewSource({ // KartaView answers chunked, without a length: count while reading. const reader = response.body?.getReader?.(); if (!reader) { + read = true; try { return await response.json(); } catch (error) { @@ -175,6 +178,7 @@ export function createKartaViewSource({ }); } } + read = true; const chunks = []; let bytes = 0; for (;;) { @@ -209,6 +213,13 @@ export function createKartaViewSource({ } finally { clearTimeout(timer); signal?.removeEventListener('abort', onAbort); + // An answer turned away unread (429, an error, too long) still holds + // its connection, one of the six the browser has: let it go. + if (response && !read) { + controller.abort(); + if (response.body && !response.body.locked) + response.body.cancel?.().catch?.(() => {}); + } } } diff --git a/src/layers/streetLevel/providers/kartaview/source.test.mjs b/src/layers/streetLevel/providers/kartaview/source.test.mjs index 5ad3f6cb8..4aed435c7 100644 --- a/src/layers/streetLevel/providers/kartaview/source.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/source.test.mjs @@ -595,3 +595,62 @@ test('a sequence page gets the full 30 s on its first try, then 30 s once more', assert.match(String(outcome?.error?.message), /did not answer in time/); assert.equal(outcome.at, 2 * PAGE_TIMEOUT_MS); }); + +test('an answer turned away unread (an error, a 429, a length past the cap) lets its connection go', async () => { + /** A response whose body is never read: records its cancel and the abort. */ + const unread = (answer) => { + const seen = { cancelled: false, signal: null }; + const fetchImpl = async (url, { signal }) => { + seen.signal = signal; + return { + ...answer, + body: { + locked: false, + getReader: () => { + throw new Error('not read'); + }, + cancel: async () => { + seen.cancelled = true; + }, + }, + }; + }; + return { seen, fetchImpl }; + }; + const cases = [ + [{ ok: false, status: 503, headers: new Map() }, /HTTP 503/], + [ + { + ok: true, + status: 200, + headers: new Map([['content-length', String(64 * 1024 * 1024)]]), + }, + /too much data/, + ], + ]; + for (const [answer, message] of cases) { + const { seen, fetchImpl } = unread(answer); + const source = createKartaViewSource({ fetchImpl }); + await assert.rejects(source.getTile({ x: 1, y: 2, z: 13 }), message); + assert.equal(seen.signal.aborted, true, `${message}: request aborted`); + assert.equal(seen.cancelled, true, `${message}: body cancelled`); + } + // A 429 is let go before its retry is asked. + const { seen, fetchImpl } = unread({ + ok: false, + status: 429, + headers: new Map([['retry-after', '0']]), + }); + const signals = []; + const source = createKartaViewSource({ + fetchImpl: async (url, init) => { + signals.push(init.signal); + if (signals.length === 1) return fetchImpl(url, init); + return ok(photoPayload([])); + }, + }); + await source.getPhoto('1'); + assert.equal(signals[0].aborted, true); + assert.equal(seen.cancelled, true); + assert.equal(signals[1].aborted, false, 'a read answer is left alone'); +}); diff --git a/src/layers/streetLevel/providers/kartaview/viewer.js b/src/layers/streetLevel/providers/kartaview/viewer.js index a9beb52c5..9aa4639c5 100644 --- a/src/layers/streetLevel/providers/kartaview/viewer.js +++ b/src/layers/streetLevel/providers/kartaview/viewer.js @@ -73,6 +73,11 @@ async function loadBitmap(url, timeoutMs) { } } +/** Free a decoded bitmap nobody will draw (an has nothing to free). */ +function release(image) { + image?.close?.(); +} + /** The panorama size a canvas this wide can show sharply at this field of view. */ export function panoSizeFor(canvasPixels, fovDeg) { const needed = (Math.max(1, canvasPixels) * 360) / Math.max(1, fovDeg); @@ -130,6 +135,10 @@ export function createKartaViewViewer({ let panoWidth = 0; /** The flat photo on screen is the 2,560 px version. */ let flatSharp = false; + /** The sharper version loading ({seq, kind}): a resize starts no second one. */ + let upgrading = null; + /** A resize came while it loaded: look again once it lands. */ + let upgradeAgain = false; /** 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. */ @@ -143,6 +152,9 @@ export function createKartaViewViewer({ function emit() { if (!photo || !showing) return; + // A lost GPU context, still being recovered: the open that redraws the + // panorama poses once it is on screen. + if (showing === 'pano' && !renderer) return; const view = showing === 'pano' ? renderer.getView() : null; const heading = cameraBearing(photo); const pose = { @@ -485,20 +497,40 @@ export function createKartaViewViewer({ ? resizedImageUrl(target.urls.pano, size) : resizedImageUrl(target.urls.full, size); if (!url) return; + // Resizes come in bursts while the panel animates: one load at a time + // for this photo, and one more look once it lands. + if (upgrading?.seq === seq && upgrading.kind === kind) { + upgradeAgain = true; + return; + } + const pending = (upgrading = { seq, kind }); + upgradeAgain = false; + let image = null; try { - const image = await load(url, { crossOrigin: kind === 'pano' }); + image = await load(url, { crossOrigin: kind === 'pano' }); if (seq !== openSeq || photo !== target || showing !== kind) return; if (kind === 'pano') { if (!renderer || size[0] <= panoWidth) return; renderer.setImage(image); + image = null; panoWidth = size[0]; } else { picture.src = image.src; + image = null; flatSharp = true; } requestRender(); } catch { /* the first version stays */ + } finally { + release(image); + if (upgrading === pending) { + upgrading = null; + if (upgradeAgain && seq === openSeq && photo === target) { + upgradeAgain = false; + upgrade(target, seq, showing); + } + } } } @@ -524,10 +556,16 @@ export function createKartaViewViewer({ resizedImageUrl(target.urls.pano, PANO_PREVIEW_SIZE), { crossOrigin: true }, ); - if (seq !== openSeq) return false; + if (seq !== openSeq) { + release(image); + return false; + } // The context may have been lost while this loaded: build on the new // canvas, or let the caller show the front view. - if (!ensureRenderer()) throw new Error(LOAD_FAILED); + if (!ensureRenderer()) { + release(image); + throw new Error(LOAD_FAILED); + } const restore = restoreView?.id === target.id ? restoreView.view : null; restoreView = null; if (restore) renderer.setView(restore); diff --git a/src/layers/streetLevel/providers/kartaview/viewer.test.mjs b/src/layers/streetLevel/providers/kartaview/viewer.test.mjs index 519e76615..1391b727c 100644 --- a/src/layers/streetLevel/providers/kartaview/viewer.test.mjs +++ b/src/layers/streetLevel/providers/kartaview/viewer.test.mjs @@ -105,8 +105,11 @@ function setup({ rendererThrows = false, steps = {}, failSteps = false, + hold = () => false, } = {}) { const loads = []; + const held = []; + const closed = []; const renderers = []; const stepped = []; const asked = []; @@ -130,7 +133,10 @@ function setup({ loads.push({ url, crossOrigin: options?.crossOrigin }); if (failLoads.some((part) => url.includes(part))) throw new Error('The KartaView photo could not be loaded'); - return { src: url }; + // A decoded bitmap: close() frees it. + const image = { src: url, close: () => closed.push(url) }; + if (!hold(url)) return image; + return new Promise((resolve) => held.push(() => resolve(image))); }, createRenderer: (canvas) => { attempts.renderer++; @@ -152,6 +158,8 @@ function setup({ stepped, asked, attempts, + held, + closed, }; } @@ -923,3 +931,54 @@ test('a docked viewer keeps the 1,280 px photo; expanding it brings the 2,560 px for (let i = 0; i < 4; i++) await settle(); assert.equal(loads.length, 2, 'once only'); }); + +test('resizes while a sharper panorama loads start no second load; the last size wins once it lands', async () => { + let first = true; + const { viewer, host, loads, renderers, held } = setup({ + hold: (url) => { + const holdIt = first && url.includes('rs:fit:2048'); + if (holdIt) first = false; + return holdIt; + }, + }); + await viewer.mount(host); + await viewer.open('pano1'); + assert.ok(await until(() => held.length === 1)); + // The panel animates open: a burst of resizes while 2,048 px loads. + parts(host).canvas.clientWidth = 1400; + for (let i = 0; i < 5; i++) viewer.resize(); + for (let i = 0; i < 4; i++) await settle(); + assert.equal(loads.length, 2, 'the preview and the one 2,048 px load'); + held[0](); + assert.ok(await until(() => renderers[0].images.length === 3)); + assert.match(renderers[0].images[1], /rs:fit:2048:1024/); + assert.match(renderers[0].images[2], /rs:fit:4096:2048/); + for (let i = 0; i < 4; i++) await settle(); + assert.equal(loads.length, 3, 'one more load, for the size asked last'); +}); + +test('a preview or sharper version nobody will draw is freed, not left decoded', async () => { + const { viewer, host, renderers, held, closed } = setup({ + hold: (url) => /rs:fit:(1024|2048):/.test(url), + }); + await viewer.mount(host); + // pano1's preview is still loading when flat1 opens over it. + const overtaken = viewer.open('pano1'); + assert.ok(await until(() => held.length === 1)); + await viewer.open('flat1'); + held[0](); + await overtaken; + assert.deepEqual(closed.length, 1); + assert.match(closed[0], /rs:fit:1024/, 'the overtaken preview'); + // pano1 shows; its sharp version lands after flat1 replaced it. + const shown = viewer.open('pano1'); + assert.ok(await until(() => held.length === 2)); + held[1](); + await shown; + assert.ok(await until(() => held.length === 3)); + await viewer.open('flat1'); + held[2](); + assert.ok(await until(() => closed.length === 2)); + assert.match(closed[1], /rs:fit:2048/, 'the overtaken sharp version'); + assert.equal(renderers[0].images.length, 1, 'only the preview was drawn'); +});