Skip to content

BUG: Add missing vtkVirtualRealityViewInteractorStyle::SetupActions#105

Merged
jcfr merged 1 commit intoKitwareMedical:masterfrom
jcfr:add-missing-SetupActions
Jan 13, 2023
Merged

BUG: Add missing vtkVirtualRealityViewInteractorStyle::SetupActions#105
jcfr merged 1 commit intoKitwareMedical:masterfrom
jcfr:add-missing-SetupActions

Conversation

@jcfr
Copy link
Contributor

@jcfr jcfr commented Jan 11, 2023 •
edited
Loading

This ensures the function vtkOpenVRRenderWindowInteractor::AddAction originally introduced in b7f02e622 (update openvr to action based input model) is called.

Considering that:

  • (1) the class vtkVirtualRealityViewInteractorStyle derives from vtkMRMLViewInteractorStyle instead of vtkOpenVRInteractorStyle (itself deriving from vtkVRInteractorStyle)

  • and (2) the function SetupActions is defined and called from vtkVRInteractorStyle::SetInteractor()

this commit copies the function SetupActions to vtkVirtualRealityViewInteractorStyle and ensures it is called.

@cpinter
Copy link
Collaborator

cpinter commented Jan 12, 2023

I tried to build this with the latest Slicer but failed with something like SetupActions method with override specifier did not override any base class functions. I guess I'd need an even newer VTK right?

@jcfr
Copy link
Contributor Author

jcfr commented Jan 12, 2023

Building again the Slicer/VTK fork currently associated with Slicer master is expected to work (I tested the build on Linux).

I will test with visual studio and report back.

@jcfr
Copy link
Contributor Author

jcfr commented Jan 12, 2023

The override keyword should be removed.

@cpinter
Copy link
Collaborator

cpinter commented Jan 12, 2023

Removed the keyword, build succeeds. The only difference I noticed in functioning is that I'm getting an endless stream of this error:
qMRMLVirtualRealityViewPrivate::updateTransformNodeWithHMDPose(void) : Unable to retrieve HMD pose

@jcfr jcfr force-pushed the add-missing-SetupActions branch 2 times, most recently from ab4e0a3 to 1a40155 Compare January 12, 2023 18:33
@jcfr @LucasGandel
BUG: Add missing vtkVirtualRealityViewInteractorStyle::SetupActions
8a6a522 
This ensures the function `vtkOpenVRRenderWindowInteractor::AddAction`
originally introduced in b7f02e622 (update openvr to action based input model)
is called.

Considering that:

* (1) the class `vtkVirtualRealityViewInteractorStyle` derives from `vtkMRMLViewInteractorStyle`
  instead of `vtkOpenVRInteractorStyle` (itself deriving from `vtkVRInteractorStyle`)

* and (2) the function `SetupActions` is defined and called from `vtkVRInteractorStyle::SetInteractor()`

this commit copies the function `SetupActions` to `vtkVirtualRealityViewInteractorStyle`
and ensures it is called.

Co-authored-by: Lucas Gandel <lucas.gandel@kitware.com>
@jcfr jcfr force-pushed the add-missing-SetupActions branch from 1a40155 to 8a6a522 Compare January 12, 2023 18:36
@jcfr
Copy link
Contributor Author

jcfr commented Jan 12, 2023

endless stream of this error ... Unable to retrieve HMD pose

This means that the call to GetOpenVRPose (introduced in Slicer/VTK@c333750 1 ) is failing to return a valid pose.

SlicerVirtualReality/VirtualReality/Widgets/qMRMLVirtualRealityView.cxx

Lines 578 to 592 in e504845

vr::TrackedDevicePose_t* tdPose;
this->RenderWindow->GetOpenVRPose(vtkEventDataDevice::HeadMountedDisplay, &tdPose);
if (tdPose == nullptr)
{
qCritical() << Q_FUNC_INFO << ": Unable to retrieve HMD pose";
}
if (tdPose == nullptr || tdPose->eTrackingResult != vr::TrackingResult_Running_OK)
{
node->SetAttribute("VirtualReality.HMDActive", "0");
}
else
{
node->SetAttribute("VirtualReality.HMDActive", "1");
}

@sankhesh @LucasGandel Could you review the commit I added to the Slicer/VTK fork and see if I missed anything ?

Footnotes

  1. https://github.com/Slicer/VTK/tree/slicer-v9.1.20220125-efbe2afc2 ↩

@jcfr
Copy link
Contributor Author

jcfr commented Jan 13, 2023

To follow up, the issue with retrieving the OpenVR pose should be fixed in Slicer/Slicer#6779

@jcfr jcfr merged commit 13bef66 into KitwareMedical:master Jan 13, 2023
@jcfr jcfr deleted the add-missing-SetupActions branch January 13, 2023 12:01
This was referenced Dec 21, 2023
Merged
Closed
@jcfr jcfr mentioned this pull request Dec 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jcfr @cpinter