fix(geometry): cut round window authored as two cap-to-cap extrusions#1330
Conversation
The AC20 round window #68400 in gable wall #67828 is authored as two IfcExtrudedAreaSolid cylinders sharing a profile and start point but extruding in OPPOSITE directions. The combined cutter mesh therefore carries an interior back-to-back cap membrane mid-wall, which the exact CSG subtract treats as a real boundary — leaving a solid plug at the seam so the window never cuts through. Add remove_internal_membrane: it buckets cap-facing cutter triangles by plane offset along the penetration axis and deletes any INTERIOR bucket whose faces point both along and against the axis (a glued membrane), welding the two solids into one continuous tube before the subtract. The whole interior cap plane is removed rather than only vertex-coincident pairs, since the two disks are often triangulated differently and pair-matching leaves a central plug (a square patch in the round hole). A no-op for ordinary single-solid openings, and it won't merge a genuine two-hole opening (a lone interior cap points only one way). Add a frame-independent regression (interior-hole flood-fill, no guessed world coordinate) plus a production submesh-path assertion.
|
@rpisarew is attempting to deploy a commit to the LTplus Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 34 minutes and 44 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesRound-Window Seam Fix and Regression Tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/geometry/src/router/voids.rs`:
- Around line 3724-3767: The current implementation in the buckets HashMap only
tracks whether each bucket has positive and negative facing cap faces, but does
not account for spatial separation. This causes unrelated caps from different
components that coincidentally share the same offset value (computed via the
bucket function) to be incorrectly classified as a glued membrane and removed.
Enhance the bucket structure to track not just the boolean directional flags,
but also the spatial footprint or positions of the cap faces within each bucket
(store the centroid coordinates or projected positions alongside the direction
booleans). Then when filtering for membranes, add an additional check that
verifies the positive and negative facing cap faces in the same bucket actually
overlap spatially or belong to the same connected component before marking the
bucket as removable.
- Around line 2769-2771: The deseaming operation via remove_internal_membrane is
currently only applied in the sequential subtract_mesh path before calling
extend_opening_mesh_through_host. However, the batched cutter construction path
that uses subtract_mesh_many is still passing the original opening_mesh without
deseaming. Apply the same deseaming pattern in the batch re-extension branch by
calling Self::remove_internal_membrane on opening_mesh before passing it to
extend_opening_mesh_through_host, ensuring consistent treatment across both
construction paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9247be30-8489-4161-aa5f-a2c890d0b08d
📒 Files selected for processing (3)
rust/geometry/src/router/voids.rsrust/geometry/tests/issue_635_boolean_clipping_test.rsrust/processing/tests/issue_1320_wall_67828_round_window.rs
remove_internal_membrane keyed interior cap planes on axis-offset and direction alone, so two laterally-separated caps that merely share an offset (e.g. side-by-side cutters abutting at one plane) would be welded, punching through solid material between the holes. Track the lateral (perpendicular-to-axis) bounding box of the +facing and -facing caps per bucket separately, treat a bucket as a membrane only where those footprints overlap, and remove only triangles inside that overlap region. The coincident-disk case (the real glued seam) is unchanged; a lone cap or disjoint neighbour sharing the offset is kept.
remove_internal_membrane ran only on the sequential subtract path. The disjoint-cutter batch path (subtract_mesh_many) extended the raw opening_mesh at both its admission and re-extension sites, so a two-extrusion cap-to-cap opening batched alongside another disjoint opening kept its internal seam membrane and left a solid plug. Apply the same deseam before extend_opening_mesh_through_host at both batch sites. A no-op for ordinary single-solid openings, and the existing mesh_is_closed_exact watertightness guard still gates batch admission, so a deseamed non-watertight cutter falls through to the sequential path (which also deseams) — no regression either way.
…_through_host (LTplus-AG#1336) Follow-up to LTplus-AG#1330. remove_internal_membrane was called explicitly before each of the 3 extend_opening_mesh_through_host call sites; a future void path (or the dead voids_2d fast path, if revived) could route an opening cutter through extend without deseaming and silently regress the two-extrusion round-window cut. Move the deseam to the top of extend_opening_mesh_through_host, the single helper every void subtract funnels through, so it can't be forgotten. Pure refactor: the 3 call sites now pass the raw opening_mesh, behaviour is identical (the helper already received the same depth_dir the explicit deseam used). Verified: round window still cuts clean (120 interior hole cells, unchanged); issue_635 11/11 + wall_opening_cut_regression + voids_production/submesh + issue_964/1167/1007/960 + door_window_calibration + engulfing_solid_void all green.
2 participants
Summary
The circular window
OG-Fenster-1(#68400) in gable wallWand-Ext-OG-3(#67828) of
AC20-FZK-Haus.ifcdoes not cut through — the wall renders solid,or with a square plug left inside the round hole. This regression surfaced
after #1320.
Reproduction
Model:
AC20-FZK-Haus.ifc(https://github.com/ibpsa/project1-wp-2-2-bim/blob/master/IFC_Files/MISC/AC20-FZK-Haus.ifc).
Load it and inspect the upper-floor gable wall
3VCarUKgH1buLo22Ozxe6J(#67828): before the fix its round window
0e2iWM3ICUNVYAkmP5YImx(#68400) isnot cut through.
Root cause
The AC20 round window is authored as two
IfcExtrudedAreaSolidcylinderssharing the same circle profile and start point but extruding in opposite
directions (depths 0.485 m and 0.415 m). The combined opening cutter mesh
therefore carries an interior back-to-back cap membrane mid-wall — a pair of
coincident, oppositely-wound cap disks (~28 tris) sitting inside the 0.30 m
wall thickness. The exact CSG subtract treats that double cap as a real
boundary and leaves a solid plug at the seam, so the void never cuts
through.
Fix
remove_internal_membrane, applied to the cutter before the subtract: itbuckets cap-facing cutter triangles by plane offset along the penetration axis
and deletes any interior bucket whose faces point both along and against the
axis (a glued membrane), welding the two solids into one continuous tube.
The whole interior cap plane is removed rather than only vertex-coincident
pairs — the two disks are often triangulated differently, so pair-matching
leaves a central plug (a square patch in the round hole).
Verification
wall_67828_round_window_cuts_through_native(thinnest-axis ray grid + interior-hole flood-fill; no guessed world
coordinate — earlier tests falsely passed by ray-casting a point that missed
the rotated mesh). Interior hole cells: 0 (solid) → 120 (clean disk).
issue_635_boolean_clipping_test.through the WASM browser path.
Regression reported in #1320.
Summary by CodeRabbit
Bug Fixes
Tests