Organize interaction between virtual fit and transducer tracking

[sadhana-r]

Closes #195
Closes #358
Closes #367

Should be merged after #375 as it uses the model colors introduced in that PR.

Depends on openlifu python fix: OpenwaterHealth/OpenLIFU-python#330

• Merge openlifu PR and update python requirements
• Fix bugs in how visibility settings interact between wizard and main window

Changes introduced

• After running virtual fit, the skin segmentaiton model node is added to the scene and displayed *at 50% opacity in VF color (blue))
• The same skin segmentation model is used for tracking (if available)
• Model rendering options have been added to the tansducer tracking module to enable visualization of the virtual fit transducer position, photoscan and skin segmentation in the main slicer scene.
• The VF result is shown at 50% opacity. The user can adjust the photoscan and skin segmentation opacity.
• Photoscans can be displayed if selected, loaded in and associated with a photoscan-volume tracking transform
• Skin segmentations can be displayed if a volume is selected and a skin mesh has been added to the scene (via tracking or virtual fit)

For review

The challenging part of this PR is ensuring that the visibility settings are enabled/disabled correctly based on the state of the scene and the currenly selected input options. I tested a few different sessions/scenarios but it would be helpful if you could test it out more incase I missed any edge cases. The visibility settings in the main slicer scene also should not intefere with how models are displayed within the wizard.
Since these enabled-ness checks need to happen whever the combo boxes are modified, the check is linked to updateInputOptions and indexChanged on the input drop downs. However, these functions are called ** ALOT** i.e. when nodes are added/removed/parameter nodes are modified etc which leads to alot of recursive function calls and unwanted behavior. To avoid this, I had to add various flags to minimize issues. Feel free to look over the code in case if you have suggestions for how to better handle this. Each commit addresses a separate issue to you can look over the changes from each commit individually.

[sadhana-r]

@ebrahimebrahim tagging you as reviewer as well incase you want to test out the visibility settings and have additional comments on how it should work.

[ebrahimebrahim]

Seeing a textured photoscan and MRI slices together in the main scene is so cool!

---

I noticed in the data module that these fiducial nodes are present after transducer tracking

Is that expected? When I enable their visibility, the MRI ones are where I last placed them, while the photoscan ones are all at the origin.

---

I did a virtual fit, then I moved the target which triggered the virtual fit result to get deleted. However the "View virtual fit transducer position" checkbox in the transducer tracking module remained enabled. When I clicked it, it got checked and disabled at the same time.

A solution may be to make updateVirtualFitResultForDisplay get called on any node add and delete events, rather than trying to manually call it in all the right place. As long as it's not an expensive call after the first time it's called and does its thing, that should be fine. Another solution is to just manually call it at the end of the VF revoking/deletion function, and hope that there isn't some other place we have forgotten to call it.

---

Somehow after running transducer tracking the Target node lost its visibility in the slice views. I was able to turn it back on using the markups module:

I am guessing the target is accidentally getting lumped in with registration fiducials at some step where you modify slice view visibility

---

If I load the session, run through transducer tracking, reload the session, and then again try to run through transducer tracking, then I get the following errors and the wizard never opens successfully:

Traceback (most recent call last):
  File "/home/ebrahimebrahim/openwater/SlicerOpenLIFU/build/lib/Slicer-5.8/qt-scripted-modules/OpenLIFUTransducerTracker.py", line 1923, in onRunTrackingClicked
    self.wizard = TransducerTrackingWizard(
  File "/home/ebrahimebrahim/openwater/SlicerOpenLIFU/build/lib/Slicer-5.8/qt-scripted-modules/OpenLIFUTransducerTracker.py", line 951, in __init__
    self.transducer.set_cloned_virtual_fit_model(best_virtual_fit_result_node)
  File "/home/ebrahimebrahim/openwater/SlicerOpenLIFU/build/lib/Slicer-5.8/qt-scripted-modules/OpenLIFULib/transducer.py", line 276, in set_cloned_virtual_fit_model
    self.cloned_virtual_fit_model.SetAndObserveTransformNodeID(virtual_fit_transform.GetID())
AttributeError: 'NoneType' object has no attribute 'SetAndObserveTransformNodeID'
[VTK] Generic Warning: In vtkMRMLSubjectHierarchyNode.cxx, line 3771
[VTK] vtkMRMLSubjectHierarchyNode::GetSubjectHierarchyNode: Invalid scene given
[Qt] void qSlicerSubjectHierarchyPluginLogic::onNodeAboutToBeRemoved(vtkObject*, vtkObject*) : Failed to access subject hierarchy node
[VTK] GetReferencingNodes: null node or referenced node
[VTK] GetReferencingNodes: null node or referenced node
[VTK] Warning: In vtkMRMLSubjectHierarchyNode.cxx, line 2156
[VTK] vtkMRMLSubjectHierarchyNode (0x590dc66ab370): GetItemDataNode: Invalid item ID given
[VTK] Warning: In vtkMRMLSubjectHierarchyNode.cxx, line 2217
[VTK] vtkMRMLSubjectHierarchyNode (0x590dc66ab370): GetItemName: Invalid item ID given
[VTK] Warning: In vtkMRMLSubjectHierarchyNode.cxx, line 2256
[VTK] vtkMRMLSubjectHierarchyNode (0x590dc66ab370): GetItemLevel: Invalid item ID given
[VTK] Warning: In vtkMRMLSubjectHierarchyNode.cxx, line 2923
[VTK] vtkMRMLSubjectHierarchyNode (0x590dc66ab370): GetItemParent: Invalid item ID given
[VTK] CreateHierarchyItem: Failed to find parent subject hierarchy item by ID 0
[VTK] Warning: In vtkMRMLSubjectHierarchyNode.cxx, line 2156
[VTK] vtkMRMLSubjectHierarchyNode (0x590dc66ab370): GetItemDataNode: Invalid item ID given

[ebrahimebrahim]

when nodes are added/removed/parameter nodes are modified etc which leads to alot of recursive function calls and unwanted behavior. To avoid this, I had to add various flags to minimize issues.

Flags can make things very confusing, but I understand that sometimes we are cornered by what events slicer exposes to us and just have to do that. But yeah if flags can be avoided it's best to avoid them I think. If you point to specific place where you were getting recursion I can take a look and see if I agree that a flag is a good solution, but it's totally your call!

[sadhana-r]

I noticed in the data module that these fiducial nodes are present after transducer tracking
Is that expected? When I enable their visibility, the MRI ones are where I last placed them, while the photoscan ones are all at the origin.

Yep this is expected! They are left in the scene so that when you re-open the transducer tracking wizard, you can see where you last placed the landmarks and don't have to start from scratch.
I belive that the photoscan ones just look like they are at the origin but they are actually in the original photoscan coordinate space (which is really small and near the origin). If you set the photoscan fiducial nodes to observe the photoscan-volume transform, you should see them where you last placed them.

[ebrahimebrahim]

I belive that the photoscan ones just look like they are at the origin but they are actually in the original photoscan coordinate space (which is really small and near the origin). If you set the photoscan fiducial nodes to observe the photoscan-volume transform, you should see them where you last placed them.

Oh, that's right -- that's probably what it was!

[sadhana-r]

Flags can make things very confusing, but I understand that sometimes we are cornered by what events slicer exposes to us and just have to do that. But yeah if flags can be avoided it's best to avoid them I think. If you point to specific place where you were getting recursion I can take a look and see if I agree that a flag is a good solution, but it's totally your call!

The recursion mainly occured in updateVirtualFitResultForDisplay. This function is called by the indexChanged signal on the input combo boxes, or when there is an update to the input options (updateInputOptions).
updateVirtualFitResultForDisplay sometimes involves cloning the transducer model node (to create the duplicate model for visualization) which triggers nodeAdded/nodeRemoved which triggers updateInputOptions.

I tried removing updateVirtualFitResultForDisplay from updateInputOptions but updateInputOptions calls self.algorithm_input_widget.update() which clears all the combo boxes first, and clearing the combo boxes triggers indexChanged on the combo boxes.

So updateVirtualFitResultForDisplay -> nodeAdded -> updateInputOptions -> indexChanged signal -> updateVirtualFitResultForDisplay

Separately, the transducer tracking wizard adds/removes alot of temporary nodes. This triggers updateInputOptions which triggers updates the photoscan/skinmesh visibility setting enabled-ness. So I added a flag to updateInputOptions so that it ignores any function calls that occur due to nodes being added/removed by the wizard.

[ebrahimebrahim]

Yikes..

Thanks for explaining the source of the recursion -- I don't have a good solution but I feel better about the flags now 🙂

[sadhana-r]

Somehow after running transducer tracking the Target node lost its visibility in the slice views. I was able to turn it back on using the markups module:

Good catch! I have added a commit to fix this.

I did a virtual fit, then I moved the target which triggered the virtual fit result to get deleted. However the "View virtual fit transducer position" checkbox in the transducer tracking module remained enabled. When I clicked it, it got checked and disabled at the same time.

Hmm, I am not able to reproduce this. When I do this, the checkbox gets disabled as expected. updateVirtualFitResultForDisplay is called in updateInputOptions which is called by nodeAdded and nodeRemoved so the function should get triggered when the virtual fit results are removed...

I have now also pushed a commit to disable the View virtual fit transducer position option when the transducer is already at the virtual fit position.

[ebrahimebrahim]

Hmm, I am not able to reproduce this. When I do this, the checkbox gets disabled as expected. updateVirtualFitResultForDisplay is called in updateInputOptions which is called by nodeAdded and nodeRemoved so the function should get triggered when the virtual fit results are removed...

I have now also pushed a commit to disable the View virtual fit transducer position option when the transducer is already at the virtual fit position.

I couldn't reproduce it either on the latest version of this branch, so never mind! Maybe some of your updates fixed it

[arhowe00]

• After running virtual fit, the skin segmentaiton model node is added to the scene and displayed *at 50% opacity in VF color (blue))
• The same skin segmentation model is used for tracking (if available)
  ...
• Skin segmentations can be displayed if a volume is selected and a skin mesh has been added to the scene (via tracking or virtual fit)

Works as stated, in many cases. Looks really cool! A few questions:

Even when there is a VF result, the skin segmentation will not be displayed if loading a session after that step has been complete? For example, I loaded Soren's "test session", and the segmentation was not displayed.

Furthermore, the transducer tracking was approved in that session. When I load the session, turn on photoscan visibility in the tt module (which requires me to first "Preview Photoscan"), I can turn on the visibility thus showing the photoscan.

So to clarify, is doing "Preview Photoscan" always a requirement? Then, when the photoscan was showing after previewing, I clicked through TT without actually modifying anything, after which the skin segmentation was displayed and checked. Should this have been displayed earlier?

Following from above, I attempted to use the
Skin segmentations can be displayed if a volume is selected and a skin mesh has been added to the scene (via tracking or virtual fit)

[sadhana-r]

Even when there is a VF result, the skin segmentation will not be displayed if loading a session after that step has been complete?

So to clarify, is doing "Preview Photoscan" always a requirement? Then, when the photoscan was showing after previewing, I clicked through TT without actually modifying anything, after which the skin segmentation was displayed and checked. Should this have been displayed earlier?

Yes - these are the expected behaviors for now! The skin segmentation is only added to the scene after the virtual fit or transducer tracking is run, and the photoscan is only loaded after preview or tracking. When disabling the options, I mention these requirements in the tooltips so its hopefully less confusing.

I agree, it would be nice to have them computed if a session with an existing VF/TT result is loaded. @ebrahimebrahim and I were discussing this as part of #391 (comment) and decided to wait till we get more feedback from Peter before making any changes.

[arhowe00]

Excellent! This is substantial work on integrating virtual fit results with transducer tracking. I also appreciate the careful handling of the visibility logic to avoid conflicts between the main scene and wizard views. I've had similar issues as yours with needing to use flags to conditionally prevent update routines. It may be possible to reduce usage of these flags if you tag nodes with MRML attributes when created (e.g. the cloned nodes) and then screen for them in a similar manner as when checking the type of the node:

    def onNodeAdded(self, caller, event, node : slicer.vtkMRMLNode) -> None:
        """ Update volume and photoscan combo boxes when nodes are added to the scene"""
        if node.IsA('vtkMRMLMarkupsFiducialNode'):
            self.watch_fiducial_node(node)
        # what would happen if we screen the node here?
        self.updateInputOptions()

However, given the complexity, your approach to managing recursive updates may already have considered this type of solution as not encompassing enough. Does the suggested approach fall short?

Regardless, approving this for merging.

[sadhana-r]

I've had similar issues as yours with needing to use flags to conditionally prevent update routines. It may be possible to reduce usage of these flags if you tag nodes with MRML attributes when created (e.g. the cloned nodes) and then screen for them in a similar manner as when checking the type of the node:

    def onNodeAdded(self, caller, event, node : slicer.vtkMRMLNode) -> None:
        """ Update volume and photoscan combo boxes when nodes are added to the scene"""
        if node.IsA('vtkMRMLMarkupsFiducialNode'):
            self.watch_fiducial_node(node)
        # what would happen if we screen the node here?
        self.updateInputOptions()

However, given the complexity, your approach to managing recursive updates may already have considered this type of solution as not encompassing enough. Does the suggested approach fall short?

This is a good idea and could work! I had not considered this.I wasn't sure if it would complain during node removal about not being able to screen the removed node but I did a small test and it seems to work. I think I will leave this as is for now, since I would have to be careful and make sure I am tagging all the required nodes/not breaking anything. I'll test this out more if ther eis time once I complete some of my other tasks.