Compare commits

...

1 Commits

Author SHA1 Message Date
Petru Conduraru be1ccbc88c 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.
2026-07-20 18:19:35 +03:00
+8
View File
@@ -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<cgal_shape_t> ops;
std::list<CGAL::Nef_polyhedron_3<Kernel_>> 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();
}