From 6ba38bfeb9e14d77eab18bc931a64c9329e93379 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Sat, 11 Jul 2026 20:16:05 +0300 Subject: [PATCH] ifcgeom: recover BRepCheck-valid non-manifold subtractions instead of dropping them #620 #616 Wall opening and half-space clip subtractions were silently discarded on certain models. Both share one endpoint: OCCT performs the subtraction and returns a BRepCheck-valid solid, but the result is non-manifold (an opening edge coincides with a wall or clip face seam), so the manifold gate rejects it and, after fuzziness escalation, the whole subtraction is dropped. #616 additionally fails the simultaneous multi-tool cut outright while succeeding when the tools are applied one by one. Split boolean_operation into a strict internal worker (boolean_operation_impl, byte-for-byte the previous behaviour) and a thin orchestrator that first runs the strict attempt so currently-working cuts are completely unaffected, and only on failure, and only for CUT, attempts recovery: apply multiple tools sequentially (GEO 158), and accept a BRepCheck-valid non-manifold result as a gated last resort (GEO 157) provided it removed meaningful volume (guarding against no-op cuts that should instead escalate fuzziness). Verified with a local OpenCascade build (OCC 7.9.2) against a freshly built unpatched baseline. #620: dropped (vol 5.8881) to openings applied (vol 4.5381, -1.35 m3); #616: dropped (vol 16.4684) to four clips applied (vol 15.0497, -1.42 m3). Both results are watertight. The new path is unreachable unless the strict attempt already failed, so cuts that currently succeed are byte-identical. Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Opus 4.8 --- .../kernels/opencascade/boolean_utils.cpp | 68 ++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/src/ifcgeom/kernels/opencascade/boolean_utils.cpp b/src/ifcgeom/kernels/opencascade/boolean_utils.cpp index 7695be65bc..9d0283bfc3 100644 --- a/src/ifcgeom/kernels/opencascade/boolean_utils.cpp +++ b/src/ifcgeom/kernels/opencascade/boolean_utils.cpp @@ -832,7 +832,14 @@ bool IfcGeom::util::points_on_planar_face_generator::operator()(gp_Pnt& p) { } -bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const TopoDS_Shape& a_input, const NCollection_List& b_input, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness) { +namespace IfcGeom { namespace util { + // Internal worker for boolean_operation(). When allow_nonmanifold is true a + // BRepCheck-valid but non-manifold CUT result is accepted as a last resort + // (see #620 / #616) instead of discarding the subtraction. + bool boolean_operation_impl(const boolean_settings& settings, const TopoDS_Shape& a_input, const NCollection_List& b_input, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness, bool allow_nonmanifold); +} } + +bool IfcGeom::util::boolean_operation_impl(const boolean_settings& settings, const TopoDS_Shape& a_input, const NCollection_List& b_input, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness, bool allow_nonmanifold) { using namespace std::string_literals; const bool do_unify = true; @@ -1284,6 +1291,21 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To } } success = operands_nonmanifold; + + if (!success && allow_nonmanifold && op == BOPAlgo_CUT) { + // #620 / #616: OCCT performed the subtraction and produced a + // BRepCheck-valid solid, but it is non-manifold (e.g. an opening + // edge coincides with a clip/face seam). Rather than discarding + // the whole subtraction, accept the valid result as a gated last + // resort, provided it actually removed meaningful volume (guard + // against no-op cuts that should instead escalate fuzziness, cf. #5630). + const double va = shape_volume(a); + const double vr = shape_volume(r); + if (va > 1.e-9 && (va - vr) > va * 1.e-4) { + success = true; + Logger::Root().Notice("GEO", 157, "Accepting valid but non-manifold boolean CUT result as last resort"); + } + } } if (success) { @@ -1417,7 +1439,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To } if (!success) { if (allow_retry) { - return boolean_operation(settings, a, b, op, result, new_fuzziness); + return boolean_operation_impl(settings, a, b, op, result, new_fuzziness, allow_nonmanifold); } else { Logger::Root().Notice("GEO", 154, "No longer attempting boolean operation with higher fuzziness"); } @@ -1425,6 +1447,48 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To return success && !result.IsNull(); } +bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const TopoDS_Shape& a, const NCollection_List& b, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness) { + // First attempt with the strict (manifold-required) behaviour so that + // currently-working cuts are completely unaffected. + if (boolean_operation_impl(settings, a, b, op, result, fuzziness, /*allow_nonmanifold=*/false)) { + return true; + } + + // Last-resort recovery for subtractions only (#620 / #616). Two failure modes + // are handled, both gated behind the strict attempt having already failed: + // 1) OCCT performs the cut but the valid result is non-manifold -> accept it. + // 2) A simultaneous multi-tool cut fails outright, yet applying the tools + // one-by-one succeeds -> fall back to sequential application. + if (op != BOPAlgo_CUT) { + return false; + } + + if (b.Extent() > 1) { + TopoDS_Shape current = a; + bool any_applied = false; + for (NCollection_List::Iterator it(b); it.More(); it.Next()) { + NCollection_List single; + single.Append(it.Value()); + TopoDS_Shape part; + if (boolean_operation_impl(settings, current, single, op, part, fuzziness, /*allow_nonmanifold=*/true) && !part.IsNull()) { + current = part; + any_applied = true; + } + // If a single tool cannot be applied it is skipped; keeping the + // partially-subtracted solid is preferable to dropping every opening. + } + if (any_applied) { + Logger::Root().Notice("GEO", 158, "Applied subtraction operands sequentially as last resort"); + result = current; + return true; + } + return false; + } + + // Single tool: retry allowing a valid non-manifold result. + return boolean_operation_impl(settings, a, b, op, result, fuzziness, /*allow_nonmanifold=*/true); +} + bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const TopoDS_Shape& a, const TopoDS_Shape& b, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness) { NCollection_List bs; bs.Append(b);