mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-05 23:41:44 +00:00
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.
This commit is contained in:
@@ -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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user