Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancements and Refactoring for XR Runtimes Integration #149

Merged
merged 41 commits into from
Dec 30, 2023

Conversation

jcfr
Copy link
Contributor

@jcfr jcfr commented Dec 30, 2023

These changes enable the integration of XR runtimes (OpenXR and OpenVR) into SlicerVirtualReality extension. Below is a summary of the key improvements, simplifications, and cleanup:

Improvements:

  • OpenXR Support: Introduce support for the OpenXR XR runtime with a dedicated CMake option.
  • Specialized Interactors: Implement specialized interactor classes and styles for OpenXR.
  • Action Manifest Path: Support computation of the action manifest path based on the selected XR runtime.
  • OpenVR Support Option: Add a CMake option (SlicerVirtualReality_HAS_OPENVR_SUPPORT) for toggling OpenVR support.
  • RenderWindow Logic: Adapt logic for RenderWindow creation and initialization for XR runtimes, improving runtime compatibility.
  • Initialization Robustness: Improve sanity checks and displayed information during runtime initialization for better user feedback.
  • Consolidated Logic: Consolidate and simplify logic for RenderWindow creation, streamlining the process.
  • Utility Function: Add a utility function (ComputeActionManifestPath()) for computing the path based on the selected XR runtime.
  • Module State Checking: Introduce the ModuleInstalled property for checking the install state of the module.
  • Association with Logic: Associate VirtualRealityLogic with VirtualRealityView.
  • Basic Tests: Introduce basic tests for view and layout nodes.
  • Gesture Processing Delegate: Add vtkVirtualRealityViewInteractorStyleDelegate for common logic related to gesture processing.
  • Wrapping Support: Enable Python wrapping for vtkVirtualRealityViewInteractor.
  • UI Enhancement: Support for XR runtime selection in the module UI.

Simplifications:

  • Event Loop Logic: Simplify event loop logic and introduce shouldConsiderQuickViewMotion() function for improved readability.
  • Variable Naming: Rename ivar from viewUpDirectionChangeSpeed to viewUpChangeSpeed for better clarity.
  • Dependency Removal: Remove explicit dependency on "openvr.h" in TransformWidget, enhancing code modularity.
  • Naming Consistency: Improve naming consistency by including "OpenVR" in interactor and interactor style names.
  • Configuration Function: Move configuration functions to the logic class, reducing duplication and improving organization.
  • Function Relocation: Move CalculateCombinedControllerPose function to the logic class for better organization.
  • Unused Function Removal: Simplify interactor style by removing an unused picker and an unused function.

Cleanup & Removal:

  • Header Cleanup: Organize and clean up header includes for improved clarity and maintainability.
  • External Header Consistency: Use angle brackets for external header includes, following a consistent practice.
  • Function Call Order: Clean up and reorder calls in the CreateRenderWindow() function for better readability.
  • Obsolete Comments Removal: Remove obsolete comments related to original interactor debugging.
  • Base Class Modification: Remove GetCurrentGesture() from the base class, streamlining the inheritance hierarchy.
  • Obsolete Variable Removal: Remove obsolete GestureEnabledButtons ivar for codebase simplification.
  • Interactor Style Cleanup: Clean up includes and header organization from vtkVirtualRealityViewInteractorStyle.

Support for both Slicer Stable and Preview

These changes are supported in both the latest Stable Slicer release (5.6.1) and the current Preview build. The relevant changes have been backported to a dedicated branches for both the Preview build1 and the Stable2 release.

For the release, the corresponding VTK sources have been downloaded in the corresponding Slicer stable build trees located in the official build machine used to build the extensions daily.

For reference, here are the pull requests for the Slicer/VTK changes used for the Release:

Relatedimprovements and fixes

See detail below.

Acknowledgment

This significant endeavor was made possible through the support of @adamrankin and his team. Their dedication has been instrumental in shaping and advancing the project. Special thanks also go to @LucasGandel and @sankhesh for their invaluable technical support and guidance. 🙏 🚀

Footnotes

  1. slicer-v9.2.20230607-1ff325c54-2: https://github.com/Slicer/VTK/tree/slicer-v9.2.20230607-1ff325c54-2

  2. slicer-5.6-v9.2.20230607-1ff325c54-2: https://github.com/Slicer/VTK/tree/slicer-5.6-v9.2.20230607-1ff325c54-2

The function `GetMappedAction()` is added to the vtkVRInteractorStyle
base class through MR-10784.

See https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10784
Removes "GestureEnabledButtons" ivar that became obsolete following
these commits:
* c5c6d6f ("BUG: Restore complex gesture support", 2023-01-29),
* 22eba9f ("BUG: Fix integration with updated Slicer event delegation and VTK OpenVR API (KitwareMedical#131)", 2023-12-21)
* 0fa6102 ("BUG: Ensure handling of gesture triggers the "End" event", 2023-12-20)
* 512efe6 ("BUG: Do not report "Unrecognized device" if handling complex gesture", 2023-12-20)

For reference, it was originally introduced in 43bef4b ("ENH: Make trigger
button configurable", 2019-01-11).
The function `GetCurrentGesture()` is added to the vtkVRRenderWindowInteractor
base class through MR-10786.

See https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10786
Since OpenXR and OpenVR runtime each require their own instance of
interactor style, this commit relocates is a step toward reducing the
amount of code that will be duplicated.
Since OpenXR and OpenVR runtime each require their own instance of
interactor style, this commit is a step toward reducing the amount
of code specific to the interactor.

The is made possible after the following vtkOpenVRRenderWindowInteractor
updates:

* Declare AddAction() functions as virtual.
  See https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10785

* Mark ComplexGesture recognition functions as public.
  See https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10786

For reference, the "GestureButton" configuration functions were
introduced through these commites:
* 43bef4b ("ENH: Make trigger button configurable", 2019-01-11)
* e268168 ("ENH: Add python-accessible functions to capture vr
  controller events, associate buttons with gestures and enable/disable
  interaction.", 2019-05-30)
Update the RenderWindowInteractor to use the recognizer. This will allow
to have dedicated interactor for OpenVR and OpenXR runtime wihout any
code duplication.
Following 22eba9f (BUG: Fix integration with updated Slicer event
delegation and VTK OpenVR API), the VirtualReality interactor style
derives from `vtkOpenVRInteractorStyle` and all the upstream
capabilities are now available.
Also removes redundant call `InteractorStyle->SetInteractor(this->Interactor)`
Calling `Interactor->SetInteractorStyle(this->InteractorStyle)` is sufficient.
Update the InteractorStyle to use a delegate implementing the common
logic related to processing of the "Pinch3D" complex gesture, handling of
the "PositionProp" interaction and update of the view magnification. This
will allow to have dedicated interactor styles for OpenVR and OpenXR
runtime without code duplication.
Group headers and remove unused ones.

Consistently use "VR [Logic|MRML|MRMLDM|Widgets] includes" comment to
introduce headers from this extension.

Re-order include groups and sort alphabetically within groups.

Remove `vr::TrackedDevicePose_t` forward declaration that became
obsolete following commit 0c8c2e8 ("ENH: Decouple update of transform
node matrix and attributes", 2023-12-26)
This is done anticipating subsequent refactoring in which the computation
of the ActionManifestPath will be moved to the logic.
Update VirtualReality logic adding `ComputeActionManifestPath()` utility
function and update VirtualRealityView to compute the path based on the
XR runtime.

To support computing the path based on the module install state, the
`ModuleInstalled` property was also added.
This commit streamlines the process of creating a RenderWindow in the
qMRMLVirtualRealityView class by making several improvements:

- Reorganize the update of QCursor in the `createRenderWindow()` function
  around the time-consuming `RenderWindow->Initialize()` call.

- Set the view node's Error status directly within `createRenderWindow()`,
  and check for view node errors before returning from `updateWidgetFromMRML()`.

- Enhance `destroyRenderWindow()` to gracefully handle calls without
  instantiated objects.

- Update `createRenderWindow()` to set a view node error if the XR runtime is
  undefined and RenderWindow creation did not occur.
Since we systematically attempt to initialize the runtime if it is
"UndefinedXRRuntime" or if it has changed, this commit also adds support
for checking for the maximum initialization attempts and avoid recursion.
Users can now conveniently select and update the current XR Runtime.
This commit removes the explicit dependency on the "openvr.h" header in the
TransformWidget. However, it's essential to note that tracking results
are not standardized, and the status will consistently be reported as
"off" for XR runtimes other than OpenVR.
For reference, the function was originally introduced in
commit 46b252d ("Added progressive rendering", 2018-04-20)

Also renamed ivar from `viewUpDirectionChangeSpeed` to `viewUpChangeSpeed`
…d()`

Since having the head-mounted display (HMD) connected may not always be
a good proxy to check if the initialization succeeded, we instead swith to
the dedicated `GetVRInitialized` function available since commit
Kitware/VTK@bddd94efd3 ("Improve OpenVR/OpenXR module and test and enable
OpenXR in CI", 2023-01-11)

Moreover, the determination of whether the event loop should run has been
refined by relying on the GetVRInitialized function as the primary criterion.
In the case of the OpenVR runtime, the use of GetHMD is retained through the
introduction of the `hmdConnected` internal variable for enhanced flexibility.
- Introduce the CMake option `SlicerVirtualReality_HAS_OPENVR_SUPPORT`,
  initialized to OFF on macOS.

- Introduce `vtkMRMLVirtualRealityConfigure.h`, a configurable header
  facilitating conditional compilation based on the presence of the macro
  `SlicerVirtualReality_HAS_OPENVR_SUPPORT`.

- Update `vtkVirtualRealityViewInteractorObserver::GetInteractorStyleDelegate`
  to dynamically return the current delegate based on the availability
  of OpenVR support.

- Ensure that when OpenVR support is disabled, proper linkage against
  the `VTK::RenderingVR` target is maintained, providing a backend-agnostic
  interface.
- Add SlicerVirtualReality_HAS_OPENXR_SUPPORT CMake option

- Add OpenXR-SDK and vtkRenderingOpenXR external projects

- Add OpenXR specialized interactor style class for.

- Add OpenXR specialized interactor class. Thanks to the shared
  `vtkVirtualRealityComplexGestureRecognizer`, there is no duplicated code.

- Add OpenXR specialized interactor style class. Thanks to the shared
  `vtkVirtualRealityViewInteractorStyleDelegate`, there is no duplicated code.

- Update vtkSlicerVirtualRealityLogic::ComputeActionManifestPath support
  for `OpenXR`.

- Update `vtkMRMLVirtualRealityViewNode` adding `OpenXR` to `XRRuntimeType` enum

- Update `qMRMLVirtualRealityView::createRenderWindow` to instantiate `vtkOpenXR*` classes

- Update  `qMRMLVirtualRealityView::currentXRRuntime` to check for OpenXR
@jcfr
Copy link
Contributor Author

jcfr commented Dec 30, 2023

Related Slicer/VTK updates:

Related VTK merge requests

These are the associated VTK merge requests with changes being reviewed & integrated upstream:

@jcfr
Copy link
Contributor Author

jcfr commented Dec 30, 2023

Other related pull requests & contributions

SlicerVirtualReality:

Slicer:

CTK:

VTK:

  • MR-10783 VR: Accommodate separate eye transforms

  • MR-10778 BUG: Recognize OpenVR gesture where buttons are pressed consecutively

  • MR-10771 BUG: Add missing Elevation3DEvent to vtkCommand::EventHasData()

  • MR-10400 ENH: Re-introduce OpenVR API for retrieving last OpenVR pose

  • MR-9892 ENH: Re-introduce support for custom logic handling VR complex gesture

  • MR-8977 vtkInstallCMakePackageHelpers: add find_package hints for OpenVR

@jcfr jcfr merged commit 240a60a into KitwareMedical:master Dec 30, 2023
@jcfr jcfr deleted the refactor-for-xr-runtime-integration branch December 30, 2023 06:21
This was linked to issues Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant