Skip to content

Asset3D archetype#3395

Merged
teh-cmc merged 14 commits intomainfrom
cmc/asset3d_archetype
Sep 22, 2023
Merged

Asset3D archetype#3395
teh-cmc merged 14 commits intomainfrom
cmc/asset3d_archetype

Conversation

@teh-cmc
Copy link
Copy Markdown
Contributor

@teh-cmc teh-cmc commented Sep 21, 2023 •
edited by github-actions bot
Loading

Everything that's needed for Asset3D.

table Asset3D {
  data: rerun.components.Blob ("attr.rerun.component_required", order: 1000);
  media_type: rerun.components.MediaType  ("attr.rerun.component_recommended", nullable, order: 2000);
  transform: rerun.components.OutOfTreeTransform3D ("attr.rerun.component_optional", nullable, order: 3000);
}

☝️ You get the idea!

Missing unit tests for cpp & py; I'll add them tomorrow.

Here's some out-of-tree fun.

rr.init("rerun_example_asset3d_out_of_tree", spawn=True)

rr.set_time_sequence("frame", 0)
rr2.log("asset", rr2.Asset3D.from_file(sys.argv[1]))
# Those points will not be affected by their parent's out-of-tree transform!
rr2.log(
    "asset/points",
    rr2.Points3D(np.vstack([xyz.ravel() for xyz in np.mgrid[3 * [slice(-10, 10, 10j)]]]).T),
)

asset = rr2.Asset3D.from_file(sys.argv[1])
for i in range(1, 20):
    rr.set_time_sequence("frame", i)

    translation = rr2.dt.TranslationRotationScale3D(translation=[0, 0, i - 10.0])
    rr2.log_components("asset", [rr2.cmp.OutOfTreeTransform3DArray.from_similar(translation)])
23-09-21_19.54.26.patched.mp4

What

Checklist

@teh-cmc teh-cmc added sdk-python Python logging API 🏹 arrow Apache Arrow sdk-rust Rust logging API 🍏 primitives Relating to Rerun primitives sdk-cpp C/C++ API specific labels Sep 21, 2023
@teh-cmc teh-cmc force-pushed the cmc/asset3d_archetype branch 6 times, most recently from 7dfcbd5 to f3aa344 Compare September 21, 2023 18:09
@teh-cmc teh-cmc marked this pull request as ready for review September 21, 2023 18:11
@teh-cmc teh-cmc force-pushed the cmc/asset3d_archetype branch from f3aa344 to eac1789 Compare September 21, 2023 18:14
emilk
emilk approved these changes Sep 22, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks amazing, except for the FromStr stuff

crates/re_components/src/load_file.rs
Comment on lines +111 to +123
.as_component_batches()
.into_iter()
.map(|comp_batch| {
let comp_batch = comp_batch.as_ref();
Ok(DataCell::from_arrow(
comp_batch.name(),
comp_batch
.try_to_arrow()
.map_err(|err| anyhow::anyhow!("serialization failed: {err}"))?,
))
})
.collect();
Copy link
Copy Markdown
Member

@emilk emilk Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is identical – we could have a helper for this, e.g. in trait Archetype

Copy link
Copy Markdown
Contributor Author

@teh-cmc teh-cmc Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to, but that would require a dependency on re_log_types for DataCell, which is circular :(

I'll add a comment/issue.

UPDATE: #3414

crates/re_space_view_spatial/src/parts/assets.rs
Comment on lines +90 to +98
let (scale, rotation, translation) =
glam::Affine3A::from(transform.0).to_scale_rotation_translation();
let transform =
glam::Affine3A::from_scale_rotation_translation(scale, rotation, translation);
Copy link
Copy Markdown
Member

@emilk emilk Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this extra dance should be justified. Why are we throwing away shearing?

Copy link
Copy Markdown
Contributor Author

@teh-cmc teh-cmc Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's copy pasted as-is from the original code... I don't know why we're throwing the shear 🤷‍♂️

Copy link
Copy Markdown
Member

@Wumpf Wumpf Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh strange! To me this looks like from before we allowed general affine transforms - for a quite a while we only had rigid + uniform scale, but nowadays we do the correct thing to the normals (https://github.com/rerun-io/rerun/blob/main/crates/re_renderer/src/renderer/mesh_renderer.rs#L228).
Away with it! :)

rerun_py/rerun_sdk/rerun/components/media_type_ext.py
return MediaType(MediaTypeExt.TEXT)

@staticmethod
def markdown() -> MediaType:
Copy link
Copy Markdown
Member

@emilk emilk Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is more pythonic - users typing MediaType.markdown() or MediaType.MARKDOWN? 🤔

I don't think we need to support both just because we do so in e.g. Rust (where we need to for technical reasons)

Copy link
Copy Markdown
Contributor Author

@teh-cmc teh-cmc Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not the same though. I would love to get rid of these methods, but it's the only way I can think of to actually provide a properly type constant without ending up in circular import hell.

@teh-cmc teh-cmc mentioned this pull request Sep 22, 2023
Closed
@teh-cmc teh-cmc force-pushed the cmc/asset3d_archetype branch from a562155 to 4a74e1f Compare September 22, 2023 16:14
@teh-cmc teh-cmc merged commit 0a7eada into main Sep 22, 2023
@teh-cmc teh-cmc deleted the cmc/asset3d_archetype branch September 22, 2023 16:24
@rerun-io rerun-io deleted a comment from Atoklabu Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Wumpf Wumpf Wumpf left review comments

@emilk emilk emilk approved these changes

Assignees

No one assigned

Labels

🏹 arrow Apache Arrow include in changelog 🍏 primitives Relating to Rerun primitives sdk-cpp C/C++ API specific sdk-python Python logging API sdk-rust Rust logging API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Asset3D mesh migration to archetypes

3 participants

@teh-cmc @emilk @Wumpf