From 6d5f041b48f210bf908974c2bb19ab3df8910189 Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Mon, 15 Jun 2026 11:30:39 -0400 Subject: [PATCH] fix(walls): bound wall miters, heal corrupt scenes, follow grid snap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related editor fixes surfaced while debugging a captured house project that rendered an infinite wall and failed to load. Infinite wall (core/systems/wall/wall-mitering.ts): Junction miters are line-line intersections, so the joint point sits ~halfThickness/sin(theta) from the junction. The only guard was an exact-parallel check (det < 1e-9), so two walls meeting at a shallow angle (a room-preset preview dragged onto an existing wall, or a wall drawn nearly collinear to its neighbour) produced a joint point far away — an infinite spike. Add a miter limit: reject joints farther than 10x half-thickness from the junction and fall back to a square joint, exactly like the parallel case. Scene load failure (core/utils/heal-scene-graph.ts + validate-build-json + use-scene migrateNodes): Capture wall-merge could leave a `children: [null]` entry (see the matching merge-walls.ts fix in private-editor) and zero-length walls. `null` children fail wall schema validation, so the whole scene fails to load. Add a shared heal step — strip non-string child refs, drop childless zero-length walls — run on every load path: import validation now repairs instead of hard-failing (with a warning), and setScene heals on the prod project-load path too. Grid snap (nodes slab/ceiling/spawn tools): These tools hardcoded a 0.5 m snap (Math.round(x*2)/2) and ignored the editor's grid-snap setting, so the cursor jumped by 0.5 while later vertices already followed the configured step. Route them through snapPointToGrid / snapScalar with gridSnapStep. Adds unit tests for the miter limit and the heal step. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/store/use-scene.ts | 6 +- .../src/systems/wall/wall-mitering.test.ts | 66 +++++++++++++++ .../core/src/systems/wall/wall-mitering.ts | 45 +++++++++- .../core/src/utils/heal-scene-graph.test.ts | 50 +++++++++++ packages/core/src/utils/heal-scene-graph.ts | 83 +++++++++++++++++++ .../src/validation/validate-build-json.ts | 16 +++- packages/nodes/src/ceiling/move-tool.tsx | 5 +- packages/nodes/src/ceiling/tool.tsx | 7 +- packages/nodes/src/slab/tool.tsx | 7 +- packages/nodes/src/spawn/tool.tsx | 23 +++-- 10 files changed, 288 insertions(+), 20 deletions(-) create mode 100644 packages/core/src/systems/wall/wall-mitering.test.ts create mode 100644 packages/core/src/utils/heal-scene-graph.test.ts create mode 100644 packages/core/src/utils/heal-scene-graph.ts diff --git a/packages/core/src/store/use-scene.ts b/packages/core/src/store/use-scene.ts index b3ac8c9b..a3e7a009 100644 --- a/packages/core/src/store/use-scene.ts +++ b/packages/core/src/store/use-scene.ts @@ -21,6 +21,7 @@ import { SiteNode } from '../schema/nodes/site' import { StairNode as StairNodeSchema } from '../schema/nodes/stair' import { StairSegmentNode as StairSegmentNodeSchema } from '../schema/nodes/stair-segment' import type { AnyNode, AnyNodeId } from '../schema/types' +import { healSceneNodes } from '../utils/heal-scene-graph' import * as nodeActions from './actions/node-actions' import { resetSceneHistoryPauseDepth } from './history-control' @@ -414,7 +415,10 @@ function migrateRoofSurfaceMaterials(node: Record) { } function migrateNodes(nodes: Record): Record { - const patchedNodes = { ...nodes } + // Repair pre-existing corruption (null children, zero-length walls) before + // any per-type migration runs, so already-saved scenes load cleanly. + const { nodes: healed } = healSceneNodes(nodes) + const patchedNodes = { ...healed } as Record for (const [id, node] of Object.entries(patchedNodes)) { // 1. Item scale migration if (node.type === 'item' && !('scale' in node)) { diff --git a/packages/core/src/systems/wall/wall-mitering.test.ts b/packages/core/src/systems/wall/wall-mitering.test.ts new file mode 100644 index 00000000..349850ed --- /dev/null +++ b/packages/core/src/systems/wall/wall-mitering.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from 'bun:test' +import type { WallNode } from '../../schema' +import { calculateLevelMiters, getWallMiterBoundaryPoints } from './wall-mitering' + +function wall(id: string, start: [number, number], end: [number, number]): WallNode { + return { + id, + type: 'wall', + object: 'node', + visible: true, + parentId: 'level_test', + children: [], + start, + end, + thickness: 0.1, + height: 2.5, + frontSide: 'interior', + backSide: 'exterior', + metadata: {}, + } as WallNode +} + +function maxBoundaryCoord(walls: WallNode[]): number { + const miter = calculateLevelMiters(walls) + let max = 0 + for (const w of walls) { + const bp = getWallMiterBoundaryPoints(w, miter) + expect(bp).not.toBeNull() + if (!bp) continue + for (const p of [bp.startLeft, bp.startRight, bp.endLeft, bp.endRight]) { + expect(Number.isFinite(p.x)).toBe(true) + expect(Number.isFinite(p.y)).toBe(true) + max = Math.max(max, Math.abs(p.x), Math.abs(p.y)) + } + } + return max +} + +describe('wall mitering miter limit', () => { + // Two 3 m walls sharing the origin, meeting at decreasing angles. Without a + // miter limit the joint point runs to infinity as the angle → 0 (∝ 1/sin θ), + // which is the "infinite wall" seen when a room-preset preview lands on top of + // an existing wall. The boundary must stay bounded near the wall length. + test.each([90, 30, 10, 5, 1, 0.1, 0.01])('stays bounded at a %s° junction', (deg) => { + const rad = (deg * Math.PI) / 180 + const walls = [ + wall('A', [0, 0], [3, 0]), + wall('B', [0, 0], [3 * Math.cos(rad), 3 * Math.sin(rad)]), + ] + // 3 m walls + a few cm of joint: anything past ~4 m is a runaway spike. + expect(maxBoundaryCoord(walls)).toBeLessThan(4) + }) + + test('still miters a normal 90° corner', () => { + const walls = [wall('A', [0, 0], [3, 0]), wall('B', [0, 0], [0, 3])] + const miter = calculateLevelMiters(walls) + const bpA = getWallMiterBoundaryPoints(walls[0]!, miter) + expect(bpA).not.toBeNull() + if (!bpA) throw new Error('expected miter boundary points') + // The shared corner is pulled to the mitred intersection, offset from the + // raw butt position (halfThickness 0.05) by the diagonal of the joint. + const startSideX = Math.min(bpA.startLeft.x, bpA.startRight.x) + expect(startSideX).toBeLessThan(-0.001) + expect(startSideX).toBeGreaterThan(-0.5) + }) +}) diff --git a/packages/core/src/systems/wall/wall-mitering.ts b/packages/core/src/systems/wall/wall-mitering.ts index e6a2f295..193f11be 100644 --- a/packages/core/src/systems/wall/wall-mitering.ts +++ b/packages/core/src/systems/wall/wall-mitering.ts @@ -35,6 +35,17 @@ type JunctionData = Map const TOLERANCE = 0.001 +// Miter joints are line-line intersections, so the joint point sits a distance +// ≈ halfThickness / sin(θ) from the junction, where θ is the angle between the +// two walls. As θ → 0 (two walls nearly collinear — e.g. a room-preset preview +// dragged on top of an existing wall, or a freshly-drawn wall almost parallel +// to its neighbour) that distance runs away to infinity and the wall renders as +// an infinite spike. Cap the joint at this multiple of the wall half-thickness; +// beyond it we fall back to a square (butt) joint, exactly like the existing +// parallel-walls guard. 10× preserves every realistic corner (a 0.1 m wall keeps +// mitering down to ~11°) while bounding the pathological near-collinear case. +const MITER_LIMIT = 10 + function pointToKey(p: Point2D, tolerance = TOLERANCE): string { const snap = 1 / tolerance return `${Math.round(p.x * snap)},${Math.round(p.y * snap)}` @@ -198,6 +209,7 @@ interface ProcessedWall { edgeA: LineEquation // Left edge edgeB: LineEquation // Right edge isPassthrough: boolean // True if wall passes through junction (T-junction) + halfThickness: number // Used to bound the miter joint against runaway spikes } function calculateJunctionIntersections( @@ -228,7 +240,14 @@ function calculateJunctionIntersections( const edgeB = createLineFromPointAndVector(pB, v) const angle = Math.atan2(v.y, v.x) - processedWalls.push({ wallId: wall.id, angle, edgeA, edgeB, isPassthrough: true }) + processedWalls.push({ + wallId: wall.id, + angle, + edgeA, + edgeB, + isPassthrough: true, + halfThickness: halfT, + }) } } else { // Normal wall endpoint (start or end) @@ -245,7 +264,14 @@ function calculateJunctionIntersections( const edgeB = createLineFromPointAndVector(pB, v) const angle = Math.atan2(v.y, v.x) - processedWalls.push({ wallId: wall.id, angle, edgeA, edgeB, isPassthrough: false }) + processedWalls.push({ + wallId: wall.id, + angle, + edgeA, + edgeB, + isPassthrough: false, + halfThickness: halfT, + }) } } @@ -275,6 +301,21 @@ function calculateJunctionIntersections( y: (wall2.edgeB.a * wall1.edgeA.c - wall1.edgeA.a * wall2.edgeB.c) / det, } + // Miter limit: `det` only catches walls that are *exactly* parallel. Two + // walls meeting at a shallow angle have a small-but-nonzero `det`, so `p` + // lands far from the junction (∝ 1/sin θ) and the wall renders as an + // infinite spike. Reject any joint farther than MITER_LIMIT half-thicknesses + // from the meeting point — those walls fall back to a square joint. + const maxMiter = MITER_LIMIT * Math.max(wall1.halfThickness, wall2.halfThickness) + const dx = p.x - meetingPoint.x + const dy = p.y - meetingPoint.y + if ( + !(Number.isFinite(p.x) && Number.isFinite(p.y)) || + dx * dx + dy * dy > maxMiter * maxMiter + ) { + continue + } + // Only assign intersection to non-passthrough walls // Passthrough walls don't receive junction data (their geometry doesn't change) if (!wall1.isPassthrough) { diff --git a/packages/core/src/utils/heal-scene-graph.test.ts b/packages/core/src/utils/heal-scene-graph.test.ts new file mode 100644 index 00000000..6b64658e --- /dev/null +++ b/packages/core/src/utils/heal-scene-graph.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, test } from 'bun:test' +import { healSceneNodes } from './heal-scene-graph' + +describe('healSceneNodes', () => { + test('strips non-string (null) children entries', () => { + const { nodes, strippedChildRefs } = healSceneNodes({ + wall_a: { + id: 'wall_a', + type: 'wall', + start: [0, 0], + end: [1, 0], + children: [null, 'item_x'], + }, + item_x: { id: 'item_x', type: 'item' }, + }) + expect(strippedChildRefs).toBe(1) + expect((nodes.wall_a as { children: string[] }).children).toEqual(['item_x']) + }) + + test('drops childless zero-length walls and removes their parent reference', () => { + const { nodes, droppedWallIds } = healSceneNodes({ + level_0: { id: 'level_0', type: 'level', children: ['wall_zero', 'wall_real'] }, + wall_zero: { id: 'wall_zero', type: 'wall', start: [5, 5], end: [5, 5], children: [] }, + wall_real: { id: 'wall_real', type: 'wall', start: [0, 0], end: [3, 0], children: [] }, + }) + expect(droppedWallIds).toEqual(['wall_zero']) + expect('wall_zero' in nodes).toBe(false) + expect((nodes.level_0 as { children: string[] }).children).toEqual(['wall_real']) + }) + + test('keeps a zero-length wall that still hosts a door/window', () => { + const { nodes, droppedWallIds } = healSceneNodes({ + wall_z: { id: 'wall_z', type: 'wall', start: [1, 1], end: [1, 1], children: ['door_1'] }, + door_1: { id: 'door_1', type: 'door' }, + }) + expect(droppedWallIds).toEqual([]) + expect('wall_z' in nodes).toBe(true) + }) + + test('passes a clean scene through untouched', () => { + const input = { + wall_a: { id: 'wall_a', type: 'wall', start: [0, 0], end: [2, 0], children: ['door_1'] }, + door_1: { id: 'door_1', type: 'door' }, + } + const { nodes, droppedWallIds, strippedChildRefs } = healSceneNodes(input) + expect(droppedWallIds).toEqual([]) + expect(strippedChildRefs).toBe(0) + expect(nodes.wall_a).toBe(input.wall_a) + }) +}) diff --git a/packages/core/src/utils/heal-scene-graph.ts b/packages/core/src/utils/heal-scene-graph.ts new file mode 100644 index 00000000..9ada1400 --- /dev/null +++ b/packages/core/src/utils/heal-scene-graph.ts @@ -0,0 +1,83 @@ +// Repairs scene-graph corruption that pre-dates the source fixes, so existing +// saved scenes still load. Two known kinds of damage, both produced by the +// capture wall-merge before it was fixed: +// +// 1. A `children` array containing a non-string entry. The merge re-attached a +// wall-hosted item without minting an id, so `undefined` was pushed into the +// wall's children — which serializes to `[null]`. The wall schema rejects +// `null` children, so the whole scene fails to load. +// 2. A zero-length wall (start === end). It renders nothing, but lingers as a +// junk node and is a foot-gun for snapping/mitering. +// +// Both are also prevented at the source now (see merge-walls.ts and the wall +// miter limit); this is the load-time safety net for already-saved scenes. + +const ZERO_LENGTH_EPS = 1e-6 + +export interface HealSceneResult { + nodes: Record + /** Ids of zero-length walls that were dropped. */ + droppedWallIds: string[] + /** Count of non-string (e.g. null) entries removed from `children` arrays. */ + strippedChildRefs: number +} + +function isWallLike(node: unknown): node is { start: [number, number]; end: [number, number] } { + if (!node || typeof node !== 'object') return false + const n = node as Record + return ( + n.type === 'wall' && + Array.isArray(n.start) && + Array.isArray(n.end) && + typeof n.start[0] === 'number' && + typeof n.start[1] === 'number' && + typeof n.end[0] === 'number' && + typeof n.end[1] === 'number' + ) +} + +/** + * Returns a healed copy of a `nodes` map. Pure — does not mutate `input`. + * Nodes that need no repair are passed through by reference. + */ +export function healSceneNodes(input: Record): HealSceneResult { + const droppedWallIds: string[] = [] + + // Pass 1: drop childless zero-length walls. (Only childless ones — a wall + // carrying a door/window must keep its hosts, degenerate or not.) + const kept: Record = {} + for (const [id, node] of Object.entries(input)) { + if (isWallLike(node)) { + const children = (node as { children?: unknown }).children + const childless = !Array.isArray(children) || children.length === 0 + const dx = node.end[0] - node.start[0] + const dz = node.end[1] - node.start[1] + if (childless && Math.hypot(dx, dz) <= ZERO_LENGTH_EPS) { + droppedWallIds.push(id) + continue + } + } + kept[id] = node + } + + const dropped = new Set(droppedWallIds) + let strippedChildRefs = 0 + + // Pass 2: clean `children` arrays — drop non-string entries (the `[null]` bug) + // and references to walls we just removed. + const nodes: Record = {} + for (const [id, node] of Object.entries(kept)) { + const children = (node as { children?: unknown })?.children + if (Array.isArray(children)) { + const cleaned = children.filter((c): c is string => typeof c === 'string' && !dropped.has(c)) + if (cleaned.length !== children.length) { + strippedChildRefs += children.length - cleaned.length + nodes[id] = { ...(node as Record), children: cleaned } + continue + } + } + nodes[id] = node + } + + return { nodes, droppedWallIds, strippedChildRefs } +} diff --git a/packages/core/src/validation/validate-build-json.ts b/packages/core/src/validation/validate-build-json.ts index 23873c3e..128bfc34 100644 --- a/packages/core/src/validation/validate-build-json.ts +++ b/packages/core/src/validation/validate-build-json.ts @@ -1,4 +1,5 @@ import { AnyNode, type AnyNodeType } from '../schema/types' +import { healSceneNodes } from '../utils/heal-scene-graph' export type ValidationSeverity = 'error' | 'warning' @@ -127,9 +128,22 @@ export function validateBuildJson(input: unknown): ValidateBuildJsonResult { } } - const nodes = nodesRaw as Record + // Heal known pre-existing corruption (null children, zero-length walls) up + // front, so a scene saved before the source fixes still imports instead of + // hard-failing schema validation. `parsed` below carries the repaired nodes. + const { nodes, droppedWallIds, strippedChildRefs } = healSceneNodes( + nodesRaw as Record, + ) const rootNodeIds = rootNodeIdsRaw as string[] + if (strippedChildRefs > 0 || droppedWallIds.length > 0) { + warnings.push({ + severity: 'warning', + code: 'auto_repaired', + message: `Repaired on import: removed ${strippedChildRefs} invalid child reference${strippedChildRefs === 1 ? '' : 's'} and ${droppedWallIds.length} zero-length wall${droppedWallIds.length === 1 ? '' : 's'}.`, + }) + } + if (rootNodeIds.length === 0) { errors.push({ severity: 'error', diff --git a/packages/nodes/src/ceiling/move-tool.tsx b/packages/nodes/src/ceiling/move-tool.tsx index 8ac04216..d6aaab1d 100644 --- a/packages/nodes/src/ceiling/move-tool.tsx +++ b/packages/nodes/src/ceiling/move-tool.tsx @@ -9,6 +9,7 @@ import { polygonAnchors, resolveAlignment, sceneRegistry, + snapScalar, useLiveTransforms, useScene, } from '@pascal-app/core' @@ -36,10 +37,10 @@ import { BufferGeometry, DoubleSide, Path, Shape, ShapeGeometry, Vector3 } from * mesh's X/Z position on rebuild (`mesh.position.x = 0`, * `mesh.position.z = 0`) so the visual transitions smoothly. * - * 0.5m grid snap (matches legacy). + * Snaps to the editor's configured grid step (Shift bypasses). */ function snap(value: number) { - return Math.round(value * 2) / 2 + return snapScalar(value, useEditor.getState().gridSnapStep) } /** Figma-style alignment-snap threshold (meters), matching the other tools. */ diff --git a/packages/nodes/src/ceiling/tool.tsx b/packages/nodes/src/ceiling/tool.tsx index f20bb607..b2369b75 100644 --- a/packages/nodes/src/ceiling/tool.tsx +++ b/packages/nodes/src/ceiling/tool.tsx @@ -6,6 +6,7 @@ import { type GridEvent, type LevelNode, snapPointAlongAngleRay, + snapPointToGrid, useScene, } from '@pascal-app/core' import { @@ -93,9 +94,9 @@ export const CeilingTool: React.FC = () => { if (!(cursorRef.current && gridCursorRef.current)) return const rawPoint: [number, number] = [event.localPosition[0], event.localPosition[2]] const bypassSnap = shiftPressed.current || event.nativeEvent?.shiftKey === true - const gridX = Math.round(rawPoint[0] * 2) / 2 - const gridZ = Math.round(rawPoint[1] * 2) / 2 - const gridPosition: [number, number] = bypassSnap ? rawPoint : [gridX, gridZ] + const gridPosition: [number, number] = bypassSnap + ? rawPoint + : [...snapPointToGrid(rawPoint, useEditor.getState().gridSnapStep)] setCursorPosition(gridPosition) setLevelY(event.localPosition[1]) const ceilingY = event.localPosition[1] + CEILING_HEIGHT diff --git a/packages/nodes/src/slab/tool.tsx b/packages/nodes/src/slab/tool.tsx index 57c95298..bc4252bc 100644 --- a/packages/nodes/src/slab/tool.tsx +++ b/packages/nodes/src/slab/tool.tsx @@ -6,6 +6,7 @@ import { type GridEvent, type LevelNode, snapPointAlongAngleRay, + snapPointToGrid, useScene, } from '@pascal-app/core' import { @@ -76,9 +77,9 @@ export const SlabTool: React.FC = () => { if (!cursorRef.current) return const rawPoint: [number, number] = [event.localPosition[0], event.localPosition[2]] const bypassSnap = shiftPressed.current || event.nativeEvent?.shiftKey === true - const gridX = Math.round(rawPoint[0] * 2) / 2 - const gridZ = Math.round(rawPoint[1] * 2) / 2 - const gridPosition: [number, number] = bypassSnap ? rawPoint : [gridX, gridZ] + const gridPosition: [number, number] = bypassSnap + ? rawPoint + : [...snapPointToGrid(rawPoint, useEditor.getState().gridSnapStep)] setCursorPosition(gridPosition) setLevelY(event.localPosition[1]) const lastPoint = points[points.length - 1] diff --git a/packages/nodes/src/spawn/tool.tsx b/packages/nodes/src/spawn/tool.tsx index 69f4475b..54e6ce75 100644 --- a/packages/nodes/src/spawn/tool.tsx +++ b/packages/nodes/src/spawn/tool.tsx @@ -1,6 +1,13 @@ 'use client' -import { emitter, type GridEvent, SpawnNode, sceneRegistry, useScene } from '@pascal-app/core' +import { + emitter, + type GridEvent, + SpawnNode, + sceneRegistry, + snapScalar, + useScene, +} from '@pascal-app/core' import { CursorSphere, getFloorStackPreviewPosition, @@ -11,7 +18,7 @@ import { useViewer } from '@pascal-app/viewer' import { useEffect, useRef } from 'react' import { type Group, Vector3 } from 'three' -const roundToHalf = (value: number) => Math.round(value * 2) / 2 +const snapToGrid = (value: number) => snapScalar(value, useEditor.getState().gridSnapStep) const worldVector = new Vector3() function getExistingSpawnIds() { @@ -31,14 +38,14 @@ function getLevelLocalPosition( if (!levelObject) { return bypassSnap ? [event.localPosition[0], 0, event.localPosition[2]] - : [roundToHalf(event.localPosition[0]), 0, roundToHalf(event.localPosition[2])] + : [snapToGrid(event.localPosition[0]), 0, snapToGrid(event.localPosition[2])] } worldVector.set(event.position[0], event.position[1], event.position[2]) levelObject.updateWorldMatrix(true, false) levelObject.worldToLocal(worldVector) return bypassSnap ? [worldVector.x, 0, worldVector.z] - : [roundToHalf(worldVector.x), 0, roundToHalf(worldVector.z)] + : [snapToGrid(worldVector.x), 0, snapToGrid(worldVector.z)] } /** @@ -58,11 +65,11 @@ const SpawnTool = () => { const onGridMove = (event: GridEvent) => { // Cursor lives in the ToolManager's building-local group. Use - // event.localPosition directly (already building-local) with the - // same half-meter snap the legacy tool uses. + // event.localPosition directly (already building-local), snapped to the + // editor's configured grid step (Shift bypasses). const bypassSnap = event.nativeEvent?.shiftKey === true - const nextX = bypassSnap ? event.localPosition[0] : roundToHalf(event.localPosition[0]) - const nextZ = bypassSnap ? event.localPosition[2] : roundToHalf(event.localPosition[2]) + const nextX = bypassSnap ? event.localPosition[0] : snapToGrid(event.localPosition[0]) + const nextZ = bypassSnap ? event.localPosition[2] : snapToGrid(event.localPosition[2]) const position: [number, number, number] = [nextX, 0, nextZ] const previewNode = SpawnNode.parse({ name: 'Spawn Point',