From d188e3beaf2f79a3ca82afc3a1d43524e53abd3b Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Sat, 11 Jul 2026 15:07:06 +0100 Subject: [PATCH 1/4] Allow process/resource type assignment via Type-suffix convention The class-pairing validation added in 10ee5aef4f rejects any type assignment whose class isn't in the buildingSMART implementer agreement map. That map only covers physical product occurrence/type pairs (IfcWallType -> IfcWall, etc); IfcTypeProcess and IfcTypeResource subtypes such as IfcTaskType, IfcProcedureType and the resource types have no entry, so previously-valid assignments like IfcTaskType -> IfcTask were rejected with "allowed occurrence classes: ". These classes still follow the schema's universal Type-suffix naming convention, so derive the pairing the same way the existing ApplicableOccurrence fallback does: strip "Type" from the relating type's class name and accept it only if the schema actually declares that entity. This can only add pairings implied by the type's own class name, so it cannot loosen the existing rejection of genuine mismatches (e.g. IfcWallType -> IfcWindow). Generated with the assistance of an AI coding tool. --- .../ifcopenshell/api/type/assign_type.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/ifcopenshell-python/ifcopenshell/api/type/assign_type.py b/src/ifcopenshell-python/ifcopenshell/api/type/assign_type.py index 3087811234..54723c754e 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/type/assign_type.py +++ b/src/ifcopenshell-python/ifcopenshell/api/type/assign_type.py @@ -196,13 +196,25 @@ class Usecase: allowed_occurrences = set( ifcopenshell.util.type.get_applicable_entities(relating_type.is_a(), schema=self.file.schema) ) + schema = ifcopenshell.schema_by_name(self.file.schema) + # The implementer agreement map has no entry for the abstract # IfcTypeProduct, which Bonsai uses for annotation types. The schema # itself defines IfcTypeProduct.ApplicableOccurrence for exactly this # purpose, so honor it when the leading class token is a valid entity. if applicable_occurrence := getattr(relating_type, "ApplicableOccurrence", None): occurrence_class = applicable_occurrence.split("/", 1)[0] - schema = ifcopenshell.schema_by_name(self.file.schema) + try: + schema.declaration_by_name(occurrence_class) + allowed_occurrences.add(occurrence_class) + except RuntimeError: + pass + # The map only covers physical product occurrence/type pairs (e.g. + # IfcWallType -> IfcWall). Process and resource types (IfcTaskType, + # IfcCrewResourceType, ...) aren't in it, but the schema's universal + # Type-suffix naming convention gives the same pairing directly. + if (type_class := relating_type.is_a()).endswith("Type"): + occurrence_class = type_class[: -len("Type")] try: schema.declaration_by_name(occurrence_class) allowed_occurrences.add(occurrence_class) From 96e2efebc86887d68fec9bf3ca971771b63d61af Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Sat, 11 Jul 2026 19:05:56 +0100 Subject: [PATCH 2/4] Route boolean-op kernel logging through the injected Logger 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. --- .../kernels/opencascade/OpenCascadeKernel.cpp | 15 ++-- .../kernels/opencascade/boolean_result.cpp | 7 +- .../kernels/opencascade/boolean_utils.cpp | 76 +++++++++---------- .../kernels/opencascade/boolean_utils.h | 11 ++- 4 files changed, 59 insertions(+), 50 deletions(-) diff --git a/src/ifcgeom/kernels/opencascade/OpenCascadeKernel.cpp b/src/ifcgeom/kernels/opencascade/OpenCascadeKernel.cpp index 0c8987a225..5a8db5346d 100644 --- a/src/ifcgeom/kernels/opencascade/OpenCascadeKernel.cpp +++ b/src/ifcgeom/kernels/opencascade/OpenCascadeKernel.cpp @@ -48,6 +48,7 @@ bool IfcGeom::OpenCascadeKernel::convert_openings(const IfcUtil::IfcBaseEntity* bst.attempt_2d = settings_.get().get(); bst.debug = settings_.get().get(); bst.precision = settings_.get().get(); + bst.logger = &logger_; std::vector< std::pair > opening_vector; @@ -118,7 +119,7 @@ bool IfcGeom::OpenCascadeKernel::convert_openings(const IfcUtil::IfcBaseEntity* auto it3_shape = std::static_pointer_cast(it3->Shape())->shape(); if (it3_shape.IsNull()) { - Logger::Root().Error("GEO", 187, "Null operand"); + logger_.Error("GEO", 187, "Null operand"); continue; } @@ -143,7 +144,7 @@ bool IfcGeom::OpenCascadeKernel::convert_openings(const IfcUtil::IfcBaseEntity* IfcGeom::util::create_solid_from_faces(list, entity_part, settings_.get().get(), true); is_manifold = util::is_manifold(entity_part); if (is_manifold) { - Logger::Root().Warning("GEO", 188, "Successfully sewed non-manifold first operand"); + logger_.Warning("GEO", 188, "Successfully sewed non-manifold first operand"); } } @@ -161,17 +162,17 @@ bool IfcGeom::OpenCascadeKernel::convert_openings(const IfcUtil::IfcBaseEntity* failure = "Empty result (no faces) for BOPAlgo_MakerVolume; original was " + std::to_string(IfcGeom::util::count(entity_part, TopAbs_FACE)); } else { is_manifold = util::is_manifold(entity_part_2); - Logger::Root().Warning("GEO", 189, std::string("Sucessfully detected exterior volume to non-manifold first operand; shape is now ") + (is_manifold ? std::string("manifold") : std::string("non-manifold"))); + logger_.Warning("GEO", 189, std::string("Sucessfully detected exterior volume to non-manifold first operand; shape is now ") + (is_manifold ? std::string("manifold") : std::string("non-manifold"))); entity_part = entity_part_2; } } catch (const Standard_Failure& e) { failure.emplace(e.GetMessageString()); } if (failure) { - Logger::Root().Warning("GEO", 190, "MakeVolume failed: " + *failure, entity); + logger_.Warning("GEO", 190, "MakeVolume failed: " + *failure, entity); } } else { - Logger::Root().Warning("GEO", 191, "Non-manifold first operand, use --make-volume to try and make manifold"); + logger_.Warning("GEO", 191, "Non-manifold first operand, use --make-volume to try and make manifold"); } } @@ -214,7 +215,7 @@ bool IfcGeom::OpenCascadeKernel::convert_openings(const IfcUtil::IfcBaseEntity* if (util::boolean_operation(bst, result, opening_list, BOPAlgo_CUT, intermediate_result)) { result = intermediate_result; } else { - Logger::Root().Message(Logger::LOG_ERROR, "GEO", 192, "Opening subtraction failed for " + boost::lexical_cast(std::distance(jt, it)) + " openings", entity); + logger_.Message(Logger::LOG_ERROR, "GEO", 192, "Opening subtraction failed for " + boost::lexical_cast(std::distance(jt, it)) + " openings", entity); } jt = it; @@ -235,7 +236,7 @@ bool IfcGeom::OpenCascadeKernel::convert_openings(const IfcUtil::IfcBaseEntity* // where we keep the first operand as is (a compound of faces probably, // unless --orient-shells was activated in which case we're already lost). if (!is_manifold) { - Logger::Root().Warning("GEO", 193, "Retrying boolean operation on individual faces"); + logger_.Warning("GEO", 193, "Retrying boolean operation on individual faces"); } continue; } diff --git a/src/ifcgeom/kernels/opencascade/boolean_result.cpp b/src/ifcgeom/kernels/opencascade/boolean_result.cpp index 64ce43eca1..600a9be96d 100644 --- a/src/ifcgeom/kernels/opencascade/boolean_result.cpp +++ b/src/ifcgeom/kernels/opencascade/boolean_result.cpp @@ -118,14 +118,14 @@ bool OpenCascadeKernel::convert_impl(const taxonomy::boolean_result::ptr br, Con const double first_operand_volume = util::shape_volume(a); if (first_operand_volume <= ALMOST_ZERO) { - Logger::Root().Message(Logger::LOG_WARNING, "GEO", 119, "Empty solid for:", c->instance); + logger_.Message(Logger::LOG_WARNING, "GEO", 119, "Empty solid for:", c->instance); } } else { for (auto& r : cr) { auto S = std::static_pointer_cast(r.Shape())->shape(); if (S.IsNull()) { - Logger::Root().Error("GEO", 120, "Null operand"); + logger_.Error("GEO", 120, "Null operand"); continue; } gp_GTrsf trsf; @@ -140,7 +140,7 @@ bool OpenCascadeKernel::convert_impl(const taxonomy::boolean_result::ptr br, Con // #2665 we also set a precision-independent threshold, because in the boolean op routine // the working fuzziness might still be increased. if (d < tol * 20. || d < 0.00002) { - Logger::Root().Message(Logger::LOG_WARNING, "GEO", 121, "Halfspace subtraction yields unchanged volume:", c->instance); + logger_.Message(Logger::LOG_WARNING, "GEO", 121, "Halfspace subtraction yields unchanged volume:", c->instance); continue; } else { S = result; @@ -159,6 +159,7 @@ bool OpenCascadeKernel::convert_impl(const taxonomy::boolean_result::ptr br, Con bst.attempt_2d = settings_.get().get(); bst.debug = settings_.get().get(); bst.precision = settings_.get().get(); + bst.logger = &logger_; TopoDS_Shape r; diff --git a/src/ifcgeom/kernels/opencascade/boolean_utils.cpp b/src/ifcgeom/kernels/opencascade/boolean_utils.cpp index 7695be65bc..d104c7ff94 100644 --- a/src/ifcgeom/kernels/opencascade/boolean_utils.cpp +++ b/src/ifcgeom/kernels/opencascade/boolean_utils.cpp @@ -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& bs, NCollection_List & c) { +int IfcGeom::util::eliminate_narrow_operands(double prec, const NCollection_List& bs, NCollection_List & c, Logger& logger) { int N = 0; NCollection_List::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 & b_input, TopoDS_Shape & result, double eps) { +bool IfcGeom::util::boolean_subtraction_2d_using_builder(const TopoDS_Shape & a_input, const NCollection_List & b_input, TopoDS_Shape & result, double eps, Logger& logger) { IfcGeom::impl::tree edge_tree; NCollection_List 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::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::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::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(); diff --git a/src/ifcgeom/kernels/opencascade/boolean_utils.h b/src/ifcgeom/kernels/opencascade/boolean_utils.h index 34bf4906b8..55bd960130 100644 --- a/src/ifcgeom/kernels/opencascade/boolean_utils.h +++ b/src/ifcgeom/kernels/opencascade/boolean_utils.h @@ -35,6 +35,7 @@ #include +#include "../../../ifcparse/IfcLogger.h" #include "../ifc_geomlibrary_api.h" namespace IfcGeom { @@ -88,13 +89,19 @@ namespace IfcGeom { int eliminate_touching_operands(double prec, const TopoDS_Shape& a, const NCollection_List& bs, NCollection_List& c); - int eliminate_narrow_operands(double prec, const NCollection_List& bs, NCollection_List & c); + int eliminate_narrow_operands(double prec, const NCollection_List& bs, NCollection_List & c, Logger& logger = Logger::Root()); - bool boolean_subtraction_2d_using_builder(const TopoDS_Shape& a_input, const NCollection_List& b_input, TopoDS_Shape& result, double eps); + bool boolean_subtraction_2d_using_builder(const TopoDS_Shape& a_input, const NCollection_List& b_input, TopoDS_Shape& result, double eps, Logger& logger = Logger::Root()); struct boolean_settings { bool debug, attempt_2d; double precision; + // Set by callers that carry a per-conversion Logger (e.g. kernels deriving + // from AbstractKernel). Falls back to the global Logger::Root() singleton, + // which IfcConvert never wires to its --log-file output, so messages logged + // through that fallback are effectively silently dropped. + Logger* logger = nullptr; + Logger& log() const { return logger ? *logger : Logger::Root(); } }; bool boolean_operation(const boolean_settings& settings, const TopoDS_Shape&, const NCollection_List&, BOPAlgo_Operation, TopoDS_Shape&, double fuzziness = -1.); From 6c590bf0082b3c56a9cbca30c60fec376eb89a93 Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Sat, 11 Jul 2026 19:44:56 +0100 Subject: [PATCH 3/4] Fix schema mismatch in ColumnPSetsOfSets.ifc test fixture The fixture declared FILE_SCHEMA(('IFC2X3')) but used IFCPROPERTYSETDEFINITIONSET(...), a defined type that only exists in IFC4+ (confirmed absent from the generated Ifc2x3-schema.cpp/ Ifc2x3-definitions.h, present in the IFC4 equivalents). The file's own FILE_NAME record ('Column_4x3.ifc') suggests it was originally exported as IFC4X3 and the schema tag was later miscopied to IFC2X3. Traced with an instrumented parser build: on encountering the unrecognized keyword, declaration_by_name() correctly throws "Entity with name 'IFCPROPERTYSETDEFINITIONSET' not found in schema 'IFC2X3'", caught by the existing IfcException handler in in_memory_file_storage::load(). The parser then falls back to parsing the trailing (#136,#138) as a plain nested SET rather than the typed value, so RelatingPropertyDefinition ends up as a bare tuple instead of an IfcPropertySetDefinitionSet-wrapped value with .is_a(). This is correct, expected behavior for content that doesn't match its declared schema - not a parser bug. Fixing the header to IFC4 (which does declare the type) resolves test_stream, test_file, and test_rocks in test_streaming_rocksdb_and_simpletyperefs.py. Generated with the assistance of an AI coding tool. --- src/ifcopenshell-python/test/fixtures/ColumnPSetsOfSets.ifc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcopenshell-python/test/fixtures/ColumnPSetsOfSets.ifc b/src/ifcopenshell-python/test/fixtures/ColumnPSetsOfSets.ifc index f124bb79db..0ab5ace614 100644 --- a/src/ifcopenshell-python/test/fixtures/ColumnPSetsOfSets.ifc +++ b/src/ifcopenshell-python/test/fixtures/ColumnPSetsOfSets.ifc @@ -2,7 +2,7 @@ ISO-10303-21; HEADER; FILE_DESCRIPTION(('ViewDefinition [CoordinationView]','RevitIdentifiers [ContentGUID: a0df3484-2dab-42c5-b806-8c10d313bee0, VersionGUID: 658c1394-f3a4-43d1-9b3c-eee44a0cd67a, NumberOfSaves: 2]','CoordinateReference [CoordinateBase: Shared Coordinates]'),'2;1'); FILE_NAME('Column_4x3.ifc','2025-03-12T13:53:30+00:00',(''),(''),'ODA SDAI 24.12','Autodesk Revit 25.4.0.32 (ENG) - IFC 25.4.0.32',''); -FILE_SCHEMA(('IFC2X3')); +FILE_SCHEMA(('IFC4')); ENDSEC; DATA; #1=IFCORGANIZATION($,'Autodesk Revit 2025 (ENG)',$,$,$); From 489084c7be6955daeb48d9084c9b30cd9d0472fc Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Mon, 13 Jul 2026 20:03:16 +0100 Subject: [PATCH 4/4] Remove stale ty lint ignore directive --- src/ifcopenshell-python/ifcopenshell/draw.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcopenshell-python/ifcopenshell/draw.py b/src/ifcopenshell-python/ifcopenshell/draw.py index bbcfa48aea..235dd1416e 100644 --- a/src/ifcopenshell-python/ifcopenshell/draw.py +++ b/src/ifcopenshell-python/ifcopenshell/draw.py @@ -541,7 +541,7 @@ def main( arranged = W.arrange_polygons( *filter(None, (ARRANGE_POLYGON_SETTINGS,)), - polies, # ty: ignore[too-many-positional-arguments] + polies, *((logger,) if logger is not None else ()), ) svg_data_3 = W.polygons_to_svg(arranged, False)