Changes supporting GPU shaping branch #1748
Conversation
gunney1
requested review from
Arlie-Capps,
BradWhitlock,
kennyweiss and
rhornung67
| * @brief Construct transformer that moves 4 starting points to 4 | ||
| * destination points. | ||
| */ | ||
| AXOM_HOST_DEVICE CoordinateTransformer(const primal::Point<T, 3>* startPts, |
There was a problem hiding this comment.
A little ASCII art diagram could be useful. I assume it's something like this:
*[2]
|
|
*[0]-----*[1]
/
/
*[3]
There was a problem hiding this comment.
Sorry, I'm not sure I understand. The 4 starting and ending points can be anywhere. This constructor simply calls setByTerminus, which explains a bit about the input requirements. I'll add that info to the constructor docs. Let me know if you think it needs more.
There was a problem hiding this comment.
I guess I don't understand what the 4 points represent then. That's what I would want to know.
There was a problem hiding this comment.
@gunney1 , correct me if I'm wrong, but I think you use this ctor to transform an arbitrary tet to a unit tet like the one @BradWhitlock diagrammed. Perhaps you could make ASCII art showing the tet before transformation and use Brad's as the tet after transformation.
There was a problem hiding this comment.
It can transform an arbitrary tet to a unit tet but only if you specify a unit tet as the second 4 points. We can transform any tet to any other tet.
There was a problem hiding this comment.
@BradWhitlock you can think of the 4 points as providing the required information to define the transformation. There are 12 unknowns in the matrix so you need 12 constraints, which are the 4 3D points. I guess if it helps to think of the points as tet vertices, I could write that...
There was a problem hiding this comment.
Maybe something like "If you think of each 4 points as the vertices of a tetrahedron, the transformation moves the starting tet to the destination tet."
Arlie-Capps
left a comment
Arlie-Capps
left a comment
There was a problem hiding this comment.
Thank you for breaking these changes out into a separate PR. I agree with Brad's and Rich's requests for improvements to the documentation, so I'm approving with the understanding that you'll address their concerns.
|
|
||
| /*! | ||
| * \brief Returns the algebraic volume of the cone | ||
| * \brief Returns the algebraic volume of the cone, |
There was a problem hiding this comment.
(I might have seen a similar comment elsewhere)
What does algebraic volume imply vs. signed volume?
I'd suggest signed volume instead of algebraic.
Summary
SphereCoordinateTransformerconstructor to build a transformer using terminus points.klee::Geometryfrom aprimal::Cone