From 173ab2a3116080151240c0bb3a7e20f54c12dc1b Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Sat, 11 Jul 2026 09:22:12 +0300 Subject: [PATCH] Fix IfcSweptDiskSolidPolygonal fillet: crash to correct rounded corners (#4094) fillet_loop() (the active FilletRadius path, called from IfcSweptDiskSolid) was unfinished ("@todo untested from here") and crashed with GEO027 "BRepAdaptor_Curve::No geometry" on every IfcSweptDiskSolidPolygonal that carries a FilletRadius, emitting no geometry. Four defects: 1. polygon_from_points() shares one point instance between adjacent edges; filleting insets it twice so the arc edge collapses to start == end. 2. (b - 1) % size with unsigned size mis-indexes the previous edge at b = 0. 3. Every vertex was filleted, including the two open-polyline endpoints; at an endpoint the corner is degenerate (ba.cross(bc) == 0), giving a zero-geometry circle, which is the GEO027 crash. 4. The arc centre was offset along the plane normal (out of plane) instead of the interior bisector, and the axis sign selected the major arc. fillet_loop now deduplicates the shared corner points, uses (b + n - 1) % n, detects an open directrix geometrically and skips its endpoints, places the arc centre on the bisector at radius / sin(angle / 2), derives the axis from the two tangent points so the minor (fillet) arc is swept, and clamps the acos argument. A polyhedral guard logs GEO328 and skips a genuinely curved directrix instead of mangling it. Verified on OCC 7.9.2: the reported 3D-poly file goes from a GEO027 crash to a rounded solid; a well-fitting radius (0.1) is a closed manifold (520 verts, 0 boundary / 0 non-manifold edges); the 2D indexed-polycurve case is fixed identically; a curved line+arc directrix no longer crashes or mangles (closed manifold); and the no-fillet path is unchanged. Co-Authored-By: Claude Opus 4.8 --- src/ifcgeom/profile_helper.cpp | 82 ++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 19 deletions(-) diff --git a/src/ifcgeom/profile_helper.cpp b/src/ifcgeom/profile_helper.cpp index eaa04e96e0..79de6ed966 100644 --- a/src/ifcgeom/profile_helper.cpp +++ b/src/ifcgeom/profile_helper.cpp @@ -3,48 +3,92 @@ using namespace ifcopenshell::geometry; taxonomy::loop::ptr ifcopenshell::geometry::fillet_loop(taxonomy::loop::ptr loop, double radius) { - std::vector pps(loop->children.size()); - for (int b = 0; b < loop->children.size(); ++b) { - int c = (b - 1) % loop->children.size(); - pps[b] = { - boost::get(loop->children[c]->start)->ccomponents(), + const size_t n = loop->children.size(); + if (n < 2) { + return loop; + } + + // FilletRadius is only meaningful for a polygonal (straight segmented) + // directrix. If any segment carries a curved basis or is parameterised rather + // than defined by its end points, the directrix is not polyhedral: leave it + // untouched instead of misinterpreting arc segments as corners. + if (!loop->is_polyhedron()) { + Logger::Root().Warning("GEO", 328, "Directrix is not polyhedral, ignoring FilletRadius"); + return loop; + } + for (const auto& e : loop->children) { + // which() == 1 selects the point3::ptr alternative of the start/end variant. + if (e->start.which() != 1 || e->end.which() != 1) { + Logger::Root().Warning("GEO", 328, "Directrix is not polyhedral, ignoring FilletRadius"); + return loop; + } + } + + // polygon_from_points() shares a single point instance between the end of one + // edge and the start of the next. Filleting insets those two vertices in + // opposite directions, so break the sharing first by giving every edge its own + // end point (mirrors what profile_helper() does for the 2d case). + for (auto& e : loop->children) { + e->end = taxonomy::make(*boost::get(e->end)->components_); + } + + // A directrix built from an open IfcPolyline is not a closed loop: its two end + // vertices are genuine endpoints, not corners, and must not be filleted. + // polygon_from_points() does not set the closed flag, so detect closure + // geometrically from the first and last vertex. + const auto& first_pt = boost::get(loop->children.front()->start)->ccomponents(); + const auto& last_pt = boost::get(loop->children.back()->end)->ccomponents(); + const bool closed = (first_pt - last_pt).norm() < 1.e-9; + + std::vector pps(n); + for (size_t b = 0; b < n; ++b) { + size_t c = (b + n - 1) % n; + pps[b] = { + boost::get(loop->children[c]->start)->ccomponents(), radius, loop->children[c], loop->children[b] }; } size_t i = pps.size(); while (i--) { + // The first vertex of an open loop is an endpoint shared with no preceding + // edge; skip it (pps[0].previous would wrap to the last edge). + if (!closed && i == 0) { + continue; + } const auto& p = pps[i]; if (p.radius && *p.radius > 0.) { auto p0 = boost::get(p.previous->start)->ccomponents(); - auto p1a = boost::get(p.previous->end)->ccomponents(); + auto corner = boost::get(p.previous->end)->ccomponents(); auto p2 = boost::get(p.next->end)->ccomponents(); - auto p1b = boost::get(p.next->start)->ccomponents(); - auto ba_ = p0 - p1a; - auto bc_ = p2 - p1b; + auto ba = (p0 - corner).normalized(); + auto bc = (p2 - corner).normalized(); - auto ba = ba_.normalized(); - auto bc = bc_.normalized(); - - const double angle = std::acos(ba.dot(bc)); + const double dot = std::max(-1., std::min(1., ba.dot(bc))); + const double angle = std::acos(dot); const double inset = *p.radius / std::tan(angle / 2.); boost::get(p.previous->end)->components() += ba * inset; boost::get(p.next->start)->components() += bc * inset; + auto A = boost::get(p.previous->end)->ccomponents(); + auto B = boost::get(p.next->start)->ccomponents(); + auto e = taxonomy::make(); e->start = p.previous->end; e->end = p.next->start; - // @todo untested from here. - - auto ab = ba.cross(bc); - - auto O = boost::get(p.previous->end)->ccomponents().head<3>() + ab * *p.radius; + // Arc centre lies on the interior bisector at radius / sin(angle/2) from + // the original corner. Orient the arc axis from the two tangent points so + // the swept arc is the minor (fillet) arc from start to end, not its major + // complement. + auto bisector = (ba + bc).normalized(); + auto O = corner.head<3>() + bisector * (*p.radius / std::sin(angle / 2.)); + auto normal = (A - O).cross(B - O); auto c = taxonomy::make(); - c->matrix = taxonomy::make(O, ab); + c->matrix = taxonomy::make(O, normal); c->radius = *p.radius; e->basis = c;