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

Split Transform3D component union type into components #6831

Closed
7 of 9 tasks
Wumpf opened this issue Jul 9, 2024 · 1 comment
Closed
7 of 9 tasks

Split Transform3D component union type into components #6831

Wumpf opened this issue Jul 9, 2024 · 1 comment
Assignees
Labels

Comments

@Wumpf
Copy link
Member

Wumpf commented Jul 9, 2024

Remaining todo items:


// Two possibilities:
// - Only legal to set one of them
// - Or apply them all in deterministic order
archetype Transform {
    mat4: Option<Mat4>,
    translation: Option<Translation3>,
    mat3: Option<Mat3>,
    rotation: Option<Rotation3D>,
    scale3: Option<Scale3D>,
    scale: Option<Scale>,
}

Old proposal

How to handle OutOfTreeTransform:

Introduce a new boolean component that coerces a transform batch to be out of tree. OutOfTreeTransform { enabled: bool }
if one encounters several several parent transforms and if they have the OutOfTreeTransform == false the viewer issues a warning and resolves in a best effort manner

To consider: The fallback provider for OutOfTreeTransform can be clever and look at transform counts, automatically assuming true if there's several transforms.

Drawback: Can no longer have out of tree transform side by side with in-tree transforms

@Wumpf Wumpf added 🏹 arrow concerning arrow codegen/idl labels Jul 9, 2024
@Wumpf Wumpf self-assigned this Jul 9, 2024
Wumpf added a commit that referenced this issue Jul 11, 2024
…6851)

### What

* Part of testing for #6831
* Part of #3210


![image](https://github.com/rerun-io/rerun/assets/1220815/51c07c54-515d-4667-8a55-27fe889c4d19)


### 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/6851?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/6851?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/6851)
- [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`.
Wumpf added a commit that referenced this issue Jul 17, 2024
### What

* Part of #6831

Previously, `Angle` was an arrow union between radian & degree. As part
of the effort of simplifying our data representations, this is now
always just radians.
Ran into a nameclash for C++'s `Angle` class and had to resort to a new
attribute that allows renaming the field for C++ only.

Simplifies the `Angle` datatype to only store radians.
As far as I can tell I managed to do this in a way that should break
almost no existing usage (ignoring deprecation warnings)

### Checklist
* [x] "passes" main test
* [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/6916?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/6916?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/6916)
- [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`.
Wumpf added a commit that referenced this issue Jul 22, 2024
### What

* Part of #6831 

Very similar to previous PRs in the series. Still doesn't remove
existing components since `Transform3D` datatype is still used in
`OutOfTreeTransform` and the `Transform3D` datatype is still used to
describe `from_parent` (that's the next thing to fix!)

### Checklist
* [x] don't regress `main` checks further
* [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/6929?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/6929?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/6929)
- [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`.
Wumpf added a commit that referenced this issue Jul 22, 2024
### What

* Based on  #6929
* Important milestone towards #6831
    * only missing the planned out of tree handling
    * and some other related tickets .. ;)
* Fixes #6863
* created follow-up issue for the aliases which I didn't get to:
#6943

Automated test + test again planet system

![image](https://github.com/user-attachments/assets/15dbda23-7b62-432a-a4a8-cfacf448c2a8)

Still need some more test & a checklist for this blob of changes, as
well as a more thorough pass on the migration guide!

### Checklist
* [x] main checks
* [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/6944?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/6944?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/6944)
- [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`.
@Wumpf
Copy link
Member Author

Wumpf commented Jul 22, 2024

via discussion with @jleibs :

  • deprecate Position3d on boxes3D
    • add Translation3D, point out that this is the same as on Transform3D + interactions wiht OutOfTreeTransform component
  • ellipsoids should have this new schema from the start
  • check to what other archetypes this may apply short term

Wumpf added a commit that referenced this issue Jul 31, 2024
### What

* Part of #6831
* Replacement for #6988

Introduces a new `LeafTransform3D` archetype that is always applicable.
It entails a copy of all of `Transform3D`'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 `Transform3D` so far: Leaf transform is much less commonly used
and deals with arrays, making it sufficiently different from
`Transform3D`. Also a lot of the extensions associated with
`Transform3D` are 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:


https://github.com/user-attachments/assets/62d26661-cd8c-4b4a-b912-063ef60e063a

Snippet demonstrating combination of `Transform3D` with
`LeafTransforms3D`:


https://github.com/user-attachments/assets/ebb2ce5b-6d9a-407d-9f21-57b92f4ac25c

Follow-up PRs will improve the interaction of various archetypes with
`LeafTransforms3D` as well as remove now unused legacy types.


### Checklist
* [x] run main ci to ensure that roundtrip & snippet tests are passing
* [x] check transform checklist again!
* [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/7015?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/7015?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/7015)
- [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`.
Wumpf added a commit that referenced this issue Aug 2, 2024
…AxisAngle` directly on `Boxes3D`/`Ellipsoids3D` (#7029)

### What

* Part of #6831 
* Fixes #6973

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](https://github.com/user-attachments/assets/78195e30-8e99-4982-89f2-f2f115e55cb1)

![image](https://github.com/user-attachments/assets/43dbd21f-d0a2-4ce6-b0ed-ea6369b8b5c1)

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

![image](https://github.com/user-attachments/assets/3ad3ba97-38c4-4bdd-8733-706173ce3e5a)


### 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/7029?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/7029?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/7029)
- [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`.
Wumpf added a commit that referenced this issue 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`.
@Wumpf Wumpf closed this as completed Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant