From 1176043c771ed6842964712bb7ee389b82bb5e68 Mon Sep 17 00:00:00 2001 From: Brandon Ross Date: Sat, 18 Jul 2026 15:40:40 -0700 Subject: [PATCH 1/2] GeomLib_Tool: Add closed-form fast path to Parameter() for analytic curves Parameter() always ran through GeomAdaptor_Curve and Extrema_ExtPC, an iterative numerical search, even for curve types where ElCLib already has an O(1) closed-form parameter computation: Line, Circle, Ellipse, Hyperbola, Parabola. That gets expensive when a shape has a lot of circular edges, since BRepCheck_Face::ClassifyWires calls into this once per wire edge to build a pcurve. Added a fast path that unwraps Geom_TrimmedCurve down to its basis curve, computes the closed-form parameter with ElCLib::Parameter, and only accepts it when the result actually falls within the curve's own range, handling periodicity via InPeriod. A trimmed arc whose closest point on the full curve isn't on the trimmed portion still falls through to the existing generic search unchanged. Verified with a small standalone harness covering full and trimmed circles, lines, and the out-of-range trimmed arc case. Combined with the BRepCheck_Face fix in the next commit, building and validating a many-hole face went from 0.57s/2.26s/9.21s at 500/1000/2000 holes down to 0.077s/0.29s/1.11s, about 8x faster at 2000 holes. --- .../TKGeomBase/GeomLib/GeomLib_Tool.cxx | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/ModelingData/TKGeomBase/GeomLib/GeomLib_Tool.cxx b/src/ModelingData/TKGeomBase/GeomLib/GeomLib_Tool.cxx index e3f7c7a56e..554e40462a 100644 --- a/src/ModelingData/TKGeomBase/GeomLib/GeomLib_Tool.cxx +++ b/src/ModelingData/TKGeomBase/GeomLib/GeomLib_Tool.cxx @@ -20,6 +20,12 @@ #include #include #include +#include +#include +#include +#include +#include +#include #include #include #include @@ -46,6 +52,70 @@ bool GeomLib_Tool::Parameter(const occ::handle& Curve, // U = 0.; double aTol = MaxDist * MaxDist; + + // Analytic curve types already have an O(1) closed-form parameter + // computation in ElCLib, so use that instead of the iterative + // Extrema_ExtPC search below when the curve (or its basis curve, if + // trimmed) is a Line, Circle, Ellipse, Hyperbola or Parabola. The + // closed-form result is only kept if it lands within the curve's own + // range, accounting for periodicity. A trimmed arc whose true closest + // point falls outside the trimmed portion still goes through the + // generic search below. + { + occ::handle aBasisCurve = Curve; + while (aBasisCurve->IsKind(STANDARD_TYPE(Geom_TrimmedCurve))) + { + aBasisCurve = occ::down_cast(aBasisCurve)->BasisCurve(); + } + + bool hasFastU = true; + double aFastU = 0.0; + if (aBasisCurve->IsKind(STANDARD_TYPE(Geom_Line))) + { + aFastU = ElCLib::Parameter(occ::down_cast(aBasisCurve)->Lin(), Point); + } + else if (aBasisCurve->IsKind(STANDARD_TYPE(Geom_Circle))) + { + aFastU = ElCLib::Parameter(occ::down_cast(aBasisCurve)->Circ(), Point); + } + else if (aBasisCurve->IsKind(STANDARD_TYPE(Geom_Ellipse))) + { + aFastU = ElCLib::Parameter(occ::down_cast(aBasisCurve)->Elips(), Point); + } + else if (aBasisCurve->IsKind(STANDARD_TYPE(Geom_Hyperbola))) + { + aFastU = ElCLib::Parameter(occ::down_cast(aBasisCurve)->Hypr(), Point); + } + else if (aBasisCurve->IsKind(STANDARD_TYPE(Geom_Parabola))) + { + aFastU = ElCLib::Parameter(occ::down_cast(aBasisCurve)->Parab(), Point); + } + else + { + hasFastU = false; + } + + if (hasFastU) + { + if (Curve->IsPeriodic()) + { + aFastU = ElCLib::InPeriod(aFastU, + Curve->FirstParameter(), + Curve->FirstParameter() + Curve->Period()); + } + if (aFastU >= Curve->FirstParameter() - PARTOLERANCE + && aFastU <= Curve->LastParameter() + PARTOLERANCE) + { + const gp_Pnt aFastP = Curve->Value(aFastU); + if (aFastP.SquareDistance(Point) <= aTol) + { + U = aFastU; + return true; + } + return false; + } + } + } // GeomAdaptor_Curve aGAC(Curve); Extrema_ExtPC extrema(Point, aGAC); From 5360886bd2ad7be2d4c13e377af5277688b238ee Mon Sep 17 00:00:00 2001 From: Brandon Ross Date: Sat, 18 Jul 2026 15:40:54 -0700 Subject: [PATCH 2/2] BRepCheck_Face: Add 2D-bbox pre-filter to ClassifyWires wire loop ClassifyWires() checks every pair of a face's wires against each other, calling IsInside() to classify a representative point of one wire against the other's region. For a face with a lot of wires, like a plate with thousands of holes, this dominates BRepCheck_Analyzer's runtime. IsInside() only needs to classify a single point, so if two wires' 2D bounding boxes don't overlap at all, the answer is already known without running the classifier: the point can't be inside the other wire's finite region. Following the same pattern IntersectWires() already uses above, this precomputes each wire's parametric bounding box once and skips the expensive test whenever the boxes are disjoint. The loop is still a check over every pair of wires, but each pair now costs an O(1) box test instead of a real point classification, which is what actually costs time when the wires are spatially separated. The box for a wire is built per edge from BRep_Tool::CurveOnSurface's trim range, clamped to the underlying curve's own parametric domain (needed to avoid an out-of-domain exception building the box). IsInside() picks its representative point from that same trim range, but without the clamp. For STEP-imported BSpline pcurves whose trim range slightly overshoots the curve's own domain, that mismatch let the box come out tighter than the point IsInside() would actually evaluate, so the disjoint-box fast path could wrongly report "false" for a point that was, in fact, inside. Caught this via a real regression test (bugs/mesh/bug27453) that started reporting a bad shape after this change. Fixed by tracking whether any edge's box needed clamping and, if so, not trusting that wire's box for the pre-filter at all, falling through to the exact classification for any pair involving it. This only gives up the fast path for wires that hit this edge case, which is rare. Ported from a fix originally written against OCCT 7.9.3, updated for the current src/ModelingAlgorithms/TKTopAlgo layout and the current bool/double/occ::handle<> style. The original version also had a bug where it returned WireBienOriente directly on the disjoint-box path, which is only correct when WireBienOriente is false; this port keeps the fixed version, which always returns false on that path regardless. Verified on current master with a harness that builds a plane face with N non-overlapping circular holes and checks it with BRepCheck_Analyzer. With this commit and the GeomLib_Tool one both reverted, the harness measured 0.57s/2.26s/9.21s at 500/1000/2000 holes. With both applied, 0.077s/0.28s/1.12s, still about 8x faster at 2000 holes (the clamp-tracking fix doesn't trigger for these synthetic, non-imported wires). Also verified directly against bugs/mesh/bug27453: a standalone harness reading the test's STEP file and running BRepCheck_Analyzer on face #13 reproduced the failure with this commit alone applied (BRepCheck_Analyzer::IsValid() false, matching CI), confirmed GeomLib_Tool was not responsible (still valid with only that commit applied), and confirmed this fix resolves it (valid again with both commits and the fix applied). --- .../TKTopAlgo/BRepCheck/BRepCheck_Face.cxx | 79 ++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/src/ModelingAlgorithms/TKTopAlgo/BRepCheck/BRepCheck_Face.cxx b/src/ModelingAlgorithms/TKTopAlgo/BRepCheck/BRepCheck_Face.cxx index 24d9d31224..62115120bd 100644 --- a/src/ModelingAlgorithms/TKTopAlgo/BRepCheck/BRepCheck_Face.cxx +++ b/src/ModelingAlgorithms/TKTopAlgo/BRepCheck/BRepCheck_Face.cxx @@ -347,6 +347,58 @@ BRepCheck_Status BRepCheck_Face::ClassifyWires(const bool Update) BRep_Builder B; TopExp_Explorer exp1, exp2; NCollection_List theListOfShape; + + // Precompute a 2D parametric bounding box for every wire, the same + // pattern IntersectWires() already uses above. IsInside() classifies a + // single representative point of wir2 against wir1's region, so if the + // two wires' boxes don't overlap, that point can't be inside wir1's + // region and the classification test can be skipped. The loop below is + // still a pair over every wire, but each pair now costs an O(1) box + // check instead of a real classification, which is what actually + // matters for a plate with many non-overlapping holes. + Geom2dAdaptor_Curve aBoxCurveAdapt; + double aBoxFirst, aBoxLast; + DataMapOfShapeBox2d aWireBoxMap; + for (exp1.Init(myShape.Oriented(TopAbs_FORWARD), TopAbs_WIRE); exp1.More(); exp1.Next()) + { + Bnd_Box2d aBoxW; + bool isBoxReliable = true; + for (exp2.Init(exp1.Current(), TopAbs_EDGE); exp2.More(); exp2.Next()) + { + const TopoDS_Edge& anEdge = TopoDS::Edge(exp2.Current()); + occ::handle aPCurve = + BRep_Tool::CurveOnSurface(anEdge, TopoDS::Face(myShape), aBoxFirst, aBoxLast); + if (aPCurve.IsNull()) + { + continue; + } + aBoxCurveAdapt.Load(aPCurve); + double aF = aBoxFirst, aL = aBoxLast; + if (aBoxCurveAdapt.FirstParameter() > aF) + { + aF = aBoxCurveAdapt.FirstParameter(); + isBoxReliable = false; + } + if (aBoxCurveAdapt.LastParameter() < aL) + { + aL = aBoxCurveAdapt.LastParameter(); + isBoxReliable = false; + } + Bnd_Box2d aBoxE; + BndLib_Add2dCurve::Add(aBoxCurveAdapt, aF, aL, 0., aBoxE); + aBoxW.Add(aBoxE); + } + // IsInside() below picks its representative point using the edge's own + // (unclamped) parameter range, not the curve's own definition domain, so + // if any edge's trim range had to be clamped to build this box, the box + // may not actually contain the point IsInside() would evaluate. Skip the + // pre-filter for such a wire rather than risk a wrong answer. + if (isBoxReliable) + { + aWireBoxMap.Bind(exp1.Current(), aBoxW); + } + } + for (exp1.Init(myShape.Oriented(TopAbs_FORWARD), TopAbs_WIRE); exp1.More(); exp1.Next()) { @@ -368,13 +420,36 @@ BRepCheck_Status BRepCheck_Face::ClassifyWires(const bool Update) myMapImb.Bind(wir1.Reversed(), theListOfShape); } + Bnd_Box2d aBox1; + bool hasBox1 = + aWireBoxMap.IsBound(exp1.Current()) && !(aBox1 = aWireBoxMap(exp1.Current())).IsVoid(); + for (exp2.Init(myShape.Oriented(TopAbs_FORWARD), TopAbs_WIRE); exp2.More(); exp2.Next()) { const TopoDS_Wire& wir2 = TopoDS::Wire(exp2.Current()); if (!wir2.IsSame(wir1)) { - - if (IsInside(wir2, WireBienOriente, FClass2d, newFace)) + bool isWire2Inside; + Bnd_Box2d aBox2; + bool hasBox2 = hasBox1 && aWireBoxMap.IsBound(exp2.Current()) + && !(aBox2 = aWireBoxMap(exp2.Current())).IsVoid(); + if (hasBox2 && aBox1.IsOut(aBox2)) + { + // wir2's point falls outside wir1's box, so it's outside wir1's + // finite bounded region. If WireBienOriente is false, that + // finite region is exactly what IsInside() tests for, so the + // point is out. If WireBienOriente is true, IsInside() tests + // for the infinite complement of that region instead, and a + // point outside the finite region is inside the complement, so + // IsInside() returns false there too. Either way the answer is + // false. + isWire2Inside = false; + } + else + { + isWire2Inside = IsInside(wir2, WireBienOriente, FClass2d, newFace); + } + if (isWire2Inside) { myMapImb(wir1).Append(wir2); }