fix(viewer): stop isolation and solo from overwriting each other's layer masks - #607
Conversation
…yers Both features hide an object by clearing its scene layer, and each stashed the previous `layers.mask` under its own private Symbol, restoring it wholesale on the way out. That only holds while they nest. Interleave them and the second to finish writes back a mask the first has since changed: solo a floor, isolate a wall, leave solo hands every level its scene layer straight back, so leaving solo un-hides exactly what the isolation filter was hiding. Clearing the filter afterwards then restores the mask isolation captured *during* solo, and the level is stuck shadow-caster-only with nothing soloed — invisible until reload. `lib/scene-visibility.ts` takes the mask over. Callers name a reason rather than a mask, the mask is recomputed from the one snapshot taken when the first reason arrived, and the original is handed back only when the last reason leaves. Order stops mattering, and two duplicated stash implementations collapse into one. The new `isolation.test.ts` drives the real pair in both interleavings; both cases fail on `main` and pass here. Co-Authored-By: Claude <noreply@anthropic.com>
Aymericr
left a comment
There was a problem hiding this comment.
Reviewed against the viewer layer/visibility contracts and tested on the PR head plus a local merge with current main. The reason-based layer ownership preserves isolation and shadow-only state in either teardown order; focused tests (9) and the viewer dependency typecheck pass. Ready to merge after the maintainer's local visual smoke.
|
Thank you, @toycenterboss-bot. The interleaved solo/isolation reproduction was excellent: it exposed that two features were independently restoring stale layer masks instead of sharing ownership. The reason-based visibility owner is simpler, removes duplicated stash logic, and the tests cover both teardown orders. I completed the final viewer-boundary and current- Merging this first, as designed. Credit for the contribution remains with Andrei Efremov in the squash commit and PR history. |
What does this PR do?
Isolation (
lib/isolation.ts) and solo's shadow-caster-only pass (lib/shadow-only.ts)both hide an object by clearing its
SCENE_LAYERbit, and each stashed the object'sprevious
layers.maskunder its own privateSymbol, restoring it wholesale on theway out.
That holds while the two nest. Interleaved, the second to finish writes back a mask the
first has since changed:
shadow-onlystashes theoriginal mask.
isolationstashes the current mask — which is now theshadow-only one.
shadow-onlyrestores what it stashed, so every level gets its scenelayer back while the isolation filter is still up. Everything the filter was
hiding reappears.
isolationrestores what it stashed, so the level ends upshadow-caster-only with nothing soloed — invisible until reload.
The other order (isolate → solo → clear isolation) breaks the same way, leaving a level
stuck in the shadow pass.
Fix. A small module,
lib/scene-visibility.ts, ownsObject3D.layersfor bothfeatures. Callers name a reason (
isolated,shadow-only) rather than handing over amask. The snapshot is taken once, when the first reason arrives; the mask is recomputed
from that snapshot while any reason holds; the original is handed back only when the last
reason leaves. Order stops mattering, and the two duplicated stash implementations
collapse into one.
Behaviour is unchanged whenever only one of the two is active.
How to test
Automated —
bun run test --filter @pascal-app/viewer:src/lib/isolation.test.tsdrives the realapplyIsolation/applyShadowOnlypair inboth interleavings. Both cases fail on
mainand pass here — I verified byrestoring just
isolation.tsandshadow-only.tsfrommainand re-running: 2 fail,1 pass (the third test covers solo being re-applied every frame, which was already
correct).
src/lib/scene-visibility.test.tscovers the module itself: reason stacking, unwindorder, idempotent re-hide, dropping a reason that was never held, and an object that was
already off the scene layer before anything hid it.
Manually, in a scene with more than one level:
mainthe hidden levels come back while the filter is still on; herethey stay hidden.
mainthe level stays invisible until reload; here everythingreturns to normal.
Checklist
bun devbun checkcleanbun check-typescleanmainNote
Medium Risk
Touches core viewer rendering visibility (layer masks) for isolation and solo; behavior is well covered by new tests but incorrect logic would show wrong geometry in the 3D view.
Overview
Fixes a bug where isolation and solo (shadow-only) each stashed and restored
Object3D.layers.maskindependently, so when both were active the feature that cleared second could restore a stale mask—e.g. hidden levels reappearing while isolation was still on, or a level stuck invisible after clearing isolation.Introduces
scene-visibility.tsas the single owner of scene-layer hiding: callers pass a reason (isolated|shadow-only) viahideFromScene/showInScene. The original mask is snapshotted once; the mask is recomputed while any reason remains; the snapshot is restored only when the last reason is dropped.isolation.tsandshadow-only.tsare wired through this API instead of per-feature Symbols.Adds
scene-visibility.test.ts(stacking, unwind order, idempotency) andisolation.test.ts(interleaved solo/isolation scenarios that failed onmain).Reviewed by Cursor Bugbot for commit 8c0d835. Bugbot is set up for automated code reviews on this repo. Configure here.