fix(street-level): address gekh's review of #768

P0 - tileMath: lonToTileX(180) wrapped to column 0, so a whole-world view at z1 loaded only the western column; 180 is now the last column. - coverage.loadTile: a superseded request (after a retire or clear) still deleted the newer request's pending entry and loading count in `finally`, letting a duplicate load attach lines that nothing could remove. Only the request that still owns the key attaches its tile or settles the key. P1 - view/tileMath: a view across the date line read as a ~358 deg box and loaded tiles on the far side of the globe. visibleBbox now returns west > east for such views and tilesForBbox splits them at +/-180; viewCentre averages across the line. - decode: the parts of a multi-line sequence were joined into one line, drawing straight segments over capture gaps. Each part is now its own line (`parts`); later parts carry `~n` in their pick id, and picking or recolouring any part acts on the whole sequence. - viewerHost: a prewarmed MapillaryJS viewer was never destroyed, leaking its WebGL context. Prewarmed adapters are tracked and released on unmount, at once when the layer went off mid-load, and per provider when only that provider is switched off. - nearestImage took the first record the API returned; it now takes the closest one that passes the filter. P2 - groundCast: the cache was cleared after a request counted its corners, dropping ones it relied on; it is now cleared first. - streetLevelControls: the viewer was resized on every render (every frame while a panorama is dragged); the ResizeObserver handles size changes and the viewer is resized once when it opens. The legend now rebuilds when its entries change, not only their count. - panelPositionControls: the portable-panel paths this PR added called the CCTV viewport sync directly; they go through onPanelResized, which the shell routes to CCTV. The "Compose camera panel controls" JSDoc is back on _initCctvPanel. New tests: zoom-1 columns, date-line tiles and bbox, multi-part decode and part pick ids, nearest image, prewarmed viewer release, and the cache ordering (fails on the old order). Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-11-06 03:14:00 +00:00 · 2026-10-02 19:00:01 +01:00
1 parent 514788303d
commit a1f8d6ca25
@@ -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:<id>` and
+ * `mly:seq:<id>~<part>` both give `<id>`; 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();
  }
@@ -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,
      });
    }
  }
@@ -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]);
+});
@@ -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,
@@ -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');
+});