fix(embed): make SET_CAMERA, RESET_COLORS and ENTITY_HOVERED actually work (#2934)

.changeset/embed-set-camera-reset-colors-entity-hovered.md

@@ -0,0 +1,52 @@
+---
+'@ifc-lite/renderer': minor
+'@ifc-lite/embed-protocol': patch
+'@ifc-lite/embed-sdk': patch
+---
+
+Three embed API commands that reported success while doing nothing now work
+(#2934). Each was broken at a different link in the chain.
+
+`SET_CAMERA` had no actuator. The handler called the store's
+`setCameraRotation`, which was `set({ cameraRotation })` and nothing more —
+every orientation entry point on the camera was either relative (`orbit`, the
+90° rotate steppers) or named a direction (`setPresetView`), so an absolute
+azimuth/elevation pair had nothing to reach. The host got a `requestId` ack
+*and* a `CAMERA_CHANGED` echo of its own numbers back, while the view never
+moved. `Camera.setRotation(azimuth, elevation)` is new on `@ifc-lite/renderer`
+— the exact inverse of `Camera.getRotation`, absolute and idempotent, keeping
+the target and orbit distance, with the same pole clamp `orbit` uses — and the
+store action now drives it the way `setProjectionMode` drives its own callback.
+
+`RESET_COLORS` cleared the wrong channel, in both directions at once.
+`SET_COLORS` bakes into the mesh colors, while `clearPendingColorUpdates`
+empties the transient overlay channel the lens, IDS, clash and schedule
+overlays own: the host's own override survived the reset, and another
+subsystem's state was destroyed by it. `SET_COLORS` now marks its writes as an
+override, which captures the colors it displaces, and `RESET_COLORS` restores
+those and leaves the overlay channel alone. The loader's own IFC style pass is
+deliberately not treated as an override, so a reset restores the model's IFC
+colors rather than stripping them.
+
+For integrators, that second half is a behaviour change on a published surface
+and not only a fix: `RESET_COLORS` no longer clears `pendingColorUpdates`. A
+host that had been sending it to clear a lens, IDS, clash or schedule overlay
+was relying on a side effect that is now gone, and must clear that overlay
+through the command that owns it. `RESET_COLORS` only undoes `SET_COLORS`.
+
+Also worth knowing before you rely on it: `RESET_COLORS` restores the entities
+the viewer holds in its primary `geometryResult`, which is the FIRST loaded
+model. In a federated embed with more than one model, `SET_COLORS` still
+colours entities in the later models and `RESET_COLORS` does not restore them,
+while both commands ack success. Single-model embeds — the common case — are
+unaffected.
+
+`ENTITY_HOVERED` was declared, exposed by the SDK, and never emitted — the SDK
+tests passed because they fabricated the event themselves. The viewer's hover
+pipeline was already there but gated behind a toolbar toggle the embed has no
+chrome to offer; the embed now enables it and emits on each hover-target
+change.
+
+`SET_CAMERA`'s `zoom` field remains unapplied and is now documented as
+reserved rather than silently dropped: it has no defined meaning on the viewer
+side, and guessing one is worse than saying so.

apps/viewer-embed/src/bridge/handler.effects.test.ts

@@ -0,0 +1,174 @@
+/* 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/. */
+
+/**
+ * `SET_CAMERA` / `SET_COLORS` / `RESET_COLORS` against the REAL store slices.
+ *
+ * `handler.test.ts` drives a recording double of the store, which is the right
+ * shape for "did the bridge dispatch the right thing" but is structurally
+ * blind to the failure these three commands actually had (#2934): the handler
+ * called a real store action, the action existed, and the action did nothing.
+ * A double records the call and passes.
+ *
+ * So this file wires the handler to `createDataSlice` / `createCameraSlice`
+ * themselves and asserts the EFFECT — the mesh color that comes back, the
+ * orientation that reaches the camera actuator, the overlay channel that is
+ * still intact afterwards.
+ */
+
+import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest';
+
+// Same narrow stand-in handler.test.ts uses: the bridge needs exactly one
+// function from the store barrel, and importing the real barrel would drag in
+// zustand + renderer + wasm. The slice creators below are imported directly,
+// so the store logic under test is the real thing.
+vi.mock('@/store/index.js', () => ({
+  toGlobalIdFromModels: (
+    _models: ReadonlyMap<string, { idOffset?: number }>,
+    _modelId: string,
+    expressId: number,
+  ): number => expressId,
+}));
+
+import { EMBED_SOURCE, PROTOCOL_VERSION } from '@ifc-lite/embed-protocol';
+import { createDataSlice } from '@/store/slices/dataSlice.js';
+import { createCameraSlice } from '@/store/slices/cameraSlice.js';
+import type { CameraRotation } from '@/store/types.js';
+import { initBridge, destroyBridge } from './handler.js';
+
+// ---------------------------------------------------------------------------
+// Window double (postMessage in, postMessage out)
+// ---------------------------------------------------------------------------
+
+function installWindow() {
+  const listeners = new Set<(e: unknown) => void>();
+  const win: any = {
+    addEventListener: (type: string, fn: (e: unknown) => void) => {
+      if (type === 'message') listeners.add(fn);
+    },
+    removeEventListener: (type: string, fn: (e: unknown) => void) => {
+      if (type === 'message') listeners.delete(fn);
+    },
+  };
+  win.parent = { postMessage: () => { /* replies are not the subject here */ } };
+  (globalThis as any).window = win;
+  return {
+    dispatch: (data: unknown) => {
+      for (const fn of [...listeners]) fn({ data, origin: 'https://host.example', source: win.parent });
+    },
+  };
+}
+
+function cmd(type: string, data?: unknown) {
+  return { source: EMBED_SOURCE, version: PROTOCOL_VERSION, type, data, requestId: 'r1' };
+}
+
+// ---------------------------------------------------------------------------
+// Real slices, composed the way the store composes them
+// ---------------------------------------------------------------------------
+
+const mesh = (expressId: number, color: [number, number, number, number]) => ({
+  expressId,
+  positions: new Float32Array([0, 0, 0, 1, 0, 0, 0, 1, 0]),
+  indices: new Uint32Array([0, 1, 2]),
+  normals: new Float32Array([0, 0, 1, 0, 0, 1, 0, 0, 1]),
+  color,
+  ifcType: 'IfcWall',
+});
+
+function makeRealState() {
+  const rotations: CameraRotation[] = [];
+  let state: any;
+  const set = (partial: any) => {
+    const updates = typeof partial === 'function' ? partial(state) : partial;
+    state = { ...state, ...updates };
+  };
+  const get = () => state;
+
+  state = {
+    ...createDataSlice(set, get, undefined as never),
+    ...createCameraSlice(set, get, undefined as never),
+    activeModelId: null,
+    models: new Map(),
+    // Stand-in for the renderer-side actuator the Viewport registers
+    // (Viewport.tsx -> camera.setRotation). What it does with the angles is
+    // `packages/renderer/src/camera-absolute-rotation.test.ts`'s subject; what
+    // matters here is that the command reaches it at all.
+    cameraCallbacks: {
+      setCameraRotation: (rotation: CameraRotation) => { rotations.push(rotation); },
+    },
+  };
+
+  return {
+    rotations,
+    getState: () => state,
+  };
+}
+
+describe('bridge commands against the real store slices', () => {
+  let win: ReturnType<typeof installWindow>;
+  let store: ReturnType<typeof makeRealState>;
+
+  beforeEach(() => {
+    win = installWindow();
+    store = makeRealState();
+    initBridge({
+      getState: store.getState as never,
+      loadModelFromUrl: vi.fn(),
+      loadModelFromBuffer: vi.fn(),
+      addModelFromUrl: vi.fn(),
+    } as never);
+  });
+
+  afterEach(() => {
+    destroyBridge();
+  });
+
+  describe('SET_CAMERA', () => {
+    it('reaches the camera actuator, not just the store field', () => {
+      // The whole defect: `setCameraRotation` wrote `cameraRotation` and
+      // stopped there, so the host got a success ack and a CAMERA_CHANGED echo
+      // of its own numbers while the view never moved.
+      win.dispatch(cmd('SET_CAMERA', { azimuth: 120, elevation: 30 }));
+
+      expect(store.rotations).toEqual([{ azimuth: 120, elevation: 30 }]);
+    });
+
+    it('records the new orientation in the store as well', () => {
+      win.dispatch(cmd('SET_CAMERA', { azimuth: 120, elevation: 30 }));
+
+      expect(store.getState().cameraRotation).toEqual({ azimuth: 120, elevation: 30 });
+    });
+  });
+
+  describe('RESET_COLORS', () => {
+    it('actually restores the color SET_COLORS baked in', () => {
+      store.getState().appendGeometryBatch([mesh(12, [1, 0, 0, 1])] as never);
+
+      win.dispatch(cmd('SET_COLORS', { colorMap: { '12': [0, 1, 0, 1] } }));
+      expect(store.getState().geometryResult.meshes[0].color).toEqual([0, 1, 0, 1]);
+
+      win.dispatch(cmd('RESET_COLORS'));
+
+      expect(store.getState().geometryResult.meshes[0].color).toEqual([1, 0, 0, 1]);
+      // And the renderer is told to re-upload the restored color, otherwise the
+      // GPU keeps showing the override.
+      expect(store.getState().pendingMeshColorUpdates.get(12)).toEqual([1, 0, 0, 1]);
+    });
+
+    it('leaves another subsystem\'s overlay colors intact', () => {
+      // `pendingColorUpdates` is the lens / IDS / clash / schedule overlay
+      // channel. RESET_COLORS used to clear exactly this and nothing else —
+      // wrong in both directions at once: the host's own override survived,
+      // and an overlay owner's state was destroyed.
+      store.getState().appendGeometryBatch([mesh(12, [1, 0, 0, 1])] as never);
+      store.getState().setPendingColorUpdates(new Map([[12, [1, 1, 0, 1]]]));
+
+      win.dispatch(cmd('SET_COLORS', { colorMap: { '12': [0, 1, 0, 1] } }));
+      win.dispatch(cmd('RESET_COLORS'));
+
+      expect(store.getState().pendingColorUpdates.get(12)).toEqual([1, 1, 0, 1]);
+    });
+  });
+});

apps/viewer-embed/src/bridge/handler.test.ts

@@ -169,6 +169,7 @@ function makeState() {
     showAllInAllModels: rec('showAllInAllModels'),
     updateMeshColors: rec('updateMeshColors'),
     clearPendingColorUpdates: rec('clearPendingColorUpdates'),
+    resetMeshColors: rec('resetMeshColors'),
     setCameraRotation: rec('setCameraRotation'),
     setSectionPlaneAxis: rec('setSectionPlaneAxis'),
     setSectionPlanePosition: rec('setSectionPlanePosition'),
@@ -814,15 +815,24 @@ describe('selection and visibility commands', () => {
   it('SET_COLORS converts string keys to numeric entity ids', async () => {
     initBridge(makeCtx(state));
     await send(fw, cmd('SET_COLORS', { colorMap: { '12': [1, 0, 0, 1] } }, 'r1'));
-    const [updates] = argsOf(state, 'updateMeshColors') as [Map<number, unknown>];
+    const [updates, options] = argsOf(state, 'updateMeshColors') as [Map<number, unknown>, { override?: boolean }];
     expect([...updates.keys()]).toEqual([12]);
     expect(updates.get(12)).toEqual([1, 0, 0, 1]);
+    // Marked as an override so the displaced colors are captured and
+    // RESET_COLORS can put them back.
+    expect(options).toEqual({ override: true });
   });
 
-  it('RESET_COLORS clears pending updates', async () => {
+  it('RESET_COLORS undoes the SET_COLORS bake and leaves the overlay channel alone', async () => {
     initBridge(makeCtx(state));
     await send(fw, cmd('RESET_COLORS', undefined, 'r1'));
-    expect(called(state, 'clearPendingColorUpdates')).toBe(true);
+    // `resetMeshColors` restores what SET_COLORS baked into
+    // geometryResult.meshes[].color. `clearPendingColorUpdates` is the
+    // lens/IDS/clash overlay channel — a different subsystem's state, which
+    // this command must not touch (#2934). See handler.effects.test.ts for the
+    // same pair asserted on the real slices rather than on this double.
+    expect(called(state, 'resetMeshColors')).toBe(true);
+    expect(called(state, 'clearPendingColorUpdates')).toBe(false);
   });
 
   it('SET_TYPE_VISIBILITY toggles only the flags that actually differ', async () => {

apps/viewer-embed/src/bridge/handler.ts

@@ -375,13 +375,20 @@ async function handleCommand(type: InboundCommandType, data: unknown, requestId?
       for (const [key, color] of Object.entries(payload.colorMap)) {
         updates.set(Number(key), color);
       }
-      state.updateMeshColors(updates);
+      // `override` so the displaced colors are captured and RESET_COLORS can
+      // put them back.
+      state.updateMeshColors(updates, { override: true });
       if (requestId) emitToParent(createResponse(requestId));
       return;
     }
 
     case 'RESET_COLORS': {
-      state.clearPendingColorUpdates();
+      // Undo SET_COLORS: restore the colors it baked into
+      // geometryResult.meshes[].color. Deliberately NOT
+      // clearPendingColorUpdates() — that is the separate lens/IDS/clash/
+      // schedule overlay channel, which SET_COLORS never writes to and this
+      // command has no claim on.
+      state.resetMeshColors();
       if (requestId) emitToParent(createResponse(requestId));
       return;
     }

apps/viewer-embed/src/components/EmbedViewer.test.ts

@@ -171,3 +171,99 @@ describe('EmbedViewer: SET_SECTION emits SECTION_CHANGED to the parent', () => {
     });
   });
 });
+
+/**
+ * ENTITY_HOVERED is a declared OutboundEventType with SDK listener plumbing
+ * and SDK tests — and, before #2934, zero `emitEvent('ENTITY_HOVERED', ...)`
+ * call sites anywhere in this app. The SDK's tests pass because they call
+ * `harness.emit('ENTITY_HOVERED', ...)` themselves, which proves the SDK
+ * dispatches an event the viewer never sent.
+ *
+ * The viewer's hover pipeline is: pointermove -> throttled `renderer.pick()`
+ * -> `setHoverState(...)` (apps/viewer/src/components/viewer/useMouseControls.ts
+ * ~703), with that whole branch gated on `hoverTooltipsEnabled`. These tests
+ * enter at `setHoverState` — the store action the pick path calls — and assert
+ * what reaches `window.parent.postMessage`, covering both remaining links: the
+ * gate the embed has to force on, and the emit. Driving `renderer.pick()`
+ * itself needs a real WebGPU device and is out of reach here.
+ */
+describe('EmbedViewer: emits ENTITY_HOVERED from the viewer hover pipeline', () => {
+  afterEach(() => {
+    useViewerStore.getState().clearHover();
+  });
+
+  it('forces hoverTooltipsEnabled on, without which the pick path never runs', () => {
+    // Defaults to false (UI_DEFAULTS.HOVER_TOOLTIPS_ENABLED) — a main-viewer
+    // toolbar toggle the embed has no chrome to offer.
+    useViewerStore.setState({ hoverTooltipsEnabled: false });
+
+    renderEmbedViewer();
+
+    expect(useViewerStore.getState().hoverTooltipsEnabled).toBe(true);
+  });
+
+  it('posts ENTITY_HOVERED to the parent when the pick path reports a hovered entity', () => {
+    const posted: EmbedMessageEnvelope[] = [];
+    Object.defineProperty(window, 'parent', {
+      configurable: true,
+      value: { postMessage: (msg: EmbedMessageEnvelope) => posted.push(msg) },
+    });
+
+    renderEmbedViewer();
+    // emitToParent withholds every non-READY message until a concrete
+    // parentOrigin is captured from a real inbound message — establish that
+    // first, same as the SET_SECTION test above.
+    dispatchInbound({ type: 'SET_THEME', data: { theme: 'light' } });
+
+    act(() => {
+      // Exactly what useMouseControls does with a pick hit.
+      useViewerStore.getState().setHoverState({ entityId: 42, screenX: 10, screenY: 20 });
+    });
+
+    const hovered = posted.find((m) => m.type === 'ENTITY_HOVERED');
+    expect(hovered?.data).toEqual({ id: 42, globalId: undefined, ifcType: undefined });
+  });
+
+  it('does not re-post for the same entity as the pointer drifts across it', () => {
+    const posted: EmbedMessageEnvelope[] = [];
+    Object.defineProperty(window, 'parent', {
+      configurable: true,
+      value: { postMessage: (msg: EmbedMessageEnvelope) => posted.push(msg) },
+    });
+
+    renderEmbedViewer();
+    dispatchInbound({ type: 'SET_THEME', data: { theme: 'light' } });
+
+    act(() => {
+      useViewerStore.getState().setHoverState({ entityId: 42, screenX: 10, screenY: 20 });
+    });
+    act(() => {
+      // Same entity, new screen position — every throttled mousemove within
+      // one mesh produces this.
+      useViewerStore.getState().setHoverState({ entityId: 42, screenX: 11, screenY: 21 });
+    });
+
+    expect(posted.filter((m) => m.type === 'ENTITY_HOVERED').length).toBe(1);
+  });
+
+  it('posts again once the pointer moves onto a different entity', () => {
+    const posted: EmbedMessageEnvelope[] = [];
+    Object.defineProperty(window, 'parent', {
+      configurable: true,
+      value: { postMessage: (msg: EmbedMessageEnvelope) => posted.push(msg) },
+    });
+
+    renderEmbedViewer();
+    dispatchInbound({ type: 'SET_THEME', data: { theme: 'light' } });
+
+    act(() => {
+      useViewerStore.getState().setHoverState({ entityId: 42, screenX: 10, screenY: 20 });
+    });
+    act(() => {
+      useViewerStore.getState().setHoverState({ entityId: 43, screenX: 30, screenY: 40 });
+    });
+
+    expect(posted.filter((m) => m.type === 'ENTITY_HOVERED').map((m) => (m.data as { id: number }).id))
+      .toEqual([42, 43]);
+  });
+});