Coding - Optimize gp_Vec, gp_Vec2d, gp_XY, and gp_XYZ classes#578
Merged
dpasukhi merged 6 commits intoOpen-Cascade-SAS:IRfrom Jun 26, 2025
Merged
Coding - Optimize gp_Vec, gp_Vec2d, gp_XY, and gp_XYZ classes#578dpasukhi merged 6 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi merged 6 commits intoOpen-Cascade-SAS:IRfrom
Conversation
Member
dpasukhi
commented
Jun 20, 2025
- Renamed parameters in IsEqual, Mirror, and Mirrored methods for consistency.
- Simplified calculations in IsEqual methods by using absolute differences.
- Enhanced Mirror methods to precompute reflection matrix terms for better performance.
- Updated Transform methods to use consistent naming for transformation parameters.
- Replaced sqrt with std::hypot in Modulus methods for better numerical stability.
- Streamlined Cross and Multiply methods in gp_XYZ and gp_XY classes for clarity.
- Improved comments and documentation for better understanding of the code.
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes several geometric classes (gp_Vec, gp_Vec2d, gp_XY, gp_XYZ, and gp_Mat) by renaming parameters for consistency, simplifying calculations (such as using absolute differences for IsEqual methods), precomputing matrix terms in mirror operations, streamlining multiplication and cross product methods, and updating transformation and exponentiation algorithms for better performance and maintainability.
- Renamed parameters and improved consistency across methods.
- Simplified mathematical computations and replaced indirect API calls with direct data member access where performance‐critical.
- Updated and optimized matrix operations including inversion, transposition, and power calculations.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/FoundationClasses/TKMath/gp/gp_XYZ.hxx | Inlined IsEqual method, added const qualifiers, and switched to direct matrix data access for improved performance. |
| src/FoundationClasses/TKMath/gp/gp_XYZ.cxx | Removed the redundant out-of-line IsEqual implementation in favor of the inline version. |
| src/FoundationClasses/TKMath/gp/gp_XY.hxx & .cxx | gp_XY.cxx was removed and header functions were updated with minor improvements. |
| src/FoundationClasses/TKMath/gp/gp_Vec2d.{hxx,cxx} | Parameter renaming and enhanced trigonometric handling in angle computations. |
| src/FoundationClasses/TKMath/gp/gp_Mat.{hxx,cxx} | Optimized matrix operations using cached variables and binary exponentiation for power computations. |
Comments suppressed due to low confidence (5)
src/FoundationClasses/TKMath/gp/gp_XYZ.cxx:15
- The out-of-line implementation of IsEqual has been removed; confirm that the inline version in the header fully satisfies all usage cases to avoid any unintended behavior.
#include <gp_XYZ.hxx>
src/FoundationClasses/TKMath/gp/gp_XYZ.hxx:304
- The function now uses direct access (myMat) for matrix multiplication to improve performance; please review that the direct access maintains necessary invariants and encapsulation.
// Direct access to matrix data for optimal performance (gp_XYZ is friend of gp_Mat)
src/FoundationClasses/TKMath/gp/gp_Mat.cxx:317
- The revised Power() function now uses fast binary exponentiation; verify that all matrix multiplication associativity rules are observed and that the behavior remains consistent for both positive and negative exponents.
while (power > 0)
src/FoundationClasses/TKMath/gp/gp_Mat.hxx:542
- Replacing multiple manual assignments with std::swap in the Transpose() function improves code clarity; ensure that this change does not affect any side‐effects expected by downstream code.
std::swap(myMat[0][1], myMat[1][0]);
- Renamed parameters in IsEqual, Mirror, and Mirrored methods for consistency. - Simplified calculations in IsEqual methods by using absolute differences. - Enhanced Mirror methods to precompute reflection matrix terms for better performance. - Updated Transform methods to use consistent naming for transformation parameters. - Replaced sqrt with std::hypot in Modulus methods for better numerical stability. - Streamlined Cross and Multiply methods in gp_XYZ and gp_XY classes for clarity. - Improved comments and documentation for better understanding of the code.
…and maintainability
…ed performance and clarity
…ations and simplify logic for improved clarity and performance
AtheneNoctuaPt
approved these changes
Jun 25, 2025
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.