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 <noreply@anthropic.com>

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<TopoDS_Shape>& 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<TopoDS_Shape>& 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<TopoDS_Shape>& 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<TopoDS_Shape>& 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<TopoDS_Shape>::Iterator it(b); it.More(); it.Next()) {
+			NCollection_List<TopoDS_Shape> 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<TopoDS_Shape> bs;
 	bs.Append(b);