r21904: Add mechanism to register dialogs for drag&drop

I took a look at this - it is technically doable to make to integrate dicom with drag and drop, but we have a policy question to sort out for dropping folders and dicom files onto the application.

Current behavior: if a folder is dropped on slicer, all individual files are added to the Add Data dialog.

Suggested behavior: all dropped folders should be sent to the dicom module and only dropped files should be handled by the Add Data dialog.

I would think of implementing this suggested behavior by adding something like a directoryDropped(QStringList) signal from the slicer main window.

Other alternatives could be:

(1) we could figure out a way for modules to register their interest in dropped directories and they would have a chance to inspect their contents before grabbing them (i.e. according to a priority scheme). This could be tricking since each module might have to do a recursive search and also it might find some files it understands and some it isn't able to load. At least for DICOM, this would be almost as much work (and redundant with) the actual import process.

(2) when the user drops a folder we could pop up a dialog asking if they want to load this as DICOM or as non-DICOM data. This strikes me as a little messy, but maybe it's not so bad.

Steve, my commit (r21904) kind of do what you suggest.
A module can now "register" an IO dialog that responds to drag&drop events.
The registered dialog must overwrite the dropEvent() method and decide if it can handle the content of the mime data or not (https://github.com/Slicer/Slicer/blob/master/Base/QTGUI/qSlicerDataDialog.cxx#L370-400). If it does, then the io manager execute the dialog.
If it does not, then the next registered IO dialog is tested and so on.
At last, the qSlicerDataDialog will handle the data if none of the registered io dialog can.

So it should work if the dicom module registers an IO dialog that only accepts DICOM directories in the dropEvent() method. No ?

I more-or-less agree with you Julien, but the problem is that without inspecting the contents of folders (and subfolders) it's not clear which IO dialog should handle a folder (DICOM folders can have arbitrary layout and filenames, with no required file extension). So I would see it as more clear at a documentation/user level to say that all folders get sent to the DICOM module but all multiple file drops go to the IO dialogs.

Or, as in my option (2) we could prompt the user to ask which action they have in mind.

I like the "ask the user" option. Let them decide instead of us guessing.

I'm confused :-), what's the problem with analyzing the contents of the mime data url path. Why the registered DICOM dialog wouldn't be able to do that ?
If you think it's too difficult to know if a url is a DICOM url, then the simpler solution of having the DICOM registered dialog always accepting any directory should be fine too.

Option (2) is also fine and should be easy to implement in qSlicerIOManager::dropEvent() which would get the list of all the readers that handle the mime data content, and popup a menu with such list (let me know if you want to go that way).

Btw, I think the dropevent should stay flexible enough to support more than just filepaths. Mime can contain images or custom content which would make Slicer support drag&drop from other applications(for example, you could drag&drop openIgt configuration to setup a connection...

The key point is that a "DICOM url" in this context means exactly "any directory" due to the ambiguity of knowing what is in the directory without looking.

Getting back to the big picture, if Ron likes the idea of prompting the user with a dialog, let's do that.

For that, a clean way to do this would be for the qSlicerAppMainWindow to issue the put up a dialog inside the dropEvent method so that if one or more directories are dropped, it asks if the data should be interpreted as DICOM or non-DICOM data. Then if the user picks DICOM, the qSlicerAppMainWindow can emit a signal like dicomImportRequest(QStringList) and the DICOM module can take it from there.

It's a shame we have to go through convoluted special cases like this, but DICOM data really is not like traditional file formats like .vtk or .nrrd.

Ok for the dialog.

However, I disagree with you concerning the clean way.
qSlicerAppMainWindow should not be aware of any IO (DICOM in this case). It's qSlicerIOManager's job. Everything can be done within qSlicerIOManager::dropEvent() as long as the DICOM module registers the appropriate dialog (note that qSlicerFileDialog is not a real dialog, but it's more like a plugin).
All that is needed is a mechanism to easily create custom qSlicerFileDialog from Python. A mechanism like qSlicerScriptedLoadableModule would be ideal, but a believe something simpler could work too (e.g. qSlicerScriptableFileDialog could fire signals when dropEvent() and dragEnterEvent() are called and it would expose slot to accept/reject those events)

I don't really care if it's the qSlicerIOManger or the qSlicerAppMainWindow that fires the signals.

To me, adding another subclass just to do this seems like overkill. If one of the existing classes emits the signal then everything can be handled with just a few lines of code. Connecting to signals seems much nicer than registering a dialog.

On the other hand, I'm trying to think of what a qSlicerScriptedFileDialog would do. In my view a pure python extension to be able to configure and extend all parts of slicer, so if this adds more scriptable functionality I'm all for it.

Scriptable file dialog added in r21951:
http://www.slicer.org/slicerWiki/index.php/Documentation/Nightly/Developers/IO#How_to_register_a_dialog_.3F
http://www.slicer.org/slicerWiki/index.php/Documentation/Nightly/Developers/IO#How_to_order_the_list_of_drag.26drop_dialogs_.3F

Impressive work Julien - but it's not building for me on mac.

In file included from /Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerFileDialog.cxx:31:
In file included from /Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerIOManager.h:15:
/Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerFileDialog.h:152:1: error: redefinition of 'QMetaTypeId<qSlicerFileDialog::IOAction>'
Q_DECLARE_METATYPE(qSlicerFileDialog::IOAction)
^~~~~~~~~~~
/Library/Frameworks/QtCore.framework/Headers/qmetatype.h:268:12: note: expanded from macro 'Q_DECLARE_METATYPE'
struct QMetaTypeId< TYPE > \
^~~~~~~
/Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerFileDialog.h:94:1: note: previous definition is here
Q_DECLARE_METATYPE(qSlicerFileDialog::IOAction)
^
/Library/Frameworks/QtCore.framework/Headers/qmetatype.h:268:12: note: expanded from macro 'Q_DECLARE_METATYPE'
struct QMetaTypeId< TYPE > \
^
In file included from /Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerModelsDialog.cxx:26:
In file included from /Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerIOManager.h:15:
/Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerFileDialog.h:152:1: error: redefinition of 'QMetaTypeId<qSlicerFileDialog::IOAction>'
Q_DECLARE_METATYPE(qSlicerFileDialog::IOAction)
^~~~~~~~~~~
/Library/Frameworks/QtCore.framework/Headers/qmetatype.h:268:12: note: expanded from macro 'Q_DECLARE_METATYPE'
struct QMetaTypeId< TYPE > \
^~~~~~~
/Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerFileDialog.h:94:1: note: previous definition is here
Q_DECLARE_METATYPE(qSlicerFileDialog::IOAction)
^
/Library/Frameworks/QtCore.framework/Headers/qmetatype.h:268:12: note: expanded from macro 'Q_DECLARE_METATYPE'
struct QMetaTypeId< TYPE > \
^
1 error generated.
make[5]: [Base/QTGUI/CMakeFiles/qSlicerBaseQTGUI.dir/qSlicerFileDialog.cxx.o] Error 1
make[5]: Waiting for unfinished jobs....

Seems the line is duplicated - if I remove one it seems to work fine. Do you have a preference for removing the one before the class declaration or the one after?

$ grep META /Users/pieper/slicer4/latest/Slicer/Base/QTGUI/qSlicerFileDialog.h
//Q_DECLARE_METATYPE(qSlicerFileDialog::IOAction)
Q_DECLARE_METATYPE(qSlicerFileDialog::IOAction)

Sorry about that. I've a slight preference for removing the second one. (I should have moved qSlicerStandardFileDialog into a separate file :-/ )

No problem ;)

Fixed in r21953

I'll try to have a look at integrating this with the dicom module this afternoon and let you know if I have questions.

To slightly improve the user experience: If I drag a dicom directory into slicer, the popup defaults to "Any Data". That is potentially confusing. It should default to "Dicom", with any data as a pull down option

@Julien - Is there an easy place to implement Ron's request?

Load DICOM by default is fixed in r22267