-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add height reference (clamp-to-ground) to ModelExperimental
#10448
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
Conversation
Thanks for the pull request @j9liu!
Reviewers, don't forget to make sure that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@j9liu did a pass through the code, looks sensible though I had an idea for refactoring ModelExperimental.update()
.
I still need to try out the code in Sandcastle, I'll do that next
tried various sandcastles, seems to be working nicely! |
@ptrgags updated! |
@j9liu the update function looks a lot nicer now!, I'll merge once CI passes |
🥇 on the |
This PR adds a
heightReference
option toModelExperimental
to allow for clamp-to-ground behavior. Most of the code is taken fromModel
, thoughModelExperimental.update
had to be reorganized so things could be updated in the right order (model matrix, bounding spheres, reference matrices).Sandcastle for
ModelExperimental
, and a sandcastle forModel
for comparison in behavior.I tried to add rendering tests for this but it was difficult to because of the globe. Even if the
Model
is positioned very high, the globe still peeks over the horizon. Then,globe.show
to false disables the clamp-to-ground behavior, so I can't prevent it from rendering during the unit test.