Driver Scale Parameter #65

brian-armstrong · 2021-10-31T23:19:03Z

Per the conversation at #23 (comment), I thought I would take a crack at the scale parameter of the reference spaces being different.

What this does do:

Allows the driver to take the scale parameter over IPC
Applies it to the Pose
Serializes/deserializes the scale to/from stored config
Allows user to edit scale from editor

What this does not do:

Determine the scale via calibration

Bonus goals: gives me familiarity with the code base and confirm this feature is still desired.

cc @pushrax

Also stash to serialized config and allow editing from editor

brian-armstrong · 2021-10-31T23:22:58Z

Protocol.h

@@ -10,7 +10,7 @@

 namespace protocol
 {
-	const uint32_t Version = 1;
+	const uint32_t Version = 2;


Seemed like the cleanest way to handle this was to just break backwards compat on the IPC

I agree, that's fine. It would be possible to preserve backwards compatibility by making a new message, and using it only if both sides support version 2, but there isn't much of a reason to implement that since there's only one client (that I'm aware of) and it's installed at the same time as the driver.

pushrax

Looks good to me, nice work!

When I have a chance in the next couple days I'll merge/build/release.

pushrax · 2021-11-01T18:02:04Z

Protocol.h

@@ -10,7 +10,7 @@

 namespace protocol
 {
-	const uint32_t Version = 1;
+	const uint32_t Version = 2;


I agree, that's fine. It would be possible to preserve backwards compatibility by making a new message, and using it only if both sides support version 2, but there isn't much of a reason to implement that since there's only one client (that I'm aware of) and it's installed at the same time as the driver.

pushrax · 2021-11-01T18:04:05Z

OpenVR-SpaceCalibratorDriver/ServerTrackedDeviceProvider.cpp

-		pose.vecWorldFromDriverTranslation[2] = rotatedTranslation.v[2] + tf.translation.v[2];
+		pose.vecWorldFromDriverTranslation[0] = tf.scale * rotatedTranslation.v[0] + tf.translation.v[0];
+		pose.vecWorldFromDriverTranslation[1] = tf.scale * rotatedTranslation.v[1] + tf.translation.v[1];
+		pose.vecWorldFromDriverTranslation[2] = tf.scale * rotatedTranslation.v[2] + tf.translation.v[2];


I'm not sure if the scale should affect tf.translation or not. Keeping it this way is fine though, the client can decide to scale the configured translation or not.

I haven't had a chance to read how it actually solves for calibration, but I'd assumed the translation factor comes out as a constant offset/mean difference from the two spaces.

pushrax · 2021-11-01T18:40:52Z

Have you tried using this with a SLAM based tracking system yet? My plan is to just release it and let people experiment. If there's positive feedback, then spending more time to integrate automatic calibration will be more justified.

brian-armstrong · 2021-11-01T20:54:05Z

Thanks for the quick review! I want to actually keep testing this out some more locally. The part I'm most unsure about is the actual application of the scale factor. I'm still trying to digest the linear algebra here :) Since that's the part that really matters I want to be sure it's doing the right thing, and that it fixes the drift I've seen locally.

My setup is a HP Reverb G2 headset with Index controllers. I think G2 is SLAM but I admittedly don't know the specifics. It is inside out with cameras though.

pushrax · 2021-11-01T21:32:14Z

G2 uses SLAM.

I haven't looked at this in a while, so I'm not too sure on the right place to apply the scale factor either. It could be where you put it, though there's a good chance it needs to be applied to vr::DriverPose_t::vecPosition instead.

OpenVR-SpaceCalibrator/lib/openvr/openvr_driver.h

Lines 2526 to 2532 in f3b4f94

    
           	/* State of driver pose, in meters and radians. */ 
        
           	/* Position of the driver tracking reference in driver world space 
        
           	* +[0] (x) is right 
        
           	* +[1] (y) is up 
        
           	* -[2] (z) is forward 
        
           	*/ 
        
           	double vecPosition[ 3 ];

If I remember correctly, the final pose in VR world space is calculated as:

worldPose = worldFromDriverTransform * driverPose * driverFromHeadTransform

such that each transform/pose is a 4x4 matrix representing an affine transformation formed from the rotation and translation inputs in DriverPose_t.

If you scale vecWorldFromDriverTranslation, it won't scale the underlying device's position in world space, it will instead offset it in world space by some proportion of the original vecWorldFromDriverTranslation.

It depends on exactly what the issue is in the first place, but if the issue is that the entire space is actually smaller or larger, applying the scale to vecPosition directly would make more sense to me. Testing should make it clear.

brian-armstrong · 2021-11-01T22:23:42Z

