fix(walls): bound wall miters, heal corrupt scenes, follow grid snap
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
cf24b62c44
commit
6d5f041b48
@@ -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<string, any>) {
|
||||
}
|
||||
|
||||
function migrateNodes(nodes: Record<string, any>): Record<string, AnyNode> {
|
||||
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<string, any>
|
||||
for (const [id, node] of Object.entries(patchedNodes)) {
|
||||
// 1. Item scale migration
|
||||
if (node.type === 'item' && !('scale' in node)) {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -35,6 +35,17 @@ type JunctionData = Map<string, WallIntersections>
|
||||
|
||||
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) {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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<string, unknown>
|
||||
/** 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<string, unknown>
|
||||
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<string, unknown>): 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<string, unknown> = {}
|
||||
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<string, unknown> = {}
|
||||
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<string, unknown>), children: cleaned }
|
||||
continue
|
||||
}
|
||||
}
|
||||
nodes[id] = node
|
||||
}
|
||||
|
||||
return { nodes, droppedWallIds, strippedChildRefs }
|
||||
}
|
||||
@@ -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<string, unknown>
|
||||
// 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<string, unknown>,
|
||||
)
|
||||
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',
|
||||
|
||||
Reference in New Issue
Block a user