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

Add archetype_native attribute and make ContainerBlueprint eager + partial #8666

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Jan 13, 2025

Introduces a new attribute that, when paired with an eager archetype, will also generate a native companion type with back-and-forth conversion methods.

This is especially useful for blueprint archetypes (although there are definitely non-blueprint examples too), which are heavily used all across the viewer, and would be very painful to use otherwise.

/// Whether we should generate an extra Rust object comprised of native Rust types.
///
/// The generated object will have the name of the archetype, prefixed by `Native`,
/// e.g. `NativePoints3D`.
///
/// Applies only to *eager* archetypes. No-op otherwise.
attribute "attr.rust.archetype_native";

With this, we now should have all the tools required to port every remaining archetype (both blueprint and data).

@teh-cmc teh-cmc added 🦀 Rust API Rust logging API 🧑‍💻 dev experience developer experience (excluding CI) codegen/idl exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jan 13, 2025
@teh-cmc
Copy link
Member Author

teh-cmc commented Jan 13, 2025

Ha yeah I also ported VisualBounds2D and Background because it didn't require any particular effort.

Copy link

github-actions bot commented Jan 13, 2025

Web viewer failed to build.

| Result | Commit | Link | Manifest |
| ------ | ------- | ----- |
| ❌ | | https://rerun.io/viewer/pr/8666 | +nightly +main |

Note: This comment is updated whenever you push a commit.

@teh-cmc teh-cmc requested review from emilk and Wumpf January 14, 2025 10:01
Copy link
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.

Nice!

});
let native_to_eager = quote! {
impl From<&#native_name> for #name {
#[rustfmt::skip] // so it doesn't take 1000 lines for no reason
Copy link
Member

Choose a reason for hiding this comment

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

What do you mean? Why would cargo fmt produce 1000 lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

rustfmt splits method chains no matter their length, so all the value.#field_name.clone().map(|batch| (batch.descriptor, batch.array)) emitted by this method turn into 4 lines each, which is really pushing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

As opposed to this neat little block 😊
image

@teh-cmc teh-cmc merged commit dd04366 into main Jan 14, 2025
25 of 26 checks passed
@teh-cmc teh-cmc deleted the cmc/eager_partial_8_native_companion branch January 14, 2025 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codegen/idl 🧑‍💻 dev experience developer experience (excluding CI) exclude from changelog PRs with this won't show up in CHANGELOG.md 🦀 Rust API Rust logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants