Roadmap list of issues/enhancements for Visual DICOM Browser

[Punzo]

RoadMap (items are not listed in priority):

long-termENH:

• implementing send in C++ in CTK (i.e. adding ctkDICOMSendJob, ctkDICOMSendWorker and ctkDICOMSend with underlining DIMSE DcmStorageSCU). This would allow to use the background/parallel operations infrastructure for SEND as well.
• add DICOMweb
• handle jobs queue in the scheduler by file (so we can restart the jobs/workers at application restart)
• add data streaming from visual browser series widgets to Slicer volume nodes.
• add support for DICOM frame set (reference https://discourse.slicer.org/t/new-frame-set-table-in-the-dicom-database/35012)

UI customization:

• visual DICOM browser (and in general CTK) uses Qpalette to change colors dynamically. This does not go well when customizing CTK/Slicer by styleSheet. Probably we should use the styleSheet approach everywhere. For example, the background yellow warning on filters does not work when applying a custom styleSheet. Discuss with Sam for a good design. Ideally the solution should:

1. use one unique style file containing the stylesheet for all the CTK classes
2. change color of qt widgets in CTK dynamically with the Property Selector, e.g.: https://wiki.qt.io/Dynamic_Properties_and_Stylesheets
3. custom Slicer apps should be able to change the style by simply rewriting and reloading the style file

Issues and minor ENH:

Logic:

• add a writing lock static variable in the ctkDICOMDatabase class
• smart managing of the inserts and memory when retrieving a series. i.e. we could have a customizable framesBatchLimit variable.
• add infrastructure to set the maximum number of concurrent workers per server (now it is just per type of job).
• the retry time delay value should be higher and exponential with a factor 2/3 (with some randomness), i.e. each subsequent retry should have a larger delay. Current default parameter is a fixed 100ms (not enough for a server to recover). Instead of using the Maximum number of retries (current default is 3), we should use the maximum waiting time (the one defined in the GUI in the server settings) -> timout warning in the UI (check the string if it is clear).
• Automatic prefetch of the full series (now the current behaviour) could be disabled and the retrieve could it be done only when the selected series are manually loaded.
• While waiting for download of a complete sequence, we can still do some user actions, but not all. I suspect that it is because we pump the event loop with app->processEvents(), but that might be unintentional. We might want to call processEvents with flags that disable user interaction, or even better, show a model popup with a "Cancel" button.
• Stopping/Deleting many jobs (>100) makes the scheduler hanging for ~2 sec. This because the deleting is done one job at the time and then a signal is emitted to queue new jobs. In Addition this methods are thread safe by a mutex which is expensive. Probably the best action would be to delete all the jobs togheter in only 1 call.
• The SCU release association (DCTMK) should be protected by a mutex, because the method in DMTCK is not thread safe and in past we had crashes for it. I had also to force the release of the association at cancel operation of worker for query and retrieve. This because the DCTMK cancel operation can fail (but returns always good() == true) and in some cases we can have eternal zombies workers. The release is done in the hanlde overriden methods of SCU.

Settings:

• servers should be in a "Servers" collapsible section, the same way as "Storage" (instead of the "Servers" label)
• Add debug option to activate the output from DCMTK (global logger to terminal).
• Style of "Apply changes" and "Discard changes" should be the same as Add/verify/remove host.
• Verify host: change response string. C-ECHO connection verified/failed. Add a "Verification" column in the "Servers" table (unknown, in progress, success, failure). C-ECHO should be run in background by the scheduler (need to implement related job and worker classes).
• server settings should be more human readeable in the qt settings file.
• Change in the UI the column names Proxy and Protocol with additional Retrieve (Retrieve Proxy etc....)
• Apply warning yellow background to modified fields.
• MedicalConnection server node, should have the Query/Retrieve/Storage checkboxes to be checked as default.
• After clicking Apply, the row selection should be preserved
• Hide restore defaults button (it can be shown only for testing)
• Verify host should work on the current modified server settings (or it should be disabled until the user apply or discard the changes).
• MedicalConnection server node, should actually NOT have the Query/Retrieve/Storage checkboxes to be checked as default.
• verification column should be updated only from jobs which started after clicking the verify button.
• Closing the application while echo workers are running make it crash. steps: run a verify server with wrong parameters and a timeout of 30 sec. The destructor of the scheduler will wait only 10 sec (hardcoded). The destructor has to wait for all jobs closing properly (echo job will not terminate before the server timeout parameter).

Visual browser UI:
Patient selection:

• By default show all local content.
• Display patient name in "Patient information" section (while there are many patient names listed above, it is not that clear which one is actually selected; it is also not possible to select and copy-paste that name). Left-align field names and values (for example, "ID:" label is on the left - which is good; but then the actual ID value is floating somewhere in the middle of the screen).
• Hitting enter in a search field should run the query (same as clicking the search button)
• When hitting the search button, the patient list always jumps to the first patient, which is quite annoying, as I keep clicking it to see if new studies have become available for the current patient on the server. Current patient selection should not be lost if Search button is pressed (if the current patient is still selected by the search criteria).
• Patient selection is lost when showing/hiding the browser in 3D Slicer
• Having a shortcut to browser previous/next patient (Ctrl-tab/Ctrl-shift-tab) would be useful when working with a research PACS (where we are not find/explore data, without necessarily knowing patient name/id).
• study sorting by date, sometimes does not work. Verify the sorting.
• at least happened once: after pressing search, download of series started. Switched to a different patient, hourglass was displayed for a while and then the application crashed.
• When changing patient if there are TCIA datasets imported from local disk and there is the test server enabled (medical connection), the UI gets stuck and there is this message logged many times:
  Failed to find patient with PatientsName= and PatientID=
• visual dicom browser does not have drop action (neither the old browser). In Slicer dropping a dicom folder will use the python code in DICOM.py and then use the old browser to import. If Slicer is using the new browser should import with the visual one.
• All the DICOM import from local disk were broken for the visual dicom browser when using it in Slicer. The issue was that several methods on the DICOM module UI were using directly the old browser. Refactored to call methods of the class DICOMbrowser.py. Then it is the DICOMbrowser that take care of calling methods from the two browsers.
• importing multiple folders for the visual dicom browser is broken
• when importing in Slicer this logging should not happen:

D: DcmDataset::read() TransferSyntax="Little Endian Explicit"
D: DcmMetaInfo::checkAndReadPreamble() TransferSyntax="Little Endian Explicit"

This happens when detailed logging in DICOM settings is checked. It is logging printed by DCMTK. Detailed logging is already as default set to false.

• We need to be able to select the enabled severs on a patient basis.

Filtering:

• when doing a search without filter (i.e. show all local database), if there is nothing in the local database, the yellow warning background fo the filter gets bugged (always on, even if doing a new search)
• Filter at the moment is both query and filter the local database (this could be confusing). Add an advanced menu under the query button to disable the query/retrieve.
• Filter local database could leave studies or patients empty (these should be filtered out). See also next two issues for more detailed info.
• I don't get some updates (study list remains empty) when setting filter criteria to "Everything from last year". I see "ctkDICOMQuery: the number of responses of the query task at patients level surpassed the maximum value of permitted results (i.e. 25)." message in the log - can it be the root cause? Or maybe just that this filter does not filter out patients. It is confusing that some filter boxes filter patients, while others let all patients displayed, even those that don't have matching studies/series/modality/date.
• The logic behind "Study" and "Series" textboxes in "Patient search" section is not clear. If I type there something then still all the patients are shown, just studies or series list is empty. When the study filter is set then patients that don't have any matching studies should not show up. Similarly, those patients and studies shold not show up that don't have any series that matches the filter criteria.
• Patients list as tabs in the tab control of the Patient widgets is not optimal. We should use the old widget from the ctkDICOMBrowser (first one, only the patient list). We could have an option to switch between them.

Thumbnails && series widgets:

• on right click menu we will need an addittion action: force redownload. (for series and studies)
• additional feature for force download (add checkable settings, default False): if any series is 3 hours old (also this should be a CTK settings variable), force redownload every time we use a search.
• additional feature for force download: enable monitoring (right click action) for a time (configurable). If any series or instance in series gets uploaded in PACs, in this case, will be automatically fetched and the UI updated (some UI icons to give the warning that it is updating). For series, we should check also if new instances have been added in the PACs to the series.
• additonal feature: add a check (when clicking the query/retrieve lense button) to a folder containing dicom files, if there are files import them and clean the folder. Create a lock file in the dicom folder and we put there the PID of the session of the user, when is accessing the DICOM files/deleting them.
  Check also if using a watcher we can automatically do the import.
  The local folder should be a "local connection", like a server. So we could add as many we want in the settings and everything will be automatic in the infrastructure (not adding hard coded specil cases, but everything will be independednt), it is just a new "protocol" comunications.
• right click delete name in the UI should be: delete from local database
• text: add transparent and improve shadows. Remove N.frames and put it as the third dimension on the rowXcolumns (e.g. 100x100x5). Elide the series description and put the full text in tooltip.
• thumbnails are quite low quality, which is visible when the image contains text or simple graphics - if there is an option to use higher-order interpolation and it is not much slower then it would be nice to switch to that
• rendering for Segmentation DICOM object does not work with the ctk thumbnail class.
• Selection in series list uses blue background. If the thumbnail image has rectangular shape then the blue text that is overlaid on the thumbnail is not readable (blue text on blue background). Maybe improving the drop shadow would fix it (to make it 4 directions instead of just 1, maybe larger offset).
  
• Wait and load selected series sometimes did not worked and the UI got stuck.
• loading a series in visual dicom browser does not have the old advanced/examine UI
• after deselecting there is a white box around thumbnails in dark mode in Slicer
• thumbnail should not be updated by jobs that started before the last scheduler API call from the series widget. Sometimes old jobs could take more time to be killed than need jobs to run successfully. Then the stopped one should be ignored.

Error report:

• Implement Job list status UI. See next two issues for more info.
• Error reporting is inconsistent. Most often I don't see any errors (just nothing shows up), but once an error message came up as a button. But then I could not remove that message. A nice solution would be to have a "Job list" button, which could show an error notification (red or yellow circle) on it when one of the jobs failed. Clicking on it the button would show the job list (it can be a popup or a collapsing section in the GUI) that would show all the jobs (in-progress and completed) along with its status (it can be just simple list, details shown as tooltip or popup; with an option to show all jobs or only in-progress jobs, a button to clear the completed jobs, a button to cancel the selected jobs, and a button to retry the selected jobs).
• Error message like "bc2343432-234241frfd-2342341s job failed to fetch data" is quite annoying. It should tell something like "Fetching data for patient John Doe [patientid] from server MedicalConnections failed. Response not received for 30 seconds. Click here to see more details." (clicking could open the job list with that task highlighted)
• we need to stream the DCMTK logging per each job to the Job list UI, this can be implemented by instantiating an Appender with a custom ErrorHandler to the SCU logger: DCM_dcmnetLogger (name: "dcmtk.dcmnet")
• when loading a series, if it is not supported (or it is supported with an extension), we should fire a popup warning.
• big warning push button for failed jobs should be removed. We should have warning/error icons at patient/study/series widgets which should refer to the job in some way.
• then we could use the same icon on patient/study/series widget to know if we are waiting for the query (loading icon)
• also a refresh button to refresh the query for patient/study/series widgets (if everything was success, this can be useful to query new changes in the server)
• clearing jobs push buttons should be outside from filter group box
• clear completed push button, should also remove the current "cancel" one
• status: if the user stop the jobs, status should be "user stopped"
• status: if the job is stopped because it failed, it should be "attempt failed" (if it will retry it) / "failed" (if it is the last attempt)
• When a request fails then it leaves 4 entries in the log: 3x failed attempts + 1x final failed attempt. This is too much noise (and if the user wants to retry he would need to pay attention not to retry all 4 entries, only 1). Could you change filtering settings so that the 3 failed attempts are not displayed by default? The simplest would be to only display the 3 failed attempts only if “Show completed” is enabled in filtering settings.
• Jobs and Settings group box should be encapsulated in one
• Having also a splitter between the main patients navigation widget and Jobs/Settings groups would be ideal.
• clicking a row in the job list should open the corrispettive patient widget in the main navigation widget.
• Details:1) do not put the separator if only one job is selected 2) separator should be either ----- or ==== (check DCMTK and use a different one) 3) do not put spaces before ":"

ctkDICOMVisualBrowser application:

• the folder selector does not reflect the actual DICOM database location (it seems that the button is not updated from application settings at startup)

Reference PR:

• Add dicom visual navigation | ⚠️ Changes removed from main #1142
• Add Visual DICOM Browser | ⚠️ Authorship of integration commit changed #1165
• ENH: Add experimental visual DICOM browser Slicer/Slicer#7525
• Add dicom job list widget for logging jobs activity in the visual DICOM browser UI #1184
• visual dicom browser enhs and bugs fixes #1191
• Fixes for the visual dicom browser Slicer/Slicer#7676
• Fixes and enhs for the visual dicom browser #1201
• ENH: Example how to use dynamic properties and stylesheets #1202

[Punzo]

• Huge amount of DICOM communication log is printed on the console. It can cause very significant delays, so the amount should be configurable (we can use DICOM/detailedLogging application settings value).

@lassoan this should already work, please try to disable DICOM/detailedLogging in the Slicer settings.

Also, the messages should not be dumped to the console but it should be shown in the job list (maybe some messages, e.g., warnings/errors could be also logged in the application log)

yes I agree, this is also described in other points.

[Punzo]

For reference: Error report is addressed in the following PR:

• Add dicom job list widget for logging jobs activity in the visual DICOM browser UI #1184.

[jcfr]

• Finalize and publish discourse post: https://hackmd.io/SFrAGE4CS8uSkI2YvDRKRg?both

[Punzo]

• Finalize and publish discourse post: https://hackmd.io/SFrAGE4CS8uSkI2YvDRKRg?both

I have done my last edit. @lassoan it would be nice if you can review (specifically the intial sentence regarding the "patient exploration tool").

[Punzo]

@lassoan, thanks for the text edits and I have applied yout comments in https://hackmd.io/SFrAGE4CS8uSkI2YvDRKRg?both.

I am going to post it now on the Slicer forum

@jcfr can you remove or replace the images and videos (see below for the new ones) in #1165 (comment), please? thanks in advance

VideoVisuaDICOMBrowser.mp4

[lassoan]

I am going to post it now on the Slicer forum

Is the feature included in today's Slicer Preview Release? If not then please wait. Also, for biggest impact, it may worth making the announcement on Tuesday (many people are not at their computers in the weekend; Monday is often too busy). It would also allow us to do some internal testing with people we ask to have a look (Steve, Rudolf Bumm, ...).

[Punzo]

Is the feature included in today's Slicer Preview Release? If not then please wait.

yes!

Also, for biggest impact, it may worth making the announcement on Tuesday (many people are not at their computers in the weekend; Monday is often too busy). It would also allow us to do some internal testing with people we ask to have a look (Steve, Rudolf Bumm, ...).

ops sorry, I should have waited, I have already posted https://discourse.slicer.org/t/new-feature-visual-dicom-browser/33874 . I can delete it if you think it would be better.

[lassoan]

OK, no problem. There is some time before the weekend, so hopefully it will catch people's attention and they can find some time to try it and give feedback.

[jcfr]

We can also add a follow post on the topic on Tuesday to bring it to the top of the list. Something along those lines:

Raw:

Now that the feature has been available :sparkles: for a few day through the Slicer `Preview` package, let us know if you have any comments or suggestions.

Thanks again for your time :pray:

Rendered:

Now that the feature has been available ✨ for a few day through the Slicer Preview package, let us know if you have any comments or suggestions.

Thanks again for your time 🙏

[Punzo]

We can also add a follow post on the topic on Tuesday to bring it to the top of the list. Something along those lines:

Raw:

Now that the feature has been available :sparkles: for a few day through the Slicer `Preview` package, let us know if you have any comments or suggestions.

Thanks again for your time :pray:

Rendered:

Now that the feature has been available ✨ for a few day through the Slicer Preview package, let us know if you have any comments or suggestions.

Thanks again for your time 🙏

@jcfr @lassoan I was thinking that we may wait for the merge of #1184 (job list) to bring the post up to the list. Let me know.

[Punzo]

for reference:

current dev CTK branch
https://github.com/Punzo/CTK/tree/visualDICOMBrowserENH

current dev Slicer branch
https://github.com/Punzo/Slicer/tree/visualDICOMBrowserENH