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

FIX: fiducials locking in coreg gui #9913

Merged
merged 4 commits into from
Nov 2, 2021

Conversation

GuillaumeFavelier
Copy link
Contributor

This PR fixes the issue with CoregistrationUI reported in #8833 about the fiducial markers being unlocked when dig info is provided.

It's an item of #8833

cc @larsoner

@GuillaumeFavelier
Copy link
Contributor Author

Does it fix your issue @larsoner ?

@larsoner
Copy link
Member

It's at least progress in the right direction in that it's not in a mixed state. Now it starts in a "coregistering" state.

The way the old version worked was different (and better IMO). If there was no -fiducials.fif, it would start in a "setting fiducials" state, and there was a "save" button. In other words it looks like this on first load:

MNE_3D_BACKEND=mayavi mne coreg -s sample -d ./subjects --fif MEG/sample/sample_audvis_raw.fif --trans MEG/sample/sample_audvis_raw-trans.fif

Screenshot from 2021-10-28 11-37-47

Moreover, if I'm happy with the MNI-based fiducials -- or if I do some click-based adjustments -- I can then just click the "save" button, and it saves it to the correct path, and locks the fids:

Screenshot from 2021-10-28 11-38-47

Then on a close-reopen, because SUBJECT/bem/SUBJECT-fiducials.fif now exists, it will just load directly into this "coregistering" state.

This is useful because the MNI-based fiducials are only a suggestion / starting point. So when -fiducials.fif is missing, pushing people into a "setting fiducials" state (that they can quickly disable permanently by saving or just for this session by locking) gets them to think about/set the fiducials properly, which can really help with coreg, especially in matched mode.

Do you want to try implementing that workflow here? If not, we can add it to the TODO. To me it would be the next highest priority for 0.24 but not a blocker for release.

@larsoner larsoner added this to the 0.24 milestone Nov 2, 2021
@larsoner larsoner merged commit 3eca0ad into mne-tools:main Nov 2, 2021
@larsoner
Copy link
Member

larsoner commented Nov 2, 2021

Thanks @GuillaumeFavelier !

@GuillaumeFavelier GuillaumeFavelier deleted the fix/coreg_lock_fids branch November 15, 2021 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants