-
Notifications
You must be signed in to change notification settings - Fork 59
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
BUG: Fix integration with updated Slicer event delegation and VTK OpenVR API #131
Merged
jcfr
merged 4 commits into
KitwareMedical:master
from
jcfr:fix-slicer-event-delegation-and-vtk-openvr-api-integration
Dec 21, 2023
Merged
BUG: Fix integration with updated Slicer event delegation and VTK OpenVR API #131
jcfr
merged 4 commits into
KitwareMedical:master
from
jcfr:fix-slicer-event-delegation-and-vtk-openvr-api-integration
Dec 21, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was
linked to
issues
Dec 16, 2023
jcfr
commented
Dec 16, 2023
jcfr
commented
Dec 16, 2023
jcfr
commented
Dec 16, 2023
jcfr
commented
Dec 16, 2023
jcfr
commented
Dec 16, 2023
VirtualReality/MRML/vtkVirtualRealityViewInteractorObserver.cxx
Outdated
Show resolved
Hide resolved
jcfr
commented
Dec 16, 2023
VirtualReality/MRML/vtkVirtualRealityViewInteractorObserver.cxx
Outdated
Show resolved
Hide resolved
jcfr
force-pushed
the
fix-slicer-event-delegation-and-vtk-openvr-api-integration
branch
from
December 16, 2023 00:42
bbd4786
to
eeaac5a
Compare
sankhesh
reviewed
Dec 19, 2023
VirtualReality/MRML/vtkVirtualRealityViewInteractorObserver.cxx
Outdated
Show resolved
Hide resolved
VirtualReality/MRML/vtkVirtualRealityViewInteractorObserver.cxx
Outdated
Show resolved
Hide resolved
VirtualReality/MRML/vtkVirtualRealityViewInteractorObserver.cxx
Outdated
Show resolved
Hide resolved
VirtualReality/MRML/vtkVirtualRealityViewInteractorObserver.cxx
Outdated
Show resolved
Hide resolved
@jcfr I can confirm that this PR allows me to compile I'll test it when I get in the office tomorrow and report here if I run into any runtime issues. |
jcfr
commented
Dec 19, 2023
VirtualReality/MRML/vtkVirtualRealityViewInteractorObserver.cxx
Outdated
Show resolved
Hide resolved
jcfr
force-pushed
the
fix-slicer-event-delegation-and-vtk-openvr-api-integration
branch
8 times, most recently
from
December 21, 2023 09:59
79cb8cd
to
6a7f5ba
Compare
…nVR API Addressing issues highlighted in Slicer/Slicer@1cc3b2c62, this commit introduces the `vtkVirtualRealityViewInteractorObserver` class. This addition streamlines event delegation to MRML displayable managers and their associated widgets by leveraging observation of the interactor style. The modification enables the `vtkVirtualRealityViewInteractorStyle` to inherit from `vtkOpenVRInteractorStyle`, eliminating the need for redundant code implementation and duplication. It also updates the use of `MapInputToAction()` to map "eventId to state" following the VTK refactoring introduced in Kitware/VTK@b7f02e622 (update openvr to action based input model). Co-authored-by: Lucas Gandel <[email protected]> Co-authored-by: Sankhesh Jhaveri <[email protected]>
This commit revisits the invocation of `vtkMRMLViewInteractorStyle::ProcessEvents` to prioritize event processing specific to `VirtualReality`. The additional processing is now performed only if the events have not been processed already. Furthermore, this commit restores the support for calling `OnStartGesture` and `OnEndGesture`, which was inadvertently omitted in the previous commit titled "BUG: Fix integration with updated Slicer event delegation and VTK OpenVR API." Co-authored-by: Sankhesh Jhaveri <[email protected]>
This commit follows up to "BUG: Fix integration with updated Slicer event delegation and VTK OpenVR API". It ensures that existing event data objects are consistently passed down to the `Delegate` functions and associated displayable managers.
…ts Callback Following the previous commit titled "BUG: Ensure passing of vtkEventData to displayable managers," this update guarantees the effective invocation of the newly introduced custom delegation function, designed to handle a calldata argument. This ensures that all processing is funneled through the `vtkVirtualRealityViewInteractorObserver::CustomProcessEvents` callback. The callback is responsible for handling 3D event data and passing it down appropriately.
jcfr
force-pushed
the
fix-slicer-event-delegation-and-vtk-openvr-api-integration
branch
from
December 21, 2023 21:39
5ee8972
to
969f93b
Compare
jcfr
deleted the
fix-slicer-event-delegation-and-vtk-openvr-api-integration
branch
December 21, 2023 21:43
3 tasks
jcfr
added a commit
to jcfr/SlicerVirtualReality
that referenced
this pull request
Dec 27, 2023
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).
jcfr
added a commit
to jcfr/SlicerVirtualReality
that referenced
this pull request
Dec 27, 2023
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).
jcfr
added a commit
that referenced
this pull request
Dec 30, 2023
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 (#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).
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addressing issues highlighted in Slicer/Slicer@1cc3b2c62, this commit introduces the
vtkVirtualRealityViewInteractorObserver
class. This addition streamlines event delegation to MRML displayable managers and their associated widgets by leveraging observation of the interactor style.The modification enables the
vtkVirtualRealityViewInteractorStyle
to inherit fromvtkOpenVRInteractorStyle
, eliminating the need for redundant code implementation and duplication.It also updates the use of
MapInputToAction()
to map "eventId to state" following the VTK refactoring introduced in Kitware/VTK@b7f02e622 (update openvr to action based input model
).This is also required the following VTK commit1:
Update: The corresponding changes along with the latest
V/OpenVR/OpenXR
VTK updates have been integrated intoSlicer/VTK
through Slicer/VTK#46 and inSlicer
through Slicer/Slicer#7487Footnotes
https://github.com/Slicer/VTK/compare/slicer-v9.2.20230607-1ff325c54-2...Slicer:VTK:slicer-v9.2.20230607-1ff325c54-2_slicervr-pr-131↩