fix(street-level): review follow-ups — shares judged against the last draw; refused pictures stay refused

From the maintainer review of #1134 (approve with follow-ups):
- lineCoverage: a share is judged against the cap the tile was last drawn
  with, not against later shares that redrew nothing, so small increases
  add up (a retained tile stayed at 100 points after its share grew to
  116). A rebuild's measuring pass no longer counts as a draw.
- photoViewer: a picture refused for its size (bytes, streamed bytes or
  decoded side) is marked `tooLarge`, and the viewer never loads that file
  again by any path; the flat fallback had retried an oversized SD file
  through an <img>, which has no size limit.
- Regressions for both: cumulative share growth below the threshold, and a
  refused SD panorama that is not fetched again as a flat photo.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Said TurkseverandClaude Opus 5.5 committed 2026-10-15 16:08:18 +01:00
1 parent de83e1c17f
commit e3bc431f9a
5 files changed
+129 -16

No files matched your search

+11 -5
View File
@@ -152,9 +152,10 @@ export function createLineCoverage({
/**
* A tile's sequences, filtered then capped: capping first could leave a
* dense tile of newer flat captures with no 360° lines at all.
* dense tile of newer flat captures with no 360° lines at all. `measure`
* only records what the lines need, for a draw that follows.
*/
function drawnSequences(entry) {
function drawnSequences(entry, { measure = false } = {}) {
const drawn = [];
const current = filter();
const selectedId = state.sequence.selectedId ?? null;
@@ -180,7 +181,12 @@ export function createLineCoverage({
}
// What every passing line would take: the budget is shared by it.
entry.pointDemand = demand;
entry.pointsDrawn = points;
if (!measure) {
entry.pointsDrawn = points;
// The cap this draw used: a share is judged against it, not against
// later shares that redrew nothing (small steps add up).
entry.drawnCap = cap;
}
return drawn;
}
@@ -204,7 +210,7 @@ export function createLineCoverage({
const share = left / (byDemand.length - i);
const cap = Math.floor(Math.min(entry.pointDemand || 0, share));
left -= cap;
const before = entry.pointCap ?? perTilePointCap;
const before = entry.drawnCap ?? perTilePointCap;
entry.pointCap = cap;
// Only a change that alters what is drawn, by enough to be worth a
// rebuild: more room for a tile that had lines left out, or well
@@ -899,7 +905,7 @@ export function createLineCoverage({
// What each tile's lines need under the new filter, then the shares,
// then one redraw each.
for (const entry of state.coverage.tiles.values())
if (entry.kind === 'sequence') drawnSequences(entry);
if (entry.kind === 'sequence') drawnSequences(entry, { measure: true });
shareCaps();
for (const entry of state.coverage.tiles.values()) redrawEntry(entry);
requestRender();
@@ -433,3 +433,43 @@ test('a selected line outside the cap does not make its tile redraw on every set
for (let i = 0; i < 3; i++) coverage.recolorSequence('short', true);
assert.equal(after.added, 0);
});
test('small share increases add up: a retained tile redraws once its share has grown past the threshold since its last draw', async (t) => {
const tuning = {
...TUNING,
maxTiles: 1,
maxSequences: 900,
maxPoints: 100,
keepOrder: true,
};
const { state, coverage } = setup(t, {
tuning,
decoded: (centre, tile) => ({
sequences: Array.from({ length: 30 }, (_, i) =>
pointsLine(`l${i}`, 4, tile, centre),
),
overview: [],
}),
});
coverage.refresh();
assert.ok(
await until(
() => state.coverage.tiles.size === 1 && !state.coverage.pending.size,
),
);
const [tile] = state.coverage.tiles.values();
assert.equal(tile.pointsDrawn, 100);
const settle = async () => {
coverage.refresh();
await until(() => !state.coverage.pending.size);
await new Promise((resolve) => setTimeout(resolve, 10));
};
// +8%: under the threshold, nothing is redrawn.
tuning.maxPoints = 108;
await settle();
assert.equal(tile.pointsDrawn, 100);
// +8% again: +16% since the tile was last drawn, so it is redrawn.
tuning.maxPoints = 116;
await settle();
assert.equal(tile.pointsDrawn, 116, 'the grown share is drawn');
});
+27 -7
View File
@@ -78,7 +78,7 @@ async function loadBitmap(url, timeoutMs, resize, failure, signal) {
// Picture hosts are not ours: refuse a file or an image past any real
// photo's size before it is decoded.
const length = Number(response.headers?.get?.('content-length'));
if (length > MAX_IMAGE_BYTES) throw new Error(failure);
if (length > MAX_IMAGE_BYTES) throw oversized(failure);
const blob = await boundedBlob(response, failure);
const bitmap = await (resize
? createImageBitmap(blob, {
@@ -90,10 +90,12 @@ async function loadBitmap(url, timeoutMs, resize, failure, signal) {
// (A resized decode reports its new size: this guards unresized ones.)
if (bitmap.width > MAX_IMAGE_SIDE || bitmap.height > MAX_IMAGE_SIDE) {
bitmap.close?.();
throw new Error(failure);
throw oversized(failure);
}
return bitmap;
} catch {
} catch (error) {
// A size refusal stays one: the viewer must not fetch it another way.
if (error?.tooLarge) throw error;
throw new Error(failure);
} finally {
clearTimeout(timer);
@@ -101,6 +103,14 @@ async function loadBitmap(url, timeoutMs, resize, failure, signal) {
}
}
/**
* A picture refused for its size (`tooLarge`): the viewer will not load
* that file another way, an <img> having no size limit at all.
*/
export function oversized(failure) {
return Object.assign(new Error(failure), { tooLarge: true });
}
/**
* The body as a Blob, read with a running count so a host that sends no
* length (or a false one) cannot fill memory before the cap is checked.
@@ -109,7 +119,7 @@ async function boundedBlob(response, failure) {
const reader = response.body?.getReader?.();
if (!reader) {
const blob = await response.blob();
if (blob.size > MAX_IMAGE_BYTES) throw new Error(failure);
if (blob.size > MAX_IMAGE_BYTES) throw oversized(failure);
return blob;
}
const chunks = [];
@@ -120,7 +130,7 @@ async function boundedBlob(response, failure) {
total += value.byteLength;
if (total > MAX_IMAGE_BYTES) {
reader.cancel().catch(() => {});
throw new Error(failure);
throw oversized(failure);
}
chunks.push(value);
}
@@ -226,8 +236,17 @@ export function createPhotoViewer({
/** ‹ › presses run one after another, each from where the last one went. */
let steps = Promise.resolve();
const loadAs = (url, options) =>
load(url, { failure: LOAD_FAILED, ...options });
/** Files refused for their size: never loaded again, by any path. */
const refused = new Set();
async function loadAs(url, options) {
if (refused.has(url)) throw oversized(LOAD_FAILED);
try {
return await load(url, { failure: LOAD_FAILED, ...options });
} catch (error) {
if (error?.tooLarge) refused.add(url);
throw error;
}
}
function requestRender() {
render?.governorRequestRender?.(`${providerId}-viewer`);
@@ -796,6 +815,7 @@ export function createPhotoViewer({
unmount() {
openSeq++;
dropSharpPanorama();
refused.clear();
openingId = null;
keepViewFor = null;
flatSharpUrl = null;
+15 -4
View File
@@ -296,12 +296,17 @@ test('a picture file or bitmap past any real photo’s size is refused', async (
headers: new Map([['content-length', String(length ?? '')]]),
blob: async () => ({ size: size ?? 10, bitmap: bitmap(width, height) }),
}));
const refusedForSize = (error) => {
assert.match(error.message, /could not be loaded/);
assert.equal(error.tooLarge, true, 'marked as a size refusal');
return true;
};
respond({ length: MAX_IMAGE_BYTES + 1 });
await assert.rejects(loadImage('https://x/a.jpg'), /could not be loaded/);
await assert.rejects(loadImage('https://x/a.jpg'), refusedForSize);
respond({ size: MAX_IMAGE_BYTES + 1 });
await assert.rejects(loadImage('https://x/a.jpg'), /could not be loaded/);
await assert.rejects(loadImage('https://x/a.jpg'), refusedForSize);
respond({ width: MAX_IMAGE_SIDE + 1 });
await assert.rejects(loadImage('https://x/a.jpg'), /could not be loaded/);
await assert.rejects(loadImage('https://x/a.jpg'), refusedForSize);
assert.equal(closed, 1, 'the oversize bitmap is released');
// No length given, a body that keeps coming: cut off while it streams.
let pulled = 0;
@@ -315,8 +320,14 @@ test('a picture file or bitmap past any real photo’s size is refused', async (
},
}),
});
await assert.rejects(loadImage('https://x/a.jpg'), /could not be loaded/);
await assert.rejects(loadImage('https://x/a.jpg'), refusedForSize);
assert.ok(pulled * 8 * 1024 * 1024 <= MAX_IMAGE_BYTES + 16 * 1024 * 1024);
// Other failures are not size refusals.
globalThis.fetch = async () => ({ ok: false, headers: new Map() });
await assert.rejects(
loadImage('https://x/a.jpg'),
(error) => !error.tooLarge,
);
respond({});
assert.equal((await loadImage('https://x/a.jpg')).width, 100);
});
@@ -295,3 +295,39 @@ test('a photo with no author to name is captioned with the instance holding it',
const named = { ...photo, creator: 'Robot8A' };
assert.equal(PANORAMAX_SOURCES.describe(named).title, null);
});
test('an SD panorama refused for its size is not fetched again as a flat photo', async () => {
const loads = [];
const viewer = createPanoramaxViewer({
getPhoto: async () => PHOTOS.pano,
step: async () => null,
onStep() {},
load: async (url, options = {}) => {
loads.push({ url, crossOrigin: options.crossOrigin });
// The picture host's SD file is past the viewer's size limits.
if (url === PHOTOS.pano.urls.sd)
throw Object.assign(new Error('too big'), { tooLarge: true });
return { src: url };
},
createRenderer: () => ({
setImage() {},
setView() {},
getView: () => ({ yaw: 0, pitch: 0, fov: 80 }),
resize() {},
clear() {},
destroy() {},
}),
});
const host = fakeHost();
await viewer.mount(host);
await viewer.open('pano');
const sdLoads = loads.filter((l) => l.url === PHOTOS.pano.urls.sd);
assert.equal(sdLoads.length, 1, 'tried once, as a bitmap, and refused');
assert.equal(sdLoads[0].crossOrigin, true);
const { picture } = parts(host);
assert.equal(picture.hidden, false, 'the flat fallback shows');
assert.notEqual(picture.src, PHOTOS.pano.urls.sd);
// Opened again later: the refused file is still not asked for.
await viewer.open('pano');
assert.equal(loads.filter((l) => l.url === PHOTOS.pano.urls.sd).length, 1);
});