Here is the implementation of vtkMRMLAbstractViewNode::Reset(...) function:
void vtkMRMLAbstractViewNode::Reset(vtkMRMLNode* defaultNode)
{
// The LayoutName is preserved by vtkMRMLNode::Reset, however the layout
// label (typically associated with the layoutName) is not preserved
// automatically.
// This require a custom behavior implemented here.
std::string layoutLabel = this->GetLayoutLabel() ? this->GetLayoutLabel() : "";
int viewGroup = this->GetViewGroup();
this->Superclass::Reset(defaultNode);
this->DisableModifiedEventOn();
this->SetLayoutLabel(layoutLabel.c_str());
this->SetViewGroup(viewGroup);
this->AxisLabels->Reset();
for (int i=0; i<vtkMRMLAbstractViewNode::AxisLabelsCount; i++)
{
this->AxisLabels->InsertNextValue(DEFAULT_AXIS_LABELS[i]);
}
this->DisableModifiedEventOff();
}
Why this->AxisLabels are set to some default medical values instead of copying them from the defaultNode node?
For custom application that works with axis labels XYZ for example this function brings a lot of problems as after calling Reset(...) axes labels will be changed to medical values APSI etc no matter what labels the defaultNode have.
Probably it is better to cast the defaultNode to vtkMRMLAbstractViewNode and if casting returns non NULL pointer then use GetAxisLabel() to set axes labels and if not then use the default medical values?
For example for now ViewControllers module uses the Reset() function and it thus I can’t use it in my custom app without some tricks that I do.
I don’t see why this additional resetting of the axis labels would be needed (node reset and copy from the default node should take care of this). Would removing AxisLabels Reset and InsertNextValue calls fix the behavior?
Yes that would be enough. I just tested that while relying on ViewControllers module and by enabling/disabling this module (i.e. enabling disabling Reset(defaultNode)) I can say that if you change the code as you proposed (by removing everything connected with AxisLabels var) axes labels will be copied from the default node.
Related question:
There is one thing I’m not sure about yet is void vtkMRMLNode::Reset(vtkMRMLNode* defaultNode) calls CopyWithScene and the latter calls Copy and it calls CopyContent. Here it is:
CopyContent uses some macros that I can’t understand now but these macros somehow adds medical orientation presets to my default XY, YZ, XZ. I removed medical orientation presets (Sagittal, Axial, Coronal) from all already created slice nodes and from the default nodes but when ViewControllers is enabled the mentionned above macros adds those medical presets.
Maybe I do something wrong but if you have something to say (maybe some key words about these macros) I would appreciate:
std::vector<std::string> axesNames({"X", "-X", "-Y", "Y", "-Z", "Z"});
std::vector<std::string> orientationPresetOld({"Axial", "Sagittal", "Coronal"});
std::vector<std::string> orientationPresetNew({"XY", "YZ", "XZ"});
std::vector<std::string> defaultOrientation({"XY", "YZ", "XZ"});
for (size_t i = 0; i < orientationPresetOld.size(); i++){
if (defaultSliceNode->HasSliceOrientationPreset(orientationPresetOld[i])){
defaultSliceNode->RemoveSliceOrientationPreset(
orientationPresetOld[i]);
}
if (!defaultSliceNode->HasSliceOrientationPreset(orientationPresetNew[i])){
defaultSliceNode->AddSliceOrientationPreset(
orientationPresetNew[i],
GenerateOrientationMatrix(orientationPresetNew[i]));
defaultSliceNode->SetDefaultOrientation(defaultOrientation[i].c_str());
}
}
On the picture you can see that vtk macros from CopyContent add medical presets:
You can update the slice node reset and copy methods to initialize the orientation presets from the default slice node.
The default orientation name for each slice view is specified in the view layout description. Therefore, if you change the orientation preset names then you may need to update the layout descriptions, too.
From Slicer Astro I have found an example how to modify vtkMRMLLayoutNode description and I applied it to my case but still I can see the default slicer AxialSagittal and Coronal presets:
vtkMRMLLayoutNode* layoutNode = vtkMRMLLayoutNode::SafeDownCast(
this->LayoutManager->mrmlScene()->GetSingletonNode("vtkMRMLLayoutNode","vtkMRMLLayoutNode"));
if(!layoutNode)
{
qCritical() << "qSlicerAstroVolumeModule::setup() : layoutNode not found!";
return;
}
for(int i = 1 ; i < 36; i++)
{
if (i == 5 || i == 11 || i == 13 || i == 18 || i == 20)
{
continue;
}
std::string layoutDescription = layoutNode->GetLayoutDescription(i);
std::vector<std::string>::const_iterator it;
std::vector<std::string>::const_iterator jt;
for(it = orientationPresetOld.begin(), jt = orientationPresetNew.begin();
it != orientationPresetOld.end() && jt != orientationPresetNew.end(); ++it, ++jt)
{
size_t found = layoutDescription.find(*it);
while (found!=std::string::npos)
{
layoutDescription.replace(found, it->size(), *jt);
found = layoutDescription.find(*it);
}
}
layoutNode->SetLayoutDescription(i, layoutDescription.c_str());
}
I don’t understand yet some things especially how node’s description influences on app behaviour and why Slicer’s astro authors use numbers like 36, 5, 11, 13, 18, 20 in this code…
I had a look at orientation presets, and it all looks good. Orientation presets are already stored in the deafult slice node, initialized by AddDefaultSliceOrientationPresets. If you want to change the defaults just make sure you update the defaults after AddDefaultSliceOrientationPresets method is called and before you add your new presets, clear the old ones.
As you can see i refers to layout indices. Instead of 5, 11, etc. the developer should have written SlicerLayoutOneUpSliceView, SlicerLayoutLightboxView, etc. (see complete list).
I took a look at AddDefaultSliceOrientationPresets implementation and the idea pretty clear and now I understand why I need to overwrite presets after this function is called.
From the IDE I can find all references to AddDefaultSliceOrientationPresets and only qSlicerViewControllersModule.cxx references to that function. I tried to comment these lines and rebuilt the target but still Reset(...) function adds medical orientation presets.
You were right, it was my carelessness: I commented AddDefaultSliceOrientationPresets from qSlicerViewControllersModule.cxx but I used to rebuild the ...Logic target instead of ...Module target. There is no any problem if I comment the section with AddDefaultSliceOrientationPresets from that module and the rebuild the correct target.
Proposition:
Does ViewControllers module really need to add orientation presets and make other settings with defaultViewNode? That makes this module difficult to use in custom application.
I think the main purpose of this module is to use controller widgets from module tab and if so then this module can be useful not only in medical software.
I propose to modify this module (if possible of course, I haven’t precisely looked at its implementation yet) in a way that it won’t do any settings with default nodes.
Default view presets need to be set somewhere. The View Controllers module seems to be a reasonable place for that. Where else would you put it?
Note that the preset is already customizable without any Slicer core code change, but if you have a preferred way of making it simpler then let us know. For example the View Controllers module could have a flag to control its behavior or could emit a signal after setting the defaults so that other modules can override those, or default orientation presets could be read from the application settings, etc.
I agree that presets should be set somwhere but in my opinion they should be set somwhere in medical module.
It is very difficult to guess that module named ViewControllers and that is aimed at some widget manipulation brings some medical staff to SlicerCAT.
To figure that out I started launching SlicerCAT with disabled modules. The first module to disable was Endoscopy (though I don’t know how I included it to app) because it is very obvious to think that medical modules makes medical changes and I was very surprised when I discovered that at ViewControllers
I’m not familiar with most medical modules that Slicer uses but I think such things should be implemented there (I can’t tell where exactly). Or probably make some GUI-less module where one does especially medical customization like manipulation with presets. Then those developers who work on SlicerCAT could disable this module and create his own (though I couldn’t do GUI-less module that is completely hidden yet but believe that is possible, I remember I created topic on that on this forum).
I think this would bring some complexity for developers. Slicer already has well structured philosophy with scene, views, slices, nodes, modules, superbuild etc and I think adding signals to control such things is like using insulating tape to weld metall.
The only solution I can see is to try to keep medical staff in medical modules.
P.S.
Sometimes I need to make changes in Slicer core that is not appropriate for medical goals (like resizing slice view while preserving field of view or aspect ratio) and for this I use cpp tool or python tool that replaces selected function implementation during build time and thus allowing to use it in superbuild. Of course I’m trying to make core changes as less as possible
This is not a medical module at all. Endoscopy just means that you visualize from inside. It is used commonly in medical imaging but just as well for inspection of pipelines, tunnels, etc.
While more than 99.9% of Slicer development has been funded for medical applications, nothing is hardcoded in Slicer for medical applications and there is no module that would only be applicable to medical imaging - all modules can be used for various industrial, engineering, etc. applications. Of course defaults must be set to something, and since medical imaging is the primary application area, the defaults are set accordingly. This should not be an issue, as Slicer core developers are willing to make things more generic and configurable, even if that means slightly increasing complexity.
Things that would be considered as workarounds in Slicer core are completely acceptable, good solutions in custom applications. For example, in a custom application you will know that no other modules will change the view preset names, so changing the default in response to a signal would be completely fine, nice and clear solution. However, if you don’t like this then you can propose a change in Slicer core to read default view presets from the application settings.
In general, source code changes are probably the best done using git, because all code changes are automatically synchronized (or you are prompted to resolve conflicts manually) whenever you rebase your Slicer core version. It is much simpler and more robust compared to patching files.
Of course these should only be used temporarily, while you are validating some ideas and once things are mature then you get them merged into the Slicer core (or make Slicer core configurable so that you can inject your custom behavior).
Thank you for the information, I’m slightly ashamed about not knowing about Endoscopy module
About ViewControllers:
The default presets are lodaded in file vtkMRMLApplicationLogic.cxx in function void vtkMRMLApplicationLogic::SetMRMLSceneInternal(vtkMRMLScene* newScene).
As I see void qSlicerViewControllersModule::readDefaultSliceViewSettings(vtkMRMLSliceNode* defaultViewNode) handles PatientRightIsScreenLeft and PatientRightIsScreenRight.
I thought that we could added property like IsAxesMedical to default node and display it as a checkbox in Edit->Application Settings->Views and if this checkbox is uncheked then View orientation combobox should be greyed out and ViewController don’t have to do any manipulations with orientation presets. How do you think?
EDIT:
I just remembered that you already proposed to set default preset from application properties - Yes that I also like.
About signal in this case:
If emitting signal directly from vtkMRMLSliceNode::AddDefaultSliceOrientationPresets(newScene) was possible I would like to do that. But instead of emitting it from ViewControllers I would prefer the method proposed in the upper paragraph with attribute.
