diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index a1afcbb40..d51be986b 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2962,7 +2962,9 @@ camera, `meshSampler.js` samples the rendered surface (`scene.sampleHeight`, steep streets and roads under trees. It uses the mesh floor's rules (the visible tileset has finished streaming, a real bare-earth prior, the shared mesh window) and probes a cell again once the camera is half as far from it, -down to 40 m, so coarse early samples are replaced as finer tiles load. Roads on elevated decks are still drawn at +down to 40 m, so coarse early samples are replaced as finer tiles load. A +failed probe keeps the last good sample and distance and is retried after the +miss cooldown. Roads on elevated decks are still drawn at ground level. Framing a photo ignores mesh samples far below the bare earth (unloaded tiles). @@ -2970,10 +2972,12 @@ The header pill is the layer switch. With one provider its chip is a layer switch too; with several, darkening the last lit chip turns the layer off. The viewer sits under the header with EXPAND, FIT/FILL, FOLLOW and close above the image. FOLLOW needs the Google 3D map stack (`attachMapStackController`) and -stops when the stack changes. Framing a photo and turning FOLLOW on claim the -camera through the application's navigation (`attachNavigation`), which -releases aircraft and satellite tracking (and is refused in the cockpit); -FOLLOW stops when another feature takes the camera. SINCE is a stepped slider whose readout names the +stops when the stack changes. Camera moves go through the application's +navigation (`attachNavigation`), which releases aircraft and satellite tracking +and is refused in the cockpit: turning FOLLOW on claims the camera at once, and +FOLLOW stops when another feature takes it; a photo takes a deferred ticket +when it starts opening and frames only if nothing newer took the camera while +it loaded. SINCE is a stepped slider whose readout names the cut-off date. The panel is portable (`street-level-panel` spec, minimum 320 × 280, `dockOnCollapse`): a floating window gives spare height to the viewer, and collapsing it, double-clicking the header or SHRINK after EXPAND diff --git a/src/layers/streetLevel/cameraFollow.js b/src/layers/streetLevel/cameraFollow.js index 3e60ec78f..09733e172 100644 --- a/src/layers/streetLevel/cameraFollow.js +++ b/src/layers/streetLevel/cameraFollow.js @@ -16,6 +16,8 @@ export function createCameraFollow({ state, parts }) { /** * The application's camera authority (`attachNavigation`): `run(noun, move)` * releases tracking and other owners before `move`, or refuses (cockpit); + * `begin(noun)` takes a deferred ticket (a generation, or false) that + * `reassert(generation)` honours only if nothing newer took the camera; * `subscribeHandoff` reports a newer owner. Without it, moves run directly. */ let navigation = null; @@ -42,6 +44,21 @@ export function createCameraFollow({ state, parts }) { } } + /** + * Ask for the camera as a photo starts opening. The ticket goes to + * `lookAtPosition`; null when the application refuses (cockpit). + */ + function beginFraming() { + if (!navigation?.begin || !navigation.reassert) return { generation: null }; + claiming = true; + try { + const generation = navigation.begin('photo'); + return generation === false ? null : { generation }; + } finally { + claiming = false; + } + } + /** Another feature took the camera: stop following it. */ function onHandoff() { if (claiming || !state.street.follow) return; @@ -108,12 +125,20 @@ export function createCameraFollow({ state, parts }) { /** Our framing flight while it is current; Cesium calls `cancel` when another flight starts. */ let framing = null; - /** Frame the current image from a short distance behind it. */ - function lookAtPosition() { + /** + * Frame the current image from a short distance behind it, unless another + * action took the camera since `ticket` (from `beginFraming`) was issued. + */ + function lookAtPosition(ticket) { const { position, bearing, altitude } = state.street; - if (!position || !state.viewer) return; + if (!ticket || !position || !state.viewer) return; + if ( + ticket.generation !== null && + !navigation?.reassert?.(ticket.generation) + ) + return; const ground = groundHeightAt(position.lon, position.lat, altitude); - claim(() => { + const fly = () => { // Owned before the call: starting it cancels the previous flight (ours // included) synchronously, and a zero-length one completes at once. const flight = {}; @@ -137,7 +162,10 @@ export function createCameraFollow({ state, parts }) { cancel: release, }, ); - }); + }; + // A reasserted ticket has already released the other owners. + if (ticket.generation === null) claim(fly); + else fly(); } /** Stop our framing flight if it is still in the air; another feature's flight is left alone. */ @@ -171,6 +199,7 @@ export function createCameraFollow({ state, parts }) { return { followCamera, + beginFraming, lookAtPosition, cancelFraming, setFollow, diff --git a/src/layers/streetLevel/cameraFollow.test.mjs b/src/layers/streetLevel/cameraFollow.test.mjs index 75f7ef384..365858e4e 100644 --- a/src/layers/streetLevel/cameraFollow.test.mjs +++ b/src/layers/streetLevel/cameraFollow.test.mjs @@ -79,7 +79,7 @@ function setup({ sampled, dem = null, globe = null, altitude = 149 }) { test('framing a photo ignores a sample kilometres underground and uses bare earth', () => { const { follow, flights } = setup({ sampled: -14_886, dem: 117 }); - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); assert.equal(flights.length, 1); const centre = heightOf(flights[0].sphere.center); assert.ok( @@ -90,7 +90,7 @@ test('framing a photo ignores a sample kilometres underground and uses bare eart test('a plausible mesh sample near bare earth wins (a street, or a modest roof)', () => { const { follow, flights } = setup({ sampled: 121, dem: 117 }); - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); assert.ok(Math.abs(heightOf(flights[0].sphere.center) - 123) < 0.01); }); @@ -101,7 +101,7 @@ test('without bare earth an absurd sample falls back to the image altitude, neve globe: undefined, altitude: 149, }); - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); assert.ok(Math.abs(heightOf(flights[0].sphere.center) - 151) < 0.01); }); @@ -119,15 +119,15 @@ test('following stands the camera at eye height above the checked ground', () => test('cancelFraming stops the framing flight while it is still ours', () => { const { follow, state } = setup({ sampled: 121, dem: 117 }); const { camera } = state.viewer; - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); follow.cancelFraming(); assert.equal(camera.cancelled, 1, 'the flight toward the closed photo stops'); follow.cancelFraming(); assert.equal(camera.cancelled, 1, 'and only once'); // Re-framing (the next photo) supersedes the first flight, not the claim. - follow.lookAtPosition(); - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); + follow.lookAtPosition(follow.beginFraming()); follow.cancelFraming(); assert.equal(camera.cancelled, 2); }); @@ -135,12 +135,12 @@ test('cancelFraming stops the framing flight while it is still ours', () => { test('cancelFraming leaves a landed flight and a newer navigation flight alone', () => { const { follow, state, flights } = setup({ sampled: 121, dem: 117 }); const { camera } = state.viewer; - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); camera.land(); follow.cancelFraming(); assert.equal(camera.cancelled, 0, 'landed: nothing to stop'); - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); // A search result flies the globe elsewhere; Cesium cancels ours first. camera.flyToBoundingSphere(null, {}); assert.equal(flights.length, 3); @@ -151,7 +151,7 @@ test('cancelFraming leaves a landed flight and a newer navigation flight alone', test('turning FOLLOW on stops the framing flight so it cannot fight the follow view', () => { const { follow, state, views } = setup({ sampled: 121, dem: 117 }); const { camera } = state.viewer; - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); follow.setFollow(true); assert.equal(camera.cancelled, 1, 'the framing tween stops'); assert.equal(views.length, 1, 'and the camera stands at the photo'); @@ -163,43 +163,73 @@ test('turning FOLLOW on stops the framing flight so it cannot fight the follow v */ function navigationFor(state, { refuse = false } = {}) { const listeners = new Set(); + let generation = 0; + const stamp = () => { + generation++; + for (const listener of listeners) listener({ generation }); + return generation; + }; const nav = { runs: [], + begins: [], run(noun, move) { nav.runs.push(noun); if (refuse) return false; - for (const listener of listeners) - listener({ generation: nav.runs.length }); + const current = stamp(); state.viewer.trackedEntity = undefined; - return move(nav.runs.length); + return move(current); + }, + /** Deferred: stamp now, release only on a successful reassert. */ + begin(noun) { + nav.begins.push(noun); + return refuse ? false : stamp(); + }, + reassert(ticket) { + if (ticket !== generation) return false; + state.viewer.trackedEntity = undefined; + return true; }, subscribeHandoff(listener) { listeners.add(listener); return () => listeners.delete(listener); }, - handOff() { - for (const listener of listeners) listener({ generation: -1 }); - }, + /** Another feature takes the camera. */ + handOff: () => stamp(), listeners, }; return nav; } -test('framing a photo claims the camera through the application, releasing tracking', () => { +test('a photo claims the camera when it starts opening and frames once loaded, releasing tracking', () => { const { follow, state, flights } = setup({ sampled: 160 }); state.viewer.trackedEntity = { id: 'aircraft' }; const nav = navigationFor(state); follow.attachNavigation(nav); - follow.lookAtPosition(); - assert.deepEqual(nav.runs, ['photo']); + const ticket = follow.beginFraming(); + assert.deepEqual(nav.begins, ['photo']); + assert.ok(state.viewer.trackedEntity, 'nothing is released while it loads'); + follow.lookAtPosition(ticket); assert.equal(state.viewer.trackedEntity, undefined); assert.equal(flights.length, 1); }); +test('a photo that finishes loading after newer navigation does not frame', () => { + const { follow, state, flights } = setup({ sampled: 160 }); + const nav = navigationFor(state); + follow.attachNavigation(nav); + const ticket = follow.beginFraming(); + // The user flies to Sacramento, and then tracks an aircraft, while it loads. + nav.handOff(); + state.viewer.trackedEntity = { id: 'aircraft' }; + follow.lookAtPosition(ticket); + assert.equal(flights.length, 0, 'no flight back to the photo'); + assert.deepEqual(state.viewer.trackedEntity, { id: 'aircraft' }); +}); + test('a refused claim (cockpit) neither frames the photo nor turns FOLLOW on', () => { const { follow, state, flights, views } = setup({ sampled: 160 }); follow.attachNavigation(navigationFor(state, { refuse: true })); - follow.lookAtPosition(); + follow.lookAtPosition(follow.beginFraming()); follow.setFollow(true); assert.equal(flights.length, 0); assert.equal(views.length, 0); diff --git a/src/layers/streetLevel/index.js b/src/layers/streetLevel/index.js index 9ec25f654..dd4478c58 100644 --- a/src/layers/streetLevel/index.js +++ b/src/layers/streetLevel/index.js @@ -414,9 +414,11 @@ export function createStreetLevelLayer({ }, /** - * The application's camera authority: `run(noun, move)` and - * `subscribeHandoff(listener)`. Framing and FOLLOW claim the camera - * through it, and FOLLOW stops when another feature takes the camera. + * The application's camera authority: `run(noun, move)`, the deferred + * `begin(noun)` / `reassert(generation)` pair and + * `subscribeHandoff(listener)`. FOLLOW claims the camera with `run`; a + * photo claims it when opening starts and frames only if it still owns + * it once loaded. FOLLOW stops when another feature takes the camera. */ attachNavigation(navigation) { parts.follow.attachNavigation(navigation); diff --git a/src/layers/streetLevel/meshSampler.js b/src/layers/streetLevel/meshSampler.js index fec297a69..2711e8c68 100644 --- a/src/layers/streetLevel/meshSampler.js +++ b/src/layers/streetLevel/meshSampler.js @@ -317,13 +317,11 @@ export function createMeshSampler({ height = validated(height, lon, lat); const previous = heights.get(key); if (height === undefined) { - // A failed refresh keeps the old sample, and waits for a closer camera. - if (previous !== undefined) - probedFrom.set(key, cameraDistance(ground, centre, previous)); - else { - if (misses.size >= cacheMax) misses.clear(); - misses.set(key, Date.now() + MESH_MISS_RETRY_MS); - } + // A miss, or a failed refresh: the old sample (if any) and the + // distance it was taken from stay, and the probe is retried after + // the same cooldown. + if (misses.size >= cacheMax) misses.clear(); + misses.set(key, Date.now() + MESH_MISS_RETRY_MS); continue; } if (previous === undefined && heights.size >= cacheMax) forget(); diff --git a/src/layers/streetLevel/meshSampler.test.mjs b/src/layers/streetLevel/meshSampler.test.mjs index 315cd60ba..065c22645 100644 --- a/src/layers/streetLevel/meshSampler.test.mjs +++ b/src/layers/streetLevel/meshSampler.test.mjs @@ -374,3 +374,32 @@ test('a sample needs a real bare-earth prior and must sit in the shared mesh win unknown.destroy(); outside.destroy(); }); + +test('a failed refresh keeps the last good distance and is retried after the cooldown', (t) => { + t.mock.timers.enable({ apis: ['setTimeout', 'Date'] }); + const { viewer, probes } = liveViewer({ surface: 90, height: 700 }); + viewer.camera.positionCartographic.longitude = 10.001 * RAD; + const sampler = liveSampler(viewer); + sampler.setEnabled(true); + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(sampler.meshAt(10.001, 50), 90); + // The camera comes down to 20 m and the refresh misses (tiles in flux). + viewer.camera.positionCartographic.height = 110; + viewer.surface = undefined; + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(probes.length, 2); + assert.equal(sampler.meshAt(10.001, 50), 90, 'the old sample stays'); + viewer.surface = 100; + t.mock.timers.tick(MESH_MISS_RETRY_MS - 100); + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(probes.length, 2, 'not before the cooldown'); + t.mock.timers.tick(100); + sampler.request([[10.001, 50]]); + probeNow(t); + assert.equal(probes.length, 3, 'retried after it'); + assert.equal(sampler.meshAt(10.001, 50), 100); + sampler.destroy(); +}); diff --git a/src/layers/streetLevel/viewerHost.js b/src/layers/streetLevel/viewerHost.js index 233b78d6d..98c9e0a41 100644 --- a/src/layers/streetLevel/viewerHost.js +++ b/src/layers/streetLevel/viewerHost.js @@ -113,6 +113,9 @@ export function createViewerHost({ state, parts }) { } const seq = ++openSeq; const current = () => seq === openSeq; + // Claimed now, honoured after loading only if nothing newer took the camera. + const ticket = + frame && !state.street.follow ? parts.follow.beginFraming() : null; Object.assign(state.street, { loading: true, error: null, @@ -127,7 +130,7 @@ export function createViewerHost({ state, parts }) { if (!current()) return false; await adapter.open(String(imageId)); if (!current()) return false; - if (frame && !state.street.follow) parts.follow.lookAtPosition(); + if (ticket && !state.street.follow) parts.follow.lookAtPosition(ticket); } catch (error) { if (current()) state.street.error = error?.message || 'Image could not be opened'; diff --git a/src/layers/streetLevel/viewerHost.test.mjs b/src/layers/streetLevel/viewerHost.test.mjs index 5668f715c..76830b1ae 100644 --- a/src/layers/streetLevel/viewerHost.test.mjs +++ b/src/layers/streetLevel/viewerHost.test.mjs @@ -54,7 +54,7 @@ function harness(adapter) { sequenceStats: () => ({ selectedId }), }; state.providers.set('mapillary', { def, instance }); - const framing = { started: 0, cancelled: 0 }; + const framing = { begun: 0, started: 0, cancelled: 0 }; const marker = []; const followed = { count: 0 }; const parts = { @@ -65,7 +65,8 @@ function harness(adapter) { }, follow: { followCamera: () => followed.count++, - lookAtPosition: () => framing.started++, + beginFraming: () => ({ generation: ++framing.begun }), + lookAtPosition: (ticket) => ticket && framing.started++, cancelFraming: () => framing.cancelled++, }, }; @@ -557,3 +558,21 @@ test('back-to-back Mapillary opens download only the newer image', async () => { ['b'], ); }); + +test('a photo asks for the camera when it starts opening and frames with that ticket once loaded', async () => { + const openGate = deferred(); + const adapter = fakeAdapter({ openGate }); + const { state, host, framing } = harness(adapter); + const opening = host.open('mapillary', 'a'); + assert.equal(framing.begun, 1, 'claimed before the download'); + await settle(); + assert.equal(framing.started, 0); + openGate.resolve(); + assert.equal(await opening, true); + assert.equal(framing.started, 1); + + // FOLLOW owns the camera already: an open neither claims nor frames. + state.street.follow = true; + assert.equal(await host.open('mapillary', 'b'), true); + assert.deepEqual([framing.begun, framing.started], [1, 1]); +}); diff --git a/src/ui/applicationShell.js b/src/ui/applicationShell.js index ec8125cdd..d400b54c2 100644 --- a/src/ui/applicationShell.js +++ b/src/ui/applicationShell.js @@ -942,6 +942,8 @@ export class StyleManager extends ShellFacade { // Photo framing and FOLLOW release tracking like any explicit flight. streetLevelLayer.attachNavigation?.({ run: (noun, move) => this._runExplicitNavigation(noun, move), + begin: (noun) => this._beginDeferredNavigation(noun), + reassert: (generation) => this._reassertNavigationHandoff(generation), subscribeHandoff: (listener) => this._navigation.subscribeCameraHandoff(listener), });