View Issue Details

IDProjectCategoryView StatusLast Update
0002849Slicer4Module Annotationspublic2018-03-22 01:33
Reporterjohan.andruejolAssigned Tojcfr 
PrioritynormalSeveritymajorReproducibilityalways
Status assignedResolutionfixed 
Product VersionSlicer 4.2.2-1 
Target VersionSlicer 4.11.0Fixed in Version 
Summary0002849: Hierachy not respected when loading the same scene multiples times
Description

When loading twice the same scene the nodes hierarchy is not of the loaded scene is not preserved the second time.
Example:
Save a scene with just a fiducial.
Import this scene.
Note that the new fiducial is added under the first "Fiducial list" with the original fiducial instead of the newly imported list.

Additional Information

It looks like the ParentNodeID in vtkMRMLHierarchyNode is not added as a referenced node ID in the scene. Thus the code in UpdateReferenceID regarding the ParentNodeID is never used.

I submitted a patch along.

TagsNo tags attached.

Relationships

related to 0003462 closedjcfr ModelMaker functionality breaks display of Slice views 
child of 0004081 assignedjcfr Ensure reference are updated before NodeAddedEvent is invoked 

Activities

2013-01-03 10:52

 

FixHierarchyWhenImportingSceneSeveralTimes.patch (2,911 bytes)
From 1d241224848fd41e21e12b663ee1de4152cfbdc4 Mon Sep 17 00:00:00 2001
From: Johan Andruejol <johan.andruejol@kitware.com>
Date: Thu, 3 Jan 2013 15:48:02 -0500
Subject: [PATCH] When loading twice the same scene the nodes hierarchy is not
 of the loaded scene is not preserved the second time.

Referencing the ParentNodeID in vtkMRMLHierarchyNode in the scene so
the code in UpdateReferenceID regarding the ParentNodeID can be used.
---
 Libs/MRML/Core/vtkMRMLHierarchyNode.cxx            |    7 ++++++-
 .../Logic/vtkSlicerAnnotationModuleLogic.cxx       |   13 ++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/Libs/MRML/Core/vtkMRMLHierarchyNode.cxx b/Libs/MRML/Core/vtkMRMLHierarchyNode.cxx
index 73c797f..987ee4d 100644
--- a/Libs/MRML/Core/vtkMRMLHierarchyNode.cxx
+++ b/Libs/MRML/Core/vtkMRMLHierarchyNode.cxx
@@ -213,6 +213,7 @@ vtkMRMLHierarchyNode* vtkMRMLHierarchyNode::GetParentNode()
 void vtkMRMLHierarchyNode::SetSceneReferences()
 {
   this->Superclass::SetSceneReferences();
+  this->Scene->AddReferencedNodeID(this->ParentNodeIDReference, this);
   this->Scene->AddReferencedNodeID(this->AssociatedNodeIDReference, this);
 }
 
@@ -226,7 +227,7 @@ void vtkMRMLHierarchyNode::UpdateScene(vtkMRMLScene *scene)
 void vtkMRMLHierarchyNode::UpdateReferences()
 {
   Superclass::UpdateReferences();
-  
+
   if (this->ParentNodeIDReference != NULL && this->Scene->GetNodeByID(this->ParentNodeIDReference) == NULL)
     {
     this->SetParentNodeID(NULL);
@@ -260,6 +261,10 @@ void vtkMRMLHierarchyNode::SetParentNodeID(const char* ref)
   this->SetSortingValue(++MaximumSortingValue);
 
   this->HierarchyIsModified(this->GetScene());
+  if (this->GetScene())
+    {
+    this->GetScene()->AddReferencedNodeID(ref, this);
+    }
 
   this->EndModify(disableModify);
 
diff --git a/Modules/Loadable/Annotations/Logic/vtkSlicerAnnotationModuleLogic.cxx b/Modules/Loadable/Annotations/Logic/vtkSlicerAnnotationModuleLogic.cxx
index 5c7e330..2d52b6a 100644
--- a/Modules/Loadable/Annotations/Logic/vtkSlicerAnnotationModuleLogic.cxx
+++ b/Modules/Loadable/Annotations/Logic/vtkSlicerAnnotationModuleLogic.cxx
@@ -341,13 +341,16 @@ void vtkSlicerAnnotationModuleLogic::OnMRMLSceneNodeAdded(vtkMRMLNode* node)
     return;
     }
 
-  bool retval = this->AddHierarchyNodeForAnnotation(annotationNode);
-  if (!retval)
+  // Hierarchy nodes already setup when importing
+  if (!this->GetMRMLScene()->IsImporting())
     {
-    vtkErrorMacro("OnMRMLSceneNodeAddedEvent: No hierarchyNode added.")
-    return;
+    bool retval = this->AddHierarchyNodeForAnnotation(annotationNode);
+    if (!retval)
+      {
+      vtkErrorMacro("OnMRMLSceneNodeAddedEvent: No hierarchyNode added.")
+      return;
+      }
     }
-  
   // we pass the hierarchy node along - it includes the pointer to the actual annotationNode
   this->AddNodeCompleted(annotationNode);
 }
