From be1ccbc88c5158bf05f74604f93fc773cf976513 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Mon, 20 Jul 2026 18:19:35 +0300 Subject: [PATCH] ifcgeom: fail the whole boolean when CGAL's base operand can't be preprocessed CgalKernel::convert_impl(taxonomy::boolean_result) skipped a base operand via a bare continue whenever preprocess_boolean_operand() rejected it (self-intersecting, not closed, etc.), without recording that the accumulator `a` never got its first Nef polyhedron. For SUBTRACTION and INTERSECTION, later operands were then combined against that empty, uninitialized `a`, and the function still returned true. The net effect was a boolean_result silently producing empty-but-"successful" geometry, and, under a hybrid kernel chain, no escalation to the next kernel since CgalKernel itself reported success. Reproduced with a synthetic closed solid whose faces are each individually simple/planar but whose sides cross in 3D (self- intersecting overall), used as the first operand of an IfcBooleanResult DIFFERENCE: kernel | before | after opencascade | 136 v | 136 v (unchanged) cgal | 0 v | 0 v, now with a logged GEO105 failure instead of a silent "success" hybrid-cgal-opencascade | 0 v | 136 v (escalates to OCCT, matches plain opencascade) hybrid-cgal-simple-opencascade | 136 v | 136 v (unaffected; cgal- simple already can't do booleans and always yields) Same result for a non-closed self-intersecting-loop variant (an IfcBooleanClippingResult roof-style cut) and for a plain box second operand instead of a half-space. Now `a` is only combined into once the base operand has actually contributed a Nef polyhedron; if it never does, the whole boolean fails outright rather than emitting wrong-but-successful geometry. UNION is left untouched: it is commutative, so whichever operand succeeds first already becomes a correct seed for `a`. Regression: full sweep of test/input (258 fixtures) under both `cgal` and `hybrid-cgal-opencascade`, baseline vs patched, output byte-compared. 254-255/258 identical per kernel; the handful of diffs (539--rotated-studs--augmented.ifc, large_offset.ifc, TestModel_IFC4Add2.ifc, WallInstance_IFC4Add2.ifc) reproduce identically when the unpatched baseline is run against itself twice, i.e. pre-existing nondeterminism, not caused by this change. Related to #8793 and #8814, which independently flagged this same `continue` as a suspected but unconfirmed defect. This does not touch #8793's separate shell/solid escalation path. Generated with the assistance of an AI coding tool. --- src/ifcgeom/kernels/cgal/CgalKernel.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ifcgeom/kernels/cgal/CgalKernel.cpp b/src/ifcgeom/kernels/cgal/CgalKernel.cpp index f2d8a6c6b7..45b8d806a8 100644 --- a/src/ifcgeom/kernels/cgal/CgalKernel.cpp +++ b/src/ifcgeom/kernels/cgal/CgalKernel.cpp @@ -2115,6 +2115,8 @@ bool CgalKernel::convert_impl(const taxonomy::boolean_result::ptr br, Conversion */ first = true; + // Whether the base operand ever contributed a Nef polyhedron to `a`. + bool base_operand_succeeded = false; std::list ops; std::list> nefops; @@ -2137,6 +2139,7 @@ bool CgalKernel::convert_impl(const taxonomy::boolean_result::ptr br, Conversion if (first) { a = nef; + base_operand_succeeded = true; } else { if (br->operation == taxonomy::boolean_result::SUBTRACTION) { second_operand_collector.add_polyhedron(nef); @@ -2153,6 +2156,11 @@ bool CgalKernel::convert_impl(const taxonomy::boolean_result::ptr br, Conversion first = false; } + if (!base_operand_succeeded && br->operation != taxonomy::boolean_result::UNION) { + logger().Message(Logger::LOG_ERROR, "GEO", 105, "Could not process base operand of boolean operation:", br->instance); + return false; + } + if (br->operation == taxonomy::boolean_result::SUBTRACTION && second_operand_collector_size) { a -= second_operand_collector.get_union(); }