New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PERF: Improve performance when removing segmentation from SH tree #7253
Conversation
@@ -1119,6 +1119,9 @@ bool vtkSubjectHierarchyItem::RemoveChild(vtkSubjectHierarchyItem* item) | |||
// as they represent the item's data node's content), then remove virtual branch | |||
if (removedItem->IsVirtualBranchParent()) | |||
{ | |||
// If the item has many virtual children, then it may invoke a large number of events. | |||
// To avoid this, block modified events on the data node until all of the children are removed. | |||
MRMLNodeModifyBlocker blocker(removedItem->DataNode); | |||
removedItem->RemoveAllChildren(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition of tkSubjectHierarchyItem::RemoveChild(vtkSubjectHierarchyItem* item)
, should this be done in these functions as well where RemoveAllChildren
is also called?
vtkSubjectHierarchyItem::DeepCopy(vtkSubjectHierarchyItem* item, bool copyChildren/*=true*/)
vtkSubjectHierarchyItem::~vtkSubjectHierarchyItem()
vtkSubjectHierarchyItem::RemoveChild(vtkIdType itemID)
vtkMRMLSubjectHierarchyNode::Copy(vtkMRMLNode *anode)
Even better, should we consider updating RemoveAllChildren
to systematically block the event ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, the same performance issue could occur with any of those functions.
I've moved the MRMLNodeModifyBlocker into RemoveAllChildren.
0c47b18
to
8d41483
Compare
When removing a vtkMRMLSegmentationNode using the SubjectHierarchy tree, all of the virtual children would be removing before the segmentation. This caused a large number of updates in various segmentation widgets as all of the widgets were regenerated when each of the segments were removed from the segmentation node. This commit improves performance by blocking modification events on the segmentation node (or any other node with virtual children), until all of the children have been removed. Rendering is also blocked during the item removal to prevent unnecessary updates of the views.
8d41483
to
f699724
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thank you.
When removing a vtkMRMLSegmentationNode using the SubjectHierarchy tree, all of the virtual children would be removing before the segmentation. This caused a large number of updates in various segmentation widgets as all of the widgets were regenerated when each of the segments were removed from the segmentation node.
This commit improves performance by blocking modification events on the segmentation node (or any other node with virtual children), until all of the children have been removed. Rendering is also blocked during the item removal to prevent unnecessary updates of the views.