Ok interesting, I wouldn't have thought to have applied it to vecPosition since the code otherwise doesn't touch it. I was considering instead applying it to the components of vecWorldFromDriverTranslation before the transforms are applied to it but I haven't worked through whether that's just equivalent to the scaling I applied anyway. I'll see if I can actually understand the parts of DriverPose_t and what math is being done here, or at least test things empirically more 😄

The correct value is probably between 0.99 and 1.01. Let's use really small steps here.

This looks like the right thing to scale

brian-armstrong · 2021-11-02T07:51:07Z

You were correct @pushrax , scaling vecPosition was the way to go. The way that I tested this was I turned on a controller from the G2 and held it against an Index controller. When I moved them around the room previously, they would appear to move further apart in SteamVR even though they were still the same actual distance apart. When I change the scale factor to 0.9924, they no longer separate as I move around the room.

brian-armstrong · 2021-11-07T23:32:15Z

Hi @pushrax

I'm sure you are quite busy. This is a gentle request to re-review the PR (if necessary) and merge/build. I'd love to find out if this patch helps others with the similar setup and issue.

jgr526 · 2021-11-08T00:25:54Z

I'm interested in trying it out whenever it's part of a release. I made some videos in the past demonstrating the drift with the Reverb G2. Excited to give this feature a try when it's available.

…

On Sun, Nov 7, 2021, 6:32 PM Brian Armstrong ***@***.***> wrote: Hi @pushrax <https://github.com/pushrax> I'm sure you are quite busy. This is a gentle request to re-review the PR (if necessary) and merge/build. I'd love to find out if this patch helps others with the similar setup and issue. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#65 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFC3HYTHNYXZXUG2YCTZBOTUK4EAVANCNFSM5HC2JGFA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

pushrax · 2021-11-08T01:01:52Z

Thanks for the reminder. I should be able to get to this tonight.

pushrax · 2021-11-08T05:46:11Z

Installer for v1.3, including this PR, is available here https://github.com/pushrax/OpenVR-SpaceCalibrator/releases

Driver Scale Parameter

Driver Scale Parameter

bd50a8c

Also stash to serialized config and allow editing from editor

brian-armstrong commented Oct 31, 2021

View reviewed changes

pushrax approved these changes Nov 1, 2021

View reviewed changes

brian-armstrong added 2 commits November 2, 2021 00:46

make the steps smaller

e599ac1

The correct value is probably between 0.99 and 1.01. Let's use really small steps here.

scale vecPosition

c6eb49d

This looks like the right thing to scale

pushrax merged commit 984b93f into pushrax:master Nov 8, 2021

Natsumi-sama pushed a commit to Natsumi-sama/OpenVR-SpaceCalibrator that referenced this pull request Feb 6, 2022

Merge pull request pushrax#65 from brian-armstrong/scale_config

ae2a3aa

Driver Scale Parameter

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Driver Scale Parameter #65

Driver Scale Parameter #65

brian-armstrong commented Oct 31, 2021 •

edited

Loading

brian-armstrong Oct 31, 2021

pushrax Nov 1, 2021

pushrax left a comment

pushrax Nov 1, 2021

pushrax Nov 1, 2021

brian-armstrong Nov 1, 2021

pushrax commented Nov 1, 2021

brian-armstrong commented Nov 1, 2021

pushrax commented Nov 1, 2021 •

edited

Loading

brian-armstrong commented Nov 1, 2021

brian-armstrong commented Nov 2, 2021

brian-armstrong commented Nov 7, 2021

jgr526 commented Nov 8, 2021 via email

pushrax commented Nov 8, 2021

pushrax commented Nov 8, 2021

Driver Scale Parameter #65

Driver Scale Parameter #65

Conversation

brian-armstrong commented Oct 31, 2021 • edited Loading

brian-armstrong Oct 31, 2021

Choose a reason for hiding this comment

pushrax Nov 1, 2021

Choose a reason for hiding this comment

pushrax left a comment

Choose a reason for hiding this comment

pushrax Nov 1, 2021

Choose a reason for hiding this comment

pushrax Nov 1, 2021

Choose a reason for hiding this comment

brian-armstrong Nov 1, 2021

Choose a reason for hiding this comment

pushrax commented Nov 1, 2021

brian-armstrong commented Nov 1, 2021

pushrax commented Nov 1, 2021 • edited Loading

brian-armstrong commented Nov 1, 2021

brian-armstrong commented Nov 2, 2021

brian-armstrong commented Nov 7, 2021

jgr526 commented Nov 8, 2021 via email

pushrax commented Nov 8, 2021

pushrax commented Nov 8, 2021

brian-armstrong commented Oct 31, 2021 •

edited

Loading

pushrax commented Nov 1, 2021 •

edited

Loading