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: Program crashed when markers were close to the image border. #24

Merged
merged 1 commit into from Nov 21, 2018

Conversation

Projects
None yet
4 participants
@zivy
Contributor

zivy commented Oct 31, 2018

Refactored the logic behind the ROI selection. We now take a fixed
sized ROI centered on the marker point and interesect it with the
image to get the final ROI which is used in the registration. Also
modified ther registration initialization so that it aligns the fixed
and moving markers even if they are not in the center of the ROI
(which is what happens when they are close to the image border).

BUG: Program crashed when markers were close to the image border.
Refactored the logic behind the ROI selection. We now take a fixed
sized ROI centered on the marker point and interesect it with the
image to get the final ROI which is used in the registration. Also
modified ther registration initialization so that it aligns the fixed
and moving markers even if they are not in the center of the ROI
(which is what happens when they are close to the image border).
@blowekamp

This comment has been minimized.

Collaborator

blowekamp commented Oct 31, 2018

I tried this fix and it helps me with the data set I was working on too! Thanks!

@blowekamp

This comment has been minimized.

Collaborator

blowekamp commented Nov 9, 2018

@pieper @jcfr May I move this patch through and update Slicer?

@blowekamp blowekamp self-requested a review Nov 9, 2018

@blowekamp

This is a great improvement to the robustness of the registration.

@jcfr

This comment has been minimized.

Contributor

jcfr commented Nov 20, 2018

@zivy Thanks for the contribution 👍

@pieper When you have a chance, could you review and integrate ?

@pieper pieper merged commit beb4e47 into pieper:master Nov 21, 2018

@pieper

This comment has been minimized.

Owner

pieper commented Nov 21, 2018

Thanks for the fix! Sorry for the slow reply - was traveling last week and now getting ready for RSNA 😄

@pieper

This comment has been minimized.

Owner

pieper commented Nov 21, 2018

I'll update the Slicer superbuild with this commit.

@zivy

This comment has been minimized.

Contributor

zivy commented Nov 21, 2018

@pieper Thanks.

Once it is merged I won't have to worry about deleting/overwriting my current Slicer installation which I fixed for myself (I've lost work in the past where I fixed/patched something locally for myself and later overwrote it with a software update).
Happy thanksgiving, 🦃 .

@jcfr

This comment has been minimized.

Contributor

jcfr commented Nov 21, 2018

@pieper

This comment has been minimized.

Owner

pieper commented Nov 21, 2018

Okay - update committed to Slicer too: http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=27566

Alas we still have this frustrating issue moving the fiducials... https://issues.slicer.org/view.php?id=4634

slicerbot pushed a commit to Slicer/Slicer that referenced this pull request Nov 21, 2018

BUG: fix LandmarkRegistration SimpleITK
Fixes issues performing landmark refinement
near image borders.

pieper/LandmarkRegistration#24

Thanks Ziv Yaniv!

From: Steve Pieper <pieper@isomics.com>

git-svn-id: http://svn.slicer.org/Slicer4/trunk@27566 3bd1e089-480b-0410-8dfb-8563597acbee
@zivy

This comment has been minimized.

Contributor

zivy commented Nov 21, 2018

I did experience that behavior (problem "grabbing" fiducials) but it appears that if I : (a) lock the window/level for the volumes (b) zoom in on the fiducials I can grab them.
I wasn't sure if this is a feature or bug so didn't complain about it.

@pieper

This comment has been minimized.

Owner

pieper commented Nov 21, 2018

Interesting clue - thanks! I don't think locking the window level is required (but I can see why it helps avoid unintended consequences).

What works for me is not zooming, but panning so that the fiducial is not in the exact center of the viewport. This kind of makes sense if the picking bug is due to invisible widgets that happen to be at the origin. This isn't a solution, but it seems like a reliable workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment