From 4c81435e3b8552c9db7d78bfbf747c618d2df3db Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Fri, 26 Jun 2026 11:12:47 -0400 Subject: [PATCH] fix(doors): correct folding fold direction + make open-clip names unique per node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues surfaced testing the baked viewer: - Folding door folded toward +z (into the room) — the joint rotation sign was inverted, so the accordion opened the wrong way ("weird position"). Flip to `(prevDirection - direction) * foldAngle` so leaves fold toward −z, matching the original inline rig. Verified panel-for-panel against the original formula at every operationState. - Openable clips were named by display name (`: open`), but the baked viewer drives playback by clip name (`useAnimations` maps name → action). Several windows share the name "Window 1", so their clips collapsed to one action and triggering any one opened the first. Key the clip name by node id (`: open`) — unique, matching the item-loop convention; the human-readable name still lives in `extras.label`. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/editor/src/lib/glb-export.test.ts | 8 ++--- packages/editor/src/lib/glb-export.ts | 29 ++++++++++--------- .../viewer/src/systems/door/door-system.tsx | 5 ++-- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/editor/src/lib/glb-export.test.ts b/packages/editor/src/lib/glb-export.test.ts index 872c87c6..669eddff 100644 --- a/packages/editor/src/lib/glb-export.test.ts +++ b/packages/editor/src/lib/glb-export.test.ts @@ -135,7 +135,7 @@ describe('prepareSceneForExport', () => { kind: 'door', label: 'Front door', openable: true, - clips: ['Front door: open'], + clips: ['door_test: open'], }) // The swing-leaf marker must not survive into glTF extras. @@ -245,7 +245,7 @@ describe('prepareSceneForExport', () => { expect(animations).toHaveLength(1) const clip = animations[0]! - expect(clip.name).toBe('Door: open') + expect(clip.name).toBe('door_swing: open') expect(clip.duration).toBe(1) // Playback intent carried in extras so consumers can play once and hold. expect(clip.userData).toEqual({ loop: false }) @@ -297,7 +297,7 @@ describe('prepareSceneForExport', () => { expect(animations).toHaveLength(1) const clip = animations[0]! - expect(clip.name).toBe('Slider: open') + expect(clip.name).toBe('door_sliding: open') expect(clip.duration).toBe(1) expect(clip.userData).toEqual({ loop: false }) @@ -321,7 +321,7 @@ describe('prepareSceneForExport', () => { const exported = scene.getObjectByProperty('name', doorId) expect(exported?.userData.openable).toBe(true) - expect(exported?.userData.clips).toEqual(['Slider: open']) + expect(exported?.userData.clips).toEqual(['door_sliding: open']) }) test('bakes a roll-up curtain into a sampled scale clip', () => { diff --git a/packages/editor/src/lib/glb-export.ts b/packages/editor/src/lib/glb-export.ts index 860daefa..31594338 100644 --- a/packages/editor/src/lib/glb-export.ts +++ b/packages/editor/src/lib/glb-export.ts @@ -526,7 +526,7 @@ function bakeOperationDoorClip( poseDoorMovingParts(node, doorObject, 0) if (tracks.length === 0) return null - return openClip(id, node, tracks) + return openClip(id, tracks) } function samplesMovePosition(flat: number[], base: THREE.Vector3): boolean { @@ -588,21 +588,24 @@ function bakeSwingDoorClip( }) if (tracks.length === 0) return null - return openClip(id, node, tracks) + return openClip(id, tracks) } /** - * Wrap an open motion in a named 1-second clip. The name uses the node's label - * when set (e.g. "Door 1: open") so a glTF player lists readable clips, falling - * back to the id. glTF has no core loop flag — the player decides — so we stamp - * `extras.loop = false` (via the clip's userData, which `GLTFExporter` - * serialises onto the animation): Pascal's `/viewer` and any extras-aware - * consumer play it once and hold the open pose; a dumb glTF player still loops. - * Consumers map a clip back to its node by walking up from a channel's target to - * the nearest ancestor carrying `extras.pascalId`, so the name stays cosmetic. + * Wrap an open motion in a named 1-second clip. The name is keyed by the node id + * (`: open`), NOT the node's display name: clip names must be unique because + * the baked viewer drives playback by clip name (`useAnimations` maps name → + * action), so two same-named openables (e.g. several "Window 1"s) would collapse + * to a single action and a trigger on one would animate another. The + * human-readable name lives in `extras.label` instead. glTF has no core loop + * flag — the player decides — so we stamp `extras.loop = false` (via the clip's + * userData, which `GLTFExporter` serialises onto the animation): Pascal's + * `/viewer` and any extras-aware consumer play it once and hold the open pose; a + * dumb glTF player still loops. Consumers map a clip back to its node by walking + * up from a channel's target to the nearest ancestor carrying `extras.pascalId`. */ -function openClip(id: string, node: AnyNode, tracks: THREE.KeyframeTrack[]): THREE.AnimationClip { - const clip = new THREE.AnimationClip(`${node.name ?? id}: open`, 1, tracks) +function openClip(id: string, tracks: THREE.KeyframeTrack[]): THREE.AnimationClip { + const clip = new THREE.AnimationClip(`${id}: open`, 1, tracks) clip.userData = { loop: false } return clip } @@ -662,7 +665,7 @@ function bakeWindowClip( poseWindowMovingParts(node, windowObject, 0) if (tracks.length === 0) return null - return openClip(id, node, tracks) + return openClip(id, tracks) } // --- Identity stamping --------------------------------------------------- diff --git a/packages/viewer/src/systems/door/door-system.tsx b/packages/viewer/src/systems/door/door-system.tsx index c1901611..ecb654dd 100644 --- a/packages/viewer/src/systems/door/door-system.tsx +++ b/packages/viewer/src/systems/door/door-system.tsx @@ -1401,7 +1401,8 @@ export function poseDoorMovingParts( const foldAngle = Math.PI * 0.44 * t // Each panel group is parented to the previous, so its rotation is the // joint angle (the change in absolute segment direction), not the absolute - // angle: alternating panels target ±foldAngle, so joints fold by ±2·angle. + // angle: alternating panels target ∓foldAngle, so joints fold by ±2·angle. + // The leading sign folds the leaves toward −z (matching the original rig). let posed = false let prevDirection = 0 for (let index = 0; index < panelCount; index++) { @@ -1409,7 +1410,7 @@ export function poseDoorMovingParts( const direction = index % 2 === 0 ? -1 : 1 if (group) { posed = true - group.rotation.y = (direction - prevDirection) * foldAngle + group.rotation.y = (prevDirection - direction) * foldAngle } prevDirection = direction }