-
Notifications
You must be signed in to change notification settings - Fork 368
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
Make Transform3D always log all its components #6909
Comments
This will add noise to the UI, and also add some logging CPU overhead, but I suggest we wait with fixing both those problems until we tackle all of #3381 |
An alternative approach to this is to do a special query when querying transforms. It would go something like this:
This would means each new log call of any of these transforms would effectively reset the other ones. The advantage of this idea is that we don't have to do the work in this PR (always log all components to reset any previously logged one). However, it is a special snow-flake querying model, just for We also want to support logging transform components without using the rr.log("planet", rr.Position3D(…));
rr.log("planet", rr.Colors(…));
rr.log("planet", rr.Translation3D(…));
rr.log("planet", rr.Rotation3D(…)); It would be surprising to the user if the translation was ignored in the viewer. There are more advanced queries that could solve this, e.g. "if the transform components are non-overlapping (like translation and rotation), then still use both…" but this now becomes a complex decision tree that again needs to be replicated and explained to users. In contrast, the approach argued by this PR:
|
### What * Closes #6909 * First steps on the road to #3381 * Creates a new issue: #7117 Best reviewed ignoring whitespace changes. The serializers for `Transform3D` and `LeafTransform3D` will now always write every component, effectively clearing all previously logged values. `Transform3D` uses `Option<C>` for each component, treating it as a single-element list component list where `None` is empty. If we ever want to support updating some components using the transform (as outlined in #3381) we would need to turn this into a tristate, i.e. something like `NoneOrZeroOrOne<C>`. `LeafTransform3D` uses `Option<Vec<C>>` for its components, but has no interface yet for logging an update. Both `Transform3D` and `LeafTransform3D` has a new `::clear()` constructor in Rust, that replaced both `::new()` and `::default()`. This is for added clarity, and to make the transition easier if we ever add an `::update()` constructor, as per #3381 ![image](https://github.com/user-attachments/assets/588224b8-daec-463d-b238-8263f578ae19) ### 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/7111?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/7111?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/7111) - [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`. --------- Co-authored-by: Andreas Reich <[email protected]>
Why is this important? Example:
The text was updated successfully, but these errors were encountered: