I think the DICOM browser in the new stable release for Mac has a very little issue. When I import a DICOM, after it finishes it closes automatically (before it would stay open and let me load the series). After the browser closes, the DICOM module keeps working but not the browser button: if I click on it to reopen the browser, nothing happens. However if I go to another module and then come back to DICOM module, the browser button works alright. Is anybody having the same funny behaviour? The nightly built from the 01/11/18 also does the same thing but the previous stable release doesn’t.
It does not close, it is just “hiding” behind the main window. Try to move around your Slicer main window, then you should be able to find the DICOM Browser window. It is quite annoying and counter-intuitive.
Oh wow that’s funny, I would never have noticed, cause it doesn’t even show in the dock
Indeed, it’s very annoying…I think I’ll just revert to the old version. Thank you!
Wow! I would never think it is that bad for a user … I don’t know how difficult/feasible it would be to fix this. I thought I raised this issue a while ago, but I cannot find the thread.
If you only have a single monitor then it is better to automatically hide the DICOM browser window after loading by unchecking “Browser persistent” checkbox. Do you mean that the DICOM browser window is not hidden after loading even if “Browser persistent” is unchecked?
@lassoan on mac the DICOM Browser window is hiding behind the main application window after the Import operation. This is not about hiding it after load.
Yes, I saw this today with Sonia too - it seems to be an issue with Qt5 on mac.
I see, thanks for the clarification. I cannot fix this, as I cannot reproduce this on Windows, but probably the DICOM browser window just need to raised to the top by calling
widget.raise_() or something similar.
I think this sounds the same issue we’ve seen in our Slicer application @lassoan. If so, it was a problem since a year ago for us which pre-dates Qt5?
@Sunderlandkyl you looked on our side, did you raise a bug or anything?
It’s not bad but the old version has everything I need and does not have this bug so I’d rather use that until I’ve finished importing all my dataset
It’s funny I cannot reproduce this with my local build or Slicer 4.10 on my desktop or 4.10 on my laptop mac (running 10.13.6 High Sierra and 10.14 Mojave respectively).
In non-persistent mode the DICOM browser closes after loading. In persistent mode it stays on top of the slicer app window.
I’ve just tried again on my work desktop (running 10.13.6 High Sierra) and after importing a new DICOM series the browser hides behind the main window independently on whether persistent mode is selected or not. This happens with Slicer 4.11.0-2018-11-0, Slicer 4.10.0 and Slicer 4.9.0-2018-10-16 but not in Slicer 4.8.1-stable. I don’t know about the latest nightly because I would need admin approval to install it and can not do it right now.
But that’s ok, when I raised the issue I thought the browser button was stuck, I hadn’t realized the browser was just hidden (sorry for that, I could have looked better). That’s not a big issue, I’ll just do all the imports at once or use the old version to avoid playing hide and seek with the DICOM browser.
I see - yes, this happens when the directory import is complete, not on loading the data (sorry, read the issue wrong). It’s probably because the confirm dialog is parented to the main app window so main window is raised when the dialog is closed. Probably the directory selector and the confirm dialog should be parented to the dicom browser window instead.
We’ve been testing this @Sunderlandkyl and found that the problem may be the use of modal popups. These popups are quite annoying anyway, so we are thinking about replacing them by widgets in the DICOM browser window.
We could move the progress bar from a popup to the bottom of the DICOM browser window and disable the rest of the window. We could probably even run indexing in a background thread (SQLite allows concurrent database access from multiple threads) and so indexing would not block the GUI. The result popup could be replaced by a textbox/button showing summary and by clicking on it we could show more detailed information.
Here is a quick mockup of the proposed layout:
The label beside the progress bar would provide a broad indication of the current operation/status (import in progress, import completed, etc.). Clicking the details button would provide the user with additional info (Number of patients imported, verbose error messages, etc.)
Any feedback or thoughts?
Thank you, this looks good to me!
We also need a Cancel button. It could be next to the progress bar or in the details window (that is shown when Details button is clicked). Probably the details window would be a better place because in the future we could add a cancel button for each task (and tasks could include network tasks, such as push/pull images to/from a server).
Note that we tried threading the indexing in the past and had some mystery crashes.
I’m sure it’s solvable, but probably best to fix the model dialog issue first and then do threading as a separate experiment.
As the SQLite FAQ says:
“Threads are evil. Avoid them.”
See this issue for more background:
Background indexing will be done in a separate development step for sure.
If threading is not stable then we may do it in a separate process, in a CLI module. We already have all the necessary asynchronous execution features in place for CLIs (progress reporting, canceling, job queuing, etc.), so we would not need to develop new infrastructure.
By the way, it seems that you’ve tried to use the same database connection in multiple threads, which I think is not allowed by default.
We could also change the indexer to collect all data in memory in a worker thread, and after every 50-100 files update the database from the main thread.
I’ve actually discovered something interesting.
- If you create a ctkDICOMBrowser from the python interactor, then import will not cause the issue
- If you create a DICOM browser through DICOMWidget.py, then the browser will have the issue
In that case, it seems to me that it might not be caused by the modal popups, but is instead caused by the rewiring of the browser widget that takes place in DICOMWidget.py.
To get a better idea of what’s going on, can someone explain to me why this widget reorganization is neccesary in DICOMDetailsBase.setup(): https://github.com/Slicer/Slicer/blob/8d765eab6445c277346f4c7248d9441dca4559dc/Modules/Scripted/DICOMLib/DICOMWidgets.py#L127-L343