From 493a79828318340581d2e445a82bf090aeb26050 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Mon, 20 Jul 2026 14:32:16 +0300 Subject: [PATCH] ifcgeom: detect and repair collinear self-intersections in profiles IfcGeom::util::wire_intersections only detected self-intersecting profile boundaries where two edges cross transversally, since GeomAPI_ExtremaCurveCurve never reports a single well-defined extremum for edges that are parallel to each other. A profile made of several disjoint loops chained together through a shared, overlapping return path (a self-intersection built from collinear overlapping edges rather than a crossing) went entirely undetected, so the raw invalid wire reached OCCT's face builder unmodified. OpenCascadeKernel::convert for a face additionally collapsed any detected self-intersection cycles down to the single largest one via select_largest, which is correct for spurious export noise but silently discards real loops when a lone boundary (no holes) resolves into several disjoint, genuinely separate parts of the same profile. Fixes issue #6287: an IfcArbitraryClosedProfileDef boundary shaped like a comb of four rectangles joined by overlapping zero-width vertical segments produced one merged, overlong extrusion instead of four separate ones, because the overlap was invisible to the crossing-only detector and, once detected in other cases, all but the largest cycle was thrown away. wire_utils.cpp adds collinear_overlap_point, which checks whether an edge endpoint lies strictly inside another parallel edge's span along their common line, and feeds that point into the existing crossing based cycle isolation logic unchanged. face.cpp now keeps every sufficiently large cycle (at least a tenth of the largest one's area) as its own outer wire when a lone boundary self-intersects, mirroring how multiple genuine outer boundaries are already turned into a compound of faces, instead of reducing them all to one. Verified against the reporter's attached file: the opening's own comb shaped profile previously produced a single merged solid spanning the full profile length (72 to 36 triangles, no gaps between window positions); it now produces four disjoint prisms with the correct gaps between them. A sweep of test/input (258 files) is byte identical for 252 files; the remaining differences recover previously missing subtraction geometry in known regression fixtures (1015, 336) or are floating point noise below 1e-12 relative (377, 487), consistent with a pre-existing nondeterminism already present in the unmodified build on at least one other fixture (423). Generated with the assistance of an AI coding tool. --- src/ifcgeom/kernels/opencascade/face.cpp | 37 ++++++++++ .../kernels/opencascade/wire_utils.cpp | 71 ++++++++++++++++--- 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/src/ifcgeom/kernels/opencascade/face.cpp b/src/ifcgeom/kernels/opencascade/face.cpp index f300984728..2561a8ec3e 100644 --- a/src/ifcgeom/kernels/opencascade/face.cpp +++ b/src/ifcgeom/kernels/opencascade/face.cpp @@ -54,6 +54,8 @@ #include #include +#include + #if OCC_VERSION_HEX < 0x70600 #include #endif @@ -343,6 +345,41 @@ bool OpenCascadeKernel::convert(const taxonomy::face::ptr face, TopoDS_Shape& re NCollection_List results; if (settings.use_wire_intersection_check && util::wire_intersections(wire, results, settings)) { Logger::Root().Warning("GEO", 161, "Self-intersections with " + boost::lexical_cast(results.Extent()) + " cycles detected"); + + if (num_bounds == 1 && results.Extent() > 1) { + // No holes to reconcile against, so every sufficiently large + // cycle becomes its own outer wire instead of just the largest. + fd.all_outer() = true; + + double max_area = 0.; + std::vector cycles; + std::vector areas; + for (NCollection_List::Iterator it(results); it.More(); it.Next()) { + const TopoDS_Wire& w = TopoDS::Wire(it.Value()); + BRepBuilderAPI_MakeFace mf(w, false); + if (!mf.IsDone()) { + continue; + } + const double area = face_area(mf.Face()); + cycles.push_back(w); + areas.push_back(area); + if (area > max_area) { + max_area = area; + } + } + + for (size_t idx = 0; idx < cycles.size(); ++idx) { + if (areas[idx] < max_area / 10.) { + Logger::Root().Warning("GEO", 328, "Ignoring self-intersection loop with area " + boost::lexical_cast(areas[idx]), face->instance); + continue; + } + wire_senses.Bind(cycles[idx].Oriented(TopAbs_FORWARD), same_sense ? TopAbs_FORWARD : TopAbs_REVERSED); + fd.wires().emplace_back(cycles[idx]); + } + + continue; + } + util::select_largest(results, wire); } diff --git a/src/ifcgeom/kernels/opencascade/wire_utils.cpp b/src/ifcgeom/kernels/opencascade/wire_utils.cpp index 638195136e..b6dfef21bd 100644 --- a/src/ifcgeom/kernels/opencascade/wire_utils.cpp +++ b/src/ifcgeom/kernels/opencascade/wire_utils.cpp @@ -30,6 +30,10 @@ #include #include #include +#include +#include +#include +#include #include #include @@ -376,6 +380,50 @@ namespace { return 0.; } } + + // Detects overlapping (not just crossing) collinear edges, which + // GeomAPI_ExtremaCurveCurve does not report as a single extremum. + bool collinear_overlap_point(const Handle(Geom_Curve)& c1, double u11, double u12, + const Handle(Geom_Curve)& c2, double u21, double u22, + double eps, double& U1, double& U2) { + if (c1->DynamicType() != STANDARD_TYPE(Geom_Line) || c2->DynamicType() != STANDARD_TYPE(Geom_Line)) { + return false; + } + + const gp_Lin& l1 = Handle(Geom_Line)::DownCast(c1)->Lin(); + const gp_Lin& l2 = Handle(Geom_Line)::DownCast(c2)->Lin(); + + if (gp_Vec(l1.Direction()).Crossed(gp_Vec(l2.Direction())).Magnitude() > eps) { + return false; + } + if (l1.Distance(l2.Location()) > eps) { + return false; + } + + if (u11 > u12) { std::swap(u11, u12); } + if (u21 > u22) { std::swap(u21, u22); } + + for (double u : { u21, u22 }) { + const gp_Pnt p = c2->Value(u); + const double t = ElCLib::Parameter(l1, p); + if (t > u11 + eps && t < u12 - eps) { + U1 = t; + U2 = u; + return true; + } + } + for (double u : { u11, u12 }) { + const gp_Pnt p = c1->Value(u); + const double t = ElCLib::Parameter(l2, p); + if (t > u21 + eps && t < u22 - eps) { + U1 = u; + U2 = t; + return true; + } + } + + return false; + } } bool IfcGeom::util::wire_intersections(const TopoDS_Wire& wire, NCollection_List& wires, const wire_tolerance_settings& settings) { @@ -453,16 +501,22 @@ bool IfcGeom::util::wire_intersections(const TopoDS_Wire& wire, NCollection_List if (i == n - 1 && j == 0) continue; double u11, u12, u21, u22, U1, U2; - GeomAPI_ExtremaCurveCurve ecc( - BRep_Tool::Curve(wd->Edge(i + 1), u11, u12), - BRep_Tool::Curve(wd->Edge(j + 1), u21, u22) - ); + Handle(Geom_Curve) crv1 = BRep_Tool::Curve(wd->Edge(i + 1), u11, u12); + Handle(Geom_Curve) crv2 = BRep_Tool::Curve(wd->Edge(j + 1), u21, u22); + GeomAPI_ExtremaCurveCurve ecc(crv1, crv2); // @todo: extend this to work in case of multiple extrema and curved segments. - const bool unbounded_intersects = (!ecc.Extrema().IsParallel() && ecc.NbExtrema() == 1 && ecc.Distance(1) < eps); + bool unbounded_intersects = (!ecc.Extrema().IsParallel() && ecc.NbExtrema() == 1 && ecc.Distance(1) < eps); if (unbounded_intersects) { ecc.Parameters(1, U1, U2); + } else { + // Edges parallel/collinear with each other are never reported by + // GeomAPI_ExtremaCurveCurve as a single crossing extremum, so an + // overlapping shared line segment falls through undetected above. + unbounded_intersects = collinear_overlap_point(crv1, u11, u12, crv2, u21, u22, eps, U1, U2); + } + if (unbounded_intersects) { if (u11 > u12) { std::swap(u11, u12); } @@ -486,6 +540,9 @@ bool IfcGeom::util::wire_intersections(const TopoDS_Wire& wire, NCollection_List intersected = true; + const gp_Pnt split_point_1 = crv1->Value(U1); + const gp_Pnt split_point_2 = crv2->Value(U2); + // Explore a forward and backward cycle from the intersection point for (int fb = 0; fb <= 1; ++fb) { const bool forward = fb == 0; @@ -505,9 +562,7 @@ bool IfcGeom::util::wire_intersections(const TopoDS_Wire& wire, NCollection_List // gp_Pnt p2 = points3d.Value(1); gp_Pnt p1 = BRep_Tool::Pnt(*v); - gp_Pnt pp1, pp2; - ecc.Points(1, pp1, pp2); - const gp_Pnt& p2 = k == i ? pp1 : pp2; + const gp_Pnt& p2 = k == i ? split_point_1 : split_point_2; // Substitute with a new edge from/to the intersection point if (p1.Distance(p2) > eps_real * 2) {