BUG: Fix nodes not getting removed in vtkMRMLSequenceNode::SetDataNod…

…eAtValue

When SetDataNodeAtValue is called, the new data node is deep-copied into the internal sequence scene, and replaces the old node in the index list if one exists. The old node however is not removed from the scene, causing the number of nodes to grow larger every time SetDataNodeAtValue was called for a value with an existing data node.

Fixed by removing the old node from the scene if one already exists for the specified value.
This commit also changes the node pointer in IndexEntryType to a vtkWeakPointer.

Fixes #7328

Libs/MRML/Core/vtkMRMLSequenceNode.cxx

@@ -412,8 +412,13 @@ vtkMRMLNode* vtkMRMLSequenceNode::SetDataNodeAtValue(vtkMRMLNode* node, const st
   this->GetSequenceScene();
   // Add a copy of the node to the sequence's scene
   vtkMRMLNode* newNode = this->DeepCopyNodeToScene(node, this->SequenceScene);
+  vtkMRMLNode* oldNode = nullptr;
   int seqItemIndex = this->GetItemNumberFromIndexValue(indexValue);
-  if (seqItemIndex<0)
+  if (seqItemIndex >= 0)
+    {
+    oldNode = this->IndexEntries[seqItemIndex].DataNode;
+    }
+  else
     {
     // The sequence item doesn't exist yet
     seqItemIndex = GetInsertPosition(indexValue);
@@ -430,6 +435,13 @@ vtkMRMLNode* vtkMRMLSequenceNode::SetDataNodeAtValue(vtkMRMLNode* node, const st
     {
     this->SetAttribute("DataNodeClassName", this->GetDataNodeClassName().c_str());
     }
+
+  if (oldNode)
+    {
+    // Remove the old node from the scene
+    this->SequenceScene->RemoveNode(oldNode);
+    }
+
   this->Modified();
   this->StorableModifiedTime.Modified();
   return newNode;
@@ -450,7 +462,11 @@ void vtkMRMLSequenceNode::RemoveDataNodeAtValue(const std::string& indexValue)
     return;
     }
   // TODO: remove associated nodes as well (such as storage node)?
-  this->SequenceScene->RemoveNode(this->IndexEntries[seqItemIndex].DataNode);
+  vtkMRMLNode* dataNode = this->IndexEntries[seqItemIndex].DataNode;
+  if (dataNode)
+    {
+    this->SequenceScene->RemoveNode(dataNode);
+    }
   this->IndexEntries.erase(this->IndexEntries.begin()+seqItemIndex);
   this->Modified();
   this->StorableModifiedTime.Modified();
@@ -782,6 +798,7 @@ int vtkMRMLSequenceNode::GetIndexTypeFromString(const std::string& indexTypeStri
   return -1;
 }
 
+//-----------------------------------------------------------
 vtkMRMLNode* vtkMRMLSequenceNode::DeepCopyNodeToScene(vtkMRMLNode* source, vtkMRMLScene* scene)
 {
   if (source == nullptr)

Libs/MRML/Core/vtkMRMLSequenceNode.h

@@ -186,7 +186,7 @@ class VTK_MRML_EXPORT vtkMRMLSequenceNode : public vtkMRMLStorableNode
   struct IndexEntryType
     {
     std::string IndexValue;
-    vtkMRMLNode* DataNode;
+    vtkWeakPointer<vtkMRMLNode> DataNode;
     std::string DataNodeID; // only used temporarily, during scene load
     };