SOR implementation for MeshClipper
#1762
Conversation
This is needed to support the SorClipper.
gunney1
requested review from
Arlie-Capps,
BradWhitlock,
bmhan12,
jcs15c,
kennyweiss,
publixsubfan and
rhornung67
|
I'm not sure that |
gunney1
changed the title
MeshClipperMeshClipper
…ture/gunney/sor-clipper
What about |
Renamed to |
| * tets points to at least NUM_TETS_PER_HEX objects. This method | ||
| * neither checks the pointer nor reallocates the space. | ||
| */ | ||
| AXOM_HOST_DEVICE inline static void hexToTets(const HexahedronType& hex, TetrahedronType* tets); |
There was a problem hiding this comment.
Once all the PRs are updated, would it make sense to revise the API to take axom::ArrayViews instead of raw pointers? They can easily wrap a raw pointer, and carry their sizes, and allocators with them.
There was a problem hiding this comment.
Because this method is used in long inner loops I'd be concerned about constructing and destructing an ArrayView every cycle. That cost can be optimized away in theory, but does it always happen? My reference precedence for this is the Hexahedron::triangulate method, in which the Tetrahedron container is a TetIndexable template parameter. It doesn't check the container size.
Moreover, verifying the ArrayView size here is no more than verifying that we claimed the correct size when constructing the ArrayView. I'm not sure if that's very meaningful.
hexToTets is a private method. If there isn't enough space for the output, it's not a user error. It'd be an error in the short computeCellsAsTetsImpl method.
| #include "axom/quest/detail/clipping/HexClipper.hpp" | ||
| #include "axom/quest/detail/clipping/Plane3DClipper.hpp" | ||
| #include "axom/quest/detail/clipping/FSorClipper.hpp" | ||
| #include "axom/quest/detail/clipping/SorClipper.hpp" | ||
| #include "axom/quest/detail/clipping/SphereClipper.hpp" | ||
| #include "axom/quest/detail/clipping/TetClipper.hpp" |
There was a problem hiding this comment.
If we're including quest.hpp, we should not need to include any specific quest headers
There was a problem hiding this comment.
Should all files be included in quest.hpp? I assumed that some files are omitted, because I don't see any file that I placed in the detail subdirectory.
| between the vertices. Given the angle, scale the bottom of bbInRz | ||
| for the worst case. | ||
| */ | ||
| double angleRange = maxAngle - minAngle; |
There was a problem hiding this comment.
Is there an assumption that the range won't include the x-axis? E.g. from -30 degrees to 30 degrees.
There was a problem hiding this comment.
No. The x-axis (that is, the axis of rotation) is included when angleRange >= M_PI. We account for this when we use angleRange to compute factor.
Arlie-Capps
left a comment
Arlie-Capps
left a comment
There was a problem hiding this comment.
I suggested some documentation changes. Will approve after those are done.
the MonotonicZSORClipper that it uses internally.
Summary
MonotonicZSORClipper, for clipping a surface-of-revolution where the 2D curve doesn't double back in the axial direction. For such a curve, the area under the curve is inside the geometry; the area above the curve is outside.SORClipper, for clipping a general SOR where the curve can double back in the axial direction. This works by splitting up the curve where it changes directions and useMotonicZSORClipperon each section.MotonicZSORClipperandSORClipper.MeshClippercode.