From 8c0d835c2fccaaf1c4d2d2ef52d640fa886c4c08 Mon Sep 17 00:00:00 2001 From: Andrei Efremov Date: Fri, 7 Aug 2026 11:06:55 +0300 Subject: [PATCH] fix(viewer): stop isolation and solo from overwriting each other's layers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/viewer/src/lib/isolation.test.ts | 71 +++++++++++++++ packages/viewer/src/lib/isolation.ts | 51 ++++------- .../viewer/src/lib/scene-visibility.test.ts | 90 +++++++++++++++++++ packages/viewer/src/lib/scene-visibility.ts | 58 ++++++++++++ packages/viewer/src/lib/shadow-only.ts | 28 ++---- 5 files changed, 245 insertions(+), 53 deletions(-) create mode 100644 packages/viewer/src/lib/isolation.test.ts create mode 100644 packages/viewer/src/lib/scene-visibility.test.ts create mode 100644 packages/viewer/src/lib/scene-visibility.ts diff --git a/packages/viewer/src/lib/isolation.test.ts b/packages/viewer/src/lib/isolation.test.ts new file mode 100644 index 0000000000..9c64c88fcd --- /dev/null +++ b/packages/viewer/src/lib/isolation.test.ts @@ -0,0 +1,71 @@ +// @ts-expect-error — bun:test is provided by the Bun runtime; viewer does not +// depend on @types/bun so the import type is unresolved at compile time. +import { afterEach, describe, expect, test } from 'bun:test' +import type { AnyNodeId } from '@pascal-app/core' +import { sceneRegistry } from '@pascal-app/core' +import * as THREE from 'three' +import { applyIsolation, clearIsolation } from './isolation' +import { SCENE_LAYER, SHADOW_ONLY_LAYER } from './layers' +import { applyShadowOnly, clearShadowOnly } from './shadow-only' + +function register(id: string): THREE.Object3D { + const obj = new THREE.Object3D() + obj.layers.set(SCENE_LAYER) + sceneRegistry.nodes.set(id, obj) + return obj +} + +/** Isolation takes node ids; the registry only cares that the key matches. */ +function isolate(...ids: string[]): void { + applyIsolation(ids as ReadonlyArray) +} + +describe('isolation and solo, interleaved', () => { + afterEach(() => { + clearIsolation() + sceneRegistry.clear() + }) + + test('leaving solo while isolated keeps the filtered scene filtered', () => { + const level = register('level-1') + const focus = register('wall-1') + const original = level.layers.mask + + applyShadowOnly(level) + isolate('wall-1') + clearShadowOnly(level) + + expect(level.layers.isEnabled(SCENE_LAYER)).toBe(false) + expect(level.layers.isEnabled(SHADOW_ONLY_LAYER)).toBe(false) + expect(focus.layers.isEnabled(SCENE_LAYER)).toBe(true) + + clearIsolation() + expect(level.layers.mask).toBe(original) + }) + + test('leaving isolation while soloed keeps the level casting shadows', () => { + const level = register('level-1') + register('wall-1') + const original = level.layers.mask + + isolate('wall-1') + applyShadowOnly(level) + clearIsolation() + + expect(level.layers.isEnabled(SCENE_LAYER)).toBe(false) + expect(level.layers.isEnabled(SHADOW_ONLY_LAYER)).toBe(true) + + clearShadowOnly(level) + expect(level.layers.mask).toBe(original) + }) + + test('solo re-applied every frame does not accumulate', () => { + const level = register('level-1') + const original = level.layers.mask + + for (let frame = 0; frame < 5; frame += 1) applyShadowOnly(level) + clearShadowOnly(level) + + expect(level.layers.mask).toBe(original) + }) +}) diff --git a/packages/viewer/src/lib/isolation.ts b/packages/viewer/src/lib/isolation.ts index 3c990ac5bb..409a80bc45 100644 --- a/packages/viewer/src/lib/isolation.ts +++ b/packages/viewer/src/lib/isolation.ts @@ -3,17 +3,10 @@ import type { AnyNodeId } from '@pascal-app/core' import { sceneRegistry } from '@pascal-app/core' import type { Object3D } from 'three' -import { SCENE_LAYER } from './layers' +import { hideFromScene, showInScene } from './scene-visibility' -// Marker on each Object3D we modify during isolation so we can restore -// the original `layers.mask` bitfield. Stored under a `Symbol` so it -// can't collide with any kind's own userData fields. -const ORIGINAL_LAYERS = Symbol('isolation:original-layers') - -type IsolationCarrier = Object3D & { [ORIGINAL_LAYERS]?: number } - -// Whether a subtree is currently isolated (some objects have SCENE_LAYER -// disabled). Read by consumers that must not act on the partial view — e.g. +// Whether a subtree is currently isolated (some objects are held off the +// scene layer). Read by consumers that must not act on the partial view — e.g. // the project-thumbnail autosave skips capturing while isolated so it never // snapshots a single focused item as the whole project's thumbnail. let isolationActive = false @@ -47,21 +40,21 @@ export function collectIsolationSubtree(ids: ReadonlyArray): Set | null): void { const keep = collectIsolationSubtree(ids as ReadonlyArray) // Iterate registered roots. For each one outside the keep set, - // disable `SCENE_LAYER` on it and on every descendant — *except* - // descendants that are themselves in `keep` (a kept node nested under - // a non-kept host: the isolated door under the hidden wall). + // hide it and every descendant — *except* descendants that are + // themselves in `keep` (a kept node nested under a non-kept host: + // the isolated door under the hidden wall). for (const [, obj] of sceneRegistry.nodes) { if (keep.has(obj)) continue hideRecursive(obj, keep) @@ -87,11 +80,7 @@ export function applyIsolation(ids: ReadonlyArray | null): void { function hideRecursive(obj: Object3D, keep: Set): void { if (keep.has(obj)) return - const carrier = obj as IsolationCarrier - if (carrier[ORIGINAL_LAYERS] === undefined) { - carrier[ORIGINAL_LAYERS] = obj.layers.mask - } - obj.layers.disable(SCENE_LAYER) + hideFromScene(obj, 'isolated') for (const child of obj.children) { hideRecursive(child, keep) } @@ -99,15 +88,11 @@ function hideRecursive(obj: Object3D, keep: Set): void { export function clearIsolation(): void { // We don't know which objects were touched without re-walking, so - // walk every registered root + its descendants and restore any - // stashed original-mask. `traverse` is cheap and idempotent here. + // walk every registered root + its descendants and drop the isolation + // reason wherever it was set. `traverse` is cheap and idempotent here. for (const [, obj] of sceneRegistry.nodes) { obj.traverse((child) => { - const carrier = child as IsolationCarrier - if (carrier[ORIGINAL_LAYERS] !== undefined) { - child.layers.mask = carrier[ORIGINAL_LAYERS] - delete carrier[ORIGINAL_LAYERS] - } + showInScene(child, 'isolated') }) } isolationActive = false diff --git a/packages/viewer/src/lib/scene-visibility.test.ts b/packages/viewer/src/lib/scene-visibility.test.ts new file mode 100644 index 0000000000..b47ad42b68 --- /dev/null +++ b/packages/viewer/src/lib/scene-visibility.test.ts @@ -0,0 +1,90 @@ +// @ts-expect-error — bun:test is provided by the Bun runtime; viewer does not +// depend on @types/bun so the import type is unresolved at compile time. +import { describe, expect, test } from 'bun:test' +import * as THREE from 'three' +import { OVERLAY_LAYER, SCENE_LAYER, SHADOW_ONLY_LAYER } from './layers' +import { hideFromScene, showInScene } from './scene-visibility' + +function sceneObject(): THREE.Object3D { + const obj = new THREE.Object3D() + obj.layers.set(SCENE_LAYER) + return obj +} + +describe('scene visibility', () => { + test('one reason hides and gives the exact mask back', () => { + const obj = sceneObject() + obj.layers.enable(OVERLAY_LAYER) + const original = obj.layers.mask + + hideFromScene(obj, 'isolated') + expect(obj.layers.isEnabled(SCENE_LAYER)).toBe(false) + expect(obj.layers.isEnabled(OVERLAY_LAYER)).toBe(true) + + showInScene(obj, 'isolated') + expect(obj.layers.mask).toBe(original) + }) + + test('the reason still standing decides the mask, whatever the order', () => { + const obj = sceneObject() + + hideFromScene(obj, 'shadow-only') + hideFromScene(obj, 'isolated') + + // Leaving solo first must not hand the scene layer back while the + // isolation filter is still up. + showInScene(obj, 'shadow-only') + expect(obj.layers.isEnabled(SCENE_LAYER)).toBe(false) + expect(obj.layers.isEnabled(SHADOW_ONLY_LAYER)).toBe(false) + + showInScene(obj, 'isolated') + expect(obj.layers.isEnabled(SCENE_LAYER)).toBe(true) + }) + + test('dropping isolation under solo leaves the object casting shadows', () => { + const obj = sceneObject() + + hideFromScene(obj, 'isolated') + hideFromScene(obj, 'shadow-only') + showInScene(obj, 'isolated') + + expect(obj.layers.isEnabled(SHADOW_ONLY_LAYER)).toBe(true) + expect(obj.layers.isEnabled(SCENE_LAYER)).toBe(false) + }) + + test('re-hiding for a reason already held changes nothing', () => { + const obj = sceneObject() + + hideFromScene(obj, 'shadow-only') + const held = obj.layers.mask + hideFromScene(obj, 'shadow-only') + expect(obj.layers.mask).toBe(held) + + showInScene(obj, 'shadow-only') + expect(obj.layers.isEnabled(SCENE_LAYER)).toBe(true) + }) + + test('dropping a reason that was never held is a no-op', () => { + const obj = sceneObject() + const original = obj.layers.mask + + showInScene(obj, 'isolated') + expect(obj.layers.mask).toBe(original) + + hideFromScene(obj, 'shadow-only') + showInScene(obj, 'isolated') + expect(obj.layers.isEnabled(SHADOW_ONLY_LAYER)).toBe(true) + expect(obj.layers.isEnabled(SCENE_LAYER)).toBe(false) + }) + + test('an object hidden while already off the scene layer stays off it', () => { + const obj = new THREE.Object3D() + obj.layers.set(OVERLAY_LAYER) + const original = obj.layers.mask + + hideFromScene(obj, 'isolated') + showInScene(obj, 'isolated') + expect(obj.layers.mask).toBe(original) + expect(obj.layers.isEnabled(SCENE_LAYER)).toBe(false) + }) +}) diff --git a/packages/viewer/src/lib/scene-visibility.ts b/packages/viewer/src/lib/scene-visibility.ts new file mode 100644 index 0000000000..86bebe8395 --- /dev/null +++ b/packages/viewer/src/lib/scene-visibility.ts @@ -0,0 +1,58 @@ +import type { Object3D } from 'three' +import { SCENE_LAYER, SHADOW_ONLY_LAYER } from './layers' + +/** + * Why an object is currently held off the scene layer. + * + * - `isolated` — outside the focused subtree of the viewer's isolation filter. + * - `shadow-only` — solo mode: out of the color passes, still casting shadows. + */ +export type HiddenReason = 'isolated' | 'shadow-only' + +/** + * Single owner of `Object3D.layers` for every feature that hides an object. + * + * Isolation and solo's shadow-caster pass both hide by clearing + * {@link SCENE_LAYER}, and they overlap freely — either can start or end while + * the other is up. While each stashed and restored the mask privately, the + * second to finish wrote back a mask the first had since changed. Recording + * *reasons* rather than masks makes the order irrelevant: the mask is + * recomputed from the one snapshot taken when the first reason arrived, and + * handed back only when the last one leaves. + */ +const HOLD = Symbol('pascal:scene-visibility:hold') + +type Hold = { original: number; reasons: Set } + +type Holder = Object3D & { [HOLD]?: Hold } + +/** Holds `obj` off the scene layer for `reason`. Idempotent per reason. */ +export function hideFromScene(obj: Object3D, reason: HiddenReason): void { + const holder = obj as Holder + const hold = holder[HOLD] ?? { original: obj.layers.mask, reasons: new Set() } + holder[HOLD] = hold + hold.reasons.add(reason) + applyHold(obj, hold) +} + +/** Drops `reason`, restoring the mask `obj` had before the first one arrived. */ +export function showInScene(obj: Object3D, reason: HiddenReason): void { + const holder = obj as Holder + const hold = holder[HOLD] + if (!hold) return + + hold.reasons.delete(reason) + if (hold.reasons.size > 0) { + applyHold(obj, hold) + return + } + + obj.layers.mask = hold.original + delete holder[HOLD] +} + +function applyHold(obj: Object3D, hold: Hold): void { + obj.layers.mask = hold.original + obj.layers.disable(SCENE_LAYER) + if (hold.reasons.has('shadow-only')) obj.layers.enable(SHADOW_ONLY_LAYER) +} diff --git a/packages/viewer/src/lib/shadow-only.ts b/packages/viewer/src/lib/shadow-only.ts index c63c10ab8e..209c985c3a 100644 --- a/packages/viewer/src/lib/shadow-only.ts +++ b/packages/viewer/src/lib/shadow-only.ts @@ -1,5 +1,5 @@ import type { Object3D } from 'three' -import { SCENE_LAYER, SHADOW_ONLY_LAYER } from './layers' +import { hideFromScene, showInScene } from './scene-visibility' /** * Shadow-caster-only hiding: removes an object (and its descendants) from the @@ -9,34 +9,22 @@ import { SCENE_LAYER, SHADOW_ONLY_LAYER } from './layers' * Uses layer masks instead of `visible = false` for two reasons: `visible` * cascades (and, critically, prunes the object from the shadow pass too), * while layers are tested per-object against the rendering camera — the main - * camera never enables {@link SHADOW_ONLY_LAYER}, but every shadow-casting + * camera never enables the shadow-only layer, but every shadow-casting * light's shadow camera does (see `lights.tsx`). * - * The original `layers.mask` is stashed under a private Symbol so - * {@link clearShadowOnly} restores the exact prior state. Both calls are - * idempotent and cheap to reapply. + * The mask itself belongs to `lib/scene-visibility.ts`, which reconciles this + * with the isolation filter. Both calls are idempotent and cheap to reapply — + * solo re-runs `applyShadowOnly` every frame so meshes rebuilt while hidden + * get re-hidden. */ - -const ORIGINAL_LAYERS = Symbol('pascal:shadow-only:original-layers') - -type ShadowOnlyCarrier = Object3D & { [ORIGINAL_LAYERS]?: number } - export function applyShadowOnly(root: Object3D): void { root.traverse((obj) => { - const carrier = obj as ShadowOnlyCarrier - if (carrier[ORIGINAL_LAYERS] === undefined) { - carrier[ORIGINAL_LAYERS] = obj.layers.mask - } - obj.layers.disable(SCENE_LAYER) - obj.layers.enable(SHADOW_ONLY_LAYER) + hideFromScene(obj, 'shadow-only') }) } export function clearShadowOnly(root: Object3D): void { root.traverse((obj) => { - const carrier = obj as ShadowOnlyCarrier - if (carrier[ORIGINAL_LAYERS] === undefined) return - obj.layers.mask = carrier[ORIGINAL_LAYERS] - delete carrier[ORIGINAL_LAYERS] + showInScene(obj, 'shadow-only') }) }