Foundation Classes - Enhance BVH Implementation - #842
Merged
dpasukhi merged 10 commits intoNov 26, 2025
Conversation
dpasukhi
commented
Nov 20, 2025
Member
- Updated BVH_BinnedBuilder to improve leaf node size condition and SAH cost evaluation.
- Refactored BVH_Box to utilize constexpr for better compile-time evaluation.
- Enhanced BVH_LinearBuilder to replace std::vector with NCollection_Vector for performance.
- Introduced new unit tests for BVH_BinnedBuilder, BVH_Box, BVH_LinearBuilder, and other components to ensure robustness and correctness.
- Added various utility functions and constants to improve code clarity and maintainability.
- Updated CMake files to include new test files for comprehensive testing coverage.
- Updated BVH_BinnedBuilder to improve leaf node size condition and SAH cost evaluation. - Refactored BVH_Box to utilize constexpr for better compile-time evaluation. - Enhanced BVH_LinearBuilder to replace std::vector with NCollection_Vector for performance. - Introduced new unit tests for BVH_BinnedBuilder, BVH_Box, BVH_LinearBuilder, and other components to ensure robustness and correctness. - Added various utility functions and constants to improve code clarity and maintainability. - Updated CMake files to include new test files for comprehensive testing coverage.
- Updated BVH_Box to utilize constexpr and noexcept for improved compile-time evaluation and safety. - Refactored BVH_BuildQueue to use atomic operations for thread-safe size tracking and improved performance. - Removed the obsolete BVH_BuildQueue.cxx file and updated related CMake configurations. - Added comprehensive unit tests for BVH_Box and BVH_BuildQueue to ensure functionality and robustness. - Enhanced various BVH traversal and building classes with new internal structures for better performance and clarity. - Updated destructor implementations to use default specifiers for consistency and clarity.
- Adjusted variable initialization for improved clarity and consistency in the Select3D_SensitivePrimitiveArray class. - Minor formatting changes to enhance code readability.
- Added a default constructor to BVH_Ray for creating an invalid ray at the origin. - Updated BVH_Ray constructor to include noexcept and documentation for clarity. - Refactored BVH_Tools' RayBoxIntersection methods to improve readability and performance, including early exit optimizations. - Introduced comprehensive unit tests for BVH_Ray and enhanced tests for BVH_Tools to ensure robustness and correctness across various scenarios.
âĶLogic - Simplified Box-Box and Point-Box square distance calculations by replacing repetitive code with loop-based implementations for better maintainability. - Introduced new internal helper methods for setting projection states and computing projections onto edges, enhancing clarity and reducing code duplication. - Updated Point-Triangle projection logic to utilize Voronoi region testing for improved accuracy in determining the nearest point on a triangle.
âĶ Semantics - Updated memory order semantics in BVH_BuildQueue to use acquire/release for size tracking and thread synchronization, ensuring proper visibility across threads. - Refactored EncodeMortonCode in BVH_RadixSorter to use constexpr for better compile-time evaluation and optimization.
AtheneNoctuaPt
requested changes
Nov 24, 2025
Comment on lines
+264
to
+272
| const Standard_Integer aVoxelX = | ||
| (std::max)(0, (std::min)(BVH::IntFloor(BVH::VecComp<T, N>::Get(aVoxelF, 0)), aDimension - 1)); | ||
| const Standard_Integer aVoxelY = | ||
| (std::max)(0, (std::min)(BVH::IntFloor(BVH::VecComp<T, N>::Get(aVoxelF, 1)), aDimension - 1)); | ||
| const Standard_Integer aVoxelZ = | ||
| (aNbEffComp > 2) | ||
| ? (std::max)(0, | ||
| (std::min)(BVH::IntFloor(BVH::VecComp<T, N>::Get(aVoxelF, 2)), aDimension - 1)) | ||
| : 0; |
Collaborator
There was a problem hiding this comment.
Can use std::clamp instead if combination of std::min and std::max
AtheneNoctuaPt
approved these changes
Nov 25, 2025
There was a problem hiding this comment.
Pull request overview
This PR enhances the BVH (Bounding Volume Hierarchy) implementation in the Foundation Classes module by improving code quality, correctness, and test coverage. The changes focus on refining builder algorithms, adding constexpr support for compile-time evaluation, and introducing comprehensive unit tests.
Key Changes:
- Fixed leaf node size condition and SAH cost evaluation in
BVH_SweepPlaneBuilder - Added
constexprtoBVH_Box,BVH_Types, and helper functions for compile-time evaluation - Introduced 13 new comprehensive test files covering BVH components
- Updated CMake configuration to include new test files
- Removed unused
BVH_BuildQueue.cxxfile - Added internal helper structures to
BVH_Traverseclasses
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Select3D_SensitivePrimitiveArray.cxx | Removed unused variable aBox |
| FILES.cmake (GTests) | Added 13 new BVH test files |
| BVH_Triangulation_Test.cxx | New tests for triangulation operations |
| BVH_Tree_Test.cxx | New tests for BVH tree operations |
| BVH_Traverse_Test.cxx | New tests for tree traversal |
| BVH_Tools_Test.cxx | New tests for BVH utility functions |
| BVH_SweepPlaneBuilder_Test.cxx | New tests for sweep plane builder |
| BVH_SpatialMedianBuilder_Test.cxx | New tests for spatial median builder |
| BVH_Ray_Test.cxx | New tests for ray operations |
| BVH_RadixSorter_Test.cxx | New tests for radix sorting |
| BVH_QuickSorter_Test.cxx | New tests for quick sorting |
| BVH_LinearBuilder_Test.cxx | New tests for linear builder |
| BVH_BuildQueue_Test.cxx | New tests for build queue |
| BVH_Box_Test.cxx | New tests for bounding box operations |
| BVH_BinnedBuilder_Test.cxx | New tests for binned builder |
| FILES.cmake (BVH) | Removed BVH_BuildQueue.cxx from build |
| BVH_Types.hxx | Added constexpr to VecComp and IntFloor |
| BVH_Traverse.hxx | Added internal helper structures |
| BVH_SweepPlaneBuilder.hxx | Fixed leaf condition and array bounds |
| BVH_SpatialMedianBuilder.hxx | Changed destructor to default |
| BVH_QueueBuilder.hxx | Changed destructor to default |
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.