Modeling - Refactor Extrema package#869
Merged
dpasukhi merged 20 commits intoOpen-Cascade-SAS:IRfrom Dec 6, 2025
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Extrema package in the Open CASCADE Technology (OCCT) library by converting legacy .gxx generic template files and .lxx inline files to modern C++ template headers (.hxx). The refactoring modernizes the codebase by:
- Replacing preprocessor-based generic programming with C++ templates
- Converting inline implementation files (
.lxx) to inline definitions within header files - Removing legacy instantiation files (
_0.cxx) - Updating type aliases to use modern
usingdeclarations instead of macro-based instantiation
Reviewed changes
Copilot reviewed 99 out of 99 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Extrema/FILES.cmake |
Removed legacy .gxx, .lxx, and _0.cxx files; added new template header files with G prefix |
Extrema_POnSurf*.hxx/lxx |
Converted inline implementations to header-only; replaced Standard_Real/Integer with double/int |
Extrema_POnCurv*.hxx |
Merged .cxx implementations into headers; modernized types and removed Standard_EXPORT |
Extrema_PCFOfEPCOfExtPC*.hxx |
Replaced concrete classes with type aliases using Extrema_GFuncExtPC template |
Extrema_LocateExtPC*.hxx |
Converted to type aliases using Extrema_GLocateExtPC template |
Extrema_LocECC*.hxx |
Replaced implementations with type aliases to Extrema_GenLocateExtCC template |
Extrema_GenLocateExtPC*.hxx/gxx |
Converted .gxx generic file to modern C++ template header |
Extrema_GenLocateExtCC*.hxx/gxx |
Converted .gxx to template class implementation |
Extrema_GGenExtPC.hxx |
New template class for generic point-curve extremum computation |
Extrema_GGenExtCC.hxx |
New template class for generic curve-curve extremum computation |
Extrema_GGExtPC.hxx |
New template class for comprehensive point-curve extremum search |
Extrema_GFuncExtPC.hxx |
New template class for extremal distance function computation |
Extrema_GCurveLocator.hxx |
New template class for locating closest point on curve |
Extrema_ExtCC2d.hxx |
Added missing include for Extrema_ECC2d.hxx |
HLRBRep_TheProjPCurOfCInter.hxx |
Updated include order and removed forward declarations |
| //! Creation of a point on surface with parameter | ||
| //! values on the surface and a Pnt from gp. | ||
| Extrema_POnSurf(const Standard_Real U, const Standard_Real V, const gp_Pnt& P); | ||
| Extrema_POnSurf(const double theU, const double theV, const gp_Pnt& theP) |
There was a problem hiding this comment.
Parameter names have changed from 'U, V, P' to 'theU, theV, theP' which follows OCCT naming conventions, but this is an API change. Existing code using positional arguments will continue to work, but code relying on named parameters will break.
| //! Creation of a point on surface with parameter | ||
| //! values on the surface and a Pnt from gp. | ||
| Extrema_POnSurfParams(const Standard_Real theU, const Standard_Real theV, const gp_Pnt& thePnt); | ||
| Extrema_POnSurfParams(const double theU, const double theV, const gp_Pnt& thePnt) |
There was a problem hiding this comment.
Changing from Standard_Real to double is a breaking API change that affects binary compatibility. While Standard_Real is typically aliased to double, explicit use of double breaks the abstraction and may cause issues with code that depends on Standard_Real being a distinct type.
Comment on lines
+64
to
+67
| Extrema_GenLocateExtPC(const ThePnt& theP, | ||
| const TheCurve& theC, | ||
| const double theU0, | ||
| const double theTolU) |
There was a problem hiding this comment.
Using double directly instead of Standard_Real deviates from OCCT conventions. According to guideline 1000000, Standard_Real should be used for all floating-point numbers to maintain consistency with the OCCT type system.
| } | ||
|
|
||
| //! Returns True if the distance is found. | ||
| bool IsDone() const { return myDone; } |
There was a problem hiding this comment.
Return type changed from Standard_Boolean to bool. This is inconsistent with OCCT coding guidelines which specify Standard_Boolean should be used for boolean values to maintain type consistency across the codebase.
Comment on lines
+151
to
+152
| inline void Extrema_GGenExtCC_ChangeIntervals(Handle(TColStd_HArray1OfReal)& theInts, | ||
| const int theNbInts) |
There was a problem hiding this comment.
Using int instead of Standard_Integer is inconsistent with OCCT guidelines. According to guideline 1000000, Standard_Integer should be used for all integer values.
| //! @param theU Parameter value | ||
| //! @param theF Output function value | ||
| //! @return True if computation succeeded | ||
| bool Value(const double theU, double& theF) override |
There was a problem hiding this comment.
Method signature uses double and bool instead of Standard_Real and Standard_Boolean. This breaks consistency with the inherited math_FunctionWithDerivative interface and OCCT type conventions.
2af8481 to
ad89501
Compare
…h modern C++ templates in Extrema_GenExtCC.pxx. Update FILES.cmake to reflect the new file extension and remove obsolete files.
…e and EPCOfExt, remove obsolete files, and update CMake configuration.
…ementations, remove obsolete files, and update CMake configuration.
…ateExtPCOfTheProjPCurOfGInter and HLRBRep_TheLocateExtPCOfTheProjPCurOfCInter, remove obsolete files, and update CMake configuration. Modernize Extrema_LocEPCOfLocateExtPC and Extrema_LocEPCOfLocateExtPC2d implementations.
…Functionality - Deleted the following files as part of the cleanup: - HLRBRep_TheLocateExtPCOfTheProjPCurOfCInter_0.cxx - Extrema_GenLocateExtPC.gxx - Extrema_LocEPCOfLocateExtPC.cxx - Extrema_LocEPCOfLocateExtPC.hxx - Extrema_LocEPCOfLocateExtPC2d.cxx - Extrema_LocEPCOfLocateExtPC2d.hxx - Extrema_LocEPCOfLocateExtPC2d_0.cxx - Extrema_LocEPCOfLocateExtPC_0.cxx - Updated Extrema_LocateExtPC and Extrema_LocateExtPC2d classes to remove dependencies on the deleted classes. - Introduced type aliases for Extrema_GenLocateExtPC to streamline the codebase. - Adjusted CMake files to reflect the removal of obsolete source files.
- Deleted the implementation file Extrema_GenLocateExtCC.gxx and created a new header file Extrema_GenLocateExtCC.hxx to define the class for locating local extremum of distance between two curves. - Updated Extrema_LocECC and Extrema_LocECC2d to use Extrema_GenLocateExtCC as a template class for 3D and 2D curve extremum locators, respectively. - Removed legacy implementation files Extrema_LocECC_0.cxx and Extrema_LocECC2d_0.cxx, consolidating functionality into the new template-based structure. - Updated Extrema_LocateExtPC and Extrema_LocateExtPC2d to utilize the new generic locator structure, enhancing code maintainability and reducing redundancy. - Cleaned up CMake files to reflect the removal of obsolete source files and ensure proper inclusion of new header files.
… functions - Replaced the implementation of Extrema_PCFOfEPCOfELPCOfLocateExtPC and Extrema_PCFOfEPCOfELPCOfLocateExtPC2d with type aliases using Extrema_GFuncExtPC. - Removed redundant class definitions and included necessary headers for point and vector types. - Deleted obsolete source files Extrema_PCFOfEPCOfELPCOfLocateExtPC_0.cxx and Extrema_PCFOfEPCOfELPCOfLocateExtPC2d_0.cxx. - Updated CMake files to reflect the removal of old source files and the addition of new headers. - Similar changes applied to Extrema_PCFOfEPCOfExtPC and Extrema_PCFOfEPCOfExtPC2d classes. - Refactored Extrema_PCLocFOfLocEPCOfLocateExtPC and Extrema_PCLocFOfLocEPCOfLocateExtPC2d to use type aliases as well.
- Deleted obsolete source files: Extrema_EPCOfELPCOfLocateExtPC2d_0.cxx, Extrema_EPCOfELPCOfLocateExtPC_0.cxx, Extrema_EPCOfExtPC2d_0.cxx, Extrema_EPCOfExtPC_0.cxx, and Extrema_GenExtPC.gxx. - Introduced a new generic class Extrema_GGenExtPC for finding extremal distances between a point and a curve, replacing the previous specific implementations. - Updated header files to use the new generic class and removed unnecessary class definitions. - Adjusted CMake files to reflect the removal of old source files and the addition of the new generic class header.
- Introduced a new header file Extrema_GGExtPC.hxx that implements a generic class for computing extremal distances between a point and various types of curves. - The class supports multiple curve types including lines, circles, ellipses, parabolas, Bezier, BSpline, and general curves, providing optimized algorithms for extremum search. - Updated FILES.cmake to include the new Extrema_GGExtPC.hxx and removed obsolete files Extrema_ELPCOfLocateExtPC2d_0.cxx and Extrema_ELPCOfLocateExtPC_0.cxx.
…oducing Extrema_GFuncExtCC - Deleted the Extrema_FuncExtCC.lxx file, which contained the implementation for the previous extremal distance calculation between curves. - Added a new header file Extrema_GFuncExtCC.hxx that implements a template class for finding extremal distances between two curves, enhancing flexibility and functionality. - Updated FILES.cmake to reflect the removal of Extrema_FuncExtCC and the addition of Extrema_GFuncExtCC.
…les accordingly - Deleted the Extrema_GenExtCC.pxx file, which contained the template implementations for the Extrema_GenExtCC class. - Updated FILES.cmake to remove references to the deprecated Extrema_ECC2d_0.cxx and Extrema_ECC_0.cxx files. - Renamed Extrema_GenExtCC.gxx to Extrema_GGenExtCC.hxx to reflect the changes in the implementation.
…ntations and introducing a new generic curve locator class
- Removed implementation files (.lxx) for Extrema_Curve2dTool and Extrema_CurveTool, consolidating definitions into header files. - Changed return types from Standard_Real and Standard_Integer to double and int respectively for better consistency and modern C++ practices. - Updated method signatures to use 'the' prefix for parameters, enhancing readability. - Refactored constructors and methods in Extrema_POnSurf and Extrema_POnSurfParams to use double instead of Standard_Real. - Removed unnecessary includes and class declarations to streamline the code. - Updated FILES.cmake to reflect the removal of .lxx files.
…ated implementations and update CMake files
…Curve2d's IsRational method for improved accuracy
ad89501 to
29617a0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.