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

bugfix(server_core) Fix judder regression caused by streamer-client pose desync #2660

Merged
merged 8 commits into from
Jan 30, 2025

Conversation

shinyquagsire23
Copy link
Contributor

@shinyquagsire23 shinyquagsire23 commented Jan 25, 2025

Found the actual fix lol, seems to make hands way less sloshy as well?

@shinyquagsire23 shinyquagsire23 requested a review from zmerp January 25, 2025 00:13
@shinyquagsire23 shinyquagsire23 changed the title (bugfix) Fix judder regression caused by streamer-client pose desync bugfix(server_core) Fix judder regression caused by streamer-client pose desync Jan 25, 2025
@shinyquagsire23
Copy link
Contributor Author

shinyquagsire23 commented Jan 25, 2025

Random footnote I just realized: We're comparing distances between timestamps here, but the client sends predicted target timestamps rather than poll timestamps, so a spike that's 20ms over the average might mess up this function (ie, the timestamps in the queue might look something like [10 20 30 55 50 60])

(It might also accidentally use a poor-quality projected pose rather than a more recent one)

@zmerp
Copy link
Member

zmerp commented Jan 25, 2025

@shinyquagsire23 What you're saying should normally not happen at default statistics history size, but it more probable if it is lowered

alvr/server_core/src/tracking/mod.rs Outdated Show resolved Hide resolved
@shinyquagsire23
Copy link
Contributor Author

bumping bc we should probably get this merged so it at least hits nightlies

@shinyquagsire23
Copy link
Contributor Author

bump 2

Copy link
Collaborator

@The-personified-devil The-personified-devil left a comment

Choose a reason for hiding this comment

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

LGTM from an implementation side, if you already talked the nature of the changes over with @zmerp, then this can get merged

@zmerp
Copy link
Member

zmerp commented Jan 29, 2025

I will review as soon as i can, tomorrow the latest.

Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

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

LGTM

@zmerp zmerp merged commit 9fe0162 into alvr-org:master Jan 30, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants