From c5f5d62ec8bb82eea381f17de893effea71677dd Mon Sep 17 00:00:00 2001 From: Andras Lasso Date: Sun, 7 Feb 2021 16:35:16 -0500 Subject: [PATCH] ENH: Display error message if node loading fails 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. --- Base/QTCore/qSlicerCoreIOManager.cxx | 27 ++- Base/QTCore/qSlicerSceneBundleReader.cxx | 4 +- Base/QTGUI/qSlicerDataDialog.cxx | 24 ++- Base/QTGUI/qSlicerIOManager.cxx | 49 +++-- Base/QTGUI/qSlicerIOManager.h | 1 + Base/QTGUI/qSlicerSaveDataDialog.cxx | 42 +---- .../Core/Testing/vtkMRMLStorageNodeTest1.cxx | 61 ++++-- Libs/MRML/Core/vtkMRMLMessageCollection.cxx | 129 +++++++++++++ Libs/MRML/Core/vtkMRMLMessageCollection.h | 67 +++++++ Libs/MRML/Core/vtkMRMLModelStorageNode.cxx | 43 ++++- Libs/MRML/Core/vtkMRMLScene.cxx | 64 +++++-- Libs/MRML/Core/vtkMRMLSequenceStorageNode.cxx | 31 +++- Libs/MRML/Core/vtkMRMLStorableNode.cxx | 25 ++- Libs/MRML/Core/vtkMRMLStorageNode.cxx | 174 +++++++++--------- .../MRML/vtkMRMLMarkupsJsonStorageNode.cxx | 157 ++++++++++------ .../Models/Logic/vtkSlicerModelsLogic.cxx | 106 ++++++----- .../Models/Logic/vtkSlicerModelsLogic.h | 16 +- .../Cxx/vtkSlicerModelsLogicAddFileTest.cxx | 3 + .../Loadable/Models/qSlicerModelsReader.cxx | 9 +- .../Logic/vtkSlicerSequencesLogic.cxx | 16 +- .../Sequences/Logic/vtkSlicerSequencesLogic.h | 9 +- .../Sequences/qSlicerSequencesReader.cxx | 5 +- 22 files changed, 748 insertions(+), 314 deletions(-) diff --git a/Base/QTCore/qSlicerCoreIOManager.cxx b/Base/QTCore/qSlicerCoreIOManager.cxx index 8937629cdc0..efd8c503e60 100644 --- a/Base/QTCore/qSlicerCoreIOManager.cxx +++ b/Base/QTCore/qSlicerCoreIOManager.cxx @@ -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())) { @@ -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) { @@ -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); @@ -536,16 +544,21 @@ bool qSlicerCoreIOManager::loadNodes(const qSlicerIO::IOFileType& fileType, bool qSlicerCoreIOManager::loadNodes(const QList& 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(fileProperties["fileType"].toString()), fileProperties, loadedNodes, userMessages) - - && res; + && success; + // Add a separator between nodes + if (userMessages && userMessages->GetNumberOfMessages() > numberOfUserMessagesBefore) + { + userMessages->AddSeparator(); + } } - return res; + return success; } //----------------------------------------------------------------------------- diff --git a/Base/QTCore/qSlicerSceneBundleReader.cxx b/Base/QTCore/qSlicerSceneBundleReader.cxx index 742b3445026..106d0817ba2 100644 --- a/Base/QTCore/qSlicerSceneBundleReader.cxx +++ b/Base/QTCore/qSlicerSceneBundleReader.cxx @@ -32,6 +32,7 @@ // MRML includes #include #include +#include // MRML Logic includes #include @@ -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 diff --git a/Base/QTGUI/qSlicerDataDialog.cxx b/Base/QTGUI/qSlicerDataDialog.cxx index 76baa0e8bab..bb2e22d532b 100644 --- a/Base/QTGUI/qSlicerDataDialog.cxx +++ b/Base/QTGUI/qSlicerDataDialog.cxx @@ -30,6 +30,7 @@ /// CTK includes #include #include +#include /// Slicer includes #include "vtkArchive.h" @@ -40,6 +41,10 @@ #include "qSlicerDataDialog_p.h" #include "qSlicerIOManager.h" #include "qSlicerIOOptionsWidget.h" +#include "vtkMRMLMessageCollection.h" + +/// VTK includes +#include //----------------------------------------------------------------------------- qSlicerDataDialogPrivate::qSlicerDataDialogPrivate(QWidget* _parent) @@ -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 files = d->selectedFiles(); for (int i = 0; i < files.count(); ++i) { files[i].unite(readerProperties); } - res = qSlicerCoreApplication::application()->coreIOManager() - ->loadNodes(files); + vtkNew 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; } //----------------------------------------------------------------------------- diff --git a/Base/QTGUI/qSlicerIOManager.cxx b/Base/QTGUI/qSlicerIOManager.cxx index cbfe8daff90..3d1ad2889e3 100644 --- a/Base/QTGUI/qSlicerIOManager.cxx +++ b/Base/QTGUI/qSlicerIOManager.cxx @@ -7,6 +7,7 @@ #include #include #include +#include // CTK includes #include "ctkScreenshotDialog.h" @@ -24,6 +25,7 @@ /// MRML includes #include #include +#include /// VTK includes #include @@ -53,13 +55,16 @@ class qSlicerIOManagerPrivate qSlicerFileDialog* findDialog(qSlicerIO::IOFileType fileType, qSlicerFileDialog::IOAction)const; - QStringList History; - QList Favorites; - QList ReadDialogs; - QList WriteDialogs; + QStringList History; + QList Favorites; + QList ReadDialogs; + QList WriteDialogs; QSharedPointer ScreenshotDialog; - QProgressDialog* ProgressDialog; + QProgressDialog* ProgressDialog{nullptr}; + + /// File dialog that next call of execDelayedFileDialog signal will execute + qSlicerFileDialog* DelayedFileDialog{nullptr}; }; //----------------------------------------------------------------------------- @@ -67,9 +72,8 @@ class qSlicerIOManagerPrivate //----------------------------------------------------------------------------- qSlicerIOManagerPrivate::qSlicerIOManagerPrivate(qSlicerIOManager& object) - :q_ptr(&object) + : q_ptr(&object) { - this->ProgressDialog = nullptr; } //----------------------------------------------------------------------------- @@ -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) { @@ -436,18 +454,23 @@ bool qSlicerIOManager::loadNodes(const QList& 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(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; } } @@ -457,7 +480,7 @@ bool qSlicerIOManager::loadNodes(const QList& files, d->stopProgressDialog(); } - return res; + return success; } //----------------------------------------------------------------------------- diff --git a/Base/QTGUI/qSlicerIOManager.h b/Base/QTGUI/qSlicerIOManager.h index 6ebaa32ab43..9517f129c0b 100644 --- a/Base/QTGUI/qSlicerIOManager.h +++ b/Base/QTGUI/qSlicerIOManager.h @@ -103,6 +103,7 @@ public slots: protected slots: void updateProgressDialog(); + void execDelayedFileDialog(); protected: friend class qSlicerFileDialog; diff --git a/Base/QTGUI/qSlicerSaveDataDialog.cxx b/Base/QTGUI/qSlicerSaveDataDialog.cxx index 715b0656fb7..cf76385475d 100644 --- a/Base/QTGUI/qSlicerSaveDataDialog.cxx +++ b/Base/QTGUI/qSlicerSaveDataDialog.cxx @@ -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); } diff --git a/Libs/MRML/Core/Testing/vtkMRMLStorageNodeTest1.cxx b/Libs/MRML/Core/Testing/vtkMRMLStorageNodeTest1.cxx index b9c0f711998..3a18c4a930e 100644 --- a/Libs/MRML/Core/Testing/vtkMRMLStorageNodeTest1.cxx +++ b/Libs/MRML/Core/Testing/vtkMRMLStorageNodeTest1.cxx @@ -21,6 +21,16 @@ #include #include +namespace +{ + enum TestReadReferenceType + { + NullptrAsReference, + TransformNodeAsReference, + ModelNodeAsReference + }; +} + //--------------------------------------------------------------------------- class vtkMRMLStorageNodeTestHelper1 : public vtkMRMLStorageNode { @@ -83,7 +93,7 @@ int TestBasics() } //--------------------------------------------------------------------------- -int TestReadData(int referenceNodeType, +int TestReadData(TestReadReferenceType referenceNodeType, const char* supportedClass, int readDataReturn, int expectedRes) @@ -94,13 +104,16 @@ int TestReadData(int referenceNodeType, storageNode->SetFileName("file.ext"); vtkNew transformNode; vtkNew modelNode; - vtkMRMLNode* referenceNode = (referenceNodeType == 0 ? vtkMRMLNode::SafeDownCast(nullptr): - (referenceNodeType == 1 ? vtkMRMLNode::SafeDownCast(transformNode.GetPointer()) : - vtkMRMLNode::SafeDownCast(modelNode.GetPointer()))); + vtkMRMLNode* referenceNode = nullptr; + switch (referenceNodeType) + { + case NullptrAsReference: referenceNode = nullptr; break; + case TransformNodeAsReference: referenceNode = transformNode; break; + case ModelNodeAsReference: referenceNode = modelNode; break; + } int res = storageNode->ReadData(referenceNode); std::cout << "StoredTime: " << storageNode->GetStoredTime() << std::endl; CHECK_INT(res, expectedRes); - return EXIT_SUCCESS; } @@ -108,29 +121,43 @@ int TestReadData(int referenceNodeType, int TestReadData() { TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); - CHECK_EXIT_SUCCESS(TestReadData(0, "invalid", 0, 0)); + CHECK_EXIT_SUCCESS(TestReadData(NullptrAsReference, "invalid", /*readDataResult=*/ 0, /*success=*/ 0)); TESTING_OUTPUT_ASSERT_ERRORS_END(); TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); - CHECK_EXIT_SUCCESS(TestReadData(0, "invalid", 1, 0)); + CHECK_EXIT_SUCCESS(TestReadData(NullptrAsReference, "invalid", /*readDataResult=*/ 1, /*success=*/ 0)); TESTING_OUTPUT_ASSERT_ERRORS_END(); TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); - CHECK_EXIT_SUCCESS(TestReadData(0, "vtkMRMLModelNode", 0, 0)); + CHECK_EXIT_SUCCESS(TestReadData(NullptrAsReference, "vtkMRMLModelNode", /*readDataResult=*/ 0, /*success=*/ 0)); TESTING_OUTPUT_ASSERT_ERRORS_END(); TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); - CHECK_EXIT_SUCCESS(TestReadData(0, "vtkMRMLModelNode", 1, 0)); + CHECK_EXIT_SUCCESS(TestReadData(NullptrAsReference, "vtkMRMLModelNode", /*readDataResult=*/ 1, /*success=*/ 0)); TESTING_OUTPUT_ASSERT_ERRORS_END(); - CHECK_EXIT_SUCCESS(TestReadData(1, "invalid", 0, 0)); - CHECK_EXIT_SUCCESS(TestReadData(1, "invalid", 1, 0)); - CHECK_EXIT_SUCCESS(TestReadData(1, "vtkMRMLModelNode", 0, 0)); - CHECK_EXIT_SUCCESS(TestReadData(1, "vtkMRMLModelNode", 1, 0)); - CHECK_EXIT_SUCCESS(TestReadData(2, "invalid", 0, 0)); - CHECK_EXIT_SUCCESS(TestReadData(2, "invalid", 1, 0)); - CHECK_EXIT_SUCCESS(TestReadData(2, "vtkMRMLModelNode", 0, 0)); - CHECK_EXIT_SUCCESS(TestReadData(2, "vtkMRMLModelNode", 1, 1)); + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); + CHECK_EXIT_SUCCESS(TestReadData(TransformNodeAsReference, "invalid", /*readDataResult=*/ 0, /*success=*/ 0)); + TESTING_OUTPUT_ASSERT_ERRORS_END(); + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); + CHECK_EXIT_SUCCESS(TestReadData(TransformNodeAsReference, "invalid", /*readDataResult=*/ 1, /*success=*/ 0)); + TESTING_OUTPUT_ASSERT_ERRORS_END(); + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); + CHECK_EXIT_SUCCESS(TestReadData(TransformNodeAsReference, "vtkMRMLModelNode", /*readDataResult=*/ 0, /*success=*/ 0)); + TESTING_OUTPUT_ASSERT_ERRORS_END(); + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); + CHECK_EXIT_SUCCESS(TestReadData(TransformNodeAsReference, "vtkMRMLModelNode", /*readDataResult=*/ 1, /*success=*/ 0)); + TESTING_OUTPUT_ASSERT_ERRORS_END(); + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); + CHECK_EXIT_SUCCESS(TestReadData(ModelNodeAsReference, "invalid", /*readDataResult=*/ 0, /*success=*/ 0)); + TESTING_OUTPUT_ASSERT_ERRORS_END(); + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); + CHECK_EXIT_SUCCESS(TestReadData(ModelNodeAsReference, "invalid", /*readDataResult=*/ 1, /*success=*/ 0)); + TESTING_OUTPUT_ASSERT_ERRORS_END(); + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); + CHECK_EXIT_SUCCESS(TestReadData(ModelNodeAsReference, "vtkMRMLModelNode", /*readDataResult=*/ 0, /*success=*/ 0)); + TESTING_OUTPUT_ASSERT_ERRORS_END(); + CHECK_EXIT_SUCCESS(TestReadData(ModelNodeAsReference, "vtkMRMLModelNode", /*readDataResult=*/ 1, /*success=*/ 1)); return EXIT_SUCCESS; } diff --git a/Libs/MRML/Core/vtkMRMLMessageCollection.cxx b/Libs/MRML/Core/vtkMRMLMessageCollection.cxx index c2ac792d608..6f38771b9e3 100644 --- a/Libs/MRML/Core/vtkMRMLMessageCollection.cxx +++ b/Libs/MRML/Core/vtkMRMLMessageCollection.cxx @@ -23,6 +23,11 @@ // MRML includes #include "vtkCommand.h" +namespace +{ + const int SEPARATOR_MESSAGE_TYPE = vtkCommand::PropertyModifiedEvent; +} + //---------------------------------------------------------------------------- vtkStandardNewMacro(vtkMRMLMessageCollection); @@ -105,6 +110,12 @@ ::AddMessage(unsigned long messageType, const std::string &messageText) this->Messages.push_back({messageType, messageText}); } +//---------------------------------------------------------------------------- +void vtkMRMLMessageCollection::AddSeparator() +{ + this->AddMessage(SEPARATOR_MESSAGE_TYPE, "\n--------\n"); +} + //---------------------------------------------------------------------------- void vtkMRMLMessageCollection @@ -116,11 +127,15 @@ ::ClearMessages() //---------------------------------------------------------------------------- vtkMRMLMessageCollection::vtkMRMLMessageCollection() { + this->CallbackCommand = vtkSmartPointer::New(); + this->CallbackCommand->SetCallback(vtkMRMLMessageCollection::CallbackFunction); + this->CallbackCommand->SetClientData(this); } //---------------------------------------------------------------------------- vtkMRMLMessageCollection::~vtkMRMLMessageCollection() { + this->SetObservedObject(nullptr); } //---------------------------------------------------------------------------- @@ -147,3 +162,117 @@ void vtkMRMLMessageCollection::AddMessages(vtkMRMLMessageCollection* source, con this->AddMessage(source->GetNthMessageType(i), prefix + source->GetNthMessageText(i)); } } + +//---------------------------------------------------------------------------- +std::string vtkMRMLMessageCollection::GetAllMessagesAsString(bool* errorFoundPtr/*=nullptr*/, bool* warningFoundPtr/*=nullptr*/) +{ + std::string messagesStr; + bool warningFound = false; + bool errorFound = false; + + // Check if we need to display bullet-point list + // (yes, if there are at least two non-separator messages) + bool showAsBulletPointList = false; + int numberOfNonSeparatorMessages = 0; + const int numberOfMessages = this->GetNumberOfMessages(); + for (int index = 0; index < numberOfMessages; ++index) + { + const unsigned long messageType = this->GetNthMessageType(index); + if (messageType != SEPARATOR_MESSAGE_TYPE) + { + numberOfNonSeparatorMessages++; + } + if (numberOfNonSeparatorMessages >= 2) + { + showAsBulletPointList = true; + break; + } + } + + // There is at least one message from the storage node. + for (int index = 0; index < numberOfMessages; ++index) + { + const unsigned long messageType = this->GetNthMessageType(index); + const std::string messageText = this->GetNthMessageText(index); + if (messageType == SEPARATOR_MESSAGE_TYPE) + { + // do not print separator at the end of the message list + if (index == numberOfMessages - 1) + { + continue; + } + } + else if (!messageText.empty() && showAsBulletPointList) + { + messagesStr += "- "; + } + switch (messageType) + { + case vtkCommand::WarningEvent: + warningFound = true; + if (!messageText.empty()) + { + messagesStr.append("Warning: "); + } + break; + case vtkCommand::ErrorEvent: + errorFound = true; + if (!messageText.empty()) + { + messagesStr.append("Error: "); + } + break; + } + if (!messageText.empty()) + { + messagesStr.append(messageText.c_str()); + messagesStr.append("\n"); + } + } + + if (errorFoundPtr) + { + *errorFoundPtr = errorFound; + } + if (warningFoundPtr) + { + *warningFoundPtr = warningFound; + } + return messagesStr; +} + +//---------------------------------------------------------------------------- +void vtkMRMLMessageCollection::CallbackFunction(vtkObject* vtkNotUsed(caller), + long unsigned int eventId, void* clientData, void* callData) +{ + vtkMRMLMessageCollection* self = reinterpret_cast(clientData); + if (!self || !callData) + { + return; + } + std::string msg = reinterpret_cast(callData); + const std::string chars = "\t\n\v\f\r "; + msg.erase(msg.find_last_not_of(chars) + 1); + self->AddMessage(eventId, msg); +} + +//---------------------------------------------------------------------------- +void vtkMRMLMessageCollection::SetObservedObject(vtkObject* observedObject) +{ + if (observedObject == this->ObservedObject) + { + // no change + return; + } + if (this->ObservedObject) + { + this->ObservedObject->RemoveObservers(vtkCommand::ErrorEvent, this->CallbackCommand); + this->ObservedObject->RemoveObservers(vtkCommand::WarningEvent, this->CallbackCommand); + } + this->ObservedObject = observedObject; + if (this->ObservedObject) + { + this->ObservedObject->AddObserver(vtkCommand::ErrorEvent, this->CallbackCommand); + this->ObservedObject->AddObserver(vtkCommand::WarningEvent, this->CallbackCommand); + } +} diff --git a/Libs/MRML/Core/vtkMRMLMessageCollection.h b/Libs/MRML/Core/vtkMRMLMessageCollection.h index bcb460f2e6d..9568b0ece28 100644 --- a/Libs/MRML/Core/vtkMRMLMessageCollection.h +++ b/Libs/MRML/Core/vtkMRMLMessageCollection.h @@ -24,12 +24,61 @@ #include "vtkMRML.h" // VTK includes +#include #include // STD includes #include #include +//@{ +/// Macros to log warning or error message (for developers) and also +/// add a message to the input message collection (to be displayed +/// to the user later). + +#ifndef vtkWarningToMessageCollectionMacro +#define vtkWarningToMessageCollectionMacro(messageCollection, devMsgPrefix, userMsg) \ + vtkWarningToMessageCollectionWithObjectMacro(this, messageCollection, devMsgPrefix, userMsg) +#endif + +#ifndef vtkErrorToMessageCollectionMacro +#define vtkErrorToMessageCollectionMacro(messageCollection, devMsgPrefix, userMsg) \ + vtkErrorToMessageCollectionWithObjectMacro(this, messageCollection, devMsgPrefix, userMsg) +#endif + +#ifndef vtkWarningToMessageCollectionWithObjectMacro +#define vtkWarningToMessageCollectionWithObjectMacro(self, messageCollection, devMsgPrefix, userMsg) \ + { \ + vtkOStreamWrapper::EndlType endl; \ + vtkOStreamWrapper::UseEndl(endl); \ + if (messageCollection) \ + { \ + vtkOStrStreamWrapper userMsgStream; \ + userMsgStream << userMsg; \ + messageCollection->AddMessage(vtkCommand::WarningEvent, userMsgStream.str()); \ + userMsgStream.rdbuf()->freeze(0); \ + } \ + vtkWarningWithObjectMacro(self, << devMsgPrefix << ": " << userMsg); \ + } +#endif + +#ifndef vtkErrorToMessageCollectionWithObjectMacro +#define vtkErrorToMessageCollectionWithObjectMacro(self, messageCollection, devMsgPrefix, userMsg) \ + { \ + vtkOStreamWrapper::EndlType endl; \ + vtkOStreamWrapper::UseEndl(endl); \ + if (messageCollection) \ + { \ + vtkOStrStreamWrapper userMsgStream; \ + userMsgStream << userMsg; \ + messageCollection->AddMessage(vtkCommand::ErrorEvent, userMsgStream.str()); \ + userMsgStream.rdbuf()->freeze(0); \ + } \ + vtkErrorWithObjectMacro(self, << devMsgPrefix << ": " << userMsg); \ + } +#endif +//@} + /// /// A class for recording warnings and errors associated with this /// vtkMRMLStorableNode. A substantially similar vtkMessageCollection @@ -60,6 +109,9 @@ class VTK_MRML_EXPORT vtkMRMLMessageCollection : public vtkObject /// Append a message to the message vector virtual void AddMessage(unsigned long messageType, const std::string &messageText); + /// Add a separator, for example to create message groups + virtual void AddSeparator(); + /// Copy all messages from another collection. /// If prefix is specified then that string is prepended to each copied message. virtual void AddMessages(vtkMRMLMessageCollection* source, const std::string& prefix=std::string()); @@ -70,6 +122,16 @@ class VTK_MRML_EXPORT vtkMRMLMessageCollection : public vtkObject /// Copy all messages from another collection. virtual void DeepCopy(vtkMRMLMessageCollection* source); + /// Return all messages in a single formatted string. + /// If optional errorFound or warningFound pointers are set then the caller get information + /// about presence of warnings or errors in the message list. + virtual std::string GetAllMessagesAsString(bool* errorFound=nullptr, bool* warningFound=nullptr); + + /// Observe error and warnings reported by observedObject. + /// For example, this can be used to capture errors from VTK classes + /// and display them to the user. + virtual void SetObservedObject(vtkObject* observedObject); + protected: vtkMRMLMessageCollection(); ~vtkMRMLMessageCollection() override; @@ -86,6 +148,11 @@ class VTK_MRML_EXPORT vtkMRMLMessageCollection : public vtkObject std::string MessageText; }; + static void CallbackFunction(vtkObject*, long unsigned int, + void* clientData, void* callData); + + vtkSmartPointer ObservedObject; + vtkSmartPointer CallbackCommand; std::vector Messages; }; diff --git a/Libs/MRML/Core/vtkMRMLModelStorageNode.cxx b/Libs/MRML/Core/vtkMRMLModelStorageNode.cxx index 052628a3d31..46ffc906a2f 100644 --- a/Libs/MRML/Core/vtkMRMLModelStorageNode.cxx +++ b/Libs/MRML/Core/vtkMRMLModelStorageNode.cxx @@ -12,9 +12,11 @@ =========================================================================auto=*/ +#include "vtkMRMLModelStorageNode.h" + #include "vtkMRMLDisplayNode.h" +#include "vtkMRMLMessageCollection.h" #include "vtkMRMLModelNode.h" -#include "vtkMRMLModelStorageNode.h" #include "vtkMRMLScene.h" // VTK includes @@ -197,8 +199,10 @@ int vtkMRMLModelStorageNode::ReadDataInternal(vtkMRMLNode *refNode) if (extension == std::string(".g") || extension == std::string(".byu")) { vtkNew reader; + this->GetUserMessages()->SetObservedObject(reader); reader->SetGeometryFileName(fullName.c_str()); reader->Update(); + this->GetUserMessages()->SetObservedObject(nullptr); meshFromFile = reader->GetOutput(); } else if (extension == std::string(".vtk")) @@ -217,8 +221,10 @@ int vtkMRMLModelStorageNode::ReadDataInternal(vtkMRMLNode *refNode) reader->ReadAllColorScalarsOn(); reader->ReadAllTCoordsOn(); reader->ReadAllFieldsOn(); + this->GetUserMessages()->SetObservedObject(reader); reader->Update(); meshFromFile = reader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); } else if (unstructuredGridReader->IsFileUnstructuredGrid()) { @@ -229,53 +235,66 @@ int vtkMRMLModelStorageNode::ReadDataInternal(vtkMRMLNode *refNode) unstructuredGridReader->ReadAllColorScalarsOn(); unstructuredGridReader->ReadAllTCoordsOn(); unstructuredGridReader->ReadAllFieldsOn(); + this->GetUserMessages()->SetObservedObject(unstructuredGridReader); unstructuredGridReader->Update(); meshFromFile = unstructuredGridReader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); } else { - vtkErrorMacro("ReadDataInternal (" << (this->ID ? this->ID : "(unknown)") << "): file " << fullName.c_str() - << " is not recognized as polydata nor as an unstructured grid."); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLModelStorageNode::ReadDataInternal", + "Failed to load model from VTK file " << fullName << " as it does not contain polydata nor unstructured grid." + << " The file might be loadable as a volume."); } coordinateSystemInFileHeader = vtkMRMLModelStorageNode::GetCoordinateSystemFromFileHeader(reader->GetHeader()); } else if (extension == std::string(".vtp")) { vtkNew reader; + this->GetUserMessages()->SetObservedObject(reader); reader->SetFileName(fullName.c_str()); reader->Update(); meshFromFile = reader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); coordinateSystemInFileHeader = vtkMRMLModelStorageNode::GetCoordinateSystemFromFieldData(meshFromFile); } else if (extension == std::string(".ucd")) { vtkNew reader; + this->GetUserMessages()->SetObservedObject(reader); reader->SetFileName(fullName.c_str()); reader->Update(); meshFromFile = reader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); } else if (extension == std::string(".vtu")) { vtkNew reader; + this->GetUserMessages()->SetObservedObject(reader); reader->SetFileName(fullName.c_str()); reader->Update(); meshFromFile = reader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); coordinateSystemInFileHeader = vtkMRMLModelStorageNode::GetCoordinateSystemFromFieldData(meshFromFile); } else if (extension == std::string(".stl")) { vtkNew reader; + this->GetUserMessages()->SetObservedObject(reader); reader->SetFileName(fullName.c_str()); reader->Update(); meshFromFile = reader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); coordinateSystemInFileHeader = vtkMRMLModelStorageNode::GetCoordinateSystemFromFileHeader(reader->GetHeader()); } else if (extension == std::string(".ply")) { vtkNew reader; + this->GetUserMessages()->SetObservedObject(reader); reader->SetFileName(fullName.c_str()); reader->Update(); meshFromFile = reader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); vtkStringArray* comments = reader->GetComments(); for (int commentIndex = 0; commentIndex < comments->GetNumberOfValues(); commentIndex++) { @@ -290,9 +309,11 @@ int vtkMRMLModelStorageNode::ReadDataInternal(vtkMRMLNode *refNode) else if (extension == std::string(".obj")) { vtkNew reader; + this->GetUserMessages()->SetObservedObject(reader); reader->SetFileName(fullName.c_str()); reader->Update(); meshFromFile = reader->GetOutput(); + this->GetUserMessages()->SetObservedObject(nullptr); coordinateSystemInFileHeader = vtkMRMLModelStorageNode::GetCoordinateSystemFromFileHeader(reader->GetComment()); } else if (extension == std::string(".meta")) // model in meta format @@ -313,7 +334,8 @@ int vtkMRMLModelStorageNode::ReadDataInternal(vtkMRMLNode *refNode) } catch(itk::ExceptionObject &ex) { - std::cout<GetUserMessages(), "vtkMRMLModelStorageNode::ReadDataInternal", + "Failed to load model from ITK .meta file " << fullName << ": " << ex.GetDescription()); return 0; } vtkNew vtkMesh; @@ -356,14 +378,21 @@ int vtkMRMLModelStorageNode::ReadDataInternal(vtkMRMLNode *refNode) } else { - vtkDebugMacro("ReadDataInternal (" << (this->ID ? this->ID : "(unknown)") - << "): Cannot read model file '" << fullName.c_str() << "' (extension = " << extension.c_str() << ")"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLModelStorageNode::ReadDataInternal", + "Failed to load model: unrecognized file extension '" << extension << "' of file '" << fullName << "'."); return 0; } } catch (...) { - vtkErrorMacro("ReadDataInternal (" << (this->ID ? this->ID : "(unknown)") << "): unknown exception while trying to read file: " << fullName.c_str()); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLModelStorageNode::ReadDataInternal", + "Failed to load model: unknown exception while trying to load the file '" << fullName << "'."); + return 0; + } + + if (this->GetUserMessages()->GetNumberOfMessagesOfType(vtkCommand::ErrorEvent) > 0) + { + // User messages are already logged, no need for logging more return 0; } diff --git a/Libs/MRML/Core/vtkMRMLScene.cxx b/Libs/MRML/Core/vtkMRMLScene.cxx index a9e70658ec8..b7099fdcc62 100644 --- a/Libs/MRML/Core/vtkMRMLScene.cxx +++ b/Libs/MRML/Core/vtkMRMLScene.cxx @@ -766,7 +766,11 @@ int vtkMRMLScene::Import() timer->StartTimer(); #endif this->SetErrorCode(0); - this->SetErrorMessage(std::string("")); + this->SetErrorMessage(""); + // Scene error messages are overwritten by each node, we use errorMessages variable + // to accumulate all the messages. + std::string errorMessages; + bool success = true; bool undoFlag = this->GetUndoFlag(); @@ -775,11 +779,8 @@ int vtkMRMLScene::Import() this->ReferencedIDChanges.clear(); // read nodes into a temp scene - vtkSmartPointer loadedNodes = vtkSmartPointer::New(); - - int parsingSuccess = this->LoadIntoScene(loadedNodes); - - if (parsingSuccess) + vtkNew loadedNodes; + if (this->LoadIntoScene(loadedNodes)) { /// In case the scene needs to change the ID of some nodes to add, the new /// ID should not be one already existing in the scene nor one of the @@ -814,6 +815,14 @@ int vtkMRMLScene::Import() this->UpdateNodeReferences(addedNodes); this->RemoveReservedIDs(); + if (this->GetErrorCode() != 0) + { + success = false; + errorMessages.append(this->GetErrorMessage()+"\n"); + this->SetErrorCode(0); + this->SetErrorMessage(""); + } + this->InvokeEvent(vtkMRMLScene::NewSceneEvent, nullptr); // Notify the imported nodes about that all nodes are created @@ -829,14 +838,17 @@ int vtkMRMLScene::Import() { node->UpdateScene(this); } - if (this->GetErrorCode() == 1) + if (this->GetErrorCode() != 0) { //vtkErrorMacro("Import: error updating node " << node->GetID()); // TODO: figure out the best way to deal with an error (encountering // it when fail to read a file), removing a node isn't quite right // (nodes are still in the scene when save it later) // this->RemoveNode(node); - // this->SetErrorCode(0); + success = false; + errorMessages.append(this->GetErrorMessage() + "\n"); + this->SetErrorCode(0); + this->SetErrorMessage(""); } } @@ -849,7 +861,9 @@ int vtkMRMLScene::Import() else { // parsing was not successful - this->SetErrorMessage (std::string("Error parsing scene file")); + success = false; + errorMessages.append("Error parsing scene file."); + errorMessages.append("\n"); this->ReferencedIDChanges.clear(); } @@ -864,12 +878,6 @@ int vtkMRMLScene::Import() importingTimer->StopTimer(); #endif - int returnCode = parsingSuccess; // nonzero = success - if (this->GetErrorCode() != 0) - { - // error was reported, return with 0 (failure) - returnCode = 0; - } #ifdef MRMLSCENE_VERBOSE timer->StopTimer(); std::cerr << "vtkMRMLScene::Import()::AddNodes:" << addNodesTimer->GetElapsedTime() << std::endl; @@ -886,7 +894,15 @@ int vtkMRMLScene::Import() // Once the import is finished, give the SH a chance to ensure consistency this->SetSubjectHierarchyNode(vtkMRMLSubjectHierarchyNode::ResolveSubjectHierarchy(this)); - return returnCode; + if (this->GetErrorCode()) + { + success = false; + errorMessages.append(this->GetErrorMessage()+"\n"); + } + + this->SetErrorCode(success ? 0 : 1); + this->SetErrorMessage(errorMessages); + return success ? 1 : 0; } //------------------------------------------------------------------------------ @@ -3798,6 +3814,11 @@ bool vtkMRMLScene::ReadFromMRB(const char* fullName, bool clear/*=false*/) { tempBaseDir = this->GetDataIOManager()->GetCacheManager()->GetRemoteCacheDirectory(); } + else + { + vtkErrorMacro("vtkMRMLScene::ReadFromMRB failed: cannot retrieve remote cache directory from DataIOManager"); + return false; + } std::stringstream unpackDirStr; unpackDirStr << tempBaseDir << "/" << vtksys::SystemTools::GetCurrentDateTime("__BundleLoadTemp-%F_%H%M%S_") << (this->RandomGenerator() % 1000); std::string unpackDir = unpackDirStr.str(); @@ -3807,12 +3828,22 @@ bool vtkMRMLScene::ReadFromMRB(const char* fullName, bool clear/*=false*/) { if (!vtksys::SystemTools::RemoveADirectory(unpackDir.c_str())) { + std::string msg = std::string("Cannot remove directory '") + unpackDir + "'." + + " Check that remote cache directory that is specified in application setting is writeable."; + vtkErrorMacro("vtkMRMLScene::ReadFromMRB failed: " << msg); + this->SetErrorCode(1); + this->SetErrorMessage(msg); return false; } } if (!vtksys::SystemTools::MakeDirectory(unpackDir.c_str())) { + std::string msg = std::string("Cannot create directory '") + unpackDir + "'." + + " Check that remote cache directory that is specified in application setting is writeable."; + vtkErrorMacro("vtkMRMLScene::ReadFromMRB failed: " << msg); + this->SetErrorCode(1); + this->SetErrorMessage(msg); return false; } @@ -3829,6 +3860,7 @@ bool vtkMRMLScene::ReadFromMRB(const char* fullName, bool clear/*=false*/) } if (!vtksys::SystemTools::RemoveADirectory(unpackDir)) { + vtkErrorMacro("vtkMRMLScene::ReadFromMRB failed: cannot remove directory '" << unpackDir << "'"); return false; } diff --git a/Libs/MRML/Core/vtkMRMLSequenceStorageNode.cxx b/Libs/MRML/Core/vtkMRMLSequenceStorageNode.cxx index 1dd5da720ae..3d7ddc73fce 100644 --- a/Libs/MRML/Core/vtkMRMLSequenceStorageNode.cxx +++ b/Libs/MRML/Core/vtkMRMLSequenceStorageNode.cxx @@ -15,8 +15,10 @@ limitations under the License. ==============================================================================*/ -#include "vtkMRMLSequenceNode.h" #include "vtkMRMLSequenceStorageNode.h" + +#include "vtkMRMLMessageCollection.h" +#include "vtkMRMLSequenceNode.h" #include "vtkMRMLScene.h" // VTK includes @@ -65,16 +67,18 @@ int vtkMRMLSequenceStorageNode::ReadDataInternal(vtkMRMLNode *refNode) vtkMRMLSequenceNode *sequenceNode = dynamic_cast (refNode); std::string fullName = this->GetFullNameFromFileName(); - if (fullName == std::string("")) + if (fullName.empty()) { - vtkErrorMacro("ReadDataInternal: File name not specified"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLSequenceStorageNode::ReadDataInternal", + "Reading sequence node file failed: file name not specified."); return 0; } // check that the file exists if (vtksys::SystemTools::FileExists(fullName.c_str()) == false) { - vtkErrorMacro("ReadDataInternal: model file '" << fullName.c_str() << "' not found."); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLSequenceStorageNode::ReadDataInternal", + "Reading sequence node file failed: file '" << fullName << "' not found."); return 0; } @@ -82,7 +86,8 @@ int vtkMRMLSequenceStorageNode::ReadDataInternal(vtkMRMLNode *refNode) std::string extension = vtkMRMLStorageNode::GetLowercaseExtensionFromFileName(fullName); if( extension.empty() ) { - vtkErrorMacro("ReadData: no file extension specified: " << fullName.c_str()); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLSequenceStorageNode::ReadDataInternal", + "Reading sequence node file failed: no file extension specified in filename '" << fullName << "'"); return 0; } @@ -93,16 +98,20 @@ int vtkMRMLSequenceStorageNode::ReadDataInternal(vtkMRMLNode *refNode) if (this->GetScene() && sequenceNode->GetSequenceScene()) { this->GetScene()->CopyRegisteredNodesToScene(sequenceNode->GetSequenceScene()); + // Data IO manager is needed so that we can get a remote cache data directory for temporary storage + sequenceNode->GetSequenceScene()->SetDataIOManager(this->GetScene()->GetDataIOManager()); } else { - vtkErrorMacro("Cannot register nodes in the sequence node"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLSequenceStorageNode::ReadDataInternal", + "Reading sequence node file failed: invalid scene"); } int success = false; if (extension == std::string(".mrb")) { vtkMRMLScene* sequenceScene = sequenceNode->GetSequenceScene(); + sequenceScene->SetErrorMessage(""); success = sequenceScene->ReadFromMRB(fullName.c_str()); if (success) { @@ -126,6 +135,16 @@ int vtkMRMLSequenceStorageNode::ReadDataInternal(vtkMRMLNode *refNode) sequenceScene->RemoveNode(embeddedSequenceNode); } } + else + { + // Error is already logged but if a user message is set in the scene then + // we add it as a user message to show it to the user. + std::string errorMessage = sequenceScene->GetErrorMessage(); + if (!errorMessage.empty()) + { + this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, errorMessage); + } + } } else { diff --git a/Libs/MRML/Core/vtkMRMLStorableNode.cxx b/Libs/MRML/Core/vtkMRMLStorableNode.cxx index 4d3d26c5e63..e2eb81474f9 100644 --- a/Libs/MRML/Core/vtkMRMLStorableNode.cxx +++ b/Libs/MRML/Core/vtkMRMLStorableNode.cxx @@ -13,6 +13,7 @@ Version: $Revision: 1.3 $ =========================================================================auto=*/ // MRML includes +#include "vtkMRMLMessageCollection.h" #include "vtkMRMLStorableNode.h" #include "vtkMRMLScene.h" #include "vtkMRMLSequenceStorageNode.h" @@ -276,8 +277,10 @@ void vtkMRMLStorableNode::UpdateScene(vtkMRMLScene *scene) return; } - int numStorageNodes = this->GetNumberOfNodeReferences(this->GetStorageNodeReferenceRole()); + bool success = true; + std::string errorMessages; + int numStorageNodes = this->GetNumberOfNodeReferences(this->GetStorageNodeReferenceRole()); vtkDebugMacro("UpdateScene: going through the storage node ids: " << numStorageNodes); for (int i=0; i < numStorageNodes; i++) { @@ -296,11 +299,20 @@ void vtkMRMLStorableNode::UpdateScene(vtkMRMLScene *scene) fname = std::string(pnode->GetURI()); } vtkDebugMacro("UpdateScene: calling ReadData, fname = " << fname.c_str()); + pnode->GetUserMessages()->ClearMessages(); if (pnode->ReadData(this) == 0) { - scene->SetErrorCode(1); - std::string msg = std::string("Error reading file ") + fname; - scene->SetErrorMessage(msg); + success = false; + std::string msg = std::string("Failed to read node ") + (this->GetName() ? this->GetName() : "(null)") + + " (" + (this->GetID() ? this->GetID() : "(null)") + ") using storage node " + + (pnode->GetID() ? pnode->GetID() : "(null)") + "."; + vtkErrorMacro("vtkMRMLStorableNode::UpdateScene failed: " << msg); + errorMessages += msg; + std::string details = pnode->GetUserMessages()->GetAllMessagesAsString(); + if (!details.empty()) + { + errorMessages += " Details:\n" + details; + } } else { @@ -312,6 +324,11 @@ void vtkMRMLStorableNode::UpdateScene(vtkMRMLScene *scene) vtkErrorMacro("UpdateScene: error getting " << i << "th storage node, id = " << (this->GetNthStorageNodeID(i) == nullptr ? "null" : this->GetNthStorageNodeID(i))); } } + if (!success) + { + scene->SetErrorCode(1); + scene->SetErrorMessage(errorMessages); + } } vtkMRMLStorageNode* vtkMRMLStorableNode::GetNthStorageNode(int n) diff --git a/Libs/MRML/Core/vtkMRMLStorageNode.cxx b/Libs/MRML/Core/vtkMRMLStorageNode.cxx index 894205b0768..3dc96e58097 100644 --- a/Libs/MRML/Core/vtkMRMLStorageNode.cxx +++ b/Libs/MRML/Core/vtkMRMLStorageNode.cxx @@ -150,9 +150,8 @@ void vtkMRMLStorageNode::WriteXML(ostream& of, int nIndent) } else { - vtkWarningMacro("WriteXML: unable to convert relative file path to absolute, still using " << this->FileName); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("WriteXML: unable to convert relative file path to absolute, still using ") + this->FileName); + vtkWarningToMessageCollectionMacro(this->GetUserMessages(), "WriteXML", + "Unable to convert relative file path to absolute, still using " << this->FileName); } } } @@ -177,9 +176,8 @@ void vtkMRMLStorageNode::WriteXML(ostream& of, int nIndent) } else { - vtkWarningMacro("WriteXML: unable to convert relative file path to absolute, still using " << this->GetNthFileName(i)); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("WriteXML: unable to convert relative file path to absolute, still using ") + this->GetNthFileName(i)); + vtkWarningToMessageCollectionMacro(this->GetUserMessages(), "WriteXML", + "Unable to convert relative file path to absolute, still using " << this->GetNthFileName(i)); } } } @@ -505,9 +503,8 @@ void vtkMRMLStorageNode::StageReadData ( vtkMRMLNode *refNode ) } else { - vtkErrorMacro("StageReadData: unable to get a URI handler for " << this->URI << ", resetting stage to idle"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("StageReadData: unable to get a URI handler for ") + this->URI + ", resetting stage to idle"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::StageReadData", + "Unable to get a URI handler for " << this->URI << ", resetting stage to idle"); this->SetReadStateIdle(); return; } @@ -521,9 +518,8 @@ void vtkMRMLStorageNode::StageReadData ( vtkMRMLNode *refNode ) } else { - vtkWarningMacro("StageReadData: No IO Manager on the scene, returning."); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("StageReadData: No IO Manager on the scene, returning.")); + vtkWarningToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::StageReadData", + "No IO Manager on the scene"); } vtkDebugMacro("StageReadData: done"); } @@ -583,9 +579,8 @@ void vtkMRMLStorageNode::StageWriteData ( vtkMRMLNode *refNode ) } if (this->URIHandler == nullptr) { - vtkErrorMacro("StageWriteData: unable to get a URI handler for " << this->URI << ", resetting stage to idle"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("StageWriteData: unable to get a URI handler for ") + this->URI + ", resetting stage to idle"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::StageWriteData", + "Unable to get a URI handler for " << this->URI << ", resetting stage to idle"); return; } vtkDebugMacro("StageWriteData: got a URI Handler"); @@ -599,9 +594,8 @@ void vtkMRMLStorageNode::StageWriteData ( vtkMRMLNode *refNode ) } else { - vtkWarningMacro("StageWriteData: No IO Manager on the scene, returning."); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("StageWriteData: No IO Manager on the scene, returning.")); + vtkWarningToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::StageWriteData", + "StageWriteData: No IO Manager on the scene"); } } @@ -734,8 +728,6 @@ int vtkMRMLStorageNode::SupportedFileType(const char *fileName) else { vtkWarningMacro("SupportedFileType: no file name to check"); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("SupportedFileType: no file name to check")); return 0; } @@ -801,9 +793,8 @@ unsigned int vtkMRMLStorageNode::AddFileName( const char* filename ) { if (!filename) { - vtkErrorMacro("AddFileName: can't add a null file name"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("AddFileName: can't add a null file name")); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::AddFileName", + "Cannot add a null file name"); return 0; } std::string filenamestr (filename); @@ -836,23 +827,20 @@ void vtkMRMLStorageNode::ResetNthFileName(int n, const char* fileName) { if (fileName == nullptr) { - vtkErrorMacro("ResetNthFileName: given file name is null (n = " << n << ")"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("ResetNthFileName: given file name is null (n = ") + std::to_string(n) + ")"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ResetNthFileName", + "ResetNthFileName: given file name is null (n = " << n << ")."); return; } if (n < 0) { - vtkErrorMacro("ResetNthFileName: invalid file name number (n = " << n << ")"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("ResetNthFileName: invalid file name number (n = ") + std::to_string(n) + ")"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ResetNthFileName", + "ResetNthFileName: invalid file name number (n = " << n << ")."); return; } else if (n >= this->GetNumberOfFileNames()) { - vtkErrorMacro("ResetNthFileName: file name number " << n << " not already set, returning."); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("ResetNthFileName: file name number ") + std::to_string(n) + " not already set, returning."); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ResetNthFileName", + "ResetNthFileName: file name number " << n << " not already set."); return; } this->FileNameList[n] = fileName; @@ -863,9 +851,8 @@ unsigned int vtkMRMLStorageNode::AddURI( const char* uri ) { if (uri == nullptr) { - vtkErrorMacro("AddURI: can't add a null URI"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("AddURI: can't add a null URI")); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::AddURI", + "Cannot add a null URI."); return 0; } std::string uristr (uri); @@ -897,9 +884,8 @@ void vtkMRMLStorageNode::ResetNthURI(int n, const char* uri) { if (uri == nullptr) { - vtkErrorMacro("ResetNthURI: given URI is null (n = " << n << ")"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("ResetNthURI: given URI is null (n = ") + std::to_string(n) + ")"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ResetNthURI", + "ResetNthURI: given URI is null (n = " << n << ")."); return; } if (n >= 0 && this->GetNumberOfURIs() >= n) @@ -908,9 +894,8 @@ void vtkMRMLStorageNode::ResetNthURI(int n, const char* uri) } else { - vtkErrorMacro("RestNthURI: URI number " << n << " not already set, returning."); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("RestNthURI: URI number ") + std::to_string(n) + " not already set, returning."); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ResetNthURI", + "ResetNthURI: URI number " << n << " not already set."); } } @@ -919,16 +904,14 @@ void vtkMRMLStorageNode::SetDataDirectory(const char *dataDirName) { if (dataDirName == nullptr) { - vtkErrorMacro("SetDataDirectory: input directory name is null, returning."); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("SetDataDirectory: input directory name is null, returning.")); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::SetDataDirectory", + "Input directory name is null."); return; } if (this->GetFileName() == nullptr) { - vtkWarningMacro("SetDataDirectory: file name is null, no reason to reset data directory."); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("SetDataDirectory: file name is null, no reason to reset data directory.")); + vtkWarningToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::SetDataDirectory", + "File name is null, no reason to reset data directory."); return; } // reset the filename @@ -963,14 +946,12 @@ void vtkMRMLStorageNode::SetURIPrefix(const char *uriPrefix) { if (uriPrefix == nullptr) { - vtkErrorMacro("SetURIPrefix: input prefix is null, returning."); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("SetURIPrefix: input prefix is null, returning.")); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::SetURIPrefix", + "Input URI prefix is null."); return; } - vtkWarningMacro("SetURIPrefix " << uriPrefix << " NOT IMPLEMENTED YET"); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("SetURIPrefix ") + uriPrefix + " NOT IMPLEMENTED YET"); + vtkWarningToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::SetURIPrefix", + "Setting URI prefix is not implemented yet."); // reset the uri // then reset all the uris in the list @@ -1021,8 +1002,6 @@ void vtkMRMLStorageNode::GetFileExtensionsFromFileTypes(vtkStringArray* inputFil if (inputFileTypes == nullptr || outputFileExtensions == nullptr) { vtkErrorMacro("vtkMRMLStorageNode::GetSupportedReadFileExtensions failed: invalid inputs"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("vtkMRMLStorageNode::GetSupportedReadFileExtensions failed: invalid inputs")); return; } outputFileExtensions->Reset(); @@ -1143,8 +1122,6 @@ int vtkMRMLStorageNode::IsFilePathRelative(const char * filepath) if (filepath == nullptr) { vtkErrorMacro("IsFilePathRelative: input file path is null! Returning 0"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("IsFilePathRelative: input file path is null! Returning 0")); return 0; } if ( this->Scene ) @@ -1164,8 +1141,6 @@ const char *vtkMRMLStorageNode::GetAbsoluteFilePath(const char *inputPath) if (inputPath == nullptr) { vtkErrorMacro("GetAbsoluteFilePath: input path is null."); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("GetAbsoluteFilePath: input path is null.")); return nullptr; } if (!this->IsFilePathRelative(inputPath)) @@ -1176,9 +1151,8 @@ const char *vtkMRMLStorageNode::GetAbsoluteFilePath(const char *inputPath) if (!this->GetScene() || !this->GetScene()->GetRootDirectory()) { - vtkErrorMacro("GetAbsoluteFilePath: have a relative path " << inputPath << " but no scene or root directory to find it from!"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("GetAbsoluteFilePath: have a relative path ") + inputPath + " but no scene or root directory to find it from!"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::GetAbsoluteFilePath", + "Relative path " << inputPath << " is specified but no scene or root directory to find it from."); return nullptr; } @@ -1228,19 +1202,20 @@ int vtkMRMLStorageNode::ReadData(vtkMRMLNode* refNode, bool temporary) { if (refNode == nullptr) { - vtkErrorMacro("ReadData: can't read into a null node"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("ReadData: can't read into a null node")); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ReadData", + "Cannot read data into a null node."); return 0; } if ( !this->CanReadInReferenceNode(refNode) ) { + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ReadData", + "Cannot read data into reference node of class " << refNode->GetClassName() << "."); return 0; } // do not read if if we are not in the scene (for example inside snapshot) - if ( !refNode->GetAddToScene() ) + if (!refNode->GetAddToScene()) { return 0; } @@ -1252,9 +1227,8 @@ int vtkMRMLStorageNode::ReadData(vtkMRMLNode* refNode, bool temporary) if (this->GetFileName() == nullptr && this->GetURI() == nullptr) { - vtkErrorMacro("ReadData: both filename and uri are null."); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("ReadData: both filename and uri are null.")); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ReadData", + "Both filename and uri are null."); return 0; } @@ -1262,29 +1236,64 @@ int vtkMRMLStorageNode::ReadData(vtkMRMLNode* refNode, bool temporary) if ( this->GetReadState() != this->TransferDone ) { // remote file download hasn't finished - vtkWarningMacro("ReadData: read state is pending, remote download hasn't finished yet"); - this->GetUserMessages()->AddMessage(vtkCommand::WarningEvent, - std::string("ReadData: read state is pending, remote download hasn't finished yet")); + vtkWarningToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ReadData", + "ReadData: read state is pending, remote download hasn't finished yet"); return 0; } vtkDebugMacro("ReadData: read state is ready, " << "URI = " << (this->GetURI() == nullptr ? "null" : this->GetURI()) << ", " << "filename = " << (this->GetFileName() == nullptr ? "null" : this->GetFileName())); - int res = this->ReadDataInternal(refNode); - if (res) + vtkMRMLStorableNode* storableNode = vtkMRMLStorableNode::SafeDownCast(refNode); + int success = this->ReadDataInternal(refNode); + if (!success) { - vtkMRMLStorableNode* storableNode = vtkMRMLStorableNode::SafeDownCast(refNode); + // failed + std::string storableNodeID = "(null)"; + std::string storableNodeName = "(null)"; if (storableNode) { - storableNode->SetAndObserveStorageNodeID(this->GetID()); + if (storableNode->GetID()) + { + storableNodeID = storableNode->GetID(); + } + if (storableNode->GetName()) + { + storableNodeName = storableNode->GetName(); + } } - this->SetReadStateIdle(); - if (!temporary) + std::string location; + if (this->GetURI()) { - this->StoredTime->Modified(); + location += std::string(" URI='") + this->GetURI() + "'"; } + if (this->GetFileName()) + { + location += std::string(" filename='") + this->GetFileName() + "'"; + } + if (location.empty()) + { + location += " - no URI or filename is specified"; + } + else + { + location = " from" + location; + } + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::ReadData", + "Failed to read node " << storableNodeName << " (" << storableNodeID << ")" << location); + + return 0; } - return res; + // success + if (storableNode) + { + storableNode->SetAndObserveStorageNodeID(this->GetID()); + } + this->SetReadStateIdle(); + if (!temporary) + { + this->StoredTime->Modified(); + } + return success; } //------------------------------------------------------------------------------ @@ -1293,9 +1302,8 @@ int vtkMRMLStorageNode::WriteData(vtkMRMLNode* refNode) this->WriteState = this->Idle; if (refNode == nullptr) { - vtkErrorMacro("WriteData: can't write, input node is null"); - this->GetUserMessages()->AddMessage(vtkCommand::ErrorEvent, - std::string("WriteData: can't write, input node is null")); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLStorageNode::WriteData", + "Cannot write " << (this->GetID() ? this->GetID() : "(null)") << ": input node is null"); return 0; } diff --git a/Modules/Loadable/Markups/MRML/vtkMRMLMarkupsJsonStorageNode.cxx b/Modules/Loadable/Markups/MRML/vtkMRMLMarkupsJsonStorageNode.cxx index b0c53d6cc43..d886d84681f 100644 --- a/Modules/Loadable/Markups/MRML/vtkMRMLMarkupsJsonStorageNode.cxx +++ b/Modules/Loadable/Markups/MRML/vtkMRMLMarkupsJsonStorageNode.cxx @@ -19,6 +19,7 @@ #include "vtkMRMLMarkupsJsonStorageNode.h" #include "vtkMRMLMarkupsDisplayNode.h" #include "vtkMRMLMarkupsNode.h" +#include "vtkMRMLMessageCollection.h" #include "vtkMRMLStaticMeasurement.h" #include "vtkMRMLScene.h" @@ -102,7 +103,8 @@ rapidjson::Document* vtkMRMLMarkupsJsonStorageNode::vtkInternal::CreateJsonDocum FILE* fp = fopen(filePath, "r"); if (!fp) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal failed: error opening the file '" << filePath); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal", + "Error opening the file '" << filePath << "'"); return nullptr; } rapidjson::Document* jsonRoot = new rapidjson::Document; @@ -110,7 +112,8 @@ rapidjson::Document* vtkMRMLMarkupsJsonStorageNode::vtkInternal::CreateJsonDocum rapidjson::FileReadStream fs(fp, buffer, sizeof(buffer)); if (jsonRoot->ParseStream(fs).HasParseError()) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal failed: error parsing the file '" << filePath); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal", + "Error parsing the file'" << filePath << "'"); delete jsonRoot; fclose(fp); return nullptr; @@ -120,16 +123,16 @@ rapidjson::Document* vtkMRMLMarkupsJsonStorageNode::vtkInternal::CreateJsonDocum // Verify schema if (!(*jsonRoot).HasMember("@schema")) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal failed: file " - << filePath << " does not contain schema information"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal", + "File reading failed. File '" << filePath << "' does not contain schema information"); delete jsonRoot; return nullptr; } rapidjson::Value& schema = (*jsonRoot)["@schema"]; if (schema.GetString() != MARKUPS_SCHEMA) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal failed: file " - << filePath << " is expected to contain @schema: " + MARKUPS_SCHEMA); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal", + "File reading failed. File '" << filePath << "' is expected to contain @schema: " << MARKUPS_SCHEMA); delete jsonRoot; return nullptr; } @@ -166,12 +169,16 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints(rapidjson::Va { if (!markupsNode) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints failed: invalid markupsNode"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints", + "File reading failed: invalid markups node"); return false; } if (!controlPointsArray.IsArray()) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints failed: invalid controlPoints item"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints", + "File reading failed: invalid controlPoints item (it is expected to be an array)."); return false; } bool wasUpdatingPoints = markupsNode->IsUpdatingPoints; @@ -202,7 +209,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints(rapidjson::Va int positionStatus = vtkMRMLMarkupsNode::GetPositionStatusFromString(controlPointItem["positionStatus"].GetString()); if (positionStatus < 0) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonDocument failed: invalid positionStatus"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints", + "File reading failed: invalid positionStatus '" << controlPointItem["positionStatus"].GetString() << "'."); return false; } cp->PositionStatus = positionStatus; @@ -212,8 +221,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints(rapidjson::Va rapidjson::Value& positionItem = controlPointItem["position"]; if (!this->ReadVector(positionItem, cp->Position)) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonDocument failed:" - << " position must be a 3-element numeric array"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints", + "File reading failed: each control point position must be a 3-element numeric array."); return false; } if (coordinateSystem == vtkMRMLStorageNode::CoordinateSystemLPS) @@ -232,8 +242,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints(rapidjson::Va rapidjson::Value& orientationItem = controlPointItem["orientation"]; if (!this->ReadVector(orientationItem, cp->OrientationMatrix, 9)) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonDocument failed: " - << " position must be a 3-element numeric array"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadControlPoints", + "File reading failed: each control point position must be a 3-element numeric array."); return false; } if (coordinateSystem == vtkMRMLStorageNode::CoordinateSystemLPS) @@ -273,12 +284,16 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements(rapidjson::Val { if (!markupsNode) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements failed: invalid markupsNode"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements", + "Markups measurement reading failed: invalid markups node."); return false; } if (!measurementsArray.IsArray()) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements failed: invalid measurements item"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements", + "Markups measurement reading failed: invalid measurements item (expected it to be an array)."); return false; } @@ -288,7 +303,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements(rapidjson::Val if (!measurementItem.HasMember("name")) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements failed: skipping measurement with no name defined"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements", + "Skipped measurement with missing 'name' attribute."); continue; } @@ -361,8 +378,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements(rapidjson::Val rapidjson::Value& controlPointValuesItem = measurementItem["controlPointValues"]; if (!controlPointValuesItem.IsArray()) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements failed:" - << " controlPointValues must be an array in measurement " << measurementName); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements", + "Measurement reading failed: controlPointValues must be an array in measurement " << measurementName); continue; } int numberOfTuples = controlPointValuesItem.GetArray().Size(); @@ -381,8 +399,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements(rapidjson::Val bool success = this->ReadVector(controlPointValuesItem, values, numberOfTuples); if (!success) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements failed:" - << " error while reading controlPointValues array from measurement " << measurementName); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements", + "Measurement reading failed: error while reading controlPointValues array from measurement " << measurementName); continue; } } @@ -398,8 +417,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements(rapidjson::Val bool success = this->ReadVector(controlPointValuesItem, values, numberOfComponents); if (!success) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements failed:" - << " error while reading controlPointValues array (all items are expected to contain the same number of components)" + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements", + "Measurement reading failed: error while reading controlPointValues array (all items are expected to contain the same number of components)" << " in measurement " << measurementName); continue; } @@ -408,8 +428,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements(rapidjson::Val } else { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements failed:" - << " invalid controlPointValues content (must contain array of doubles or arrays) in measurement " << measurementName); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::ReadMeasurements", + "Measurement reading failed: invalid controlPointValues content (must contain array of doubles or arrays) in measurement " << measurementName); continue; } measurement->SetControlPointValues(controlPointValues); @@ -424,7 +445,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonValue( { if (!markupsNode) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonDocument failed: invalid markupsNode"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonValue", + "Markups reading failed: invalid markupsNode"); return false; } @@ -459,8 +482,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonValue( { if (!this->ReadControlPoints(markupObject["controlPoints"], coordinateSystem, markupsNode)) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonDocument failed:" - << " invalid controlPoints item"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonValue", + "Markups reading failed: invalid controlPoints item."); return false; } } @@ -469,8 +493,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonValue( { if (!this->ReadMeasurements(markupObject["measurements"], markupsNode)) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonDocument failed:" - << " invalid measurements item"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsNodeFromJsonValue", + "Markups reading failed: invalid measurements item."); return false; } } @@ -484,16 +509,18 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsDisplayNodeFromJso { if (!displayNode) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsDisplayNodeFromJsonDocument failed:" - << " invalid displayNode"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsDisplayNodeFromJsonValue", + "Markups reading failed: invalid invalid display node."); return false; } rapidjson::Value& displayItem = markupObject["display"]; if (!displayItem.IsObject()) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsDisplayNodeFromJsonDocument failed:" - << " invalid display item is not found"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::UpdateMarkupsDisplayNodeFromJsonValue", + "Markups reading failed: display item is not found."); return false; } @@ -639,7 +666,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::WriteBasicProperties( } if (markupsType.empty()) { - vtkErrorWithObjectMacro(this->External, "WriteData failed: invalid class name"); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::WriteBasicProperties", + "Writing failed: invalid class name '" << markupsType << "'"); return false; } writer.Key("type"); writer.String(markupsType.c_str()); @@ -671,8 +700,9 @@ bool vtkMRMLMarkupsJsonStorageNode::vtkInternal::WriteControlPoints( if (coordinateSystem != vtkMRMLStorageNode::CoordinateSystemRAS && coordinateSystem != vtkMRMLStorageNode::CoordinateSystemLPS) { - vtkErrorWithObjectMacro(this->External, "vtkMRMLMarkupsJsonStorageNode::WriteControlPoints failed:" - << " Invalid coordinate system: " << coordinateSystem); + vtkErrorToMessageCollectionWithObjectMacro(this->External, this->External->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::vtkInternal::WriteControlPoints", + "Writing failed: Invalid coordinate system '" << coordinateSystem << "'"); return false; } @@ -937,33 +967,42 @@ vtkMRMLMarkupsNode* vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile(con vtkMRMLScene* scene = this->GetScene(); if (!scene) { - vtkErrorMacro("vtkMRMLMarkupsJsonStorageNode::AddMarkupsNodeFromFile failed: invalid scene"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile", + "Adding markups node from file failed: invalid scene."); return nullptr; } if (!filePath) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::ReadDataInternal failed: invalid filename"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile", + "Adding markups node from file failed: invalid filename."); return nullptr; } rapidjson::Document* jsonRoot = this->Internal->CreateJsonDocumentFromFile(filePath); if (!jsonRoot) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::ReadDataInternal failed: error opening the file '" << filePath); + // error is already logged return nullptr; } rapidjson::Value& markups = (*jsonRoot)["markups"]; if (!markups.IsArray()) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::AddNewMarkupsNodeFromFile failed: cannot read markup from file " << filePath - << " (does not contain valid 'markups' array)"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile", + "Adding markups node from file failed: no valid valid 'markups' array is found" + << " in file '" << filePath << "'."); delete jsonRoot; return nullptr; } if (markupIndex >= static_cast(markups.GetArray().Size())) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::AddNewMarkupsNodeFromFile failed: cannot read markup from file " << filePath - << " requested markup index " << markupIndex << " is not found (number of available markups: " << markups.GetArray().Size()); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile", + "Adding markups node from file failed: requested markup index" << markupIndex << " not found" + << " (number of available markups: " << markups.GetArray().Size() + << " in file '" << filePath << "'."); delete jsonRoot; return nullptr; } @@ -972,7 +1011,10 @@ vtkMRMLMarkupsNode* vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile(con std::string className = this->Internal->GetMarkupsClassNameFromJsonValue(markup); if (!markup.HasMember("type")) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::AddNewMarkupsNodeFromFile failed: cannot find required 'type' value"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile", + "Adding markups node from file failed: required 'type' value is not found" + << " in file '" << filePath << "'."); delete jsonRoot; return nullptr; } @@ -990,7 +1032,10 @@ vtkMRMLMarkupsNode* vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile(con vtkMRMLMarkupsNode* markupsNode = vtkMRMLMarkupsNode::SafeDownCast(scene->AddNewNodeByClass(className.c_str(), newNodeName)); if (!markupsNode) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::ReadDataInternal failed: cannot instantiate class '" << className); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), + "vtkMRMLMarkupsJsonStorageNode::AddNewMarkupsNodeFromFile", + "Adding markups node from file failed: cannot instantiate class '" << className + << " in file '" << filePath << "'."); delete jsonRoot; return nullptr; } @@ -1033,20 +1078,22 @@ int vtkMRMLMarkupsJsonStorageNode::ReadDataInternal(vtkMRMLNode *refNode) { if (!refNode) { - vtkErrorMacro("ReadDataInternal: null reference node!"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal", + "Reading markups node file failed: null reference node."); return 0; } const char* filePath = this->GetFileName(); if (!filePath) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::ReadDataInternal failed: invalid filename"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal", + "Reading markups node file failed: invalid filename."); return 0; } rapidjson::Document* jsonRoot = this->Internal->CreateJsonDocumentFromFile(filePath); if (!jsonRoot) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::ReadDataInternal failed: error opening the file '" << filePath); + // error is already logged return 0; } @@ -1063,8 +1110,9 @@ int vtkMRMLMarkupsJsonStorageNode::ReadDataInternal(vtkMRMLNode *refNode) std::string className = this->Internal->GetMarkupsClassNameFromJsonValue(markup); if (className != refNode->GetClassName()) { - vtkErrorMacro("vtkMRMLMarkupsStorageNode::ReadDataInternal failed: cannot read " << refNode->GetClassName() - << " markups class from file " << filePath << " (it contains " << className << ")"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::ReadDataInternal", + "Reading markups node file failed: cannot read " << refNode->GetClassName() + << " markups class from file " << filePath << " (it contains " << className << ")."); return 0; } @@ -1082,7 +1130,8 @@ int vtkMRMLMarkupsJsonStorageNode::WriteDataInternal(vtkMRMLNode *refNode) std::string fullName = this->GetFullNameFromFileName(); if (fullName.empty()) { - vtkErrorMacro("vtkMRMLMarkupsJsonStorageNode: File name not specified"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::WriteDataInternal", + "Writing markups node file failed: file name not specified."); return 0; } vtkDebugMacro("WriteDataInternal: have file name " << fullName.c_str()); @@ -1096,7 +1145,8 @@ int vtkMRMLMarkupsJsonStorageNode::WriteDataInternal(vtkMRMLNode *refNode) if (markupsNode == nullptr) { - vtkErrorMacro("WriteData: unable to cast input node " << refNode->GetID() << " to a known markups node"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::WriteDataInternal", + "Writing markups node file failed: unable to cast input node " << refNode->GetID() << " to a known markups node."); return 0; } @@ -1104,7 +1154,8 @@ int vtkMRMLMarkupsJsonStorageNode::WriteDataInternal(vtkMRMLNode *refNode) FILE* fp = fopen(fullName.c_str(), "wb"); if (!fp) { - vtkErrorMacro("WriteData: unable to open file " << fullName.c_str() << " for writing"); + vtkErrorToMessageCollectionMacro(this->GetUserMessages(), "vtkMRMLMarkupsJsonStorageNode::WriteDataInternal", + "Writing markups node file failed: unable to open file '" << fullName.c_str() << "' for writing."); return 0; } diff --git a/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.cxx b/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.cxx index c95eebeae9f..7ed692f2310 100644 --- a/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.cxx +++ b/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.cxx @@ -14,6 +14,7 @@ /// MRML includes #include #include +#include "vtkMRMLMessageCollection.h" #include #include #include @@ -209,7 +210,8 @@ vtkMRMLModelNode* vtkSlicerModelsLogic::AddModel(vtkAlgorithmOutput* polyData) //---------------------------------------------------------------------------- int vtkSlicerModelsLogic::AddModels (const char* dirname, const char* suffix, - int coordinateSystem /*=vtkMRMLStorageNode::CoordinateSystemLPS*/) + int coordinateSystem /*=vtkMRMLStorageNode::CoordinateSystemLPS*/, + vtkMRMLMessageCollection* userMessages/*=nullptr*/) { std::string ssuf = suffix; itksys::Directory dir; @@ -226,7 +228,7 @@ int vtkSlicerModelsLogic::AddModels (const char* dirname, const char* suffix, { std::string fullPath = std::string(dir.GetPath()) + "/" + filename; - if (this->AddModel(fullPath.c_str(), coordinateSystem) == nullptr) + if (this->AddModel(fullPath.c_str(), coordinateSystem, userMessages) == nullptr) { res = 0; } @@ -238,11 +240,13 @@ int vtkSlicerModelsLogic::AddModels (const char* dirname, const char* suffix, //---------------------------------------------------------------------------- vtkMRMLModelNode* vtkSlicerModelsLogic::AddModel(const char* filename, - int coordinateSystem /*=vtkMRMLStorageNode::CoordinateSystemLPS*/) + int coordinateSystem /*=vtkMRMLStorageNode::CoordinateSystemLPS*/, + vtkMRMLMessageCollection* userMessages/*=nullptr*/) { - if (this->GetMRMLScene() == nullptr || - filename == nullptr) + if (this->GetMRMLScene() == nullptr || filename == nullptr) { + vtkErrorToMessageCollectionMacro(userMessages, "vtkSlicerModelsLogic::AddModel", + "Invalid scene or filename"); return nullptr; } vtkNew modelNode; @@ -278,7 +282,7 @@ vtkMRMLModelNode* vtkSlicerModelsLogic::AddModel(const char* filename, // check to see which node can read this type of file if (mStorageNode->SupportedFileType(name.c_str())) { - storageNode = mStorageNode.GetPointer(); + storageNode = mStorageNode; mStorageNode->SetCoordinateSystem(coordinateSystem); } @@ -288,36 +292,39 @@ vtkMRMLModelNode* vtkSlicerModelsLogic::AddModel(const char* filename, storageNode = mStorageNode; } */ - if (storageNode != nullptr) + if (!storageNode) { - std::string baseName = storageNode->GetFileNameWithoutExtension(fname.c_str()); - std::string uname( this->GetMRMLScene()->GetUniqueNameByString(baseName.c_str())); - modelNode->SetName(uname.c_str()); - - this->GetMRMLScene()->AddNode(storageNode.GetPointer()); - this->GetMRMLScene()->AddNode(displayNode.GetPointer()); - - // Set the scene so that SetAndObserve[Display|Storage]NodeID can find the - // node in the scene (so that DisplayNodes return something not empty) - modelNode->SetScene(this->GetMRMLScene()); - modelNode->SetAndObserveStorageNodeID(storageNode->GetID()); - modelNode->SetAndObserveDisplayNodeID(displayNode->GetID()); - - this->GetMRMLScene()->AddNode(modelNode.GetPointer()); - - // now set up the reading - vtkDebugMacro("AddModel: calling read on the storage node"); - int retval = storageNode->ReadData(modelNode.GetPointer()); - if (retval != 1) - { - vtkErrorMacro("AddModel: error reading " << filename); - this->GetMRMLScene()->RemoveNode(modelNode.GetPointer()); - return nullptr; - } + vtkErrorToMessageCollectionMacro(userMessages, "vtkSlicerModelsLogic::AddModel", + "Could not find a suitable storage node for file '" << filename << "'."); + return nullptr; } - else + std::string baseName = storageNode->GetFileNameWithoutExtension(fname.c_str()); + std::string uname( this->GetMRMLScene()->GetUniqueNameByString(baseName.c_str())); + modelNode->SetName(uname.c_str()); + + this->GetMRMLScene()->AddNode(storageNode.GetPointer()); + this->GetMRMLScene()->AddNode(displayNode.GetPointer()); + + // Set the scene so that SetAndObserve[Display|Storage]NodeID can find the + // node in the scene (so that DisplayNodes return something not empty) + modelNode->SetScene(this->GetMRMLScene()); + modelNode->SetAndObserveStorageNodeID(storageNode->GetID()); + modelNode->SetAndObserveDisplayNodeID(displayNode->GetID()); + + this->GetMRMLScene()->AddNode(modelNode.GetPointer()); + + // now set up the reading + vtkDebugMacro("AddModel: calling read on the storage node"); + storageNode->GetUserMessages()->ClearMessages(); + int success = storageNode->ReadData(modelNode.GetPointer()); + if (!success) { - vtkErrorMacro("Couldn't read file: " << filename); + vtkErrorMacro("AddModel: error reading " << filename); + if (userMessages) + { + userMessages->AddMessages(storageNode->GetUserMessages()); + } + this->GetMRMLScene()->RemoveNode(modelNode.GetPointer()); return nullptr; } @@ -326,16 +333,15 @@ vtkMRMLModelNode* vtkSlicerModelsLogic::AddModel(const char* filename, //---------------------------------------------------------------------------- int vtkSlicerModelsLogic::SaveModel (const char* filename, vtkMRMLModelNode *modelNode, - int coordinateSystem/*=-1*/) + int coordinateSystem/*=-1*/, vtkMRMLMessageCollection* userMessages/*=nullptr*/) { if (modelNode == nullptr || filename == nullptr) - { - vtkErrorMacro("SaveModel: unable to proceed, filename is " << - (filename == nullptr ? "null" : filename) << - ", model node is " << - (modelNode == nullptr ? "null" : modelNode->GetID())); - return 0; - } + { + vtkErrorToMessageCollectionMacro(userMessages, "vtkSlicerModelsLogic::SaveModel", + "Failed to save model node " << ((modelNode && modelNode->GetID()) ? modelNode->GetID() : "(null)") + << " into file '" << (filename ? filename : "(null)") << "'."); + return 0; + } vtkMRMLModelStorageNode *storageNode = nullptr; vtkMRMLStorageNode *snode = modelNode->GetStorageNode(); @@ -345,11 +351,8 @@ int vtkSlicerModelsLogic::SaveModel (const char* filename, vtkMRMLModelNode *mod } if (storageNode == nullptr) { - storageNode = vtkMRMLModelStorageNode::New(); - storageNode->SetScene(this->GetMRMLScene()); - this->GetMRMLScene()->AddNode(storageNode); + storageNode = vtkMRMLModelStorageNode::SafeDownCast(this->GetMRMLScene()->AddNewNodeByClass("vtkMRMLModelStorageNode")); modelNode->SetAndObserveStorageNodeID(storageNode->GetID()); - storageNode->Delete(); } if (coordinateSystem >= 0) @@ -368,9 +371,16 @@ int vtkSlicerModelsLogic::SaveModel (const char* filename, vtkMRMLModelNode *mod storageNode->SetFileName(filename); } - int res = storageNode->WriteData(modelNode); - - return res; + int success = storageNode->WriteData(modelNode); + if (!success) + { + vtkErrorMacro("vtkSlicerModelsLogic::SaveModel: error saving " << filename); + if (userMessages) + { + userMessages->AddMessages(storageNode->GetUserMessages()); + } + } + return success; } //---------------------------------------------------------------------------- diff --git a/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.h b/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.h index 3fa89b0b3e2..1d1f14bf8b6 100644 --- a/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.h +++ b/Modules/Loadable/Models/Logic/vtkSlicerModelsLogic.h @@ -25,6 +25,7 @@ // VTK includes #include +class vtkMRMLMessageCollection; class vtkMRMLModelNode; class vtkMRMLStorageNode; class vtkMRMLTransformNode; @@ -56,19 +57,28 @@ class VTK_SLICER_MODELS_MODULE_LOGIC_EXPORT vtkSlicerModelsLogic /// A display node and a storage node are also added into the scene /// \param coordinateSystem If coordinate system is not specified /// in the file then this coordinate system is used. Default is LPS. - vtkMRMLModelNode* AddModel(const char* filename, int coordinateSystem = vtkMRMLStorageNode::CoordinateSystemLPS); + /// \param userMessages User-displayable warning or error messages can be received if userMessages object is + /// specified. + vtkMRMLModelNode* AddModel(const char* filename, int coordinateSystem = vtkMRMLStorageNode::CoordinateSystemLPS, + vtkMRMLMessageCollection* userMessages = nullptr); /// Create model nodes and /// read their polydata from a specified directory /// \param coordinateSystem If coordinate system is not specified /// in the file then this coordinate system is used. Default is LPS. - int AddModels(const char* dirname, const char* suffix, int coordinateSystem = vtkMRMLStorageNode::CoordinateSystemLPS); + /// \param userMessages User-displayable warning or error messages can be received if userMessages object is + /// specified. + int AddModels(const char* dirname, const char* suffix, int coordinateSystem = vtkMRMLStorageNode::CoordinateSystemLPS, + vtkMRMLMessageCollection* userMessages = nullptr); /// Write model's polydata to a specified file /// \param coordinateSystem If coordinate system is not specified /// in the file then this coordinate system is used. Default is -1, which means that /// the coordinate system specified in the storage node will be used. - int SaveModel(const char* filename, vtkMRMLModelNode *modelNode, int coordinateSystem = vtkMRMLStorageNode::CoordinateSystemLPS); + /// \param userMessages User-displayable warning or error messages can be received if userMessages object is + /// specified. + int SaveModel(const char* filename, vtkMRMLModelNode *modelNode, int coordinateSystem = vtkMRMLStorageNode::CoordinateSystemLPS, + vtkMRMLMessageCollection* userMessages = nullptr); /// Transform models's polydata static void TransformModel(vtkMRMLTransformNode *tnode, diff --git a/Modules/Loadable/Models/Testing/Cxx/vtkSlicerModelsLogicAddFileTest.cxx b/Modules/Loadable/Models/Testing/Cxx/vtkSlicerModelsLogicAddFileTest.cxx index 3b342062b4b..1786237aaa1 100644 --- a/Modules/Loadable/Models/Testing/Cxx/vtkSlicerModelsLogicAddFileTest.cxx +++ b/Modules/Loadable/Models/Testing/Cxx/vtkSlicerModelsLogicAddFileTest.cxx @@ -22,6 +22,7 @@ #include "vtkSlicerModelsLogic.h" // MRML includes +#include "vtkMRMLCoreTestingMacros.h" #include #include @@ -95,7 +96,9 @@ bool testAddEmptyFile(const char * filePath) bool testAddFile(const char * filePath) { vtkNew modelsLogic; + TESTING_OUTPUT_ASSERT_ERRORS_BEGIN(); vtkMRMLModelNode* model = modelsLogic->AddModel(filePath); + TESTING_OUTPUT_ASSERT_ERRORS_END(); if (model != nullptr) { std::cerr << "Error line " << __LINE__ diff --git a/Modules/Loadable/Models/qSlicerModelsReader.cxx b/Modules/Loadable/Models/qSlicerModelsReader.cxx index 9add6ee37bc..e1e1dd12cde 100644 --- a/Modules/Loadable/Models/qSlicerModelsReader.cxx +++ b/Modules/Loadable/Models/qSlicerModelsReader.cxx @@ -19,6 +19,7 @@ ==============================================================================*/ // Qt includes +#include #include // Slicer includes @@ -29,6 +30,7 @@ #include "vtkSlicerModelsLogic.h" // MRML includes +#include "vtkMRMLMessageCollection.h" #include "vtkMRMLModelNode.h" #include @@ -104,8 +106,9 @@ bool qSlicerModelsReader::load(const IOProperties& properties) QString fileName = properties["fileName"].toString(); this->setLoadedNodes(QStringList()); - if (d->ModelsLogic.GetPointer() == nullptr) + if (!d->ModelsLogic) { + qCritical() << Q_FUNC_INFO << (" failed: Models logic is invalid."); return false; } int coordinateSystem = vtkMRMLStorageNode::CoordinateSystemLPS; // default @@ -113,10 +116,12 @@ bool qSlicerModelsReader::load(const IOProperties& properties) { coordinateSystem = properties["coordinateSystem"].toInt(); } + this->userMessages()->ClearMessages(); vtkMRMLModelNode* node = d->ModelsLogic->AddModel( - fileName.toUtf8(), coordinateSystem); + fileName.toUtf8(), coordinateSystem, this->userMessages()); if (!node) { + // errors are already logged and userMessages contain details that can be displayed to users return false; } this->setLoadedNodes( QStringList(QString(node->GetID())) ); diff --git a/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.cxx b/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.cxx index 8fbd39e5261..61505b89d75 100644 --- a/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.cxx +++ b/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.cxx @@ -32,6 +32,7 @@ #include "vtkMRMLCameraNode.h" #include "vtkMRMLLabelMapVolumeNode.h" #include "vtkMRMLMarkupsFiducialNode.h" +#include "vtkMRMLMessageCollection.h" #include "vtkMRMLModelNode.h" #include "vtkMRMLScalarVolumeNode.h" #include "vtkMRMLScene.h" @@ -136,7 +137,7 @@ void vtkSlicerSequencesLogic::OnMRMLSceneNodeRemoved(vtkMRMLNode* node) } //---------------------------------------------------------------------------- -vtkMRMLSequenceNode* vtkSlicerSequencesLogic::AddSequence(const char* filename) +vtkMRMLSequenceNode* vtkSlicerSequencesLogic::AddSequence(const char* filename, vtkMRMLMessageCollection* userMessages/*=nullptr*/) { if (this->GetMRMLScene() == nullptr || filename == nullptr) { @@ -157,7 +158,8 @@ vtkMRMLSequenceNode* vtkSlicerSequencesLogic::AddSequence(const char* filename) } else { - vtkErrorMacro("vtkSlicerSequencesLogic::AddSequence failed: unrecognized file extension"); + vtkErrorToMessageCollectionMacro(userMessages, "vtkSlicerSequencesLogic::AddSequence", + "Failed to read sequence. Unsupported file type: " << filename); return nullptr; } @@ -184,7 +186,7 @@ vtkMRMLSequenceNode* vtkSlicerSequencesLogic::AddSequence(const char* filename) // the sequence name is based on the file name (vtksys call should work even if // file is not on disk yet) std::string name = vtksys::SystemTools::GetFilenameName(fname); - vtkDebugMacro("AddModel: got Sequence name = " << name.c_str()); + vtkDebugMacro("AddSequence: got Sequence name = " << name.c_str()); // check to see which node can read this type of file if (storageNode->SupportedFileType(name.c_str())) @@ -205,10 +207,14 @@ vtkMRMLSequenceNode* vtkSlicerSequencesLogic::AddSequence(const char* filename) // now set up the reading vtkDebugMacro("AddSequence: calling read on the storage node"); - int retval = storageNode->ReadData(sequenceNode); - if (retval != 1) + int success = storageNode->ReadData(sequenceNode); + if (!success) { vtkErrorMacro("AddSequence: error reading " << filename); + if (userMessages) + { + userMessages->AddMessages(storageNode->GetUserMessages()); + } this->GetMRMLScene()->RemoveNode(sequenceNode); this->GetMRMLScene()->RemoveNode(storageNode); return nullptr; diff --git a/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.h b/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.h index e8bd1e46788..d747fbc6a0a 100644 --- a/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.h +++ b/Modules/Loadable/Sequences/Logic/vtkSlicerSequencesLogic.h @@ -34,6 +34,7 @@ #include "vtkSlicerSequencesModuleLogicExport.h" +class vtkMRMLMessageCollection; class vtkMRMLNode; class vtkMRMLSequenceNode; class vtkMRMLSequenceBrowserNode; @@ -50,9 +51,11 @@ class VTK_SLICER_SEQUENCES_MODULE_LOGIC_EXPORT vtkSlicerSequencesLogic : /// /// Add into the scene a new mrml sequence node and - /// read its data from a specified file - /// A storage node is also added into the scene - vtkMRMLSequenceNode* AddSequence(const char* filename); + /// read its data from a specified file. + /// A storage node is also added into the scene. + /// User-displayable warning or error messages can be received if userMessages object is + /// specified. + vtkMRMLSequenceNode* AddSequence(const char* filename, vtkMRMLMessageCollection* userMessages=nullptr); /// Refreshes the output of all the active browser nodes. Called regularly by a timer. void UpdateAllProxyNodes(); diff --git a/Modules/Loadable/Sequences/qSlicerSequencesReader.cxx b/Modules/Loadable/Sequences/qSlicerSequencesReader.cxx index 86b3abf7b32..074ee7481c8 100644 --- a/Modules/Loadable/Sequences/qSlicerSequencesReader.cxx +++ b/Modules/Loadable/Sequences/qSlicerSequencesReader.cxx @@ -20,6 +20,7 @@ // Qt includes #include +#include // Slicer includes #include "qSlicerSequencesReader.h" @@ -104,11 +105,13 @@ bool qSlicerSequencesReader::load(const IOProperties& properties) this->setLoadedNodes(QStringList()); if (d->SequencesLogic.GetPointer() == 0) { + qCritical() << Q_FUNC_INFO << (" failed: Sequences logic is invalid."); return false; } - vtkMRMLSequenceNode* node = d->SequencesLogic->AddSequence(fileName.toLatin1()); + vtkMRMLSequenceNode* node = d->SequencesLogic->AddSequence(fileName.toUtf8(), this->userMessages()); if (!node) { + // errors are already logged and userMessages contain details that can be displayed to users return false; }