Skip to content
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

BUG: Similarity3DTransform::SetScale should recompute m_Offset #2627

Conversation

N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Jul 2, 2021

Closes #2629 "Similarity3DTransform::SetScale should recompute m_Offset", which said:

When Similarity3DTransform::SetScale is called, it does not compute
m_Offset, which appears to cause an incorrect result from
similarity3DTransform.TransformPoint when TransformPoint is called
directly after SetScale.

Included a unit test (GoogleTest) to ensure that the issue is fixed.

@github-actions github-actions bot added area:Core Issues affecting the Core module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Jul 2, 2021
@dzenanz
Copy link
Member

dzenanz commented Jul 5, 2021

I guess you got the formula wrong.
https://open.cdash.org/test/436797765

Addressed issue InsightSoftwareConsortium#2629
"Similarity3DTransform::SetScale should recompute m_Offset", which said:

> When `Similarity3DTransform::SetScale` is called, it does not compute
> `m_Offset`, which appears to cause an incorrect result from
> `similarity3DTransform.TransformPoint` when `TransformPoint` is called
> directly after `SetScale`.

Included a unit test (GoogleTest) to ensure that the issue is fixed.
@N-Dekker N-Dekker force-pushed the SimilarityTransform.SetCenterAndScale branch from 7944f73 to 0dcd550 Compare July 8, 2021 10:44
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jul 8, 2021
@N-Dekker N-Dekker marked this pull request as ready for review July 8, 2021 12:24
@N-Dekker N-Dekker changed the title WIP: Add SimilarityTransform.SetCenterAndScale GoogleTest BUG: Similarity3DTransform::SetScale should recompute m_Offset Jul 8, 2021
@N-Dekker N-Dekker linked an issue Jul 8, 2021 that may be closed by this pull request
@dzenanz dzenanz merged commit 53bb498 into InsightSoftwareConsortium:master Jul 8, 2021
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jul 12, 2021
@thewtex
Copy link
Member

thewtex commented Aug 11, 2021

Added to release for ITK v5.2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Similarity3DTransform::SetScale should recompute m_Offset
3 participants