From 183e4c47f7a413a974c5863f11cfce80308fc638 Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Thu, 16 Jul 2026 15:29:22 +0100 Subject: [PATCH] Add SVG edge classification on/off + render settings Add svg-use-edge-classification (default off, preserving today's linework), svg-render-crease-edges, and svg-render-sharp-edges settings, gating the existing 5-class classification feature so it can be disabled entirely (falling back to the pre-classification whole-shape output) or have individual classes suppressed. Also fixes a bug uncovered while wiring this into Bonsai: ready(), where geometry_settings() actually gets read into the serializer, was only ever invoked explicitly by IfcConvert's CLI driver and isn't exposed to Python. Every Svg* setting -- including the three from previous rounds -- silently stayed at its hardcoded constructor default when the serializer was constructed directly through the Python bindings, as Bonsai does. Fixed by calling ready() from SvgSerializer's own constructor, safe since it only reads geometry_settings() with no other side effects, and settings are always finalized before construction in every call path. Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Sonnet 5 --- src/ifcgeom/ConversionSettings.h | 20 +++++++++++++++- .../ifcopenshell/geom/main.py | 6 +++++ src/serializers/SvgSerializer.cpp | 23 ++++++++++++++++++- src/serializers/SvgSerializer.h | 18 ++++++++++++++- 4 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/ifcgeom/ConversionSettings.h b/src/ifcgeom/ConversionSettings.h index 49d0fccbfb..621174cce2 100644 --- a/src/ifcgeom/ConversionSettings.h +++ b/src/ifcgeom/ConversionSettings.h @@ -389,6 +389,24 @@ namespace ifcopenshell { static constexpr bool defaultvalue = false; }; + struct SvgUseEdgeClassification : public SettingBase { + static constexpr const char* const name = "svg-use-edge-classification"; + static constexpr const char* const description = "SVG edge classification (issue #3668): enable the 5-class boundary/outline/sharp/crease/flush scheme. When false (the default), falls back to the original unclassified linework."; + static constexpr bool defaultvalue = false; + }; + + struct SvgRenderCreaseEdges : public SettingBase { + static constexpr const char* const name = "svg-render-crease-edges"; + static constexpr const char* const description = "SVG edge classification (issue #3668): whether to emit 'crease' (concave) projection edges. Only relevant when svg-use-edge-classification is enabled."; + static constexpr bool defaultvalue = true; + }; + + struct SvgRenderSharpEdges : public SettingBase { + static constexpr const char* const name = "svg-render-sharp-edges"; + static constexpr const char* const description = "SVG edge classification (issue #3668): whether to emit 'sharp' (convex) projection edges. Only relevant when svg-use-edge-classification is enabled."; + static constexpr bool defaultvalue = true; + }; + struct KeepBoundingBoxes : public SettingBase { static constexpr const char* const name = "keep-bounding-boxes"; static constexpr const char* const description = @@ -671,7 +689,7 @@ namespace ifcopenshell { }; class Settings : public SettingsContainer< - std::tuple + std::tuple > {}; } diff --git a/src/ifcopenshell-python/ifcopenshell/geom/main.py b/src/ifcopenshell-python/ifcopenshell/geom/main.py index bf5fc00098..61c7d2a10e 100644 --- a/src/ifcopenshell-python/ifcopenshell/geom/main.py +++ b/src/ifcopenshell-python/ifcopenshell/geom/main.py @@ -109,6 +109,12 @@ SETTING = Literal[ "reorient-shells", "site-local-placement", "surface-colour", + "svg-emit-flush-edges", + "svg-render-crease-edges", + "svg-render-sharp-edges", + "svg-ridge-angle-min-degrees", + "svg-use-edge-classification", + "svg-valley-angle-min-degrees", "triangulation-type", "unify-shapes", "use-material-names", diff --git a/src/serializers/SvgSerializer.cpp b/src/serializers/SvgSerializer.cpp index 2a4cb9c2b4..79101dfb67 100644 --- a/src/serializers/SvgSerializer.cpp +++ b/src/serializers/SvgSerializer.cpp @@ -105,6 +105,9 @@ bool SvgSerializer::ready() { svg_ridge_angle_min_deg_ = geometry_settings().get().get(); svg_valley_angle_min_deg_ = geometry_settings().get().get(); svg_emit_flush_edges_ = geometry_settings().get().get(); + svg_use_edge_classification_ = geometry_settings().get().get(); + svg_render_crease_edges_ = geometry_settings().get().get(); + svg_render_sharp_edges_ = geometry_settings().get().get(); return true; } @@ -1398,8 +1401,20 @@ void SvgSerializer::write(const geometry_data& data) { // is still registered via add()/it->second.add() below, unchanged, for correct // occlusion; these buckets only affect which class each edge's visible portion is // later extracted as (see hlr_calc::extract() in SvgSerializer.h). + // + // Gated behind svg_use_edge_classification_ (default false): the whole block must + // be skipped, not just individually suppressed per-edge, so that when disabled + // classified_edge_buckets stays empty for *every* product in the document, not + // just this one. hlr_calc::extract() only takes the classified-buckets branch + // when its shared classified_shapes_ list is non-empty; if even one product added + // classified buckets while others didn't, those others would silently fall back + // to unclassified linework while this one used classification, an inconsistent + // mix. Leaving classified_edge_buckets empty here means add_classified_edges() is + // never called for this product either, so every product uniformly falls through + // to the pre-existing product_shapes_ fallback -- the original, pre-classification + // linework. std::map classified_edge_buckets; - { + if (svg_use_edge_classification_) { NCollection_IndexedDataMap, TopTools_ShapeMapHasher> edge_face_map; TopExp::MapShapesAndAncestors(*compound_to_hlr, TopAbs_EDGE, TopAbs_FACE, edge_face_map); @@ -1426,6 +1441,12 @@ void SvgSerializer::write(const geometry_data& data) { if (cls == edge_style_class::flush && !svg_emit_flush_edges_) { continue; } + if (cls == edge_style_class::crease && !svg_render_crease_edges_) { + continue; + } + if (cls == edge_style_class::sharp && !svg_render_sharp_edges_) { + continue; + } std::string name = edge_style_class_name(cls); auto bucket_it = classified_edge_buckets.find(name); diff --git a/src/serializers/SvgSerializer.h b/src/serializers/SvgSerializer.h index 223bc5e441..9da7963f7c 100644 --- a/src/serializers/SvgSerializer.h +++ b/src/serializers/SvgSerializer.h @@ -600,6 +600,9 @@ protected: double svg_ridge_angle_min_deg_; double svg_valley_angle_min_deg_; bool svg_emit_flush_edges_; + bool svg_use_edge_classification_; + bool svg_render_crease_edges_; + bool svg_render_sharp_edges_; IfcParse::IfcFile* file; const IfcUtil::IfcBaseEntity* storey_; @@ -657,6 +660,9 @@ public: , svg_ridge_angle_min_deg_(45.) , svg_valley_angle_min_deg_(12.) , svg_emit_flush_edges_(false) + , svg_use_edge_classification_(false) + , svg_render_crease_edges_(true) + , svg_render_sharp_edges_(true) , file(0) , storey_(0) , xcoords_begin(0) @@ -665,7 +671,17 @@ public: , hlr(nullptr) , namespace_prefix_("data-") , subtraction_settings_(ON_SLABS_AT_FLOORPLANS) - {} + { + // ready() only reads geometry_settings() (already valid at this point, since the base + // WriteOnlyGeometrySerializer initializer above has run) and has no other side effects, + // so it's safe to call here. This is needed because ready() is otherwise only invoked + // explicitly by IfcConvert.cpp's CLI driver -- callers that construct this serializer + // directly via the Python bindings (e.g. Bonsai's drawing generation, which never calls + // a ready()-equivalent because it isn't exposed via SWIG) would otherwise silently keep + // every settings::Svg* member at its hardcoded constructor default forever, regardless + // of what ifcopenshell.geom.settings().set(...) was actually configured to. + ready(); + } void addXCoordinate(const boost::shared_ptr& fi) { xcoords.push_back(fi); } void addYCoordinate(const boost::shared_ptr& fi) { ycoords.push_back(fi); } void addSizeComponent(const boost::shared_ptr& fi) { radii.push_back(fi); }