diff --git a/src/layers/streetLevel/groundCast.js b/src/layers/streetLevel/groundCast.js index aaf4f7f96..f3bfeea04 100644 --- a/src/layers/streetLevel/groundCast.js +++ b/src/layers/streetLevel/groundCast.js @@ -133,13 +133,14 @@ export function densifyLine(coords, maxStep = GROUND_CAST_DENSIFY_DEG) { /** * A bare-earth height cache over the terrain service. - * @param {{terrain: {resolveEllipsoidalGround: Function}, step?: number, lift?: number, maxCorners?: number}} options + * @param {{terrain: {resolveEllipsoidalGround: Function}, step?: number, lift?: number, maxCorners?: number, cacheMax?: number}} options */ export function createGroundCaster({ terrain, step = GROUND_CAST_STEP_DEG, lift = GROUND_CAST_LIFT_M, maxCorners = GROUND_CAST_MAX_CORNERS, + cacheMax = GROUND_CAST_CACHE_MAX, }) { if (typeof terrain?.resolveEllipsoidalGround !== 'function') throw new TypeError('Ground casting requires a terrain service'); @@ -169,6 +170,9 @@ export function createGroundCaster({ async function resolve(points, signal) { if (signal?.aborted) return false; + // Make room first: clearing after counting would drop corners this + // request already treated as cached. + if (heights.size + maxCorners > cacheMax) heights.clear(); const missing = missingCorners(points); if (!missing.length) return true; if (missing.length > maxCorners) return false; @@ -178,7 +182,6 @@ export function createGroundCaster({ lat: Number((j * step).toFixed(6)), })), ); - if (heights.size + missing.length > GROUND_CAST_CACHE_MAX) heights.clear(); let complete = true; missing.forEach(([key], n) => { const result = results?.[n]; diff --git a/src/layers/streetLevel/groundCast.test.mjs b/src/layers/streetLevel/groundCast.test.mjs index b8402c351..d1c35b221 100644 --- a/src/layers/streetLevel/groundCast.test.mjs +++ b/src/layers/streetLevel/groundCast.test.mjs @@ -219,3 +219,22 @@ test('castLine refines heights with sampled mesh and densifies finer', async () const unsampled = caster.castLine(line, { meshAt: () => undefined }); assert.ok(unsampled[2] > 100 && unsampled[2] < 110); }); + +test('a full cache is cleared before a request counts its corners, so the request still casts', async () => { + const caster = createGroundCaster({ + terrain: fakeTerrain(), + step: 0.001, + maxCorners: 8, + cacheMax: 5, + }); + // One cell's four corners are cached... + assert.equal(await caster.prepare([[10.0005, 50.0005]]), true); + // ...then a line over that cell and the next needs two more. Clearing after + // counting would have kept only the two new corners and lost the first cell. + const line = [ + [10.0002, 50.0005], + [10.0018, 50.0005], + ]; + assert.equal(await caster.prepareLines([line]), true); + assert.ok(caster.castLine(line), 'the whole line casts'); +}); diff --git a/src/layers/streetLevel/index.js b/src/layers/streetLevel/index.js index 2173efd83..787b5ebc6 100644 --- a/src/layers/streetLevel/index.js +++ b/src/layers/streetLevel/index.js @@ -221,7 +221,7 @@ export function createStreetLevelLayer({ } function deactivate(entry) { - if (state.street.providerId === entry.def.id) parts.viewerHost.unmount(); + parts.viewerHost.unmount(entry.def.id); entry.instance.deactivate(); parts.credits.hide(state.viewer, entry.def); } diff --git a/src/layers/streetLevel/providers/mapillary/coverage.js b/src/layers/streetLevel/providers/mapillary/coverage.js index c4302606e..783045bb0 100644 --- a/src/layers/streetLevel/providers/mapillary/coverage.js +++ b/src/layers/streetLevel/providers/mapillary/coverage.js @@ -47,6 +47,32 @@ function sequenceColor({ selected = false } = {}) { : Cesium.Color.fromCssColorString(COLORS.coverage).withAlpha(0.92); } +/** Separates a multi-part sequence's part index from its id in pick ids. */ +const PART_SEPARATOR = '~'; + +/** Pick ids for every part of a sequence; the first part is unsuffixed. */ +function partIds(sequence) { + return sequence.parts.map((_, index) => + index + ? `${PICK_PREFIX.sequence}${sequence.id}${PART_SEPARATOR}${index}` + : `${PICK_PREFIX.sequence}${sequence.id}`, + ); +} + +/** + * Sequence id for a picked line, whichever part was hit: `mly:seq:` and + * `mly:seq:~` both give ``; null for any other pick. + * @param {string} pickId + * @returns {string|null} + */ +export function sequenceIdFromPick(pickId) { + if (typeof pickId !== 'string' || !pickId.startsWith(PICK_PREFIX.sequence)) + return null; + const rest = pickId.slice(PICK_PREFIX.sequence.length); + const cut = rest.indexOf(PART_SEPARATOR); + return cut === -1 ? rest : rest.slice(0, cut); +} + /** * Camera-driven coverage: z0–5 `overview` points from orbit down to 60 km, * then z11–14 sequence polylines clamped to terrain and 3D tiles. Decoded @@ -84,64 +110,55 @@ export function createCoverage({ state, source }) { ); } - function sequenceInstance(sequence, geometry) { - return new Cesium.GeometryInstance({ - geometry, - id: `${PICK_PREFIX.sequence}${sequence.id}`, - attributes: { - color: Cesium.ColorGeometryInstanceAttribute.fromColor( - sequenceColor({ - selected: sequence.id === state.sequence.selectedId, - }), - ), - }, - }); - } - /** * Primitives for a tile's sequences, in draw batches: ground primitives * draped on the globe, plus (in terrain mode) plain polylines at the cast - * bare-earth heights for every sequence whose heights are cached. + * heights for every part whose heights are cached. Each part of a sequence + * with a capture gap is its own line. */ function buildSequencePrimitives(sequences) { const ground = terrainMode() ? state.context.groundCaster : null; const meshAt = ground ? state.context.meshSampler?.meshAt : undefined; const draped = []; const cast = []; + let drawn = 0; for (const sequence of sequences) { if (!passesImageryFilter(sequence, filter())) continue; - const flat = ground?.castLine(sequence.coordinates, { meshAt }); - let positions; - try { - positions = flat - ? Cesium.Cartesian3.fromDegreesArrayHeights(flat) - : Cesium.Cartesian3.fromDegreesArray(sequence.coordinates.flat()); - } catch { - continue; - } - if (positions.length < 2) continue; - if (flat) - cast.push( - sequenceInstance( - sequence, - new Cesium.PolylineGeometry({ + drawn++; + const color = Cesium.ColorGeometryInstanceAttribute.fromColor( + sequenceColor({ selected: sequence.id === state.sequence.selectedId }), + ); + const ids = partIds(sequence); + sequence.parts.forEach((coordinates, index) => { + const flat = ground?.castLine(coordinates, { meshAt }); + let positions; + try { + positions = flat + ? Cesium.Cartesian3.fromDegreesArrayHeights(flat) + : Cesium.Cartesian3.fromDegreesArray(coordinates.flat()); + } catch { + return; + } + if (positions.length < 2) return; + const geometry = flat + ? new Cesium.PolylineGeometry({ positions, width: COVERAGE_LINE_WIDTH_PX, vertexFormat: Cesium.PolylineColorAppearance.VERTEX_FORMAT, arcType: Cesium.ArcType.NONE, - }), - ), - ); - else - draped.push( - sequenceInstance( - sequence, - new Cesium.GroundPolylineGeometry({ + }) + : new Cesium.GroundPolylineGeometry({ positions, width: COVERAGE_LINE_WIDTH_PX, - }), - ), + }); + (flat ? cast : draped).push( + new Cesium.GeometryInstance({ + geometry, + id: ids[index], + attributes: { color }, + }), ); + }); } const primitives = []; for (let i = 0; i < draped.length; i += SEQUENCE_PRIMITIVE_BATCH) @@ -165,7 +182,7 @@ export function createCoverage({ state, source }) { allowPicking: true, }), }); - return { primitives, count: draped.length + cast.length, draped }; + return { primitives, count: drawn, draped }; } /** @@ -179,7 +196,7 @@ export function createCoverage({ state, source }) { const { signal } = entry.castAbort; const lines = entry.sequenceList .filter((sequence) => passesImageryFilter(sequence, filter())) - .map((sequence) => sequence.coordinates); + .flatMap((sequence) => sequence.parts); const caster = state.context.groundCaster; caster.prepareLines(lines, { signal }).then(() => { const attached = [...state.coverage.tiles.values()].includes(entry); @@ -200,9 +217,8 @@ export function createCoverage({ state, source }) { if (entry.meshCells) return entry.meshCells; const cells = new Map(); for (const sequence of entry.sequenceList) - for (const [lon, lat] of densifyLine( - sequence.coordinates, - MESH_DENSIFY_DEG, + for (const [lon, lat] of sequence.parts.flatMap((part) => + densifyLine(part, MESH_DENSIFY_DEG), )) cells.set( `${Math.round(lon / MESH_CELL_DEG)},${Math.round(lat / MESH_CELL_DEG)}`, @@ -414,8 +430,13 @@ export function createCoverage({ state, source }) { const bytes = await fetching; // A refresh that still wants this tile must not throw the bytes away; // only an abort (tile no longer wanted, or zoom changed) does. + // A retire, clear or newer request for this tile owns it now. + const current = () => + state.coverage.pending.get(key) === controller && + !state.coverage.tiles.has(key); if ( controller.signal.aborted || + !current() || kind !== state.coverage.kind || tile.z !== state.coverage.zoom ) @@ -449,13 +470,21 @@ export function createCoverage({ state, source }) { state.coverage.lastError = null; requestRender(); } catch (error) { - if (controller.signal.aborted) return; + if ( + controller.signal.aborted || + state.coverage.pending.get(key) !== controller + ) + return; state.coverage.lastError = error?.message || 'Coverage tile failed'; if (error?.keyRequired) state.keyRequired = true; } finally { - state.coverage.pending.delete(key); - state.coverage.loading = Math.max(0, state.coverage.loading - 1); - if (!state.coverage.pending.size) purgeStale(); + // Only the request that still owns the key settles it: a superseded one + // must not drop a newer request's entry or its loading count. + if (state.coverage.pending.get(key) === controller) { + state.coverage.pending.delete(key); + state.coverage.loading = Math.max(0, state.coverage.loading - 1); + if (!state.coverage.pending.size) purgeStale(); + } notify(); } } @@ -615,26 +644,27 @@ export function createCoverage({ state, source }) { return null; } - /** Recolour one sequence in place (selection highlight). */ + /** Recolour one sequence, every part of it, in place (selection highlight). */ function recolorSequence(id, selected) { - const instanceId = `${PICK_PREFIX.sequence}${id}`; + const value = sequenceColor({ selected }); for (const entry of state.coverage.tiles.values()) { const sequence = entry.sequences.get(id); if (!sequence) continue; - for (const { primitive } of entry.primitives || []) { - if (!primitive.ready) continue; - try { - const attributes = - primitive.getGeometryInstanceAttributes(instanceId); - if (attributes) - attributes.color = Cesium.ColorGeometryInstanceAttribute.toValue( - sequenceColor({ selected }), - attributes.color, - ); - } catch { - /* instance not in this primitive */ + for (const instanceId of partIds(sequence)) + for (const { primitive } of entry.primitives || []) { + if (!primitive.ready) continue; + try { + const attributes = + primitive.getGeometryInstanceAttributes(instanceId); + if (attributes) + attributes.color = Cesium.ColorGeometryInstanceAttribute.toValue( + value, + attributes.color, + ); + } catch { + /* instance not in this primitive */ + } } - } } requestRender(); } diff --git a/src/layers/streetLevel/providers/mapillary/decode.js b/src/layers/streetLevel/providers/mapillary/decode.js index dfdb9be4f..d2067098d 100644 --- a/src/layers/streetLevel/providers/mapillary/decode.js +++ b/src/layers/streetLevel/providers/mapillary/decode.js @@ -21,15 +21,16 @@ export function decodeCoverageTile(bytes, address, { images = false } = {}) { for (let i = 0; i < sequenceLayer.length; i++) { const feature = sequenceLayer.feature(i); const props = feature.properties || {}; - const coordinates = []; - for (const ring of feature.loadGeometry()) { - for (const point of ring) { - coordinates.push( - tileLocalToLonLat(point.x, point.y, sequenceLayer.extent, x, y, z), - ); - } + // A sequence with a capture gap is a multi-line: keep each part as its + // own line, so no straight segment is drawn across the gap. + const parts = []; + for (const line of feature.loadGeometry()) { + const part = line.map((point) => + tileLocalToLonLat(point.x, point.y, sequenceLayer.extent, x, y, z), + ); + if (part.length >= 2) parts.push(part); } - if (coordinates.length < 2) continue; + if (!parts.length) continue; result.sequences.push({ id: String(props.id ?? feature.id ?? `${x}/${y}/${i}`), imageId: props.image_id != null ? String(props.image_id) : null, @@ -39,7 +40,7 @@ export function decodeCoverageTile(bytes, address, { images = false } = {}) { quality: Number.isFinite(props.quality_score) ? props.quality_score : null, - coordinates, + parts, }); } } diff --git a/src/layers/streetLevel/providers/mapillary/decode.test.mjs b/src/layers/streetLevel/providers/mapillary/decode.test.mjs index 464a54d53..af522f2e7 100644 --- a/src/layers/streetLevel/providers/mapillary/decode.test.mjs +++ b/src/layers/streetLevel/providers/mapillary/decode.test.mjs @@ -5,8 +5,20 @@ import { decodeCoverageTile } from './decode.js'; const zigzag = (value) => (value << 1) ^ (value >> 31); +/** One line part: MoveTo the first point, LineTo the rest (relative). */ +function linePart(points) { + const out = [9, zigzag(points[0][0]), zigzag(points[0][1])]; + out.push(((points.length - 1) << 3) | 2); + for (let i = 1; i < points.length; i++) + out.push( + zigzag(points[i][0] - points[i - 1][0]), + zigzag(points[i][1] - points[i - 1][1]), + ); + return out; +} + /** A minimal mly1_public-shaped tile: one sequence line and one overview point. */ -function sampleTile() { +function sampleTile({ geometry = null } = {}) { const writer = new PbfWriter(); writer.writeMessage( 3, @@ -37,14 +49,17 @@ function sampleTile() { feature.writeVarintField(1, 7); feature.writePackedVarint(2, [0, 0, 1, 1, 2, 2]); feature.writeVarintField(3, 2); // LineString - feature.writePackedVarint(4, [ - 9, - zigzag(100), - zigzag(100), // MoveTo - 10, - zigzag(200), - zigzag(200), // LineTo ×1 - ]); + feature.writePackedVarint( + 4, + geometry || [ + 9, + zigzag(100), + zigzag(100), // MoveTo + 10, + zigzag(200), + zigzag(200), // LineTo ×1 + ], + ); }, null, ); @@ -86,8 +101,9 @@ test('sequences and overview points decode into lon/lat records', () => { assert.equal(sequence.id, 'seq-42'); assert.equal(sequence.capturedAt, 1_700_000_000); assert.equal(sequence.isPano, true); - assert.equal(sequence.coordinates.length, 2); - const [[lon0, lat0], [lon1, lat1]] = sequence.coordinates; + assert.equal(sequence.parts.length, 1); + assert.equal(sequence.parts[0].length, 2); + const [[lon0, lat0], [lon1, lat1]] = sequence.parts[0]; assert.ok( lon0 > -180 && lon0 < 0 && lon1 > lon0, 'west half of tile 0/0 at z1', @@ -110,3 +126,35 @@ test('an empty tile decodes to empty lists', () => { }, ); }); + +test('a sequence with a capture gap keeps its parts as separate lines', () => { + // Two parts with a gap between (300,300) and (1000,1000); the second part's + // MoveTo is relative to the end of the first, as in the MVT encoding. + const first = [ + [100, 100], + [300, 300], + ]; + const second = [ + [700, 700], + [900, 700], + ]; + const geometry = [ + ...linePart(first), + 9, + zigzag(second[0][0] - first[1][0]), + zigzag(second[0][1] - first[1][1]), + ((second.length - 1) << 3) | 2, + zigzag(second[1][0] - second[0][0]), + zigzag(second[1][1] - second[0][1]), + ]; + const [sequence] = decodeCoverageTile(sampleTile({ geometry }), { + x: 0, + y: 0, + z: 1, + }).sequences; + assert.equal(sequence.parts.length, 2); + assert.equal(sequence.parts[0].length, 2); + assert.equal(sequence.parts[1].length, 2); + // No part ends where the next begins: the gap is not bridged. + assert.notDeepEqual(sequence.parts[0].at(-1), sequence.parts[1][0]); +}); diff --git a/src/layers/streetLevel/providers/mapillary/index.js b/src/layers/streetLevel/providers/mapillary/index.js index 0f3dbcaa6..436addf8e 100644 --- a/src/layers/streetLevel/providers/mapillary/index.js +++ b/src/layers/streetLevel/providers/mapillary/index.js @@ -1,4 +1,4 @@ -import { createCoverage } from './coverage.js'; +import { createCoverage, sequenceIdFromPick } from './coverage.js'; import { createSequences } from './sequences.js'; import { createMapillaryViewer } from './viewer.js'; import { passesImageryFilter } from '../../filter.js'; @@ -168,8 +168,9 @@ export function createMapillaryProvider({ source }) { }, handlePick(pickId) { - if (pickId.startsWith(PICK_PREFIX.sequence)) { - parts.sequences.select(pickId.slice(PICK_PREFIX.sequence.length)); + const sequenceId = sequenceIdFromPick(pickId); + if (sequenceId) { + parts.sequences.select(sequenceId); return true; } if (pickId.startsWith(PICK_PREFIX.image)) { @@ -195,7 +196,17 @@ export function createMapillaryProvider({ source }) { radius: NEAREST_RADIUS_M, limit: NEAREST_LIMIT, }); - const hit = images.find((record) => + // The API returns the images in the radius in no particular order. + const metres = (record) => { + const [lon2, lat2] = (record.computed_geometry || record.geometry) + ?.coordinates || [NaN, NaN]; + const dx = (lon2 - lon) * 111_320 * Math.cos((lat * Math.PI) / 180); + const dy = (lat2 - lat) * 110_540; + const d = Math.hypot(dx, dy); + return Number.isFinite(d) ? d : Infinity; + }; + const byDistance = [...images].sort((a, b) => metres(a) - metres(b)); + const hit = byDistance.find((record) => passesImageryFilter( { isPano: record.is_pano === true, diff --git a/src/layers/streetLevel/providers/mapillary/provider.test.mjs b/src/layers/streetLevel/providers/mapillary/provider.test.mjs index 5cbcbc822..de1c01f0c 100644 --- a/src/layers/streetLevel/providers/mapillary/provider.test.mjs +++ b/src/layers/streetLevel/providers/mapillary/provider.test.mjs @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { createMapillaryProvider, mapillaryImageUrl } from './index.js'; +import { sequenceIdFromPick } from './coverage.js'; import { validateProviders } from '../../registry.js'; import { PROVIDER_COLORS } from '../../policy.js'; @@ -92,3 +93,28 @@ test('nearestImage honours the imagery filter and reports key status', async () assert.deepEqual(await flat.status(), { configured: true }); assert.equal(flat.coverageStats().keyRequired, false); }); + +test('any part of a multi-part sequence picks the whole sequence', () => { + assert.equal(sequenceIdFromPick('mly:seq:abc'), 'abc'); + assert.equal(sequenceIdFromPick('mly:seq:abc~2'), 'abc'); + assert.equal(sequenceIdFromPick('mly:img:abc'), null); + assert.equal(sequenceIdFromPick(null), null); +}); + +test('nearestImage picks the closest image, not the first the API returned', async () => { + const at = (id, lon, lat) => ({ + id, + is_pano: false, + captured_at: 10, + geometry: { type: 'Point', coordinates: [lon, lat] }, + }); + const source = fakeSource({ + nearest: [ + at('far', 2.0004, 1), // ~45 m east + at('near', 2.00005, 1), // ~5 m east + at('mid', 2, 1.0002), // ~22 m north + ], + }); + const instance = createMapillaryProvider({ source }).create(fakeContext()); + assert.equal(await instance.nearestImage({ lat: 1, lon: 2 }), 'near'); +}); diff --git a/src/layers/streetLevel/tileMath.js b/src/layers/streetLevel/tileMath.js index efc495ad3..d6ba315b4 100644 --- a/src/layers/streetLevel/tileMath.js +++ b/src/layers/streetLevel/tileMath.js @@ -12,6 +12,8 @@ function clampLat(lat) { /** Tile column for a longitude at zoom z. */ export function lonToTileX(lon, z) { const n = 2 ** z; + // 180° is the east edge of the last column, not the west edge of the first. + if (lon >= 180) return n - 1; const wrapped = ((((lon + 180) % 360) + 360) % 360) - 180; return Math.min(n - 1, Math.max(0, Math.floor(((wrapped + 180) / 360) * n))); } @@ -91,13 +93,38 @@ export function countTilesForBbox(bbox, z) { } /** - * Enumerate the tiles at zoom z covering a bbox, ordered from the centre of + * Enumerate the tiles at zoom z covering a bbox (west > east means it crosses + * the date line), ordered from the centre of * the box outwards so a progressive renderer fills in what the user is most * likely looking at first. `limit` caps the list; the result reports whether * the cap was hit. * @returns {{tiles: Array<{x:number,y:number,z:number}>, truncated: boolean, total: number}} */ export function tilesForBbox(bbox, z, { limit = Infinity } = {}) { + // A box across the date line comes as west > east: split it at ±180°. + if (Array.isArray(bbox) && Number(bbox[0]) > Number(bbox[2])) { + const [west, south, east, north] = bbox.map(Number); + const n = 2 ** z; + const tiles = [ + ...tilesForBbox([west, south, 180, north], z).tiles, + ...tilesForBbox([-180, south, east, north], z).tiles, + ]; + const x0 = lonToTileX(west, z); + const x1 = lonToTileX(east, z) + n; + const cx = ((x0 + x1) / 2) % n; + const cy = (latToTileY(north, z) + latToTileY(south, z)) / 2; + const dx = (x) => Math.min(Math.abs(x - cx), n - Math.abs(x - cx)); + tiles.sort( + (a, b) => + dx(a.x) ** 2 + (a.y - cy) ** 2 - (dx(b.x) ** 2 + (b.y - cy) ** 2), + ); + const truncated = tiles.length > limit; + return { + tiles: truncated ? tiles.slice(0, limit) : tiles, + truncated, + total: tiles.length, + }; + } const box = normalizeBbox(bbox); if (!box) return { tiles: [], truncated: false, total: 0 }; const x0 = lonToTileX(box.west, z); diff --git a/src/layers/streetLevel/tileMath.test.mjs b/src/layers/streetLevel/tileMath.test.mjs index 40e450a2d..19f290451 100644 --- a/src/layers/streetLevel/tileMath.test.mjs +++ b/src/layers/streetLevel/tileMath.test.mjs @@ -85,3 +85,27 @@ test('overviewZoomForHeight covers the globe above the sequence ceiling', () => assert.equal(overviewZoomForHeight(10_000_000), 1); assert.equal(overviewZoomForHeight(25_000_000), 0); }); + +test('longitude 180 is the last column, so a world view keeps the eastern hemisphere', () => { + assert.equal(lonToTileX(180, 1), 1); + assert.equal(lonToTileX(179.99, 1), 1); + assert.equal(lonToTileX(-180, 1), 0); + const columns = new Set( + tilesForBbox([-180, -85, 180, 85], 1).tiles.map((tile) => tile.x), + ); + assert.deepEqual([...columns].sort(), [0, 1]); +}); + +test('a box across the date line takes tiles on both sides of it, not the far side of the globe', () => { + const { tiles } = tilesForBbox([179, -1, -179, 1], 8); + const columns = new Set(tiles.map((tile) => tile.x)); + assert.ok(columns.has(255), 'west of the line'); + assert.ok(columns.has(0), 'east of the line'); + assert.ok( + [...columns].every((x) => x <= 1 || x >= 254), + 'nothing from the middle of the map', + ); + // The limit keeps the tiles nearest the line. + const limited = tilesForBbox([170, -1, -170, 1], 8, { limit: 4 }).tiles; + assert.ok(limited.every((tile) => tile.x <= 8 || tile.x >= 247)); +}); diff --git a/src/layers/streetLevel/view.js b/src/layers/streetLevel/view.js index 4b114f777..2c1425b72 100644 --- a/src/layers/streetLevel/view.js +++ b/src/layers/streetLevel/view.js @@ -68,7 +68,24 @@ export function visibleBbox(viewer, { grid = 5 } = {}) { south = Math.min(south, lat); north = Math.max(north, lat); } - if (east - west > 0 && north - south > 0) return [west, south, east, north]; + // Hits on both sides of ±180° read as a box around the whole globe; + // measured in 0–360° they are a narrow box across the date line, which + // comes back as west > east (the convention tilesForBbox splits). + if (east - west > 180) { + let west360 = Infinity; + let east360 = -Infinity; + for (const carto of hits) { + const lon = Cesium.Math.toDegrees(carto.longitude); + const shifted = lon < 0 ? lon + 360 : lon; + west360 = Math.min(west360, shifted); + east360 = Math.max(east360, shifted); + } + if (east360 - west360 < east - west) { + west = west360 > 180 ? west360 - 360 : west360; + east = east360 > 180 ? east360 - 360 : east360; + } + } + if (west !== east && north - south > 0) return [west, south, east, north]; } const rectangle = camera.computeViewRectangle?.(scene.globe?.ellipsoid); if (!rectangle) return null; @@ -84,8 +101,13 @@ export function visibleBbox(viewer, { grid = 5 } = {}) { export function viewCentre(viewer) { if (!viewer) return null; const bbox = visibleBbox(viewer); - if (bbox) - return { lat: (bbox[1] + bbox[3]) / 2, lon: (bbox[0] + bbox[2]) / 2 }; + if (bbox) { + // Across the date line (west > east) the middle is on the far side of 0°. + const span = bbox[2] - bbox[0] + (bbox[0] > bbox[2] ? 360 : 0); + let lon = bbox[0] + span / 2; + if (lon > 180) lon -= 360; + return { lat: (bbox[1] + bbox[3]) / 2, lon }; + } const carto = viewer.camera?.positionCartographic; if (!carto) return null; return { diff --git a/src/layers/streetLevel/view.test.mjs b/src/layers/streetLevel/view.test.mjs index c9c9997e6..0e9696578 100644 --- a/src/layers/streetLevel/view.test.mjs +++ b/src/layers/streetLevel/view.test.mjs @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { cameraHeightAboveGround } from './view.js'; +import * as Cesium from 'cesium'; +import { cameraHeightAboveGround, viewCentre, visibleBbox } from './view.js'; const RAD = Math.PI / 180; /** A viewer 300 m above a street 1,600 m up (Denver-like). */ @@ -42,3 +43,36 @@ test('with no ground sample the ellipsoidal height is used', () => { assert.equal(cameraHeightAboveGround(hidden, { groundAt: () => null }), 1900); assert.equal(cameraHeightAboveGround({}), null); }); + +/** A camera whose screen rays land on a grid of lon/lat points. */ +function gridViewer(lons, lats) { + const points = []; + for (const lat of lats) + for (const lon of lons) + points.push(Cesium.Cartesian3.fromDegrees(lon, lat)); + let next = 0; + return { + scene: { canvas: { clientWidth: 100, clientHeight: 100 }, globe: {} }, + camera: { + pickEllipsoid: () => points[next++ % points.length], + positionCartographic: { longitude: 0, latitude: 0, height: 1000 }, + }, + }; +} + +test('a view across the date line is a narrow box with west > east', () => { + const viewer = gridViewer([178, 179, -179, -178], [-1, 0, 1]); + const bbox = visibleBbox(viewer); + assert.ok(bbox[0] > 177 && bbox[0] < 179, `west ${bbox[0]}`); + assert.ok(bbox[2] < -177 && bbox[2] > -179, `east ${bbox[2]}`); + const centre = viewCentre(viewer); + assert.ok(Math.abs(Math.abs(centre.lon) - 180) < 0.5, `centre ${centre.lon}`); +}); + +test('an ordinary view keeps west < east', () => { + const bbox = visibleBbox(gridViewer([10, 11, 12], [50, 51])); + assert.ok(bbox[0] < bbox[2]); + assert.ok( + Math.abs(viewCentre(gridViewer([10, 11, 12], [50, 51])).lon - 11) < 1e-6, + ); +}); diff --git a/src/layers/streetLevel/viewerHost.js b/src/layers/streetLevel/viewerHost.js index 57c2cd46b..28768c344 100644 --- a/src/layers/streetLevel/viewerHost.js +++ b/src/layers/streetLevel/viewerHost.js @@ -9,6 +9,12 @@ export function createViewerHost({ state, parts }) { /** @type {{id: string, promise: Promise}|null} */ let mounting = null; let openSeq = 0; + /** + * Adapters stood up by `prewarm` that are not the active one: each holds a + * live viewer (a WebGL context for MapillaryJS) until `unmount` releases it. + * @type {Map} + */ + const warmed = new Map(); function notify() { state.notify?.(); @@ -84,6 +90,7 @@ export function createViewerHost({ state, parts }) { throw new Error('Street-level viewer was closed'); } active = { id: entry.def.id, adapter, unsubscribe }; + warmed.delete(entry.def.id); adapter.setRenderMode?.(state.street.renderMode); return adapter; })(); @@ -193,22 +200,44 @@ export function createViewerHost({ state, parts }) { const host = state.street.host; if (!host || !state.enabled) return; for (const entry of entries) { + const adapter = entry.instance.viewer; + if (!adapter.prewarm || !entry.on) continue; try { - await entry.instance.viewer.prewarm?.(host); + await adapter.prewarm(host); } catch { /* the real open reports errors */ } + if (active?.id === entry.def.id) continue; + // Switched off while it loaded: release it now rather than keep it. + if (!state.enabled || !entry.on) adapter.unmount(); + else warmed.set(entry.def.id, adapter); } } - /** Tear the mounted adapter down (layer disabled or destroyed). */ - function unmount() { - close(); - mounting = null; - if (!active) return; - active.unsubscribe(); - active.adapter.unmount(); - active = null; + /** + * Tear viewers down: every one (layer disabled or destroyed), or only the + * given provider's, active or prewarmed (that provider switched off). + * @param {string} [providerId] + */ + function unmount(providerId) { + if ( + !providerId || + active?.id === providerId || + mounting?.id === providerId + ) { + close(); + mounting = null; + if (active) { + active.unsubscribe(); + active.adapter.unmount(); + active = null; + } + } + for (const [id, adapter] of warmed) { + if (providerId && id !== providerId) continue; + warmed.delete(id); + adapter.unmount(); + } } return { diff --git a/src/layers/streetLevel/viewerHost.test.mjs b/src/layers/streetLevel/viewerHost.test.mjs index d40d1d0be..018fc2025 100644 --- a/src/layers/streetLevel/viewerHost.test.mjs +++ b/src/layers/streetLevel/viewerHost.test.mjs @@ -87,3 +87,43 @@ test('unmounting while a mount is in flight does not leave it active', async () assert.equal(adapter.calls.unmount, 1); assert.equal(state.street.open, false); }); + +test('a prewarmed viewer is released when the layer goes off, or at once if it went off mid-load', async () => { + const adapter = fakeAdapter(); + let release = null; + adapter.prewarm = () => + new Promise((resolve) => { + release = resolve; + }); + const { state, host } = harness(adapter); + const entry = state.providers.get('mapillary'); + entry.on = true; + + // Warmed, then the layer is switched off: unmount releases the warm viewer. + const warming = host.prewarm([entry]); + release(); + await warming; + assert.equal(adapter.calls.unmount, 0); + host.unmount(); + assert.equal(adapter.calls.unmount, 1, 'the WebGL viewer is destroyed'); + + // Switched off while the library was still loading: released on arrival. + const late = host.prewarm([entry]); + state.enabled = false; + release(); + await late; + assert.equal(adapter.calls.unmount, 2); +}); + +test('switching one provider off releases only its viewer', async () => { + const adapter = fakeAdapter(); + adapter.prewarm = async () => {}; + const { state, host } = harness(adapter); + const entry = state.providers.get('mapillary'); + entry.on = true; + await host.prewarm([entry]); + host.unmount('panoramax'); + assert.equal(adapter.calls.unmount, 0, 'another provider leaves it alone'); + host.unmount('mapillary'); + assert.equal(adapter.calls.unmount, 1); +}); diff --git a/src/ui/applicationShell.js b/src/ui/applicationShell.js index 5dd05030d..260fc4bd6 100644 --- a/src/ui/applicationShell.js +++ b/src/ui/applicationShell.js @@ -933,7 +933,7 @@ export class StyleManager extends ShellFacade { return this._runExplicitNavigation('camera', () => focus(cameraId)); } - /** Compose camera panel controls from the existing camera port and application actions. */ + /** Compose the Street Level panel controls from its layer and application actions. */ _initStreetLevelPanel() { const { streetLevelLayer } = this.services; this._streetLevelControls?.destroy(); @@ -959,6 +959,7 @@ export class StyleManager extends ShellFacade { this._streetLevelControls.connect(); } + /** Compose camera panel controls from the existing camera port and application actions. */ _initCctvPanel() { const { cctvLayer } = this.services; this._cctvControls?.destroy(); @@ -1506,6 +1507,7 @@ export class StyleManager extends ShellFacade { * @returns {void} */ _onPanelResized(panelId) { + if (panelId === 'cctv-panel') this._syncCctvPanelViewport(); if (panelId === 'street-level-panel') this._streetLevelControls?.onPanelResized(); } diff --git a/src/ui/panelPositionControls.js b/src/ui/panelPositionControls.js index 97ecf634a..5ec2476a5 100644 --- a/src/ui/panelPositionControls.js +++ b/src/ui/panelPositionControls.js @@ -416,9 +416,6 @@ export class PanelPositionControls { // storage unavailable } this._layoutRightPanels(); - if (panelId === 'cctv-panel') { - this._syncCctvPanelViewport(); - } this._onPanelResized?.(panelId); } @@ -629,9 +626,7 @@ export class PanelPositionControls { swallowNextClick = false; }, 0); this._savePanelPosition(panelId, panelEl); - if (panelId === 'cctv-panel') { - this._syncCctvPanelViewport(); - } + this._onPanelResized?.(panelId); }; this._cancelDrag = cancel; @@ -716,9 +711,6 @@ export class PanelPositionControls { const onUp = () => { cancel(); this._savePanelPosition(panelId, panelEl); - if (panelId === 'cctv-panel') { - this._syncCctvPanelViewport(); - } this._onPanelResized?.(panelId); }; diff --git a/src/ui/streetLevelControls.js b/src/ui/streetLevelControls.js index 39a6a4b53..5f879759b 100644 --- a/src/ui/streetLevelControls.js +++ b/src/ui/streetLevelControls.js @@ -366,7 +366,13 @@ export class StreetLevelControls { el.sinceRange.setAttribute('aria-valuetext', view.since.label); } } - if (el.legend && el.legend.childElementCount !== view.legend.length) { + // Rebuild when the swatches change, not only their count: one provider + // can replace another with the same number of entries. + const legendKey = view.legend + .map((entry) => `${entry.key}:${entry.color}:${entry.label}`) + .join('|'); + if (el.legend && this._legendKey !== legendKey) { + this._legendKey = legendKey; el.legend.replaceChildren( ...view.legend.map((entry) => { const item = document.createElement('li'); @@ -406,7 +412,11 @@ export class StreetLevelControls { if (viewer.link) el.imageLink.href = viewer.link; if (viewer.linkLabel) el.imageLink.textContent = viewer.linkLabel; } - if (state.street.open) this.layer.resizeViewer?.(); + // Size changes reach the viewer through the ResizeObserver; resizing on + // every render ran each frame while a panorama was dragged. Opening is the + // one moment the element may not have reported a size yet. + if (state.street.open && !this._viewerWasOpen) this.layer.resizeViewer?.(); + this._viewerWasOpen = state.street.open === true; } _renderMeta(view) {