diff --git a/tomviz/DataPropertiesPanel.cxx b/tomviz/DataPropertiesPanel.cxx index b4468f967..6552b796e 100644 --- a/tomviz/DataPropertiesPanel.cxx +++ b/tomviz/DataPropertiesPanel.cxx @@ -181,11 +181,15 @@ QList DataPropertiesPanel::getArraysInfo(DataSource* dataSource) { QList arraysInfo; + // If the number of scalar arrays has changed, reset the indexes. + auto scalars = dataSource->listScalars(); + if (scalars.size() != m_scalarIndexes.size()) + m_scalarIndexes.clear(); + // If we don't have the scalar indexes, we sort the names and then save the // indexes, these will be used to preserve the displayed order even after // a rename. if (m_scalarIndexes.isEmpty()) { - auto scalars = dataSource->listScalars(); auto sortedScalars = scalars; // sort the scalars names diff --git a/tomviz/DataSource.cxx b/tomviz/DataSource.cxx index e155d454e..85a5ada17 100644 --- a/tomviz/DataSource.cxx +++ b/tomviz/DataSource.cxx @@ -168,6 +168,7 @@ DataSource::DataSource(const QString& label, DataSourceType dataType, DataSource::~DataSource() { + ModuleManager::instance().removeAllModules(this); if (this->Internals->ProducerProxy) { vtkNew controller; controller->UnRegisterProxy(this->Internals->ProducerProxy); diff --git a/tomviz/EmdFormat.cxx b/tomviz/EmdFormat.cxx index 7d6c68e78..eb10fa92b 100644 --- a/tomviz/EmdFormat.cxx +++ b/tomviz/EmdFormat.cxx @@ -156,6 +156,11 @@ bool EmdFormat::readNode(h5::H5ReadWrite& reader, const std::string& emdNode, GenericHDF5Format::relabelXAndZAxes(image); DataSource::setTiltAngles(image, angles); DataSource::setType(image, DataSource::TiltSeries); + + // Spacing of the tilt axis should be 1.0 + double spacing[3]; + image->GetSpacing(spacing); + image->SetSpacing(spacing[0], spacing[1], 1.0); } return true; diff --git a/tomviz/ExternalPythonExecutor.cxx b/tomviz/ExternalPythonExecutor.cxx index 82ce12c2b..1c8e18962 100644 --- a/tomviz/ExternalPythonExecutor.cxx +++ b/tomviz/ExternalPythonExecutor.cxx @@ -5,7 +5,6 @@ #include "ExternalPythonExecutor.h" #include "DataSource.h" -#include "EmdFormat.h" #include "Operator.h" #include "OperatorPython.h" #include "Pipeline.h" diff --git a/tomviz/Pipeline.cxx b/tomviz/Pipeline.cxx index 78ad0237d..20d573e47 100644 --- a/tomviz/Pipeline.cxx +++ b/tomviz/Pipeline.cxx @@ -7,7 +7,6 @@ #include "DataSource.h" #include "DockerExecutor.h" #include "DockerUtilities.h" -#include "EmdFormat.h" #include "ExternalPythonExecutor.h" #include "ModuleManager.h" #include "Operator.h" @@ -180,11 +179,13 @@ Pipeline::Future* Pipeline::execute(DataSource* dataSource) auto operators = dataSource->operators(); if (operators.size() < 1) { + emit finished(); return emptyFuture(); } Operator* firstModifiedOperator = operators.first(); if (!isModified(dataSource, &firstModifiedOperator)) { + emit finished(); return emptyFuture(); } @@ -269,6 +270,37 @@ Pipeline::Future* Pipeline::execute(DataSource* ds, Operator* start, auto pipelineFuture = new PipelineFutureInternal( this, branchFuture->operators(), branchFuture, operators.last() == end); + + // After the pipeline finishes, move the modules to the back and + // remove the old child data source. + // If the operator to be removed has a child, move those modules to + // the back. Otherwise, move the transformed data source modules. + DataSource* oldChild = nullptr; + if (m_lastOperatorChildRemoved) { + // This indicates that an operator was just removed. Use that. + oldChild = m_lastOperatorChildRemoved; + m_lastOperatorChildRemoved = nullptr; + } else { + // Get either the child data source of the last operator or the + // transformed data source (which could be the root data source). + auto lastOp = operators.last(); + oldChild = lastOp->childDataSource() ? lastOp->childDataSource() + : transformedDataSource(); + } + + connect(pipelineFuture, &Pipeline::Future::finished, oldChild, + [this, oldChild]() { + auto newChild = transformedDataSource(); + if (newChild != oldChild) { + // If the child is not the same, move the modules to the new one + ModuleManager::instance().moveModules(oldChild, newChild); + if (!oldChild->forkable()) { + // Remove the old child data source if it was not forkable + ModuleManager::instance().removeChildDataSource(oldChild); + } + } + }); + connect(pipelineFuture, &Pipeline::Future::finished, this, &Pipeline::finished); @@ -355,56 +387,68 @@ void Pipeline::branchFinished() if (operators.isEmpty()) { return; } - auto start = future->operators().first()->dataSource(); + auto start = operators.first()->dataSource(); auto newData = future->result(); - // We only add the transformed child data source if the last operator - // doesn't already have an explicit child data source i.e. - // hasChildDataSource is true. + + // Set the output data on the final operator's child data source auto lastOp = start->operators().last(); - if (!lastOp->hasChildDataSource()) { - DataSource* newChildDataSource = nullptr; - if (lastOp->childDataSource() == nullptr) { - newChildDataSource = new DataSource("Output"); - newChildDataSource->setPersistenceState( - tomviz::DataSource::PersistenceState::Transient); - newChildDataSource->setForkable(false); - newChildDataSource->setParent(this); - lastOp->setChildDataSource(newChildDataSource); - auto rootDataSource = dataSource(); - // connect signal to flow units and spacing to child data source. - connect(dataSource(), &DataSource::dataPropertiesChanged, - [rootDataSource, newChildDataSource]() { - // Only flow the properties if no user modifications have been - // made. - if (!newChildDataSource->unitsModified()) { - newChildDataSource->setUnits(rootDataSource->getUnits(), - false); - double spacing[3]; - rootDataSource->getSpacing(spacing); - newChildDataSource->setSpacing(spacing, false); - } - }); - } + QString label = "Output"; + + auto child = lastOp->childDataSource(); + if (child) { + // Remove the old child, and create a new one from the output data. + // We will always use the output data for the final output. + label = child->label(); + ModuleManager::instance().removeChildDataSource(child); + lastOp->setChildDataSource(nullptr); + } - // Update the type if necessary - DataSource::DataSourceType type = DataSource::hasTiltAngles(newData) - ? DataSource::TiltSeries - : DataSource::Volume; - lastOp->childDataSource()->setData(newData); - lastOp->childDataSource()->setType(type); - lastOp->childDataSource()->dataModified(); - - if (newChildDataSource != nullptr) { - emit lastOp->newChildDataSource(newChildDataSource); - // Move modules from root data source. - moveModulesDown(newChildDataSource); + // Remove any children produced by the operators. We currently do not + // support intermediate data sources. + for (auto op : operators) { + if (op->childDataSource()) { + ModuleManager::instance().removeChildDataSource(op->childDataSource()); + op->setChildDataSource(nullptr); } } - else { - // If this is the only operator, make sure the modules are moved down. - if (start->operators().size() == 1) - moveModulesDown(lastOp->childDataSource()); - } + + // Create one + child = new DataSource(label); + child->setPersistenceState(tomviz::DataSource::PersistenceState::Transient); + lastOp->setChildDataSource(child); + + // TODO: the following should be set to this, once we get + // intermediate datasets working. + // child->setForkable(hasChildDataSource()); + child->setForkable(false); + + // TODO: when we get intermediate datasets working, this data source + // should have the same pipeline, with pauses in the pipeline at + // forkable data sources. + child->setParent(this); + + auto rootDataSource = dataSource(); + // connect signal to flow units and spacing to child data source. + connect(dataSource(), &DataSource::dataPropertiesChanged, child, + [rootDataSource, child]() { + // Only flow the properties if no user modifications have been + // made. + if (!child->unitsModified()) { + child->setUnits(rootDataSource->getUnits(), false); + double spacing[3]; + rootDataSource->getSpacing(spacing); + child->setSpacing(spacing, false); + } + }); + + DataSource::DataSourceType type = DataSource::hasTiltAngles(newData) + ? DataSource::TiltSeries + : DataSource::Volume; + child->setData(newData); + child->setType(type); + child->dataModified(); + + emit lastOp->newChildDataSource(child); } void Pipeline::pause() @@ -482,22 +526,18 @@ void Pipeline::addDataSource(DataSource* dataSource) &Operator::newChildDataSource), [this](DataSource* ds) { addDataSource(ds); }); - // We need to ensure we move add datasource to the end of the branch - auto operators = op->dataSource()->operators(); - if (operators.size() > 1) { - auto transformedDataSourceOp = - findTransformedDataSourceOperator(op->dataSource()); - if (transformedDataSourceOp != nullptr) { - auto transformedDataSource = transformedDataSourceOp->childDataSource(); - transformedDataSourceOp->setChildDataSource(nullptr); - op->setChildDataSource(transformedDataSource); - emit operatorAdded(op, transformedDataSource); - } else { - emit operatorAdded(op); - } - } else { - emit operatorAdded(op); - } + // This might also be the root datasource, which is okay + auto oldChild = transformedDataSource(op->dataSource()); + + // This is just to make the modules "move down" in the view before + // the operator is ran. + DataSource* outputDS = nullptr; + auto transformedDataSourceOp = + findTransformedDataSourceOperator(op->dataSource()); + if (transformedDataSourceOp) + outputDS = oldChild; + + emit operatorAdded(op, outputDS); }); // Wire up operatorRemoved. TODO We need to check the branch of the // pipeline we are currently executing. @@ -508,21 +548,16 @@ void Pipeline::addDataSource(DataSource* dataSource) if (!op->isNew()) { m_operatorsDeleted = true; } - if (op->childDataSource() != nullptr) { - auto transformedDataSource = op->childDataSource(); - auto operators = op->dataSource()->operators(); - // We have an operator to move it to. - if (!operators.isEmpty()) { - auto newOp = operators.last(); - op->setChildDataSource(nullptr); - newOp->setChildDataSource(transformedDataSource); - emit newOp->dataSourceMoved(transformedDataSource); - } - // Clean it up - else { - transformedDataSource->removeAllOperators(); - transformedDataSource->deleteLater(); - } + + if (op->dataSource()->operators().isEmpty() && op->childDataSource()) { + // The last operator was removed. Move all the modules to the root. + ModuleManager::instance().moveModules(op->childDataSource(), + op->dataSource()); + ModuleManager::instance().removeChildDataSource(op->childDataSource()); + } else if (op->childDataSource()) { + // Save this so we can move the modules from it and delete it + // later. + m_lastOperatorChildRemoved = op->childDataSource(); } // If pipeline is running see if we can safely remove the operator @@ -607,24 +642,6 @@ Pipeline::Future* Pipeline::emptyFuture() return future; } -void Pipeline::moveModulesDown(DataSource* newChildDataSource) -{ - bool oldMoveObjectsEnabled = - ActiveObjects::instance().moveObjectsEnabled(); - ActiveObjects::instance().setMoveObjectsMode(false); - auto view = ActiveObjects::instance().activeView(); - foreach (Module* module, ModuleManager::instance().findModules( - dataSource(), nullptr)) { - // TODO: We should really copy the module properties as well. - auto newModule = ModuleManager::instance().createAndAddModule( - module->label(), newChildDataSource, view); - // Copy over properties using the serialization code. - newModule->deserialize(module->serialize()); - ModuleManager::instance().removeModule(module); - } - ActiveObjects::instance().setMoveObjectsMode(oldMoveObjectsEnabled); -} - #include "Pipeline.moc" } // namespace tomviz diff --git a/tomviz/Pipeline.h b/tomviz/Pipeline.h index 7fe1c1b70..a93e6be3c 100644 --- a/tomviz/Pipeline.h +++ b/tomviz/Pipeline.h @@ -133,8 +133,6 @@ private slots: private: DataSource* findTransformedDataSource(DataSource* dataSource); Operator* findTransformedDataSourceOperator(DataSource* dataSource); - // Move modules down below the new data source - void moveModulesDown(DataSource* newChildDataSource); void addDataSource(DataSource* dataSource); bool beingEdited(DataSource* dataSource) const; bool isModified(DataSource* dataSource, Operator** firstModified) const; @@ -145,6 +143,7 @@ private slots: QScopedPointer m_executor; ExecutionMode m_executionMode = Threaded; int m_editingOperators = 0; + DataSource* m_lastOperatorChildRemoved = nullptr; }; /// Return from getCopyOfImagePriorTo for caller to track async operation. @@ -166,7 +165,6 @@ class Pipeline::Future : public QObject virtual ~Future() override{}; vtkSmartPointer result() { return m_imageData; } - void setResult(vtkSmartPointer result) { m_imageData = result; } void setResult(vtkImageData* result) { m_imageData = result; } QList operators() { return m_operators; } void deleteWhenFinished(); diff --git a/tomviz/PipelineExecutor.cxx b/tomviz/PipelineExecutor.cxx index e19a2dad2..db75af702 100644 --- a/tomviz/PipelineExecutor.cxx +++ b/tomviz/PipelineExecutor.cxx @@ -83,7 +83,6 @@ Pipeline::Future* ExternalPipelineExecutor::execute(vtkDataObject* data, if (!m_temporaryDir->isValid()) { displayError("Directory Error", "Unable to create temporary directory."); return Pipeline::emptyFuture(); - ; } QString origFileName = originalFileName(); @@ -169,12 +168,10 @@ Pipeline::Future* ExternalPipelineExecutor::execute(vtkDataObject* data, &ExternalPipelineExecutor::pipelineStarted); connect(m_progressReader.data(), &ProgressReader::pipelineFinished, this, [this, future]() { + // Read the modified active scalars auto transformedFilePath = QDir(workingDir()).filePath(TRANSFORM_FILENAME); - vtkSmartPointer transformedData = - vtkImageData::New(); - vtkImageData* transformedImageData = - vtkImageData::SafeDownCast(transformedData.Get()); + vtkNew transformedImageData; // Make sure we don't ask the user about subsampling QVariantMap options = { { "askForSubsample", false } }; if (EmdFormat::read(transformedFilePath.toLatin1().data(), @@ -186,7 +183,6 @@ Pipeline::Future* ExternalPipelineExecutor::execute(vtkDataObject* data, .arg(transformedFilePath)); } emit future->finished(); - transformedImageData->FastDelete(); }); connect(future, &Pipeline::Future::finished, this, &ExternalPipelineExecutor::reset); @@ -250,11 +246,6 @@ void ExternalPipelineExecutor::operatorStarted(Operator* op) { op->setState(OperatorState::Running); emit op->transformingStarted(); - - auto pythonOp = qobject_cast(op); - if (pythonOp != nullptr) { - pythonOp->createChildDataSource(); - } } void ExternalPipelineExecutor::operatorFinished(Operator* op) @@ -277,7 +268,6 @@ void ExternalPipelineExecutor::operatorFinished(Operator* op) if (EmdFormat::read(fileInfo.filePath().toLatin1().data(), childData, options)) { childOutput[name] = childData; - emit pipeline()->finished(); } else { displayError( "Read Error", diff --git a/tomviz/PipelineModel.cxx b/tomviz/PipelineModel.cxx index c9db88e73..a98e36e24 100644 --- a/tomviz/PipelineModel.cxx +++ b/tomviz/PipelineModel.cxx @@ -204,8 +204,6 @@ bool PipelineModel::TreeItem::remove(DataSource* source) // Resume but don't execute as we are removing this data source. pipeline->resume(); } - } else if (childItem->module()) { - ModuleManager::instance().removeModule(childItem->module()); } } if (parent()) { @@ -779,14 +777,6 @@ void PipelineModel::dataSourceAdded(DataSource* dataSource) connect(pipeline, &Pipeline::operatorAdded, this, &PipelineModel::operatorAdded, Qt::UniqueConnection); - // Fire signal to indicate that the transformed data source has been modified - // when the pipeline has been executed. - // TODO This should probably be move else where! - connect(pipeline, &Pipeline::finished, [this, pipeline]() { - auto transformed = pipeline->transformedDataSource(); - emit dataSourceModified(transformed); - }); - // When restoring a data source from a state file it will have its operators // before we can listen to the signal above. Display those operators. foreach (auto op, dataSource->operators()) { diff --git a/tomviz/PythonUtilities.cxx b/tomviz/PythonUtilities.cxx index b756bda63..30e149472 100644 --- a/tomviz/PythonUtilities.cxx +++ b/tomviz/PythonUtilities.cxx @@ -221,6 +221,29 @@ Python::Dict& Python::Dict::operator=(const Python::Object& other) return *this; } +long long Python::Dict::size() const +{ + // If we use ssize_t, we get an error on Windows... + return PyDict_Size(m_smartPyObject->GetPointer()); +} + +QStringList Python::Dict::keys() const +{ + QStringList ret; + + List list = PyDict_Keys(m_smartPyObject->GetPointer()); + for (int i = 0; i < list.length(); ++i) + ret.append(list[i].toString()); + + return ret; +} + +bool Python::Dict::delItem(const QString& key) +{ + return PyDict_DelItemString(m_smartPyObject->GetPointer(), + key.toLatin1().data()) == 0; +} + Python::Object Python::Dict::operator[](const QString& key) { return operator[](key.toLatin1().data()); diff --git a/tomviz/PythonUtilities.h b/tomviz/PythonUtilities.h index e320c24dd..fff3cdac6 100644 --- a/tomviz/PythonUtilities.h +++ b/tomviz/PythonUtilities.h @@ -107,6 +107,9 @@ class Python Dict(const Dict& other); Dict(const Object& obj); Dict& operator=(const Object& other); + long long size() const; + QStringList keys() const; + bool delItem(const QString& key); Object operator[](const QString& key); Object operator[](const char* key); void set(const QString& key, const Object& value); diff --git a/tomviz/Tvh5Format.cxx b/tomviz/Tvh5Format.cxx index 78131993f..f27d2238e 100644 --- a/tomviz/Tvh5Format.cxx +++ b/tomviz/Tvh5Format.cxx @@ -180,7 +180,6 @@ bool Tvh5Format::loadDataSource(h5::H5ReadWrite& reader, if (parent) { // This is a child data source. Hook it up to the operator parent. parent->setChildDataSource(dataSource); - parent->setHasChildDataSource(true); parent->newChildDataSource(dataSource); // If it has a parent, it will be deserialized later. } else { diff --git a/tomviz/modules/ModuleManager.cxx b/tomviz/modules/ModuleManager.cxx index 7a1daefc0..572b8b49e 100644 --- a/tomviz/modules/ModuleManager.cxx +++ b/tomviz/modules/ModuleManager.cxx @@ -440,6 +440,21 @@ QList ModuleManager::findModulesGeneric( return modules; } +void ModuleManager::moveModules(DataSource* from, DataSource* to) +{ + bool oldMoveObjectsEnabled = ActiveObjects::instance().moveObjectsEnabled(); + ActiveObjects::instance().setMoveObjectsMode(false); + auto view = ActiveObjects::instance().activeView(); + for (auto* module : findModules(from, nullptr)) { + // TODO: We should really copy the module properties as well. + auto* newModule = createAndAddModule(module->label(), to, view); + // Copy over properties using the serialization code. + newModule->deserialize(module->serialize()); + removeModule(module); + } + ActiveObjects::instance().setMoveObjectsMode(oldMoveObjectsEnabled); +} + QJsonArray jsonArrayFromXml(pugi::xml_node node) { // Simple function, just iterates through the elements and fills the array. diff --git a/tomviz/modules/ModuleManager.h b/tomviz/modules/ModuleManager.h index 65187df57..329d29f74 100644 --- a/tomviz/modules/ModuleManager.h +++ b/tomviz/modules/ModuleManager.h @@ -66,6 +66,9 @@ class ModuleManager : public QObject QList findModulesGeneric(const MoleculeSource* dataSource, const vtkSMViewProxy* view); + // Move modules from one data source to another + void moveModules(DataSource* from, DataSource* to); + /// Save the application state as JSON, use stateDir as the base for relative /// paths. bool serialize(QJsonObject& doc, const QDir& stateDir, diff --git a/tomviz/operators/Operator.cxx b/tomviz/operators/Operator.cxx index d20d3af8c..7fad28d0d 100644 --- a/tomviz/operators/Operator.cxx +++ b/tomviz/operators/Operator.cxx @@ -193,23 +193,26 @@ void Operator::createNewChildDataSource( const QString& label, vtkSmartPointer childData, DataSource::DataSourceType type, DataSource::PersistenceState state) { - if (this->childDataSource() == nullptr) { - DataSource* childDS = - new DataSource(vtkImageData::SafeDownCast(childData), type, - this->dataSource()->pipeline(), state); - childDS->setLabel(label); - setChildDataSource(childDS); - setHasChildDataSource(true); - emit Operator::newChildDataSource(childDS); + auto child = childDataSource(); + if (!child) { + child = new DataSource(vtkImageData::SafeDownCast(childData), type, + this->dataSource()->pipeline(), state); + child->setLabel(label); + setChildDataSource(child); + emit Operator::newChildDataSource(child); } // Reuse the existing "Output" data source. else { - childDataSource()->setData(childData); - childDataSource()->setLabel(label); - childDataSource()->setForkable(true); - childDataSource()->dataModified(); - setHasChildDataSource(true); + child->setData(childData); + child->setType(type); + child->setLabel(label); + child->setPersistenceState(state); + child->dataModified(); } + // TODO: the following should be set to this, once we get + // intermediate datasets working. + // child->setForkable(hasChildDataSource()); + child->setForkable(false); } void Operator::cancelTransform() diff --git a/tomviz/operators/Operator.h b/tomviz/operators/Operator.h index c429ad959..7b33b3b0c 100644 --- a/tomviz/operators/Operator.h +++ b/tomviz/operators/Operator.h @@ -82,9 +82,19 @@ class Operator : public QObject virtual OperatorResult* resultAt(int index) const; /// Set whether the operator is expected to produce a child DataSource. + /// TODO: this is beginning to take on a new meaning, since every + /// operator now produces a child data source. The new meaning is: + /// should the child data source be kept and not thrown away when + /// another operator is performed on it? + /// Maybe we should change the naming? virtual void setHasChildDataSource(bool value); /// Get whether the operator is expected to produce a child DataSource. + /// TODO: this is beginning to take on a new meaning, since every + /// operator now produces a child data source. The new meaning is: + /// should the child data source be kept and not thrown away when + /// another operator is performed on it? + /// Maybe we should change the naming? virtual bool hasChildDataSource() const; /// Set the child DataSource. Can be nullptr. diff --git a/tomviz/operators/OperatorPython.cxx b/tomviz/operators/OperatorPython.cxx index fd598d4ef..dc2cb893e 100644 --- a/tomviz/operators/OperatorPython.cxx +++ b/tomviz/operators/OperatorPython.cxx @@ -167,13 +167,6 @@ OperatorPython::OperatorPython(DataSource* parentObject) this, SLOT(updateChildDataSource(vtkSmartPointer)), connectionType); - // This connection is needed so we can create new child data sources in the UI - // thread from a pipeline worker threads. - connect(this, SIGNAL(newChildDataSource(const QString&, - vtkSmartPointer)), - this, SLOT(createNewChildDataSource(const QString&, - vtkSmartPointer)), - connectionType); connect( this, SIGNAL(newOperatorResult(const QString&, vtkSmartPointer)), @@ -264,22 +257,26 @@ void OperatorPython::setJSONDescription(const QString& str) QJsonObject::size_type size = childDatasetArray.size(); if (size != 1) { qCritical() << "Only one child dataset is supported for now. Found" - << size << " but only the first will be used"; + << size << "but only one will be used"; } if (size > 0) { - setHasChildDataSource(true); QJsonObject dataSourceNode = childDatasetArray[0].toObject(); QJsonValueRef nameValue = dataSourceNode["name"]; QJsonValueRef labelValue = dataSourceNode["label"]; - if (!nameValue.isUndefined() && !nameValue.isNull() && - !labelValue.isUndefined() && !labelValue.isNull()) { + if (!nameValue.isUndefined() && !nameValue.isNull()) { + // This variable is currently unused m_childDataSourceName = nameValue.toString(); + } + if (!labelValue.isUndefined() && !labelValue.isNull()) { m_childDataSourceLabel = labelValue.toString(); - } else if (nameValue.isNull()) { - qCritical() << "No name given for child DataSet"; } else if (labelValue.isNull()) { qCritical() << "No label given for child DataSet"; } + + // TODO: when we add intermediate data sources, we should add + // an option to the description.json to keep intermediate data + // sources, such as this option. + // setHasChildDataSource(dataSourceNode["keepIntermediate"].toBool()); } } @@ -347,36 +344,32 @@ void OperatorPython::setScript(const QString& str) void OperatorPython::createChildDataSource() { - // Create child datasets in advance. Keep a map from DataSource to name - // so that we can match Python script return dictionary values containing - // child data after the script finishes. - if (hasChildDataSource() && !childDataSource()) { - // Create uninitialized data set as a placeholder for the data - vtkSmartPointer childData = - vtkSmartPointer::New(); - childData->ShallowCopy( - vtkImageData::SafeDownCast(dataSource()->dataObject())); - emit newChildDataSource(m_childDataSourceLabel, childData); - } + if (childDataSource()) + return; + + vtkNew childData; + childData->ShallowCopy(dataSource()->imageData()); + createNewChildDataSource(m_childDataSourceLabel, childData); } bool OperatorPython::updateChildDataSource(Python::Dict outputDict) { - if (hasChildDataSource()) { - Python::Object pyDataObject; - pyDataObject = outputDict[m_childDataSourceName]; - if (!pyDataObject.isValid()) { - qCritical() << "No child dataset named " << m_childDataSourceName - << "defined in dictionary returned from Python script.\n"; - return false; - } else { - vtkObjectBase* vtkobject = Python::VTK::convertToDataObject(pyDataObject); - vtkDataObject* dataObject = vtkDataObject::SafeDownCast(vtkobject); - if (dataObject) { - TemporarilyReleaseGil releaseMe; - emit childDataSourceUpdated(dataObject); - } - } + QStringList keys = outputDict.keys(); + if (keys.isEmpty()) + return false; + + if (keys.size() > 1) { + qCritical() << "Warning: more than one key was received in " + << "updateChildDataSource. Only one will be used"; + } + + Python::Object pyDataObject = outputDict[keys[0]]; + + vtkObjectBase* vtkobject = Python::VTK::convertToDataObject(pyDataObject); + vtkDataObject* dataObject = vtkDataObject::SafeDownCast(vtkobject); + if (dataObject) { + TemporarilyReleaseGil releaseMe; + emit childDataSourceUpdated(dataObject); } return true; @@ -385,21 +378,18 @@ bool OperatorPython::updateChildDataSource(Python::Dict outputDict) bool OperatorPython::updateChildDataSource( QMap> output) { - if (hasChildDataSource()) { - for (QMap>::const_iterator iter = - output.begin(); - iter != output.end(); ++iter) { - - if (iter.key() != m_childDataSourceName) { - qCritical() << "No child dataset named " << m_childDataSourceName - << "defined in dictionary returned from Python script.\n"; - return false; - } + if (output.isEmpty()) { + qCritical() << "No output found in updateChildDataSource"; + return false; + } - emit childDataSourceUpdated(iter.value()); - } + if (output.size() > 1) { + qCritical() << "Warning: more than one output was received in " + << "updateChildDataSource. Only one will be used"; } + emit childDataSourceUpdated(output.first()); + return true; } @@ -414,8 +404,6 @@ bool OperatorPython::applyTransform(vtkDataObject* data) Q_ASSERT(data); - createChildDataSource(); - Python::Object result; { Python python; @@ -457,18 +445,22 @@ bool OperatorPython::applyTransform(vtkDataObject* data) check = result.isDict(); } + // Record whether an output was encountered (i. e., a dict was + // returned from the python function that contains a child dataset) + bool outputEncountered = false; + bool errorEncountered = false; if (check) { Python python; Python::Dict outputDict = result.toDict(); - // Support setting child data from the output dictionary - errorEncountered = !updateChildDataSource(outputDict); - // Results (tables, etc.) for (int i = 0; i < m_resultNames.size(); ++i) { - Python::Object pyDataObject; - pyDataObject = outputDict[m_resultNames[i]]; + Python::Object pyDataObject = outputDict[m_resultNames[i]]; + + // Remove the item from the dictionary so that we do not think + // it is a child after this loop. + outputDict.delItem(m_resultNames[i]); if (!pyDataObject.isValid()) { errorEncountered = true; @@ -489,13 +481,24 @@ bool OperatorPython::applyTransform(vtkDataObject* data) } } - if (errorEncountered) { + // At this point, all results should have been removed from the + // dictionary. Only children should remain. + if (!errorEncountered && outputDict.size() > 0) { + outputEncountered = true; + errorEncountered = !updateChildDataSource(outputDict); + } + if (errorEncountered) { qCritical() << "Dictionary return from Python script is:\n" << outputDict.toString(); } } + if (outputEncountered) { + // Set the output data on the vtkDataObject + data->ShallowCopy(childDataSource()->dataObject()); + } + return !errorEncountered; } @@ -646,13 +649,27 @@ EditOperatorWidget* OperatorPython::getEditorContentsWithData( void OperatorPython::updateChildDataSource(vtkSmartPointer data) { - // Check to see if a child data source has already been created. If not, - // create it here. + // This function can be called either when a progress update is + // performed, or when an output dict was returned from a python + // operation. Both are stored in the child data source. + + if (!childDataSource()) + createChildDataSource(); + auto dataSource = childDataSource(); Q_ASSERT(dataSource); // Now deep copy the new data to the child source data if needed dataSource->copyData(data); + + if (dataSource->hasTiltAngles()) + dataSource->setType(DataSource::TiltSeries); + + // Keep the same active scalars that the parent had if possible + QString prevActiveScalars = this->dataSource()->activeScalars(); + if (dataSource->listScalars().contains(prevActiveScalars)) + dataSource->setActiveScalars(prevActiveScalars); + emit dataSource->dataChanged(); emit dataSource->dataPropertiesChanged(); ActiveObjects::instance().renderAllViews(); diff --git a/tomviz/operators/ReconstructionOperator.cxx b/tomviz/operators/ReconstructionOperator.cxx index 46caeb0dd..80bf6967d 100644 --- a/tomviz/operators/ReconstructionOperator.cxx +++ b/tomviz/operators/ReconstructionOperator.cxx @@ -36,7 +36,6 @@ ReconstructionOperator::ReconstructionOperator(DataSource* source, QObject* p) } setSupportsCancel(true); setTotalProgressSteps(m_extent[1] - m_extent[0] + 1); - setHasChildDataSource(true); connect( this, static_castShallowCopy(reconstructionImage); return true; } } // namespace tomviz diff --git a/tomviz/python/tomviz/executor.py b/tomviz/python/tomviz/executor.py index 0be4f5ca2..a977cdc16 100644 --- a/tomviz/python/tomviz/executor.py +++ b/tomviz/python/tomviz/executor.py @@ -10,7 +10,6 @@ import abc import stat import json -import six import errno from tqdm import tqdm @@ -684,7 +683,7 @@ def _load_transform_functions(operators): def _write_child_data(result, operator_index, output_file_path, dims): - for (label, dataobject) in six.iteritems(result): + for (label, dataobject) in result.items(): # Only need write out data if the operator made updates. output_path = '.' if output_file_path is not None: @@ -738,6 +737,9 @@ def execute(operators, start_at, data_file_path, output_file_path, if dims is not None: # Convert to native type, as is required by itk data.spacing = [float(d.values[1] - d.values[0]) for d in dims] + if data.tilt_angles is not None and data.tilt_axis is not None: + # Spacing for tilt axis should just be 1.0 + data.spacing[data.tilt_axis] = 1.0 operators = operators[start_at:] transforms = _load_transform_functions(operators) @@ -755,6 +757,12 @@ def execute(operators, start_at, data_file_path, output_file_path, _write_child_data(result, operator_index, output_file_path, dims) + # Update the data with the result + if len(result) > 1: + logger.warning('Multiple results found. ' + 'Only one will be used') + data = next(iter(result.values())) + progress.finished(operator_index) operator_index += 1 diff --git a/tomviz/python/tomviz/external_dataset.py b/tomviz/python/tomviz/external_dataset.py index b60aeeeca..a05138dcd 100644 --- a/tomviz/python/tomviz/external_dataset.py +++ b/tomviz/python/tomviz/external_dataset.py @@ -31,7 +31,9 @@ def active_scalars(self): @active_scalars.setter def active_scalars(self, array): - self.arrays[self.active_name] = array + if self.active_name is None: + self.active_name = 'Scalars' + self.set_scalars(self.active_name, array) @property def scalars_names(self): @@ -42,6 +44,20 @@ def scalars(self, name=None): name = self.active_name return self.arrays[name] + def set_scalars(self, name, array): + if self.active_name is None: + self.active_name = name + + # Throw away any arrays whose sizes don't match + for n, a in list(self.arrays.items()): + if n != name and a.shape != array.shape: + print('Warning: deleting array', n, 'because its shape', + a.shape, 'does not match the shape of new array', name, + array.shape) + del self.arrays[n] + + self.arrays[name] = array + @property def spacing(self): return self._spacing @@ -55,13 +71,18 @@ def spacing(self, v): self._spacing = v - def create_child_dataset(self): - child = copy.deepcopy(self) - # Set tilt angles to None to be consistent with internal dataset - child.tilt_angles = None - # If the parent had tilt angles, set the spacing of the tilt - # axis to match that of x, as is done in the internal dataset - if self.tilt_angles is not None and self.spacing is not None: - s = self.spacing - child.spacing = [s[0], s[1], s[0]] + def create_child_dataset(self, volume=True): + # Create an empty dataset with the same spacing as the parent + child = Dataset({}) + + child.spacing = copy.deepcopy(self.spacing) + + if volume: + if self.tilt_axis is not None: + # Ignore the tilt angle spacing. Set it to another spacing. + child.spacing[self.tilt_axis] = child.spacing[1] + else: + child.tilt_angles = copy.deepcopy(self.tilt_angles) + child.tilt_axis = self.tilt_axis + return child diff --git a/tomviz/python/tomviz/internal_dataset.py b/tomviz/python/tomviz/internal_dataset.py index 28a1d4a4f..706a95a97 100644 --- a/tomviz/python/tomviz/internal_dataset.py +++ b/tomviz/python/tomviz/internal_dataset.py @@ -23,6 +23,9 @@ def scalars_names(self): def scalars(self, name=None): return utils.get_array(self._data_object, name) + def set_scalars(self, name, array): + utils.set_array(self._data_object, array, name=name) + @property def spacing(self): return utils.get_spacing(self._data_object) @@ -59,8 +62,8 @@ def white(self): return None return utils.get_array(self._data_source.white_data) - def create_child_dataset(self): - new_data = utils.make_child_dataset(self._data_object) + def create_child_dataset(self, volume=True): + new_data = utils.make_child_dataset(self._data_object, volume) return Dataset(new_data, self._data_source) diff --git a/tomviz/python/tomviz/utils.py b/tomviz/python/tomviz/utils.py index 0c5c5028f..e7b156c23 100644 --- a/tomviz/python/tomviz/utils.py +++ b/tomviz/python/tomviz/utils.py @@ -91,10 +91,7 @@ def arrays(dataobject): @with_vtk_dataobject -def set_array(dataobject, newarray, minextent=None, isFortran=True): - # Set the extent if needed, i.e. if the minextent is not the same as - # the data object starting index, or if the newarray shape is not the same - # as the size of the dataobject. +def set_array(dataobject, newarray, isFortran=True, name=None): # isFortran indicates whether the NumPy array has Fortran-order indexing, # i.e. i,j,k indexing. If isFortran is False, then the NumPy array uses # C-order indexing, i.e. k,j,i indexing. @@ -119,28 +116,37 @@ def set_array(dataobject, newarray, minextent=None, isFortran=True): if not is_numpy_vtk_type(arr): arr = arr.astype(np.float32) - if minextent is None: - minextent = dataobject.GetExtent()[::2] - sameindex = list(minextent) == list(dataobject.GetExtent()[::2]) - sameshape = list(vtkshape) == list(dataobject.GetDimensions()) - if not sameindex or not sameshape: - extent = 6*[0] - extent[::2] = minextent - extent[1::2] = \ - [x + y - 1 for (x, y) in zip(minextent, vtkshape)] - dataobject.SetExtent(extent) - # Now replace the scalars array with the new array. vtkarray = np_s.numpy_to_vtk(arr) vtkarray.Association = dsa.ArrayAssociation.POINT do = dsa.WrapDataObject(dataobject) - oldscalars = do.PointData.GetScalars() - arrayname = "Scalars" - if oldscalars is not None: - arrayname = oldscalars.GetName() - del oldscalars - do.PointData.append(arr, arrayname) - do.PointData.SetActiveScalars(arrayname) + pd = do.PointData + + if name is None: + oldscalars = pd.GetScalars() + if oldscalars is not None: + name = oldscalars.GetName() + else: + name = 'Scalars' + + if list(vtkshape) != list(dataobject.GetDimensions()): + old_dims = tuple(dataobject.GetDimensions()) + # Find the arrays to remove + num_arrays = pd.GetNumberOfArrays() + arr_names = [pd.GetArray(i).GetName() for i in range(num_arrays)] + arr_names = [x for x in arr_names if x != name] + for n in arr_names: + print('Warning: deleting array', n, 'because its shape', + old_dims, 'does not match the shape of the new array', + vtkshape) + pd.RemoveArray(n) + + # Now set the new dimensions + dataobject.SetDimensions(vtkshape) + + pd.append(arr, name) + if pd.GetNumberOfArrays() == 1: + pd.SetActiveScalars(name) @with_vtk_dataobject @@ -192,15 +198,22 @@ def get_coordinate_arrays(dataobject): @with_vtk_dataobject -def make_child_dataset(dataobject): +def make_child_dataset(dataobject, volume=True): """Creates a child dataset with the same size as the reference_dataset. """ from vtk import vtkImageData new_child = vtkImageData() new_child.CopyStructure(dataobject) - input_spacing = dataobject.GetSpacing() - # For a reconstruction we copy the X spacing from the input dataset - child_spacing = (input_spacing[0], input_spacing[1], input_spacing[0]) + child_spacing = list(dataobject.GetSpacing()) + tilt_angles = get_tilt_angles(dataobject) + + if tilt_angles is not None: + if volume: + # For a reconstruction we copy the X spacing from the input dataset + child_spacing[2] = child_spacing[0] + else: + set_tilt_angles(new_child, tilt_angles) + new_child.SetSpacing(child_spacing) return new_child