From b9deb9c63d223935e2420019f839404a3117f000 Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Sat, 11 Jul 2026 13:10:14 +0100 Subject: [PATCH 01/14] Git ignores CLAUDE.local.md file This allows a file that will be automatically picked up by Claude. It can either be a copy of a CLAUDE.md, or a one line file pointing to a shared common file. i.e. @~/.claude/conventions-ifcopenshell.md --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a5c719cbb3..656b9d0524 100644 --- a/.gitignore +++ b/.gitignore @@ -127,6 +127,7 @@ src/ifcopenshell-python/ifcopenshell/express/*.exp.cache.dat # temp files from AI coding tools *.claude +CLAUDE.local.md *.py.tmp* *.json.tmp* From 438c0955f2268f798d3ca592c63dd50550b34b65 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 22:48:46 +0300 Subject: [PATCH 02/14] Map IfcAsymmetricIShapeProfileDef standalone in IFC4+ (#1367) In IFC2X3 IfcAsymmetricIShapeProfileDef is a subtype of IfcIShapeProfileDef, so the IfcIShapeProfileDef mapping dispatched it by inheritance. From IFC4 onwards it is a standalone subtype of IfcParameterizedProfileDef, so nothing mapped it and the extruded solid came out empty (GEO326, 0 verts). Add a dedicated map_impl that builds the twelve-point asymmetric section (independent bottom/top flange widths, thicknesses, fillet/edge radii and flange slopes), plus a guarded BIND. Both are wrapped in SCHEMA_IfcAsymmetricIShapeProfileDef_HAS_BottomFlangeWidth, which is only defined where the type is standalone, so IFC2X3 keeps its existing subtype route unchanged. Verified on OCC 7.9.2: an IFC4 asymmetric extrusion goes from 0 verts to a correct 72-vert solid (bottom flange wider than top); IFC2X3 output is unchanged. Co-Authored-By: Claude Opus 4.8 --- .../mapping/IfcAsymmetricIShapeProfileDef.cpp | 92 +++++++++++++++++++ src/ifcgeom/mapping/mapping.i | 6 +- 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp diff --git a/src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp b/src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp new file mode 100644 index 0000000000..dce783f978 --- /dev/null +++ b/src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp @@ -0,0 +1,92 @@ +/******************************************************************************** + * * + * This file is part of IfcOpenShell. * + * * + * IfcOpenShell is free software: you can redistribute it and/or modify * + * it under the terms of the Lesser GNU General Public License as published by * + * the Free Software Foundation, either version 3.0 of the License, or * + * (at your option) any later version. * + * * + * IfcOpenShell is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * Lesser GNU General Public License for more details. * + * * + * You should have received a copy of the Lesser GNU General Public License * + * along with this program. If not, see . * + * * + ********************************************************************************/ + +#include "mapping.h" +#define mapping POSTFIX_SCHEMA(mapping) +using namespace ifcopenshell::geometry; + +#include "../profile_helper.h" + +// In IFC2X3 IfcAsymmetricIShapeProfileDef is a subtype of IfcIShapeProfileDef and is +// therefore dispatched (and handled) by the IfcIShapeProfileDef mapping. From IFC4 +// onwards it is a standalone subtype of IfcParameterizedProfileDef with its own +// Bottom*/Top* attributes, so nothing mapped it and the extrusion came out empty. +// The presence of the standalone BottomFlangeWidth attribute is the discriminator: +// it is only defined in the schemas where the type is standalone (IFC4 / IFC4X3). +#ifdef SCHEMA_IfcAsymmetricIShapeProfileDef_HAS_BottomFlangeWidth + +taxonomy::ptr mapping::map_impl(const IfcSchema::IfcAsymmetricIShapeProfileDef* inst) { + // Bottom flange (half width), overall depth (half), web (half thickness). + const double xb = inst->BottomFlangeWidth() / 2.0 * length_unit_; + const double xt = inst->TopFlangeWidth() / 2.0 * length_unit_; + const double y = inst->OverallDepth() / 2.0 * length_unit_; + const double d1 = inst->WebThickness() / 2.0 * length_unit_; + + // Bottom flange thickness; top flange thickness defaults to the bottom one. + const double ftb = inst->BottomFlangeThickness() * length_unit_; + const double ftt = inst->TopFlangeThickness().get_value_or(inst->BottomFlangeThickness()) * length_unit_; + + // Optional fillet radii (web/flange transition) and flange edge radii. + const double fb = inst->BottomFlangeFilletRadius().get_value_or(0.) * length_unit_; + const double ft_top = inst->TopFlangeFilletRadius().get_value_or(0.) * length_unit_; + const double feb = inst->BottomFlangeEdgeRadius().get_value_or(0.) * length_unit_; + const double fet = inst->TopFlangeEdgeRadius().get_value_or(0.) * length_unit_; + + // Optional flange slopes: the inner edge of the flange rises towards the web. + const double bottomSlope = inst->BottomFlangeSlope().get_value_or(0.) * angle_unit_; + const double topSlope = inst->TopFlangeSlope().get_value_or(0.) * angle_unit_; + const double dyb = (xb - d1) * tan(bottomSlope); + const double dyt = (xt - d1) * tan(topSlope); + + const double tol = settings_.get().get(); + + if (xb < tol || xt < tol || y < tol || d1 < tol || ftb < tol || ftt < tol) { + logger_.Message(Logger::LOG_NOTICE, "GEO", 264, "Skipping zero sized profile:", inst); + return nullptr; + } + + taxonomy::matrix4::ptr m4; + bool has_position = true; +#ifdef SCHEMA_IfcParameterizedProfileDef_Position_IS_OPTIONAL + has_position = !!inst->Position(); +#endif + if (has_position) { + m4 = taxonomy::cast(map(inst->Position())); + } + + // Twelve corner points, running counter-clockwise from the bottom-left, with the + // bottom flange (xb) possibly wider than the top flange (xt). Fillet/edge radii are + // attached to the corner they round, matching the symmetric IfcIShapeProfileDef. + return profile_helper(m4, { + {{-xb,-y}}, + {{xb,-y}}, + {{xb,-y + ftb}, {feb}}, + {{d1,-y + ftb + dyb},{fb} }, + {{d1,y - ftt - dyt},{ft_top} }, + {{xt,y - ftt}, {fet}}, + {{xt,y}}, + {{-xt,y}}, + {{-xt,y - ftt}, {fet}}, + {{-d1,y - ftt - dyt},{ft_top} }, + {{-d1,-y + ftb + dyb},{fb} }, + {{-xb,-y + ftb}, {feb}} + }); +} + +#endif diff --git a/src/ifcgeom/mapping/mapping.i b/src/ifcgeom/mapping/mapping.i index c67d0f7f4a..8766238dc3 100644 --- a/src/ifcgeom/mapping/mapping.i +++ b/src/ifcgeom/mapping/mapping.i @@ -89,7 +89,11 @@ BIND(IfcRectangleHollowProfileDef); BIND(IfcRectangleProfileDef); BIND(IfcTrapeziumProfileDef); BIND(IfcCShapeProfileDef); -// IfcAsymmetricIShapeProfileDef included +// In IFC2X3 IfcAsymmetricIShapeProfileDef is a subtype of IfcIShapeProfileDef and is +// mapped by it; from IFC4 onwards it is a standalone type and needs its own binding. +#ifdef SCHEMA_IfcAsymmetricIShapeProfileDef_HAS_BottomFlangeWidth +BIND(IfcAsymmetricIShapeProfileDef); +#endif BIND(IfcIShapeProfileDef); BIND(IfcLShapeProfileDef); BIND(IfcTShapeProfileDef); From a8d0ef3437c7d15978bd5a7a46a1ba3c8811f65f Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Sat, 11 Jul 2026 15:16:06 +0300 Subject: [PATCH 03/14] Add AI-generated marker to IfcAsymmetricIShapeProfileDef.cpp Comply with AGENTS.md: new AI-generated files must carry a top-of-file comment indicating AI assistance. Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Opus 4.8 --- src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp b/src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp index dce783f978..c00209e13c 100644 --- a/src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp +++ b/src/ifcgeom/mapping/IfcAsymmetricIShapeProfileDef.cpp @@ -1,3 +1,4 @@ +// This file was generated with the assistance of an AI coding tool. /******************************************************************************** * * * This file is part of IfcOpenShell. * From 061bb90d50de164ee001b24f9aaf25abce810d2e Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Sat, 11 Jul 2026 09:12:18 +0300 Subject: [PATCH 04/14] Warn when a face inner boundary intersects another boundary (#527) A face whose inner boundary crosses the outer boundary (or another inner boundary) is invalid per the schema. Open Cascade silently heals or drops such a face, so the intended hole is lost or the face is corrupted with no diagnostic at all (the 2018 report saw a dropped face; on the current line the face survives as wrong geometry, still silently). After the wires are collected, if a face has inner boundaries, measure the BRepExtrema distance between each inner wire and every earlier wire. Two non intersecting loops have strictly positive distance, so a distance at or below the modelling precision means the boundaries touch or cross; emit a warning (GEO 402) naming the offending face. This is diagnostic only, no geometry change. The message is emitted via the kernel logger() rather than Logger::Root(): IfcConvert configures a local Logger and worker logs merge into it, while Logger::Root() is a separate unconfigured singleton whose messages are discarded (a latent issue affecting some existing GEO messages too). Verified on OCC 7.9.2 with synthesized IFC4 faces: an inner triangle crossing the outer edge, and one straddling the bottom edge, each emit one GEO 402; a valid 4x4 hole emits none and triangulates identically (area 84.0), in both sequential and multithreaded runs. Pure inner self intersection and full containment are distinct classes and intentionally left untouched. Co-Authored-By: Claude Opus 4.8 --- src/ifcgeom/kernels/opencascade/face.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/ifcgeom/kernels/opencascade/face.cpp b/src/ifcgeom/kernels/opencascade/face.cpp index 024ff0f13b..f300984728 100644 --- a/src/ifcgeom/kernels/opencascade/face.cpp +++ b/src/ifcgeom/kernels/opencascade/face.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -356,6 +357,27 @@ bool OpenCascadeKernel::convert(const taxonomy::face::ptr face, TopoDS_Shape& re return false; } + // #527: A face whose inner boundary intersects the outer boundary (or + // another inner boundary) is invalid per the schema. Open Cascade heals or + // drops such a face silently, so the intended hole is lost with no + // diagnostic. The distance between two non-intersecting loops is strictly + // positive; a distance at (or below) the modelling precision means the + // boundaries touch or cross. Emit a clear warning so the invalid input is + // not silently lost. wires() is ordered outer-first, inner-bounds after. + if (fd.wires().size() > 1) { + const auto& fwires = fd.wires(); + bool reported = false; + for (size_t i = 1; i < fwires.size() && !reported; ++i) { + for (size_t j = 0; j < i && !reported; ++j) { + BRepExtrema_DistShapeShape dss(fwires[i], fwires[j]); + if (dss.IsDone() && dss.Value() < precision_) { + logger().Warning("GEO", 402, "Face inner boundary intersects another face boundary", face->instance); + reported = true; + } + } + } + } + if (fd.surface().IsNull()) { // Use the first wire to find a plane manually for polygonal wires const TopoDS_Wire& wire = fd.wires().front(); From eb7324e7fc077cd3c18a629194069f6c29ca8b8e Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 20:38:15 +0300 Subject: [PATCH 05/14] IfcConvert: add --fail-on-error to exit non-zero when conversion logs errors (#1118) IfcConvert returned a success exit code even when geometry conversion logged errors and silently dropped elements (for example a failed TopoDS::Shell build under layerset slicing produced valid looking output with most objects missing), so CI and scripts could not detect a partial conversion. Add an opt-in --fail-on-error flag that makes IfcConvert exit non-zero when any error was logged during processing, reusing the existing MaxSeverity based failure check already used for --validate. The default exit behaviour is unchanged, so pipelines that tolerate individual element failures are unaffected. Co-Authored-By: Claude Opus 4.8 --- src/ifcconvert/IfcConvert.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp index acd5421638..38d245757c 100644 --- a/src/ifcconvert/IfcConvert.cpp +++ b/src/ifcconvert/IfcConvert.cpp @@ -252,6 +252,10 @@ int main(int argc, char** argv) { ("stderr-progress", "output progress to stderr stream") ("yes,y", "answer 'yes' automatically to possible confirmation queries (e.g. overwriting an existing output file)") ("no-progress", "suppress possible progress bar type of prints that use carriage return") + ("fail-on-error", "return a non-zero exit code when one or more errors were logged during " + "geometry conversion (e.g. an element failed to convert). By default IfcConvert exits " + "successfully as long as an output file could be written, even if some elements were " + "silently dropped. Enable this flag so scripts and CI can detect partial conversions.") ("log-format", po::value(&log_format), "log format: plain or json") ("log-file", new po::typed_value(&log_file), "redirect log output to file"); @@ -449,6 +453,7 @@ int main(int argc, char** argv) { const bool mmap = vmap.count("mmap") != 0; const bool no_progress = vmap.count("no-progress") != 0; + const bool fail_on_error = vmap.count("fail-on-error") != 0; const bool quiet = vmap.count("quiet") != 0; const bool stderr_progress = vmap.count("stderr-progress") != 0; @@ -1220,6 +1225,11 @@ int main(int argc, char** argv) { successful = false; } + if (fail_on_error && logger.MaxSeverity() >= Logger::LOG_ERROR) { + logger.Error("SYS", 26, "Errors encountered during processing, failing due to --fail-on-error."); + successful = false; + } + if (logger.Verbosity() == Logger::LOG_PERF) { logger.PrintPerformanceStats(); } From dd9fa65629a378ba1818b773a926586044a2d25d Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Tue, 7 Jul 2026 17:54:21 +0300 Subject: [PATCH 06/14] Fix cgal kernel under-tessellating large-radius arcs (#8051) The CGAL kernels (cgal and cgal-simple) allocate arc segments as a fraction of the full circle via CircleSegments, ignoring the radius. A large-radius arc that spans a small angle therefore collapsed to a single chord, turning curved curtain-wall mullions straight while the OpenCascade kernel (which meshes by deflection) kept them curved. evaluate_conic now also enforces a deflection-based floor on the number of segments, keeping the chord deviation within mesher-linear-deflection, matching OpenCascade. Small circles are unchanged (CircleSegments floor still dominates); only large-radius curves get denser. Co-Authored-By: Claude Opus 4.8 --- src/ifcgeom/kernels/cgal/CgalKernel.cpp | 28 ++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/kernels/cgal/CgalKernel.cpp b/src/ifcgeom/kernels/cgal/CgalKernel.cpp index f0747338d3..be109dc407 100644 --- a/src/ifcgeom/kernels/cgal/CgalKernel.cpp +++ b/src/ifcgeom/kernels/cgal/CgalKernel.cpp @@ -391,6 +391,11 @@ namespace { } }; + // Representative radius used to size the polygonal approximation of a conic. + // For an ellipse the larger semi-axis is the conservative choice. + inline double conic_radius(const taxonomy::circle::ptr& c) { return c->radius; } + inline double conic_radius(const taxonomy::ellipse::ptr& e) { return e->radius > e->radius2 ? e->radius : e->radius2; } + struct cgal_curve_creation_visitor { Settings& settings_; parameter_range param; @@ -425,7 +430,28 @@ namespace { if (b <= a) { b += 2 * M_PI; } - int num_segments = (int)std::ceil(std::fabs(a - b) / (2 * M_PI) * settings_.get().get()); + const double span = std::fabs(a - b); + int num_segments = (int)std::ceil(span / (2 * M_PI) * settings_.get().get()); + // CircleSegments allocates segments as a fraction of the *full* circle and is + // radius-agnostic. A large-radius arc spanning a small angle therefore collapses + // to a single chord (issue #8051: curved curtain-wall mullions became straight in + // the CGAL kernels while OpenCascade, which meshes by deflection, kept them curved). + // Enforce a deflection-based floor so the chord deviation stays within the mesher's + // linear deflection, matching OpenCascade behaviour. + const double radius = conic_radius(t); + const double deflection = settings_.get().get(); + if (deflection > 0. && radius > deflection) { + const double max_segment_angle = 2.0 * std::acos(1.0 - deflection / radius); + if (max_segment_angle > 0.) { + const int num_segments_deflection = (int)std::ceil(span / max_segment_angle); + if (num_segments_deflection > num_segments) { + num_segments = num_segments_deflection; + } + } + } + if (num_segments < 1) { + num_segments = 1; + } double du = (b - a) / num_segments; taxonomy::point3 P; // @nb for loop is not inclusive of the both end points From 0d70812641824054c5d90ddadf6a1fb2529abb65 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Tue, 7 Jul 2026 22:11:34 +0300 Subject: [PATCH 07/14] Make CGAL circle-segments 0-default deflection-driven (rework #8368) Address maintainer request on #8368: instead of a deflection floor on top of a fixed CircleSegments count, use one mode or the other. When CircleSegments == 0 (the new default) the CGAL kernel derives the conic segment count from MesherLinearDeflection, matching the deflection based meshing OpenCascade already does and fixing #8051. When CircleSegments is non zero it is used directly as a fixed, radius independent count. CircleSegments is only read by the CGAL kernel; OpenCascade meshes by deflection and never reads it, so the new default has no effect there. Update the setting description and the ifcconvert / geometry-settings docs. Co-Authored-By: Claude Opus 4.8 --- src/ifcgeom/ConversionSettings.h | 4 +- src/ifcgeom/kernels/cgal/CgalKernel.cpp | 40 +++++++++++-------- .../docs/ifcconvert/usage.rst | 8 +++- .../docs/ifcopenshell/geometry_settings.rst | 4 +- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/ifcgeom/ConversionSettings.h b/src/ifcgeom/ConversionSettings.h index febd789f6b..c022bfdd1c 100644 --- a/src/ifcgeom/ConversionSettings.h +++ b/src/ifcgeom/ConversionSettings.h @@ -361,8 +361,8 @@ namespace ifcopenshell { struct CircleSegments : public SettingBase { static constexpr const char* const name = "circle-segments"; - static constexpr const char* const description = "Number of segments to approximate full circles in CGAL kernel."; - static constexpr int defaultvalue = 16; + static constexpr const char* const description = "Number of segments to approximate full circles in the CGAL kernel. When 0 (the default) the segment count is derived from mesher-linear-deflection instead, so curves stay within the deflection tolerance regardless of radius."; + static constexpr int defaultvalue = 0; }; struct CgalSmoothAngleDegrees : public SettingBase { diff --git a/src/ifcgeom/kernels/cgal/CgalKernel.cpp b/src/ifcgeom/kernels/cgal/CgalKernel.cpp index be109dc407..f2d8a6c6b7 100644 --- a/src/ifcgeom/kernels/cgal/CgalKernel.cpp +++ b/src/ifcgeom/kernels/cgal/CgalKernel.cpp @@ -431,22 +431,30 @@ namespace { b += 2 * M_PI; } const double span = std::fabs(a - b); - int num_segments = (int)std::ceil(span / (2 * M_PI) * settings_.get().get()); - // CircleSegments allocates segments as a fraction of the *full* circle and is - // radius-agnostic. A large-radius arc spanning a small angle therefore collapses - // to a single chord (issue #8051: curved curtain-wall mullions became straight in - // the CGAL kernels while OpenCascade, which meshes by deflection, kept them curved). - // Enforce a deflection-based floor so the chord deviation stays within the mesher's - // linear deflection, matching OpenCascade behaviour. - const double radius = conic_radius(t); - const double deflection = settings_.get().get(); - if (deflection > 0. && radius > deflection) { - const double max_segment_angle = 2.0 * std::acos(1.0 - deflection / radius); - if (max_segment_angle > 0.) { - const int num_segments_deflection = (int)std::ceil(span / max_segment_angle); - if (num_segments_deflection > num_segments) { - num_segments = num_segments_deflection; - } + // CircleSegments controls how conics (circles, ellipses, arcs) are approximated + // in the CGAL kernel. Two modes, one or the other: + // - CircleSegments == 0 (the default): the segment count is derived from + // MesherLinearDeflection, so the chord deviation stays within the mesher's + // linear deflection regardless of radius. This matches the deflection based + // meshing the OpenCascade kernel already does and fixes issue #8051, where + // large radius arcs (curved curtain wall mullions) collapsed to straight chords + // because a fixed segment count is radius agnostic. + // - CircleSegments > 0: it is used directly as the number of segments for a full + // circle, giving deterministic, radius independent output. + int num_segments; + const int circle_segments = settings_.get().get(); + if (circle_segments > 0) { + num_segments = (int)std::ceil(span / (2 * M_PI) * circle_segments); + } else { + const double radius = conic_radius(t); + const double deflection = settings_.get().get(); + if (deflection > 0. && radius > deflection) { + const double max_segment_angle = 2.0 * std::acos(1.0 - deflection / radius); + num_segments = (int)std::ceil(span / max_segment_angle); + } else { + // Radius within the deflection tolerance (or no deflection set): a chord per + // quarter turn already keeps the deviation within tolerance. + num_segments = (int)std::ceil(span / (M_PI / 2.)); } } if (num_segments < 1) { diff --git a/src/ifcopenshell-python/docs/ifcconvert/usage.rst b/src/ifcopenshell-python/docs/ifcconvert/usage.rst index 3c6846d25b..6910e7e6fe 100644 --- a/src/ifcopenshell-python/docs/ifcconvert/usage.rst +++ b/src/ifcopenshell-python/docs/ifcconvert/usage.rst @@ -311,8 +311,12 @@ CLI Manual output. --force-space-transparency arg Overrides transparency of spaces in geometry output. - --circle-segments arg (= 16) Number of segments to approximate full - circles in CGAL kernel. + --circle-segments arg (= 0) Number of segments to approximate full + circles in the CGAL kernel. When 0 (the + default) the segment count is derived from + mesher-linear-deflection instead, so curves + stay within the deflection tolerance + regardless of radius. --cgal-smooth-angle-degrees arg (= -1) Angle in degrees under which adjacent facets will have averaged vertex diff --git a/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst b/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst index f73c7edc28..6862088c6c 100644 --- a/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst +++ b/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst @@ -228,10 +228,10 @@ circle-segments +------+-----------------------+---------+ | Type | IfcConvert Option | Default | +======+=======================+=========+ -| INT | ``--circle-segments`` | 16 | +| INT | ``--circle-segments`` | 0 | +------+-----------------------+---------+ -Number of segments to approximate full circles in CGAL kernel. +Number of segments to approximate full circles in the CGAL kernel. When 0 (the default) the segment count is derived from mesher-linear-deflection instead, so curves stay within the deflection tolerance regardless of radius. context-identifiers ^^^^^^^^^^^^^^^^^^^ From 7e3d2f936da147dcf28a6a21a8097a48cf0fc8bd Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 18:24:14 +0300 Subject: [PATCH 08/14] build: do not request the header-only Boost.System component (build against Boost 1.70+) Boost.System has been header-only since Boost 1.69 and its compiled stub library was removed in newer Boost, so listing system in the requested find_package components makes configuration fail on Boost 1.70 and up (for example Boost 1.90 errors with "Could not find boost_system"). Boost.System is still pulled in transitively by thread / iostreams where it is needed, so drop it from the explicit component list. Verified: with this change IfcOpenShell configures and builds IfcConvert cleanly against Homebrew Boost 1.90 and OpenCASCADE 7.9.2. Co-Authored-By: Claude Opus 4.8 --- cmake/CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmake/CMakeLists.txt b/cmake/CMakeLists.txt index f16b40c447..5f76c8dfcb 100644 --- a/cmake/CMakeLists.txt +++ b/cmake/CMakeLists.txt @@ -314,8 +314,12 @@ if(WASM_BUILD) else() # @todo review this, shouldn't this be all possible header-only now? # ... or rewritten using C++17 features? + # Boost.System has been header-only since 1.69 and its compiled stub library + # was dropped in newer Boost, so requesting it as a component makes + # find_package fail on Boost 1.70 and up (for example Boost 1.90). It is + # still pulled in transitively by thread / iostreams where needed, so do not + # request it explicitly. set(BOOST_COMPONENTS - system program_options regex thread From 3e55c5126c75bb41a19ece7a8f8d3509326d5e2f Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 16:28:56 +0300 Subject: [PATCH 09/14] ifcgeom: honour PnIndex in triangulated and polygonal face sets (#3434) IfcTriangulatedFaceSet and IfcPolygonalFaceSet used CoordIndex values to index Coordinates.CoordList directly, ignoring the optional PnIndex attribute. When PnIndex is present it remaps point references, so a CoordIndex value i must resolve as CoordList[PnIndex[i-1]-1] (both 1-based). Without the indirection any model carrying a PnIndex was built from the wrong points. Add a resolve() helper in both mappings that applies the PnIndex indirection when present and is a plain bounds-checked lookup otherwise, with bounds checks at both index levels. When PnIndex is absent the behavior is unchanged. Co-Authored-By: Claude Opus 4.8 --- src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp | 33 ++++++++++++------- .../mapping/IfcTriangulatedFaceSet.cpp | 22 ++++++++++--- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp b/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp index c3f7ff7219..c965db542a 100644 --- a/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp +++ b/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp @@ -39,8 +39,25 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPolygonalFaceSet* inst) { int max_index = (int)points.size(); + // When the optional PnIndex is present, CoordIndex values do not index into + // CoordList directly but into PnIndex, which in turn remaps to CoordList. + // Both index levels are 1-based per the IFC specification. + auto pn_index = inst->PnIndex(); + auto resolve = [&](int idx) -> const taxonomy::point3::ptr& { + if (pn_index) { + if (idx < 1 || idx > (int)pn_index->size()) { + throw IfcParse::IfcException("IfcPolygonalFaceSet PnIndex out of bounds for index " + boost::lexical_cast(idx)); + } + idx = (*pn_index)[idx - 1]; + } + if (idx < 1 || idx > max_index) { + throw IfcParse::IfcException("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast(idx)); + } + return points[idx - 1]; + }; + auto shell = taxonomy::make(); - + for (auto& f : *polygonal_faces) { auto fa = taxonomy::make(); shell->children.push_back(fa); @@ -52,17 +69,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPolygonalFaceSet* inst) { auto indices = f->CoordIndex(); taxonomy::point3::ptr previous; for (std::vector::const_iterator jt = indices.begin(); jt != indices.end(); ++jt) { - if (*jt < 1 || *jt > max_index) { - throw IfcParse::IfcException("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast(*jt)); - } - auto current = points[(*jt) - 1]; + auto current = resolve(*jt); if (jt != indices.begin()) { loop->children.push_back(taxonomy::make(previous, current)); } previous = current; } if (!indices.empty()) { - auto current = points[indices.front() - 1]; + auto current = resolve(indices.front()); loop->children.push_back(taxonomy::make(previous, current)); } } @@ -77,17 +91,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPolygonalFaceSet* inst) { loop->external = false; for (std::vector::const_iterator jt = li.begin(); jt != li.end(); ++jt) { - if (*jt < 1 || *jt > max_index) { - throw IfcParse::IfcException("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast(*jt)); - } - auto current = points[(*jt) - 1]; + auto current = resolve(*jt); if (jt != li.begin()) { loop->children.push_back(taxonomy::make(previous, current)); } previous = current; } if (!li.empty()) { - auto current = points[li.front() - 1]; + auto current = resolve(li.front()); loop->children.push_back(taxonomy::make(previous, current)); } } diff --git a/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp b/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp index 776ea59605..f234c3bd3f 100644 --- a/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp +++ b/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp @@ -39,6 +39,23 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcTriangulatedFaceSet* inst) { int max_index = (int)points.size(); + // When the optional PnIndex is present, CoordIndex values do not index into + // CoordList directly but into PnIndex, which in turn remaps to CoordList. + // Both index levels are 1-based per the IFC specification. + auto pn_index = inst->PnIndex(); + auto resolve = [&](int idx) -> const taxonomy::point3::ptr& { + if (pn_index) { + if (idx < 1 || idx > (int)pn_index->size()) { + throw IfcParse::IfcException("IfcTriangulatedFaceSet PnIndex out of bounds for index " + boost::lexical_cast(idx)); + } + idx = (*pn_index)[idx - 1]; + } + if (idx < 1 || idx > max_index) { + throw IfcParse::IfcException("IfcTriangulatedFaceSet index out of bounds for index " + boost::lexical_cast(idx)); + } + return points[idx - 1]; + }; + auto shell = taxonomy::make(); for (auto& indices : indices_list) { @@ -51,10 +68,7 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcTriangulatedFaceSet* inst) { loop->external = true; taxonomy::point3::ptr first, previous; for (std::vector::const_iterator jt = indices.begin(); jt != indices.end(); ++jt) { - if (*jt < 1 || *jt > max_index) { - throw IfcParse::IfcException("IfcTriangulatedFaceSet index out of bounds for index " + boost::lexical_cast(*jt)); - } - const taxonomy::point3::ptr& current = points[(*jt) - 1]; + const taxonomy::point3::ptr& current = resolve(*jt); if (jt == indices.begin()) { first = current; } else { From 380675e2144ac1cbc732009e4ea5ed6c35e3e379 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 16:03:53 +0300 Subject: [PATCH 10/14] ifcparse: strip XML-illegal control characters in escape_xml (#2043, #3074) escape_xml escaped the five XML metacharacters but passed control characters (0x00 to 0x1F other than tab, newline and carriage return) through unchanged. Those bytes are illegal in XML 1.0 and cannot be represented even as numeric character references, so any IFC string containing them produced non-well-formed XML and SVG output. Strip those illegal control characters before escaping. Bytes belonging to a valid UTF-8 multibyte sequence are always >= 0x80, so filtering on the low control range leaves real text intact. This is the shared helper used by the SVG serializer text and attribute sites (audited: all route through it) and by the XML/Collada paths, so both reports are resolved at one place. Co-Authored-By: Claude Opus 4.8 --- src/ifcparse/IfcUtil.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/ifcparse/IfcUtil.cpp b/src/ifcparse/IfcUtil.cpp index efb5426021..4f0204b44f 100644 --- a/src/ifcparse/IfcUtil.cpp +++ b/src/ifcparse/IfcUtil.cpp @@ -187,6 +187,15 @@ void IfcUtil::sanitate_material_name(std::string& str) { } void IfcUtil::escape_xml(std::string& str) { + // Strip characters that are illegal in XML 1.0. Control characters other + // than tab (0x09), newline (0x0A) and carriage return (0x0D) are not valid + // XML 1.0 characters and cannot even be represented as numeric character + // references, so they would otherwise make the serialized XML/SVG output + // non-well-formed. Bytes belonging to a valid UTF-8 multibyte sequence are + // always >= 0x80, so filtering on the low control range leaves them intact. + str.erase(std::remove_if(str.begin(), str.end(), [](unsigned char c) { + return c < 0x20 && c != '\t' && c != '\n' && c != '\r'; + }), str.end()); boost::replace_all(str, "&", "&"); boost::replace_all(str, "\"", """); boost::replace_all(str, "'", "'"); From e389939092dff27b65b2f3c51d7c18e08a73d8f1 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 15:41:58 +0300 Subject: [PATCH 11/14] serializers: expand IfcPropertySetDefinitionSet in XML output (#6330) Property sets contained in an IfcPropertySetDefinitionSet were exported as an empty element in XML. The XmlSerializer already had a block to expand such a set into its member property sets, but it was gated behind #ifdef SCHEMAS_HAS_IfcPropertySetDefinitionSet while the schema generator emits SCHEMA_HAS_IfcPropertySetDefinitionSet (singular). The plural spelling is defined nowhere, so the block was dead code and a RelatingPropertyDefinition holding a set produced nothing. Correct the macro name so the set is expanded and its property sets are serialized. The parse layer already reads these nested sets (they are reachable from util.element), so this only completes the XML path. Co-Authored-By: Claude Opus 4.8 --- src/serializers/schema_dependent/XmlSerializer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/serializers/schema_dependent/XmlSerializer.cpp b/src/serializers/schema_dependent/XmlSerializer.cpp index e97e8bfb74..38fb8ce5b3 100644 --- a/src/serializers/schema_dependent/XmlSerializer.cpp +++ b/src/serializers/schema_dependent/XmlSerializer.cpp @@ -305,7 +305,7 @@ ptree* descend(Logger& logger, ifcopenshell::geometry::abstract_mapping* mapping (logger, object, &IfcSchema::IfcObject::IsDefinedBy, &IfcSchema::IfcRelDefinesByProperties::RelatingPropertyDefinition); -#ifdef SCHEMAS_HAS_IfcPropertySetDefinitionSet +#ifdef SCHEMA_HAS_IfcPropertySetDefinitionSet aggregate_of::ptr property_set_sets = get_related (logger, object, &IfcSchema::IfcObject::IsDefinedBy, &IfcSchema::IfcRelDefinesByProperties::RelatingPropertyDefinition); From a0f493b47154926993a0191bf1e2fc7a692bd2fe Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 15:37:44 +0300 Subject: [PATCH 12/14] IfcConvert: report an error when the output file cannot be opened (#438) Converting to a path whose directory does not exist (or is not writable) failed silently: the serializer's ready() check correctly returned false, but IfcConvert deleted the temp file and returned EXIT_FAILURE without any message, so the user saw no reason for the failure. Log a SYS error naming the output file before returning, matching the existing "Unable to open output file" reporting used elsewhere. Co-Authored-By: Claude Opus 4.8 --- src/ifcconvert/IfcConvert.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp index 38d245757c..5bb0e94baf 100644 --- a/src/ifcconvert/IfcConvert.cpp +++ b/src/ifcconvert/IfcConvert.cpp @@ -890,6 +890,7 @@ int main(int argc, char** argv) { } if (!serializer->ready()) { + logger.Error("SYS", 25, "Unable to open output file '" + IfcUtil::path::to_utf8(output_filename) + "' for writing; check that the directory exists and is writable"); IfcUtil::path::delete_file(IfcUtil::path::to_utf8(output_temp_filename)); write_log(!quiet); return EXIT_FAILURE; From d16c283aef3a22f75a55ed6069adaa914f8ba651 Mon Sep 17 00:00:00 2001 From: Ryan Schultz Date: Sat, 11 Jul 2026 15:54:20 -0500 Subject: [PATCH 13/14] Add bulk-load of selected drawings' annotations (#8525) SHIFT+CTRL+CLICK on Activate Drawing now imports the annotations of all selected drawings without switching the active view or camera, then selects their cameras with the first as active. SHIFT+CTRL+ALT+CLICK also selects the loaded annotation objects. The drawing camera is imported when missing so annotations land in the correct collection. Loading is idempotent. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/drawing/operator.py | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/drawing/operator.py b/src/bonsai/bonsai/bim/module/drawing/operator.py index 2bcc620424..7c53067a00 100644 --- a/src/bonsai/bonsai/bim/module/drawing/operator.py +++ b/src/bonsai/bonsai/bim/module/drawing/operator.py @@ -2343,7 +2343,9 @@ class ActivateDrawingBase(tool.Ifc.Operator): "Activates the selected drawing view.\n\n" + "ALT+CLICK to keep the viewport position.\n\n" + "SHIFT+CLICK to load a quick preview of the drawing view.\n\n" - + "SHIFT+CTRL+CLICK to load the annotations of all selected drawings without switching views" + + "SHIFT+CTRL+CLICK to load the annotations of all selected drawings without switching views, " + + "then select their cameras (the first selected drawing's camera becomes active).\n\n" + + "SHIFT+CTRL+ALT+CLICK to do the same but also select the annotations, not just the cameras" ) drawing: bpy.props.IntProperty() @@ -2365,16 +2367,25 @@ class ActivateDrawingBase(tool.Ifc.Operator): default=False, options={"SKIP_SAVE"}, ) + include_annotations_in_selection: bpy.props.BoolProperty( + name="Include Annotations In Selection", + description="Also select the loaded annotation objects, not just the drawing cameras.", + default=False, + options={"SKIP_SAVE"}, + ) if TYPE_CHECKING: drawing: int should_view_from_camera: bool use_quick_preview: bool load_selected_annotations: bool + include_annotations_in_selection: bool def invoke(self, context, event) -> set["rna_enums.OperatorReturnItems"]: if event.type == "LEFTMOUSE" and event.shift and event.ctrl: self.load_selected_annotations = True + if event.alt: + self.include_annotations_in_selection = True return self.execute(context) if event.type == "LEFTMOUSE" and event.alt: self.should_view_from_camera = False @@ -2389,15 +2400,34 @@ class ActivateDrawingBase(tool.Ifc.Operator): bpy.ops.bim.load_drawings() if self.load_selected_annotations: + objs_to_select = [] + active_camera = None for d in props.drawings: if not (d.is_drawing and d.is_selected): continue selected_drawing = tool.Ifc.get().by_id(d.ifc_definition_id) # Importing the camera (if missing) ensures the drawing's # collection exists so the annotations get collected into it. - if not tool.Ifc.get_object(selected_drawing): - tool.Drawing.import_drawing(selected_drawing) - tool.Drawing.import_annotations_in_group(tool.Drawing.get_drawing_group(selected_drawing)) + if not (camera := tool.Ifc.get_object(selected_drawing)): + camera = tool.Drawing.import_drawing(selected_drawing) + group = tool.Drawing.get_drawing_group(selected_drawing) + tool.Drawing.import_annotations_in_group(group) + + if active_camera is None: + active_camera = camera + objs_to_select.append(camera) + if self.include_annotations_in_selection: + for element in tool.Drawing.get_group_elements(group) or []: + if element.is_a("IfcAnnotation") and element.ObjectType != "DRAWING": + if annotation_obj := tool.Ifc.get_object(element): + objs_to_select.append(annotation_obj) + + # Select the checked drawings' objects, with the first drawing's camera as active. + bpy.ops.object.select_all(action="DESELECT") + for obj in objs_to_select: + obj.select_set(True) + if active_camera is not None: + context.view_layer.objects.active = active_camera return {"FINISHED"} drawing = tool.Ifc.get().by_id(self.drawing) @@ -2486,7 +2516,9 @@ class ActivateDrawing(bpy.types.Operator, ActivateDrawingBase): "Activates the selected drawing view.\n\n" + "ALT+CLICK to keep the viewport position.\n\n" + "SHIFT+CLICK to load a quick preview of the drawing view.\n\n" - + "SHIFT+CTRL+CLICK to load the annotations of all selected drawings without switching views" + + "SHIFT+CTRL+CLICK to load the annotations of all selected drawings without switching views, " + + "then select their cameras (the first selected drawing's camera becomes active).\n\n" + + "SHIFT+CTRL+ALT+CLICK to do the same but also select the annotations, not just the cameras" ) From 0b7e25a3ef2bfc66a4f2715a07c3f67fa020888e Mon Sep 17 00:00:00 2001 From: Ryan Schultz Date: Sat, 11 Jul 2026 16:44:22 -0500 Subject: [PATCH 14/14] Docs: clarify immediate vs. any-depth spatial selectors The location and parent filters both match at any depth in the spatial hierarchy, which surprises users who want only the elements immediately under a given container. Document that the parent query key resolves the direct parent only (e.g. query:"parent.Name"="My Site"), add a matching filter example, and note the immediacy on the parent value key. Co-Authored-By: Claude Opus 4.8 --- .../docs/ifcopenshell-python/selector_syntax.rst | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/ifcopenshell-python/docs/ifcopenshell-python/selector_syntax.rst b/src/ifcopenshell-python/docs/ifcopenshell-python/selector_syntax.rst index 5ae16c5ec0..f8098c83af 100644 --- a/src/ifcopenshell-python/docs/ifcopenshell-python/selector_syntax.rst +++ b/src/ifcopenshell-python/docs/ifcopenshell-python/selector_syntax.rst @@ -72,6 +72,8 @@ Filtering is typically used to select any IFC element or type. "``IfcPump, location=""Level 3""``", "Locations bubble up the hierarchy. So if a pump is in a space and that space is on Level 3, then you can say ""all pumps on level 3"" which will include that pump in the space." + "``IfcElement, query:""parent.Name""=""My Site""``", "Only elements *immediately* under ""My Site"" in the spatial hierarchy. Unlike the ``location`` and ``parent`` filters, which both match at any depth, the ``parent`` query key resolves the direct parent only, so nested storeys (and their contents) are excluded." + The filter elements syntax works by specifying one or more groups of filters separated by a ``+`` character. Each filter group will return a set of filtered elements, and these are unioned together. @@ -111,6 +113,15 @@ will search through all IfcTypeProducts and IfcProducts in the IFC project. "Parent", "Filter", "``parent{{=}}{{value}}``", "``parent=Foo`` specifies the criteria that elements must be a direct or indirect child in the spatial hierarchy to an element with a ``Name`` attribute with a value of ``Foo``." "Query", "Filter", "``query:{{keys}}{{=}}{{value}}``", "``query:types.count=0`` specifies the criteria that elements must have zero type occurrences. The query keys corresponds to the syntax used in the `Getting element values`_ section" +.. note:: + + The ``location`` and ``parent`` filters both match at **any depth** in the + spatial hierarchy. To match only elements *immediately* contained in (or + aggregated under) a spatial element, use the ``parent`` query key, which + resolves the direct parent only. For example, + ``query:"parent.Name"="My Site"`` selects elements directly under ``My + Site`` but excludes anything nested inside its sub-storeys or spaces. + When you specify a filter with a ``{{=}}`` check, you can choose from one of the following comparison checks: @@ -191,7 +202,7 @@ Valid keys are: "``storey``", "Gets the first IfcBuildingStorey spatial element that an element is contained in." "``building``", "Gets the first IfcBuilding spatial element that an element is contained in." "``site``", "Gets the first IfcSite spatial element that an element is contained in." - "``parent``", "Gets the parent element in the spatial hierarchy." + "``parent``", "Gets the **immediate** parent element in the spatial hierarchy (the direct spatial container, or the direct aggregate/nest/fill/void parent). Combine with ``.Name`` in a query filter to match only immediate children, e.g. ``query:""parent.Name""=""My Site""``." "``classification``", "Gets the element's classification reference(s)" "``group``", "Gets the element's group(s)" "``system``", "Gets the element's system(s). This is a subset of group(s)."