-
-
Notifications
You must be signed in to change notification settings - Fork 668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add inverse capability to KernelTransform #4529
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -508,6 +508,43 @@ KernelTransform<TParametersValueType, VDimension>::GetFixedParameters() const -> | |
} | ||
|
||
|
||
template <typename TParametersValueType, unsigned int VDimension> | ||
bool | ||
KernelTransform<TParametersValueType, VDimension>::GetInverse(Self * inverseTransform) const | ||
{ | ||
if (!inverseTransform) | ||
{ | ||
return false; | ||
} | ||
|
||
auto deepCopyLandmarks = [](const typename PointSetType::ConstPointer source, | ||
typename PointSetType::Pointer destination) { | ||
typename PointSetType::PointsContainer::ConstIterator sourceIt = source->GetPoints()->Begin(); | ||
|
||
typename PointSetType::PointIdentifier i{ 0 }; | ||
while (sourceIt != source->GetPoints()->End()) | ||
{ | ||
destination->SetPoint(i, sourceIt->Value()); | ||
++i; | ||
++sourceIt; | ||
} | ||
}; | ||
|
||
// make a deep copy of the source and target landmarks | ||
typename PointSetType::Pointer sourceLandmarks = PointSetType::New(); | ||
typename PointSetType::Pointer targetLandmarks = PointSetType::New(); | ||
deepCopyLandmarks(this->GetSourceLandmarks(), sourceLandmarks); | ||
deepCopyLandmarks(this->GetTargetLandmarks(), targetLandmarks); | ||
Comment on lines
+536
to
+537
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please consider directly copying the internal STL containers of the landmarks: sourceLandmarks->GetPoints()->CastToSTLContainer() =
m_SourceLandmarks->GetPoints()->CastToSTLConstContainer();
targetLandmarks->GetPoints()->CastToSTLContainer() =
m_TargetLandmarks->GetPoints()->CastToSTLConstContainer();
|
||
|
||
// inversion comes from reversing the landmarks | ||
inverseTransform->SetSourceLandmarks(targetLandmarks); | ||
inverseTransform->SetTargetLandmarks(sourceLandmarks); | ||
inverseTransform->SetStiffness(this->GetStiffness()); | ||
Comment on lines
+540
to
+542
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's OK for a class to directly access its own data members, without calling Get and Set. So more like: inverseTransform->m_SourceLandmarks = targetLandmarks;
inverseTransform->m_TargetLandmarks = sourceLandmarks;
inverseTransform->m_Stiffness = m_Stiffness; Right? |
||
inverseTransform->ComputeWMatrix(); | ||
|
||
return true; | ||
} | ||
|
||
template <typename TParametersValueType, unsigned int VDimension> | ||
void | ||
KernelTransform<TParametersValueType, VDimension>::PrintSelf(std::ostream & os, Indent indent) const | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider using
auto
here. For the return type ofType::New()
we have agreed thatauto
is appropriate: https://github.com/InsightSoftwareConsortium/ITKSoftwareGuide/blob/d1562dd6dbc929ba18fdf8ce8aa86e8337597d7e/SoftwareGuide/Latex/Appendices/CodingStyleGuide.tex#L1558-L1562There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto
would be even nicer 😇