Glassvision/IfcOpenShell
1
0
Fork 0
mirror of https://github.com/IfcOpenShell/IfcOpenShell.git synced 2026-08-13 02:47:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Route boolean-op kernel logging through the injected Logger

Browse Source 
src/ifcgeom/kernels/opencascade/boolean_utils.cpp, OpenCascadeKernel.cpp,
and boolean_result.cpp logged diagnostics (including the "Processed
fully in 2D" family of messages) through the global Logger::Root()
singleton. IfcConvert's main() constructs its own Logger and wires it
to --log-file via SetOutput(), then threads that instance through
Converter/kernel constructors as logger_ (see AbstractKernel). Since
Logger::Root() is never itself configured with an output stream, every
Notice/Warning/Message call through it was silently dropped instead of
reaching the log file - Logger::Message's log1_/log2_ null checks just
no-op.

This made src/ifcopenshell-python/test/test_wall_opening.py fail: it
asserts on specific log messages that the underlying boolean-op code
was still emitting correctly, just to nowhere. The geometry itself was
never wrong.

Add a Logger*, defaulting to null, to boolean_settings (with a log()
accessor falling back to Logger::Root() for the few remaining
call sites with no injected logger available), thread it through
eliminate_narrow_operands and boolean_subtraction_2d_using_builder,
and have OpenCascadeKernel/boolean_result.cpp populate it from their
inherited logger_ member instead of relying on the global singleton.

Generated with the assistance of an AI coding tool.
This commit is contained in:
Stephen Boddy
2026-07-11 19:05:56 +01:00
parent d188e3beaf
commit 96e2efebc8
4 changed files with 59 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ifcgeom/kernels/opencascade/boolean_utils.cpp
+38 -38
View File
@@ -405,7 +405,7 @@ bool IfcGeom::util::is_extrusion(const gp_Vec & v, const TopoDS_Shape & s, TopoD
return true;
}
int IfcGeom::util::eliminate_narrow_operands(double prec, const NCollection_List<TopoDS_Shape>& bs, NCollection_List<TopoDS_Shape> & c) {
int IfcGeom::util::eliminate_narrow_operands(double prec, const NCollection_List<TopoDS_Shape>& bs, NCollection_List<TopoDS_Shape> & c, Logger& logger) {
int N = 0;
NCollection_List<TopoDS_Shape>::Iterator it(bs);
for (; it.More(); it.Next()) {
@@ -418,7 +418,7 @@ int IfcGeom::util::eliminate_narrow_operands(double prec, const NCollection_List
bool is_narrow = min_dimension < prec;
Logger::Root().Notice("GEO", 122, "Min OBB dimension of operand = " + std::to_string(min_dimension));
logger.Notice("GEO", 122, "Min OBB dimension of operand = " + std::to_string(min_dimension));
if (!is_narrow) {
c.Append(it.Value());
@@ -573,7 +573,7 @@ int IfcGeom::util::eliminate_touching_operands(double prec, const TopoDS_Shape &
return N;
}
bool IfcGeom::util::boolean_subtraction_2d_using_builder(const TopoDS_Shape & a_input, const NCollection_List<TopoDS_Shape> & b_input, TopoDS_Shape & result, double eps) {
bool IfcGeom::util::boolean_subtraction_2d_using_builder(const TopoDS_Shape & a_input, const NCollection_List<TopoDS_Shape> & b_input, TopoDS_Shape & result, double eps, Logger& logger) {
IfcGeom::impl::tree<int> edge_tree;
NCollection_List<TopoDS_Shape> ab_input = b_input;
@@ -703,7 +703,7 @@ bool IfcGeom::util::boolean_subtraction_2d_using_builder(const TopoDS_Shape & a_
if (u11 < U1 && U1 < u12 && u21 < U2 && U2 < u22) {
// Edge curves belonging to different operands intersect, don't process
// using builder.
Logger::Root().Notice("GEO", 123, "Intersecting boundaries");
logger.Notice("GEO", 123, "Intersecting boundaries");
return false;
}
}
@@ -750,7 +750,7 @@ bool IfcGeom::util::boolean_subtraction_2d_using_builder(const TopoDS_Shape & a_
// any effect and marked as redundant. Feeding it to the builder algo
// will likely cause problems.
redundant[std::distance(wires.begin(), it)] = true;
Logger::Root().Notice("GEO", 124, "Subtraction operand outside of outer bound");
logger.Notice("GEO", 124, "Subtraction operand outside of outer bound");
}
}
@@ -790,7 +790,7 @@ bool IfcGeom::util::boolean_subtraction_2d_using_builder(const TopoDS_Shape & a_
if (wire_clss[wire_index]->Perform(p2d) == TopAbs_IN) {
// A wire is contained within another operand
redundant[other_index] = true;
Logger::Root().Notice("GEO", 125, "Subtraction operand contained in other");
logger.Notice("GEO", 125, "Subtraction operand contained in other");
}
}
}
@@ -848,7 +848,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
std::stringstream ss;
ss << "bool-" << std::this_thread::get_id() << "-" << (operation_counter_++);
debug_identifier = ss.str();
Logger::Root().Notice("GEO", 126, "Boolean debug identifier: " + debug_identifier);
settings.log().Notice("GEO", 126, "Boolean debug identifier: " + debug_identifier);
}
if (fuzziness < 0.) {
@@ -884,7 +884,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
a = unify(a_input, fuzziness * 1000.);
Logger::Root().Message(
settings.log().Message(
Logger::LOG_DEBUG, "GEO", 127,
"Simplified operand A from "s +
std::to_string(count(a_input, TopAbs_FACE)) +
@@ -896,7 +896,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
NCollection_List<TopoDS_Shape>::Iterator it(b_input);
for (; it.More(); it.Next()) {
b.Append(unify(it.Value(), fuzziness));
Logger::Root().Message(
settings.log().Message(
Logger::LOG_DEBUG, "GEO", 128,
"Simplified operand B from "s +
std::to_string(count(it.Value(), TopAbs_FACE)) +
@@ -924,7 +924,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
auto N = bounding_box_overlap(fuzziness, a, b, b_tmp);
if (N) {
Logger::Root().Notice("GEO", 129, "Eliminated " + std::to_string(N) + " disjoint operands");
settings.log().Notice("GEO", 129, "Eliminated " + std::to_string(N) + " disjoint operands");
std::swap(b, b_tmp);
}
}
@@ -935,7 +935,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
b_tmp.Clear();
auto N = eliminate_touching_operands(fuzziness, a, b, b_tmp);
if (N) {
Logger::Root().Notice("GEO", 130, "Eliminated " + std::to_string(N) + " touching operands");
settings.log().Notice("GEO", 130, "Eliminated " + std::to_string(N) + " touching operands");
std::swap(b, b_tmp);
}
}
@@ -944,9 +944,9 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
PERF("boolean subtraction: eliminate narrow");
b_tmp.Clear();
auto N = eliminate_narrow_operands(fuzziness, b, b_tmp);
auto N = eliminate_narrow_operands(fuzziness, b, b_tmp, settings.log());
if (N) {
Logger::Root().Notice("GEO", 131, "Eliminated " + std::to_string(N) + " narrow operands");
settings.log().Notice("GEO", 131, "Eliminated " + std::to_string(N) + " narrow operands");
std::swap(b, b_tmp);
}
}
@@ -960,21 +960,21 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
}
if (b.Extent() == 0) {
Logger::Root().Warning("GEO", 132, "No other operands remaining, using first operand");
settings.log().Warning("GEO", 132, "No other operands remaining, using first operand");
result = a;
return true;
}
if (!is_2d && Logger::LOG_NOTICE >= Logger::Root().Verbosity()) {
if (!is_2d && Logger::LOG_NOTICE >= settings.log().Verbosity()) {
PERF("preliminary manifoldness check");
if (!a.IsNull()) {
Logger::Root().Notice("GEO", 133, "Operand A is " + (is_manifold(a) ? ""s : "non-"s) + "manifold");
settings.log().Notice("GEO", 133, "Operand A is " + (is_manifold(a) ? ""s : "non-"s) + "manifold");
}
NCollection_List<TopoDS_Shape>::Iterator it(b);
for (int i = 0; it.More(); it.Next(), ++i) {
Logger::Root().Notice("GEO", 134, "Operand B " + std::to_string(i) + " is " + (is_manifold(it.Value()) ? ""s : "non-"s) + "manifold");
settings.log().Notice("GEO", 134, "Operand B " + std::to_string(i) + " is " + (is_manifold(it.Value()) ? ""s : "non-"s) + "manifold");
}
}
@@ -1014,7 +1014,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
const double fuzz = (std::min)(min_length_orig / 3., fuzziness);
Logger::Root().Notice("GEO", 135, "Used fuzziness: " + std::to_string(fuzz));
settings.log().Notice("GEO", 135, "Used fuzziness: " + std::to_string(fuzz));
const double new_fuzziness = fuzziness * 10.;
const bool allow_retry = new_fuzziness - 1e-15 <= settings.precision * 10000. && new_fuzziness < min_length_orig;
@@ -1048,7 +1048,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
}
if (is_extrusion_a) {
Logger::Root().Notice("GEO", 136, "Operand A 1/1 is an extrusion");
settings.log().Notice("GEO", 136, "Operand A 1/1 is an extrusion");
NCollection_List<TopoDS_Shape>::Iterator it(b);
for (int nb = 1; it.More(); it.Next(), ++nb) {
@@ -1064,10 +1064,10 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
}
if (is_extrusion_b) {
Logger::Root().Notice("GEO", 137, "Operand B " + std::to_string(nb) + "/" + std::to_string(b.Extent()) + " is an extrusion");
settings.log().Notice("GEO", 137, "Operand B " + std::to_string(nb) + "/" + std::to_string(b.Extent()) + " is an extrusion");
if (b_interval.first < a_interval.first + (fuzz * 100.) && b_interval.second > a_interval.second - (fuzz * 100.)) {
Logger::Root().Notice("GEO", 138, "Operand B creates a through hole");
settings.log().Notice("GEO", 138, "Operand B creates a through hole");
// Align b with a operand
gp_Trsf trsf;
@@ -1091,7 +1091,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
PERF("boolean operation: 2d builder");
// First try using face builder
boolean_op_2d_success = boolean_subtraction_2d_using_builder(a_face, b_faces, face_result, fuzziness);
boolean_op_2d_success = boolean_subtraction_2d_using_builder(a_face, b_faces, face_result, fuzziness, settings.log());
}
if (!boolean_op_2d_success) {
@@ -1107,23 +1107,23 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
BRepPrimAPI_MakePrism mp(face_result, gp_Vec(gp::DY()) * (a_interval.second - a_interval.first));
if (mp.IsDone()) {
if (b_remainder_3d.Extent()) {
Logger::Root().Notice("GEO", 139, std::to_string(b_remainder_3d.Extent()) + " operands remaining to process in 3D");
settings.log().Notice("GEO", 139, std::to_string(b_remainder_3d.Extent()) + " operands remaining to process in 3D");
b = b_remainder_3d;
s1s.Clear();
s1s.Append(mp.Shape());
} else {
Logger::Root().Notice("GEO", 140, "Processed fully in 2D");
settings.log().Notice("GEO", 140, "Processed fully in 2D");
result = mp.Shape();
return true;
}
} else {
Logger::Root().Notice("GEO", 141, "Failed to extrude 2D boolean result. Retrying in 3D.");
settings.log().Notice("GEO", 141, "Failed to extrude 2D boolean result. Retrying in 3D.");
}
} else {
Logger::Root().Notice("GEO", 142, "Failed to perform 2D boolean operation. Retrying in 3D.");
settings.log().Notice("GEO", 142, "Failed to perform 2D boolean operation. Retrying in 3D.");
}
} else {
Logger::Root().Notice("GEO", 143, "No second operands can be processed as 2D inner bounds. Retrying in 3D.");
settings.log().Notice("GEO", 143, "No second operands can be processed as 2D inner bounds. Retrying in 3D.");
}
}
}
@@ -1145,7 +1145,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
}
if (builder->IsDone()) {
if (false && builder->DSFiller()->HasWarning(STANDARD_TYPE(BOPAlgo_AlertAcquiredSelfIntersection))) {
Logger::Root().Notice("GEO", 144, "Builder reports self-intersection in output");
settings.log().Notice("GEO", 144, "Builder reports self-intersection in output");
success = false;
/*
@@ -1159,7 +1159,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
}
*/
} else if(builder->DSFiller()->HasWarning(STANDARD_TYPE(BOPAlgo_AlertBadPositioning)) && !TopoDS_Iterator(*builder).More()) {
Logger::Root().Notice("GEO", 145, "Builder reports bad positioning and result is empty");
settings.log().Notice("GEO", 145, "Builder reports bad positioning and result is empty");
success = false;
} else {
TopoDS_Shape r = *builder;
@@ -1173,7 +1173,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
fix.Perform();
r = fix.Shape();
} catch (...) {
Logger::Root().Error("GEO", 146, "Shape healing failed on boolean result");
settings.log().Error("GEO", 146, "Shape healing failed on boolean result");
}
}
@@ -1184,7 +1184,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
success = ana.IsValid() != 0;
if (!success) {
Logger::Root().Notice("GEO", 147, "Boolean operation yields invalid result");
settings.log().Notice("GEO", 147, "Boolean operation yields invalid result");
std::stringstream str;
bool any_emitted = false;
@@ -1214,7 +1214,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
dump(r);
Logger::Root().Notice("GEO", 148, str.str());
settings.log().Notice("GEO", 148, str.str());
}
}
@@ -1334,7 +1334,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
if (op == BOPAlgo_CUT && has_open_shells && all_faces_included_in_result && result_n_faces > first_op_n_faces) {
success = false;
Logger::Root().Notice("GEO", 149, "Boolean result discarded because subtractions results in only the addition of faces");
settings.log().Notice("GEO", 149, "Boolean result discarded because subtractions results in only the addition of faces");
} else {
// when there are edges or vertex-edge distances close to the used fuzziness, the
// output is not trusted and the operation is attempted with a higher fuzziness.
@@ -1380,7 +1380,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
static const char* const reason_strings[] = { "edge length", "vertex-edge", "face-face" };
std::stringstream str;
str << "Boolean operation result failing " << reason_strings[reason] << " interference check, with fuzziness " << fuzziness << " with length " << v;
Logger::Root().Notice("GEO", 150, str.str());
settings.log().Notice("GEO", 150, str.str());
}
}
@@ -1389,7 +1389,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
}
} else {
Logger::Root().Notice("GEO", 151, "Boolean operation yields non-manifold result");
settings.log().Notice("GEO", 151, "Boolean operation yields non-manifold result");
}
}
}
@@ -1399,7 +1399,7 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
#if OCC_VERSION_HEX >= 0x70200
if (builder->HasError(STANDARD_TYPE(BOPAlgo_AlertBOPNotAllowed))) {
Logger::Root().Error("GEO", 152, "Invalid operands. Using first operand");
settings.log().Error("GEO", 152, "Invalid operands. Using first operand");
result = a;
success = true;
}
@@ -1412,14 +1412,14 @@ bool IfcGeom::util::boolean_operation(const boolean_settings& settings, const To
#endif
std::string str_str = str.str();
if (str_str.size()) {
Logger::Root().Notice("GEO", 153, str_str);
settings.log().Notice("GEO", 153, str_str);
}
}
if (!success) {
if (allow_retry) {
return boolean_operation(settings, a, b, op, result, new_fuzziness);
} else {
Logger::Root().Notice("GEO", 154, "No longer attempting boolean operation with higher fuzziness");
settings.log().Notice("GEO", 154, "No longer attempting boolean operation with higher fuzziness");
}
}
return success && !result.IsNull();