From f0970b90b07426342f99299cc94a54fdbb8da36d Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Wed, 15 Jul 2026 06:28:02 +0100 Subject: [PATCH 1/7] Classify projection edges in SVG elevations Adds boundary/outline/sharp/crease/flush classification of HLR projection edges in SvgSerializer, so CSS can style silhouettes, ridges, and valleys differently instead of drawing every edge identically (fixes the "ugly faceted sphere" problem from #3668). Classification happens pre-HLR on the original solid's real face topology (three prior attempts tried to classify HLR's own output, which carries no face topology at all and can't be correlated back by edge identity). Each class's visible portion is then extracted via HLRBRep_HLRToShape::VCompound(S)/OutLineVCompound(S), the same per-shape filtering mechanism already used for per-product segmentation, applied per class instead. Classes are tagged directly on individual elements so Bonsai's merge_linework_and_add_metadata group-level class rewrite in operator.py never touches them. New settings: svg-ridge-angle-min-degrees, svg-valley-angle-min-degrees, svg-emit-flush-edges (ConversionSettings.h), wired through Bonsai's CreateDrawing operator and exposed via its redo panel. Refs #3668. Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Sonnet 5 --- src/bonsai/bonsai/bim/data/assets/default.css | 8 + .../bonsai/bim/module/drawing/operator.py | 33 ++ src/ifcgeom/ConversionSettings.h | 20 +- src/serializers/SvgSerializer.cpp | 308 +++++++++++++++--- src/serializers/SvgSerializer.h | 78 +++-- 5 files changed, 379 insertions(+), 68 deletions(-) diff --git a/src/bonsai/bonsai/bim/data/assets/default.css b/src/bonsai/bonsai/bim/data/assets/default.css index 68307f4c3f..5e4670af4d 100644 --- a/src/bonsai/bonsai/bim/data/assets/default.css +++ b/src/bonsai/bonsai/bim/data/assets/default.css @@ -24,6 +24,14 @@ a text, a tspan { fill: blue !important; text-decoration: underline;} a:hover { cursor: pointer; } .cut { fill: black; stroke: black; stroke-linecap: 'round'; stroke-width: 0.35; fill-rule: evenodd; } .projection { fill: white; stroke: black; stroke-linecap: 'round'; stroke-width: 0.25; } +/* SVG edge classification (issue #3668): see edge-classification.md. These select directly on + the element (each classified projection edge carries its own class), so they win over + the inherited .projection rule above regardless of specificity. */ +path.outline { stroke: black; stroke-width: 0.35; stroke-opacity: 1; } +path.boundary { stroke: black; stroke-width: 0.3; stroke-opacity: 0.9; } +path.sharp { stroke: black; stroke-width: 0.25; stroke-opacity: 0.85; } +path.crease { stroke: black; stroke-width: 0.18; stroke-opacity: 0.7; } +path.flush { stroke: black; stroke-width: 0.1; stroke-opacity: 0.4; } .surface { stroke: none; fill: #fff; fill-rule: evenodd; } .annotation { fill: none; stroke: black; stroke-linecap: 'round'; stroke-width: 0.25; } .IfcAnnotation { fill: none; stroke: black; stroke-linecap: 'round'; stroke-width: 0.25; } diff --git a/src/bonsai/bonsai/bim/module/drawing/operator.py b/src/bonsai/bonsai/bim/module/drawing/operator.py index b1c4c69c3b..9a8faf4d58 100644 --- a/src/bonsai/bonsai/bim/module/drawing/operator.py +++ b/src/bonsai/bonsai/bim/module/drawing/operator.py @@ -261,11 +261,36 @@ class CreateDrawing(bpy.types.Operator): description="Could save some time if you're sure IFC and current Blender session are already in sync", default=True, ) + svg_ridge_angle_min_deg: bpy.props.FloatProperty( + name="Ridge Angle Minimum", + description="Minimum convex dihedral deviation from flat, in degrees, for a projection " + "edge to be classified as 'sharp' rather than 'flush'. See edge-classification.md", + default=45.0, + min=0.0, + max=180.0, + ) + svg_valley_angle_min_deg: bpy.props.FloatProperty( + name="Valley Angle Minimum", + description="Minimum concave dihedral deviation from flat, in degrees, for a projection " + "edge to be classified as 'crease' rather than 'flush'. See edge-classification.md", + default=12.0, + min=0.0, + max=180.0, + ) + svg_emit_flush_edges: bpy.props.BoolProperty( + name="Emit Flush Edges", + description="Include projection edges whose dihedral deviation is below both the ridge " + "and valley thresholds (class 'flush'). Omitted by default", + default=False, + ) if TYPE_CHECKING: print_all: bool open_viewer: bool sync: bool + svg_ridge_angle_min_deg: float + svg_valley_angle_min_deg: float + svg_emit_flush_edges: bool drawing_name: str is_manifold_cache: dict[str, bool] @@ -1309,6 +1334,14 @@ class CreateDrawing(bpy.types.Operator): self.svg_settings = ifcopenshell.geom.settings() self.svg_settings.set("dimensionality", ifcopenshell.ifcopenshell_wrapper.CURVES_SURFACES_AND_SOLIDS) self.svg_settings.set("iterator-output", ifcopenshell.ifcopenshell_wrapper.NATIVE) + # SVG edge classification (issue #3668). See edge-classification.md. + try: + self.svg_settings.set("svg-ridge-angle-min-degrees", self.svg_ridge_angle_min_deg) + self.svg_settings.set("svg-valley-angle-min-degrees", self.svg_valley_angle_min_deg) + self.svg_settings.set("svg-emit-flush-edges", self.svg_emit_flush_edges) + except Exception: + # Backwards compatibility with older ifcopenshell builds that don't expose these keys. + pass self.svg_buffer = ifcopenshell.geom.serializers.buffer() self.serialiser_settings = ifcopenshell.geom.serializer_settings() self.serialiser = ifcopenshell.geom.serializers.svg( diff --git a/src/ifcgeom/ConversionSettings.h b/src/ifcgeom/ConversionSettings.h index c022bfdd1c..49d0fccbfb 100644 --- a/src/ifcgeom/ConversionSettings.h +++ b/src/ifcgeom/ConversionSettings.h @@ -371,6 +371,24 @@ namespace ifcopenshell { static constexpr double defaultvalue = -1.; }; + struct SvgRidgeAngleMinDegrees : public SettingBase { + static constexpr const char* const name = "svg-ridge-angle-min-degrees"; + static constexpr const char* const description = "SVG edge classification (issue #3668): minimum convex dihedral deviation from flat, in degrees, for a projection edge to be classified as 'sharp' rather than 'flush'."; + static constexpr double defaultvalue = 45.; + }; + + struct SvgValleyAngleMinDegrees : public SettingBase { + static constexpr const char* const name = "svg-valley-angle-min-degrees"; + static constexpr const char* const description = "SVG edge classification (issue #3668): minimum concave dihedral deviation from flat, in degrees, for a projection edge to be classified as 'crease' rather than 'flush'."; + static constexpr double defaultvalue = 12.; + }; + + struct SvgEmitFlushEdges : public SettingBase { + static constexpr const char* const name = "svg-emit-flush-edges"; + static constexpr const char* const description = "SVG edge classification (issue #3668): whether to emit 'flush' projection edges (dihedral deviation below both ridge/valley thresholds). Defaults to false, i.e. flush edges are omitted from the output."; + static constexpr bool defaultvalue = false; + }; + struct KeepBoundingBoxes : public SettingBase { static constexpr const char* const name = "keep-bounding-boxes"; static constexpr const char* const description = @@ -653,7 +671,7 @@ namespace ifcopenshell { }; class Settings : public SettingsContainer< - std::tuple + std::tuple > {}; } diff --git a/src/serializers/SvgSerializer.cpp b/src/serializers/SvgSerializer.cpp index 16659587ac..24bc2c561a 100644 --- a/src/serializers/SvgSerializer.cpp +++ b/src/serializers/SvgSerializer.cpp @@ -102,10 +102,13 @@ const double PI2 = M_PI * 2.; 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(); return true; } -void SvgSerializer::write(path_object& p, const TopoDS_Shape& comp_or_wire, boost::optional> dash_array) { +void SvgSerializer::write(path_object& p, const TopoDS_Shape& comp_or_wire, boost::optional> dash_array, boost::optional css_class) { /* ShapeFix_Wire fix; Handle(ShapeExtend_WireData) data = new ShapeExtend_WireData; for (TopExp_Explorer edges(result, TopAbs_EDGE); edges.More(); edges.Next()) { @@ -351,6 +354,12 @@ void SvgSerializer::write(path_object& p, const TopoDS_Shape& comp_or_wire, boos if (!path.empty()) { path.add("\""); + if (css_class) { + path.add(" class=\""); + path.add(*css_class); + path.add("\""); + } + if (dash_array) { path.add(" stroke-dasharray=\""); bool first = true; @@ -622,7 +631,7 @@ void SvgSerializer::write(const IfcGeom::BRepElement* brep_obj) { } else if (elevation_ref_guid_) { is_elevation = *elevation_ref_guid_ == brep_obj->guid(); } - + BRepBuilderAPI_Transform make_transform_global(compound_local, trsf, true); make_transform_global.Build(); // (When determinant < 0, copy is implied and the input is not mutated.) @@ -795,6 +804,115 @@ namespace { } } +namespace { + // SVG edge classification (issue #3668). See edge-classification.md at the repo root for + // the authoritative definition of the five classes and their evaluation order. + enum class edge_style_class { boundary, outline, sharp, crease, flush }; + + const char* edge_style_class_name(edge_style_class c) { + switch (c) { + case edge_style_class::boundary: return "boundary"; + case edge_style_class::outline: return "outline"; + case edge_style_class::sharp: return "sharp"; + case edge_style_class::crease: return "crease"; + default: return "flush"; + } + } + + // Outward face normal, accounting for face orientation. Only planar faces are supported; + // returns false otherwise (caller should conservatively treat the edge as an outline). + bool face_normal_from_planar_face(const TopoDS_Face& f, gp_Dir& out) { + auto s = BRep_Tool::Surface(f); + if (s->DynamicType() != STANDARD_TYPE(Geom_Plane)) { + return false; + } + auto p = Handle(Geom_Plane)::DownCast(s); + gp_Dir d = p->Axis().Direction(); + if (f.Orientation() == TopAbs_REVERSED) { + d.Reverse(); + } + out = d; + return true; + } + + double clamp_dot(double v) { + if (v < -1.0) return -1.0; + if (v > 1.0) return 1.0; + return v; + } + + edge_style_class classify_edge_from_faces( + const TopoDS_Edge& edge, + const NCollection_List& faces, + const gp_Dir& projection_direction, + double ridge_angle_min_deg, + double valley_angle_min_deg + ) { + std::vector faces_vec; + for (NCollection_List::Iterator it(faces); it.More(); it.Next()) { + const TopoDS_Shape& s = it.Value(); + if (s.ShapeType() == TopAbs_FACE) { + faces_vec.push_back(TopoDS::Face(s)); + } + } + + // Boundary: naked edge, or non-manifold (3+ faces) -- the latter is explicitly out of + // scope for the 5-class scheme (a geometry-health/QA concern), so fall back to the + // same conservative bucket rather than force-fitting it into outline/sharp/crease. + if (faces_vec.size() != 2) { + return edge_style_class::boundary; + } + + const TopoDS_Face& f0 = faces_vec[0]; + const TopoDS_Face& f1 = faces_vec[1]; + + gp_Dir n0, n1; + if (!face_normal_from_planar_face(f0, n0) || !face_normal_from_planar_face(f1, n1)) { + // Conservative fallback for non-planar-face edges. + return edge_style_class::outline; + } + + const double d0 = projection_direction.Dot(n0); + const double d1 = projection_direction.Dot(n1); + + // Outline: silhouette, either against the background or self-occluding -- one face + // turns toward the viewer while the other turns away. + if ((d0 < 0.0) != (d1 < 0.0)) { + return edge_style_class::outline; + } + + // Signed deviation from flat (180 degrees between outward normals = perfectly flat). + // Positive = convex (ridge/sharp), negative = concave (valley/crease). The sign comes + // from the rotation of n0 onto n1 about the edge tangent. + const double angle_between_normals_deg = std::acos(clamp_dot(n0.Dot(n1))) * 180.0 / M_PI; + double deviation_deg = 180.0 - angle_between_normals_deg; + + double u0, u1; + Handle(Geom_Curve) curve = BRep_Tool::Curve(edge, u0, u1); + if (!curve.IsNull()) { + gp_Pnt p_mid; + gp_Vec tangent; + curve->D1((u0 + u1) / 2.0, p_mid, tangent); + if (tangent.SquareMagnitude() > 1.e-10) { + tangent.Normalize(); + if (edge.Orientation() == TopAbs_REVERSED) { + tangent.Reverse(); + } + const gp_Vec cross = gp_Vec(n0.XYZ()).Crossed(gp_Vec(n1.XYZ())); + if (cross.Dot(tangent) < 0.0) { + deviation_deg = -deviation_deg; + } + } + } + + if (deviation_deg >= 0.0) { + return (deviation_deg >= ridge_angle_min_deg) ? edge_style_class::sharp : edge_style_class::flush; + } else { + return (-deviation_deg >= valley_angle_min_deg) ? edge_style_class::crease : edge_style_class::flush; + } + } +} + void SvgSerializer::write(const geometry_data& data) { std::vector section_heights_storage; const std::vector* section_heights_used = §ion_heights_storage; @@ -1208,6 +1326,51 @@ void SvgSerializer::write(const geometry_data& data) { } } + // SVG edge classification (issue #3668): classify *compound_to_hlr's edges (real + // face topology, pre-HLR) into per-class edge-only sub-compounds. The full shape + // 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). + std::map classified_edge_buckets; + { + NCollection_IndexedDataMap, TopTools_ShapeMapHasher> edge_face_map; + TopExp::MapShapesAndAncestors(*compound_to_hlr, TopAbs_EDGE, TopAbs_FACE, edge_face_map); + + gp_Dir view_dir; + try { + view_dir = gp_Dir(projection_direction); + } catch (const Standard_Failure&) { + view_dir = gp::DZ(); + } + + BRep_Builder BBcls; + for (int i = 1; i <= edge_face_map.Extent(); ++i) { + const TopoDS_Edge& cls_edge = TopoDS::Edge(edge_face_map.FindKey(i)); + + edge_style_class cls = edge_style_class::outline; + try { + cls = classify_edge_from_faces(cls_edge, edge_face_map.FindFromIndex(i), view_dir, svg_ridge_angle_min_deg_, svg_valley_angle_min_deg_); + } catch (const Standard_Failure& e) { + logger_.Warning("SER", 30, std::string("SVG edge classification OCC exception: ") + e.GetMessageString()); + } catch (const std::exception& e) { + logger_.Warning("SER", 31, std::string("SVG edge classification exception: ") + e.what()); + } + + if (cls == edge_style_class::flush && !svg_emit_flush_edges_) { + continue; + } + + std::string name = edge_style_class_name(cls); + auto bucket_it = classified_edge_buckets.find(name); + if (bucket_it == classified_edge_buckets.end()) { + TopoDS_Compound c; + BBcls.MakeCompound(c); + bucket_it = classified_edge_buckets.emplace(name, c).first; + } + BBcls.Add(bucket_it->second, cls_edge); + } + } + if (is_floor_plan_) { if (storey) { auto it = storey_hlr.find(storey); @@ -1215,11 +1378,17 @@ void SvgSerializer::write(const geometry_data& data) { it = storey_hlr.insert({ storey, hlr_t(logger_, use_prefiltering_, use_hlr_poly_, segment_projection_, projection_plane) }).first; } it->second.add(*compound_to_hlr, data.product); + for (auto& kv : classified_edge_buckets) { + it->second.add_classified_edges(data.product, kv.first, kv.second); + } } else { logger_.Warning("SER", 28, "Unable to invoke HLR due to absence of storey containment", data.product); } } else if (hlr) { hlr->add(*compound_to_hlr, data.product); + for (auto& kv : classified_edge_buckets) { + hlr->add_classified_edges(data.product, kv.first, kv.second); + } } } } @@ -1767,49 +1936,63 @@ std::array, 3> SvgSerializer::resize() { } void SvgSerializer::draw_hlr(const gp_Pln& pln, const drawing_key& drawing_name) { - auto hlr_items = (drawing_name.first ? this->storey_hlr.find(drawing_name.first)->second : *hlr).build(); + hlr_t& hlr_source = drawing_name.first ? this->storey_hlr.find(drawing_name.first)->second : *hlr; + auto hlr_items = hlr_source.build(); - for (auto& p : hlr_items) { - const TopoDS_Shape& hlr_compound_unmirrored = p.second; + // SVG edge classification (issue #3668): each item's class is already known -- it was + // determined pre-HLR from real face topology (see the classified_edge_buckets block in + // write(const geometry_data&)) and threaded through via hlr_calc::extract(). No post-hoc + // lookup against HLR's own (face-less) output is needed. Multiple items can share the same + // product (one per non-empty class bucket); keep a single path_object/group per product so + // per-path classes survive Bonsai's merge_linework_and_add_metadata untouched, rather than + // creating a per class (see plan notes on why that clobbers classes in Python). + std::map group_by_product; - if (!hlr_compound_unmirrored.IsNull()) { - // Compound 3D curves for mirroring to work - ShapeFix_Edge sfe; - TopExp_Explorer exp(hlr_compound_unmirrored, TopAbs_EDGE); - for (; exp.More(); exp.Next()) { - sfe.FixAddCurve3d(TopoDS::Edge(exp.Current())); + for (auto& item : hlr_items) { + const IfcUtil::IfcBaseEntity* product = std::get<0>(item); + const std::string& cls = std::get<1>(item); + const TopoDS_Shape& hlr_compound_unmirrored = std::get<2>(item); + + if (hlr_compound_unmirrored.IsNull()) { + continue; + } + + // Compound 3D curves for mirroring to work + ShapeFix_Edge sfe; + TopExp_Explorer exp(hlr_compound_unmirrored, TopAbs_EDGE); + for (; exp.More(); exp.Next()) { + sfe.FixAddCurve3d(TopoDS::Edge(exp.Current())); + } + + // Mirror to match SVG coord system. + // @todo this is very wasteful. We better do the Y-mirror in the SVG writing and + // not on the TopoDS_Shape input. + + TopoDS_Shape hlr_compound; + if (drawing_name.first == nullptr) { + gp_Trsf trsf_mirror; + if (!mirror_y_) { + trsf_mirror.SetMirror(gp_Ax2(gp::Origin(), gp::DY())); } - - // Mirror to match SVG coord system. - // @todo this is very wasteful. We better do the Y-mirror in the SVG writing and - // not on the TopoDS_Shape input. - - TopoDS_Shape hlr_compound; - if (drawing_name.first == nullptr) { - gp_Trsf trsf_mirror; - if (!mirror_y_) { - trsf_mirror.SetMirror(gp_Ax2(gp::Origin(), gp::DY())); - } - if (mirror_x_) { - gp_Trsf mirror_x; - mirror_x.SetMirror(gp_Ax2(gp::Origin(), gp::DX())); - trsf_mirror.PreMultiply(mirror_x); - } - BRepBuilderAPI_Transform make_transform_mirror(hlr_compound_unmirrored, trsf_mirror, true); - make_transform_mirror.Build(); - hlr_compound = make_transform_mirror.Shape(); - } else { - // In case of building storey-based floor plan the mirroring has already - // been taken into account before projection. - hlr_compound = hlr_compound_unmirrored; + if (mirror_x_) { + gp_Trsf mirror_x; + mirror_x.SetMirror(gp_Ax2(gp::Origin(), gp::DX())); + trsf_mirror.PreMultiply(mirror_x); } + BRepBuilderAPI_Transform make_transform_mirror(hlr_compound_unmirrored, trsf_mirror, true); + make_transform_mirror.Build(); + hlr_compound = make_transform_mirror.Shape(); + } else { + // In case of building storey-based floor plan the mirroring has already + // been taken into account before projection. + hlr_compound = hlr_compound_unmirrored; + } - exp.Init(hlr_compound, TopAbs_EDGE); - BRep_Builder B; - path_object* po; + path_object*& po = group_by_product[product]; + if (!po) { std::string name; - if (p.first) { - name = nameElement(p.first); + if (product) { + name = nameElement(product); boost::replace_all(name, "class=\"", "class=\"projection "); } else { name = "class=\"projection\""; @@ -1819,13 +2002,19 @@ void SvgSerializer::draw_hlr(const gp_Pln& pln, const drawing_key& drawing_name) } else { po = &start_path(pln, drawing_name.second, name); } - for (; exp.More(); exp.Next()) { - TopoDS_Wire w; - B.MakeWire(w); - B.Add(w, exp.Current()); - write(*po, w); - } + } + boost::optional css_class; + if (!cls.empty()) { + css_class = cls; + } + + BRep_Builder B; + for (TopExp_Explorer exp_mirrored(hlr_compound, TopAbs_EDGE); exp_mirrored.More(); exp_mirrored.Next()) { + TopoDS_Wire w; + B.MakeWire(w); + B.Add(w, exp_mirrored.Current()); + write(*po, w, boost::none, css_class); } } } @@ -2236,6 +2425,35 @@ void SvgSerializer::doWriteHeader() { " fill: none;\n" " stroke-opacity: 0.6;\n" " }\n" + // SVG edge classification (issue #3668) -- see edge-classification.md. These + // select directly on the element (each classified edge carries its own + // class), not on an ancestor , so they win over the inherited .projection + // path rule above regardless of specificity. + " path.outline {\n" + " stroke: #000000;\n" + " stroke-width: 0.35px;\n" + " stroke-opacity: 1;\n" + " }\n" + " path.boundary {\n" + " stroke: #000000;\n" + " stroke-width: 0.3px;\n" + " stroke-opacity: 0.9;\n" + " }\n" + " path.sharp {\n" + " stroke: #000000;\n" + " stroke-width: 0.25px;\n" + " stroke-opacity: 0.85;\n" + " }\n" + " path.crease {\n" + " stroke: #000000;\n" + " stroke-width: 0.18px;\n" + " stroke-opacity: 0.7;\n" + " }\n" + " path.flush {\n" + " stroke: #000000;\n" + " stroke-width: 0.1px;\n" + " stroke-opacity: 0.4;\n" + " }\n" " .IfcDoor path,\n" " .Symbol path {\n" " fill: none;\n" diff --git a/src/serializers/SvgSerializer.h b/src/serializers/SvgSerializer.h index 7e564c589b..223bc5e441 100644 --- a/src/serializers/SvgSerializer.h +++ b/src/serializers/SvgSerializer.h @@ -56,6 +56,7 @@ #include #include #include +#include typedef std::pair drawing_key; @@ -212,9 +213,16 @@ namespace { private: const HLRAlgo_Projector& projector_; const std::list>* product_shapes_ = nullptr; + // SVG edge classification (issue #3668): per-(product, class) edge-only sub-shapes, + // classified pre-HLR on the original (real-face) topology. Queried via + // VCompound(S)/OutLineVCompound(S), which correlate by the identity of the *original* + // edges added to the algorithm -- not by the reconstructed output -- so this works even + // though HLR's own output compounds carry no face topology at all. Empty class string + // means "unclassified" (used for the two fallback cases below). + const std::list>* classified_shapes_ = nullptr; public: - typedef std::list> result_type; + typedef std::list> result_type; hlr_calc(const HLRAlgo_Projector& projector) : projector_(projector) {} @@ -223,24 +231,37 @@ namespace { product_shapes_ = product_shapes; } + void set_classified_shapes(const std::list>* classified_shapes) { + classified_shapes_ = classified_shapes; + } + result_type operator()(boost::blank&) const { throw std::runtime_error(""); } + template + result_type extract(HlrToShapeT& hlr_shapes) { + result_type r; + if (classified_shapes_ && !classified_shapes_->empty()) { + for (auto& t : *classified_shapes_) { + r.push_back({ std::get<0>(t), std::get<1>(t), occt_join(hlr_shapes.OutLineVCompound(std::get<2>(t)), hlr_shapes.VCompound(std::get<2>(t))) }); + } + } else if (product_shapes_) { + for (auto& p : *product_shapes_) { + r.push_back({ p.first, std::string(), occt_join(hlr_shapes.OutLineVCompound(p.second), hlr_shapes.VCompound(p.second)) }); + } + } else { + r.push_back({ nullptr, std::string(), occt_join(hlr_shapes.OutLineVCompound(), hlr_shapes.VCompound()) }); + } + return r; + } + result_type operator()(opencascade::handle& algo) { algo->Projector(projector_); algo->Update(); algo->Hide(); HLRBRep_HLRToShape hlr_shapes(algo); - if (product_shapes_) { - std::list> r; - for (auto& p : *product_shapes_) { - r.push_back({ p.first, occt_join(hlr_shapes.OutLineVCompound(p.second), hlr_shapes.VCompound(p.second)) }); - } - return r; - } else { - return { {nullptr, occt_join(hlr_shapes.OutLineVCompound(), hlr_shapes.VCompound())}}; - } + return extract(hlr_shapes); } result_type operator()(opencascade::handle& algo) { @@ -248,15 +269,7 @@ namespace { algo->Update(); HLRBRep_PolyHLRToShape hlr_shapes; hlr_shapes.Update(algo); - if (product_shapes_) { - std::list> r; - for (auto& p : *product_shapes_) { - r.push_back({ p.first, occt_join(hlr_shapes.OutLineVCompound(p.second), hlr_shapes.VCompound(p.second)) }); - } - return r; - } else { - return { {nullptr, occt_join(hlr_shapes.OutLineVCompound(), hlr_shapes.VCompound()) } }; - } + return extract(hlr_shapes); } }; @@ -367,6 +380,8 @@ namespace { std::multimap large_ortho_faces_; std::list> items_; + // SVG edge classification (issue #3668): see add_classified_edges(). + std::list> classified_items_; Logger& logger_; @@ -391,6 +406,16 @@ namespace { projector_ = HLRAlgo_Projector(trsf, false, 1.); } + // SVG edge classification (issue #3668): register an edge-only sub-shape of `product`'s + // original (pre-HLR, real-face) geometry under a given class name (e.g. "outline", + // "sharp"). The full shape must still be added via add() as usual for correct occlusion; + // this only affects which *class* each edge's visible portion is later extracted as, via + // HLRBRep_HLRToShape::VCompound(S)/OutLineVCompound(S) in hlr_calc, which correlate by the + // identity of the original edges within S. + void add_classified_edges(const IfcUtil::IfcBaseEntity* product, const std::string& cls, const TopoDS_Shape& edges) { + classified_items_.push_back({ product, cls, edges }); + } + bool is_obscured_(TopoDS_Shape* sit) { const TopoDS_Shape& s = *sit; @@ -510,7 +535,7 @@ namespace { } } - std::list> build() { + std::list> build() { size_t n_included = 0; for (auto it = items_.begin(); it != items_.end(); ++it) { if (!use_prefiltering_ || !is_obscured_(&it->second)) { @@ -522,11 +547,12 @@ namespace { if (use_prefiltering_) { logger_.Notice("SER", 35, "Included " + std::to_string(n_included) + " elements out of " + std::to_string(items_.size()) + " after prefiltering"); } - + hlr_calc vis(projector_); if (segment_projection_) { vis.set_product_shape(&items_); } + vis.set_classified_shapes(&classified_items_); return boost::apply_visitor(vis, engine_); } }; @@ -570,6 +596,11 @@ protected: int profile_threshold_; + // SVG edge classification (issue #3668): see classify_edge_from_faces() in SvgSerializer.cpp. + double svg_ridge_angle_min_deg_; + double svg_valley_angle_min_deg_; + bool svg_emit_flush_edges_; + IfcParse::IfcFile* file; const IfcUtil::IfcBaseEntity* storey_; std::multimap paths; @@ -623,6 +654,9 @@ public: , mirror_x_(false) , unify_inputs_(false) , profile_threshold_(-1) + , svg_ridge_angle_min_deg_(45.) + , svg_valley_angle_min_deg_(12.) + , svg_emit_flush_edges_(false) , file(0) , storey_(0) , xcoords_begin(0) @@ -641,7 +675,7 @@ public: bool ready(); void write(const IfcGeom::TriangulationElement* /*o*/) {} void write(const IfcGeom::BRepElement* o); - void write(path_object& p, const TopoDS_Shape& wire, boost::optional> dash_array=boost::none); + void write(path_object& p, const TopoDS_Shape& wire, boost::optional> dash_array=boost::none, boost::optional css_class=boost::none); void write(const geometry_data& data); path_object& start_path(const gp_Pln& p, const IfcUtil::IfcBaseEntity* storey, const std::string& id); path_object& start_path(const gp_Pln& p, const std::string& drawing_name, const std::string& id); From 8857396a1f86d5b93681d3a7e627f0f240123c6c Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Wed, 15 Jul 2026 23:28:43 +0100 Subject: [PATCH 2/7] Fix SVG edge classification sign/threshold bugs Fixes three bugs in classify_edge_from_faces() found via real-world testing against a dedicated stress-test scene (icosphere, Suzanne, cylinders/cones at various orientations, a dihedral-angle sweep rig): - The outline (silhouette) test used a bare sign comparison, so a face at or near exactly edge-on to the camera could land on the wrong side of zero and fall through to angle-based classification instead of being drawn as outline. Now uses a tolerance band around zero, matching an equivalent check already used elsewhere in this file. - The signed deviation-from-flat formula was inverted (180 - angle instead of angle), so small, genuinely near-flat facet angles came out with a large computed deviation and always classified as sharp/crease, never flush. This is why thresholds appeared to have no effect. Also replaced the edge/wire-orientation-based convexity sign (unreliable on real BRep topology, verified wrong against a known fully-convex icosphere) with a simpler position-based test. - A specific edge that was previously missing entirely (not just misclassified) reappears correctly as a side effect of the outline fix above; no separate change was needed for it. A fourth issue (folds viewed through an opening, e.g. a box missing a face, should read as crease rather than sharp) was attempted via a back-facing sign flip, but reverted: it broke the fixes above broadly, since "both faces back-facing" isn't a rare look-through-a-hole case once HLR has already filtered to visible edges only. Documented in a code comment for whoever picks this up next. Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Sonnet 5 --- src/serializers/SvgSerializer.cpp | 90 +++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 23 deletions(-) diff --git a/src/serializers/SvgSerializer.cpp b/src/serializers/SvgSerializer.cpp index 24bc2c561a..7770659aa7 100644 --- a/src/serializers/SvgSerializer.cpp +++ b/src/serializers/SvgSerializer.cpp @@ -872,39 +872,83 @@ namespace { return edge_style_class::outline; } - const double d0 = projection_direction.Dot(n0); - const double d1 = projection_direction.Dot(n1); + // Note the negation: `projection_direction` (as constructed by the caller from the + // drawing plane's axis) points from the scene *towards the camera*, not into the scene. + // A face that's actually front-facing (visible, facing the viewer) has an outward normal + // pointing the same general way as that -- i.e. a *positive* dot product -- so negate + // here to get the more intuitive "front-facing is negative" convention used below. + // Confirmed against this feature's own real-world test scene: the SOUTH ELEVATION + // camera's placement matrix transforms local +Z (what the un-negated projection_direction + // is built from) to world (0, 1, 0), while the camera's actual Blender-convention view + // direction (local -Z) transforms to world (0, -1, 0) -- i.e. exactly opposite. + const double d0 = -projection_direction.Dot(n0); + const double d1 = -projection_direction.Dot(n1); - // Outline: silhouette, either against the background or self-occluding -- one face - // turns toward the viewer while the other turns away. - if ((d0 < 0.0) != (d1 < 0.0)) { + // Front/back/edge-on classification of each face relative to the view direction, using + // a tolerance band around zero rather than a bare sign comparison. A face at or near + // edge-on to the camera (|d| within the band) is common for regular/symmetric + // tessellations viewed from "nice" angles (icospheres, N-gon cylinder/cone + // approximations) and must count as outline on both its edges, not just the one that + // happens to pair it with a clearly front-facing neighbour. + constexpr double kOutlineDotEps = 1.e-5; + const bool front0 = d0 < -kOutlineDotEps; + const bool back0 = d0 > kOutlineDotEps; + const bool front1 = d1 < -kOutlineDotEps; + const bool back1 = d1 > kOutlineDotEps; + + // Outline: silhouette, either a genuine front/back flip, or either face is at/near + // edge-on to the view direction (also covers both faces edge-on at once). + if (!(front0 && front1) && !(back0 && back1)) { return edge_style_class::outline; } - // Signed deviation from flat (180 degrees between outward normals = perfectly flat). - // Positive = convex (ridge/sharp), negative = concave (valley/crease). The sign comes - // from the rotation of n0 onto n1 about the edge tangent. - const double angle_between_normals_deg = std::acos(clamp_dot(n0.Dot(n1))) * 180.0 / M_PI; - double deviation_deg = 180.0 - angle_between_normals_deg; + // Signed deviation from flat (0 degrees between outward normals = perfectly flat, i.e. + // coplanar faces have identical outward normals). Positive = convex (ridge/sharp), + // negative = concave (valley/crease). + // + // Sign via a position-based (not orientation-based) test: find a vertex of f1 that + // isn't one of the shared edge's own endpoints, and check which side of f0's plane it + // falls on. If it's behind f0's plane (opposite side from f0's outward normal), f1 + // curves back towards the solid's interior relative to f0 -- a convex fold, like a box + // corner. This avoids relying on TopoDS_Edge/wire orientation semantics (which proved + // unreliable in practice: an earlier attempt using edge.Orientation() combined with + // cross(n0, n1) gave a self-consistent-looking but wrong sign on real BRep topology -- + // verified against known-convex geometry, e.g. every edge of a convex icosphere, where + // that approach misclassified a majority of edges as concave). + double deviation_deg = std::acos(clamp_dot(n0.Dot(n1))) * 180.0 / M_PI; - double u0, u1; - Handle(Geom_Curve) curve = BRep_Tool::Curve(edge, u0, u1); - if (!curve.IsNull()) { - gp_Pnt p_mid; - gp_Vec tangent; - curve->D1((u0 + u1) / 2.0, p_mid, tangent); - if (tangent.SquareMagnitude() > 1.e-10) { - tangent.Normalize(); - if (edge.Orientation() == TopAbs_REVERSED) { - tangent.Reverse(); - } - const gp_Vec cross = gp_Vec(n0.XYZ()).Crossed(gp_Vec(n1.XYZ())); - if (cross.Dot(tangent) < 0.0) { + TopoDS_Vertex ev0, ev1; + TopExp::Vertices(edge, ev0, ev1); + const gp_Pnt edge_p0 = BRep_Tool::Pnt(ev0); + const gp_Pnt edge_p1 = BRep_Tool::Pnt(ev1); + + for (TopExp_Explorer vexp(f1, TopAbs_VERTEX); vexp.More(); vexp.Next()) { + const gp_Pnt p = BRep_Tool::Pnt(TopoDS::Vertex(vexp.Current())); + if (p.Distance(edge_p0) > Precision::Confusion() && p.Distance(edge_p1) > Precision::Confusion()) { + const bool convex = gp_Vec(edge_p0, p).Dot(gp_Vec(n0.XYZ())) < 0.0; + if (!convex) { deviation_deg = -deviation_deg; } + break; } } + // NOTE: an attempt to flip the sign when "both faces back-facing" (viewing a fold's + // reverse/inside surface through an opening, e.g. a box with a face removed) was tried + // here and reverted -- see edge-classification.md follow-up notes. front0/back0 (and + // front1/back1) reliably distinguish "genuine front/back flip" for the outline test + // above, but using them to guess "are we looking at this fold from behind" is unsound: + // by the time an edge is visible in the output at all, HLR has already decided it's not + // occluded, so for an ordinary closed solid essentially every remaining edge still + // reads as "both back-facing" about as often as "both front-facing" (there's no cheap + // way here to tell "genuinely viewed through a hole" apart from "ordinary far side of a + // closed shape that happens to share this classification bucket"). Enabling either + // polarity of this flip corrupted otherwise-correct classification broadly (verified + // against a fully-convex icosphere test case, where it manufactured large numbers of + // spurious `crease` edges that should have been `flush`). Needs a different approach + // (e.g. an explicit visibility/occlusion signal rather than inferring it from face + // normals) before revisiting. + if (deviation_deg >= 0.0) { return (deviation_deg >= ridge_angle_min_deg) ? edge_style_class::sharp : edge_style_class::flush; } else { From 2e9e75c7bd2364c18cf5ef3a7d91895d5db54f74 Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Thu, 16 Jul 2026 00:36:54 +0100 Subject: [PATCH 3/7] Fix Issue 4: gate the back-facing crease flip by threshold Re-enable the view-relative sign flip for folds seen through an opening (e.g. a box with a face removed), reverted in the previous commit after it corrupted unrelated geometry. The earlier revert's diagnosis was slightly off: bucket reassignment can't affect HLR's own visibility computation, so the corruption was actually an asymmetric-threshold artifact -- an unconditional flip re-tested small, correctly-flush deviations against the much smaller valley threshold instead of the ridge one. Gating the flip so it only reinterprets folds that already clear their own pre-flip threshold fixes the box case while leaving every other test object's classification unchanged (verified against the full test scene). Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Sonnet 5 --- src/serializers/SvgSerializer.cpp | 53 ++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/src/serializers/SvgSerializer.cpp b/src/serializers/SvgSerializer.cpp index 7770659aa7..f8fb64fa0c 100644 --- a/src/serializers/SvgSerializer.cpp +++ b/src/serializers/SvgSerializer.cpp @@ -933,21 +933,44 @@ namespace { } } - // NOTE: an attempt to flip the sign when "both faces back-facing" (viewing a fold's - // reverse/inside surface through an opening, e.g. a box with a face removed) was tried - // here and reverted -- see edge-classification.md follow-up notes. front0/back0 (and - // front1/back1) reliably distinguish "genuine front/back flip" for the outline test - // above, but using them to guess "are we looking at this fold from behind" is unsound: - // by the time an edge is visible in the output at all, HLR has already decided it's not - // occluded, so for an ordinary closed solid essentially every remaining edge still - // reads as "both back-facing" about as often as "both front-facing" (there's no cheap - // way here to tell "genuinely viewed through a hole" apart from "ordinary far side of a - // closed shape that happens to share this classification bucket"). Enabling either - // polarity of this flip corrupted otherwise-correct classification broadly (verified - // against a fully-convex icosphere test case, where it manufactured large numbers of - // spurious `crease` edges that should have been `flush`). Needs a different approach - // (e.g. an explicit visibility/occlusion signal rather than inferring it from face - // normals) before revisiting. + // View-relative flip for folds seen from behind through an opening (e.g. the "Rotated + // Box w/Boundary" test object -- a box with one face removed; the 3 interior lines + // visible through the opening read as the *inside* of an ordinary convex box corner, + // which should look like a crease, not a sharp ridge). Two earlier unconditional + // versions of this flip (triggered on plain back0&&back1, with no further gate) were + // tried and reverted -- see edge-classification.md follow-up notes -- because they + // corrupted otherwise-correct classification broadly, manifesting as spurious `crease` + // edges on a fully-convex icosphere test case that has no opening at all. + // + // That corruption wasn't a fundamental inability to distinguish "genuinely seen through + // a hole" from "ordinary far side of closed geometry": bucket membership here is purely + // a post-hoc query key into an already-completed, correct HLR visibility computation, + // so reclassifying an edge can never make a genuinely hidden edge appear or vice versa. + // The real cause is a threshold-crossing artifact: near the silhouette, facet-normal + // noise on regular/symmetric tessellations (icospheres, N-gon cylinder/cone + // approximations) makes some genuinely near-edge-on facets test as "back" under the + // flat-normal-based back0/back1 test even though they're still visible. An unconditional + // negate then took their small, correctly-`flush` solid-relative deviation and re-tested + // it against the *other* threshold -- `ridge_angle_min_deg` (45 degrees by default) and + // `valley_angle_min_deg` (12 degrees by default) are deliberately asymmetric, so a gentle + // ~20 degree convex facet transition that safely sits under the ridge threshold crosses + // well over the much smaller valley threshold once flipped, becoming a spurious `crease`. + // + // Fix: gate the flip so it can only reinterpret a fold that would already be visible + // (sharp or crease) under its own pre-flip threshold -- i.e. only folds sharp/deep + // enough to draw from the front get reinterpreted as the opposite class from behind. + // Gentle tessellation-noise deviations that are correctly `flush` either way never cross + // the asymmetric threshold gap, because they never reach the flip at all. Verified + // against the full test scene: every object's classification is byte-for-byte unchanged + // except "Rotated Box w/Boundary", whose 3 interior lines now correctly read `crease` + // (previously all 4 non-boundary edges read `sharp`). + if (back0 && back1) { + const bool would_show_unflipped = + (deviation_deg >= 0.0) ? (deviation_deg >= ridge_angle_min_deg) : (-deviation_deg >= valley_angle_min_deg); + if (would_show_unflipped) { + deviation_deg = -deviation_deg; + } + } if (deviation_deg >= 0.0) { return (deviation_deg >= ridge_angle_min_deg) ? edge_style_class::sharp : edge_style_class::flush; From 2ac92f01e46c4e324ed148c4951ec9841d376087 Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Thu, 16 Jul 2026 04:02:57 +0100 Subject: [PATCH 4/7] Fix missing silhouette on curved analytic column/pile faces Circular-profile IfcColumn/IfcPile elements produce a genuine analytic cylindrical BRep face (via BRepPrimAPI_MakePrism), not a tessellated facet. The edge classification/extraction pipeline is edge-identity-based end to end, but a smooth surface's silhouette is synthesized by HLR on the fly and has no corresponding pre-existing edge to bucket, so it was silently dropped once any edge in the product had been classified. Add a face-level pass that includes any non-planar face directly in the outline bucket, giving HLR's per-face OutLine reconstruction a face identity to correlate against. Purely additive: diffing the whole test scene's output before and after shows only the two previously-missing tangent lines appear, nothing else changes. Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Sonnet 5 --- src/serializers/SvgSerializer.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/serializers/SvgSerializer.cpp b/src/serializers/SvgSerializer.cpp index f8fb64fa0c..2a4cb9c2b4 100644 --- a/src/serializers/SvgSerializer.cpp +++ b/src/serializers/SvgSerializer.cpp @@ -1436,6 +1436,29 @@ void SvgSerializer::write(const geometry_data& data) { } BBcls.Add(bucket_it->second, cls_edge); } + + // Non-planar faces (e.g. a real analytic cylindrical wall from a + // circular-profile column/pile, swept via BRepPrimAPI_MakePrism rather than + // faceted) have a silhouette that HLR synthesizes on the fly -- it is not a + // pre-existing topological edge, so the edge-only loop above can never bucket + // it. OutLineVCompound(S) correlates a curved face's silhouette by the + // identity of the originating *face*, not any edge, so add the non-planar + // face itself into the outline bucket alongside whatever edges it already + // contributed (top/bottom/seam), giving HLR's per-face OutLine reconstruction + // something to match against. + for (TopExp_Explorer fexp(*compound_to_hlr, TopAbs_FACE); fexp.More(); fexp.Next()) { + const TopoDS_Face& f = TopoDS::Face(fexp.Current()); + if (BRep_Tool::Surface(f)->DynamicType() != STANDARD_TYPE(Geom_Plane)) { + std::string name = edge_style_class_name(edge_style_class::outline); + auto bucket_it = classified_edge_buckets.find(name); + if (bucket_it == classified_edge_buckets.end()) { + TopoDS_Compound c; + BBcls.MakeCompound(c); + bucket_it = classified_edge_buckets.emplace(name, c).first; + } + BBcls.Add(bucket_it->second, f); + } + } } if (is_floor_plan_) { From 183e4c47f7a413a974c5863f11cfce80308fc638 Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Thu, 16 Jul 2026 15:29:22 +0100 Subject: [PATCH 5/7] 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); } From f3a7a35acfdb14118f069d63cc25de2cf932cf4c Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Thu, 16 Jul 2026 15:29:44 +0100 Subject: [PATCH 6/7] Expose SVG edge classification settings in drawing UI Add UseEdgeClassification, RenderCreases, ValleyAngleMinDegrees, RenderSharp, RidgeAngleMinDegrees, and RenderFlush to EPset_Drawing, following the existing HasUnderlay/DPI/PerspectiveShiftX pattern. The master toggle defaults off, preserving current linework output; the three dependent controls only show in the panel once it's on. Removes the previous dormant, transient operator-redo properties for the ridge/valley thresholds and flush-edge toggle, which were never persisted per-drawing or exposed in any panel, replacing them with the persistent camera properties read in setup_serialiser(). Generated with the assistance of an AI coding tool. Co-Authored-By: Claude Sonnet 5 --- .../bonsai/bim/data/pset/EPset_Drawing.ifc | 8 +++- .../bonsai/bim/module/drawing/operator.py | 37 ++++----------- src/bonsai/bonsai/bim/module/drawing/prop.py | 44 ++++++++++++++++++ src/bonsai/bonsai/bim/module/drawing/ui.py | 13 ++++++ src/bonsai/bonsai/tool/drawing.py | 18 ++++++++ src/bonsai/test/tool/test_drawing.py | 45 +++++++++++++++++++ 6 files changed, 135 insertions(+), 30 deletions(-) diff --git a/src/bonsai/bonsai/bim/data/pset/EPset_Drawing.ifc b/src/bonsai/bonsai/bim/data/pset/EPset_Drawing.ifc index 114b766ff8..4d8bc1ad47 100644 --- a/src/bonsai/bonsai/bim/data/pset/EPset_Drawing.ifc +++ b/src/bonsai/bonsai/bim/data/pset/EPset_Drawing.ifc @@ -5,7 +5,7 @@ FILE_NAME('EPset_Drawing.ifc','2020-01-01T00:00:00',$,$,'EPset_Drawing','EPset_D FILE_SCHEMA(('IFC4')); ENDSEC; DATA; -#1=IFCPROPERTYSETTEMPLATE('2JhNIvqZrFnAgxfhK0XVQX',$,'EPset_Drawing','',.PSET_OCCURRENCEDRIVEN.,'IfcAnnotation/DRAWING',(#23,#22,#27,#24,#29,#30,#19,#12,#26,#9,#8,#7,#6,#4,#18,#11,#5,#20,#25,#14,#10,#17,#28,#16,#3,#21,#13,#15,#2)); +#1=IFCPROPERTYSETTEMPLATE('2JhNIvqZrFnAgxfhK0XVQX',$,'EPset_Drawing','',.PSET_OCCURRENCEDRIVEN.,'IfcAnnotation/DRAWING',(#23,#22,#27,#24,#29,#30,#19,#12,#26,#9,#8,#7,#6,#4,#18,#11,#5,#20,#25,#14,#10,#17,#28,#16,#3,#21,#13,#15,#2,#31,#32,#33,#34,#35,#36)); #2=IFCSIMPLEPROPERTYTEMPLATE('23JavTMk98ZxXhrUEnjAcf',$,'TargetView','',.P_SINGLEVALUE.,'IfcLabel',$,$,$,$,$,.READWRITE.); #3=IFCSIMPLEPROPERTYTEMPLATE('1yVWUt5H9DAOuu0OaMMLpe',$,'Scale','The scale of this drawing represented as a numerator and denominator, such as 1/100',.P_SINGLEVALUE.,'IfcLabel',$,$,$,$,$,.READWRITE.); #4=IFCSIMPLEPROPERTYTEMPLATE('3gsuPBtU93b8f0gg1pjkq6',$,'HumanScale','The scale of this drawing in human readable format, such as 1:100',.P_SINGLEVALUE.,'IfcLabel',$,$,$,$,$,.READWRITE.); @@ -35,5 +35,11 @@ DATA; #28=IFCSIMPLEPROPERTYTEMPLATE('1YSnFzurrEyRNtoLdmmddP',$,'BringToFront','The objects with these SVG classes will render in front of all other objects.Ex: IfcBeam, IfcColumn',.P_SINGLEVALUE.,'IfcText',$,$,$,$,$,.READWRITE.); #29=IFCSIMPLEPROPERTYTEMPLATE('0lP6Y8q9v2QhDnR4sT7uVx',$,'PerspectiveShiftX','Horizontal perspective camera shift stored as drawing metadata using Blender camera shift units.',.P_SINGLEVALUE.,'IfcReal',$,$,$,$,$,.READWRITE.); #30=IFCSIMPLEPROPERTYTEMPLATE('2mR8b1NcW5EoFyG7hJ9kLp',$,'PerspectiveShiftY','Vertical perspective camera shift stored as drawing metadata using Blender camera shift units.',.P_SINGLEVALUE.,'IfcReal',$,$,$,$,$,.READWRITE.); +#31=IFCSIMPLEPROPERTYTEMPLATE('1cFVJnqT13m8ItkMHaI1tp',$,'UseEdgeClassification','Enable the boundary/outline/sharp/crease/flush SVG edge classification scheme (issue #3668). When false, drawings use the original unclassified linework.',.P_SINGLEVALUE.,'IfcBoolean',$,$,$,$,$,.READWRITE.); +#32=IFCSIMPLEPROPERTYTEMPLATE('2kB$mxBgnBUvhjh0Ti0c4P',$,'RenderCreases','Whether to render ''crease'' (concave) edges. Only relevant when UseEdgeClassification is enabled.',.P_SINGLEVALUE.,'IfcBoolean',$,$,$,$,$,.READWRITE.); +#33=IFCSIMPLEPROPERTYTEMPLATE('3MSIJNW$T8r9Hl12kk0BY$',$,'ValleyAngleMinDegrees','Minimum concave dihedral deviation from flat, in degrees, for a projection edge to be classified as ''crease'' rather than ''flush''.',.P_SINGLEVALUE.,'IfcReal',$,$,$,$,$,.READWRITE.); +#34=IFCSIMPLEPROPERTYTEMPLATE('2epSGfC4bFM9gb1X7zBIp4',$,'RenderSharp','Whether to render ''sharp'' (convex) edges. Only relevant when UseEdgeClassification is enabled.',.P_SINGLEVALUE.,'IfcBoolean',$,$,$,$,$,.READWRITE.); +#35=IFCSIMPLEPROPERTYTEMPLATE('3TZwsEjkr5WRDKcgrYzSIA',$,'RidgeAngleMinDegrees','Minimum convex dihedral deviation from flat, in degrees, for a projection edge to be classified as ''sharp'' rather than ''flush''.',.P_SINGLEVALUE.,'IfcReal',$,$,$,$,$,.READWRITE.); +#36=IFCSIMPLEPROPERTYTEMPLATE('2Jua$lO754vgZOkBoHM2gA',$,'RenderFlush','Whether to render ''flush'' edges (dihedral deviation below both ridge/valley thresholds). Only relevant when UseEdgeClassification is enabled.',.P_SINGLEVALUE.,'IfcBoolean',$,$,$,$,$,.READWRITE.); ENDSEC; END-ISO-10303-21; diff --git a/src/bonsai/bonsai/bim/module/drawing/operator.py b/src/bonsai/bonsai/bim/module/drawing/operator.py index 9a8faf4d58..b6017f0704 100644 --- a/src/bonsai/bonsai/bim/module/drawing/operator.py +++ b/src/bonsai/bonsai/bim/module/drawing/operator.py @@ -261,36 +261,11 @@ class CreateDrawing(bpy.types.Operator): description="Could save some time if you're sure IFC and current Blender session are already in sync", default=True, ) - svg_ridge_angle_min_deg: bpy.props.FloatProperty( - name="Ridge Angle Minimum", - description="Minimum convex dihedral deviation from flat, in degrees, for a projection " - "edge to be classified as 'sharp' rather than 'flush'. See edge-classification.md", - default=45.0, - min=0.0, - max=180.0, - ) - svg_valley_angle_min_deg: bpy.props.FloatProperty( - name="Valley Angle Minimum", - description="Minimum concave dihedral deviation from flat, in degrees, for a projection " - "edge to be classified as 'crease' rather than 'flush'. See edge-classification.md", - default=12.0, - min=0.0, - max=180.0, - ) - svg_emit_flush_edges: bpy.props.BoolProperty( - name="Emit Flush Edges", - description="Include projection edges whose dihedral deviation is below both the ridge " - "and valley thresholds (class 'flush'). Omitted by default", - default=False, - ) if TYPE_CHECKING: print_all: bool open_viewer: bool sync: bool - svg_ridge_angle_min_deg: float - svg_valley_angle_min_deg: float - svg_emit_flush_edges: bool drawing_name: str is_manifold_cache: dict[str, bool] @@ -1334,11 +1309,15 @@ class CreateDrawing(bpy.types.Operator): self.svg_settings = ifcopenshell.geom.settings() self.svg_settings.set("dimensionality", ifcopenshell.ifcopenshell_wrapper.CURVES_SURFACES_AND_SOLIDS) self.svg_settings.set("iterator-output", ifcopenshell.ifcopenshell_wrapper.NATIVE) - # SVG edge classification (issue #3668). See edge-classification.md. + # SVG edge classification (issue #3668). See edge-classification.md. Settings are + # per-drawing, stored in EPset_Drawing and read into self.cprops by import_camera_props. try: - self.svg_settings.set("svg-ridge-angle-min-degrees", self.svg_ridge_angle_min_deg) - self.svg_settings.set("svg-valley-angle-min-degrees", self.svg_valley_angle_min_deg) - self.svg_settings.set("svg-emit-flush-edges", self.svg_emit_flush_edges) + self.svg_settings.set("svg-use-edge-classification", self.cprops.use_edge_classification) + self.svg_settings.set("svg-render-crease-edges", self.cprops.render_creases) + self.svg_settings.set("svg-valley-angle-min-degrees", self.cprops.valley_angle_min_degrees) + self.svg_settings.set("svg-render-sharp-edges", self.cprops.render_sharp) + self.svg_settings.set("svg-ridge-angle-min-degrees", self.cprops.ridge_angle_min_degrees) + self.svg_settings.set("svg-emit-flush-edges", self.cprops.render_flush) except Exception: # Backwards compatibility with older ifcopenshell builds that don't expose these keys. pass diff --git a/src/bonsai/bonsai/bim/module/drawing/prop.py b/src/bonsai/bonsai/bim/module/drawing/prop.py index f75de7fd34..f22d7128d8 100644 --- a/src/bonsai/bonsai/bim/module/drawing/prop.py +++ b/src/bonsai/bonsai/bim/module/drawing/prop.py @@ -536,6 +536,50 @@ class BIMCameraProperties(PropertyGroup): default=True, update=get_update_layer_callback("has_annotation", "HasAnnotation"), ) + use_edge_classification: BoolProperty( + name="Use Edge Classification", + description="Classify projection edges into boundary/outline/sharp/crease/flush " + "instead of drawing all linework identically. See edge-classification.md", + default=False, + update=get_update_layer_callback("use_edge_classification", "UseEdgeClassification"), + ) + render_creases: BoolProperty( + name="Render Creases", + description="Render 'crease' (concave) projection edges", + default=True, + update=get_update_layer_callback("render_creases", "RenderCreases"), + ) + valley_angle_min_degrees: FloatProperty( + name="Valley Angle Minimum", + description="Minimum concave dihedral deviation from flat, in degrees, for a projection " + "edge to be classified as 'crease' rather than 'flush'", + default=12.0, + min=0.0, + max=180.0, + update=get_update_layer_callback("valley_angle_min_degrees", "ValleyAngleMinDegrees"), + ) + render_sharp: BoolProperty( + name="Render Sharp", + description="Render 'sharp' (convex) projection edges", + default=True, + update=get_update_layer_callback("render_sharp", "RenderSharp"), + ) + ridge_angle_min_degrees: FloatProperty( + name="Ridge Angle Minimum", + description="Minimum convex dihedral deviation from flat, in degrees, for a projection " + "edge to be classified as 'sharp' rather than 'flush'", + default=45.0, + min=0.0, + max=180.0, + update=get_update_layer_callback("ridge_angle_min_degrees", "RidgeAngleMinDegrees"), + ) + render_flush: BoolProperty( + name="Render Flush", + description="Render 'flush' projection edges (dihedral deviation below both ridge/valley " + "thresholds). Omitted by default", + default=False, + update=get_update_layer_callback("render_flush", "RenderFlush"), + ) target_view: EnumProperty( name="Target View", default="PLAN_VIEW", diff --git a/src/bonsai/bonsai/bim/module/drawing/ui.py b/src/bonsai/bonsai/bim/module/drawing/ui.py index e0df93a4a6..640ebf91a3 100644 --- a/src/bonsai/bonsai/bim/module/drawing/ui.py +++ b/src/bonsai/bonsai/bim/module/drawing/ui.py @@ -113,6 +113,19 @@ class BIM_PT_camera(Panel): row.prop(props, "fill_mode") row = self.layout.row() row.prop(props, "cut_mode") + + row = self.layout.row() + row.prop(props, "use_edge_classification") + if props.use_edge_classification: + row = self.layout.row() + row.prop(props, "render_creases") + row.prop(props, "valley_angle_min_degrees") + row = self.layout.row() + row.prop(props, "render_sharp") + row.prop(props, "ridge_angle_min_degrees") + row = self.layout.row() + row.prop(props, "render_flush") + row = self.layout.row() row.prop(props, "width") row = self.layout.row() diff --git a/src/bonsai/bonsai/tool/drawing.py b/src/bonsai/bonsai/tool/drawing.py index 7493b02d33..8b6ca68b0e 100644 --- a/src/bonsai/bonsai/tool/drawing.py +++ b/src/bonsai/bonsai/tool/drawing.py @@ -1072,6 +1072,12 @@ class Drawing(bonsai.core.tool.Drawing): camera_props.has_annotation = True camera_props.target_view = "PLAN_VIEW" camera_props.is_nts = False + camera_props.use_edge_classification = False + camera_props.render_creases = True + camera_props.valley_angle_min_degrees = 12.0 + camera_props.render_sharp = True + camera_props.ridge_angle_min_degrees = 45.0 + camera_props.render_flush = False camera.shift_x = 0.0 camera.shift_y = 0.0 @@ -1101,6 +1107,18 @@ class Drawing(bonsai.core.tool.Drawing): camera_props.has_annotation = bool(pset["HasAnnotation"]) if "IsNTS" in pset: camera_props.is_nts = bool(pset["IsNTS"]) + if "UseEdgeClassification" in pset: + camera_props.use_edge_classification = bool(pset["UseEdgeClassification"]) + if "RenderCreases" in pset: + camera_props.render_creases = bool(pset["RenderCreases"]) + if "ValleyAngleMinDegrees" in pset: + camera_props.valley_angle_min_degrees = float(pset["ValleyAngleMinDegrees"]) + if "RenderSharp" in pset: + camera_props.render_sharp = bool(pset["RenderSharp"]) + if "RidgeAngleMinDegrees" in pset: + camera_props.ridge_angle_min_degrees = float(pset["RidgeAngleMinDegrees"]) + if "RenderFlush" in pset: + camera_props.render_flush = bool(pset["RenderFlush"]) if "DPI" in pset: camera_props.dpi = int(pset["DPI"]) if "LineworkMode" in pset: diff --git a/src/bonsai/test/tool/test_drawing.py b/src/bonsai/test/tool/test_drawing.py index 9d69fe51f6..a14b4d9d79 100644 --- a/src/bonsai/test/tool/test_drawing.py +++ b/src/bonsai/test/tool/test_drawing.py @@ -112,6 +112,51 @@ class TestImportCameraProps(NewFile): assert camera.shift_x == 0.0 assert camera.shift_y == 0.0 + def test_defaults_edge_classification_props_when_pset_is_absent(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + drawing = ifc.createIfcAnnotation(ObjectType="DRAWING") + camera = bpy.data.cameras.new("Camera") + + subject.import_camera_props(drawing, camera) + + props = subject.get_camera_props(camera) + assert props.use_edge_classification is False + assert props.render_creases is True + assert props.valley_angle_min_degrees == pytest.approx(12.0) + assert props.render_sharp is True + assert props.ridge_angle_min_degrees == pytest.approx(45.0) + assert props.render_flush is False + + def test_imports_edge_classification_props_from_drawing_pset(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + drawing = ifc.createIfcAnnotation(ObjectType="DRAWING") + pset = ifcopenshell.api.pset.add_pset(ifc, product=drawing, name="EPset_Drawing") + ifcopenshell.api.pset.edit_pset( + ifc, + pset=pset, + properties={ + "UseEdgeClassification": True, + "RenderCreases": False, + "ValleyAngleMinDegrees": 8.0, + "RenderSharp": False, + "RidgeAngleMinDegrees": 30.0, + "RenderFlush": True, + }, + ) + camera = bpy.data.cameras.new("Camera") + + subject.import_camera_props(drawing, camera) + + props = subject.get_camera_props(camera) + assert props.use_edge_classification is True + assert props.render_creases is False + assert props.valley_angle_min_degrees == pytest.approx(8.0) + assert props.render_sharp is False + assert props.ridge_angle_min_degrees == pytest.approx(30.0) + assert props.render_flush is True + class TestSyncPerspectiveCameraShifts(NewFile): def test_round_trips_perspective_camera_shifts_through_drawing_pset(self): From 93c0290131f73c9bbc5984cee84e1fe6af7fbba0 Mon Sep 17 00:00:00 2001 From: Stephen Boddy Date: Fri, 17 Jul 2026 17:54:06 +0100 Subject: [PATCH 7/7] Minor tweak to the default lining weights The crease and sharp weighting seemed flipped to my sensibilities, so now crease is heavier than sharp. I also added a commented out block for debug colours in case someone wants to quickly use bright colours to diagnose future problems. --- src/bonsai/bonsai/bim/data/assets/default.css | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/bonsai/bonsai/bim/data/assets/default.css b/src/bonsai/bonsai/bim/data/assets/default.css index 5e4670af4d..d82d97e8f2 100644 --- a/src/bonsai/bonsai/bim/data/assets/default.css +++ b/src/bonsai/bonsai/bim/data/assets/default.css @@ -29,13 +29,23 @@ a:hover { cursor: pointer; } the inherited .projection rule above regardless of specificity. */ path.outline { stroke: black; stroke-width: 0.35; stroke-opacity: 1; } path.boundary { stroke: black; stroke-width: 0.3; stroke-opacity: 0.9; } -path.sharp { stroke: black; stroke-width: 0.25; stroke-opacity: 0.85; } -path.crease { stroke: black; stroke-width: 0.18; stroke-opacity: 0.7; } +path.crease { stroke: black; stroke-width: 0.25; stroke-opacity: 0.85; } +path.sharp { stroke: black; stroke-width: 0.18; stroke-opacity: 0.7; } path.flush { stroke: black; stroke-width: 0.1; stroke-opacity: 0.4; } -.surface { stroke: none; fill: #fff; fill-rule: evenodd; } -.annotation { fill: none; stroke: black; stroke-linecap: 'round'; stroke-width: 0.25; } -.IfcAnnotation { fill: none; stroke: black; stroke-linecap: 'round'; stroke-width: 0.25; } -.IfcGeographicElement { fill: none; stroke: black; stroke-linecap: 'round'; stroke-width: 1; } + +/* Debug CSS for troubleshooting edge classification */ +/* +path.outline { stroke: black; stroke-width: 0.35; stroke-opacity: 1; } +path.boundary { stroke: orange; stroke-width: 0.3; stroke-opacity: 0.9; } +path.crease { stroke: green; stroke-width: 0.25; stroke-opacity: 0.85; } +path.sharp { stroke: red; stroke-width: 0.18; stroke-opacity: 0.7; } +path.flush { stroke: blue; stroke-width: 0.1; stroke-opacity: 0.4; } +*/ + +.surface {fill: white; stroke-width: 0.1;} +.annotation { fill: none; stroke: black; stroke-linecap: 'round'; stroke-width: 0.3; } +.IfcAnnotation { fill: none; stroke: black; stroke-linecap: 'round'; stroke-width: 0.3; } +/* .IfcGeographicElement { fill: none; stroke: rgb(150, 150, 150); stroke-linecap: 'round'; stroke-dasharray: 1, 2;} */ .PredefinedType-LINEWORK { stroke: black; stroke-width: 0.25; } .PredefinedType-LINEWORK.dashed { stroke-dasharray: 3, 2; } .PredefinedType-LINEWORK.fine { stroke-width: 0.18; stroke: #777777; }