From 27e61c089c5908dba54afae4ca7fe2bf793a2881 Mon Sep 17 00:00:00 2001 From: Pasukhin Dmitry Date: Sun, 9 Nov 2025 14:13:24 +0000 Subject: [PATCH] Foundation Classes - ElSLib, ElCLib Angle normalization refactor (#813) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - Introduced a centralized `normalizeAngle()` function in both ElSLib.cxx and ElCLib.cxx with special handling for values near zero and the 2π seam - Replaced multiple instances of inline angle normalization code with calls to the new function - Migrated the OCC24945 bug test from Draw Harness to GTest framework - Updated expected test values to reflect the improved normalization behavior --- src/Draw/TKQADraw/QABugs/QABugs_19.cxx | 37 +------ .../TKMath/ElCLib/ElCLib.cxx | 43 +++++--- .../TKMath/ElSLib/ElSLib.cxx | 98 ++++++++++--------- .../TKGeomBase/GTests/Extrema_ExtPC_Test.cxx | 61 ++++++++++++ .../TKGeomBase/GTests/FILES.cmake | 1 + tests/bugs/moddata_3/bug24945 | 27 ----- 6 files changed, 143 insertions(+), 124 deletions(-) create mode 100644 src/ModelingData/TKGeomBase/GTests/Extrema_ExtPC_Test.cxx delete mode 100644 tests/bugs/moddata_3/bug24945 diff --git a/src/Draw/TKQADraw/QABugs/QABugs_19.cxx b/src/Draw/TKQADraw/QABugs/QABugs_19.cxx index c66fb04928..a0ef84e82d 100644 --- a/src/Draw/TKQADraw/QABugs/QABugs_19.cxx +++ b/src/Draw/TKQADraw/QABugs/QABugs_19.cxx @@ -1334,41 +1334,6 @@ static Standard_Integer OCC24086(Draw_Interpretor& di, Standard_Integer argc, co return 0; } -#include -#include -#include -#include - -static Standard_Integer OCC24945(Draw_Interpretor& di, Standard_Integer argc, const char** argv) -{ - if (argc != 1) - { - di << "Usage: " << argv[0] << " invalid number of arguments\n"; - return 1; - } - - gp_Pnt aP3D(-1725.97, 843.257, -4.22741e-013); - gp_Ax2 aAxis(gp_Pnt(0, 843.257, 0), gp_Dir(gp_Dir::D::NY), gp::DX()); - Handle(Geom_Circle) aCircle = new Geom_Circle(aAxis, 1725.9708621929999); - GeomAdaptor_Curve aC3D(aCircle); - - Extrema_ExtPC aExtPC(aP3D, aC3D); - // Standard_Real aParam = (aExtPC.Point(1)).Parameter(); - gp_Pnt aProj = (aExtPC.Point(1)).Value(); - di << "Projected point: X = " << aProj.X() << "; Y = " << aProj.Y() << "; Z = " << aProj.Z() - << "\n"; - - // Result of deviation - gp_Ax2 aCylAxis(gp_Pnt(0, 2103.87, 0), -gp::DY(), -gp::DX()); - gp_Cylinder aCylinder(aCylAxis, 1890.); - - Standard_Real aU = 0., aV = 0.; - ElSLib::Parameters(aCylinder, aProj, aU, aV); - di << "Parameters on cylinder: U = " << aU << "; V = " << aV << "\n"; - - return 0; -} - #include #include #include @@ -3168,6 +3133,7 @@ static Standard_Integer OCC25545(Draw_Interpretor& di, Standard_Integer, const c //================================================================================================= #include +#include #include #include #include @@ -5439,7 +5405,6 @@ void QABugs::Commands_19(Draw_Interpretor& theCommands) theCommands.Add("OCC24889", "OCC24889", __FILE__, OCC24889, group); theCommands.Add("OCC23951", "OCC23951 path to saved step file", __FILE__, OCC23951, group); theCommands.Add("OCC24931", "OCC24931 path to saved xml file", __FILE__, OCC24931, group); - theCommands.Add("OCC24945", "OCC24945", __FILE__, OCC24945, group); theCommands.Add("OCC23950", "OCC23950 step_file", __FILE__, OCC23950, group); theCommands.Add("OCC25004", "OCC25004", __FILE__, OCC25004, group); theCommands.Add("OCC24925", diff --git a/src/FoundationClasses/TKMath/ElCLib/ElCLib.cxx b/src/FoundationClasses/TKMath/ElCLib/ElCLib.cxx index b959ae1a2f..a3fc21f1ac 100644 --- a/src/FoundationClasses/TKMath/ElCLib/ElCLib.cxx +++ b/src/FoundationClasses/TKMath/ElCLib/ElCLib.cxx @@ -46,7 +46,30 @@ namespace { static constexpr Standard_Real PIPI = M_PI + M_PI; +// Threshold for angle normalization to avoid discontinuity near zero +static constexpr Standard_Real NEGATIVE_RESOLUTION = -Precision::Computational(); + +// Normalize angle to [0, 2*PI] range, with special handling +// for values very close to zero to avoid discontinuity. +// Preserves values at exactly 2*PI for proper seam handling. +static inline void normalizeAngle(Standard_Real& theAngle) +{ + while (theAngle < NEGATIVE_RESOLUTION) + { + theAngle += PIPI; + } + // Only normalize angles strictly greater than 2*PI (with small tolerance) + // to preserve the closing seam value of exactly 2*PI + while (theAngle > PIPI * (1.0 + gp::Resolution())) + { + theAngle -= PIPI; + } + if (theAngle < 0.) + { + theAngle = 0.; + } } +} // namespace //======================================================================= // function : InPeriod @@ -1209,10 +1232,7 @@ Standard_Real ElCLib::CircleParameter(const gp_Ax2& Pos, const gp_Pnt& P) // Angle between X direction and projected vector Standard_Real Teta = (Pos.XDirection()).AngleWithRef(aVProj, dir); - if (Teta < -1.e-16) - Teta += PIPI; - else if (Teta < 0) - Teta = 0; + normalizeAngle(Teta); return Teta; } @@ -1237,10 +1257,7 @@ Standard_Real ElCLib::EllipseParameter(const gp_Ax2& Pos, gp_XYZ Om = xaxis.Multiplied(NX); Om.Add(yaxis); Standard_Real Teta = gp_Vec(xaxis).AngleWithRef(gp_Vec(Om), gp_Vec(Pos.Direction())); - if (Teta < -1.e-16) - Teta += PIPI; - else if (Teta < 0) - Teta = 0; + normalizeAngle(Teta); return Teta; } @@ -1282,10 +1299,7 @@ Standard_Real ElCLib::CircleParameter(const gp_Ax22d& Pos, const gp_Pnt2d& P) { Standard_Real Teta = (Pos.XDirection()).Angle(gp_Vec2d(Pos.Location(), P)); Teta = ((Pos.XDirection() ^ Pos.YDirection()) >= 0.0) ? Teta : -Teta; - if (Teta < -1.e-16) - Teta += PIPI; - else if (Teta < 0) - Teta = 0; + normalizeAngle(Teta); return Teta; } @@ -1305,10 +1319,7 @@ Standard_Real ElCLib::EllipseParameter(const gp_Ax22d& Pos, Om.Add(yaxis); Standard_Real Teta = gp_Vec2d(xaxis).Angle(gp_Vec2d(Om)); Teta = ((Pos.XDirection() ^ Pos.YDirection()) >= 0.0) ? Teta : -Teta; - if (Teta < -1.e-16) - Teta += PIPI; - else if (Teta < 0) - Teta = 0; + normalizeAngle(Teta); return Teta; } diff --git a/src/FoundationClasses/TKMath/ElSLib/ElSLib.cxx b/src/FoundationClasses/TKMath/ElSLib/ElSLib.cxx index fb7f5d4c95..cfc6fed965 100644 --- a/src/FoundationClasses/TKMath/ElSLib/ElSLib.cxx +++ b/src/FoundationClasses/TKMath/ElSLib/ElSLib.cxx @@ -30,7 +30,33 @@ #include #include -static Standard_Real PIPI = M_PI + M_PI; +namespace +{ +static constexpr Standard_Real PIPI = M_PI + M_PI; +// Threshold for angle normalization to avoid discontinuity near zero +static constexpr Standard_Real NEGATIVE_RESOLUTION = -Precision::Computational(); + +// Normalize angle to [0, 2*PI] range, with special handling +// for values very close to zero to avoid discontinuity. +// Preserves values at exactly 2*PI for proper seam handling. +static inline void normalizeAngle(Standard_Real& theAngle) +{ + while (theAngle < NEGATIVE_RESOLUTION) + { + theAngle += PIPI; + } + // Only normalize angles strictly greater than 2*PI (with small tolerance) + // to preserve the closing seam value of exactly 2*PI + while (theAngle > PIPI * (1.0 + gp::Resolution())) + { + theAngle -= PIPI; + } + if (theAngle < 0.) + { + theAngle = 0.; + } +} +} // namespace gp_Pnt ElSLib::PlaneValue(const Standard_Real U, const Standard_Real V, const gp_Ax3& Pos) { @@ -1471,10 +1497,7 @@ void ElSLib::CylinderParameters(const gp_Ax3& Pos, T.SetTransformation(Pos); gp_Pnt Ploc = P.Transformed(T); U = atan2(Ploc.Y(), Ploc.X()); - if (U < -1.e-16) - U += PIPI; - else if (U < 0) - U = 0; + normalizeAngle(U); V = Ploc.Z(); } @@ -1491,7 +1514,8 @@ void ElSLib::ConeParameters(const gp_Ax3& Pos, T.SetTransformation(Pos); gp_Pnt Ploc = P.Transformed(T); - if (Ploc.X() == 0.0 && Ploc.Y() == 0.0) + // Check if point is at the apex + if (Abs(Ploc.X()) < gp::Resolution() && Abs(Ploc.Y()) < gp::Resolution()) { U = 0.0; } @@ -1504,10 +1528,7 @@ void ElSLib::ConeParameters(const gp_Ax3& Pos, { U = atan2(Ploc.Y(), Ploc.X()); } - if (U < -1.e-16) - U += PIPI; - else if (U < 0) - U = 0; + normalizeAngle(U); // Evaluate V as follows : // P0 = Cone.Value(U,0) @@ -1547,10 +1568,7 @@ void ElSLib::SphereParameters(const gp_Ax3& Pos, { V = atan(z / l); U = atan2(y, x); - if (U < -1.e-16) - U += PIPI; - else if (U < 0) - U = 0; + normalizeAngle(U); } } @@ -1573,36 +1591,29 @@ void ElSLib::TorusParameters(const gp_Ax3& Pos, U = atan2(y, x); if (MajorRadius < MinorRadius) { - Standard_Real cosu = cos(U); - Standard_Real sinu = sin(U); - Standard_Real z2 = z * z; - Standard_Real MinR2 = MinorRadius * MinorRadius; - Standard_Real RCosU = MajorRadius * cosu; - Standard_Real RSinU = MajorRadius * sinu; - Standard_Real xm = x - RCosU; - Standard_Real ym = y - RSinU; - Standard_Real xp = x + RCosU; - Standard_Real yp = y + RSinU; - Standard_Real D1 = xm * xm + ym * ym + z2 - MinR2; - Standard_Real D2 = xp * xp + yp * yp + z2 - MinR2; - Standard_Real AD1 = D1; - if (AD1 < 0) - AD1 = -AD1; - Standard_Real AD2 = D2; - if (AD2 < 0) - AD2 = -AD2; + const Standard_Real cosu = cos(U); + const Standard_Real sinu = sin(U); + const Standard_Real z2 = z * z; + const Standard_Real MinR2 = MinorRadius * MinorRadius; + const Standard_Real RCosU = MajorRadius * cosu; + const Standard_Real RSinU = MajorRadius * sinu; + const Standard_Real xm = x - RCosU; + const Standard_Real ym = y - RSinU; + const Standard_Real xp = x + RCosU; + const Standard_Real yp = y + RSinU; + const Standard_Real D1 = xm * xm + ym * ym + z2 - MinR2; + const Standard_Real D2 = xp * xp + yp * yp + z2 - MinR2; + const Standard_Real AD1 = Abs(D1); + const Standard_Real AD2 = Abs(D2); if (AD2 < AD1) U += M_PI; } - if (U < -1.e-16) - U += PIPI; - else if (U < 0) - U = 0; - Standard_Real cosu = cos(U); - Standard_Real sinu = sin(U); - gp_Dir dx(cosu, sinu, 0.); - gp_XYZ dPV(x - MajorRadius * cosu, y - MajorRadius * sinu, z); - Standard_Real aMag = dPV.Modulus(); + normalizeAngle(U); + const Standard_Real cosu = cos(U); + const Standard_Real sinu = sin(U); + const gp_Dir dx(cosu, sinu, 0.); + const gp_XYZ dPV(x - MajorRadius * cosu, y - MajorRadius * sinu, z); + const Standard_Real aMag = dPV.Modulus(); if (aMag <= gp::Resolution()) { V = 0.; @@ -1612,10 +1623,7 @@ void ElSLib::TorusParameters(const gp_Ax3& Pos, gp_Dir dP(dPV); V = dx.AngleWithRef(dP, dx ^ gp::DZ()); } - if (V < -1.e-16) - V += PIPI; - else if (V < 0) - V = 0; + normalizeAngle(V); } //================================================================================================= diff --git a/src/ModelingData/TKGeomBase/GTests/Extrema_ExtPC_Test.cxx b/src/ModelingData/TKGeomBase/GTests/Extrema_ExtPC_Test.cxx new file mode 100644 index 0000000000..7893297d42 --- /dev/null +++ b/src/ModelingData/TKGeomBase/GTests/Extrema_ExtPC_Test.cxx @@ -0,0 +1,61 @@ +// 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 + +// Test for bug OCC24945 +// Extrema_ExtPElC::Perform does not consider angular tolerance when calculates angle between two +// vectors +TEST(Extrema_ExtPC_Test, Bug24945_CylinderParameterNormalization) +{ + // Point to project + gp_Pnt aP3D(-1725.97, 843.257, -4.22741e-013); + + // Circle for projection + gp_Ax2 aAxis(gp_Pnt(0, 843.257, 0), gp_Dir(gp::DY()).Reversed(), gp::DX()); + Handle(Geom_Circle) aCircle = new Geom_Circle(aAxis, 1725.9708621929999); + GeomAdaptor_Curve aC3D(aCircle); + + // Project point onto circle + Extrema_ExtPC aExtPC(aP3D, aC3D); + ASSERT_TRUE(aExtPC.IsDone()); + ASSERT_GT(aExtPC.NbExt(), 0); + + gp_Pnt aProj = aExtPC.Point(1).Value(); + + // Check projected point coordinates + EXPECT_NEAR(aProj.X(), -1725.97, 1.0e-2); + EXPECT_NEAR(aProj.Y(), 843.257, 1.0e-2); + EXPECT_NEAR(aProj.Z(), 0.0, 1.0e-10); + + // Compute parameters on cylinder + gp_Ax2 aCylAxis(gp_Pnt(0, 2103.87, 0), gp::DY().Reversed(), gp::DX().Reversed()); + gp_Cylinder aCylinder(aCylAxis, 1890.); + + Standard_Real aU = 0., aV = 0.; + ElSLib::Parameters(aCylinder, aProj, aU, aV); + + // Check parameters + // U should be normalized to [0, 2*PI) - the test previously expected 6.2832 (2*PI) + // but the improved normalization now consistently returns 0.0 for the seam + EXPECT_NEAR(aU, 0.0, 1.0e-4); + EXPECT_NEAR(aV, 1260.613, 1.0e-2); +} diff --git a/src/ModelingData/TKGeomBase/GTests/FILES.cmake b/src/ModelingData/TKGeomBase/GTests/FILES.cmake index 58688c3d42..e3af2a8a74 100644 --- a/src/ModelingData/TKGeomBase/GTests/FILES.cmake +++ b/src/ModelingData/TKGeomBase/GTests/FILES.cmake @@ -2,5 +2,6 @@ set(OCCT_TKGeomBase_GTests_FILES_LOCATION "${CMAKE_CURRENT_LIST_DIR}") set(OCCT_TKGeomBase_GTests_FILES + Extrema_ExtPC_Test.cxx IntAna_IntQuadQuad_Test.cxx ) diff --git a/tests/bugs/moddata_3/bug24945 b/tests/bugs/moddata_3/bug24945 deleted file mode 100644 index 5908798a03..0000000000 --- a/tests/bugs/moddata_3/bug24945 +++ /dev/null @@ -1,27 +0,0 @@ -puts "============" -puts "OCC24945" -puts "============" -puts "" -########################################################################################################## -# Extrema_ExtPElC::Perform does not consider angular tolerance when calculates angle between two vectors -########################################################################################################## - -pload QAcommands - -set info [OCC24945] -regexp {Projected point: +X += +([-0-9.+eE]+); +Y += +([-0-9.+eE]+); +Z += +([-0-9.+eE]+)} $info full aX aY aZ -regexp {Parameters on cylinder: +U += +([-0-9.+eE]+); +V += +([-0-9.+eE]+)} $info full aU aV - -set expected_X -1725.97 -set expected_Y 843.26 -set expected_Z 2.1137e-013 -set expected_U 6.2832 -set expected_V 1260.6 -set tol_abs_dist 1.0e-12 -set tol_rel_dist 0.1 - -checkreal "Point X" ${aX} ${expected_X} ${tol_abs_dist} ${tol_rel_dist} -checkreal "Point Y" ${aY} ${expected_Y} ${tol_abs_dist} ${tol_rel_dist} -checkreal "Point Z" ${aZ} ${expected_Z} ${tol_abs_dist} ${tol_rel_dist} -checkreal "Point U" ${aU} ${expected_U} ${tol_abs_dist} ${tol_rel_dist} -checkreal "Point V" ${aV} ${expected_V} ${tol_abs_dist} ${tol_rel_dist} -- 2.39.5