arch: enforce layer boundaries — ceiling dispatch, store relocation, shared helper#382
Conversation
Items (e.g. solar panels) can now be placed on sloped roof surfaces. The placement system computes euler rotation from the roof surface normal so items sit flush on the slope instead of going inside. - Add roofStrategy to placement-strategies with enter/move/click/leave - Wire roof:enter/move/click/leave events in the placement coordinator - Add calculateRoofRotation in placement-math using surface normals - Support full 3D cursor rotation for sloped surfaces - Items on roofs are parented to the level with world-space rotation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Render a distinct, static plan symbol for each door type in the registry floor-plan builder (`packages/nodes/src/door/floorplan.ts`), independent of the door's live open/close animation: - single / hinged: fixed 90° swing with a dashed quarter-circle arc - double / french: two mirrored half-width leaves + dashed arcs - folding / bifold: static zigzag accordion (~80% span) on the wall face - sliding: bypass — two overlapping panels on parallel tracks + arrow - pocket: thin white leaf, ~60% closed, sliding into the solid wall - barn: surface-mounted panel parked over the wall, dashed closed-ghost + slide arrow The swing arc is dashed in screen-pixel units (the renderer uses non-scaling-stroke). Symbols are oriented by hingesSide / swingDirection / slideDirection as appropriate. Also includes pre-existing working-tree changes unrelated to the door symbols: group move/rotate transform and box-select tweaks, and a regenerated ifc-converter next-env.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Snap the wall draft / endpoint-move point onto existing wall geometry — corners, midpoints, wall–wall intersections, and along-wall edges — and show a beacon at the snap point whose glyph encodes what it caught (square = corner, triangle = midpoint, ✕ = intersection, circle = edge). - Pure snap geometry extracted to wall-snap-geometry.ts (unit-tested). - Ephemeral useWallSnapIndicator store drives a 3D pillar+glyph beacon and a 2D SVG glyph beacon, both indigo to match the alignment guides. - Gated by a new persisted "Magnetic snap" toggle in the Display menu (useEditor); honored by draw + commit + endpoint-move in both views. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend the per-door-type plan symbols in the registry floor-plan builder (packages/nodes/src/door/floorplan.ts): - open doorway (openingKind === 'opening'): bare gap, no leaf/arc/panel (mirrors the 3D system, which renders only the cutout for openings) - garage sectional: closed leaf + side tracks into the garage + dashed parked ghost at the inner end - garage roll-up: closed leaf + coil barrel (capsule) with a coil hint - garage tilt-up: closed leaf + dashed parked panel + dashed curved up-and-over swing path - gate the swing arc to actual swing doors (hinged/double/french) so other types fall back to the plain footprint Garage mechanisms sit on the interior (door-local -z) side to match the 3D garage builders, independent of swingDirection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… shared helper Three architectural fixes to bring the branch into full compliance: 1. **ceiling-system kind check → CeilingCutCapability** Replace `child.type === 'item'` branch in `ceiling-system` with registry dispatch. Add `CeilingCutCapability` type to `packages/core` registry types, implement `buildCeilingHole` on `itemDefinition`, and rewrite `collectRecessedItemHoles` → `collectCeilingHoles` to dispatch through `nodeRegistry` — viewer never again inspects a node's kind directly. 2. **useAlignmentGuides + useWallSnapIndicator → packages/editor** These stores are editor-only UI (snap beacons, alignment guides). Move them from `packages/core/src/store/` to `packages/editor/src/store/`, re-export from `packages/editor`, and update all 34 consumer files across `packages/editor` and `packages/nodes` to import from `@pascal-app/editor`. 3. **findLevelAncestorId extracted to core** `item-light-system` had a private `resolveNodeLevelId` that duplicated level-ancestor traversal logic. Extract it as `findLevelAncestorId` in `packages/core` (spatial-grid-sync), export it, and replace the local copy. All four packages typecheck cleanly (zero errors). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- floorplan-alignment-guide-layer: keep useAlignmentGuides from editor, add useViewer from main - use-placement-coordinator: keep spatialGridManager (our add), drop duplicate useAlignmentGuides from core block - roof-tool: keep snapScalar added by main, useAlignmentGuides stays in editor import - stair-tool: keep syncAutoStairOpenings added by main, useAlignmentGuides stays in editor import - box-select-tool: remove stale collectNodeIdsInBounds (extracted to select-candidates.ts by main), keep new module-level Box3/Vector3 scratch objects - column/tool + shelf/tool: merge getFloorStackPreviewPosition (main) with useAlignmentGuides (ours) in editor import Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Aymericr
left a comment
There was a problem hiding this comment.
Review summary
Reviewed the full diff (76 files, +1996/-331), built packages/core, ran tsc --noEmit for all four packages, and ran the new wall-snap-geometry tests. The architectural changes called out in the PR description are good and correct, but the PR is much larger than its description suggests, and CI's quality job is failing.
✅ What's solid
CeilingCutCapability(registry dispatch). Clean replacement for thechild.type === 'item'branch inceiling-system. Capability dispatch throughnodeRegistry,buildCeilingHolelives onitemDefinition, and the viewer never branches on kind. Therecessedflag onassetSchemais the right place for it.useAlignmentGuides+useWallSnapIndicator→packages/editor. Both stores are pure editor-UI concerns; moving them out ofpackages/coreis the right layer fix. Re-exports + 34 consumer updates look clean.findLevelAncestorIdextracted to core. Replaces the localresolveNodeLevelIdinitem-light-system. The 16-iteration parent-chain guard is a nice touch — protects against corrupt graphs.- WebGPU empty-buffer hardening. The encoder-poison failure mode is real (one bad draw discards every other draw in the frame, not just itself). The defense-in-depth is well done:
createPlaceholderGeometry(3 zero-vertices) avoids the count-0 attribute case.hasDrawableGeometrypredicate is the renderer-level safety net.installEmptyDrawGuardwrapssetRenderObjectFunctionand warns once per geometry in dev.MergedOutlineNodecarries the same guard inside its three custom passes (necessary, since they replacesetRenderObjectFunction).- Door-system
hideEmptyGeometryMeshesand the ceiling degenerate fallback are belt-and-suspenders for the same class of bug.
- Recessed seating math.
placement-strategies.tscorrectly switches Y from-itemHeight(hanging) to0(flush with ceiling plane) whenasset.recessedis true, and the inset-rectangle hole math inbuildCeilingHolematches the old hard-coded constant (0.82) so behaviour is preserved. item-light-systemlight.visibletoggling. Settinglight.visible = falsefor idle / fading-out / zero-intensity lights is a proper fix — point lights still cost shadow-map work even at intensity 0. Filtering out non-finite scores before slicing is defensive.- Wall-snap geometry. Pure module + 7 unit tests, all green locally. Priority order (corner > intersection > midpoint > along-wall) and configurable radii read sensibly. Wall-tool clears the indicator on commit/cancel/unmount in every code path.
- Per-door-type floor-plan symbols. Independent of the live open/close animation, oriented by hinge/swing/slide direction, garage mechanisms drawn on the interior side to match the 3D builders. Looks self-contained.
After running npm run build in packages/core (as the PR description requires), all four packages typecheck cleanly. Confirmed.
⚠️ What needs to change before merge
1. CI quality job is failing — 8 lint errors that aren't on main.
Locally bun check reports Found 8 errors on this branch vs 0 errors on origin/main. All trivially auto-fixable. The errors I can identify:
formatviolations (Biome formatter would re-print):packages/nodes/src/{ceiling/move-tool,column/tool,door/floorplan,fence/move-endpoint-tool,wall/tool}.tsx,packages/nodes/src/shared/wall-opening-alignment.tsassist/source/organizeImports:packages/editor/src/store/use-alignment-guides.ts(zustand vs@pascal-app/coreorder),packages/editor/src/index.tsxlint/correctness/noUnusedImports:packages/editor/src/components/tools/item/use-placement-coordinator.tsx:17
Run bun check --write (or bunx biome check --write .) and commit. Should be a one-line fix.
2. PR description doesn't match the actual diff.
The description describes only the architectural cleanup commit (34a24bb6). But the branch also lands:
- Roof surface placement support for items (commit
3731eb32, May 18) - Per-door-type floor-plan symbols + garage/open-doorway variants (
06749a74,4d1f8dc0) feat(editor): magnetic wall-snap with per-kind beacon (2D + 3D) (acd01ae0)Fix recessed ceiling fixtures and draw safety(ed768125) — the WebGPU hardeninglight.visibletoggling initem-light-system- New
useCeilingEventshook (175 lines) replacing the ceiling-grid mesh raycast path
These are substantive features and should be either (a) split into separate PRs or (b) at minimum listed and explained in the PR body so reviewers know what they're approving. Right now the title says "arch: enforce layer boundaries" but the diff is closer to "branch sync + 5 features + arch cleanup".
3. .claude/launch.json is committed.
This is an IDE/launch config (bun run dev on port 3002). It should be .gitignored like .vscode/launch.json, not committed to the public repo.
Nits (non-blocking)
- The
INSET = 0.82constant inbuildCeilingHolecarries a comment "Same constant the old ceiling-system used". Worth lifting to a named export onitemDefinitionor aCEILING_FIXTURE_TRIM_OVERLAPconstant in core, so the single source of truth is obvious if the trim geometry ever changes. useCeilingEventsreimplementspointInPolygonprivately whenpointInPolygonis already exported from@pascal-app/core(spatial-grid-manager). Minor duplication.- The merge history (~12 "Merge branch 'main' of github.com:pascalorg/editor" commits) makes the branch hard to bisect. Future PRs would benefit from rebasing onto main rather than repeated merges. Not blocking.
Verification I ran
✅ npx tsc --noEmit on packages/{core,viewer,nodes,editor} — clean (after building core dist)
✅ bun test packages/editor/.../wall-snap-geometry.test.ts — 7/7 pass
❌ bun check — 8 errors (vs 0 on main)
I have not run bun dev or test the visual behaviour in-browser (recessed downlight cuts, magnetic snap beacons, sloped-roof item placement, garage door symbols). The architectural and type-level changes are sound; the visual / integration parts I'm trusting the manual test pass.
Verdict
Architecturally a 👍. The three layer-boundary fixes are clean and the WebGPU hardening is good defensive work. But the PR is bigger than advertised, CI is red on lint, and .claude/launch.json shouldn't ship. Once those are fixed (and ideally the PR body is updated to describe everything that's actually in here), this is good to merge.
Run bun check --write to clear 8 Biome errors (formatting + import order + one unused import). Untrack .claude/launch.json and add it plus .claude/settings.local.json to .gitignore so local IDE/agent configs stop landing in commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring in 10 commits from upstream — relative-move grab-offset fix, walkthrough collisions, split-view sync, action-bar dead controls, placement metadata strip on commit, and friends. Resolved 5 import-block conflicts where our arch-fix move of useAlignmentGuides (core → editor) overlapped with upstream's unrelated import changes: - column/tool.tsx, shelf/tool.tsx — drop now-unused core symbols (movingFootprintAnchors, resolveAlignment, snapPointToGrid, sceneRegistry), keep useAlignmentGuides from editor, add useEditor from editor. - group-transform-shared.ts — keep upstream's nodeRegistry + resolveBuildingForLevel imports, keep our Matrix4 (still used by levelFrame). - floorplan-registry-move-overlay.tsx — drop both useAlignmentGuides and snapPointToGrid from core (move from editor already in place on the next line). - polygon-centroid-move.ts — combine upstream's getSegmentGridStep with our useAlignmentGuides from editor. All four packages typecheck clean, bun check has 0 errors, wall-snap-geometry tests 7/7 pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…shared helper (pascalorg#382) * Add roof surface placement support for items Items (e.g. solar panels) can now be placed on sloped roof surfaces. The placement system computes euler rotation from the roof surface normal so items sit flush on the slope instead of going inside. - Add roofStrategy to placement-strategies with enter/move/click/leave - Wire roof:enter/move/click/leave events in the placement coordinator - Add calculateRoofRotation in placement-math using surface normals - Support full 3D cursor rotation for sloped surfaces - Items on roofs are parented to the level with world-space rotation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fixed conflict * editor: per-door-type floor-plan symbols Render a distinct, static plan symbol for each door type in the registry floor-plan builder (`packages/nodes/src/door/floorplan.ts`), independent of the door's live open/close animation: - single / hinged: fixed 90° swing with a dashed quarter-circle arc - double / french: two mirrored half-width leaves + dashed arcs - folding / bifold: static zigzag accordion (~80% span) on the wall face - sliding: bypass — two overlapping panels on parallel tracks + arrow - pocket: thin white leaf, ~60% closed, sliding into the solid wall - barn: surface-mounted panel parked over the wall, dashed closed-ghost + slide arrow The swing arc is dashed in screen-pixel units (the renderer uses non-scaling-stroke). Symbols are oriented by hingesSide / swingDirection / slideDirection as appropriate. Also includes pre-existing working-tree changes unrelated to the door symbols: group move/rotate transform and box-select tweaks, and a regenerated ifc-converter next-env.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix recessed ceiling fixtures and draw safety * feat(editor): magnetic wall-snap with per-kind beacon (2D + 3D) Snap the wall draft / endpoint-move point onto existing wall geometry — corners, midpoints, wall–wall intersections, and along-wall edges — and show a beacon at the snap point whose glyph encodes what it caught (square = corner, triangle = midpoint, ✕ = intersection, circle = edge). - Pure snap geometry extracted to wall-snap-geometry.ts (unit-tested). - Ephemeral useWallSnapIndicator store drives a 3D pillar+glyph beacon and a 2D SVG glyph beacon, both indigo to match the alignment guides. - Gated by a new persisted "Magnetic snap" toggle in the Display menu (useEditor); honored by draw + commit + endpoint-move in both views. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * editor: garage and open-doorway floor-plan symbols Extend the per-door-type plan symbols in the registry floor-plan builder (packages/nodes/src/door/floorplan.ts): - open doorway (openingKind === 'opening'): bare gap, no leaf/arc/panel (mirrors the 3D system, which renders only the cutout for openings) - garage sectional: closed leaf + side tracks into the garage + dashed parked ghost at the inner end - garage roll-up: closed leaf + coil barrel (capsule) with a coil hint - garage tilt-up: closed leaf + dashed parked panel + dashed curved up-and-over swing path - gate the swing arc to actual swing doors (hinged/double/french) so other types fall back to the plain footprint Garage mechanisms sit on the interior (door-local -z) side to match the 3D garage builders, independent of swingDirection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * arch: enforce layer boundaries — registry dispatch, store relocation, shared helper Three architectural fixes to bring the branch into full compliance: 1. **ceiling-system kind check → CeilingCutCapability** Replace `child.type === 'item'` branch in `ceiling-system` with registry dispatch. Add `CeilingCutCapability` type to `packages/core` registry types, implement `buildCeilingHole` on `itemDefinition`, and rewrite `collectRecessedItemHoles` → `collectCeilingHoles` to dispatch through `nodeRegistry` — viewer never again inspects a node's kind directly. 2. **useAlignmentGuides + useWallSnapIndicator → packages/editor** These stores are editor-only UI (snap beacons, alignment guides). Move them from `packages/core/src/store/` to `packages/editor/src/store/`, re-export from `packages/editor`, and update all 34 consumer files across `packages/editor` and `packages/nodes` to import from `@pascal-app/editor`. 3. **findLevelAncestorId extracted to core** `item-light-system` had a private `resolveNodeLevelId` that duplicated level-ancestor traversal logic. Extract it as `findLevelAncestorId` in `packages/core` (spatial-grid-sync), export it, and replace the local copy. All four packages typecheck cleanly (zero errors). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: fix lint, untrack .claude/launch.json Run bun check --write to clear 8 Biome errors (formatting + import order + one unused import). Untrack .claude/launch.json and add it plus .claude/settings.local.json to .gitignore so local IDE/agent configs stop landing in commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
What does this PR do?
Architectural cleanup (
34a24bb6)Three fixes to bring the branch into full compliance with the layer-boundary rules:
1. ceiling-system kind check → registry dispatch.
ceiling-systemwas branching onchild.type === 'item'to decide whether a ceiling child cuts a hole — a hard-coded kind check in the viewer layer. IntroducesCeilingCutCapabilityinpackages/coreregistry types, implementsbuildCeilingHoleonitemDefinition(recessed items only), and rewritescollectRecessedItemHoles→collectCeilingHolesto dispatch throughnodeRegistry. The viewer now has zero kind-specific branches for ceiling holes.2.
useAlignmentGuides+useWallSnapIndicator→packages/editor. Both stores are editor-only UI (snap beacons, alignment guides drawn over the canvas). They were sitting inpackages/core, which must not know about editor-UI concepts. Moved topackages/editor/src/store/, re-exported frompackages/editor, and updated 34 consumer files acrosspackages/editorandpackages/nodesto import from@pascal-app/editor.3.
findLevelAncestorIdextracted to core.item-light-systemhad a privateresolveNodeLevelIdfunction that walked the parent chain to find a level ancestor — duplicating traversal logic. Extracted asfindLevelAncestorId(nodeId, nodes): string | nullinpackages/coreand the viewer's local copy removed.Recessed ceiling fixtures + WebGPU draw safety (
ed768125)recessedflag toassetSchema. Recessed items now sit flush with the ceiling plane (y = 0) instead of hanging below (y = -itemHeight).buildCeilingHole) uses an inset rectangle matching the existing trim overlap (0.82) so hole geometry matches the previous hard-coded behaviour exactly.createPlaceholderGeometryreturns a 3-vertex sentinel so attributes are never zero-count.hasDrawableGeometrypredicate gates renderer-level draws.installEmptyDrawGuardwrapssetRenderObjectFunctionand warns once per geometry in dev.MergedOutlineNodecarries the same guard inside its three custom passes (since they replacesetRenderObjectFunction).hideEmptyGeometryMeshesand the ceiling degenerate-fallback are belt-and-suspenders for the same class of bug.item-light-systemnow toggleslight.visible = falsefor idle / fading-out / zero-intensity lights — point lights still cost shadow-map work at intensity 0.Magnetic wall-snap (
acd01ae0)wall-snap-geometrypure module + 7 unit tests covering corner, intersection, midpoint, and along-wall snap candidates.Per-door-type floor-plan symbols (
06749a74,4d1f8dc0)Other changes
useCeilingEventshook (175 lines) replacing the ceiling-grid mesh raycast path.All four packages (
core,viewer,nodes,editor) typecheck with zero errors.How to test
npm run buildinpackages/coreto rebuild dist (nodes reads core from dist).bun dev— open a scene with recessed downlights parented to a ceiling. Confirm holes render flush with the ceiling and the fixture sits aty = 0.npx tsc -p packages/core/tsconfig.json --noEmit && npx tsc -p packages/viewer/tsconfig.json --noEmit && npx tsc -p packages/nodes/tsconfig.json --noEmit && npx tsc -p packages/editor/tsconfig.json --noEmit— should print no errors.bun check— should print zero errors.Screenshots / screen recording
To add for the door floor-plan symbols and snap beacons.
Checklist
bun devbun checkto verify)mainbranch