Merge pull request #589 from lassoan/fix_dataset_array_combobox_crash

Fixed ctkVTKDataSetArrayComboBox crashing and not updating

Libs/Visualization/VTK/Widgets/Testing/Cpp/ctkVTKDataSetArrayComboBoxTest1.cpp

@@ -57,6 +57,35 @@ int ctkVTKDataSetArrayComboBoxTest1(int argc, char * argv [] )
   ctkVTKDataSetArrayComboBox comboBox;
   comboBox.dataSetModel()->setAttributeTypes(ctkVTKDataSetModel::AllAttribute);
   comboBox.setDataSet(dataSet.GetPointer());
+
+  if (comboBox.count()!=2)
+    {
+    std::cerr << "Line " << __LINE__ << " - Expected 2 items in the combobox\n"
+                  "\tCurrent count: " << comboBox.count() << "\n";
+    return EXIT_FAILURE;
+    }
+
+  comboBox.setNoneEnabled(true);
+  if (comboBox.count()!=3)
+    {
+    std::cerr << "Line " << __LINE__ << " - Expected 3 items in the combobox\n"
+                  "\tCurrent count: " << comboBox.count() << "\n";
+    return EXIT_FAILURE;
+    }
+  if (!comboBox.itemText(0).isEmpty())
+    {
+    std::cerr << "Line " << __LINE__ << " - First combo box item text is expected to be empty\n";
+    return EXIT_FAILURE;
+    }
+
+  comboBox.setNoneEnabled(false);
+  if (comboBox.count()!=2)
+    {
+    std::cerr << "Line " << __LINE__ << " - Expected 2 items in the combobox\n"
+                  "\tCurrent count: " << comboBox.count() << "\n";
+    return EXIT_FAILURE;
+    }
+
   comboBox.show();
 
   if (argc < 2 || QString(argv[1]) != "-I")

Libs/Visualization/VTK/Widgets/Testing/Cpp/ctkVTKDataSetModelTest1.cpp

@@ -336,6 +336,66 @@ int ctkVTKDataSetModelTest1(int argc, char * argv [] )
     return EXIT_FAILURE;
     }
 
+  dataSetModel.setAttributeTypes(ctkVTKDataSetModel::NoAttribute | ctkVTKDataSetModel::ScalarsAttribute);
+  vtkNew<vtkIntArray> intsLaterAdded;
+  {
+    intsLaterAdded->SetName("IntsLaterAdded");
+    int added = dataSet->GetPointData()->AddArray(intsLaterAdded.GetPointer());
+    locations[intsLaterAdded.GetPointer()] = vtkAssignAttribute::POINT_DATA;
+    if (added == -1)
+      {
+      std::cerr << "Line " << __LINE__ << " - Failed to add intsLaterAdded array";
+      return EXIT_FAILURE;
+      }
+  }
+  if (!checkItems(__LINE__, QList<vtkAbstractArray*>() << notAttributeArrays << intsLaterAdded.GetPointer(),
+                  &dataSetModel, locations))
+    {
+    return EXIT_FAILURE;
+    }
+
+  QList<QStandardItem*> items = dataSetModel.findItems("");
+  if(items.count() != 0)
+    {
+    std::cerr << "Line " << __LINE__ << " - Expected 0 NULL item\n"
+                  "\tCurrent count: " << items.count() << "\n";
+    return EXIT_FAILURE;
+    }
+
+  dataSetModel.setIncludeNullItem(true);
+  items = dataSetModel.findItems("");
+  if(items.count() != 1)
+    {
+    std::cerr << "Line " << __LINE__ << " - Expected 1 NULL item\n"
+                  "\tCurrent count: " << items.count() << "\n";
+    return EXIT_FAILURE;
+    }
+  vtkAbstractArray * currentDataArray = dataSetModel.arrayFromItem(items.at(0));
+  if (currentDataArray != 0)
+    {
+    std::cerr << "Line " << __LINE__ << " - Problem with model - Incorrect array associated with NULL item:\n"
+                  "\tExpected: 0\n"
+                  "\tCurrent: " << currentDataArray << "\n";
+    return EXIT_FAILURE;
+    }
+  int loc = dataSetModel.locationFromItem(items.at(0));
+  if (loc != dataSetModel.nullItemLocation())
+    {
+    std::cerr << "Line " << __LINE__ << " - Problem with model - Incorrect location associated with NULL item:\n"
+                  "\tExpected: " << dataSetModel.nullItemLocation() << "\n"
+                  "\tCurrent: " << loc << "\n";
+    return EXIT_FAILURE;
+    }
+
+  dataSetModel.setIncludeNullItem(false);
+  items = dataSetModel.findItems("");
+  if(items.count() != 0)
+    {
+    std::cerr << "Line " << __LINE__ << " - Expected 0 NULL item\n"
+                  "\tCurrent count: " << items.count() << "\n";
+    return EXIT_FAILURE;
+    }
+
   dataSetModel.setAttributeTypes(ctkVTKDataSetModel::AllAttribute);
   if (!checkItems(__LINE__, QList<vtkAbstractArray*>() << notAttributeArrays
                   << scalars.GetPointer() << vectors.GetPointer()

Libs/Visualization/VTK/Widgets/ctkVTKDataSetArrayComboBox.cpp

@@ -168,6 +168,20 @@ void ctkVTKDataSetArrayComboBox::setAttributeTypes(const ctkVTKDataSetModel::Att
   this->dataSetModel()->setAttributeTypes(attributeTypes);
 }
 
+// ----------------------------------------------------------------------------
+bool ctkVTKDataSetArrayComboBox::noneEnabled()const
+{
+  Q_D(const ctkVTKDataSetArrayComboBox);
+  return this->dataSetModel()->includeNullItem();
+}
+
+// ----------------------------------------------------------------------------
+void ctkVTKDataSetArrayComboBox::setNoneEnabled(bool noneEnabled)
+{
+  Q_D(ctkVTKDataSetArrayComboBox);
+  return this->dataSetModel()->setIncludeNullItem(noneEnabled);
+}
+
 // --------------------------------------------------------------------------
 ctkVTKDataSetModel* ctkVTKDataSetArrayComboBox::dataSetModel()const
 {

Libs/Visualization/VTK/Widgets/ctkVTKDataSetArrayComboBox.h

@@ -40,6 +40,7 @@ class CTK_VISUALIZATION_VTK_WIDGETS_EXPORT ctkVTKDataSetArrayComboBox
 {
   Q_OBJECT
   Q_PROPERTY(ctkVTKDataSetModel::AttributeTypes attributeTypes READ attributeTypes WRITE setAttributeTypes)
+  Q_PROPERTY(bool noneEnabled READ noneEnabled WRITE setNoneEnabled)
 
 public:
   /// Superclass typedef
@@ -61,6 +62,15 @@ public:
   ctkVTKDataSetModel::AttributeTypes attributeTypes()const;
   void setAttributeTypes(const ctkVTKDataSetModel::AttributeTypes& attributeTypes);
 
+  /// Set/Get NoneEnabled flags
+  /// An additional empty item is added into the list, where the user can select.
+  /// It is recommended to enable this if the combobox is used to select active scalar of the
+  /// observed VTK data set, because if there is no None option is available then the combobox selects
+  /// the first array automatically if an array becomes available, causing unintended change of the VTK data set
+  /// (and often infinite loop of widget/MRML node updates).
+  void setNoneEnabled(bool enable);
+  bool noneEnabled()const; 
+
   /// Return a pointer to the model used to populate the combobox.
   /// \sa dataSet()
   ctkVTKDataSetModel* dataSetModel()const;

Libs/Visualization/VTK/Widgets/ctkVTKDataSetModel.cpp

@@ -47,8 +47,12 @@ public:
                                                      vtkDataSetAttributes * dataSetAttributes);
 
   vtkSmartPointer<vtkDataSet> DataSet;
+  vtkSmartPointer<vtkPointData> DataSetPointData;
+  vtkSmartPointer<vtkCellData> DataSetCellData;
+
   bool ListenAbstractArrayModifiedEvent;
   ctkVTKDataSetModel::AttributeTypes AttributeType;
+  bool IncludeNullItem;
 };
 
 
@@ -58,6 +62,7 @@ ctkVTKDataSetModelPrivate::ctkVTKDataSetModelPrivate(ctkVTKDataSetModel& object)
 {
   this->ListenAbstractArrayModifiedEvent = false;
   this->AttributeType = ctkVTKDataSetModel::AllAttribute;
+  this->IncludeNullItem = false;
 }
 
 //------------------------------------------------------------------------------
@@ -138,6 +143,7 @@ QList<vtkAbstractArray*> ctkVTKDataSetModelPrivate::attributeArrayToInsert(
 ctkVTKDataSetModel::ctkVTKDataSetModel(QObject *_parent)
   : QStandardItemModel(_parent)
   , d_ptr(new ctkVTKDataSetModelPrivate(*this))
+  , NullItemLocation(-2) // -1 is already used
 {
   Q_D(ctkVTKDataSetModel);
   d->init();
@@ -168,7 +174,7 @@ void ctkVTKDataSetModel::setDataSet(vtkDataSet* dataSet)
   this->qvtkReconnect(d->DataSet, dataSet, vtkCommand::ModifiedEvent,
                       this, SLOT(onDataSetModified(vtkObject*)) );
   d->DataSet = dataSet;
-  this->updateDataSet();
+  this->onDataSetModified(dataSet);
 }
 
 //------------------------------------------------------------------------------
@@ -197,6 +203,33 @@ void ctkVTKDataSetModel::setAttributeTypes(const AttributeTypes& attributeTypes)
   this->updateDataSet();
 }
 
+// ----------------------------------------------------------------------------
+bool ctkVTKDataSetModel::includeNullItem()const
+{
+  Q_D(const ctkVTKDataSetModel);
+  return d->IncludeNullItem;
+}
+
+// ----------------------------------------------------------------------------
+void ctkVTKDataSetModel::setIncludeNullItem(bool includeNullItem)
+{
+  Q_D(ctkVTKDataSetModel);
+  if (d->IncludeNullItem == includeNullItem)
+    {
+    // no change
+    return;
+    }
+  if (includeNullItem)
+    {
+    this->insertNullItem();
+    }
+  else
+    {
+    this->removeNullItem();
+    }
+  d->IncludeNullItem = includeNullItem;
+}
+
 //------------------------------------------------------------------------------
 vtkAbstractArray* ctkVTKDataSetModel::arrayFromItem(QStandardItem* arrayItem)const
 {
@@ -208,6 +241,13 @@ vtkAbstractArray* ctkVTKDataSetModel::arrayFromItem(QStandardItem* arrayItem)con
   Q_ASSERT(arrayPointer.isValid());
   vtkAbstractArray* array = static_cast<vtkAbstractArray*>(
     reinterpret_cast<void *>(arrayPointer.toLongLong()));
+  if (arrayItem->data(ctkVTK::LocationRole).toInt() == this->NullItemLocation)
+    {
+    // null item
+    Q_ASSERT(array==0);
+    return 0;
+    }
+
   Q_ASSERT(array);
   return array;
 }
@@ -277,7 +317,23 @@ bool ctkVTKDataSetModel::listenNodeModifiedEvent()const
 void ctkVTKDataSetModel::updateDataSet()
 {
   Q_D(ctkVTKDataSetModel);
-  this->setRowCount(0);
+
+  // Remove all items (except the first one, if there is a NULL item)
+  if (d->IncludeNullItem)
+    {
+    if (this->rowCount()<1)
+      {
+      this->insertNullItem();
+      }
+    else
+      {
+      this->setRowCount(1);
+      }
+    }
+  else
+    {
+    this->setRowCount(0);
+    }
 
   if (d->DataSet.GetPointer() == 0)
     {
@@ -318,7 +374,11 @@ void ctkVTKDataSetModel
 ::insertArray(vtkAbstractArray* array, int location, int row)
 {
   Q_D(ctkVTKDataSetModel);
-  Q_ASSERT(vtkAbstractArray::SafeDownCast(array));
+  if (vtkAbstractArray::SafeDownCast(array)==0)
+    {
+    // it is normal, it happens when arrays are pre-allocated for a data set
+    return;
+    }
 
   QList<QStandardItem*> items;
   for (int i= 0; i < this->columnCount(); ++i)
@@ -401,6 +461,36 @@ void ctkVTKDataSetModel::updateArrayFromItem(vtkAbstractArray* array, QStandardI
 void ctkVTKDataSetModel::onDataSetModified(vtkObject* dataSet)
 {
   Q_UNUSED(dataSet);
+  Q_D(ctkVTKDataSetModel);
+
+  // If a point or cell data array is added or removed then DataSet's Modified is not invoked.
+  // Therefore, we need to add observers to the point and cell data objects to make sure
+  // the list of arrays is kept up-to-date.
+
+  vtkPointData* dataSetPointData = d->DataSet ? d->DataSet->GetPointData() : 0;
+  this->qvtkReconnect(d->DataSetPointData, dataSetPointData, vtkCommand::ModifiedEvent,
+                      this, SLOT(onDataSetPointDataModified(vtkObject*)) );
+  d->DataSetPointData = dataSetPointData;
+
+  vtkCellData* dataSetCellData = d->DataSet ? d->DataSet->GetCellData() : 0;
+  this->qvtkReconnect(d->DataSetCellData, dataSetCellData, vtkCommand::ModifiedEvent,
+                      this, SLOT(onDataSetCellDataModified(vtkObject*)) );
+  d->DataSetCellData = dataSetCellData;
+
+  this->updateDataSet();
+}
+
+//------------------------------------------------------------------------------
+void ctkVTKDataSetModel::onDataSetPointDataModified(vtkObject* dataSetPointData)
+{
+  Q_UNUSED(dataSetPointData);
+  this->updateDataSet();
+}
+
+//------------------------------------------------------------------------------
+void ctkVTKDataSetModel::onDataSetCellDataModified(vtkObject* dataSetCellData)
+{
+  Q_UNUSED(dataSetCellData);
   this->updateDataSet();
 }
 
@@ -426,3 +516,44 @@ void ctkVTKDataSetModel::onItemChanged(QStandardItem * item)
   Q_ASSERT(array);
   this->updateArrayFromItem(array, item);
 }
+
+//------------------------------------------------------------------------------
+void ctkVTKDataSetModel::insertNullItem()
+{
+  QStandardItem* nullItem = new QStandardItem();
+  nullItem->setData(QVariant::fromValue(qlonglong(0)), ctkVTK::PointerRole);
+  nullItem->setData(this->NullItemLocation, ctkVTK::LocationRole);
+  nullItem->setText(QString());
+  this->insertRow(0,nullItem);
+}
+
+//------------------------------------------------------------------------------
+void ctkVTKDataSetModel::removeNullItem()
+{
+  if (this->rowCount() <= 0)
+    {
+    return;
+    }
+  // NULL item must be the first one
+  QStandardItem* nullItem = this->item(0);
+  Q_ASSERT(nullItem);
+  if (nullItem == 0)
+    {
+    return;
+    }
+  // NULL item has a special location value
+  int nullItemLocation = nullItem->data(ctkVTK::LocationRole).toInt();
+  Q_ASSERT(nullItemLocation == this->NullItemLocation);
+  if (nullItemLocation != this->NullItemLocation)
+    {
+    return;
+    }
+  // the first item is indeed the NULL item, so we remove it now
+  this->removeRow(0);
+}
+
+//------------------------------------------------------------------------------
+int ctkVTKDataSetModel::nullItemLocation()const
+{
+  return this->NullItemLocation;
+}

Libs/Visualization/VTK/Widgets/ctkVTKDataSetModel.h

@@ -53,11 +53,18 @@ class CTK_VISUALIZATION_VTK_WIDGETS_EXPORT ctkVTKDataSetModel
   QVTK_OBJECT
   Q_FLAGS(AttributeType AttributeTypes)
 
-  /// This property holds the type of attribute that should be listed in the model.s
+  /// This property holds the type of attribute that should be listed in the model.
   /// By default all attributes are considered.
   /// \sa ctkVTKDataSetModel::AllAttribute
   Q_PROPERTY(AttributeTypes attributeTypes READ attributeTypes WRITE setAttributeTypes)
 
+  /// This property allows adding a 'Null' item to the model, which is useful when
+  /// it is necessary to offer the user an option to not select any of the items
+  /// (for example, in a combo box there is always a selected item and it may be
+  /// necessary to allow the user to not select any of the attributes).
+  /// By default no 'Null' item is included.
+  Q_PROPERTY(bool includeNullItem READ includeNullItem WRITE setIncludeNullItem)
+
 public:
   typedef ctkVTKDataSetModel Self;
   typedef QStandardItemModel Superclass;
@@ -85,6 +92,10 @@ public:
   AttributeTypes attributeTypes()const;
   void setAttributeTypes(const AttributeTypes& attributeTypes);
 
+  bool includeNullItem()const;
+  void setIncludeNullItem(bool includeNullItem);
+  int nullItemLocation()const;
+
   /// Return the vtkAbstractArray associated to the index.
   /// 0 if the index doesn't contain a vtkAbstractArray
   inline vtkAbstractArray* arrayFromIndex(const QModelIndex& arrayIndex)const;
@@ -107,6 +118,8 @@ public:
 
 protected Q_SLOTS:
   void onDataSetModified(vtkObject* dataSet);
+  void onDataSetPointDataModified(vtkObject* dataSetPointData);
+  void onDataSetCellDataModified(vtkObject* dataSetCellData);
   void onArrayModified(vtkObject* array);
   void onItemChanged(QStandardItem * item);
 
@@ -120,9 +133,12 @@ protected:
   virtual void updateArrayFromItem(vtkAbstractArray* array, QStandardItem* item);
   virtual void updateDataSet();
   virtual void populateDataSet();
+  virtual void insertNullItem();
+  virtual void removeNullItem();
 
 protected:
   QScopedPointer<ctkVTKDataSetModelPrivate> d_ptr;
+  int NullItemLocation;
 
 private:
   Q_DECLARE_PRIVATE(ctkVTKDataSetModel);