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

Use LeafTranslation (centers), LeafRotationQuat and LeafRotationAxisAngle directly on Boxes3D/Ellipsoids3D #7029

Merged
merged 8 commits into from
Aug 2, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jul 31, 2024

What

Makes Boxes3D & Ellipsoids3D handle multiple leaf transforms correctly, transferring all transformation concerns to the TransformContext.

This is in line with where we want to go as part of #6831 (mostly because it eliminates the need for the Rotation3D union), but there's some drawbacks with this PR:

  • with_rotations / rotations= is no longer there
    • we could backfill this with some restrictions
  • boxes used to use Position3D, data that did so breaks now

Pictures of some examples & snippets that are still working correctly:
image
image

Enabled box visualizer on this one to ensure the sizes align.
image

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@Wumpf Wumpf added include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Jul 31, 2024
Copy link

github-actions bot commented Jul 31, 2024

Deployed docs

Commit Link
80a5892 https://landing-qye6bv7b5-rerun.vercel.app/docs

@Wumpf Wumpf added the 🍏 primitives Relating to Rerun primitives label Jul 31, 2024
@Wumpf Wumpf added the do-not-merge Do not merge this PR label Jul 31, 2024
@Wumpf
Copy link
Member Author

Wumpf commented Aug 1, 2024

@rerun-bot full-check

Copy link

github-actions bot commented Aug 1, 2024

@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Aug 2, 2024
@teh-cmc
Copy link
Member

teh-cmc commented Aug 2, 2024

Merging this so we can unpile PRs -- I'm sure we'll have plenty more time to talk about rotation APIs before 0.18.

@Wumpf Wumpf merged commit 848837a into main Aug 2, 2024
73 of 75 checks passed
@Wumpf Wumpf deleted the andreas/leaftransform-boxes-ellipsoids branch August 2, 2024 08:03
Wumpf added a commit that referenced this pull request Aug 2, 2024
### What

* Part of  #6831
* Builds on top of #7029

Porting considerations handled by previous PRs

### 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/7030?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/7030?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/7030)
- [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
include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages 🍏 primitives Relating to Rerun primitives
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Boxes3DVisualizer does not account for rotations when computing bounding boxes
2 participants