 |
Reminder sent to: alexy, nicole, pieper Hi Alex, Is it something Alex could look at ? Thanks |
 |
This is actually caused by changes in SubjectHierachy module. When you load any model of fiber it now adds itself to the SubjectHierarchy (this is happening in the qSlicerSubjectHierarchyModule::onNodeAdded() method) However vtkMRMLSubjectHierarchyNode is not derived from vtkMRMLDisplayableHierarchyNode, and the logic for setting visibility does assume that. I can fix this assumption so that the code does not crash, but I don't know if it is a good idea to have all models be a part of SubjectHierarchy by default. Csaba should comment on this. |
 |
Please note that this happens with MODELs as well. It is not specific to fiber bundles or diffusion. I tested that at the end of writing the bug report, and did not change the original title. So this is a very basic Slicer crash. |
|
|
Reminder sent to: pinter Hi Csaba, Could you comment on Alex point ? Thanks |
 |
In vtkMRMLModelHierarchyLogic:290 SafeCast is done, but the result is not checked. It should be checked if it's not NULL. I made a point not to involve Subject hierarchy in the other hierarchies, this is why I came up with the concept of "nested hierarchy", Jc you may remember. It was necessary to create SH nodes for supported data nodes in order to make SH actually usable, as the users didn't know that they have to "add" nodes into the hierarchy, and it just seemed to be not working. I don't think a simple bug like this means that it was a bad decision. We just have to fix the bug. |
|
|
Fixed crash on change of visibility of ModelHierarchy nodes introduced by making them part of SubjectHierarchy. At revision: 23813 However, if all Model/Fiber nodes are part of SubjectHierarchy now should vtkMRMLSubjectHierarchyNode be derived from vtkMRMLDisplayableHierarchyNode not from vtkMRMLHierarchyNode as it is today? |
|
|
If you call GetAssociatedChildrenNodes or GetChildrenModelNodes instead of GetAllChildrenNodes, then it will work fine. It will mean using a vtkCollection instead of an std::vector |
|
|
It used to be a displayable hierarchy node back in the day, which caused issues in Data module etc (as far as I remember). I was also specifically asked not to mix SH with the other hierarchies, so I'd say let's keep it separate, so that users who want to use Data, Models, etc. can use those. I was asked to help create a DTI tracts plugin in SH so that the fiber bundles can be handled in a very similar way. That way the user could use this feature from both places. I don't know anything about fiber bundles so I'd appreciate if someone could get me started. Apparently there are bugs, sorry about that! Btw there are too many GetChildren'ish methods in the hierarchies and it's very easy to get lost. For example I don't really know the difference between GetChildren, GetAllChildren, and GetAssociatedChildrenNodes. Maybe they just need better names, but there's an ambiguity whether a function returns the hierarchy nodes (because only those can be actual children of hierarchy nodes), or the associated data nodes. |
|
|
@alexy: I just ese your commit message. Let's make it clear that ModelHierarchy is not just not part of SubjectHierarchy, but should be independent. |
|
|
I agree that SubjectHierarchy should be separate hierarchy. However each link in hierarchy contains two Hierarchy nodes one associated with the "data" node (1) and a hierarchy parent node it points to (2). Node (2) represents logical hierarchy like Models, Transforms, etc. Node (1) represents just a link to a parent and also defines common visualization parameters: visibility color, etc. That is why node (1) is better be a vtkMRMLDisplayableHierarchy. Subject Hierarchy module creates (1) as vtkMRMLSubjectHierarchyNode which I am not sure is a good idea, while (2) I agree should be a vtkMRMLSubjectHierarchyNode and does not need to be a vtkMRMLDisplayableHierarchy. |
|
|
Hi Alex, |
|
|
Does this last part about auto-creation of nested associations sound like a good idea? |
|
|
I see now that the model nodes in a model hierarchy are indeed not associated to a "leaf" hierarchy node. Alex, can you please explain how they work then? |
|
|
I confirmed that not just this but more issues are caused by SH in Models. I'm going to apply two fixes. First I'm going to disable auto SH creation by default and allow the user to turn it on when they switch to the SH module. The other fix will be more complicated, this is why I tried to get information about model hierarchy, without success so far. The model hierarchy works differently than the others, and this is why SH interferes with it. I'll need the help of the person who owns Models. Thanks! |
|
|
Hi Csaba, Thanks for your patience with this. I guess Alex and Nicole should be able to answer questions. @Alaex, @Nicole: Could the following page be updated with some high level info ? Thanks |
|
|
Thanks Jc, Actually the drawing on the link you provided is consistent with what I understood for the hierarchies, but not with reality. I'm reading the code in the hope of understanding, and found the flag vtkMRMLHierarchyNode::AllowMultipleChildren which is only used in some scene models for allowing or not allowing drops, but nothing more. The drawing should reflect that hierarchy nodes allow multiple children, do NOT require an explicit hierarchy node for each leaf, and what kind of connector is used for each connection. I'm researching further and try to fix the bug. It's indeed quite important to fix and I'm on it... |
|
|
Committed fix This does not fix the whole issue, only if the user does not use subject hierarchy! We still need to sort out the multiple leaf thing. |
 |
From the email I sent before reading this whole thread: The model hierarchy does add leaf hierarchy nodes automatically when model nodes are dragged under hierarchies, they're just hidden (hide from editor flag set to 1, the Q mrml scene model doesn't show them). It looks like this (my bad ASCII upward pointing arrows represent the parent node id on hierarchy nodes): Top level model hierarchy node (hide from editors = 0) -> no associated node id (by convention) Hierarchy nodes just point up to their parents. There are also display nodes in there, left out for an attempt at clarity. I was experimenting with double hierarchy trees for reporting (similar to what you might need for SH) and it gets very tricky because of the built in assumptions of a model node only being the associated node id for a single hierarchy node. Does that help a bit? I know that there was a picture that I took of a white board when Steve was explaining it to me at one point, but I haven't been able to dig it up yet. |
|
|
Also from email: "So there are leaf hierarchy nodes, just hidden from the editor. Indeed, I can see them in Data module if I check the "Show Hidden nodes" checkbox. Now that this is cleared up, I can implement my other fix that enforces nested associations (creates them for SH if a second hierarchy node associates to the data node that was also in SH)." This is very good news, means that the drawing is indeed right (sigh!). |
|
|
Committed fix |
|
|
I played around with the rulers and ROIs in the annotations module and only found one thing that wasn't working: |
|
|
Thanks so much, Nicole, for trying it out! I'm glad these things seem to work well now. |
- Anonymous
