From b394b59b446d01274081d3c2500f29ff22925d43 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 23:02:52 +0300 Subject: [PATCH] Keep valid non-manifold subtraction result instead of dropping the cut (#1015) util::boolean_operation requires a manifold result: success is !is_manifold(a) || is_manifold(result). When a real subtraction (e.g. a mitered IfcHalfSpaceSolid beam-end cut that grazes fine end detailing, retry also fails the manifold test, boolean_operation returns false, and the caller silently keeps the un-subtracted first operand. The entire subtraction chain is dropped, so booleans-on output is identical to booleans-off. Remember a BRepCheck-valid CUT result that was rejected solely for being non-manifold, and use it as a last resort once all fuzziness retries are exhausted. A new defaulted param allow_nonmanifold_fallback is passed false into the retry recursion so the decision is deferred to the finest (most accurate) top-level frame. New diagnostic GEO 155. The success path returns early exactly as before, so the change is inert for every currently-passing cut. Verified on OCC 7.9.2: models that already convert (IFC4 #4118, #5473; IFC2X3 #5186, #511) are byte-identical before and after, while the #1015 beam goes from an un-subtracted 40-vert prism to a correctly mitered 514-vert solid (end wedge of 0.87 m removed, matching the fitted half-space distance). Co-Authored-By: Claude Opus 4.8 --- .../kernels/opencascade/boolean_utils.cpp | 34 +++++++++++++++++-- .../kernels/opencascade/boolean_utils.h | 2 +- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/ifcgeom/kernels/opencascade/boolean_utils.cpp b/src/ifcgeom/kernels/opencascade/boolean_utils.cpp index d104c7ff94..08e379561c 100644 --- a/src/ifcgeom/kernels/opencascade/boolean_utils.cpp +++ b/src/ifcgeom/kernels/opencascade/boolean_utils.cpp @@ -832,9 +832,18 @@ 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) { +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, bool allow_nonmanifold_fallback) { using namespace std::string_literals; + // A geometrically valid (BRepCheck_Analyzer) subtraction result that is + // rejected only for being non-manifold is remembered here. Some real models + // (e.g. a mitered/half-space beam end cut that grazes fine end detailing, + // see #1015) can only be resolved into such a result. Silently discarding + // the whole cut and returning the un-subtracted first operand is worse than + // keeping this result, so it is used as a last resort once the fuzziness + // retries are exhausted. + TopoDS_Shape nonmanifold_fallback; + const bool do_unify = true; const bool do_subtraction_eliminate_disjoint_bbox = true; const bool do_subtraction_eliminate_touching = true; @@ -1390,6 +1399,12 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To } else { settings.log().Notice("GEO", 151, "Boolean operation yields non-manifold result"); + // r passed BRepCheck_Analyzer above and was rejected solely for + // being non-manifold. Remember it as a last-resort fallback so a + // difficult but valid subtraction is not dropped entirely (#1015). + if (op == BOPAlgo_CUT && nonmanifold_fallback.IsNull() && !r.IsNull()) { + nonmanifold_fallback = r; + } } } } @@ -1417,10 +1432,25 @@ 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); + // Retry at a higher fuzziness in the hope of a clean manifold result. + // The retry is not allowed to apply the non-manifold fallback itself: + // that decision is deferred to this (finer-fuzziness) frame so that, + // if every retry also fails, the most accurate fallback is used. + if (boolean_operation(settings, a, b, op, result, new_fuzziness, false)) { + return true; + } } else { settings.log().Notice("GEO", 154, "No longer attempting boolean operation with higher fuzziness"); } + + if (allow_nonmanifold_fallback && !nonmanifold_fallback.IsNull()) { + // Every attempt failed the manifold check but produced a geometrically + // valid subtraction. Keeping it preserves the boolean cut instead of + // silently returning the un-subtracted first operand (#1015). + settings.log().Message(Logger::LOG_WARNING, "GEO", 155, "Boolean subtraction result is non-manifold, using it regardless to preserve the cut"); + result = nonmanifold_fallback; + return true; + } } return success && !result.IsNull(); } diff --git a/src/ifcgeom/kernels/opencascade/boolean_utils.h b/src/ifcgeom/kernels/opencascade/boolean_utils.h index 55bd960130..cd848c949d 100644 --- a/src/ifcgeom/kernels/opencascade/boolean_utils.h +++ b/src/ifcgeom/kernels/opencascade/boolean_utils.h @@ -104,7 +104,7 @@ namespace IfcGeom { Logger& log() const { return logger ? *logger : Logger::Root(); } }; - bool boolean_operation(const boolean_settings& settings, const TopoDS_Shape&, const NCollection_List&, BOPAlgo_Operation, TopoDS_Shape&, double fuzziness = -1.); + bool boolean_operation(const boolean_settings& settings, const TopoDS_Shape&, const NCollection_List&, BOPAlgo_Operation, TopoDS_Shape&, double fuzziness = -1., bool allow_nonmanifold_fallback = true); bool boolean_operation(const boolean_settings& settings, const TopoDS_Shape&, const TopoDS_Shape&, BOPAlgo_Operation, TopoDS_Shape&, double fuzziness = -1.);