I’d say that’s a bunch of legacy code and I’m sure there’s a better design - probably the whole widget could be made simpler to maintain and more usable if you are up for it.
Development of DICOM module had been hindered by having widgets in CTK and therefore need to agree with MITK folks on design (and for example a few years ago they did not agree to store derived values in the database, only standard DICOM fields).
Since only Slicer developers contributed to the DICOM browser in the last few years and probably this will not change in the near future, I think we should be able to make changes that we need. If CTK users have the capability to update their applications according to non-backward compatible changes in CTK DICOM infrastructure then we can do the rework in CTK. If CTK users preferred keeping things as they are now, then we would need to fork CTK DICOM features in Slicer core. @jcfr @pieper how do you think we should manage this? How could we start a discussion with CTK community - by posting an issue on github?
Yes, if we are ready to take on a redesign of the widget we should contact Marco (he plans to be at project week in Las Palmas so we can coordinate there and/or before/after). If it turns out we don’t have the same goals we can diverge and put a new browser either in CTK or Slicer directly. We only need to share as much infrastructure as actually makes sense for our projects (which should only benefit - if it hinders we don’t need to do it).
2 Likes
After discussing this with @cpinter and @lassoan, there is another way that we could organize the DICOM browser.
The DICOM browser widget could be opened in the central widget like is done in ShapePopulationViewer, ex:
We could still provide the option to use the pop-up window for applications/users that would prefer it.
In the longer term, it would be useful if we could re-organize and simplify the DICOM widget to minimize the modifications that are required in Slicer. Are there any modules that rely on the current structure of the DICOM popup?
1 Like
This is just a mockup to show where the browser would appear (in the view layout, as a one-up view). The final GUI will be improved - we can move menu actions at the top to the module panel on the left, etc.
Overall, I very much like this approach. I do not know if anyone really needs to have DICOM browser in a separate window - I never use that functionality, and I am 100% sure no one likes the bug that triggered this conversation.
At some point @che85 experimented with something like this in a context of a custom modile, and he ran into some issues, but I don’t recall the details now. Christian, can you comment, if you remember?
Are there any modules that rely on the current structure of the DICOM popup?
I don’t know if this is the right way to do things, but in some cases logic of the module is accessed via the popup widget, eg see Slicer/Applications/SlicerApp/Testing/Python/RSNAVisTutorial.py at main · Slicer/Slicer · GitHub. It is not exactly a dependency on the structure that you asked about, but probably something to be considered. I don’t know why this is the way logic of the module is accessed.
I’m not sure how the ShapePopulationViewer looks (not available for 4.10 on Mac), but is the idea to make the DICOM browser shown as a tab? (and allow switching back to the 3D view tab?). That sounds nice.
On the topic of the modal dialog, I think this issue is related (and several others attached to it): https://issues.slicer.org/view.php?id=1792
The DICOM browser will be shown in the view layout. We would add a one-up layout with the DICOM browser, but we could add define layouts that have slice and other usual views.
Switch using tabs: we could add a list of recent and/or favorite view layouts and switch between them using tabs or toolbar buttons.
This is how ShapePopulationViewer looks:
The idea is that it defines a custom layout containing a custom widget. When entering the module, it remembers the current layout and switches to the custom one. When leaving the module, then it restores the previous layout. It’s nice because it fits into the layout logic without any hacking, and I also like module selection controlling it.
For the DICOM module, we could now have something meaningful in the module panel such as image preview, database directory selector, etc. If the user clicks the DCM button then it switches to the module as currently but the layout would be replaced by the DICOM browser custom layout. Then when the user wants to do something, they need to switch to another module, in which case they get back the viewers. We could also automatically switch to the Data module after successful loading to make the new data appear in both the data tree and the viewers.
I see. Thanks for the explanations. DICOM browsing is a very temporal/temporary interaction, so I think requiring to change layouts every time (even automatically) might feel heavy or inconsistent with the rest of the UI. Whereas the ShapePopulationViewer is still using the “main” area for data viewing (+some extra controls).
1 Like
I am pretty sure that we fixed that in QuantitativeReporting but we never added the DICOM browser back into the module.
In QuantitativeReporting we added it directly in the module widget
and
So it’s a tab that once clicked hides the slice viewers and uses the whole width of the Slicer window. Once data is loaded the user can return to the previous step where the slice viewers and the module widget width should get restored to its original state.
NOTE: I think there was an issue with resizing the width of the module widget after trying to restore the original state.
The two main options seems to be A. switching layouts, B. temporarily hide the central widget (view layout). I don’t really see any advantages option B. but there seems to be a couple of disadvantages:
- Not having a central widget may not be a valid application main window layout in Qt (https://stackoverflow.com/questions/3531031/qmainwindow-with-only-qdockwidgets-and-no-central-widget)
- We could not show the DICOM browser while in another module (while if the DICOM browser is in the view layout then the user could switch to that layout and back - without exiting from the current module)
- We would lose 3-4 lines across the whole screen due to the Slicer logo if we just extended the module panel to full window width.
- In the long term, we’ll probably allow creating view layouts more dynamically (similarly to ParaView) and will support multiple monitors. If we manage DICOM browser outside of the view layout then the DICOM module will not benefit from this infrastructure.
- We cannot easily have combined view layouts (e.g., show a DICOM browser with a slice or 3D view). I don’t know how useful this would be now, but maybe when we have dynamic/multi-monitor support then this will be more important.
1 Like
[Update: I’m not sure where did @fedorov’s comment go about the browser in the module panel but this was an answer to that]
I like this too, except that then where do the options go that are currently occupying this space? Also we’re losing the opportunity to improve the module panel to include actually useful supplementary information like thumbnails etc.
Sorry for the mishap …
Here’s the proposed mockup again
If you are talking about the content below, I would say they can easily go into some log or advanced view. I personally never use the “Recent activity” list, and start listener is a quite advanced option.
Not losing, just making it available only in the full view. I am just thinking about the 80/20 rule. In my experience, the abbreviated view would address most of the uses of the DICOM module. Just my 2c. It would be great if other users share their perspective so we try to get a more representative sample.
1 Like
I’ve created a pull request that integrates the DICOM browser into a new view and layout.
I’d appreciate everyone’s thoughts/comments.