-- 
1.7.9.msysgit.0

finetjul

finetjul

2013-01-03 14:47

administrator   ~0007608

Last edited: 2013-01-04 03:44

Partially fixed in r21561:
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21561

The import of annotations is still broken, as part of r21561, a failing test has been created (vtkSlicerAnnotationModuleLogicImportSceneTest).

finetjul

finetjul

2013-01-04 03:47

administrator   ~0007609

Maybe Johan's solution is correct, vtkSlicerAnnotationModuleLogic::OnMRMLSceneNodeAdded() shouldn't do anything when the scene is in import/restore state.

nicole

nicole

2013-01-04 07:53

administrator   ~0007610

svn 21562[1] checks for scene restore/import before adding hierarchies.
That fixes importing a scene with fiducials, but rulers are still getting re-assigned to the newly imported ruler hierarchy.

[1]http://viewvc.slicer.org/viewvc.cgi/Slicer4/trunk/Modules/Loadable/Annotations/Logic/vtkSlicerAnnotationModuleLogic.cxx?r1=21544&r2=21562

nicole

nicole

2013-01-04 10:19

administrator   ~0007611

I was looking at the parent/associated node/child node ids and everything looked fine after importing a ruler scene, then I tried:

Open Slicer
Add a ruler
Save scene
Import scene

Look at the annotations module, the original ruler moved under the imported ruler list.
Double click on the second item in the second ruler list and rename it.
It jumps back to the first list.

Reimporting the file, only the first one jumps into the new ruler list, and it jumps back when I rename it. I suspect something in the scene model rather than the mrml code, so I'm reassigning this issue back to Julien.

jcfr

jcfr

2015-11-11 11:08

administrator   ~0013589

Fixed in r24723
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=24723

jcfr

jcfr

2015-11-12 08:58

administrator   ~0013599

Reverted in r24734
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=24734

Related Changesets

Slicer: 2145-support-for-installing-extension-from-file 959e9b16

2013-01-03 19:46:32

finetjul

Details Diff
BUG: Fix import of hierarchy node with existing ID

Note that it does not fix the import of annotation hierarchies.
For some reasons (see vtkSlicerAnnotationModuleLogic::OnSceneNodeAdded),
the hierarchies are automatically loaded into the wrong hierarchy node
parent.
Issue 0002849

git-svn-id: http://svn.slicer.org/Slicer4/trunk@21561 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Libs/MRML/Core/Testing/vtkMRMLSceneImportIDModelHierarchyConflictTest.cxx Diff File
mod - Libs/MRML/Core/vtkMRMLHierarchyNode.cxx Diff File
mod - Modules/Loadable/Annotations/Testing/Cxx/CMakeLists.txt Diff File
add - Modules/Loadable/Annotations/Testing/Cxx/vtkSlicerAnnotationModuleLogicImportSceneTest.cxx Diff File

Slicer: 2145-support-for-installing-extension-from-file 98ac3bf5

2013-01-04 12:51:54

naucoin

Details Diff
BUG: don't add hierarchy nodes if the scene is importing/restoring. Issue 0002849

git-svn-id: http://svn.slicer.org/Slicer4/trunk@21562 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Modules/Loadable/Annotations/Logic/vtkSlicerAnnotationModuleLogic.cxx Diff File

Import 2017-06-07 23:51:09: master dd5c50f1

2015-11-11 15:04:19

jcfr

Details Diff
BUG: Workaround MRML Scene import limitation. Fixes 0003462, 0002849 and 0004080

As documented in 0004081 [1], references to other nodes are not valid
when the NodeAddedEvent is invoked during the import of a scene in
an existing scene.

This prevented the qMRMLSceneModel from working well when it is
observing the scene and updating itself as new node are added
during import.

This commit workarounds the root issue by forcing all SceneModel
to have "lazyUpdate" enabled.

When lazyUpdate is enabled, the model ignores added node events when the
scene is importing/restoring, but synchronize with the scene once its
imported/restored.

This affected a broad set of modules and was captured in the following
issues:

* 3462: ModelMaker functionality breaks display of Slice views
* 2849: Hierachy not respected when loading the same scene multiples times
* 4080: Transform module is not updated properly after importing a scene

Reviewed-by: Andras Lasso <lasso@queensu.ca>
Reviewed-by: Steve Pieper <pieper@isomics.com>
Tested-by: Andras Lasso <lasso@queensu.ca>
Tested-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

git-svn-id: http://svn.slicer.org/Slicer4/trunk@24723 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Libs/MRML/Widgets/qMRMLSceneModel.cxx Diff File

Import 2017-06-07 23:51:09: master 7c91689f

2015-11-12 12:50:11

jcfr

Details Diff
BUG: Revert workaround for MRML Scene import limitation. See 0004081

This commit reverts r24723 (BUG: Workaround MRML Scene import
limitation. Fixes 0003462, 0002849 and 0004080).

It turns out that this commit had unintended side effects. The following
tests were failing:

py_LandmarkRegistration
py_SlicerMRBMultipleSaveRestoreTest
py_SlicerMRBMultipleSaveRestoreLoopTest
py_SlicerMRBTest
py_NeurosurgicalPlanningTutorialMarkupsSelfTest
qMRMLSceneModelTest

After concerting with the core team, since the correct fix involves
changes in the core of MRML, we decided to revert this commit and mark
the tickets 0003462, 0002849, 0004080 as known issues in the release note.

git-svn-id: http://svn.slicer.org/Slicer4/trunk@24734 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Libs/MRML/Widgets/qMRMLSceneModel.cxx Diff File

Issue History

Date Modified Username Field Change
2013-01-03 10:52 johan.andruejol New Issue
2013-01-03 10:52 johan.andruejol Status new => assigned
2013-01-03 10:52 johan.andruejol Assigned To => pieper
2013-01-03 10:52 johan.andruejol File Added: FixHierarchyWhenImportingSceneSeveralTimes.patch
2013-01-03 14:43 finetjul Assigned To pieper => finetjul
2013-01-03 14:47 finetjul Note Added: 0007608
2013-01-03 14:47 finetjul Assigned To finetjul => nicole
2013-01-03 14:47 finetjul Category Base Code => Module Annotations
2013-01-04 03:44 finetjul Note Edited: 0007608
2013-01-04 03:47 finetjul Note Added: 0007609
2013-01-04 07:53 nicole Note Added: 0007610
2013-01-04 10:19 nicole Assigned To nicole => finetjul
2013-01-04 10:19 nicole Note Added: 0007611
2015-11-03 20:14 jcfr Relationship added related to 0003462
2015-11-03 20:14 jcfr Assigned To finetjul => jcfr
2015-11-03 20:14 jcfr Target Version => Slicer 4.5.0-1
2015-11-11 08:18 jcfr Relationship added child of 0004081
2015-11-11 11:08 jcfr Note Added: 0013589
2015-11-11 11:08 jcfr Status assigned => resolved
2015-11-11 11:08 jcfr Fixed in Version => Slicer 4.5.0-1
2015-11-11 11:08 jcfr Resolution open => fixed
2015-11-12 08:17 jcfr Status resolved => assigned
2015-11-12 08:17 jcfr Fixed in Version Slicer 4.5.0-1 =>
2015-11-12 08:17 jcfr Target Version Slicer 4.5.0-1 => Slicer 4.5.1
2015-11-12 08:58 jcfr Note Added: 0013599
2016-10-12 02:56 jcfr Target Version Slicer 4.5.1 => Slicer 4.7.0
2017-06-07 23:27 Changeset attached => Slicer 2145-support-for-installing-extension-from-file 98ac3bf5
2017-06-07 23:27 finetjul Changeset attached => Slicer 2145-support-for-installing-extension-from-file 959e9b16
2017-06-10 08:51 jcfr Changeset attached => Slicer master 7c91689f
2017-06-10 08:51 jcfr Changeset attached => Slicer master dd5c50f1
2017-09-27 11:13 lassoan Target Version Slicer 4.7.0 => Slicer 4.9.0
2018-03-22 01:33 jcfr Target Version Slicer 4.9.0 => Slicer 4.11.0