mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-06 03:14:00 +00:00
fix(street-level): settle loading on pan, keep zoom 0, drop late poses
Fixes from gekh's re-review of 061d269, each with a regression test that
fails on the old code:
- A pan that drops a tile still loading now settles the loading count
(the aborted request no longer owns its key, so its `finally` never
did), and purges the previous zoom's tiles once nothing is pending.
LOADING no longer sticks on the status pill and chip.
- Zoom 0 (above 15,000 km) is a zoom: `zoom == null` instead of `!zoom`.
- A pose that arrives after a photo was closed is dropped, in the
MapillaryJS adapter and in the viewer host, so the marker and sequence
selection do not come back.
- Ground casting passes the tile's signal to the terrain request, so a
cancelled tile stops its height batch instead of holding the queue.
- The server's in-memory tile cache honours the 24 h TTL like the disk.
- The viewer's one-time resize runs for every photo, not just the first.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
061d2698f5
commit
a5e16e9470
11 files changed
+115
-6
No files matched your search
@@ -77,6 +77,13 @@ const _inFlight = new Map();
|
|||||||
function memoryGet(key) {
|
function memoryGet(key) {
|
||||||
const hit = _memory.get(key);
|
const hit = _memory.get(key);
|
||||||
if (!hit) return null;
|
if (!hit) return null;
|
||||||
|
// Same 24 h life as the disk copy: a long-running server must not keep
|
||||||
|
// serving stale (or empty) coverage until eviction.
|
||||||
|
if (Date.now() - hit.at > TILE_DISK_TTL_MS) {
|
||||||
|
_memory.delete(key);
|
||||||
|
_memoryBytes -= hit.bytes.length;
|
||||||
|
return null;
|
||||||
|
}
|
||||||
// Re-insert to mark as most recently used.
|
// Re-insert to mark as most recently used.
|
||||||
_memory.delete(key);
|
_memory.delete(key);
|
||||||
_memory.set(key, hit);
|
_memory.set(key, hit);
|
||||||
|
|||||||
@@ -181,6 +181,7 @@ export function createGroundCaster({
|
|||||||
lon: Number((i * step).toFixed(6)),
|
lon: Number((i * step).toFixed(6)),
|
||||||
lat: Number((j * step).toFixed(6)),
|
lat: Number((j * step).toFixed(6)),
|
||||||
})),
|
})),
|
||||||
|
{ signal },
|
||||||
);
|
);
|
||||||
let complete = true;
|
let complete = true;
|
||||||
missing.forEach(([key], n) => {
|
missing.forEach(([key], n) => {
|
||||||
|
|||||||
@@ -238,3 +238,19 @@ test('a full cache is cleared before a request counts its corners, so the reques
|
|||||||
assert.equal(await caster.prepareLines([line]), true);
|
assert.equal(await caster.prepareLines([line]), true);
|
||||||
assert.ok(caster.castLine(line), 'the whole line casts');
|
assert.ok(caster.castLine(line), 'the whole line casts');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('cancelling a prepare cancels its terrain request', async () => {
|
||||||
|
const seen = [];
|
||||||
|
const caster = createGroundCaster({
|
||||||
|
terrain: {
|
||||||
|
async resolveEllipsoidalGround(coords, options) {
|
||||||
|
seen.push(options?.signal);
|
||||||
|
return coords.map(() => ({ ellipsoid: 100, source: 'reearth' }));
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const controller = new AbortController();
|
||||||
|
await caster.prepare([[10.0005, 50.0005]], { signal: controller.signal });
|
||||||
|
assert.equal(seen.length, 1);
|
||||||
|
assert.equal(seen[0], controller.signal, 'the tile signal reaches the proxy');
|
||||||
|
});
|
||||||
@@ -537,7 +537,7 @@ export function createCoverage({ state, source }) {
|
|||||||
let bbox = visibleBbox(viewer);
|
let bbox = visibleBbox(viewer);
|
||||||
if (kind === 'overview' && (!bbox || zoom <= 1))
|
if (kind === 'overview' && (!bbox || zoom <= 1))
|
||||||
bbox = [-180, -85, 180, 85];
|
bbox = [-180, -85, 180, 85];
|
||||||
if (!zoom || !bbox) {
|
if (zoom == null || !bbox) {
|
||||||
state.coverage.hint =
|
state.coverage.hint =
|
||||||
'Point the camera at the globe for street-level coverage';
|
'Point the camera at the globe for street-level coverage';
|
||||||
clear();
|
clear();
|
||||||
@@ -559,10 +559,14 @@ export function createCoverage({ state, source }) {
|
|||||||
if (!wanted.has(key)) removeTile(key);
|
if (!wanted.has(key)) removeTile(key);
|
||||||
for (const [key, controller] of [...state.coverage.pending])
|
for (const [key, controller] of [...state.coverage.pending])
|
||||||
if (!wanted.has(key)) {
|
if (!wanted.has(key)) {
|
||||||
|
// The aborted request no longer owns its key, so its `finally` will
|
||||||
|
// not settle the count: settle it here.
|
||||||
controller.abort();
|
controller.abort();
|
||||||
state.coverage.pending.delete(key);
|
state.coverage.pending.delete(key);
|
||||||
|
state.coverage.loading = Math.max(0, state.coverage.loading - 1);
|
||||||
}
|
}
|
||||||
for (const tile of tiles) loadTile(tile, kind);
|
for (const tile of tiles) loadTile(tile, kind);
|
||||||
|
if (!state.coverage.pending.size) purgeStale();
|
||||||
// The camera moved: cells that were out of the sampler's range may not be.
|
// The camera moved: cells that were out of the sampler's range may not be.
|
||||||
if (terrainMode())
|
if (terrainMode())
|
||||||
for (const entry of state.coverage.tiles.values())
|
for (const entry of state.coverage.tiles.values())
|
||||||
|
|||||||
@@ -241,3 +241,39 @@ test('a tile whose lines cannot be cast is not rebuilt for nothing', async () =>
|
|||||||
assert.equal(viewer.scene.primitives.items.size, 0);
|
assert.equal(viewer.scene.primitives.items.size, 0);
|
||||||
coverage.clear();
|
coverage.clear();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('panning off a tile that is still loading settles the loading count', async () => {
|
||||||
|
const { viewer, source, state, coverage } = setup();
|
||||||
|
coverage.refresh();
|
||||||
|
assert.equal(state.coverage.loading, 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,
|
||||||
|
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');
|
||||||
|
coverage.clear();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('zoom 0 is a zoom: the whole-earth view still shows coverage', () => {
|
||||||
|
const { viewer, source, state, coverage } = setup();
|
||||||
|
viewer.view.height = 20_000_000;
|
||||||
|
coverage.refresh();
|
||||||
|
assert.equal(state.coverage.zoom, 0);
|
||||||
|
assert.equal(state.coverage.hint, '');
|
||||||
|
assert.deepEqual(
|
||||||
|
source.calls.map((call) => call.key),
|
||||||
|
['0/0/0'],
|
||||||
|
);
|
||||||
|
coverage.clear();
|
||||||
|
});
|
||||||
@@ -69,6 +69,9 @@ export function createMapillaryViewer({ source, render } = {}) {
|
|||||||
viewer.getPosition(),
|
viewer.getPosition(),
|
||||||
viewer.getPointOfView(),
|
viewer.getPointOfView(),
|
||||||
]);
|
]);
|
||||||
|
// Closed (or never opened) while the pose was in flight: a late
|
||||||
|
// `image` event must not bring the photo back.
|
||||||
|
if (pendingOpen === null) return;
|
||||||
if (image) current = describe(image);
|
if (image) current = describe(image);
|
||||||
if (!current || !viewer) return;
|
if (!current || !viewer) return;
|
||||||
emit({
|
emit({
|
||||||
|
|||||||
@@ -25,7 +25,7 @@ export function createViewerHost({ state, parts }) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function applyPose(pose) {
|
function applyPose(pose) {
|
||||||
if (!active || pose.providerId !== active.id) return;
|
if (!active || !state.street.open || pose.providerId !== active.id) return;
|
||||||
const previousSequence = state.street.sequenceId;
|
const previousSequence = state.street.sequenceId;
|
||||||
Object.assign(state.street, {
|
Object.assign(state.street, {
|
||||||
providerId: pose.providerId,
|
providerId: pose.providerId,
|
||||||
|
|||||||
@@ -27,17 +27,22 @@ function fakeAdapter({ failMounts = 0 } = {}) {
|
|||||||
calls.mount++;
|
calls.mount++;
|
||||||
if (calls.mount <= failMounts) throw new Error('library failed to load');
|
if (calls.mount <= failMounts) throw new Error('library failed to load');
|
||||||
},
|
},
|
||||||
async open(id) {
|
/** A pose event from the library, as MapillaryJS fires them. */
|
||||||
calls.open.push(id);
|
emitPose(id) {
|
||||||
emit?.({
|
emit?.({
|
||||||
providerId: 'mapillary',
|
providerId: 'mapillary',
|
||||||
imageId: id,
|
imageId: id,
|
||||||
|
sequenceId: `seq-${id}`,
|
||||||
position: { lon: 1, lat: 2 },
|
position: { lon: 1, lat: 2 },
|
||||||
bearing: 90,
|
bearing: 90,
|
||||||
isPano: false,
|
isPano: false,
|
||||||
externalUrl: `https://example.test/${id}`,
|
externalUrl: `https://example.test/${id}`,
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
async open(id) {
|
||||||
|
calls.open.push(id);
|
||||||
|
this.emitPose(id);
|
||||||
|
},
|
||||||
close() {},
|
close() {},
|
||||||
unmount() {
|
unmount() {
|
||||||
calls.unmount++;
|
calls.unmount++;
|
||||||
@@ -127,3 +132,17 @@ test('switching one provider off releases only its viewer', async () => {
|
|||||||
host.unmount('mapillary');
|
host.unmount('mapillary');
|
||||||
assert.equal(adapter.calls.unmount, 1);
|
assert.equal(adapter.calls.unmount, 1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('a pose that arrives after the photo was closed does not bring it back', async () => {
|
||||||
|
const adapter = fakeAdapter();
|
||||||
|
const { state, host } = harness(adapter);
|
||||||
|
await host.open('mapillary', 'a');
|
||||||
|
host.close();
|
||||||
|
const before = { ...state.street };
|
||||||
|
// The library's `image` event for a photo that was still loading.
|
||||||
|
adapter.emitPose('a');
|
||||||
|
assert.equal(state.street.open, false);
|
||||||
|
assert.equal(state.street.imageId, null);
|
||||||
|
assert.equal(state.street.sequenceId, null);
|
||||||
|
assert.deepEqual(state.street, before);
|
||||||
|
});
|
||||||
@@ -202,6 +202,24 @@ test('fetchTile serves from memory after one upstream fetch and strips the image
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('a tile held in memory past the 24 h TTL is fetched again', async (t) => {
|
||||||
|
const tile = { layer: 'coverage', z: 9, x: 3, y: 3 };
|
||||||
|
await withDeferredUpstream([tile], async ({ upstream, settle }) => {
|
||||||
|
const first = fetchTile(tile);
|
||||||
|
await settle();
|
||||||
|
upstream[0].release();
|
||||||
|
assert.equal((await first).source, 'upstream');
|
||||||
|
assert.equal((await fetchTile(tile)).source, 'memory');
|
||||||
|
const now = Date.now();
|
||||||
|
t.mock.method(Date, 'now', () => now + 25 * 60 * 60 * 1000);
|
||||||
|
const again = fetchTile(tile);
|
||||||
|
await settle();
|
||||||
|
assert.equal(upstream.length, 2, 'neither cache answers a stale tile');
|
||||||
|
upstream[1].release();
|
||||||
|
assert.equal((await again).source, 'upstream');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A deferred upstream for in-flight tests: every fetch waits until released,
|
* A deferred upstream for in-flight tests: every fetch waits until released,
|
||||||
* and records whether its signal was aborted. Coordinates use z9, which the
|
* and records whether its signal was aborted. Coordinates use z9, which the
|
||||||
|
|||||||
@@ -404,6 +404,9 @@ export class StreetLevelControls {
|
|||||||
button.classList.toggle('is-active', active);
|
button.classList.toggle('is-active', active);
|
||||||
button.setAttribute('aria-checked', String(active));
|
button.setAttribute('aria-checked', String(active));
|
||||||
}
|
}
|
||||||
|
// Track open → closed here too, so the next photo gets its resize.
|
||||||
|
const wasOpen = this._viewerWasOpen;
|
||||||
|
this._viewerWasOpen = state.street.open === true;
|
||||||
if (!viewer.open) return;
|
if (!viewer.open) return;
|
||||||
if (el.imageBy) el.imageBy.textContent = viewer.captionLeft;
|
if (el.imageBy) el.imageBy.textContent = viewer.captionLeft;
|
||||||
if (el.imageWhen) el.imageWhen.textContent = viewer.captionRight;
|
if (el.imageWhen) el.imageWhen.textContent = viewer.captionRight;
|
||||||
@@ -415,8 +418,7 @@ export class StreetLevelControls {
|
|||||||
// Size changes reach the viewer through the ResizeObserver; resizing on
|
// Size changes reach the viewer through the ResizeObserver; resizing on
|
||||||
// every render ran each frame while a panorama was dragged. Opening is the
|
// every render ran each frame while a panorama was dragged. Opening is the
|
||||||
// one moment the element may not have reported a size yet.
|
// one moment the element may not have reported a size yet.
|
||||||
if (state.street.open && !this._viewerWasOpen) this.layer.resizeViewer?.();
|
if (state.street.open && !wasOpen) this.layer.resizeViewer?.();
|
||||||
this._viewerWasOpen = state.street.open === true;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
_renderMeta(view) {
|
_renderMeta(view) {
|
||||||
|
|||||||
@@ -500,4 +500,7 @@ test('the viewer is resized when it opens, not on every render (review #8)', ()
|
|||||||
assert.equal(layer.calls.resize, 1, 'once on open');
|
assert.equal(layer.calls.resize, 1, 'once on open');
|
||||||
for (let i = 0; i < 5; i++) layer.publish(uiState({ open: true }));
|
for (let i = 0; i < 5; i++) layer.publish(uiState({ open: true }));
|
||||||
assert.equal(layer.calls.resize, 1, 'not again while it stays open');
|
assert.equal(layer.calls.resize, 1, 'not again while it stays open');
|
||||||
|
layer.publish(uiState({ open: false }));
|
||||||
|
layer.publish(uiState({ open: true }));
|
||||||
|
assert.equal(layer.calls.resize, 2, 'the next photo resizes again');
|
||||||
}));
|
}));
|
||||||
Reference in new issue
Block a user