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.

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

Spine Yaw Compensation to keep spine trackers from drifting #1243

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jabberrock
Copy link

@jabberrock jabberrock commented Nov 16, 2024

Prevents the spine trackers from drifting, by nudging them to stay aligned to the headset.

Players are usually standing facing forwards or walking forwards, where the spine trackers are aligned to the yaw of the headset. However due to yaw drift, the spine trackers will eventually rotate to the side, requiring a reset. This feature adds a small centering rotation to compensate for the yaw drift. (For now, this compensation only activates when your trackers are pointing "up", e.g. when you're standing or sitting straight up.)

The correction amount should be approximately the maximum yaw drift of your gyroscope (0.3 degrees/sec is a good default).

image

@github-actions github-actions bot added Area: Skeletal Model Deals with the model of the skeleton and its pose Area: Application Protocol Related to communication with apps like the GUI, overlay, games Area: Translation Improvements or additions to translations Area: Server Related to the server labels Nov 16, 2024
@jabberrock jabberrock force-pushed the jabber-spine-yaw-compensation branch from 8cb2d68 to 99c15ec Compare November 16, 2024 20:17
@Eirenliel
Copy link
Member

Please rebase SolarXR, it was merged.

@Eirenliel
Copy link
Member

I think this can be enabled for legs too since it only works when standing up! Ideally, the setting would be part of Drift compensation setting that's now with 3 selectors: Off, Static/Simple (Old one), Dynamic/Advanced (this one). Then for this to have a setting of compensation speed, maybe a slider in 0.05deg/min intervals, and a selector for which parts to apply it: Upper body, Legs, Arms, Feet. There also needs to be an infomation that Dynamic only works with HMD and Static can work without it.

But this can be merged after review already, and settings can be added in the future. Very excited to RC this!

@jabberrock jabberrock force-pushed the jabber-spine-yaw-compensation branch from 99c15ec to 4d632d3 Compare November 16, 2024 21:35
@jabberrock
Copy link
Author

jabberrock commented Nov 16, 2024

@Eirenliel thanks taking a look! Definitely can look into putting this into the drift compensation setting later. (Or do you think I should just update it now?)

If this works well, I was going to add alignment between upper leg and lower leg. Maybe even foot?

But I'm not sure if upper leg can be aligned to the spine. For example, when I stand straight, I notice that my legs are twisted slightly away from my body. I was thinking maybe "balancing" the left and right legs so that they're at the same angle away from the spine...

@Eirenliel
Copy link
Member

If this works well, I was going to add alignment between upper leg and lower leg. Maybe even foot?

Upper and lower leg should have the same yaw, so they're already averaged.

But I'm not sure if upper leg can be aligned to the spine. For example, when I stand straight, I notice that my legs are twisted slightly away from my body. I was thinking maybe "balancing" the left and right legs so that they're at the same angle away from the spine...

It shouldn't matter if you reset the same way too. Also since it's a setting people can turn it off if it doesn't work for them and we can improve it in the future.

(Or do you think I should just update it now?)

If you want, we're not merging util Monday anyway.

Also pelase fix build errors (lint)

@Eirenliel
Copy link
Member

This needs to be Beta'd before it can be merged.

@jabberrock
Copy link
Author

@Eirenliel How do I get the build system to rebuild, and how does the beta process work? Sorry, it's my first PR. Thanks!

@Erimelowo
Copy link
Member

Since you're a first-time contributor, someone else needs to add their approval for the CI to run.

@jabberrock
Copy link
Author

Sigh oops I didn't realize there was a Java linter too

@Eirenliel
Copy link
Member

For Beta you need to create a post in beta-testing-forum on Discord! If you can't, ask some contributors

@jabberrock
Copy link
Author

@ButterscotchV , I remember you said I should move the Tracker.yawCorrectionInRad into TrackerResetHandler right?

@ButterscotchV
Copy link
Member

@ButterscotchV , I remember you said I should move the Tracker.yawCorrectionInRad into TrackerResetHandler right?

Probably should go there, yeah. The definitions of these things are kinda mangle together, but most of the drift corrective type stuff is around there.
Also maybe some of the code from this PR could be moved out of the HumanSkeleton class, as it's largely unrelated besides using the trackers from there?

@jabberrock
Copy link
Author

I realized in SolarXR I named it yawCorrection, so I'm going to rename this whole feature "Spine Yaw Correction" for consistency. I updated the names to match.

@jabberrock jabberrock force-pushed the jabber-spine-yaw-compensation branch from ec55b86 to 7f7da7e Compare November 25, 2024 23:10
@github-actions github-actions bot added the Area: GUI Related to the GUI label Nov 25, 2024
@jabberrock
Copy link
Author

Additional changes:

  1. Extracted algorithm into SpineYawCorrection class
  2. Moved applying the yaw correction rotation into TrackerResetsHandler
  3. Renamed all references of "compensation" to "correction"

@jabberrock
Copy link
Author

Freshly tested:

  1. When not enabled, no yaw correction happens
  2. When enabled, and chest & hip trackers are pointing forward, yaw correction happens
  3. When chest tracker is pointing forward, but hip tracker is pointing up, no yaw correction happens

@jabberrock jabberrock force-pushed the jabber-spine-yaw-compensation branch from c4d131b to 69ac737 Compare November 26, 2024 00:39
@jabberrock
Copy link
Author

@Erimelowo , could you take another look? Thanks!

@Eirenliel, it looks like the general sentiment of the beta is positive. Most people have been testing v2 and v3, which is a more sophisticated version of this change. Would it be reasonable to merge this?

Copy link
Member

@ButterscotchV ButterscotchV left a comment

Choose a reason for hiding this comment

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

I've tested with the recommended changes and it seems to work as intended. I had weird results from AutoBone, but I still don't really know whether it's this PR causing it.

}

private fun updateTrackers(trackers: List<Tracker>) {
for ((parentTracker, tracker) in trackers.zipWithNext()) {
Copy link
Member

Choose a reason for hiding this comment

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

zipWithNext() initializes a list every time it's called, this should probably just be moved into the initialization of upperBodyTrackers.

}

companion object {
private fun filterImuTrackers(vararg trackers: Tracker?): List<Tracker> = trackers.filterNotNull().filter { it.isImu() }.toList()
Copy link
Member

Choose a reason for hiding this comment

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

Non-IMU trackers should not be filtered, they should be used for reference but not adjusted.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, this probably shouldn't filter null trackers either? I'm not sure you would want to correct a hip tracker with the head tracker when every other tracker is missing. If this is intended behaviour then that is fine.

Comment on lines +196 to +199
const degreeFormat = new Intl.NumberFormat(currentLocales, {
style: 'unit',
unit: 'degree',
});
Copy link
Member

Choose a reason for hiding this comment

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

We should maybe use our translation framework? Idk about this

Copy link
Member

Choose a reason for hiding this comment

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

@ImUrX Can I get your feedback on this 🐱

Copy link
Member

Choose a reason for hiding this comment

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

its fine, i discovered a better integrated way on Fluent recently but for now this is how we been doing it, and if its per second you can do degree-per-second for the unit

<NumberSelector
control={control}
name="yawCorrectionSettings.amountInDegPerSec"
valueLabelFormat={(value) => degreeFormat.format(value)}
Copy link
Member

Choose a reason for hiding this comment

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

This label should say in degrees per second, not just degrees. It is confusing as it is.

Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Application Protocol Related to communication with apps like the GUI, overlay, games Area: GUI Related to the GUI Area: Server Related to the server Area: Skeletal Model Deals with the model of the skeleton and its pose Area: Translation Improvements or additions to translations Type: Enhancement Adds or improves a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants