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

ViewCoordinates documentation is wrong or at least misleading #7028

Closed
rafaelspring opened this issue Jul 31, 2024 · 4 comments · Fixed by #7034
Closed

ViewCoordinates documentation is wrong or at least misleading #7028

rafaelspring opened this issue Jul 31, 2024 · 4 comments · Fixed by #7034
Assignees
Labels
🪳 bug Something isn't working 📖 documentation Improvements or additions to documentation 🏎️ Quick Issue Can be fixed in a few hours or less

Comments

@rafaelspring
Copy link

rafaelspring commented Jul 31, 2024

Describe the bug
https://rerun.io/docs/concepts/spaces-and-transforms#view-coordinates

Quote:

For 3D spaces it can be used to log what the up-axis is in your coordinate system. This will help Rerun set a good default view of your 3D scene, as well as make the virtual eye interactions more natural. This can be done with rr.log("world", rr.ViewCoordinates(up="+Z"), static=True).

I followed this and (using C++) I put this in my code
rec.log_static("world", rerun::ViewCoordinates::RIGHT_HAND_Y_DOWN);

But the view camera in the viewer is still gimbal-locking, basically same as without the above line.

To Reproduce
Follow steps in the quote.

Expected behavior
View camera up axis actually changing to what's been set.

To Fix
The path needs to be "/" instead of "world".
Maybe there's some bigger context that I'm missing but that camera gimbal-lock made the (otherwise great) viewer immediately unusable for me. I needed a solution and rr.log("world", rr.ViewCoordinates(up="+Z"), static=True) is what I found and it didn't help.

@rafaelspring rafaelspring added 👀 needs triage This issue needs to be triaged by the Rerun team 🪳 bug Something isn't working labels Jul 31, 2024
@Wumpf
Copy link
Member

Wumpf commented Jul 31, 2024

Thanks for filing this! world is just an example for a potential origin of the view and nothing special. I agree that this is written in a way that is very easy to understand. So really it should be something like your/view/origin and explain what this is all about.

The "lock" your experiencing on the camera btw. is not actual gimbal lock which would be if a degree of freedom is lost because axis are aligning. What's happening instead is that our camera maintains an abstract up vector (which is influenced by ViewCoordinates) that stays constant. Once the camera is tilted such that that its actual up vector coincides with the abstract notion of the world's up we don't allow further rotation. It's essentially preventing the camera to be up-side down.
Agreed though that we should probably also come up with something different here.

Related to:

@Wumpf Wumpf added 📖 documentation Improvements or additions to documentation 🏎️ Quick Issue Can be fixed in a few hours or less and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels Jul 31, 2024
@rafaelspring
Copy link
Author

I used the term "gimbal lock" loosely here but I think we all know what is meant by that.
But TBF you actually do lose a motion DOF when you align with the up-axis.

I assume you're parameterizing your camera view by a pivot point, distance and 2-dof phi and theta?
You don't have to btw.
It's totally possible to have camera that's easy to navigate and doesn't suffer from gimbal lock.
The model for that is to treat your scene as a "ball" and then any drag operation will project the clicked points down to the ball and bring the start and current point into alignment by rotating the ball. We use this model in our Dot3D app and all our users tell us it's the most natural camera controls they know.

@rafaelspring
Copy link
Author

Also, w.r.t my original point:
I think it'd be great if the solution proposed in the docs helps Rerun.io beginners like me. I found the "/" solution kind of by accident and was almost at a point where I gave up on Rerun.io, despite it being great otherwise. The camera locking is definitely an issue, and as long as it is, the solution shouldn't be far.

@Wumpf
Copy link
Member

Wumpf commented Jul 31, 2024

We use this model in our Dot3D app and all our users tell us it's the most natural camera controls they know.

There's plenty of suggestions floating around in various tickets, many attached with strong opinions on what's the best solution, typically implicitly tied to some particular field. I believe there's some universal improvements to be made (kicking out the constrained orbit camera may be one of them, but maybe also not as it's very popular in some fields :/), but for the most part this is a lack-of-easy-configurability story. Some of the associated tickets are linked here.

That said, the input is appreciated, thank you! :)

@Wumpf Wumpf self-assigned this Aug 1, 2024
Wumpf added a commit that referenced this issue Aug 1, 2024
### What

* Fixes #7028

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7034?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7034?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7034)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working 📖 documentation Improvements or additions to documentation 🏎️ Quick Issue Can be fixed in a few hours or less
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants