Vr tracker support by joreg · Pull Request #213 · stride3d/stride

joreg · 2018-09-24T15:41:15Z

added a list of TrackedItems to Xenko.VirtualReality.VRDevice
for now only supported by OpenVR, the other APIs return new TrackedItem[0]
it returns all devices, including HMD, Controllers, GenericTrackers, LightHouses,..
added DeviceClass enum to file DeviceEnums.cs (previously named DeviceState.cs). the enum is taken from OpenVR
a TrackedItem also returns its SerialNumber so we can distinguish several devices of the same class
a TrackedItem also returns its BatteryPercentage
SetTrackingSpace on VRDevice now allows to choose between Seated, Standing and Raw

CLAassistant · 2018-09-24T15:41:23Z

All committers have signed the CLA.

Kryptos-FR

A few remarks on a pure technical/language side. I'll let @xen2 do the proper review.

sources/engine/Xenko.VirtualReality/TrackedDevice.cs

Kryptos-FR · 2018-09-24T23:04:21Z

sources/engine/Xenko.VirtualReality/DeviceEnums.cs

+// Distributed under the MIT license. See the LICENSE.md file in the project root for more information.
+namespace Xenko.VirtualReality
+{
+    public enum DeviceState


Why moving the enum into a different file?

before, it was one enum in a file with the same name as the enum (DeviceState). since i added a second enum concerning the Device i thought, instead of adding a second file, i'd put them both in the same file and renaming the file to a more generic name, ie. DeviceEnums.cs.

please let me know if you prefer one file per enum and i'll change it accordingly.

Don't mind so much with so small enums, but so far most of the code is one file per enum, so might be better to keep consistency.

Kryptos-FR · 2018-09-24T23:05:23Z

sources/engine/Xenko.VirtualReality/DummyDevice.cs


        public override TouchController RightHand => null;

+        public override TrackedDevice[] TrackedDevices => null;


I personally prefer collection properties to return empty rather than null.

done. now returns new ..[0]

Kryptos-FR · 2018-09-24T23:07:11Z

sources/engine/Xenko.VirtualReality/OpenVR/OpenVR.cs

            }
        }

+        public class TrackedDevice


Not a fan of having classes with the same name in different namespaces. Can cause confusion and makes it harder to find/navigate the code.

renamed the Xenko.VirtualReality one to TrackedItem so the one in OpenVR can keep the name of its original api.

…an null

…onfused with OpenVR.TrackedDevice

xen2 · 2018-10-07T09:32:45Z

I assume this is ready for review? I will take a look and merge it soon.

…uctor since it doesn't change at runtime (cherry picked from commit b6a8f4912637b7f89b0017da07df3ae6efc53a57)

joreg · 2018-10-07T23:03:51Z

@tebjan i put the SerialNumber and DeviceClass getters in Update instead of the constructor because i wasn't sure if they couldn't actually change at runtime: see, the TrackedDevice is only an index in to OpenVRs list of devices. to my understandings controllers and trackers can be added/removed (ie. paired/unpaired) at runtime which i'd assume can change their order in the list. therefore i thought putting those in update is the saver option.

if we are certain it can not happen, then of course it should go to the constructor. but then not only SerialNumber but also DeviceClass.

@xen2 otherwise yes, please.

tebjan · 2018-10-07T23:58:46Z

@joreg ah you are right, if the index of the same physical device changes during runtime, it would create confusion. we could test that with the vive trackers and the controllers.

…me for a given slot

joreg · 2018-10-10T18:09:49Z

@xen2 i just checked, and indeed, devices can be added at runtime, so i moved the serialnumber check back to update. interestingly though it seems they cannot be removed at runtime. once a slot is occupied it is occupied for runtime, even if the device is turned off.

also i allowed myself to expose another feature we need: VRDevice.SetTrackingSpace()

tebjan · 2018-10-10T23:20:10Z

added a slight improvement by re-using the same string builder in every frame.

xen2 · 2018-10-11T00:27:43Z

sources/engine/Xenko.VirtualReality/OpenVR/OpenVR.cs

+                serialNumberStringBuilder.Clear();
+                Valve.VR.OpenVR.System.GetStringTrackedDeviceProperty((uint)TrackerIndex, ETrackedDeviceProperty.Prop_SerialNumber_String, serialNumberStringBuilder, StringBuilderSize, ref error);
+                if (error == ETrackedPropertyError.TrackedProp_Success)
+                    SerialNumber = serialNumberStringBuilder.ToString();


Does that mean it will allocate every frame? (something we are trying to avoid)

what do you mean by "allocate every frame"? what those lines do, is accessing the serialnumber property of the trackeddevice with the index TrackerIndex. i don't see anything getting allocated there.

StringBuilder.ToString() will allocate a new string at every update.

ok, any idea to improve that?

maybe we could check somehow whether the device slot has changed? that and create is the only moment when the serial number needs reading...

another idea would be to place the code into the getter of the property or a method of the TrackedDevice, then it would only be read and allocated if someone asks for it...

tebjan · 2018-09-24T16:13:55Z

sources/engine/Xenko.VirtualReality/OpenVR/OpenVR.cs

+            {
+                var error = ETrackedPropertyError.TrackedProp_Success;
+                var result = new System.Text.StringBuilder((int)64);
+                Valve.VR.OpenVR.System.GetStringTrackedDeviceProperty((uint)TrackerIndex, ETrackedDeviceProperty.Prop_SerialNumber_String, result, 64, ref error);


this is expensive and only needs to be called once. you can check whether the SerialNumber is null and only then update it.

tebjan · 2018-10-11T14:00:52Z

sources/engine/Xenko.VirtualReality/OpenVR/OpenVR.cs

+                serialNumberStringBuilder.Clear();
+                Valve.VR.OpenVR.System.GetStringTrackedDeviceProperty((uint)TrackerIndex, ETrackedDeviceProperty.Prop_SerialNumber_String, serialNumberStringBuilder, StringBuilderSize, ref error);
+                if (error == ETrackedPropertyError.TrackedProp_Success)
+                    SerialNumber = serialNumberStringBuilder.ToString();


maybe we could check somehow whether the device slot has changed? that and create is the only moment when the serial number needs reading...

tebjan · 2018-10-13T15:56:35Z

sources/engine/Xenko.VirtualReality/OpenVR/OpenVR.cs

+                serialNumberStringBuilder.Clear();
+                Valve.VR.OpenVR.System.GetStringTrackedDeviceProperty((uint)TrackerIndex, ETrackedDeviceProperty.Prop_SerialNumber_String, serialNumberStringBuilder, StringBuilderSize, ref error);
+                if (error == ETrackedPropertyError.TrackedProp_Success)
+                    SerialNumber = serialNumberStringBuilder.ToString();


another idea would be to place the code into the getter of the property or a method of the TrackedDevice, then it would only be read and allocated if someone asks for it...

joreg · 2018-10-25T15:00:43Z

ouright, will move SerialNumber, DeviceClass and Batteries into getters.

…o getters

xen2 · 2018-10-26T04:47:37Z

Seems like there is some line ending issues in most edited files (I see the warning when opening https://github.com/vvvv/xenko/edit/VRTrackerSupport/sources/engine/Xenko.VirtualReality/OpenVR/OpenVR.cs)

your CRLF setting should be set to "auto" (core.autocrlf=true)

joreg · 2018-10-26T11:32:29Z

indeed, i've just moved to a new PC where this wasn't set correctly. i checked all files this PR modifies and only found an inconsistency in this one mentioned file though..

joreg · 2018-11-29T20:25:18Z

hei @xen2 do you see any further issues with this?

xen2 · 2019-01-18T03:07:42Z

@joreg sorry for the delay...
Let's merge this for 3.1.

xen2 · 2019-01-19T14:38:40Z

sources/engine/Xenko.VirtualReality/DeviceEnums.cs

+        Controller,         //The device is a controller
+        GenericTracker,     //The device is a tracker
+        TrackingReference,  //The device is a camera, Lighthouse base station, or other device that supplies tracking ground truth.
+        DisplayRedirect     //Accessories that aren't necessarily tracked themselves, but may redirect video output from other tracked devices


Can you put those comments following the /// <summary>My summary here.</summary> xml convention?
Since you bothered adding them, better make them visible to API users as well 😄

…l files and added xml comments

sources/engine/Xenko.VirtualReality/DeviceClass.cs

sources/engine/Xenko.VirtualReality/OpenVR/OpenVrTrackedDevice.cs

sources/engine/Xenko.VirtualReality/TrackedItem.cs

sources/engine/Xenko.VirtualReality/TrackingSpace.cs

xen2 · 2019-01-25T02:18:13Z

LGTM, merging
Thanks!

# Conflicts: # sources/engine/Xenko.VirtualReality/OpenVR/OpenVrTrackedDevice.cs # sources/engine/Xenko.VirtualReality/TrackedItem.cs

tebjan and others added 3 commits September 20, 2018 16:06

version bump to 3.0.0.6

e8a64ce

Merge branch 'master' of https://github.com/xenko3d/xenko

a784087

added TrackedDevices for OpenVR, other VR APIs return null for now

a7decbe

joreg mentioned this pull request Sep 24, 2018

support for vive trackers #136

Closed

Kryptos-FR reviewed Sep 24, 2018

View reviewed changes

joreg added 3 commits October 1, 2018 17:25

TrackedDevice collection properties now return empty array, rather th…

b60b013

…an null

renamed Xenko.VirtualReality.TrackedDevice to TrackedItem to not be c…

46a7602

…onfused with OpenVR.TrackedDevice

added BatteryPercentage to TrackedItem

8f0dfc8

removed double ToString call and moved serial number getter to constr…

2b5c591

…uctor since it doesn't change at runtime (cherry picked from commit b6a8f4912637b7f89b0017da07df3ae6efc53a57)

joreg added 2 commits October 10, 2018 19:57

moved SerialNumber getter back to update since it can change at runti…

6bbf2e9

…me for a given slot

added VRDevice.SetTrackingSpace

3d56559

reusing same StringBuilder in ever frame to get the SerialNumber

b4b9744

xen2 reviewed Oct 11, 2018

View reviewed changes

tebjan reviewed Oct 25, 2018

View reviewed changes

moved BatteryPercentage, SerialNumber and DeviceClass from update int…

dfe9c05

…o getters

fixed line-endings to LF

5ab34de

xen2 force-pushed the master branch from 88dcb74 to 1535057 Compare November 19, 2018 15:10

xen2 self-assigned this Jan 18, 2019

xen2 reviewed Jan 19, 2019

View reviewed changes

Split DeviceState, DeviceClass and TrackingSpace enums into individua…

f6ae750

…l files and added xml comments

xen2 reviewed Jan 19, 2019

View reviewed changes

sources/engine/Xenko.VirtualReality/DeviceClass.cs Outdated Show resolved Hide resolved

xen2 reviewed Jan 19, 2019

View reviewed changes

sources/engine/Xenko.VirtualReality/OpenVR/OpenVrTrackedDevice.cs Outdated Show resolved Hide resolved

xen2 reviewed Jan 19, 2019

View reviewed changes

sources/engine/Xenko.VirtualReality/TrackedItem.cs Outdated Show resolved Hide resolved

xen2 reviewed Jan 19, 2019

View reviewed changes

sources/engine/Xenko.VirtualReality/TrackingSpace.cs Outdated Show resolved Hide resolved

fixed copyright info

84d62ba

xen2 merged commit 7e41209 into stride3d:master Jan 25, 2019

xen2 added a commit that referenced this pull request Jan 28, 2019

[VR] Fix UWP build (#213)

bdc87cc

tebjan pushed a commit to vvvv/stride that referenced this pull request Mar 29, 2020

VR tracker support (stride3d#213)

6a207c8

# Conflicts: # sources/engine/Xenko.VirtualReality/OpenVR/OpenVrTrackedDevice.cs # sources/engine/Xenko.VirtualReality/TrackedItem.cs


		public override TouchController RightHand => null;

		public override TrackedDevice[] TrackedDevices => null;

Uh oh!

Conversation

joreg commented Sep 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Sep 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kryptos-FR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xen2 commented Oct 7, 2018

Uh oh!

joreg commented Oct 7, 2018

Uh oh!

tebjan commented Oct 7, 2018

Uh oh!

joreg commented Oct 10, 2018

Uh oh!

tebjan commented Oct 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joreg commented Oct 25, 2018

Uh oh!

xen2 commented Oct 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joreg commented Oct 26, 2018

Uh oh!

joreg commented Nov 29, 2018

Uh oh!

xen2 commented Jan 18, 2019

Uh oh!

xen2 Jan 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xen2 commented Jan 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

joreg commented Sep 24, 2018 •

edited

Loading

CLAassistant commented Sep 24, 2018 •

edited

Loading

xen2 commented Oct 26, 2018 •

edited

Loading

xen2 Jan 19, 2019 •

edited

Loading