Streamline Endoscopy UX removing "Use this curve" button

[jcfr]

Selecting the curve is now sufficient to work with the associated flythrough path. Also listen to the more specific PointEndInteractionEvent to only update the curve after user is done updating the curve.

Instantiate one EndoscopyLogic and simplify its API

... introducing:

• updateCameraFromOrientationAtIndex()
• getNumberOfControlPoints()
• saveOrientationAtIndex()
• removeOrientationAtIndex()
• createTransformFromInputCurve() / getTransformFromInputCurve() / setInputCurveTransform()
• createCursorFromInputCurve() / getCursorFromInputCurve() / setInputCurveCursor()

... renaming:

• setControlPoints to setControlPointsByResamplingAndInterpolationFromInputCurve
• interpolateOrientations to interpolateOrientationsForControlPoints
• loadCameraOrientations() to getCameraOrientationsFromInputCurve()
• saveCameraOrientations() to setInputCurveCameraOrientations()
• onPlayButtonToggled() to setPlaybackEnabled() (to allow programmatic start/stop of playback)

... updating:

• interpolateOrientationsForControlPoints() to require a cameraOrientations parameter. This allows to remove the need for maintaining a dedicated cameraOrientations instance variable

.. removing:

• createCursor()

It also revisits how camera, camera transform ad camera cursor are referenced. These are now directly referenced from the input curve using Camera, CameraTransform and CameraCursor reference role.

Other changes:

• convert relative orientation API to staticmethod
• remove unneeded instance variable related to Endoscopy model export
• consolidate all widget update into updateWidgetFromMRML()
• remove ignoreInputCurveModified instance variable and instead introduce a updatingControlPoints variable to guard setControlPoints()
• allow to set and get the input curve CameraOrientations independently of the EndoscopyWidget. This is done through getCameraOrientationsFromInputCurve() and setInputCurveCameraOrientations()

[jcfr]

To ensure all parameters can be updated programmatically and independently of the UI, it would also be nice to either introduce parameter node or set dedicated attributes directly on the curve node for the following:

• frame
• frame skip
• frame delay
• view angle
• plane normal

Association of the following object also needs to be captured at the MRML level (instead of a logic variable):

• camera
• cursor transform
• cursor fiducial

@Leengit Is it something you could help with ?

[jcfr]

Association of the following object also needs to be captured at the MRML level (instead of a logic variable)

@lassoan Do we already have an established pattern to "define" the equivalent of reference role from python ?

[lassoan]

Do we already have an established pattern to "define" the equivalent of reference role from python ?

I'm not exactly sure what you mean but we store the user's node selections on the GUI as node references in the parameter node.

[Leengit]

This looks great -- thank you for the effort! I've made a bunch of suggestions.

[jcfr]

you mean but we store the user's node selections on the GUI as node references in the parameter node.

@lassoan Flythrough parameters (e.g view angle) are saved as regular node attributes, relationship to nodes are set as node references ("Camera", "CameraTransform" and "CameraCursor")

Since the "inputCurve" is what is driving the workflow, I didn't introduce a parameter node.

By convention, should we start these with a lower-case letter ? I kept the upper-case letter for historical reason but this can be changed. I am asking before reference role are currently lower-cased (e.g display, storage, ...)

[Leengit]

To get you a quick review, I have not yet looked at your most recent changes to the code. However, my testing by running 3D Slicer shows that almost everything is beautiful. The one problem I am still seeing is that when I Close Scene, my console prints out

Close main MRML scene
~/git/Slicer/Slicer-SuperBuild-Release-Qt5.15.2/Slicer-build/bin/Python/slicer/util.py:2786: UserWarning: does not have observer
  warn("does not have observer")
~/git/Slicer/Slicer-SuperBuild-Release-Qt5.15.2/Slicer-build/bin/Python/slicer/util.py:2771: UserWarning: already has observer
  warn("already has observer")
QXcbConnection: XCB error: 3 (BadWindow), sequence: 39328, resource id: 6308414, major code: 40 (TranslateCoords), minor code: 0

All told, this represents an improvement so I am marking it as approved.

[Leengit]

This is looking good. I've made a few comments about the code; use them as you see fit.

[Leengit]

