fix(viewer): collapse IfcGridAxis line-overlay ownership to the symbolic path

.changeset/grid-line-single-owner.md

@@ -0,0 +1,9 @@
+---
+'@ifc-lite/viewer': patch
+---
+
+Stop drawing every `IfcGridAxis` twice in the 3D viewport, which also made section-clipping of grid lines inert and let the two copies disagree in elevation.
+
+The viewport fed the `ifcGrid`-visibility toggle from two independent sources at once: `useSymbolicAnnotations` (its grid buckets, section-clipped against the active cut plane and rebased by the TS-side `originShift`) and `useGridLines3D` (the wasm `parseGridLines` API, unclipped and rebased only by RTC). Both uploaded to their own renderer line-overlay channel whenever the toggle was on, so every axis drew twice, issue #862's grid section-clipping never had any effect (the unclipped copy always drew the full grid), and a federated or re-aligned model with a nonzero `originShift` could show the two copies at different elevations.
+
+Grid lines in the viewport now draw only from `useSymbolicAnnotations`, which already section-clips and origin-shift-rebases its grid buckets. The redundant `useGridLines3D` hook is removed. `parseGridLines`/`parseGridAxes` themselves are unchanged — they remain published `@ifc-lite/geometry` SDK surface for embedders who want raw, unclipped grid geometry with no annotation/storey semantics.

apps/viewer/src/components/viewer/Viewport.tsx

@@ -65,7 +65,6 @@ import {
   type SectionClipForGrid,
 } from '../../hooks/useSymbolicAnnotations.js';
 import { useAlignmentLines3D } from '../../hooks/useAlignmentLines3D.js';
-import { useGridLines3D } from '../../hooks/useGridLines3D.js';
 import { useDxfUnderlays3DLines } from '../../hooks/useDxfUnderlay.js';
 import { uploadDxfLines3DGuarded } from './dxf-lines-3d-upload.js';
 import { subscribeViewportHealth } from './device-loss-report.js';
@@ -1487,18 +1486,19 @@ export function Viewport({
     );
   }, [alignmentVertices3D, isInitialized]);
 
-  // Structural-grid (IfcGridAxis) lines, gated by the `ifcGrid` type-visibility
-  // toggle (issue #967). Parsed once per source + cached; only the upload/clear
-  // is toggled so flipping visibility doesn't re-parse.
-  const gridVertices3D = useGridLines3D();
-  useEffect(() => {
-    const renderer = rendererRef.current;
-    if (!renderer || !isInitialized) return;
-    renderer.setLineOverlay(
-      'grid',
-      !ifcGridVisible || gridVertices3D.length === 0 ? null : gridVertices3D,
-    );
-  }, [gridVertices3D, ifcGridVisible, isInitialized]);
+  // Structural-grid (IfcGridAxis) lines used to also draw from a second,
+  // independent extractor (`useGridLines3D`, backed by the wasm
+  // `parseGridLines` API) uploaded to the renderer's own 'grid' line-overlay
+  // channel. That copy was never section-clipped and never received the
+  // TS-side `originShift` elevation rebase `useSymbolicAnnotations` applies
+  // to its grid buckets (see `elevationRebaseFor` in
+  // `symbolic-parse-cache.ts`), so every axis drew twice, #862's grid
+  // section-clipping was inert (the unclipped copy always drew the full
+  // grid), and a federated/re-aligned model with nonzero `originShift` could
+  // show the two copies at different elevations (issue #3368). Grid lines
+  // now draw ONLY from `useSymbolicAnnotations`'s `annotationVertices3D`
+  // above, which already section-clips and rebases them when `ifcGridVisible`
+  // (`gridEnabled`) is on — see its `effectiveGridEnabled` branch.
 
   // DXF reference-layer line paths in the 3D viewport (issue #2043,
   // follow-up to #1782/#1929's 2D-only DXF underlay). Gated by each

apps/viewer/src/components/viewer/grid-overlay-single-owner.test.ts

@@ -0,0 +1,153 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+/**
+ * Issue #3368: two independent extractors both produced `IfcGridAxis` line
+ * geometry for the 3D viewport, gated on the same `ifcGrid` toggle, and both
+ * drew:
+ *
+ *  - `useSymbolicAnnotations` (backed by `rust/processing/src/symbolic/grid.rs`)
+ *    section-clips its grid buckets against the active cut plane and applies
+ *    the TS-side `originShift` elevation rebase (`elevationRebaseFor` in
+ *    `symbolic-parse-cache.ts`) to every primitive, grid included — see
+ *    `symbolic-parse.elevationFrame.test.ts`.
+ *  - `useGridLines3D` (backed by `rust/wasm-bindings/src/api/grid_lines.rs`'s
+ *    `parseGridLines`) drew the SAME axes unclipped and without that rebase.
+ *
+ * `Viewport.tsx` uploaded both as separate renderer line-overlay channels
+ * ('annotation' and 'grid') whenever `ifcGridVisible` was on. Consequences:
+ * every axis was drawn twice, issue #862's section-clipping of grid lines
+ * was inert (the unclipped copy always drew the full grid), and for a
+ * federated/re-aligned model with nonzero `originShift` the two copies sat
+ * at different elevations.
+ *
+ * The fix collapses viewer grid-line ownership onto the symbolic path only
+ * (already clip-aware and origin-shift-aware) and retires the redundant
+ * `useGridLines3D` hook and its upload in `Viewport.tsx`. This test pins
+ * that collapse structurally: it fails as long as a second, independent
+ * grid-line source is wired into the viewport.
+ *
+ * `parseGridLines` / `parseGridAxes` themselves stay — they're published SDK
+ * surface (`packages/geometry`) for embedders who want raw, unclipped grid
+ * geometry with no annotation/storey semantics. This test is about the
+ * VIEWER's internal wiring, not the wasm API.
+ */
+
+import { describe, it } from 'node:test';
+import assert from 'node:assert/strict';
+import { existsSync, readFileSync } from 'node:fs';
+import { fileURLToPath } from 'node:url';
+import path from 'node:path';
+import { buildParseResult } from '../../lib/overlay-parse/symbolic-parse.js';
+import { createEmptyFlatSymbolic, type FlatSymbolic } from '../../lib/overlay-parse/symbolic-flat.js';
+
+const dir = path.dirname(fileURLToPath(import.meta.url));
+const viewportSource = readFileSync(path.join(dir, 'Viewport.tsx'), 'utf8');
+const hookPath = path.join(dir, '../../hooks/useGridLines3D.ts');
+
+describe('grid line overlay has one owner (issue #3368)', () => {
+  it('Viewport does not import the redundant raw grid-line hook', () => {
+    assert.ok(
+      !/from ['"][^'"]*useGridLines3D(\.js)?['"]/.test(viewportSource) &&
+        !/\buseGridLines3D\s*\(/.test(viewportSource),
+      'Viewport.tsx still imports/calls useGridLines3D — a second, unclipped, ' +
+        'origin-shift-unaware IfcGridAxis source is wired into the viewport ' +
+        "alongside useSymbolicAnnotations' clipped grid buckets, so section " +
+        'clipping stays inert and the two copies can disagree in Y (#3368).',
+    );
+  });
+
+  it("Viewport does not upload a separate, unclipped 'grid' line-overlay channel", () => {
+    assert.ok(
+      !/setLineOverlay\(\s*['"]grid['"]/.test(viewportSource),
+      "Viewport.tsx still uploads a 'grid' line-overlay channel independent " +
+        "of the clipped 'annotation' channel that already carries grid " +
+        'buckets when ifcGrid is visible (#3368).',
+    );
+  });
+
+  it('the redundant useGridLines3D hook has been retired', () => {
+    assert.ok(
+      !existsSync(hookPath),
+      'useGridLines3D.ts still exists — it has no remaining call site once ' +
+        'the viewer draws grid lines from a single (symbolic) owner, so it ' +
+        'is dead code that could be re-wired back into a second draw path.',
+    );
+  });
+});
+
+/**
+ * Consequence 3 (issue #3368): "the copies can disagree in Y". Quantify the
+ * mechanism directly from the real code both sides used.
+ *
+ *  - The surviving (symbolic) path bucketed a grid axis's `worldY` through
+ *    `buildParseResult`'s `ensureBucket`, which subtracts
+ *    `elevationRebase.primitive` — the TS-side `originShift` component
+ *    (`elevationRebaseFor` in `symbolic-parse-cache.ts`) that a wasm
+ *    primitive never carries, since the wasm extractor only ever removes the
+ *    RTC Z (`rust/processing/src/symbolic/rebase.rs`).
+ *  - The retired raw path (`useGridLines3D` -> wasm `parseGridLines`) handed
+ *    that same RTC-only `worldY` straight to `renderer.setLineOverlay('grid', ...)`
+ *    with NO further processing: `useGridLines3D.ts` (now deleted) never
+ *    referenced `elevationRebase`/`originShift`/`totalYupOffset`, and neither
+ *    did the `Viewport.tsx` effect that used to upload its output.
+ *
+ * A federated/re-aligned model sets a nonzero `originShift`, so
+ * `elevationRebase.primitive !== 0`, and the two Y values genuinely diverge —
+ * this is not hypothetical. A model needing no rebase (`primitive === 0`,
+ * the fixture's second case) hides the bug by accident, which is exactly why
+ * origin-frame symmetry must be avoided when reproducing this class of
+ * defect.
+ */
+describe('the two owners disagreed in Y for a re-aligned model (issue #3368)', () => {
+  const RTC_ONLY_WORLD_Y = 12.5; // wasm primitive.worldY: RTC Z already removed, nothing else.
+  const ORIGIN_SHIFT_Y = 3.75; // originShift.y for a re-aligned/federated model.
+
+  function flatWithOneGridAxis(worldY: number): FlatSymbolic {
+    const flat = createEmptyFlatSymbolic();
+    flat.typeNames = ['IfcGridAxis'];
+    flat.polyPoints = Float32Array.from([0, 0, 10, 0]);
+    flat.polyStart = Uint32Array.from([0, 2]);
+    flat.polyOwner = Uint32Array.from([7]);
+    flat.polyWorldY = Float32Array.from([worldY]);
+    flat.polyFlags = Uint8Array.from([0]);
+    flat.polyType = Uint16Array.from([0]);
+    return flat;
+  }
+
+  it('a nonzero originShift moves the symbolic (clipped) copy away from the raw (unclipped) one', () => {
+    const result = buildParseResult(flatWithOneGridAxis(RTC_ONLY_WORLD_Y), {
+      elevationRebase: { primitive: ORIGIN_SHIFT_Y, storeyTable: ORIGIN_SHIFT_Y },
+    });
+    const buckets = [...result.gridByStorey.values()];
+    assert.strictEqual(buckets.length, 1, 'one grid axis makes one bucket');
+    const symbolicY = buckets[0].storeyElevation;
+    assert.ok(symbolicY !== null);
+    // The retired raw path returned RTC_ONLY_WORLD_Y verbatim -- no rebase.
+    const rawPathY = RTC_ONLY_WORLD_Y;
+    assert.ok(
+      Math.abs((symbolicY as number) - rawPathY) > 1e-6,
+      `the symbolic bucket (${symbolicY}) must diverge from the raw path's ` +
+        `unrebased value (${rawPathY}) by the origin shift (${ORIGIN_SHIFT_Y}), ` +
+        "reproducing #3368's \"copies can disagree in Y\"",
+    );
+    assert.ok(
+      Math.abs((symbolicY as number) - (rawPathY - ORIGIN_SHIFT_Y)) < 1e-6,
+      `expected symbolic Y = raw Y - originShift.y = ${rawPathY - ORIGIN_SHIFT_Y}, got ${symbolicY}`,
+    );
+  });
+
+  it('a model needing no rebase hides the divergence by accident (why symmetry must be avoided)', () => {
+    const result = buildParseResult(flatWithOneGridAxis(RTC_ONLY_WORLD_Y), {
+      elevationRebase: { primitive: 0, storeyTable: 0 },
+    });
+    const buckets = [...result.gridByStorey.values()];
+    const symbolicY = buckets[0].storeyElevation;
+    assert.ok(
+      Math.abs((symbolicY as number) - RTC_ONLY_WORLD_Y) < 1e-6,
+      'with no origin shift the two paths coincidentally agree -- this is the ' +
+        'symmetric case the reproduction must not rely on',
+    );
+  });
+});

apps/viewer/src/hooks/symbolic-parse-cache.ts

@@ -167,9 +167,9 @@ export function ensureParseFor(stores: IfcDataStore[]): Promise<void>[] {
         notifyCacheChange();
       } catch (error) {
         // Cache empty on failure so we don't retry a doomed parse every tick
-        // (matches useGridLines3D / useAlignmentLines3D — a model whose
-        // annotation section is malformed would otherwise re-run the
-        // full-source WASM walk on every `stores` dependency change).
+        // (matches useAlignmentLines3D — a model whose annotation section is
+        // malformed would otherwise re-run the full-source WASM walk on
+        // every `stores` dependency change).
         // eslint-disable-next-line no-console
         console.warn('[useSymbolicAnnotations] parse failed:', error);
         PARSE_CACHE.set(key, createEmptyParseResult());

apps/viewer/src/hooks/useDxfUnderlay.ts

@@ -121,8 +121,8 @@ export function useDxfUnderlaysForDrawing(params: {
  * for `renderer.setLineOverlay('dxf', …)` (issue #2043). Independent of the 2D
  * panel's section-axis/plan-view gating in {@link useDxfUnderlaysForDrawing}
  * — the 3D overlay renders regardless of section state, matching how the
- * alignment/grid 3D overlays are always-eligible (`useAlignmentLines3D`,
- * `useGridLines3D`).
+ * alignment 3D overlay is always-eligible (`useAlignmentLines3D`); grid 3D
+ * lines are drawn by `useSymbolicAnnotations` instead (issue #3368).
  */
 export function useDxfUnderlays3DLines(
   coordinateInfo: GeometryResult['coordinateInfo'] | undefined,

apps/viewer/src/hooks/useSymbolicAnnotations.retryStorm.test.ts

@@ -4,8 +4,8 @@
 
 /**
  * Regression test for the retry-storm defect found in the error-path sweep
- * (see the sibling fix already applied in useGridLines3D.ts / useAlignmentLines3D.ts
- * — "Cache empty on failure so we don't retry a doomed parse every tick").
+ * (see the sibling fix already applied in useAlignmentLines3D.ts —
+ * "Cache empty on failure so we don't retry a doomed parse every tick").
  *
  * `ensureParseFor` memoizes a successful parse in `PARSE_CACHE` keyed by the
  * source's content hash, and a `PARSE_INFLIGHT` map de-dupes concurrent calls