Skip to content

feat(ifc): preserve unsupported IFC geometry - #553

Open
yorhodes wants to merge 11 commits into
pascalorg:mainfrom
yorhodes:codex/ifc-type-support
Open

feat(ifc): preserve unsupported IFC geometry#553
yorhodes wants to merge 11 commits into
pascalorg:mainfrom
yorhodes:codex/ifc-type-support

Conversation

@yorhodes

@yorhodes yorhodes commented Jul 27, 2026

Copy link
Copy Markdown

Summary

  • add a format-neutral imported-mesh built-in node with registry-driven 3D and floorplan geometry
  • preserve unsupported or non-parametric IFC elements as serialized triangle geometry with source display colors
  • import IFC spaces as room zones and keep storey-relative elevations consistent across native and fallback nodes
  • map door families from IfcDoor.OperationType and glazing from Pset_DoorCommon.GlazingAreaFraction
  • allow browser and Node callers to configure the WebIFC WASM path
  • fix metre-normalized mesh coordinates, duplicate stair flights, malformed-space isolation, and roof-slab fallback
  • round serialized positions to 0.1 mm and normals to 0.001, and keep baked imported meshes immovable

Architecture

  • source-format parsing remains isolated in @pascal-app/ifc-converter
  • the reusable mesh schema stays in core, while Three.js and floorplan builders live in packages/nodes/src/imported-mesh
  • imported meshes are import-only and hidden from the empty-object palette
  • no viewer/editor kind-specific dispatch or project-name/material-name heuristics are introduced
  • README wording now distinguishes native parameter recovery from stair/roof hierarchy preservation

Review fixes

  • WebIFC GetFlatMesh coordinates remain in metres; only the STEP-derived origin offset is unit-scaled
  • stair-flight descendants are claimed by their native stair so they do not render twice
  • each malformed IfcSpace is isolated and missing Name values are guarded
  • roof and landing slabs skip native conversion only after a non-empty mesh has been extracted and cached
  • cleanup behavior was removed from this PR and split into fix(ifc): avoid merging adjacent wall assemblies #603 with all four fixture counts
  • imported meshes no longer advertise movable capability
  • triangle payloads are rounded before serialization

Verification

  • builds: @pascal-app/core, @pascal-app/ifc-converter, and @pascal-app/nodes
  • Biome check on all changed IFC/imported-mesh paths
  • bun test packages/ifc-converter/tests — 11 passed
  • bun test packages/core/src — passed
  • bun test packages/nodes/src — 946 passed, 1 skipped
  • 04-ifc-open-house regression: imported mesh bounds align with native wall bounds within 1 m and both roof slabs are preserved
  • duplex regression with one IfcSpace Name replaced by null: all 21 spaces import and no IFCSTAIRFLIGHT fallback remains

Note

Medium Risk
Large changes to IFC import (coordinates, parenting, and fallback vs native paths) can misplace geometry or affect slab/wall support; schema and event bus additions are additive and well-tested on fixtures.

Overview
Adds a first-class imported-mesh node in core (schema, union, level children, editor events) and registers it in packages/nodes with Three.js geometry, floorplan bounds, selectable/deletable-but-not-movable behavior, and palette hidden.

The IFC converter is reworked so elements without a reliable native mapping become serialized triangle buffers (with colors, rounded positions/normals, and a dedicated WebIFC axis mapping that avoids double swapYZ). IfcSpace becomes room zones; stair flights drop parametric stair nodes in favor of meshes; roof/landing slabs with mesh skip horizontal SlabNode conversion. Parenting uses storey inference by elevation when containment is missing, level heights from IFC storey spacing, and shared attachNodeToGraph. Native conversion fixes include wall centerlines from profile, level-local elevations, and doors/windows only when wall-hosted (with OperationType / GlazingAreaFraction semantics). wasmPath is configurable for Node tests.

New unit/integration tests cover door/storey helpers and fixture regressions (mesh–wall alignment, roof slabs, stair flights, malformed spaces).

Reviewed by Cursor Bugbot for commit 972db34. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread packages/ifc-converter/src/index.ts
Comment thread packages/ifc-converter/src/index.ts
Comment thread packages/nodes/src/imported-mesh/definition.ts
Comment thread packages/ifc-converter/src/cleanup.ts Outdated
Comment thread packages/ifc-converter/src/index.ts

@Aymericr Aymericr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — it's tackling a real gap, and the layering is the part I most want to keep. A format-neutral imported-mesh node in core (packages/core/src/schema/nodes/imported-mesh.ts) with the Three.js and floorplan builders in packages/nodes/src/imported-mesh/ is the right shape: no kind-specific dispatch leaked into the viewer or editor, no project-name or material-name heuristics, and presentation.hidden: true genuinely keeps it out of the palette (apps/editor/components/build-tab.tsx:90 is the only consumer of that flag). Replacing the skippedBeams / skippedItems console.warn with actual preserved geometry is a clear improvement, and it gives #158 (AutoCAD) and #174 (Sweet Home 3D) a primitive to reuse later. The Pset_DoorCommon / OperationType mapping in door-semantics.ts is also the right call — standardized IFC properties instead of name sniffing — and every value it emits validates against DoorNode. Your Verification section is honest: I reproduced all of it (nodes 877 pass/1 skip, ifc-converter 9 pass, core 760 pass, duplex 245 nodes / 100 fallbacks exactly). Merge onto main is clean and the merged tree typechecks and passes biome, so the stale merge base costs nothing.

The blocker is a unit bug. extractImportedMeshPrimitives multiplies GetFlatMesh vertices by unitFactor (packages/ifc-converter/src/index.ts:684), but web-ifc already normalizes to metres — I checked by reading raw GetFlatMesh output directly, and on 04-ifc-open-house.ifc (which declares MILLIMETRE, unitFactor 0.001) the raw wall bbox is already -5.05..5.05 m. So on that file native walls land correctly at ±5 m while the imported-mesh bbox extent comes out 0.0107 x 0.0078 x 0.0058 m instead of ~10 x 3 x 8 m. Same on 10-sample-house.ifc, where all 4 zones also get ceilingHeight clamped to the 0.1 floor. Two of the four bundled fixtures are millimetre files, and I think they just weren't in the loop — everything you validated (duplex, and paris) happens to be unitFactor 1.

The fix is small: scale only originOffset (raw STEP data), not the flat-mesh coords.

if (swapYZ) {
  positions.push(
    world[0]! - originOffset[0]! * unitFactor,
    world[1]! - originOffset[2]! * unitFactor - levelElevation,
    -(world[2]! + originOffset[1]! * unitFactor),
  )
}

I applied exactly that and confirmed 04-ifc-open-house becomes 10.65 x 7.78 x 5.80 m and sample-house coverings align with the native walls, while duplex and the georeferenced paris file are byte-identical to before. Could you add a regression test asserting the imported-mesh bbox lands within a metre of the native wall bbox on a millimetre fixture? That's the guard that would have caught this.

Three more I'd want fixed before merge:

  1. Stair flights double-render. IFCSTAIRFLIGHT is in fallbackTypes (index.ts:2245) but the IFCSTAIR pass never registers child flight express IDs, so duplex emits 2 native stairs plus 2 flight meshes over the same volume. After expressIdToNodeId.set(stairExpressID, nodeId), claim the descendants from childrenMap so the fallback skips them. (Bugbot flagged this one and it's real.)

  2. space.Name null-deref silently deletes all zones. index.ts:2229 reads space.Name.value after guarding only space.Name?.value. web-ifc yields literal null for unset optionals — I verified this on duplex (Description, ObjectType, ElevationWithFlooring are all null). Because the try/catch at index.ts:2165 wraps the whole loop, one IfcSpace with LongName but no Name wipes every remaining zone in the file. Guard the read, and move the try/catch inside the loop so a malformed space skips only itself.

  3. ROOF slabs are lost, not rerouted. On 04-ifc-open-house, main emits 2 slabs ("South roof" / "North roof", both PredefinedType=ROOF); this branch emits 0 and no compensating imported-mesh appears. The skip at index.ts:1750 assumes the mesh fallback catches them and it doesn't here. Extract primitives first and only skip the native path when primitives.length > 0.

On scope: the diff has grown well past its title. Wall centerline recomputation, the 5x wall-merge tolerance cut, the material merge guard, level height/index assignment, storey inference, level-relative elevations, skylight/landing/roof skipping, unhosted-opening removal, and IfcSpace → zone are each independently risky. Two are measurably behavior-changing: the tolerance cut takes 05-paris-ground-floor from mergedWallGroups 15 / removedMergedWalls 28 to 8 / 13, leaving 100 walls where main leaves 85; and the material guard that ships with it is a no-op whenever only one fragment has a material association, since wallMaterialCompatible returns true if either side is missing (cleanup.ts:212 — also Bugbot). Could you split the cleanup.ts changes into their own PR with before/after wall counts for all four fixtures? That would let the mesh-preservation work land on its own merits.

Two smaller things worth folding in while you're here:

  • capabilities.movable will double-transform. The converter bakes level-local world coordinates into primitives and leaves position at [0,0,0], but move-registry-node-tool commits an absolute plan position and ParametricNodeRenderer applies node.position on the outer group (parametric-node-renderer.tsx:86). Simplest fix consistent with the "import-only" framing: drop movable for now and keep selectable + deletable. (Reasoned from the two code paths, not observed in the UI — worth a manual check.)
  • Payload. Positions and normals serialize as unrounded doubles, so imported-mesh is 96% of duplex's 4.48 MB scene and 91% of paris's 6.66 MB. Rounding positions to 1e-4 m (0.1 mm) and normals to 1e-3 cuts duplex to 1.97 MB with no visible change. Worth doing before this reaches hosted scene_graph rows.

One design note rather than a defect: the mesh-hull zone fallback (meshFootprint, index.ts:748) is a convex hull, so a concave or L-shaped space with no swept profile gets an inflated polygon that still counts as a successful import. I'd rather skip the zone and let the mesh preserve exact geometry, or set metadata.footprintApproximated — a wrong room boundary looks authoritative in a way a missing one doesn't.

Fix the unit scaling, the stair duplication, the Name deref, and the ROOF slabs, split out cleanup.ts, and I think this is close.

@yorhodes

yorhodes commented Aug 5, 2026

Copy link
Copy Markdown
Author

Addressed the requested review items in f5635ae:

  • fixed the blocker by leaving GetFlatMesh coordinates in metres and scaling only the STEP origin offset
  • added a millimetre-fixture regression that compares imported-mesh and native wall bounds within 1 m
  • claimed stair-flight descendants so duplex no longer emits IFCSTAIRFLIGHT meshes over native stairs
  • guarded missing IfcSpace Name values and isolated failures per space; a mutated duplex fixture still imports all 21 spaces
  • cached roof/landing slab primitives and only skips native slab conversion when the mesh is non-empty; open-house preserves both roof slabs
  • removed movable capability from imported meshes
  • rounded positions to 1e-4 m and normals to 1e-3
  • corrected the README so stair/roof hierarchy nodes are not described as proven full parametric conversion
  • removed all cleanup.ts behavior changes from this PR

The cleanup changes are now isolated in #603, including main-vs-branch wall counts for all four bundled fixtures. I also made material compatibility explicit when only one fragment has a material association.

Verification is updated in the PR description: converter tests 11 passed, nodes 946 passed / 1 skipped, core passed, all three affected packages build, and focused Biome checks pass.

Comment thread packages/ifc-converter/src/index.ts Outdated
Comment thread packages/ifc-converter/src/index.ts Outdated
Comment thread packages/ifc-converter/src/index.ts
@yorhodes
yorhodes requested a review from Aymericr August 5, 2026 19:55
Comment thread packages/ifc-converter/src/index.ts
@yorhodes

yorhodes commented Aug 5, 2026

Copy link
Copy Markdown
Author

One scope clarification on the storey-related changes that remain in this PR:

They are coupled to imported-mesh preservation rather than being a separate cleanup pass. Pascal renders level children in level-local coordinates and stacks levels using LevelNode.height. IFC elements are not always spatially contained by an IfcBuildingStorey—roof assemblies are commonly aggregated under IfcRoof / IfcBuilding—so preserving their triangles without storey inference can leave the geometry orphaned or attached to the wrong level. Once attached, subtracting the selected storey elevation is necessary to avoid applying elevation twice when Pascal stacks the level. Ordering levels and deriving their heights from IFC storey elevations keeps native and fallback nodes in the same coordinate frame.

The inference is source-semantic only: prefer explicit IFC spatial containment; otherwise select the nearest storey at or below the element elevation, with the lowest storey as the below-grade fallback. Focused tests cover below-all, between-storeys, above-all, and no-storey cases.

I did separate the unrelated wall-cleanup behavior into #603 as requested. If you would still prefer the storey normalization as a prerequisite PR, I can split it, but #553 would then need to depend on that PR for correctly placed fallback geometry.

Comment thread packages/ifc-converter/src/door-semantics.ts
Comment thread packages/ifc-converter/src/index.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit be1afd5. Configure here.

Comment thread packages/ifc-converter/src/index.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants