mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-09 17:31:45 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<TopoDS_Shape>& 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<TopoDS_Shape>& 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();
|
||||
}
|
||||
|
||||
@@ -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<TopoDS_Shape>&, BOPAlgo_Operation, TopoDS_Shape&, double fuzziness = -1.);
|
||||
bool boolean_operation(const boolean_settings& settings, const TopoDS_Shape&, const NCollection_List<TopoDS_Shape>&, 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.);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user