Skip to content

Commit

Permalink
ENH: Display error message if node loading fails
Browse files Browse the repository at this point in the history 
Data saving has been recently updated to report errors and warnings. This commit extends the developed
error reporting mechanism for data loading.

Slicer simply did not do anything when it failed to read a file. This was particularly a problem when loading many files
(such as scene) because then the user could not easily see if something has failed.
Users also had difficulty in determining why exactly loading failed (e.g., they tried to load a .vtk file as a model,
but the file happened to contain a volume).

User displayable error/warning messages are now passed from VTK or ITK reader/writer to qSlicerIOManager
(VTK/ITK reader -> MRML storage node -> module logic -> IO plugin -> IO manager) via message collection class
and displayed in a popup window if reading fails.

Detailed error reporting is implemented for scene files, MRML scene bundles (.mrb), markups, models, and sequences.

Improved data loading using drag-and-drop: the caller application is no longer blocked until data is loaded.
List of files are retrieved from the drag-and-drop event, the event is accepted, and the call returns.
Load data dialog is displayed by a singleshot timer call.
  • Loading branch information
@lassoan
lassoan committed Feb 7, 2021
1 parent 3eee112 commit c5f5d62
Show file tree
Hide file tree
Showing 22 changed files with 748 additions and 314 deletions.
27 changes: 20 additions & 7 deletions Base/QTCore/qSlicerCoreIOManager.cxx
View file
Expand Up @@ -480,12 +480,15 @@ bool qSlicerCoreIOManager::loadNodes(const qSlicerIO::IOFileType& fileType,

// If no readers were able to read and load the file(s), success will remain false
bool success = false;
int numberOfUserMessagesBefore = userMessages ? userMessages->GetNumberOfMessages() : 0;
QString userMessagePrefix = QString("Loading %1 - ").arg(parameters["fileName"].toString());

QStringList nodes;
foreach (qSlicerFileReader* reader, readers)
{
QTime timeProbe;
timeProbe.start();
reader->userMessages()->ClearMessages();
reader->setMRMLScene(d->currentScene());
if (!reader->canLoadFile(parameters["fileName"].toString()))
{
Expand All @@ -494,8 +497,7 @@ bool qSlicerCoreIOManager::loadNodes(const qSlicerIO::IOFileType& fileType,
bool currentFileSuccess = reader->load(parameters);
if (userMessages)
{
QString prefix = parameters["fileName"].toString() + ": ";
userMessages->AddMessages(reader->userMessages(), prefix.toStdString());
userMessages->AddMessages(reader->userMessages(), userMessagePrefix.toStdString());
}
if (!currentFileSuccess)
{
Expand All @@ -511,6 +513,12 @@ bool qSlicerCoreIOManager::loadNodes(const qSlicerIO::IOFileType& fileType,
break;
}

if (!success && userMessages != nullptr && userMessages->GetNumberOfMessages() == numberOfUserMessagesBefore)
{
// Make sure that at least one message is logged if reading failed.
userMessages->AddMessage(vtkCommand::ErrorEvent, (QString(tr("%1 load failed.")).arg(userMessagePrefix)).toStdString());
}

loadedFileParameters.insert("nodeIDs", nodes);

emit newFileLoaded(loadedFileParameters);
Expand All @@ -536,16 +544,21 @@ bool qSlicerCoreIOManager::loadNodes(const qSlicerIO::IOFileType& fileType,
bool qSlicerCoreIOManager::loadNodes(const QList<qSlicerIO::IOProperties>& files,
vtkCollection* loadedNodes, vtkMRMLMessageCollection* userMessages/*=nullptr*/)
{
bool res = true;
bool success = true;
foreach(qSlicerIO::IOProperties fileProperties, files)
{
res = this->loadNodes(
int numberOfUserMessagesBefore = userMessages ? userMessages->GetNumberOfMessages() : 0;
success = this->loadNodes(
static_cast<qSlicerIO::IOFileType>(fileProperties["fileType"].toString()),
fileProperties, loadedNodes, userMessages)

&& res;
&& success;
// Add a separator between nodes
if (userMessages && userMessages->GetNumberOfMessages() > numberOfUserMessagesBefore)
{
userMessages->AddSeparator();
}
}
return res;
return success;
}

//-----------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion Base/QTCore/qSlicerSceneBundleReader.cxx
View file
Expand Up @@ -32,6 +32,7 @@
// MRML includes
#include <vtkMRMLScene.h>
#include <vtkMRMLStorageNode.h>
#include <vtkMRMLMessageCollection.h>

// MRML Logic includes
#include <vtkMRMLApplicationLogic.h>
Expand Down Expand Up @@ -89,8 +90,9 @@ bool qSlicerSceneBundleReader::load(const qSlicerIO::IOProperties& properties)
clear = properties["clear"].toBool();
}

this->mrmlScene()->SetErrorMessage("");
bool success = this->mrmlScene()->ReadFromMRB(file.toUtf8(), clear);

this->userMessages()->AddMessage(vtkCommand::ErrorEvent, this->mrmlScene()->GetErrorMessage());
if (success)
{
// Set default scene file format to mrb
Expand Down
24 changes: 19 additions & 5 deletions Base/QTGUI/qSlicerDataDialog.cxx
View file
Expand Up @@ -30,6 +30,7 @@
/// CTK includes
#include <ctkCheckableHeaderView.h>
#include <ctkCheckableModelHelper.h>
#include <ctkMessageBox.h>

/// Slicer includes
#include "vtkArchive.h"
Expand All @@ -40,6 +41,10 @@
#include "qSlicerDataDialog_p.h"
#include "qSlicerIOManager.h"
#include "qSlicerIOOptionsWidget.h"
#include "vtkMRMLMessageCollection.h"

/// VTK includes
#include <vtkNew.h>

//-----------------------------------------------------------------------------
qSlicerDataDialogPrivate::qSlicerDataDialogPrivate(QWidget* _parent)
Expand Down Expand Up @@ -579,21 +584,30 @@ bool qSlicerDataDialog::exec(const qSlicerIO::IOProperties& readerProperties)
}
}

bool res = false;
bool success = false;
if (d->exec() != QDialog::Accepted)
{
d->reset();
return res;
return success;
}
QList<qSlicerIO::IOProperties> files = d->selectedFiles();
for (int i = 0; i < files.count(); ++i)
{
files[i].unite(readerProperties);
}
res = qSlicerCoreApplication::application()->coreIOManager()
->loadNodes(files);
vtkNew<vtkMRMLMessageCollection> userMessages;
success = qSlicerCoreApplication::application()->coreIOManager()->loadNodes(files, nullptr, userMessages);
if (!success)
{
ctkMessageBox messageBox;
messageBox.setIcon(QMessageBox::Critical);
messageBox.setWindowTitle(tr("Adding data failed"));
messageBox.setText(tr("Error occurred while loading the selected files. Click 'Show details' button and check the application log for more information."));
messageBox.setDetailedText(QString::fromStdString(userMessages->GetAllMessagesAsString()));
messageBox.exec();
}
d->reset();
return res;
return success;
}

//-----------------------------------------------------------------------------
Expand Down
49 changes: 36 additions & 13 deletions Base/QTGUI/qSlicerIOManager.cxx
View file
Expand Up @@ -7,6 +7,7 @@
#include <QMetaProperty>
#include <QProgressDialog>
#include <QSettings>
#include <QTimer>

// CTK includes
#include "ctkScreenshotDialog.h"
Expand All @@ -24,6 +25,7 @@
/// MRML includes
#include <vtkMRMLNode.h>
#include <vtkMRMLScene.h>
#include <vtkMRMLMessageCollection.h>

/// VTK includes
#include <vtkCollection.h>
Expand Down Expand Up @@ -53,23 +55,25 @@ class qSlicerIOManagerPrivate
qSlicerFileDialog* findDialog(qSlicerIO::IOFileType fileType,
qSlicerFileDialog::IOAction)const;

QStringList History;
QList<QUrl> Favorites;
QList<qSlicerFileDialog*> ReadDialogs;
QList<qSlicerFileDialog*> WriteDialogs;
QStringList History;
QList<QUrl> Favorites;
QList<qSlicerFileDialog*> ReadDialogs;
QList<qSlicerFileDialog*> WriteDialogs;

QSharedPointer<ctkScreenshotDialog> ScreenshotDialog;
QProgressDialog* ProgressDialog;
QProgressDialog* ProgressDialog{nullptr};

/// File dialog that next call of execDelayedFileDialog signal will execute
qSlicerFileDialog* DelayedFileDialog{nullptr};
};

//-----------------------------------------------------------------------------
// qSlicerIOManagerPrivate methods

//-----------------------------------------------------------------------------
qSlicerIOManagerPrivate::qSlicerIOManagerPrivate(qSlicerIOManager& object)
:q_ptr(&object)
: q_ptr(&object)
{
this->ProgressDialog = nullptr;
}

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -336,13 +340,27 @@ void qSlicerIOManager::dropEvent(QDropEvent *event)
dialog->dropEvent(event);
if (event->isAccepted())
{
dialog->exec();
// If we immediately executed the dialog here then we would block the application
// that initiated the drag-and-drop operation. Therefore we return and open the dialog
// with a timer.
d->DelayedFileDialog = dialog;
QTimer::singleShot(0, this, SLOT(execDelayedFileDialog()));
break;
}
}
}
}

//-----------------------------------------------------------------------------
void qSlicerIOManager::execDelayedFileDialog()
{
Q_D(qSlicerIOManager);
if (d->DelayedFileDialog)
{
d->DelayedFileDialog->exec();
}
}

//-----------------------------------------------------------------------------
void qSlicerIOManager::addHistory(const QString& path)
{
Expand Down Expand Up @@ -436,18 +454,23 @@ bool qSlicerIOManager::loadNodes(const QList<qSlicerIO::IOProperties>& files,
Q_D(qSlicerIOManager);

bool needStop = d->startProgressDialog(files.count());
bool res = true;
bool success = true;
foreach(qSlicerIO::IOProperties fileProperties, files)
{
res = this->loadNodes(
int numberOfUserMessagesBefore = userMessages ? userMessages->GetNumberOfMessages() : 0;
success = this->loadNodes(
static_cast<qSlicerIO::IOFileType>(fileProperties["fileType"].toString()),
fileProperties,
loadedNodes, userMessages) && res;
loadedNodes, userMessages) && success;
if (userMessages && userMessages->GetNumberOfMessages() > numberOfUserMessagesBefore)
{
userMessages->AddSeparator();
}

this->updateProgressDialog();
if (d->ProgressDialog->wasCanceled())
{
res = false;
success = false;
break;
}
}
Expand All @@ -457,7 +480,7 @@ bool qSlicerIOManager::loadNodes(const QList<qSlicerIO::IOProperties>& files,
d->stopProgressDialog();
}

return res;
return success;
}

//-----------------------------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions Base/QTGUI/qSlicerIOManager.h
View file
Expand Up @@ -103,6 +103,7 @@ public slots:

protected slots:
void updateProgressDialog();
void execDelayedFileDialog();

protected:
friend class qSlicerFileDialog;
Expand Down
42 changes: 2 additions & 40 deletions Base/QTGUI/qSlicerSaveDataDialog.cxx
View file
Expand Up @@ -1351,48 +1351,10 @@ void qSlicerSaveDataDialogPrivate::updateStatusIconFromMessageCollection(int row
QString messagesStr;
bool warningFound = false;
bool errorFound = false;

if (userMessages && userMessages->GetNumberOfMessages() > 0)
if (userMessages)
{
// There is at least one message from the storage node.
const int numberOfMessages = userMessages->GetNumberOfMessages();

for (int index = 0; index < numberOfMessages; ++index)
{
const unsigned long messageType = userMessages->GetNthMessageType(index);
const std::string messageText = userMessages->GetNthMessageText(index);
if (!messageText.empty() && numberOfMessages > 1)
{
messagesStr += "- ";
}
switch (messageType)
{
case vtkCommand::WarningEvent:
warningFound = true;
if (!messageText.empty())
{
messagesStr.append(tr("Warning: "));
}
break;
case vtkCommand::ErrorEvent:
errorFound = true;
if (!messageText.empty())
{
messagesStr.append(tr("Error: "));
}
break;
}
if (!messageText.empty())
{
messagesStr.append(tr(messageText.c_str()));
if (index < numberOfMessages - 1)
{
messagesStr.append("\n");
}
}
}
messagesStr = QString::fromStdString(userMessages->GetAllMessagesAsString(&errorFound, &warningFound));
}

this->setStatusIcon(row, (!success || errorFound) ? this->ErrorIcon : (warningFound ? this->WarningIcon : QIcon()), messagesStr);
}

Expand Down

0 comments on commit c5f5d62

Please sign in to comment.