fix(viewer): stop isolation and solo from overwriting each other's layer masks - #607
Open
toycenterboss-bot wants to merge 1 commit into
Open
fix(viewer): stop isolation and solo from overwriting each other's layer masks#607toycenterboss-bot wants to merge 1 commit into
toycenterboss-bot wants to merge 1 commit into
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>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.