Enhance the LF viewer and adding more options from the keyboard #343
Conversation
toolbox/gui/view_leadfields.m
Outdated
| iChannel = 1; | ||
| iQuiverSize = 1; |
There was a problem hiding this comment.
The convention in the rest of the code is "iVariableName" being an index.
https://neuroimage.usc.edu/brainstorm/Tutorials/Scripting#Naming_conventions
These are not indices.
Can you please rename the variables?
| '<TR><TD><B>Down arrow</B></TD><TD>Next ref channel (green color)</TD></TR>'.... | ||
| '<TR><TD><B>Shift + uparrow</B></TD><TD>Increase the vector length</TD></TR>'... | ||
| '<TR><TD><B>Shift + downarrow</B></TD><TD>Decrease the vector length</TD></TR>'... | ||
| '<TR><TD><B>Shift + L</B></TD><TD>Toggle on/off logarithmic scale</TD></TR>'... |
There was a problem hiding this comment.
Looks like this is "Shift + Enter" instead
There was a problem hiding this comment.
I think both are working,
"Shift + Enter" and "Shift + L"
toolbox/gui/view_leadfields.m
Outdated
| '</TABLE>'], 'Keyboard shortcuts'); | ||
|
|
||
| % === NUMBERS : VIEW SHORTCUTS === | ||
| case '0' |
There was a problem hiding this comment.
Delegate all this to figure_3d instead.
In the otherwise block, it should call figure_3d/FigureKeyPressedCallback and set the orientation.
Or am I missing something?
There was a problem hiding this comment.
You're right,
I did not manage to link the current function with the figure_3d
There was a problem hiding this comment.
The block otherwise in the switch calls KeyPressFcn_bak(hQuiver, keyEvent), which actually calls the original handler from figure_3d (figure_3d/FigureKeyPressedCallback). You don't have to do add anything so that figure_3d processes the orientation keys.
If you have any trouble, add a breakpoint at the beginning of figure_3d/FigureKeyPressedCallback to check what is wrong.
In general, if you need to call a function in a different file, use the syntax figure_3d('FunctionName', param1, param2...)
There was a problem hiding this comment.
Ok, I got it, the problem was in the figure handles,
Now it's working.
Thanks :)
toolbox/gui/view_leadfields.m
Outdated
| function GetLeadField | ||
| % Update the LF according to the selected channels only | ||
| for iLF = 1:length(HeadmodelFiles) | ||
| LF_finale{iLF} = HeadmodelMat{iLF}.Gain(iChannels,:); | ||
| end | ||
| end | ||
|
|
||
| %% ===== LEADFIELD TO LOG SPACE ===== | ||
| function lf_log = logScaledLeadField(lf) |
There was a problem hiding this comment.
As this is a function, it would be better if it were named as an action.
For example: ScaleLeadfield
But since this is called only once and not very long, I suggest you integrate it directly into where it is required.
(and not keep it as a separate function)
There was a problem hiding this comment.
Hi, This is just an open comment. Please don't modify your intentions.
- the integration of the scaling function in the main flow. I personally wouldn't integrate it. Keeping simple, atomic, operations in a codebase, abstracted of everything else, as much as possible, I think makes the code easier to follow, debug and, eventually, reuse code, etc... This might be a bad example because of its simplicity, but still.
- Totally agree with the renaming. Would suggest LogScaleLeadfield, but ScaleLeadfield is just fine.
Thanks for reading.
There was a problem hiding this comment.
Hi, I have corrected the PR according to the discussion, and regarding this comment I changed only the name of the sub-function "ScaleLeadfield", I think also it's more convenient to have as a subfunction, but this is also my comment.
so, @ftadel if you don't like it I will change it :)
toolbox/gui/view_leadfields.m
Outdated
|
|
||
| %= = From this line, these functions are part of the figure3d | ||
| %% ===== SET STANDARD VIEW ===== | ||
| function SetStandardView(hFig, viewNames) |
There was a problem hiding this comment.
I don't understand why you copy-pasted a function from figure_3d here.
If you need to call a function in a different file, use the syntax figure_3d('FunctionName', param1, param2...)
There was a problem hiding this comment.
I did not manage to use figure_3d in this function, but if you think it's possible I will try to do it.
…to display channels names
Hi @ftadel,
This PR is to complete the PR that @juangpc proposed (#337 (comment)), and it includes also the proposed option.
It includes useful options to visualize and manipulate the LF vectors, all these options are managed from keyboard shortcuts, they can be displayed with the 'H' keyboard key, as in this figure :
With this version, the average reference option can be also selected from the same list as the channels (R) instead to have multiple popup panels, the "target channels (T) can be also selected from a dedicated list, which is useful in the case where users want to see the LF between two specific channels.
There are also functions that change the scale, the length, and the width of the vectors. Furthermore, there is a way to threshold the displayed vectors by fixing the Amplitude threshold as shown in the figure. All this interactively with the figure & the keyboard (as usual with Branstom figure 👍 ).
For the standard view keyboard (1-9), I have used the same subfunctions "SetStandardView" that I have copy-pasted to this function.
All these options are for visualization purposes, the next dev should focus on functions that can
quantitatively compare these differents LF models.
Also, the idea behind these new options is not only for the LF, but also to extend the use for the optical flow (#311), as previously discussed with @LeoNouvelle @martcous & @sbaillet
I hope this will be useful :)
A+
Takfarinas