From 3c2774ea5d580cec828b024640f82fcfbe54f4e8 Mon Sep 17 00:00:00 2001 From: Dmitrii Kulikov <164657232+AtheneNoctuaPt@users.noreply.github.com> Date: Mon, 21 Jul 2025 11:10:27 +0100 Subject: [PATCH] Modelling - GeomFill_CorrectedFrenet hangs in some cases (#630) - Added parameter progression check to prevent infinite loops when step size becomes too small - Improved code formatting and variable naming for better maintainability - Added comprehensive test suite covering the infinite loop scenario and edge cases --- .../TKGeomAlgo/GTests/FILES.cmake | 5 +- .../GTests/GeomFill_CorrectedFrenet_Test.cxx | 170 ++++++++++++++++++ .../GeomFill/GeomFill_CorrectedFrenet.cxx | 53 +++--- 3 files changed, 200 insertions(+), 28 deletions(-) create mode 100644 src/ModelingAlgorithms/TKGeomAlgo/GTests/GeomFill_CorrectedFrenet_Test.cxx diff --git a/src/ModelingAlgorithms/TKGeomAlgo/GTests/FILES.cmake b/src/ModelingAlgorithms/TKGeomAlgo/GTests/FILES.cmake index 35c1a08a11..dbb0c31eb7 100644 --- a/src/ModelingAlgorithms/TKGeomAlgo/GTests/FILES.cmake +++ b/src/ModelingAlgorithms/TKGeomAlgo/GTests/FILES.cmake @@ -1,5 +1,6 @@ # Test source files for TKGeomAlgo set(OCCT_TKGeomAlgo_GTests_FILES_LOCATION "${CMAKE_CURRENT_LIST_DIR}") - + set(OCCT_TKGeomAlgo_GTests_FILES -) + GeomFill_CorrectedFrenet_Test.cxx +) \ No newline at end of file diff --git a/src/ModelingAlgorithms/TKGeomAlgo/GTests/GeomFill_CorrectedFrenet_Test.cxx b/src/ModelingAlgorithms/TKGeomAlgo/GTests/GeomFill_CorrectedFrenet_Test.cxx new file mode 100644 index 0000000000..1805095a95 --- /dev/null +++ b/src/ModelingAlgorithms/TKGeomAlgo/GTests/GeomFill_CorrectedFrenet_Test.cxx @@ -0,0 +1,170 @@ +// Copyright (c) 2025 OPEN CASCADE SAS +// +// This file is part of Open CASCADE Technology software library. +// +// This library is free software; you can redistribute it and/or modify it under +// the terms of the GNU Lesser General Public License version 2.1 as published +// by the Free Software Foundation, with special exception defined in the file +// OCCT_LGPL_EXCEPTION.txt. Consult the file LICENSE_LGPL_21.txt included in OCCT +// distribution for complete text of the license and disclaimer of any warranty. +// +// Alternatively, this file may be used under the terms of Open CASCADE +// commercial license or contractual agreement. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +//================================================================================================== + +TEST(GeomFill_CorrectedFrenet, EndlessLoopPrevention) +{ + TColgp_Array1OfPnt aPoles(1, 4); + aPoles(1) = gp_Pnt(0.0, 0.0, 0.0); + aPoles(2) = gp_Pnt(1.0, 0.0, 0.0); + aPoles(3) = gp_Pnt(1.0, 1.0, 0.0); + aPoles(4) = gp_Pnt(0.0, 1.0, 0.0); + + TColStd_Array1OfReal aKnots(1, 2); + aKnots(1) = 0.0; + aKnots(2) = 1.0; + + TColStd_Array1OfInteger aMults(1, 2); + aMults(1) = 4; + aMults(2) = 4; + + Handle(Geom_BSplineCurve) aCurve = new Geom_BSplineCurve(aPoles, aKnots, aMults, 3); + Handle(GeomAdaptor_Curve) anAdaptor = new GeomAdaptor_Curve(aCurve); + + GeomFill_CorrectedFrenet aCorrectedFrenet(Standard_False); + aCorrectedFrenet.SetCurve(anAdaptor); + + Standard_Real aParam1 = 0.0; + Standard_Real aParam2 = 1.0; + + gp_Vec aTangent1, aNormal1, aBinormal1; + gp_Vec aTangent2, aNormal2, aBinormal2; + + EXPECT_NO_THROW({ + aCorrectedFrenet.D0(aParam1, aTangent1, aNormal1, aBinormal1); + aCorrectedFrenet.D0(aParam2, aTangent2, aNormal2, aBinormal2); + }); + + EXPECT_GT(aTangent1.Magnitude(), 1e-10); + EXPECT_GT(aNormal1.Magnitude(), 1e-10); + EXPECT_GT(aBinormal1.Magnitude(), 1e-10); + + EXPECT_GT(aTangent2.Magnitude(), 1e-10); + EXPECT_GT(aNormal2.Magnitude(), 1e-10); + EXPECT_GT(aBinormal2.Magnitude(), 1e-10); +} + +//================================================================================================== + +TEST(GeomFill_CorrectedFrenet, SmallStepHandling) +{ + TColgp_Array1OfPnt aPoles(1, 2); + aPoles(1) = gp_Pnt(0.0, 0.0, 0.0); + aPoles(2) = gp_Pnt(1e-10, 0.0, 0.0); + + TColStd_Array1OfReal aKnots(1, 2); + aKnots(1) = 0.0; + aKnots(2) = 1.0; + + TColStd_Array1OfInteger aMults(1, 2); + aMults(1) = 2; + aMults(2) = 2; + + Handle(Geom_BSplineCurve) aCurve = new Geom_BSplineCurve(aPoles, aKnots, aMults, 1); + Handle(GeomAdaptor_Curve) anAdaptor = new GeomAdaptor_Curve(aCurve); + + GeomFill_CorrectedFrenet aCorrectedFrenet(Standard_False); + aCorrectedFrenet.SetCurve(anAdaptor); + + gp_Vec aTangent, aNormal, aBinormal; + + EXPECT_NO_THROW({ aCorrectedFrenet.D0(0.5, aTangent, aNormal, aBinormal); }); +} + +//================================================================================================== + +TEST(GeomFill_CorrectedFrenet, ParameterProgressionGuarantee) +{ + TColgp_Array1OfPnt aPoles(1, 3); + aPoles(1) = gp_Pnt(0.0, 0.0, 0.0); + aPoles(2) = gp_Pnt(0.5, 0.5, 0.0); + aPoles(3) = gp_Pnt(1.0, 0.0, 0.0); + + TColStd_Array1OfReal aKnots(1, 2); + aKnots(1) = 0.0; + aKnots(2) = 1.0; + + TColStd_Array1OfInteger aMults(1, 2); + aMults(1) = 3; + aMults(2) = 3; + + Handle(Geom_BSplineCurve) aCurve = new Geom_BSplineCurve(aPoles, aKnots, aMults, 2); + Handle(GeomAdaptor_Curve) anAdaptor = new GeomAdaptor_Curve(aCurve); + + GeomFill_CorrectedFrenet aCorrectedFrenet(Standard_False); + aCorrectedFrenet.SetCurve(anAdaptor); + + for (Standard_Real aParam = 0.1; aParam <= 0.9; aParam += 0.1) + { + gp_Vec aTangent, aNormal, aBinormal; + + EXPECT_NO_THROW({ aCorrectedFrenet.D0(aParam, aTangent, aNormal, aBinormal); }); + + EXPECT_GT(aTangent.Magnitude(), 1e-12); + EXPECT_GT(aNormal.Magnitude(), 1e-12); + EXPECT_GT(aBinormal.Magnitude(), 1e-12); + } +} + +//================================================================================================== + +TEST(GeomFill_CorrectedFrenet, ActualReproducerCase) +{ + TColgp_Array1OfPnt aPoints(1, 4); + aPoints(1) = gp_Pnt(-1, -1, 0); + aPoints(2) = gp_Pnt(0, -2, 0); + aPoints(3) = gp_Pnt(0, -2, -1); + aPoints(4) = gp_Pnt(0, -1, -1); + + ShapeExtend_WireData anExtend; + for (Standard_Integer i = 2; i <= aPoints.Length(); i++) + { + Handle(Geom_Curve) aCurve = GC_MakeSegment(aPoints(i - 1), aPoints(i)).Value(); + TopoDS_Edge anEdge = BRepBuilderAPI_MakeEdge(aCurve).Edge(); + anExtend.Add(anEdge); + } + + BRepAdaptor_CompCurve anAdaptor(anExtend.WireAPIMake()); + + GeomFill_CorrectedFrenet aCorrectedFrenet(Standard_False); + + // This SetCurve call should not hang (was causing infinite loops) + EXPECT_NO_THROW({ aCorrectedFrenet.SetCurve(anAdaptor.ShallowCopy()); }); + + // Verify we can evaluate the trihedron at various parameters + gp_Vec aTangent, aNormal, aBinormal; + + EXPECT_NO_THROW({ aCorrectedFrenet.D0(0.0, aTangent, aNormal, aBinormal); }); + + EXPECT_NO_THROW({ aCorrectedFrenet.D0(0.5, aTangent, aNormal, aBinormal); }); + + EXPECT_NO_THROW({ aCorrectedFrenet.D0(1.0, aTangent, aNormal, aBinormal); }); +} diff --git a/src/ModelingAlgorithms/TKGeomAlgo/GeomFill/GeomFill_CorrectedFrenet.cxx b/src/ModelingAlgorithms/TKGeomAlgo/GeomFill/GeomFill_CorrectedFrenet.cxx index 6487645dfe..c597d90918 100644 --- a/src/ModelingAlgorithms/TKGeomAlgo/GeomFill/GeomFill_CorrectedFrenet.cxx +++ b/src/ModelingAlgorithms/TKGeomAlgo/GeomFill/GeomFill_CorrectedFrenet.cxx @@ -506,44 +506,53 @@ Standard_Boolean GeomFill_CorrectedFrenet::InitInterval(const Standard_Real TColgp_SequenceOfVec& SeqTangent, TColgp_SequenceOfVec& SeqNormal) const { - Bnd_Box Boite; gp_Vec Tangent, Normal, BN, cross; TColStd_SequenceOfReal parameters; TColStd_SequenceOfReal EvolAT; - Standard_Real Param = First, LengthMin, L, norm; + Standard_Real Param = First; Standard_Boolean isZero = Standard_True, isConst = Standard_True; - Standard_Integer i; gp_Pnt PonC; gp_Vec D1; frenet->SetInterval(First, Last); // To have right evaluation at bounds GeomFill_SnglrFunc CS(myCurve); - BndLib_Add3dCurve::Add(CS, First, Last, 1.e-2, Boite); - LengthMin = Boite.GetGap() * 1.e-4; + Bnd_Box aCurveBoundBox; + BndLib_Add3dCurve::Add(CS, First, Last, 1.e-2, aCurveBoundBox); + const Standard_Real LengthMin = aCurveBoundBox.GetGap() * 1.e-4; aT = gp_Vec(0, 0, 0); aN = gp_Vec(0, 0, 0); - Standard_Real angleAT = 0., currParam, currStep = Step; + Standard_Real angleAT = 0.; + Standard_Real currStep = Step; Handle(Geom_Plane) aPlane; Standard_Boolean isPlanar = Standard_False; if (!myForEvaluation) + { isPlanar = FindPlane(myCurve, aPlane); + } - i = 1; - currParam = Param; - Standard_Real DLast = Last - Precision::PConfusion(); + Standard_Integer i = 1; + Standard_Real currParam = Param; + const Standard_Real DLast = Last - Precision::PConfusion(); while (Param < Last) { if (currParam > DLast) { + if (Abs(DLast - Param) < Precision::SquareConfusion()) + { + Param = currParam; + } + currStep = DLast - Param; currParam = Last; } if (isPlanar) + { currParam = Last; + } frenet->D0(currParam, Tangent, Normal, BN); if (prevTangent.Angle(Tangent) < M_PI / 3 || i == 1) @@ -577,18 +586,22 @@ Standard_Boolean GeomFill_CorrectedFrenet::InitInterval(const Standard_Real // Evaluate the Next step CS.D1(Param, PonC, D1); - L = Max(PonC.XYZ().Modulus() / 2, LengthMin); - norm = D1.Magnitude(); + Standard_Real L = Max(PonC.XYZ().Modulus() / 2, LengthMin); + Standard_Real norm = D1.Magnitude(); if (norm < Precision::Confusion()) { norm = Precision::Confusion(); } currStep = L / norm; if (currStep > Step) + { currStep = Step; // default value + } } else + { currStep /= 2; // Step too long ! + } currParam = Param + currStep; } @@ -613,24 +626,12 @@ Standard_Boolean GeomFill_CorrectedFrenet::InitInterval(const Standard_Real Handle(TColStd_HArray1OfReal) pararr = new TColStd_HArray1OfReal(1, Length); Handle(TColStd_HArray1OfReal) angleATarr = new TColStd_HArray1OfReal(1, Length); - for (i = 1; i <= Length; i++) + for (Standard_Integer aParamIndex = 1; aParamIndex <= Length; ++aParamIndex) { - pararr->ChangeValue(i) = parameters(i); - angleATarr->ChangeValue(i) = EvolAT(i); + pararr->ChangeValue(aParamIndex) = parameters(aParamIndex); + angleATarr->ChangeValue(aParamIndex) = EvolAT(aParamIndex); } -#ifdef OCCT_DEBUG - if (Affich) - { - std::cout << "NormalEvolution" << std::endl; - for (i = 1; i <= Length; i++) - { - std::cout << "(" << pararr->Value(i) << ", " << angleATarr->Value(i) << ")" << std::endl; - } - std::cout << std::endl; - } -#endif - Law_Interpolate lawAT(angleATarr, pararr, Standard_False, Precision::PConfusion()); lawAT.Perform(); Handle(Law_BSpline) BS = lawAT.Curve(); -- 2.39.5