New LeafTransform3D, replacing OutOfTreeTransform3D
#7015
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…asset3d more often in the scene if there's several
Wumpf
added
🍏 primitives
Deployed docs
|
6 tasks
Wumpf
force-pushed
the
andreas/introduce-leaf-transform3d
branch
from
7c89204 to
5baf54a
Compare
This was referenced Jul 30, 2024
Merged
|
@rerun-bot full-check |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/10163375918 |
|
@rerun-bot full-check |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/10179322264 |
jleibs
approved these changes
Jul 31, 2024
crates/store/re_types/definitions/rerun/archetypes/leaf_transforms3d.fbs
Outdated
Show resolved
Hide resolved
| /// From the point of view of the entity's coordinate system, | ||
| /// all components are applied in the inverse order they are listed here. | ||
| /// E.g. if both a translation and a max3x3 transform are present, | ||
| /// the 3x3 matrix is applied first, followed by the translation. |
There was a problem hiding this comment.
Would be nice to see this one drawn out in a diagram some day.
crates/viewer/re_space_view_spatial/src/visualizers/assets3d.rs
Outdated
Show resolved
Hide resolved
Comment on lines
-196
to
-202
| if spatial_ctx.space_view_class_identifier == SpatialSpaceView2D::identifier() { | ||
| self.data.add_bounding_box( | ||
| entity_path.hash(), | ||
| bounding_box_for_textured_rect(&textured_rect), | ||
| spatial_ctx.world_from_entity, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Is this just no longer necessary?
There was a problem hiding this comment.
It still is, but moved inside of textured_rect_from_tensor
Wumpf
added a commit
that referenced
this pull request
Jul 31, 2024
…, remove `Transform3D` component (#7000) ### What * build on top of #7015 Removes * Scale3D datatypes * Transform3D datatype * TranslationRotationScale3D datatype * Transform3D component compatibility considerations were already done all in previous PRs. For some of these there's utilities to make actual use similar to before. ### 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/7000?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/7000?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/7000) - [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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
OutOfTreeTransform3D(encapsulating full transform) withOutOfTreeTransform(a bool component) #6988Introduces a new
LeafTransform3Darchetype that is always applicable. It entails a copy of all ofTransform3D's components - axis length and transform relation have been omitted so far.Surprisingly, I didn't have much need for the extensive extensions we have on
Transform3Dso far: Leaf transform is much less commonly used and deals with arrays, making it sufficiently different fromTransform3D. Also a lot of the extensions associated withTransform3Dare there for legacy reasons - with the new more componetized interface we get much more reasonable ergonomics out of the box!This PR entails a major overhaul of the
TransformContext. For sure not the last time we do this (looking at you 2D transform handling & not-so-great 2D<->3D interactions!), but the intention is to be a bit more forward looking and to enforce use of leaf transforms everywhere.Single component leaf transforms are supported everywhere now. Multi component leaf transforms logs a warning for all visualizers except Mesh3D and Asset3D where it bottoms out in instantiating the mesh multiple times:
Screen.Recording.2024-07-30.at.11.10.57.mov
Snippet demonstrating combination of
Transform3DwithLeafTransforms3D:Screen.Recording.2024-07-30.at.12.15.53.mov
Follow-up PRs will improve the interaction of various archetypes with
LeafTransforms3Das well as remove now unused legacy types.Checklist
mainbuild: rerun.io/viewernightlybuild: rerun.io/viewerCHANGELOG.mdand the migration guideTo run all checks from
main, comment on the PR with@rerun-bot full-check.