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.

Sign up for GitHub

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

Jump to bottom

Fix windows and add utilities to visualize frames and test the interface #7

Merged
merged 16 commits into from
Jul 9, 2021
Merged

Fix windows and add utilities to visualize frames and test the interface #7

merged 16 commits into from
Jul 9, 2021

Conversation

S-Dafarra
Copy link
Member

@S-Dafarra S-Dafarra commented Jun 17, 2021 •
edited
Loading

This PR does the following:

  • it fixes the usage of this device on Windows with SteamVR. In particular:
    • it uses GLEW to setup OpenGL (in Ubuntu it was not necessary)
    • it restores the blue and green render for debug purposes
    • it workarounds (what is seems to be) a bug in the implementation of the OpenXr interface of SteamVR. In particular, it seems that the definition of the (0,0) pixel is different in monado and SteamVR, causing the cropping of the image to be wrong
    • uses two separate swapchains for the composition layers of the two eyes (basically to render the 3D space for the eyes). When using a single swapchain, there is an annoying flickering on the right eye.
    • It adds a dry run in the initialization to make sure that the correct interaction profile is loaded. This is important because when using the device as a subdevice of the JoypadControlServer, the number of buttons and axis is retrieved only at startup, while with OpenXR this information is available only after few cycles.
  • it adds a utility based on the iDyntree visualizer to visualize the frames published with the transform server.
  • it adds a separate test to debug eventual problems in the openXR interface without using the DeviceDriver machinery,

@S-Dafarra S-Dafarra changed the base branch from fixWindows to main June 25, 2021 08:02
@S-Dafarra S-Dafarra changed the title Add utility to visualize frames and test for the interface Fix windows and add utilities to visualize frames and test the interface Jun 25, 2021
@S-Dafarra S-Dafarra mentioned this pull request Jun 25, 2021
Closed
traversaro
traversaro approved these changes Jun 25, 2021
View reviewed changes
Copy link
Contributor

@traversaro traversaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment.

S-Dafarra and others added 14 commits June 28, 2021 14:13
@S-Dafarra
Using GLFW caps to be compatible with Windows.
a1eeaed
@S-Dafarra
Fix Windows compilation
0831009
@S-Dafarra
Added initialization of GLEW to avoid segfaults.
4a1f85c
@S-Dafarra
Restored green and blue render for debug.
bd0d46a
@S-Dafarra
Fixed visualization issues on Windows.
bb42ada 
The right eye was flickering. Fixed by inverting the left and right eye in the swapchain.
The textures were not visualized. The mipmap was not properly set, with OpenGL marking the texture as incomplete.
In addition, it seems that the cropping works differently from Monado to SteamVR.
@S-Dafarra
Fixed use of JoypadControlServer.
359b97f 
The JoypadControlServer gets the number of buttons only at startup, while OpenXr may set the interaction profile after few iterations.
@S-Dafarra
The depth of the composition layer was not properly initialized.
599c045
@S-Dafarra
Using two separate swapchains for left and right projection view.
2962a15
@S-Dafarra
Republish on frametransform last known position if the communication …
67e501b 
…got lost.
@S-Dafarra
Added utility to visualize frames from transform server.
d6a37e9
@S-Dafarra
Fixed typo.
94293fa
@S-Dafarra
Bumped iDynTree version.
eed7056
@S-Dafarra
Added separate test to check memory leaks.
f20690b
@S-Dafarra
Enable the compilation of the separate test from CMake option.
721c8f1
@S-Dafarra
Copy link
Member Author

I just rebased on top of main and on top of some modifications I did on the branch https://github.com/dic-iit/yarp-device-openxrheadset/tree/fixWindows in these days.

@S-Dafarra
Using c++20 instead of manually setting the flag for MSVC.
18b48aa 
Bumping the minimum Cmake version since cxx_std_20 is available only from 3.12.
@traversaro
Copy link
Contributor

@S-Dafarra was the review request on @AngeloRendiniello intentional or it is a typo?

@S-Dafarra
Copy link
Member Author

@S-Dafarra was the review request on @AngeloRendiniello intentional or it is a typo?

You mentioned to me he would have worked on a related thing soon, so I thought to start involving him in the development process. I hope I did not misunderstand 😁

@S-Dafarra
Copy link
Member Author

Also, @kouroshD let me know if you need time to go through the code, or I can proceed.

@traversaro
Copy link
Contributor

@S-Dafarra was the review request on @AngeloRendiniello intentional or it is a typo?

You mentioned to me he would have worked on a related thing soon, so I thought to start involving him in the development process. I hope I did not misunderstand 😁

Ahhh, ok! In the end it turned out that compiling YARP for HoloLens2 (vcpkg triplet: arm64-uwp) was not so obvious (see robotology/robotology-vcpkg-ports#15 (comment)) and given our limited needs (we just need to get the camera feed from a RTSP camera visualized) probably using simpler libraries already available in vcpkg for arm64-uwp . However, if @AngeloRendiniello wanted to have a look at a not so simple OpenXR application, feel free to do so!

@kouroshD
Copy link

kouroshD commented Jul 1, 2021

Also, @kouroshD let me know if you need time to go through the code, or I can proceed.

Yes, if it is not blocking this PR, I would like to review it. Probably, it will become the first days of next week.

kouroshD
kouroshD approved these changes Jul 8, 2021
View reviewed changes
Copy link

@kouroshD kouroshD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @S-Dafarra. Looks good to me.

@S-Dafarra
Copy link
Member Author

Thanks @traversaro and @kouroshD, I am merging.

@S-Dafarra S-Dafarra merged commit b560d60 into main Jul 9, 2021
@S-Dafarra S-Dafarra deleted the separateTest branch July 9, 2021 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kouroshD kouroshD kouroshD approved these changes

@traversaro traversaro traversaro approved these changes

@AngeloRendiniello AngeloRendiniello Awaiting requested review from AngeloRendiniello

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@S-Dafarra @traversaro @kouroshD