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

Avatar auto scale: match avatar eye height to user eye height. #942

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Avatar auto scale: match avatar eye height to user eye height. #942

wants to merge 5 commits into from

Conversation

Phil-Palmer
Copy link
Contributor

@Phil-Palmer Phil-Palmer commented Dec 28, 2020 •
edited
Loading

This PR adds a 'Real Height' button to the avatar scale slider. The button scales the avatar so that its eye height matches the user's real-world eye height (derived from the 'User real world height' setting). In this way, the world is displayed at 1:1 scale to the user in VR (fixes #939).

Tests done

  • In VR, kneel to position your head as if you were 1m tall.
  • Set 'User real-world height' to 1m (in Settings > Controls).
  • Open the Avatar app and choose the default avatar, or one with a height similar to the default avatar.
  • Click on the sliders icon at the top-right.
  • Click the '1x' scale button and observe that the world is miniaturised because the avatar is bigger than the user.
  • Click the 'Real Height' scale button and observe that the world is displayed at actual size (1:1 scale) because the avatar scale is matched to the user's height.

Screenshots

image

@Phil-Palmer
Avatar auto scale: match avatar eye height to user eye height.
a391489 
This adds an 'Auto' button to the avatar scale slider.  The Auto button scales the avatar so that its eye height matches the user's real-world eye height (based on the 'User real world height' setting).  In this way, the world is displayed at exactly 100% scale to the user in VR.

The '1x' button now sets the avatar scale to 1.0 even if the current scale is at the same 'notch' as 1.0 on the scale slider (previously it had no effect in that situation).

Test done:
- In VR, kneel to position your head as if you were 1m tall.
- Set 'User real-world height' to 1m (in Settings > Controls).
- Open the Avatar app and click on the sliders icon at the top-right.
- Click the '1x' button and observe that the world is miniaturised.
- Click the 'Auto' button and observe that the world is displayed at actual size.
@digisomni digisomni added allow-build-upload Allows the upload of a build for non white-listed users enhancement New feature or request labels Dec 28, 2020
@digisomni digisomni added this to the 2021.1.0 Release milestone Dec 28, 2020
@vircadia-build-notifier
Copy link

The following links are available:
build (macOS-latest, full)

build (ubuntu-18.04, full)

build (ubuntu-18.04, android)

build (windows-latest, full)

@ctrlaltdavid ctrlaltdavid added needs CR (code review) needs testing (QA) The PR is ready for testing scripting api change Change is made in the scripting API labels Dec 29, 2020
ctrlaltdavid
ctrlaltdavid requested changes Dec 29, 2020
View reviewed changes
interface/src/avatar/MyAvatar.cpp Outdated Show resolved Hide resolved
interface/src/avatar/MyAvatar.cpp Outdated Show resolved Hide resolved
interface/src/avatar/MyAvatar.h Outdated Show resolved Hide resolved
interface/src/avatar/MyAvatar.h Outdated Show resolved Hide resolved
@Phil-Palmer @ctrlaltdavid
Apply suggestions from code review
63c3121 
Improve comments and fix bracing style.

Co-authored-by: David Rowe <[email protected]>
@vircadia-build-notifier
Copy link

The following links are available:
build (ubuntu-18.04, full)

build (macOS-latest, full)

build (windows-latest, full)

build (ubuntu-18.04, android)

@Phil-Palmer
Copy link
Contributor Author

Thanks for the changes @ctrlaltdavid - I committed them.

I think the button label "Auto" would be unclear. What would you think of this instead?

image

@Phil-Palmer Phil-Palmer mentioned this pull request Dec 29, 2020
Open
@JulianGro
Copy link
Contributor

Personally I think "Real Height" would be more descriptive. Not sure if there is space for that though. Maybe putting the "1x" on the right would make enough space?

@ctrlaltdavid ctrlaltdavid added CR Approved At least one code reviewer has approved the PR. and removed needs CR (code review) labels Dec 29, 2020
@ctrlaltdavid
Copy link
Collaborator

Julian's suggestion sounds good to me.

@Phil-Palmer
Copy link
Contributor Author

Phil-Palmer commented Dec 29, 2020 •
edited
Loading

Thanks, I renamed the button "Real Height" in e89e80d :

image

@vircadia-build-notifier
Copy link

The following links are available:
build (ubuntu-18.04, full)

build (macOS-latest, full)

build (windows-latest, full)

build (ubuntu-18.04, android)

@Phil-Palmer
Copy link
Contributor Author

Phil-Palmer commented Jan 5, 2021 •
edited
Loading

@ctrlaltdavid, @JulianGro
I'm thinking it would be neater to postpone this feature until I've fixed another avatar scale problem:

If I choose an avatar taller than [the default avatar?], it squats to my eye level instead of standing straight. The workaround is to reduce my real-world height a little.
Presumably if I chose an avatar shorter than [the default avatar?], it would hang from my head or pull my viewpoint closer to the floor, depending on my recentre setting.

Originally posted by @Phil-Palmer in #939 (comment)

It should be easier to communicate the combined result of the two fixes together, rather than releasing them separately.
Does that sound good to you? If so, what should I do with this PR? Mark it as a draft / 'do not merge'?
Thanks!

@digisomni
Copy link
Member

Marking it as draft will be fine for the moment. :)

@Phil-Palmer Phil-Palmer marked this pull request as draft January 7, 2021 22:40
@digisomni digisomni removed this from the 2021.1.0 Release milestone Jan 27, 2021
@digisomni digisomni added this to the 2021.1.1 Release milestone Jan 27, 2021
@ctrlaltdavid ctrlaltdavid removed needs testing (QA) The PR is ready for testing CR Approved At least one code reviewer has approved the PR. labels Feb 28, 2021
@digisomni digisomni added the dormant This PR is on hold but has interest/use surrounding it. label Nov 6, 2021
@Penguin-Guru
Copy link

This seems like a good feature. About the button text-- I think "real height" is fine but what about "user height"? The user is the actual point of reference, although technically it's the eye height and not the user/avatar height that's synced.

@stale
Copy link

stale bot commented May 21, 2022

Hello! Is this still an issue?

@stale stale bot added the stale Issue / PR has not had activity label May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ctrlaltdavid ctrlaltdavid ctrlaltdavid approved these changes

Assignees
No one assigned
Labels
allow-build-upload Allows the upload of a build for non white-listed users dormant This PR is on hold but has interest/use surrounding it. enhancement New feature or request scripting api change Change is made in the scripting API stale Issue / PR has not had activity
Projects
None yet
Milestone
2022.1.1 Selene Release
Development

Successfully merging this pull request may close these issues.

World scale is unnatural unless user height equals avatar height
6 participants
@Phil-Palmer @vircadia-build-notifier @JulianGro @ctrlaltdavid @digisomni @Penguin-Guru