fix(geometry): material-layer walls emit a watertight skin, not a doubled "ghost face"#1311
Merged
Merged
Conversation
…bled "ghost face" A material-layer wall was sliced into per-layer slabs, each CAPPED at its interface planes to be an independently closed solid. Every SHARED interface was therefore capped twice — a coincident, oppositely-wound, full-cross-section sheet. The wall still rendered solid (the interior caps are backface-culled) but the emitted mesh was non-watertight (degree-4 interface edges) and ~3x the triangles: the "ghost face" reported on opening-cut layered walls. AC-20 #37781: 272 tris / 73 degree-4 edges -> 160 tris / 0 degree-4, watertight. Slice without capping, carving each band off a running REMAINDER so both sides of every interface come from the same clip of the same mesh (matching tessellation, so the bands weld with no T-junctions). The union of the open bands is exactly the wall's watertight outer skin, partitioned per material. The 2D section consumed the capped slabs to reconstruct per-layer fills, so the @ifc-lite/drawing-2d PolygonBuilder loop builder is now bidirectional: each open band assembles into a ring whose implicit head->tail chord re-creates the interface line the cap used to draw. Per-layer section fills are unchanged (verified by an open-band reconstruction test).
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe Rust geometry slicer is rewritten to progressively carve uncapped layer bands from a running remainder mesh, eliminating coincident ghost faces and triangle duplication. To handle the resulting open bands, the TypeScript polygon builder adds bidirectional head/tail loop construction with interface-aware closure, principal-axis estimation, and endpoint-pair stitching to reconstruct multi-layer walls. The renderer removes its culled pipeline and renders all layer slices double-sided. The viewer integrates opaque base cross-sections (held separately in ChangesCap-free layer slicing and multi-layer wall reconstruction
Sequence Diagram(s)sequenceDiagram
participant Geometry as Geometry Slicer
participant PolygonBuilder as Polygon Builder
participant Renderer as Renderer
participant Viewer as 2D Overlay
Geometry->>Geometry: Carve cap-free bands from remainder
Geometry->>PolygonBuilder: Uncapped layer band segments
PolygonBuilder->>PolygonBuilder: Bidirectional head/tail loop closure
PolygonBuilder->>PolygonBuilder: Interface-aware fragment stitching
PolygonBuilder->>Renderer: Per-layer colorized polygons
PolygonBuilder->>Viewer: Opaque layerBaseCutPolygons
Renderer->>Renderer: Render layer slices double-sided (no cull)
Viewer->>Viewer: Render base first, then per-layer colors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac3ceeb32c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…eview) Codex review: the watertight-bands change dropped the section fill of an INTERIOR layer of a 3+ layer wall. Such a band has no wall face — its plan section is two disconnected end strips that no single chain can close — so the bidirectional loop builder, which closes a 2-layer U-band fine, emitted nothing for the core. The PolygonBuilder now collects band fragments that are too short to close on their own and stitches them end-to-end at the interface chords, only closing a chain on itself once no other fragment remains (merging across-first matters: an interior band is thin, so its interface chord is longer than the band thickness, and a naive nearest-endpoint close would collapse the band). Fragments that already close into a non-degenerate loop on their own (a 2-layer U-band, the finish-on-both-faces case) stay separate, so existing per-loop behaviour is unchanged. Scoped to multi-material (per-layer) groups only. Adds a 3-layer regression test (the core layer fills with the correct area); all 46 drawing-2d tests pass.
Collaborator
Author
|
viewer now messes up multilayer display... |
Since #1311 each material layer is emitted as an OPEN band (watertight union, no doubled interface sheet), so the 3D section cap re-closes each band at its interface chords. On a wall with a door/window opening, an interior layer (no broad face) sections into 4+ disconnected vertical strips, and the greedy nearest-endpoint stitch hopped one strip to the strip ACROSS the opening — one self-overlapping polygon that bridged the void and failed to triangulate. The cut then read HOLLOW, which is what "the whole wall looks hollow in 3D section view" was. Close open bands along the interface LINES instead: detect the band's principal (length) axis via the endpoint covariance (robust to rotated walls), cluster endpoints onto the <=2 parallel interface lines, and pair them CONSECUTIVELY along each line (a scanline rule). That closes every solid chunk and leaves the opening between chunks empty. Ambiguous, near-square endpoint clouds fall back to the previous stitch, so no case regresses. Tests: new polygon-builder-opening suite (2-layer + opening, 3-layer interior core + opening, rotated wall); full drawing-2d 49/49 green.
… hollow The per-layer cap is reconstructed from open (cap-free) layer bands, so an exotic wall the reconstruction cannot disambiguate can still leave a gap and the 3D section cut reads see-through. Add a backstop that guarantees a solid cut regardless. For every MULTI-material entity, also build its full closed cross-section (buildBasePolygons): combining all of an entity's cut segments drops the open interfaces and leaves the watertight outer skin, which closes into the solid chunks with no interface stitching needed. This base is carried in a NEW Drawing2D.layerBaseCutPolygons that ONLY the 3D section overlay reads, so the flat 2D drawing, SVG export, and measure/snap paths are untouched. The overlay triangulates polygons in array order into one draw, so the viewer prepends the (colourless => opaque uniform cap fill) base polygons before the per-layer colours: the colours show where they reconstruct, and solid cut material shows everywhere else. Net effect: per-layer colour where possible, never a hollow-looking cut. Tests: buildBasePolygons suite (full section per layered entity, opening stays empty, none for single-material); full drawing-2d 51/51 green; drawing-2d + changed viewer files typecheck clean.
…t hollow Multilayer walls read HOLLOW in normal (uncut) 3D, not only in section. Root cause: the renderer backface-culled material-layer slices (the culledPipeline, cullMode 'back'), assuming their winding was reliably outward. That held for the OLD closed per-layer slabs, where the cull hid the coincident interface caps that would otherwise z-fight. Since #1311 the slabs are OPEN bands whose union is the wall's watertight outer skin (no caps), and IFC winding is not reliably outward, so culling dropped every inward-wound face and punched holes into the skin — the wall looked like a thin see-through shell. Render layer slices DOUBLE-SIDED like all other IFC geometry: every face of the watertight skin draws, so the wall reads solid. With no coincident caps left there is nothing to z-fight, so the dedicated culled pipeline and the isLayer batch flag that selected it are removed. GEOM_CLASS_LAYER_SLICE stays — it now only marks per-layer section fills, not culling. renderer + spatial typecheck clean; drawing-2d 51/51. element.rs change is the stale-comment update only (no code change).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffe5ac50dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2
to
+3
| "@ifc-lite/drawing-2d": patch | ||
| "@ifc-lite/renderer": patch |
There was a problem hiding this comment.
When this changeset is published, only @ifc-lite/drawing-2d and @ifc-lite/renderer receive new versions, but the mesh-slicing fix lives in rust/geometry/src/router/layers.rs and is shipped to npm through @ifc-lite/wasm. Consumers can therefore get the new renderer that deliberately stops culling layer slices while still resolving the old wasm runtime that emits capped, coincident layer slabs, reintroducing the z-fighting/hollow wall case this PR fixes. Add a patch bump for @ifc-lite/wasm (and the dependent geometry package if releases rely on it) so the renderer and wasm geometry stay in lockstep.
Useful? React with 👍 / 👎.
Comment on lines
+105
to
+108
| const colors = new Set(entitySegments.map((s) => colorKey(s.color))); | ||
| if (colors.size < 2) continue; | ||
| // Colourless build ⇒ closed-loop path (the combined section is closed). | ||
| const base = this.buildColorGroupPolygons(entitySegments, undefined) |
There was a problem hiding this comment.
This treats every multi-colour cut as a material-layer wall, but DrawingPolygon.color is also used for non-layer splits such as a frame+glass window. In that case buildBasePolygons() emits a colourless base polygon behind the transparent glass/frame parts; useRenderUpdates uploads that base without a material colour, so the shader fills it with the opaque cap style before blending the glass polygon, making transparent window cuts read as opaque solid material. Restrict the base backstop to actual layer-slice entities instead of colors.size > 1 alone.
Useful? React with 👍 / 👎.
This was referenced Jun 23, 2026
louistrue
added a commit
that referenced
this pull request
Jun 23, 2026
#1311 changed rust/geometry + rust/processing (compiled into @ifc-lite/wasm) but the changeset only listed @ifc-lite/drawing-2d and @ifc-lite/renderer. `changeset publish` only publishes packages whose local version is not already on npm, so the new wasm mesh generation would never reach npm consumers and the hollow-wall fix in the release notes would not actually ship. Add @ifc-lite/wasm to the changeset. @ifc-lite/geometry needs no bump: it builds with tsc and imports @ifc-lite/wasm as an external runtime dependency (workspace:^), so consumers pick up the republished wasm transitively at install time. Mark @ifc-lite/viewer private. apps/viewer is a Vite app deployed via Vercel, not a library: it has no main/module/exports/bin entry point, so the published package is unusable as a dependency. It was never marked private (unlike its sibling app @ifc-lite/viewer-embed), so every release dumps a ~29MB / 1100-file tarball of the whole app tree (full src/, built dist/ incl. the Cesium asset bundle, configs, a stray .turbo build log) to npm with zero consumers. The real consumable viewer library is @ifc-lite/viewer-core. The #1311 overlay change in apps/viewer ships via Vercel deployment from main, not via npm, so the viewer does not belong in the changeset. Existing npm 1.x versions are left in place. Verified with `changeset version`: bumps wasm 2.13.0->2.13.1, drawing-2d 1.18.3->1.18.4, renderer 1.29.1->1.29.2; viewer is not versioned or published (private + absent from the changeset).
louistrue
added a commit
that referenced
this pull request
Jun 23, 2026
#1328) #1311 changed rust/geometry + rust/processing (compiled into @ifc-lite/wasm) but the changeset only listed @ifc-lite/drawing-2d and @ifc-lite/renderer. `changeset publish` only publishes packages whose local version is not already on npm, so the new wasm mesh generation would never reach npm consumers and the hollow-wall fix in the release notes would not actually ship. Add @ifc-lite/wasm to the changeset. @ifc-lite/geometry needs no bump: it builds with tsc and imports @ifc-lite/wasm as an external runtime dependency (workspace:^), so consumers pick up the republished wasm transitively at install time. Mark @ifc-lite/viewer private. apps/viewer is a Vite app deployed via Vercel, not a library: it has no main/module/exports/bin entry point, so the published package is unusable as a dependency. It was never marked private (unlike its sibling app @ifc-lite/viewer-embed), so every release dumps a ~29MB / 1100-file tarball of the whole app tree (full src/, built dist/ incl. the Cesium asset bundle, configs, a stray .turbo build log) to npm with zero consumers. The real consumable viewer library is @ifc-lite/viewer-core. The #1311 overlay change in apps/viewer ships via Vercel deployment from main, not via npm, so the viewer does not belong in the changeset. Existing npm 1.x versions are left in place. Verified with `changeset version`: bumps wasm 2.13.0->2.13.1, drawing-2d 1.18.3->1.18.4, renderer 1.29.1->1.29.2; viewer is not versioned or published (private + absent from the changeset).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Material-layer walls (
IfcMaterialLayerSetUsage) rendered correctly but the emitted mesh carried a spurious, full-cross-section "ghost face" offset by a layer thickness: it was non-watertight and ~3× the triangles of the correct solid. Reported on opening-cut layered walls (e.g.AC-20-Smiley-West-10-Bldg.ifc#37781).This is finding #1 from the geometry audit. (Finding #2 — "opening not subtracted on clipped hosts" — turned out to be the #1297 local-frame bug, already fixed by #1310; findings #3/#4 are lower-priority and tracked separately.)
Root cause
The slicer (
rust/geometry/src/router/layers.rs) cut a layered wall into per-layer slabs and capped each slab at its interface planes so every slab was an independently closed solid. Every shared interface was therefore capped twice — two coincident, oppositely-wound, full-cross-section sheets. The wall still rendered solid (the interior caps areGEOM_CLASS_LAYER_SLICEbackface-culled), but the mesh had degree-4 interface edges (non-manifold) and triple the triangles.AC-20#37781Fix
Rust slicer: slice without capping, carving each band off a running remainder so both sides of every interface come from the same clip of the same mesh. The cut tessellations then match exactly, so the open bands weld edge-for-edge (no T-junctions) — the union of the bands is precisely the wall's watertight outer skin, partitioned per material. (A first cut that capped each interface once still left T-junctions where a twice-clipped middle band diverged from its neighbour; the progressive carve eliminates them — verified: synthetic 3-layer wall and real
AC-20#37781both reportopen=0, degree-4=0.)2D section (
@ifc-lite/drawing-2d): the section reconstructs per-layer fills from these slab meshes (useDrawingGenerationfeedsgeometryResult.meshesstraight into the cutter), and a forward-only loop builder strands an open band's segments → no fill. ThePolygonBuilderloop builder is now bidirectional: each open band assembles into a ring whose implicit head→tail chord re-creates the interface line the cap used to draw, so per-layer section fills are unchanged.Tests
material_layers_local_frame_test.rsupdated: the slab union must be watertight (no open edges) and carry no doubled interface sheet (no degree-4 edges).polygon-builder.test.ts: new open-band reconstruction test — two cap-free U-bands sharing an interface still yield one filled polygon per layer.material_layers6/6,merge_layers2/2,wall_opening_cut_regression7/7, lib 389/0);drawing-2d45/45.🤖 Generated with Claude Code
Follow-up folded in: solid layer look in 3D section view
The watertight (cap-free) bands made layered walls read hollow when cut in the 3D section view, not just at minor gaps. Two commits fix that while keeping the watertight geometry from this PR (no Rust change):
fix(drawing-2d): solid per-layer section cap on opening-cut walls. The 3D section clip and the cap are both gated on the section tool, so the cap is always generated when you cut. Its per-layer reconstruction from open bands broke on any 3+ layer wall with an opening: an interior layer (no broad face) sections into 4+ disconnected strips, and the greedy nearest-endpoint stitch joined a strip to the one ACROSS the opening, emitting one self-overlapping polygon that bridged the void and failed to triangulate. That layer is the bulk of the wall thickness, so the cut read hollow. Closure now runs along the interface lines (paired consecutively, scanline-style, on the band's principal/length axis, so it is robust to rotated walls), which fills each solid chunk and leaves the opening empty.feat(drawing-2d): opaque base-cap backstop so section cuts never read hollow. For each multi-material entity the builder also emits its full closed cross-section (the watertight union always closes, so no interface stitching is needed), carried in a newDrawing2D.layerBaseCutPolygonsthat ONLY the 3D section overlay consumes. The overlay draws this opaque base behind the per-layer colours, so colours show where they reconstruct and solid cut material shows everywhere else. The flat 2D drawing, SVG export, and measure/snap paths readcutPolygonsand are untouched.fix(renderer): draw material-layer slices double-sided so walls aren't hollow. The deeper cause of "hollow" was in NORMAL (uncut) 3D, not just section. The renderer backface-culled the slices, assuming reliable outward winding. That held for the OLD closed slabs (the cull hid their coincident interface caps). The slabs are now open bands whose union is the wall's watertight outer skin (no caps), and IFC winding is not reliably outward, so culling dropped inward-wound faces and punched holes, making the wall look like a thin see-through shell. The slices now render double-sided like all other IFC geometry, so every face of the watertight skin draws and the wall reads solid; with no coincident caps left there is nothing to z-fight, so the dedicated culled pipeline and theisLayerbatch flag are removed (GEOM_CLASS_LAYER_SLICEstays as the per-layer-fill marker only).Tests: new
polygon-builder-opening.test.ts(2-layer + opening, 3-layer interior core + opening, rotated wall, base-cap cases); fulldrawing-2dnow 51/51;drawing-2d,renderer, and the changed viewer files typecheck clean.Summary by CodeRabbit