Use 2-pass candidate search for mesh clipping #1756
Merged
+563
−113
Conversation
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
…te search. The 2-pass search uses more detailed screening than the single-pass bounding-box collision criterion, so it screens more accurately. Some repeated operations like checking overlaps and calling the primitive clipping code have been factored out to make the main loop more compact.
gunney1
requested review from
Arlie-Capps,
BradWhitlock,
bmhan12,
jcs15c,
kennyweiss,
publixsubfan and
rhornung67
gunney1
added
Quest
gunney1
changed the base branch from
develop
to
feature/gunney/reduce-device-copies
Arlie-Capps
reviewed
Jan 5, 2026
Arlie-Capps
reviewed
Jan 5, 2026
kennyweiss
approved these changes
Jan 6, 2026
Comment on lines
+1059
to
+1070
| TetrahedronType tets[] = {TetrahedronType(oct[0], oct[3], oct[1], oct[2]), | ||
| TetrahedronType(oct[0], oct[3], oct[2], oct[4]), | ||
| TetrahedronType(oct[0], oct[3], oct[4], oct[5]), | ||
| TetrahedronType(oct[0], oct[3], oct[5], oct[1])}; | ||
| double octVol = 0.0; | ||
| for(int i = 0; i < 4; ++i) | ||
| { | ||
| double tetVol = tets[i].signedVolume(); | ||
| SLIC_ASSERT(tetVol >= -EPS); // Tet may be degenerate but not inverted. | ||
| octVol += axom::utilities::abs(tetVol); | ||
| } | ||
| return octVol; |
Member
There was a problem hiding this comment.
Should this be implemented within the Octahedron class as Octahedron::signedVolume() ?
Contributor
Author
There was a problem hiding this comment.
I'm not sure. The octs from discretizing spheres and cones should convex, but general octs may be concave. I don't know if that matters but it seems like something to think about.
| { | ||
| using ATOMIC_POL = typename axom::execution_space<ExecSpace>::atomic_policy; | ||
| constexpr bool tryFixOrientation = false; | ||
| if(screenLevel >= 3) |
Member
There was a problem hiding this comment.
Would it make sense to define a ScreenLevel enum w/ meaningful names for the different levels?
Contributor
Author
There was a problem hiding this comment.
I don't think so. It's just a way for me to do some experimentation to understand the effects of various things I was trying and to compare to similar codes. Once that is done, I believe that screening will prove to be low cost and usually helpful and I plan to remove the parameter.
Arlie-Capps
approved these changes
Jan 6, 2026
An error occurred while trying to automatically change base from
feature/gunney/reduce-device-copies
to
feature/gunney/mesh-clipper-statistics
January 7, 2026 14:14
An error occurred while trying to automatically change base from
feature/gunney/reduce-device-copies
to
feature/gunney/mesh-clipper-statistics
January 7, 2026 14:14
gunney1
changed the base branch from
feature/gunney/reduce-device-copies
to
develop
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.
Summary
MeshClipperImpl::computeClipVolumes3DTets.The 2-pass BVH candidate search is more selective than the single-pass bounding box search, because we can use a more accurate screening at the leaves. The 2-pass is implemented only for tet-screened clips in this PR. The non-screened version and the hex-screened version will be updated in a later PR. (They are less likely to be used when performance is critical.)
The re-factoring is a partial fix for issue #1704.