Skip to content

fix(viewer): clear pendingCameraRotation on a session reset - #3375

Open
BIMvoice wants to merge 1 commit into
mainfrom
fix-3364-pending-camera-rotation-reset
Open

fix(viewer): clear pendingCameraRotation on a session reset#3375
BIMvoice wants to merge 1 commit into
mainfrom
fix-3364-pending-camera-rotation-reset

Conversation

@BIMvoice

Copy link
Copy Markdown
Collaborator

Summary

  • resetViewerState reset cameraRotation to its default but left pendingCameraRotation holding the outgoing model's value.
  • cameraSlice's existing replay path (setCameraCallbacks reads pendingCameraRotation and, if set, calls setCameraRotation with it — meant for a rotation set before the renderer registered) then applies the outgoing model's stale rotation onto the freshly reset camera, if the next model's Viewport calls setCameraCallbacks before any new rotation is set.
  • Root cause in one sentence: cameraTeardown's owns array and session-reset patch never included pendingCameraRotation, so a session reset couldn't clear it.

Fixes #3364.

Family context

This is the fourth instance of "model-relative state survives a teardown because a field is missing from the slice's clear-list" (after #3367, #3371, #3372).

Overlap with PR #3367 (fix-2934-embed-controls, still open): it edits the same four spots this PR does — cameraTeardown's owns array and reset patch in apps/viewer/src/store/slices/cameraSlice.ts, and PINNED_SESSION_RESET_KEYS / PINNED_OWNED_KEYS in apps/viewer/src/store/teardown-registry.test.ts — adding interactionMode and pendingInteractionMode. Whichever of these two PRs merges second will conflict on those four spots. The correct resolution keeps all three fieldsinteractionMode, pendingInteractionMode, and pendingCameraRotation — never one side taken wholesale.

This PR also updates the doc comment above cameraTeardown, which claimed pendingCameraRotation was deliberately unowned ("renderer/host wiring that outlives a file swap"). That was true before this fix and false after it, so the comment is corrected rather than left contradicting the code.

Test plan

  • RED proven: stashed only cameraSlice.ts (source, not the test files) via git stash push --keep-index -- apps/viewer/src/store/slices/cameraSlice.ts, ran the new cameraSlice.test.ts cases — 2 fail / 5 pass — then restored by exact stash SHA (git stash apply <sha> + git stash drop, never a bare pop).
  • GREEN after the fix: npx tsx --import ./src/test/vite-module-hooks.mjs --test --test-timeout=120000 --test-concurrency=1 src/store/slices/cameraSlice.test.ts src/store/teardown-registry.test.ts src/store/teardown.idempotence.test.ts from apps/viewer — 14/14 pass.
  • node scripts/check-module-size.mjs exits 0.
  • PINNED_SESSION_RESET_KEYS and PINNED_OWNED_KEYS in teardown-registry.test.ts updated in the same commit (alphabetically, pendingCameraRotation before pendingColorUpdates).
  • git status --porcelain clean before pushing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436

resetViewerState reset cameraRotation to its default but left
pendingCameraRotation holding the outgoing model's value. If the next
model's Viewport then calls setCameraCallbacks before any new rotation
is set, cameraSlice's existing replay path (setCameraCallbacks reads
pendingCameraRotation and calls setCameraRotation with it) applies the
outgoing model's stale rotation onto the freshly reset camera.

Add pendingCameraRotation to cameraTeardown's owns array and its
session-reset patch, alongside cameraRotation. Update the doc comment
above cameraTeardown, which claimed pendingCameraRotation was
deliberately unowned "renderer/host wiring that outlives a file swap" —
that was true before this fix and is false after it.

Fixes #3364.

Note for whoever merges next: PR #3367 (branch fix-2934-embed-controls)
touches the same four spots — cameraTeardown's owns array and reset
patch in cameraSlice.ts, and PINNED_SESSION_RESET_KEYS /
PINNED_OWNED_KEYS in teardown-registry.test.ts — adding
interactionMode and pendingInteractionMode. The correct merge
resolution keeps all three fields (interactionMode,
pendingInteractionMode, pendingCameraRotation), never one side taken
wholesale.

Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
@BIMvoice
BIMvoice requested a review from louistrue as a code owner August 28, 2026 05:00
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

Or wait 17 minutes for your next included review.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 80992981-dcbb-4eb8-bd33-2fb90c576aae

📥 Commits

Reviewing files that changed from the base of the PR and between 5a431e5 and 244a2cb.

📒 Files selected for processing (3)
  • apps/viewer/src/store/slices/cameraSlice.test.ts
  • apps/viewer/src/store/slices/cameraSlice.ts
  • apps/viewer/src/store/teardown-registry.test.ts

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Viewer benchmark

1 metric(s) exceeded the regression threshold (advisory only, not blocking).

01_Snowdon_Towers_Sample_Structural(1).ifc

Baseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric Current Baseline Delta Threshold Status
firstBatchWaitMs 2132ms 2905ms -26.6% +50%
firstVisibleGeometryMs 2903ms 3652ms -20.5% +50%
streamCompleteMs 3268ms 3598ms -9.2% +50%
spatialReadyMs 1562ms 1032ms +51.4% +50%
metadataCompleteMs 2282ms 3063ms -25.5% +50%
totalWallClockMs 3800ms 3700ms +2.7% +50%

AC20-FZK-Haus.ifc

Baseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric Current Baseline Delta Threshold Status
firstBatchWaitMs 348ms 1075ms -67.6% +50%
firstVisibleGeometryMs 1289ms 1572ms -18.0% +50%
streamCompleteMs 1449ms 1980ms -26.8% +50%
spatialReadyMs 1014ms 915ms +10.8% +50%
metadataCompleteMs 1172ms 1392ms -15.8% +50%
totalWallClockMs 3000ms 3300ms -9.1% +50%

Refresh the baseline from a CI run: dispatch the Benchmark workflow with record_baseline, download the benchmark-baseline artifact, and commit baseline.json (see tests/benchmark/README.md).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A pending camera rotation survives a file swap and replays onto the next model

1 participant