Transform3D no longer sets all its components to empty array by default#11911
Transform3D no longer sets all its components to empty array by default#11911
Conversation
# Conflicts: # crates/store/re_tf/src/transform_resolution_cache.rs
Wumpf
added
🍏 primitives
|
Web viewer built successfully.
View image diff on kitdiff. Note: This comment is updated whenever you push a commit. |
grtlr
left a comment
grtlr
left a comment
There was a problem hiding this comment.
Nice! Only some minor questions.
| pub const ATTR_RERUN_COMPONENT_OPTIONAL: &str = "attr.rerun.component_optional"; | ||
| pub const ATTR_RERUN_COMPONENT_RECOMMENDED: &str = "attr.rerun.component_recommended"; | ||
| pub const ATTR_RERUN_COMPONENT_REQUIRED: &str = "attr.rerun.component_required"; | ||
| pub const ATTR_RERUN_LOG_MISSING_AS_EMPTY: &str = "attr.rerun.log_missing_as_empty"; |
There was a problem hiding this comment.
Are we sure to update the snippets? Or should we add migration code?
There was a problem hiding this comment.
as per Slack message: yeah because migration code would be complicated and brittle - nulling out all [] is only legal if all transform components are missing.
Personally I also don't like it - the code IS producing different data now. Not in a viewer-breaking manner sure, but it's somewhat breaking
| /// \example archetypes/instance_poses3d_combined title="Regular & instance transforms in tandem" image="https://static.rerun.io/leaf_transform3d/41674f0082d6de489f8a1cd1583f60f6b5820ddf/1200w.png" | ||
| /// \example archetypes/mesh3d_instancing !api title="3D mesh with instancing" image="https://static.rerun.io/mesh3d_leaf_transforms3d/c2d0ee033129da53168f5705625a9b033f3a3d61/1200w.png" | ||
| table InstancePoses3D ( | ||
| "attr.docs.category": "Spatial 3D", |
There was a problem hiding this comment.
Makes me wonder: How did InstancePose3D get away without the missing attribute?
There was a problem hiding this comment.
it didn't. We always accepted that this has different semantics there. Having the attribute in the first place was compromise nobody was happy with and somehow we arrived having it not there. I vaguely remember that being a conscious decision, but I can't summon the why....
Wumpf
left a comment
Wumpf
left a comment
There was a problem hiding this comment.
(why did this go into review mode?! I wanted to reply only)
| /// \example archetypes/instance_poses3d_combined title="Regular & instance transforms in tandem" image="https://static.rerun.io/leaf_transform3d/41674f0082d6de489f8a1cd1583f60f6b5820ddf/1200w.png" | ||
| /// \example archetypes/mesh3d_instancing !api title="3D mesh with instancing" image="https://static.rerun.io/mesh3d_leaf_transforms3d/c2d0ee033129da53168f5705625a9b033f3a3d61/1200w.png" | ||
| table InstancePoses3D ( | ||
| "attr.docs.category": "Spatial 3D", |
There was a problem hiding this comment.
it didn't. We always accepted that this has different semantics there. Having the attribute in the first place was compromise nobody was happy with and somehow we arrived having it not there. I vaguely remember that being a conscious decision, but I can't summon the why....
There was a problem hiding this comment.
as per Slack message: yeah because migration code would be complicated and brittle - nulling out all [] is only legal if all transform components are missing.
Personally I also don't like it - the code IS producing different data now. Not in a viewer-breaking manner sure, but it's somewhat breaking
2 participants
Related
What
As described in
we changed our query semantics such that this is no longer necessary. This greatly improves navigation of transforms int he Viewer & queries and has the potential to speed logging of transforms up significantly (especially SDK sided).
In other words.
Before:
After:
