COMP: fix incorrect casts from scalar or vector to points #479
Conversation
Compilation errors were purposely introduced by InsightSoftwareConsortium/ITK#3237
|
FYI @N-Dekker! |
| @@ -119,7 +119,7 @@ OraGeometryReader::GenerateData() | |||
| MetaDataDoubleType * zvecMeta = | |||
| dynamic_cast<MetaDataDoubleType *>(dic["zdistancebaseunitcs2imagingcs_cm"].GetPointer()); | |||
| double zvec = zvecMeta->GetMetaDataObjectValue(); | |||
| tiltTransform->SetCenter(itk::MakeVector(0., -10. * yvec, -10. * zvec)); | |||
| tiltTransform->SetCenter(itk::MakePoint(0., -10. * yvec, -10. * zvec)); | |||
There was a problem hiding this comment.
Thank you @SimonRit I hope you agree that InsightSoftwareConsortium/ITK#3237, declaring those Point(v) constructors explicit does really help to make ITK user code better. 😃 In this particular case, calling itk::MakePoint certainly looks more appropriate than calling itk::MakeVector 👍
There was a problem hiding this comment.
Honestly, it's not obvious to me why an itk::Point is not an itk::Vector. I have often fought against itk when using vectors, matrices and points and I'm pretty sure that there are many more places where RTK code could be improved... As far as I know, this is not well documented, but I might have missed a good explanation.
There was a problem hiding this comment.
@SimonRit Well, I guess that itk::Point and itk::Vector are just like in https://www.scratchapixel.com/lessons/mathematics-physics-for-computer-graphics/geometry/points-vectors-and-normals (but then in an N-dimensional space):
Points and Vectors
...
Here, a point is a position in a three-dimensional space. A vector, on the other hand, usually means a direction (and some corresponding magnitude, or size) in three-dimensional space.
Right?
There was a problem hiding this comment.
Yes but from the same source Three-dimensional points and vectors are of course similar in that they are both represented by the aforementioned tuple notation. Which is why I would not have used two different types.
There was a problem hiding this comment.
Points and vectors are similar, but they are not the same. At least, I find it helpful to distinguish between them, by using different types for them. Even though I did not invent itk::Point and itk::Vector myself 😃
| @@ -82,8 +82,7 @@ main(int argc, char * argv[]) | |||
| spacing[1] = 2.; | |||
| spacing[2] = 1.; | |||
| reader->SetSpacing(spacing); | |||
| ReaderType::OutputImagePointType origin(0.); | |||
There was a problem hiding this comment.
@SimonRit This particular line of code still compiles with ITK_LEGACY_REMOVE=ON, right? The intention of InsightSoftwareConsortium/ITK#3237 was to avoid implicit conversion from a single value to a Point (as well as from a Vector to a Point). It should still allow an initialization of the form PointType origin(0.).
Of course, if the line of code isn't useful anymore, it's fine to me to have it removed anyway. 👍
There was a problem hiding this comment.
Yes, line below was failing but not this one.
| @@ -82,8 +82,7 @@ main(int argc, char * argv[]) | |||
| spacing[1] = 2.; | |||
| spacing[2] = 1.; | |||
| reader->SetSpacing(spacing); | |||
| ReaderType::OutputImagePointType origin(0.); | |||
| reader->SetOrigin(0.); | |||
| reader->SetOrigin(itk::MakePoint(0., 0., 0.)); | |||
There was a problem hiding this comment.
For the record, the following should also work fine, assuming the right PointType definition (double 3D, I guess):
reader->SetOrigin(PointType());
The coordinates of a point are properly zero-initialized by the expression PointType().
But of course, reader->SetOrigin(itk::MakePoint(0., 0., 0.)) is also OK! 😃
Compilation errors were purposely introduced by
InsightSoftwareConsortium/ITK#3237