Thank you for your continued efforts. In case it isn't on your radar, when I Close Scene I still get messages in the 3D Slicer Python Console

/home/local/KHQ/lee.newberg/git/Slicer/Slicer-SuperBuild-Release-Qt5.15.2/Slicer-build/bin/Python/slicer/util.py:2786: UserWarning: does not have observer
  warn("does not have observer")
[Qt] void qSlicerSaveDataDialogPrivate::updateStatusIconFromMessageCollection(int, vtkMRMLMessageCollection*, bool) Data save information: ""
[Qt] void qSlicerSaveDataDialogPrivate::updateStatusIconFromMessageCollection(int, vtkMRMLMessageCollection*, bool) Data save information: ""
[Qt] void qSlicerSaveDataDialogPrivate::updateStatusIconFromMessageCollection(int, vtkMRMLMessageCollection*, bool) Data save information: ""
[Qt] void qSlicerSaveDataDialogPrivate::updateStatusIconFromMessageCollection(int, vtkMRMLMessageCollection*, bool) Data save information: ""
[Qt] void qSlicerSaveDataDialogPrivate::updateStatusIconFromMessageCollection(int, vtkMRMLMessageCollection*, bool) Data save information: ""
[Qt] void qSlicerSaveDataDialogPrivate::updateStatusIconFromMessageCollection(int, vtkMRMLMessageCollection*, bool) Data save information: ""
[Qt] Close main MRML scene

[jcfr]

Can you locally modify util.py (e.g in the build tree) by adding an exception like this ?

diff --git a/Base/Python/slicer/util.py b/Base/Python/slicer/util.py
index 6e4c482edd..df3df4a75e 100644
--- a/Base/Python/slicer/util.py
+++ b/Base/Python/slicer/util.py
@@ -2784,6 +2784,7 @@ class VTKObservationMixin:
             obj.RemoveObserver(tag)
         except KeyError:
             warn("does not have observer")
+            raise Exception("does not have observer")
 
     def getObserver(self, obj, event, method, default=None):
         try:

Since I am unable to reproduce, this should allow you to identify the source of the message.

[Leengit]

Can you locally modify util.py....

I get the below. (First, I download a CT chest, segment its airway, build a curve inside the airway, play the Endoscopy flythrough, save data to a new directory and then Close Scene.)

~/git/Slicer/Slicer-SuperBuild-Release-Qt5.15.2/Slicer-build/bin/Python/slicer/util.py:2786: UserWarning: does not have observer
  warn("does not have observer")
Traceback (most recent call last):
  File "~/git/Slicer/Slicer-SuperBuild-Release-Qt5.15.2/Slicer-build/bin/Python/slicer/util.py", line 2783, in removeObserver
    group, tag, priority = methods.pop(method)
KeyError: <bound method AirwaySegmentationWidget.updateGUIFromParameterNode of <AirwaySegmentation.AirwaySegmentationWidget object at 0x7f9f08336e50>>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "~/git/Slicer/ExtensionsIndex-SuperBuild-Release-Qt5.15.2/AirwaySegmentation-build/lib/Slicer-5.7/qt-scripted-modules/AirwaySegmentation.py", line 125, in onSceneStartClose
    self.setParameterNode(None)
  File "~/git/Slicer/ExtensionsIndex-SuperBuild-Release-Qt5.15.2/AirwaySegmentation-build/lib/Slicer-5.7/qt-scripted-modules/AirwaySegmentation.py", line 169, in setParameterNode
    self.removeObserver(self._parameterNode, vtk.vtkCommand.ModifiedEvent, self.updateGUIFromParameterNode)
  File "~/git/Slicer/Slicer-SuperBuild-Release-Qt5.15.2/Slicer-build/bin/Python/slicer/util.py", line 2787, in removeObserver
    raise Exception("does not have observer")
Exception: does not have observer
[Qt] Close main MRML scene

[jcfr]

Thanks for the feedback, the stacktrace shows that the issue is unrelated to the Endoscopy module and originated from AirwaySegmentationWidget.

I will look into fixing AirwaySegmentationWidget, in the meantime, would appreciate if you could approve 🙏

[Leengit]

Thanks for all your effort and good work. I approve.