Thresholding of isosurface without creating a new one #31

chinmaychinara91 · 2024-08-09T18:11:25Z

remove the threshold isosurface slider feature from Surface tab
add a pop up UI or maybe a menu item to change isosurface mesh in the figure window itself
remove deleting and regeneration of isosurface file rather just update the surface data in the already present file
add keyboard shortcut (Shift+uparrow/downarrow) for isovalue change; menu option on popup for fine tuning isovalue
verify working with >=R2016b (R2017a used at USC BCI for Polhemus work)

Other related issues:

for SEEG implantation, prevent loading multiple instances of the same figure (isosurface+MRI Viewer)

@tmedani

… figure

…ading it

…pup menu on 3D figure

chinmaychinara91 · 2024-08-09T22:18:21Z

@rcassani I have added something like this: keyboard shortcut (Shift+uparrow/downarrow) for coarse isovalue change; menu option on popup for fine isovalue change:

It is ready for review. Might need some more code and GUI cleanups. But if you can test the basic functionalities and let me know I can go forward with cleaning up.

@tmedani

chinmaychinara91 · 2024-08-12T22:42:43Z

toolbox/gui/figure_3d.m

-        jPanel.add(jButtonSet, BorderLayout.EAST);
+        java_setcb(spinmodel, 'StateChangedCallback', @(h,ev)tess_isosurface(CtFile, jSpinner.getValue()));
+        jPanel.add(jSpinner, BorderLayout.EAST);
+        % jButtonSet = gui_component('button', [], '', 'Set', [], [], @(h,ev)tess_isosurface(CtFile, jSpinner.getValue()));


for commit 1b99a59 also removed the need to another button click

rcassani · 2024-08-21T20:11:13Z

This crashes if there is already a isoSurface in the Subject's anatomy and CT segmentation is started from CT volume with right-click > CT segmentation > Generate threshold mesh from CT

@chinmaychinara91, @tmedani, Did you use the proposed changes to search for the ideal threshold value from a CT where the ideal threshold is not known?

I did, and I found quite cumbersome to search for the desired threshold value. While the changes in this PR remove the slider from the Surface, the proposed input in the context menu does not accept an arbitrary number just pre-defined +-50 steps with the up/down arrows; similar to the key shortcuts, but for some reason there the steps are +-100.

Let's keep the slider in the Surface tab for now.
What do you think?

…figure window

chinmaychinara91 · 2024-08-21T21:11:31Z

This crashes if there is already a isoSurface in the Subject's anatomy and CT segmentation is started from CT volume with right-click > CT segmentation > Generate threshold mesh from CT

Fixed. check dc3ccce

@chinmaychinara91, @tmedani, Did you use the proposed changes to search for the ideal threshold value from a CT where the ideal threshold is not known? I did, and I found quite cumbersome to search for the desired threshold value.

So from the feedback from Johnson on how the isoValue threshold is used when doing manual labelling is there is no one isoValue that we can say is the best. There could be some ideal cases but in reality the isovalue threshold might need adjustment while localizing every contact. He has been using 3D slicer to do the same thing and it does something similar but they have a slider feature for threshold and the mesh generation is faster/almost real time there probably because of C++. Maybe we can adjust the numbers from 50/100 to something better.

the proposed input in the context menu does not accept an arbitrary number just pre-defined +-50 steps with the up/down arrows; similar to the key shortcuts, but for some reason there the steps are +-100.

Yes this is not an editable text field rather it reads the current isoValue from the comment and updates it as the initial value. Discussed this with @tmedani, the up down arrows in the context menu can then be used is for fine tuning by +-50. The shortcut keys (shift+up/down) is for a coarser tuning +-100. Just chose these values arbitrarily but we can come up with a better number. Do you think we should have it as an editable field so that people can enter values into it directly?

Let's keep the slider in the Surface tab for now.

I feel we should test and debug this approach and remove the slider option from Surface tab.

Let me know what you think.

tmedani · 2024-08-21T21:53:44Z

Hey guys @chinmaychinara91 @rcassani
Sorry for not being so active here!

From user perspective (and from what we have previously discussed), it is better to have this shortcut active.
(If its implementation is not hurting any other part of the code)

The CTs do not have fixed threshold, each electrode or even each contact may need a different threshold to be visible. Using the shift+up/down is faster for visual inspection and then contact editing, also this is inline with what 3D slicer uses,
and what some clinicians are expecting: just shortcuts and less-click and menus.

As @chinmaychinara91 proposed, we can increase the steps [+/-100] and keep the menu for fine tuning if required.

I will double check with Chinmay later today about the values.

rcassani · 2024-08-22T15:06:56Z

@chinmaychinara91, @tmedani
If you have tested and think that's a good usability, that's ok for me.
Then let me know when the code is read to be reviewed.

also popup menu isoValue display size updated

chinmaychinara91 · 2024-08-23T23:19:27Z

@rcassani this is ready for review.

rcassani

It works as it should. There is a bug with the wrong selection of CT file, this is one of the items reported in the PR (brainstorm-tools#732) and needs to be solved.

While the code does not delete and create a new isosurface file. If does rewrite the content at every change. Is there a value on doing this?

Personally, I found more useful the slider implementation, it allows to do big jumps in the threshold to do binary search of thresholds for identify contacts.

@chinmaychinara91, Did you take a look to the GUI of the "Edit MRI registration" as I asked you before doing these changes?
https://neuroimage.usc.edu/brainstorm/Tutorials/Yokogawa#Refine_the_MRI_registration

We can do this kind of GUI, to provide tools to manipulate the isosurface to locate electrodes, just to mention some:

threshold slider
presets to thresholds, so once user identifies them
turn on/off the centroid option
the get coordinates cross (since the iEEG can be open without 3D figures)
and others

Also, all the manipulations would not need to save any change in disk, as the relevant information is the electrode positions, not the changes in the isosurface.

(Proof of concept)

@tmedani, any thoughts?

rcassani · 2024-09-06T14:33:59Z

toolbox/gui/figure_3d.m

+                    % update isoValue threshold (Shift + Up/Down)
+                    elseif ismember('shift', keyEvent.Modifier) && ~isempty(iIsoSurf)


Simplify to elseif ismember('shift', keyEvent.Modifier), inside that condition, then check if there is CT isosurface, otherwise do nothing.
This leave the option of other shift actions in the future.

rcassani · 2024-09-06T14:35:37Z

toolbox/gui/figure_3d.m

                    Handles = bst_figures('GetFigureHandles', hFig);
+                    % Check if there is SEEG isosurface displayed on the figure


This will be check in the shift condtion.
See: comment for line 1081

rcassani · 2024-09-06T14:35:42Z

toolbox/gui/figure_3d.m

+    % Get Surface
+    TessInfo = getappdata(hFig, 'Surface');


This will be check in the shift condtion.
See: comment for line 1081

rcassani · 2024-09-06T14:46:55Z

toolbox/gui/figure_3d.m

+                        % get the CT file and structure
+                        SubjectFile = getappdata(hFig, 'SubjectFile');
+                        sSubject = bst_get('Subject', SubjectFile);
+                        iCt = find(cellfun(@(x) ~isempty(regexp(x, '_volct', 'match')), {sSubject.Anatomy.FileName}));
+                        CtFile = sSubject.Anatomy(iCt(1)).FileName;


This is a bug reported in the PR2

Wrong CT volume can be selected thresholding CT with slider
This can happen if there are more than one CT in the Subject. For example is isosurface is create from popmenu on the 2nd CT (in the image below) using the arrows or the popmenu to change the threshold will do it for the 1st CT.

A better way to fix this would be to add the filename of the CT that gave origin to the isosurface in the isosurface structure. Maybe in the history

rcassani · 2024-09-06T14:47:33Z

toolbox/gui/figure_3d.m

+        % get the CT file and its structure
+        SubjectFile = getappdata(hFig, 'SubjectFile');
+        sSubject = bst_get('Subject', SubjectFile);
+        iCt = find(cellfun(@(x)(~isempty(regexp(x, '_volct', 'match'))), {sSubject.Anatomy.FileName}));
+        CtFile = sSubject.Anatomy(iCt(1)).FileName;


Same potential problem on getting the wrong CT

chinmaychinara91 · 2024-09-06T15:55:10Z

@rcassani thank you for the detailed feedback. I will discuss these with @tmedani and get back to you.

chinmaychinara91 · 2024-09-11T22:19:21Z

@chinmaychinara91, Did you take a look to the GUI of the "Edit MRI registration" as I asked you before doing these changes? https://neuroimage.usc.edu/brainstorm/Tutorials/Yokogawa#Refine_the_MRI_registration

We can do this kind of GUI, to provide tools to manipulate the isosurface to locate electrodes, just to mention some:

threshold slider

presets to thresholds, so once user identifies them

turn on/off the centroid option

the get coordinates cross (since the iEEG can be open without 3D figures)

and others

Also, all the manipulations would not need to save any change in disk, as the relevant information is the electrode positions, not the changes in the isosurface.

We agree with your proposal. Let me work on it. This sounds more user friendly as well.

chinmaychinara91 · 2024-11-04T19:43:20Z

https://neuroimage.usc.edu/brainstorm/Tutorials/Yokogawa#Refine_the_MRI_registration

We can do this kind of GUI, to provide tools to manipulate the isosurface to locate electrodes

closing this PR. Will start a separate PR based on the above recommendation

chinmaychinara91 added 6 commits August 8, 2024 15:08

better way to find files

b01c08e

for SEEG implantation, prevent loading multiple instances of the same…

77b048d

… figure

update isosurface patch with new values rather than deleting and relo…

a4e7fb4

…ading it

added comments to commit a4e7fb4

5f0b71d

remove isosurface threshold slider from Surface tab; add option to po…

966fbeb

…pup menu on 3D figure

set the min and max limits of CT thresholding to the popup menu option

1b09efb

chinmaychinara91 marked this pull request as ready for review August 9, 2024 22:18

chinmaychinara91 added 3 commits August 9, 2024 23:56

fix isoValue value display update on the menu pop up

8aaf10a

move the isovalue menu to the top

d9d65bb

keyboard shortcut for larger threshold; menu option for fine tuning

1b99a59

chinmaychinara91 commented Aug 12, 2024

View reviewed changes

chinmaychinara91 added 3 commits August 12, 2024 16:22

refactor code and update comments

3a1a9f7

use regexp instead of strfind

b347f1f

updated history text and mesh fields

ccb301d

bug fix: if isosurface is present in database but not open ; open in …

dc3ccce

…figure window

chinmaychinara91 mentioned this pull request Aug 22, 2024

bugfix: Index Exceeds Array Bounds brainstorm-tools/brainstorm3#734

Closed

chinmaychinara91 added 2 commits August 23, 2024 16:12

coarse tuning +/- 300; fine tuning +/- 50

a5878bf

also popup menu isoValue display size updated

refactor code with comments

1459c25

chinmaychinara91 marked this pull request as draft August 23, 2024 23:18

chinmaychinara91 marked this pull request as ready for review August 23, 2024 23:19

rcassani requested changes Sep 6, 2024

View reviewed changes

chinmaychinara91 closed this Nov 4, 2024

chinmaychinara91 deleted the refactor-updated-ieeg-2 branch November 4, 2024 19:45

chinmaychinara91 restored the refactor-updated-ieeg-2 branch November 4, 2024 20:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Thresholding of isosurface without creating a new one #31

Thresholding of isosurface without creating a new one #31

chinmaychinara91 commented Aug 9, 2024 •

edited

Loading

chinmaychinara91 commented Aug 9, 2024 •

edited

Loading

chinmaychinara91 Aug 12, 2024 •

edited

Loading

rcassani commented Aug 21, 2024

chinmaychinara91 commented Aug 21, 2024 •

edited

Loading

tmedani commented Aug 21, 2024

rcassani commented Aug 22, 2024

chinmaychinara91 commented Aug 23, 2024

rcassani left a comment

rcassani Sep 6, 2024

rcassani Sep 6, 2024

rcassani Sep 6, 2024

rcassani Sep 6, 2024

rcassani Sep 6, 2024

chinmaychinara91 commented Sep 6, 2024

chinmaychinara91 commented Sep 11, 2024 •

edited

Loading

chinmaychinara91 commented Nov 4, 2024

		% update isoValue threshold (Shift + Up/Down)
		elseif ismember('shift', keyEvent.Modifier) && ~isempty(iIsoSurf)

		Handles = bst_figures('GetFigureHandles', hFig);
		% Check if there is SEEG isosurface displayed on the figure

Thresholding of isosurface without creating a new one #31

Thresholding of isosurface without creating a new one #31

Conversation

chinmaychinara91 commented Aug 9, 2024 • edited Loading

chinmaychinara91 commented Aug 9, 2024 • edited Loading

chinmaychinara91 Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

rcassani commented Aug 21, 2024

chinmaychinara91 commented Aug 21, 2024 • edited Loading

tmedani commented Aug 21, 2024

rcassani commented Aug 22, 2024

chinmaychinara91 commented Aug 23, 2024

rcassani left a comment

Choose a reason for hiding this comment

rcassani Sep 6, 2024

Choose a reason for hiding this comment

rcassani Sep 6, 2024

Choose a reason for hiding this comment

rcassani Sep 6, 2024

Choose a reason for hiding this comment

rcassani Sep 6, 2024

Choose a reason for hiding this comment

rcassani Sep 6, 2024

Choose a reason for hiding this comment

chinmaychinara91 commented Sep 6, 2024

chinmaychinara91 commented Sep 11, 2024 • edited Loading

chinmaychinara91 commented Nov 4, 2024

chinmaychinara91 commented Aug 9, 2024 •

edited

Loading

chinmaychinara91 commented Aug 9, 2024 •

edited

Loading

chinmaychinara91 Aug 12, 2024 •

edited

Loading

chinmaychinara91 commented Aug 21, 2024 •

edited

Loading

chinmaychinara91 commented Sep 11, 2024 •

edited

Loading