From d7abd72eacdb41639bf23e40de8614985a7fdd1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Loke=20Str=C3=B8m?= Date: Fri, 27 Feb 2026 09:39:14 +0100 Subject: [PATCH] Modeling, Hyperbola - Fixing over constraining direction errors (#1100) - Removed `gce_InvertAxis` and `gce_InvertRadius` error checks from hyperbola constructors - Added collinear point validation to the 3-point constructors - Updated documentation to reflect the correct validation rules --- .../TKGeomBase/GTests/FILES.cmake | 1 + .../TKGeomBase/GTests/gce_MakeHypr_Test.cxx | 266 ++++++++++++++++++ .../TKGeomBase/gce/gce_MakeHypr.cxx | 51 ++-- .../TKGeomBase/gce/gce_MakeHypr.hxx | 9 +- .../TKGeomBase/gce/gce_MakeHypr2d.cxx | 49 ++-- .../TKGeomBase/gce/gce_MakeHypr2d.hxx | 5 + 6 files changed, 336 insertions(+), 45 deletions(-) create mode 100644 src/ModelingData/TKGeomBase/GTests/gce_MakeHypr_Test.cxx diff --git a/src/ModelingData/TKGeomBase/GTests/FILES.cmake b/src/ModelingData/TKGeomBase/GTests/FILES.cmake index 638bec741a..4ae3377112 100644 --- a/src/ModelingData/TKGeomBase/GTests/FILES.cmake +++ b/src/ModelingData/TKGeomBase/GTests/FILES.cmake @@ -10,4 +10,5 @@ set(OCCT_TKGeomBase_GTests_FILES GeomConvert_Test.cxx Hermit_Test.cxx IntAna_IntQuadQuad_Test.cxx + gce_MakeHypr_Test.cxx ) diff --git a/src/ModelingData/TKGeomBase/GTests/gce_MakeHypr_Test.cxx b/src/ModelingData/TKGeomBase/GTests/gce_MakeHypr_Test.cxx new file mode 100644 index 0000000000..7a1a047033 --- /dev/null +++ b/src/ModelingData/TKGeomBase/GTests/gce_MakeHypr_Test.cxx @@ -0,0 +1,266 @@ +// 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 + +// ==================== gce_MakeHypr (3D) ==================== + +TEST(gce_MakeHyprTest, FromAxis_ValidRadii_Done) +{ + gce_MakeHypr aMaker(gp_Ax2(), 10.0, 5.0); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 10.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 5.0, Precision::Confusion()); +} + +TEST(gce_MakeHyprTest, FromAxis_MajorLessThanMinor_Done) +{ + gce_MakeHypr aMaker(gp_Ax2(), 3.0, 7.0); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 3.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 7.0, Precision::Confusion()); +} + +TEST(gce_MakeHyprTest, FromAxis_EqualRadii_Done) +{ + gce_MakeHypr aMaker(gp_Ax2(), 5.0, 5.0); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); +} + +TEST(gce_MakeHyprTest, FromAxis_ZeroRadii_Done) +{ + gce_MakeHypr aMaker(gp_Ax2(), 0.0, 0.0); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); +} + +TEST(gce_MakeHyprTest, FromAxis_NegativeMajor_NegativeRadius) +{ + gce_MakeHypr aMaker(gp_Ax2(), -1.0, 5.0); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_NegativeRadius); +} + +TEST(gce_MakeHyprTest, FromAxis_NegativeMinor_NegativeRadius) +{ + gce_MakeHypr aMaker(gp_Ax2(), 5.0, -1.0); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_NegativeRadius); +} + +TEST(gce_MakeHyprTest, FromAxis_BothNegative_NegativeRadius) +{ + gce_MakeHypr aMaker(gp_Ax2(), -3.0, -2.0); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_NegativeRadius); +} + +TEST(gce_MakeHyprTest, FromPoints_Valid_Done) +{ + const gp_Pnt aCenter(0.0, 0.0, 0.0); + const gp_Pnt aS1(10.0, 0.0, 0.0); + const gp_Pnt aS2(0.0, 5.0, 0.0); + + gce_MakeHypr aMaker(aS1, aS2, aCenter); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 10.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 5.0, Precision::Confusion()); +} + +TEST(gce_MakeHyprTest, FromPoints_MajorLessThanMinor_Done) +{ + const gp_Pnt aCenter(0.0, 0.0, 0.0); + const gp_Pnt aS1(3.0, 0.0, 0.0); + const gp_Pnt aS2(0.0, 7.0, 0.0); + + gce_MakeHypr aMaker(aS1, aS2, aCenter); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 3.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 7.0, Precision::Confusion()); +} + +TEST(gce_MakeHyprTest, FromPoints_ConfusedS1Center_ConfusedPoints) +{ + const gp_Pnt aCenter(1.0, 2.0, 3.0); + const gp_Pnt aS1(1.0, 2.0, 3.0); + const gp_Pnt aS2(4.0, 5.0, 6.0); + + gce_MakeHypr aMaker(aS1, aS2, aCenter); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_ConfusedPoints); +} + +TEST(gce_MakeHyprTest, FromPoints_ConfusedS2Center_ConfusedPoints) +{ + const gp_Pnt aCenter(1.0, 2.0, 3.0); + const gp_Pnt aS1(4.0, 5.0, 6.0); + const gp_Pnt aS2(1.0, 2.0, 3.0); + + gce_MakeHypr aMaker(aS1, aS2, aCenter); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_ConfusedPoints); +} + +TEST(gce_MakeHyprTest, FromPoints_ConfusedS1S2_ConfusedPoints) +{ + const gp_Pnt aCenter(0.0, 0.0, 0.0); + const gp_Pnt aS1(5.0, 5.0, 5.0); + const gp_Pnt aS2(5.0, 5.0, 5.0); + + gce_MakeHypr aMaker(aS1, aS2, aCenter); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_ConfusedPoints); +} + +TEST(gce_MakeHyprTest, FromPoints_Collinear_ColinearPoints) +{ + const gp_Pnt aCenter(0.0, 0.0, 0.0); + const gp_Pnt aS1(10.0, 0.0, 0.0); + const gp_Pnt aS2(5.0, 0.0, 0.0); + + gce_MakeHypr aMaker(aS1, aS2, aCenter); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_ColinearPoints); +} + +// ==================== gce_MakeHypr2d (2D) ==================== + +TEST(gce_MakeHypr2dTest, FromMajorAxis_ValidRadii_Done) +{ + gce_MakeHypr2d aMaker(gp_Ax2d(), 10.0, 5.0, true); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr2d& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 10.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 5.0, Precision::Confusion()); +} + +TEST(gce_MakeHypr2dTest, FromMajorAxis_MajorLessThanMinor_Done) +{ + gce_MakeHypr2d aMaker(gp_Ax2d(), 3.0, 7.0, true); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr2d& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 3.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 7.0, Precision::Confusion()); +} + +TEST(gce_MakeHypr2dTest, FromMajorAxis_NegativeRadius_NegativeRadius) +{ + gce_MakeHypr2d aMaker(gp_Ax2d(), -1.0, 5.0, true); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_NegativeRadius); +} + +TEST(gce_MakeHypr2dTest, FromAx22d_ValidRadii_Done) +{ + gce_MakeHypr2d aMaker(gp_Ax22d(), 8.0, 4.0); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); +} + +TEST(gce_MakeHypr2dTest, FromAx22d_NegativeRadius_NegativeRadius) +{ + gce_MakeHypr2d aMaker(gp_Ax22d(), 5.0, -2.0); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_NegativeRadius); +} + +TEST(gce_MakeHypr2dTest, FromPoints_Valid_Done) +{ + const gp_Pnt2d aCenter(0.0, 0.0); + const gp_Pnt2d aS1(10.0, 0.0); + const gp_Pnt2d aS2(0.0, 5.0); + + gce_MakeHypr2d aMaker(aS1, aS2, aCenter); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr2d& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 10.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 5.0, Precision::Confusion()); +} + +TEST(gce_MakeHypr2dTest, FromPoints_MajorLessThanMinor_Done) +{ + const gp_Pnt2d aCenter(0.0, 0.0); + const gp_Pnt2d aS1(3.0, 0.0); + const gp_Pnt2d aS2(0.0, 7.0); + + gce_MakeHypr2d aMaker(aS1, aS2, aCenter); + EXPECT_TRUE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_Done); + + const gp_Hypr2d& aHypr = aMaker.Value(); + EXPECT_NEAR(aHypr.MajorRadius(), 3.0, Precision::Confusion()); + EXPECT_NEAR(aHypr.MinorRadius(), 7.0, Precision::Confusion()); +} + +TEST(gce_MakeHypr2dTest, FromPoints_ConfusedS1Center_ConfusedPoints) +{ + const gp_Pnt2d aCenter(1.0, 2.0); + const gp_Pnt2d aS1(1.0, 2.0); + const gp_Pnt2d aS2(4.0, 5.0); + + gce_MakeHypr2d aMaker(aS1, aS2, aCenter); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_ConfusedPoints); +} + +TEST(gce_MakeHypr2dTest, FromPoints_ConfusedS2Center_ConfusedPoints) +{ + const gp_Pnt2d aCenter(1.0, 2.0); + const gp_Pnt2d aS1(4.0, 5.0); + const gp_Pnt2d aS2(1.0, 2.0); + + gce_MakeHypr2d aMaker(aS1, aS2, aCenter); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_ConfusedPoints); +} + +TEST(gce_MakeHypr2dTest, FromPoints_Collinear_ColinearPoints) +{ + const gp_Pnt2d aCenter(0.0, 0.0); + const gp_Pnt2d aS1(10.0, 0.0); + const gp_Pnt2d aS2(5.0, 0.0); + + gce_MakeHypr2d aMaker(aS1, aS2, aCenter); + EXPECT_FALSE(aMaker.IsDone()); + EXPECT_EQ(aMaker.Status(), gce_ColinearPoints); +} diff --git a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.cxx b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.cxx index 3b58f22a0b..71e7c6fef6 100644 --- a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.cxx +++ b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.cxx @@ -15,43 +15,46 @@ // commercial license or contractual agreement. #include +#include #include #include #include #include #include -//========================================================================= -// Creation d une Hyperbole 3d de gp de centre
et de sommets + -// et . + -// donne le grand axe . + -// donne le grand rayon et le petit rayon. + -//========================================================================= +//================================================================================================== + gce_MakeHypr::gce_MakeHypr(const gp_Pnt& S1, const gp_Pnt& S2, const gp_Pnt& Center) { - gp_Dir XAxis(gp_XYZ(S1.XYZ() - Center.XYZ())); - gp_Lin L(Center, XAxis); - double D = S1.Distance(Center); - double d = L.Distance(S2); - if (d > D) + const gp_XYZ aVecS1 = S1.XYZ() - Center.XYZ(); + const gp_XYZ aVecS2 = S2.XYZ() - Center.XYZ(); + const double aDistS1 = aVecS1.Modulus(); + const double aDistS2 = aVecS2.Modulus(); + if (aDistS1 <= gp::Resolution() || aDistS2 <= gp::Resolution() + || S1.Distance(S2) <= gp::Resolution()) { - TheError = gce_InvertAxis; + TheError = gce_ConfusedPoints; + return; } - else + + const gp_Dir aXAxis(aVecS1); + const gp_Lin aLine(Center, aXAxis); + const double aMinorDist = aLine.Distance(S2); + if (aMinorDist <= gp::Resolution()) { - gp_Dir Norm(XAxis.Crossed(gp_Dir(gp_XYZ(S2.XYZ() - Center.XYZ())))); - TheHypr = gp_Hypr(gp_Ax2(Center, Norm, XAxis), D, d); - TheError = gce_Done; + TheError = gce_ColinearPoints; + return; } + const gp_Dir aNorm(aXAxis.Crossed(gp_Dir(aVecS2))); + TheHypr = gp_Hypr(gp_Ax2(Center, aNorm, aXAxis), aDistS1, aMinorDist); + TheError = gce_Done; } +//================================================================================================== + gce_MakeHypr::gce_MakeHypr(const gp_Ax2& A2, const double MajorRadius, const double MinorRadius) { - if (MajorRadius < MinorRadius) - { - TheError = gce_InvertRadius; - } - else if (MajorRadius < 0.0) + if (MajorRadius < 0.0 || MinorRadius < 0.0) { TheError = gce_NegativeRadius; } @@ -62,17 +65,23 @@ gce_MakeHypr::gce_MakeHypr(const gp_Ax2& A2, const double MajorRadius, const dou } } +//================================================================================================== + const gp_Hypr& gce_MakeHypr::Value() const { StdFail_NotDone_Raise_if(TheError != gce_Done, "gce_MakeHypr::Value() - no result"); return TheHypr; } +//================================================================================================== + const gp_Hypr& gce_MakeHypr::Operator() const { return Value(); } +//================================================================================================== + gce_MakeHypr::operator gp_Hypr() const { return Value(); diff --git a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.hxx b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.hxx index a0d1800516..10e7806c02 100644 --- a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.hxx +++ b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr.hxx @@ -76,8 +76,7 @@ public: //! MinorRadius. //! For the hyperbola the MajorRadius can be lower than the //! MinorRadius. - //! The status is "NegativeRadius" if MajorRadius < 0.0 and - //! "InvertRadius" if MinorRadius > MajorRadius. + //! The status is "NegativeRadius" if MajorRadius < 0.0 or MinorRadius < 0.0 Standard_EXPORT gce_MakeHypr(const gp_Ax2& A2, const double MajorRadius, const double MinorRadius); @@ -91,11 +90,7 @@ public: //! Warning //! If an error occurs (that is, when IsDone returns //! false), the Status function returns: - //! - gce_NegativeRadius if MajorRadius is less than 0.0; - //! - gce_InvertRadius if: - //! - the major radius (computed with Center, S1) is - //! less than the minor radius (computed with Center, S1 and S2), or - //! - MajorRadius is less than MinorRadius; or + //! - gce_ConfusedPoints if any two of S1, S2 and Center are coincident; //! - gce_ColinearPoints if S1, S2 and Center are collinear. Standard_EXPORT gce_MakeHypr(const gp_Pnt& S1, const gp_Pnt& S2, const gp_Pnt& Center); diff --git a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.cxx b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.cxx index b4ae08011f..86fb08ece2 100644 --- a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.cxx +++ b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.cxx @@ -15,6 +15,7 @@ // commercial license or contractual agreement. #include +#include #include #include #include @@ -22,31 +23,37 @@ #include #include -//========================================================================= -// Creation d une Hyperbola 2d de gp de centre
et de sommets + -// et . + -// donne le grand axe . + -// donne le grand rayon et le petit rayon. + -//========================================================================= +//================================================================================================== + gce_MakeHypr2d::gce_MakeHypr2d(const gp_Pnt2d& S1, const gp_Pnt2d& S2, const gp_Pnt2d& Center) { - gp_Dir2d XAxis(gp_XY(S1.XY() - Center.XY())); - gp_Dir2d YAxis(gp_XY(S2.XY() - Center.XY())); - gp_Ax22d Axis(Center, XAxis, YAxis); - gp_Lin2d L(Center, XAxis); - double D1 = S1.Distance(Center); - double D2 = L.Distance(S2); - if (D1 >= D2) + const gp_XY aVecS1 = S1.XY() - Center.XY(); + const gp_XY aVecS2 = S2.XY() - Center.XY(); + const double aDistS1 = aVecS1.Modulus(); + const double aDistS2 = aVecS2.Modulus(); + if (aDistS1 <= gp::Resolution() || aDistS2 <= gp::Resolution() + || S1.Distance(S2) <= gp::Resolution()) { - TheHypr2d = gp_Hypr2d(Axis, D1, D2); - TheError = gce_Done; + TheError = gce_ConfusedPoints; + return; } - else + + const gp_Dir2d aXAxis(aVecS1); + const gp_Dir2d aYAxis(aVecS2); + const gp_Ax22d anAxis(Center, aXAxis, aYAxis); + const gp_Lin2d aLine(Center, aXAxis); + const double aMinorDist = aLine.Distance(S2); + if (aMinorDist <= gp::Resolution()) { - TheError = gce_InvertAxis; + TheError = gce_ColinearPoints; + return; } + TheHypr2d = gp_Hypr2d(anAxis, aDistS1, aMinorDist); + TheError = gce_Done; } +//================================================================================================== + gce_MakeHypr2d::gce_MakeHypr2d(const gp_Ax2d& MajorAxis, const double MajorRadius, const double MinorRadius, @@ -63,6 +70,8 @@ gce_MakeHypr2d::gce_MakeHypr2d(const gp_Ax2d& MajorAxis, } } +//================================================================================================== + gce_MakeHypr2d::gce_MakeHypr2d(const gp_Ax22d& A, const double MajorRadius, const double MinorRadius) @@ -78,17 +87,23 @@ gce_MakeHypr2d::gce_MakeHypr2d(const gp_Ax22d& A, } } +//================================================================================================== + const gp_Hypr2d& gce_MakeHypr2d::Value() const { StdFail_NotDone_Raise_if(TheError != gce_Done, "gce_MakeHypr2d::Value() - no result"); return TheHypr2d; } +//================================================================================================== + const gp_Hypr2d& gce_MakeHypr2d::Operator() const { return Value(); } +//================================================================================================== + gce_MakeHypr2d::operator gp_Hypr2d() const { return Value(); diff --git a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.hxx b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.hxx index b5a12a92fb..c9ce3c3573 100644 --- a/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.hxx +++ b/src/ModelingData/TKGeomBase/gce/gce_MakeHypr2d.hxx @@ -70,6 +70,11 @@ public: //! - the major axis of the hyperbola is defined by Center and point S1, //! - the major radius is the distance between Center and S1, and //! - the minor radius is the distance between point S2 and the major axis. + //! Warning + //! If an error occurs (that is, when IsDone returns + //! false), the Status function returns: + //! - gce_ConfusedPoints if any two of S1, S2 and Center are coincident; + //! - gce_ColinearPoints if S1, S2 and Center are collinear. Standard_EXPORT gce_MakeHypr2d(const gp_Pnt2d& S1, const gp_Pnt2d& S2, const gp_Pnt2d& Center); //! Constructs a hyperbola with major and minor radii MajorRadius and