From cce35ea93680d3df440cea76db578b0ee6c5fa9e Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Sun, 1 Mar 2020 16:31:18 -0500 Subject: [PATCH 01/14] Make child dataset for external match internal Previously, the child dataset for the external pipeline would create a deep copy of the parent dataset. This was not done in the internal dataset. Instead, in the internal dataset, the child dataset would be empty, and it would only have spacing and dimensions that matched that of the parent (with the exception of when the parent was a tilt series, then the spacing of the tilt axis of the child would be altered). Additionally, in the internal dataset, if the active_scalars were set on the empty child dataset, a new scalars named 'Scalars' would be created. The changes in this commit cause the external dataset to match the behavior of the internal dataset, in that, the child dataset is an empty dataset with the same spacing as that of the parent (except for the tilt series parent case), and if the active scalars get set on an empty child dataset, a new scalars named 'Scalars' would be created. Signed-off-by: Patrick Avery --- tomviz/python/tomviz/external_dataset.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tomviz/python/tomviz/external_dataset.py b/tomviz/python/tomviz/external_dataset.py index b60aeeeca..62daa25fa 100644 --- a/tomviz/python/tomviz/external_dataset.py +++ b/tomviz/python/tomviz/external_dataset.py @@ -31,6 +31,9 @@ def active_scalars(self): @active_scalars.setter def active_scalars(self, array): + if self.active_name is None: + self.active_name = 'Scalars' + self.arrays[self.active_name] = array @property @@ -56,12 +59,13 @@ 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]] + # Create an empty dataset with the same spacing as the parent + child = Dataset({}) + + if self.spacing is not None: + child.spacing = copy.deepcopy(self.spacing) + if self.tilt_angles is not None and self.tilt_axis is not None: + # Ignore the tilt angle spacing. Set it to another spacing. + child.spacing[self.tilt_axis] = child.spacing[1] + return child From 4ab9bdee026610db7f8b4f39499ba5bd987a04ca Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Thu, 5 Mar 2020 08:38:34 -0500 Subject: [PATCH 02/14] Allow tilt series child data sources Previously, child data sources from python operators could only be volumes. However, new changes coming soon will allow child data sources to be tilt series as well. Add in a check so that a child data source is appropriately marked as a tilt series if it has tilt angles. Signed-off-by: Patrick Avery --- tomviz/operators/OperatorPython.cxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tomviz/operators/OperatorPython.cxx b/tomviz/operators/OperatorPython.cxx index fd598d4ef..88173c0fc 100644 --- a/tomviz/operators/OperatorPython.cxx +++ b/tomviz/operators/OperatorPython.cxx @@ -653,6 +653,10 @@ void OperatorPython::updateChildDataSource(vtkSmartPointer data) // Now deep copy the new data to the child source data if needed dataSource->copyData(data); + + if (dataSource->hasTiltAngles()) + dataSource->setType(DataSource::TiltSeries); + emit dataSource->dataChanged(); emit dataSource->dataPropertiesChanged(); ActiveObjects::instance().renderAllViews(); From e8ac3cf88dd6dc7571bb6631ea7da7bacaffba1b Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Mon, 2 Mar 2020 07:28:43 -0500 Subject: [PATCH 03/14] Allow non-active scalars to be modified in python In the python operator "Dataset" classes, allow the non-active scalars to be modified via "set_scalars()". If there are scalars already present in the dataset, and the newly set scalars have different dimensions compared to the other scalars, the other scalars are discarded. Additionally, this allows a child dataset to be either a volume or another tilt series, so that the user can indicate whether the tilt angles and spacing should be preserved. Signed-off-by: Patrick Avery --- tomviz/python/tomviz/external_dataset.py | 29 ++++++++--- tomviz/python/tomviz/internal_dataset.py | 7 ++- tomviz/python/tomviz/utils.py | 65 ++++++++++++++---------- 3 files changed, 67 insertions(+), 34 deletions(-) diff --git a/tomviz/python/tomviz/external_dataset.py b/tomviz/python/tomviz/external_dataset.py index 62daa25fa..a05138dcd 100644 --- a/tomviz/python/tomviz/external_dataset.py +++ b/tomviz/python/tomviz/external_dataset.py @@ -33,8 +33,7 @@ def active_scalars(self): def active_scalars(self, array): if self.active_name is None: self.active_name = 'Scalars' - - self.arrays[self.active_name] = array + self.set_scalars(self.active_name, array) @property def scalars_names(self): @@ -45,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 @@ -58,14 +71,18 @@ def spacing(self, v): self._spacing = v - def create_child_dataset(self): + def create_child_dataset(self, volume=True): # Create an empty dataset with the same spacing as the parent child = Dataset({}) - if self.spacing is not None: - child.spacing = copy.deepcopy(self.spacing) - if self.tilt_angles is not None and self.tilt_axis is not None: + 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 From 732628907a33ba18af9ed0b78657406919e16d0f Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Thu, 5 Mar 2020 09:48:19 -0500 Subject: [PATCH 04/14] Give the tilt axis a spacing of 1.0 In the EMD format, the spacing of the tilt axis was previously calculated by comparing the distance between the first and second angles. Other formats, though, like the GenericHDF5 format, do not give the tilt axis a spacing. For consistency, and so that the data does not change spacing when a GenericHDF5 file is loaded and used in an external pipeline, use a spacing of 1.0 both in the EMD format, and in the pipeline executor. Signed-off-by: Patrick Avery --- tomviz/EmdFormat.cxx | 5 +++++ tomviz/python/tomviz/executor.py | 3 +++ 2 files changed, 8 insertions(+) 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/python/tomviz/executor.py b/tomviz/python/tomviz/executor.py index 0be4f5ca2..88f7447c1 100644 --- a/tomviz/python/tomviz/executor.py +++ b/tomviz/python/tomviz/executor.py @@ -738,6 +738,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) From 1b7472094b3dfb14efa69f55897ff6089be00ed7 Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Thu, 5 Mar 2020 09:50:26 -0500 Subject: [PATCH 05/14] Use same active scalars as parent if possible For the internal pipeline, try to set the child data sources' active scalars to match that of the parent if possible. This prevents the active scalars changing due to operations that produce child data sources. Signed-off-by: Patrick Avery --- tomviz/operators/OperatorPython.cxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tomviz/operators/OperatorPython.cxx b/tomviz/operators/OperatorPython.cxx index 88173c0fc..cb1a4d63a 100644 --- a/tomviz/operators/OperatorPython.cxx +++ b/tomviz/operators/OperatorPython.cxx @@ -657,6 +657,11 @@ void OperatorPython::updateChildDataSource(vtkSmartPointer 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(); From 42c8ea0b433b010ad1b7132c9c14c001e98c0be3 Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Tue, 10 Mar 2020 09:51:46 -0400 Subject: [PATCH 06/14] Check number of scalars in properties panel update When live updates are performed on data that contains multiple scalars, the number of scalars can change in between updates. Add in a safety check in the DataPropertiesPanel to account for this. This prevents a crash. Signed-off-by: Patrick Avery --- tomviz/DataPropertiesPanel.cxx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 From a9173445aa763fbd13dfc1ab7bafcacc9e0d787b Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Tue, 31 Mar 2020 07:07:35 -0400 Subject: [PATCH 07/14] Add method to get keys from python dict Signed-off-by: Patrick Avery --- tomviz/PythonUtilities.cxx | 11 +++++++++++ tomviz/PythonUtilities.h | 1 + 2 files changed, 12 insertions(+) diff --git a/tomviz/PythonUtilities.cxx b/tomviz/PythonUtilities.cxx index b756bda63..cd3237049 100644 --- a/tomviz/PythonUtilities.cxx +++ b/tomviz/PythonUtilities.cxx @@ -221,6 +221,17 @@ Python::Dict& Python::Dict::operator=(const Python::Object& other) return *this; } +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; +} + 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..36fcb1d79 100644 --- a/tomviz/PythonUtilities.h +++ b/tomviz/PythonUtilities.h @@ -107,6 +107,7 @@ class Python Dict(const Dict& other); Dict(const Object& obj); Dict& operator=(const Object& other); + QStringList keys() const; Object operator[](const QString& key); Object operator[](const char* key); void set(const QString& key, const Object& value); From 7e21bdef37e3959379dc8115a390c900932dbecd Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Tue, 31 Mar 2020 09:09:26 -0400 Subject: [PATCH 08/14] Add Dict functions to get size and delete items Signed-off-by: Patrick Avery --- tomviz/PythonUtilities.cxx | 11 +++++++++++ tomviz/PythonUtilities.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/tomviz/PythonUtilities.cxx b/tomviz/PythonUtilities.cxx index cd3237049..a4a78fc20 100644 --- a/tomviz/PythonUtilities.cxx +++ b/tomviz/PythonUtilities.cxx @@ -221,6 +221,11 @@ Python::Dict& Python::Dict::operator=(const Python::Object& other) return *this; } +ssize_t Python::Dict::size() const +{ + return PyDict_Size(m_smartPyObject->GetPointer()); +} + QStringList Python::Dict::keys() const { QStringList ret; @@ -232,6 +237,12 @@ QStringList Python::Dict::keys() const 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 36fcb1d79..3d5fcdc2c 100644 --- a/tomviz/PythonUtilities.h +++ b/tomviz/PythonUtilities.h @@ -107,7 +107,9 @@ class Python Dict(const Dict& other); Dict(const Object& obj); Dict& operator=(const Object& other); + ssize_t 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); From 73ba20ecfa5fe972c61c81ee28d5818a1419e746 Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Tue, 31 Mar 2020 20:14:14 -0400 Subject: [PATCH 09/14] Added moveModules function to ModuleManager This is for a little more general use than the previous "Pipeline::moveModulesDown()" was. Signed-off-by: Patrick Avery --- tomviz/Pipeline.cxx | 23 +++-------------------- tomviz/Pipeline.h | 3 --- tomviz/modules/ModuleManager.cxx | 15 +++++++++++++++ tomviz/modules/ModuleManager.h | 3 +++ 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/tomviz/Pipeline.cxx b/tomviz/Pipeline.cxx index 78ad0237d..7a90f3c75 100644 --- a/tomviz/Pipeline.cxx +++ b/tomviz/Pipeline.cxx @@ -397,13 +397,14 @@ void Pipeline::branchFinished() if (newChildDataSource != nullptr) { emit lastOp->newChildDataSource(newChildDataSource); // Move modules from root data source. - moveModulesDown(newChildDataSource); + ModuleManager::instance().moveModules(dataSource(), newChildDataSource); } } else { // If this is the only operator, make sure the modules are moved down. if (start->operators().size() == 1) - moveModulesDown(lastOp->childDataSource()); + ModuleManager::instance().moveModules(dataSource(), + lastOp->childDataSource()); } } @@ -607,24 +608,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..dbe6d179d 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; @@ -166,7 +164,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/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, From 0ea37899ee8637d5958e82a5a311f25dabda3954 Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Wed, 1 Apr 2020 07:18:24 -0400 Subject: [PATCH 10/14] Remove unused header Signed-off-by: Patrick Avery --- tomviz/ExternalPythonExecutor.cxx | 1 - 1 file changed, 1 deletion(-) 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" From 5c027e669026671f3f354134c81b7cbedcf17b34 Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Tue, 7 Apr 2020 04:05:46 -0400 Subject: [PATCH 11/14] Allow operators after child-producing operators Previously, if other operators were performed after a child-producing operator (such as reconstruction), the behavior would be somewhat unpredictable, depending on several factors, including whether a description.json file was present, and whether the pipeline was internal or external. This fixes the behavior so that operators can be performed after child-producing operators. This was done by making sure that the output from child-producing operators gets passed down as input to the next operator. This needed to be done both in the external pipeline, and in the internal pipeline. Additionally, this also required that modules be passed down to the output of the next operator as well. This commit additionally allows child-producing operators to work in the internal pipeline without the description.json file present, which was not possible before (it required that the child be named explicitly). The external pipeline should not emit that the pipeline is finished if a child datasource is encountered. That was removed. The snapshot operator does not have any useful behavior, currently. But it should work well with intermediate data sources, since it will essentially create an intermediate data source. Signed-off-by: Patrick Avery --- tomviz/Pipeline.cxx | 190 ++++++++++++-------- tomviz/Pipeline.h | 1 + tomviz/PipelineExecutor.cxx | 14 +- tomviz/PipelineModel.cxx | 2 - tomviz/Tvh5Format.cxx | 1 - tomviz/operators/Operator.cxx | 29 +-- tomviz/operators/Operator.h | 10 ++ tomviz/operators/OperatorPython.cxx | 130 +++++++------- tomviz/operators/ReconstructionOperator.cxx | 3 +- tomviz/python/tomviz/executor.py | 9 +- 10 files changed, 218 insertions(+), 171 deletions(-) diff --git a/tomviz/Pipeline.cxx b/tomviz/Pipeline.cxx index 7a90f3c75..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,57 +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. - ModuleManager::instance().moveModules(dataSource(), 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) - ModuleManager::instance().moveModules(dataSource(), - 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() @@ -483,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. @@ -509,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 diff --git a/tomviz/Pipeline.h b/tomviz/Pipeline.h index dbe6d179d..a93e6be3c 100644 --- a/tomviz/Pipeline.h +++ b/tomviz/Pipeline.h @@ -143,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. 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..e84c84136 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()) { 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/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 cb1a4d63a..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,8 +649,13 @@ 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); 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 88f7447c1..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: @@ -758,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 From f4c2d6609c0000053be0d491e0827c6babb7da9e Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Tue, 7 Apr 2020 11:00:30 -0400 Subject: [PATCH 12/14] Use long long instead of ssize_t ssize_t is producing an error on Windows. Signed-off-by: Patrick Avery --- tomviz/PythonUtilities.cxx | 3 ++- tomviz/PythonUtilities.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tomviz/PythonUtilities.cxx b/tomviz/PythonUtilities.cxx index a4a78fc20..30e149472 100644 --- a/tomviz/PythonUtilities.cxx +++ b/tomviz/PythonUtilities.cxx @@ -221,8 +221,9 @@ Python::Dict& Python::Dict::operator=(const Python::Object& other) return *this; } -ssize_t Python::Dict::size() const +long long Python::Dict::size() const { + // If we use ssize_t, we get an error on Windows... return PyDict_Size(m_smartPyObject->GetPointer()); } diff --git a/tomviz/PythonUtilities.h b/tomviz/PythonUtilities.h index 3d5fcdc2c..fff3cdac6 100644 --- a/tomviz/PythonUtilities.h +++ b/tomviz/PythonUtilities.h @@ -107,7 +107,7 @@ class Python Dict(const Dict& other); Dict(const Object& obj); Dict& operator=(const Object& other); - ssize_t size() const; + long long size() const; QStringList keys() const; bool delItem(const QString& key); Object operator[](const QString& key); From 44823813848fcfe74f0d92d6e4f8693b4819833a Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Thu, 9 Apr 2020 15:07:26 -0400 Subject: [PATCH 13/14] When a datasource is destroyed, remove all modules This ensures the modules get cleaned up. Signed-off-by: Patrick Avery --- tomviz/DataSource.cxx | 1 + 1 file changed, 1 insertion(+) 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); From b4616b7381404cc8760ed81acf3667f6c188b5d5 Mon Sep 17 00:00:00 2001 From: Patrick Avery Date: Thu, 9 Apr 2020 17:22:33 -0400 Subject: [PATCH 14/14] Do not emit data modified after pipeline finishes There is a new datasource every time now, so this is not necessary. Additionally, this is causing crashing when closing the application when operators are present, and it is also causing crashing occasionally when deleting datasources that have operators. Signed-off-by: Patrick Avery --- tomviz/PipelineModel.cxx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tomviz/PipelineModel.cxx b/tomviz/PipelineModel.cxx index e84c84136..a98e36e24 100644 --- a/tomviz/PipelineModel.cxx +++ b/tomviz/PipelineModel.cxx @@ -777,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()) {