View Issue Details

IDProjectCategoryView StatusLast Update
0002428Slicer4Module ModelMakerpublic2012-09-20 10:41
ReporterlassoanAssigned Tofinetjul 
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version 
Target VersionSlicer 4.2.0Fixed in VersionSlicer 4.2.0 
Summary0002428: Model maker creates less models than the number of labels in the input image
Description

It seems that the first time the Model maker creates models from a labelmap there are no models generated for the last 3 labels.

Win7, 64-bit, nightly release Slicer 4.1.0-2012-08-18

How to reproduce:
Load MRHead sample
Go to the editor module, draw 6 structures with different colors
*Go to the model maker, create a new hierarchy, generate the models
=> ERROR: Only 3 models are visible (the 1st, 2nd, and 3rd), the 4th, 5th, 6th models are generated with the correct colors, but with incorrect geometry (the same as the 1st, 2nd, and 3rd)

*Go to model maker again, click Apply =>All the models are correctly appear

TagsNo tags attached.

Relationships

related to 0002108 closednicole Crash when deleting a model after deleting its parent hierarchy node 
related to 0002526 closedpieper entering the models module results in crash 
parent of 0002447 closednicole model maker mixes things up. 
related to 0002330 closednicole When there is a model in /tmp Model Maker produces that model as output of any execution 
related to 0002430 closedfinetjul Models jump around in the MRML tree view when visibility is changed 
related to 0002096 closedfinetjul transform node disappears when volume is dropped inside 
related to 0002482 closednicole Ghost models in the 3D view cannot be deleted 
related to 0002538 closedfinetjul slice pipeline causing a crash when restoring scene views with models + slices visible in 3d 
related to 0002539 closedfinetjul Update node reference before AddNode to scene 
related to 0002569 closedfinetjul GetReferencedSubScene does not work 

Activities

nicole

nicole

2012-08-22 14:01

administrator   ~0005742

If delete the hierarchy in the models module, slicer crashes.
The files are written to disk with no errors, investigate how the command line logic is reading them in (suspect that the poly data structures aren't used cleanly).

nicole

nicole

2012-08-22 14:21

administrator   ~0005744

When I didn't delete the temporary files and loaded the .mrml scene file from a fresh start of Slicer, all the models showed up in the 3d view, but in the models module, the first three didn't show up in the Models hierarchy. Digging into the nodes via python, using the mrml id from the Data module, the Models hierarchy had 6 children:
modelshnode = slicer.mrmlScene.GetNodeByID("vtkMRMLModelHierarchyNode1")
modelshnode.GetNumberOfChildrenNodes()
but the first three are the slice model nodes:

modelshnode.GetNthChildNode(0).GetAssociatedNode().GetName()
'Red Volume Slice'
modelshnode.GetNthChildNode(1).GetAssociatedNode().GetName()
'Yellow Volume Slice'
modelshnode.GetNthChildNode(2).GetAssociatedNode().GetName()
'Green Volume Slice'
modelshnode.GetNthChildNode(3).GetAssociatedNode().GetName()
'Model_4_connective_tissue'
modelshnode.GetNthChildNode(4).GetAssociatedNode().GetName()
'Model_5_blood'
modelshnode.GetNthChildNode(5).GetAssociatedNode().GetName()
'Model_6_organ'

So it's not just the CLI reload, it's something about the generated scene from model maker clashing on load.

nicole

nicole

2012-08-23 13:56

administrator   ~0005768

svn 20847 fixed the incorrect hierarchy issue

nicole

nicole

2012-08-24 13:03

administrator   ~0005778

Last edited: 2012-08-24 13:20

The poly data pointers on the "missing" models' display nodes are incorrect, the display node and the model node should point to the same poly data, but the first missing model has different pointers:

m4 = slicer.mrmlScene.GetNodeByID("vtkMRMLModelNode4")
m4d = m4.GetDisplayNode()
m4.GetPolyData()
(vtkPolyData)0x4969c00
m4d.GetPolyData()
(vtkPolyData)0x4969c58

The following makes the first model show up in 3d:
m4d.SetPolyData(m4.GetPolyData())

So, my guess is that when the model maker scene is read in, the model display node's reference to the poly data isn't getting re-set when the node id references are being updated to avoid clashes with the model nodes already in the scene.

Further digging, the yellow slice node has it's poly data messed up, on a fresh start they poly data pointers are the same, but after model maker load, they're different:
m2 = slicer.mrmlScene.GetNodeByID("vtkMRMLModelNode2")
m2.GetPolyData()

(vtkPolyData)0x4969f70
m2.GetDisplayNode().GetPolyData()
(vtkPolyData)0x4969fc8
m2.GetName()
'Yellow Volume Slice'

nicole

nicole

2012-08-24 13:24

administrator   ~0005780

Last edited: 2012-08-27 07:10

vtkMRMLSceneImportIDConflictTest is passing, may need to be updated to exercise this behaviour, as the model maker triggers the update by passing in a collection of nodes. Actually, the test just doesn't include poly data, need a new one.

nicole

nicole

2012-08-27 11:26

administrator   ~0005811

svn 20872 adds a test to show failing behaviour:
vtkMRMLSceneImportIDModelHierarchyConflictTest

nicole

nicole

2012-08-27 13:16

administrator   ~0005817

Last edited: 2012-08-27 13:20

In the vtkMRMLSceneImportIDModelHierarchyConflictTest, when the scene is imported from the xml string, the reading of the xml attributes on the model node sets the display node id and that triggers events that lead to the original display node in the scene that is returned on GetNodeByID("vtkMRMLModelDisplayNode1") to have it's poly data set to null.
Shouldn't it be importing into a separate scene though?
But there's a line in LoadIntoScene:
parser->SetMRMLScene(this);
So the nodes are getting loaded into the current scene and then the id conflicts are resolved, but because during the loading process there are conflicting IDs getting set and observed, the original nodes are over written.

Back trace:
838 int res = this->LoadIntoScene(scene);
(gdb) n
Hardware watchpoint 2: modelDisplayNode->PolyData

Old value = (vtkPolyData ) 0x7a8a10
New value = (vtkPolyData
) 0x0
0x00007ffff7b4cbea in vtkMRMLModelDisplayNode::SetPolyData (this=0x7a96a0, _arg=0x0) at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLModelDisplayNode.cxx:20
20 vtkCxxSetObjectMacro(vtkMRMLModelDisplayNode, PolyData, vtkPolyData)
(gdb) bt
#0 0x00007ffff7b4cbea in vtkMRMLModelDisplayNode::SetPolyData (this=0x7a96a0, _arg=0x0) at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLModelDisplayNode.cxx:20
0000001 0x00007ffff7b5319b in vtkMRMLModelNode::OnDisplayNodeAdded (this=0x7ae250, dnode=0x7a96a0) at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLModelNode.cxx:756
0000002 0x00007ffff7b082ea in vtkMRMLDisplayableNode::SetAndObserveNthDisplayNode (this=0x7ae250, n=0, dnode=0x7a96a0)
at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLDisplayableNode.cxx:338
0000003 0x00007ffff7b07cb7 in vtkMRMLDisplayableNode::UpdateNthDisplayNode (this=0x7ae250, n=0)
at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLDisplayableNode.cxx:264
0000004 0x00007ffff7b08147 in vtkMRMLDisplayableNode::SetAndObserveNthDisplayNodeID (this=0x7ae250, n=0, displayNodeID=0x7ae5e8 "vtkMRMLModelDisplayNode1")
at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLDisplayableNode.cxx:323
0000005 0x000000000045cef9 in vtkMRMLDisplayableNode::AddAndObserveDisplayNodeID (this=0x7ae250, displayNodeID=0x7ae5e8 "vtkMRMLModelDisplayNode1")
at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLDisplayableNode.h:281
0000006 0x00007ffff7b070a3 in vtkMRMLDisplayableNode::ReadXMLAttributes (this=0x7ae250, atts=0x7ac040)
at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLDisplayableNode.cxx:94
0000007 0x00007ffff7bef485 in vtkMRMLParser::StartElement (this=0x7abf30, tagName=0x7ad5b4 "Model", atts=0x7ac010)
---Type <return> to continue, or q <return> to quit---
at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLParser.cxx:122
0000008 0x00007fffed4d040d in vtkXMLParserStartElement (parser=0x7abf30, name=0x7ad5b4 "Model", atts=0x7ac010)
at /projects/birn/nicole/Slicer4/S4-SuperBuild/VTK/IO/vtkXMLParser.cxx:523
[...]
0000018 0x00007fffed4cf64c in vtkXMLParser::ParseXML (this=0x7abf30) at /projects/birn/nicole/Slicer4/S4-SuperBuild/VTK/IO/vtkXMLParser.cxx:375
0000019 0x00007fffed4ced31 in vtkXMLParser::Parse (this=0x7abf30) at /projects/birn/nicole/Slicer4/S4-SuperBuild/VTK/IO/vtkXMLParser.cxx:252
0000020 0x00007fffed4ce834 in vtkXMLParser::Parse (this=0x7abf30, inputString=
0x7abc68 "<MRML version=\"18916\" userTags=\"\"> <Model id=\"vtkMRMLModelNode1\" name=\"New Model1\" displayNodeRef=\"vtkMRMLModelDisplayNode1\" ></Model> <ModelDisplay id=\"vtkMRMLModelDisplayNode1\" name=\"New Display "...) at /projects/birn/nicole/Slicer4/S4-SuperBuild/VTK/IO/vtkXMLParser.cxx:190
0000021 0x00007ffff7c0cb1b in vtkMRMLScene::LoadIntoScene (this=0x6fe9f0, nodeCollection=0x7abea0) at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLScene.cxx:1019
0000022 0x00007ffff7c0ba04 in vtkMRMLScene::Import (this=0x6fe9f0) at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLScene.cxx:838
0000023 0x000000000053477c in vtkMRMLSceneImportIDModelHierarchyConflictTest ()
at /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/Testing/vtkMRMLSceneImportIDModelHierarchyConflictTest.cxx:106

finetjul

finetjul

2012-08-28 05:34

administrator   ~0005822

I'm not sure why the MRML Scene is set onto the parser, but more importantly, why is it set to the nodes created by the parser.
It would be interesting to try not setting the scene and see where it fails.

nicole

nicole

2012-08-29 08:01

administrator   ~0005853

Previous changes to the observation of display nodes:
http://viewvc.slicer.org/viewvc.cgi/Slicer4/trunk/Libs/MRML/Core/vtkMRMLDisplayableNode.cxx?r1=19976&r2=19977&;

alexy

alexy

2012-09-07 11:04

developer   ~0006009

Last edited: 2012-09-07 11:04

I committed a workaround (revision: 20954) that temporarily sets scene to NULL, kindof as Julien suggested. That fixed the vtkMRMLSceneImportIDModelHierarchyConflictTest test. However the original issue with ModelMaker still remains. I debugged that scenario and found that:

  • at some point while importing the ModelMaker scene the new models (created by CLI) are referencing incorrectly old display nodes that belong to slices.
  • at the end of MM scene import all references are OK and the scene is valid (you can save it and load back and it will display all models). Also new actors for imported nodes are created and added to the renderer in the correct state. I cannot see anything wrong with the actors at the end of scene import.

My suspicion is that the intermediate state when two models (slice and MM model) point to the same display node somehow creates problems but I don't know how they manifest.

nicole

nicole

2012-09-07 12:31

administrator   ~0006012

Last edited: 2012-09-07 12:42

Loading the model maker results is causing a crash for me now (64 bit linux, debug build). But when I tried again not going through the editor but just loading a label map, it didn't crash. Still not seeing models 1-3 though.

alexy

alexy

2012-09-08 06:04

developer   ~0006013

Nicole, could you provide stack trace for the crash? I can revert my change.
In the current design there are two problems that need to be addressed (by Julien?)

  1. We should not be calling SetAndObserve display nodes before scene is completely loaded and all references adjusted
  2. GetDisplayNode is not using cached nodes anymore, it gets the node from the scene by ID, so if it is called before the references are adjusted or SetAndObserve is called by ApplicationLogic after CLI is done, it may return wrong display node.
alexy

alexy

2012-09-08 10:01

developer   ~0006014

In the current design there are two problems that need to be addressed

  1. We should not be calling SetAndObserve display nodes before scene
    is completely loaded and all references adjusted
  2. GetDisplayNode is not using cached nodes anymore, it gets the node
    from the scene by ID, so if it is called before the references are
    adjusted or SetAndObserve is called by ApplicationLogic after CLI is
    done, it may return wrong display node.
pieper

pieper

2012-09-08 12:43

administrator   ~0006020

I (Steve) will take a look too - that way Alex, Nicole, and I can discuss it with Julien when he returns if it hasn't yet been resolved.

pieper

pieper

2012-09-11 04:56

administrator   ~0006030

I added a new self test that illustrates this issue - you can run it from the Modules->Testing->TestCases->Scene Import menu item and then clicking the Reload and Test button. We make 4 models.

As of revision 20968, If you look in the 3D view and change visibility of Model_4_4 (blue) from the models module you can see that Model_1_1 (green) is coincident with the blue model. This appears to be because multiple model display nodes have the same polydata and model nodes do not have the same polydata as their display nodes.

http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=20965

pieper

pieper

2012-09-11 05:22

administrator   ~0006031

Note that this test configures the CLI not to delete the intermediate data files. The scene location is printed by the test (In the case of my mac build, the scene below). This scene can be loaded and all four models appear correctly. This indicates something is broken in the scene loading of the CLI.

/var/folders/5g/8696sbjd1blch9pwpqwzq8c40000gn/T/CJIIH_AxHfeJFEGaAFaA.mrml

pieper

pieper

2012-09-11 08:23

administrator   ~0006036

Here's a sample scene from the model maker (create by the test script):

http://slicer.kitware.com/midas3/folder/775

The scene appears to load correctly in that all the disks appear.

However if you do the following:

  • download the MRHead sample data
  • import this scene
  • try to turn on visibility of the green or yellow slice -> ERROR: nothing appears

More oddity:

  • red slice visibility work
  • after making red slice visible, then scrolling the green slice causes slice model to appear.
pieper

pieper

2012-09-11 13:18

administrator   ~0006037

Alex and I spent some time debugging this and found that the GetDisplayNode call was triggered in the SliceModelDisplayableManager during scene import. Since the node ids of the displayble node were not yet updated, the polydata of the display nodes was being set to incorrect values.

http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=20972
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=20971
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=20970

finetjul

finetjul

2012-09-13 02:41

administrator   ~0006047

Last edited: 2012-09-26 06:13

The scene is no longer being set to nodes in vtkMRMLParser.
That way, there is no need to remove it from vtkMRMLDispayableNode::ReadXMLAttributes()

http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=20984

Note that now MRML nodes must reimplement vtkMRMLNode::SetSceneReferences()

lassoan

lassoan

2012-09-16 08:11

developer   ~0006100

Tested on Slicer-4.1.0-2012-09-14-win-amd64.exe and it works well. Thanks to all for fixing this!

Issue History

Date Modified Username Field Change
2012-08-22 11:45 lassoan New Issue
2012-08-22 11:45 lassoan Status new => assigned
2012-08-22 11:45 lassoan Assigned To => nicole
2012-08-22 11:46 lassoan Relationship added related to 0002108
2012-08-22 13:41 nicole Relationship added related to 0002330
2012-08-22 14:01 nicole Note Added: 0005742
2012-08-22 14:01 nicole Target Version => Slicer 4.2.0 - Feature freeze Sept 1st 2012
2012-08-22 14:21 nicole Note Added: 0005744
2012-08-23 13:56 nicole Note Added: 0005768
2012-08-24 13:03 nicole Note Added: 0005778
2012-08-24 13:20 nicole Note Edited: 0005778
2012-08-24 13:24 nicole Note Added: 0005780
2012-08-27 07:10 nicole Note Edited: 0005780
2012-08-27 11:26 nicole Note Added: 0005811
2012-08-27 13:16 nicole Note Added: 0005817
2012-08-27 13:20 nicole Note Edited: 0005817
2012-08-28 05:34 finetjul Note Added: 0005822
2012-08-29 08:01 nicole Note Added: 0005853
2012-08-29 13:41 jcfr Relationship added related to 0002447
2012-08-29 13:42 jcfr Relationship replaced parent of 0002447
2012-09-02 10:14 finetjul Relationship added related to 0002430
2012-09-02 10:15 finetjul Relationship added related to 0002096
2012-09-04 13:23 alexy Assigned To nicole => alexy
2012-09-07 11:04 alexy Note Added: 0006009
2012-09-07 11:04 alexy Note Edited: 0006009
2012-09-07 12:31 nicole Note Added: 0006012
2012-09-07 12:42 nicole Note Edited: 0006012
2012-09-08 06:04 alexy Note Added: 0006013
2012-09-08 10:01 alexy Note Added: 0006014
2012-09-08 10:01 alexy Assigned To alexy => finetjul
2012-09-08 12:43 pieper Note Added: 0006020
2012-09-11 04:56 pieper Note Added: 0006030
2012-09-11 05:22 pieper Note Added: 0006031
2012-09-11 08:23 pieper Note Added: 0006036
2012-09-11 13:18 pieper Note Added: 0006037
2012-09-11 13:18 pieper Status assigned => resolved
2012-09-11 13:18 pieper Fixed in Version => Slicer 4.2.0 - Feature freeze Sept 1st 2012
2012-09-11 13:18 pieper Resolution open => fixed
2012-09-13 02:41 finetjul Note Added: 0006047
2012-09-16 08:11 lassoan Note Added: 0006100
2012-09-16 08:11 lassoan Status resolved => closed
2012-09-18 12:57 nicole Relationship added related to 0002482
2012-09-20 10:41 pieper Relationship added related to 0002526
2012-09-20 11:18 finetjul Relationship added related to 0002538
2012-09-20 12:37 finetjul Relationship added related to 0002539
2012-09-26 06:12 finetjul Relationship added related to 0002569
2012-09-26 06:13 finetjul Note Edited: 0006047