Skip to content

AffineLogStackTransform: uninitialized local variable 'rdorigin' used #162

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.

Sign up for GitHub

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

Jump to bottom
Closed
N-Dekker opened this issue Jul 12, 2019 · 2 comments
Closed

AffineLogStackTransform: uninitialized local variable 'rdorigin' used #162

N-Dekker opened this issue Jul 12, 2019 · 2 comments

Comments

@N-Dekker
Copy link
Member

AffineLogStackTransform::ReadCenterOfRotationIndex(ReducedDimensionInputPointType&) has a suspicious compilation warning from VS2017:

elastix\components\transforms\affinelogstacktransform\elxaffinelogstacktransform.hxx(574): warning C4700: uninitialized local variable 'rdorigin' used

dummyImage->SetOrigin( rdorigin );

elastix/Components/Transforms/AffineLogStackTransform/elxAffineLogStackTransform.hxx

Line 574 in 32a1b34

dummyImage->SetOrigin( rdorigin );

How shall we fix it? Is this function still in use?
@N-Dekker
Copy link
Member Author

I think the dummyImage->SetOrigin( rdorigin ) call can just be removed. But I need to figure out how to test it!

N-Dekker added a commit that referenced this issue Sep 24, 2019
@N-Dekker
WIP: Add error messages + abort, to see if code is covered by tests
24d3041 
Is the suspicious `dummyImage->SetOrigin( rdorigin )` call on line 574 of Components\Transforms\AffineLogStackTransform\elxAffineLogStackTransform.hxx being tested?

See issue #162 ("AffineLogStackTransform: uninitialized local variable 'rdorigin' used")
@N-Dekker N-Dekker mentioned this issue Sep 24, 2019
Merged
N-Dekker added a commit that referenced this issue Sep 30, 2019
@N-Dekker
WIP: Remove AffineLogStackTransform<TElastix>::ReadCenterOfRotationIndex
6ba04e4 
`AffineLogStackTransform::ReadCenterOfRotationIndex` appears to have a serious bug, as noted by issue #162 ("AffineLogStackTransform: uninitialized local variable 'rdorigin' used")

As the function is only intended to support an obsolete elastix transform parameter file, it was decided to just remove the function, rather than fixing its implementation, at a sprint with Marius @mstaring and Stefan @stefanklein.
N-Dekker added a commit that referenced this issue Oct 1, 2019
@N-Dekker
BUG: No longer read rotation center as index (elastix transform < 3.402)
47c470f 
No longer read "CenterOfRotation" from an old transform parameter file, which is the rotation center of the transformation, specified by its index coordinates.

elastix version >= 3.402 no longer specifies the rotation center by its index coordinates.

As explained by Stefan @stefanklein

"Having a center of rotation in index coordinates in the TransformParameter file is bad practice, since its conversion to a world coordinate depends on the knowledge of the size/spacing/origin/directioncosines of the fixed image that was used for registration. That information is stored in the TransformParameter file as well, but it is easy to forget changing the CenterOfRotationIndex when adjusting the (fixed image) resample domain."

Also discussed with Marius @mstaring

Fixes issue #162 ("AffineLogStackTransform: uninitialized local variable 'rdorigin' used")
@mstaring
Copy link
Member

mstaring commented Oct 2, 2019

fixed by the above commits

@mstaring mstaring closed this as completed Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mstaring @N-Dekker