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

end-to-end arrow-based text entries #654

Merged
merged 190 commits into from
Jan 3, 2023
Merged

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Dec 28, 2022

This PR does the necessary for arrow-based text-views to work end-to-end.

Nothing too exciting/surprising besides these two points:

  • try_add_arrow_data_msg now has some extra logic to determine the ObjectType of an entity based on the components in the MsgBundle.
    This expands upon @jleibs' hack that was there before, and still is very much a hack.
    We need this because text views are completely separate.

  • TimePoint has been elevated to the rank of Component!
    TimePoint is now a component and it is now automatically stored with the rest of an entity's components when inserting a new row.
    This allows us to easily query for all the timelines of a row without having to maintain yet another custom/dedicated index.
    We need this because text views list all their different timestamps for every row.
    I originally implemented it that way because it was the simplest/frictionless way of supporting this use case... but it turned out to actually be pretty elegant, I think.
    This is currently blocked by a performance issue though.

  • DataStore now keeps an extra index for keeping track of per-message data (see also MsgId-as-a-component #655)

To try it:

  • rust: cargo run -p rerun_sdk --example log_text_entries
  • python: just py-build && RERUN_EXP_ARROW=mixed examples/text_logging/main.py

examples/text_logging/main.py in mixed mode:

image

Closes #523
Requires #653 #655

Base automatically changed from cmc/datastore/msgid_as_component to main January 3, 2023 07:57
crates/rerun_sdk/examples/log_text_entries.rs Show resolved Hide resolved
Comment on lines +121 to +132
let bundle = MsgBundle::new(
MsgId::random(),
obj_path.clone(),
time_point,
[
rects.map(|rects| rects.try_into().unwrap()),
colors.map(|colors| colors.try_into().unwrap()),
]
.into_iter()
.flatten()
.collect(),
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: rewritten this way so that the vector of ComponentBundles passed to MsgBundle::new isn't empty, so that we can craft an array of MsgIds with the appropriate length (i.e. number of instances)!

crates/rerun_sdk/examples/log_text_entries.rs Outdated Show resolved Hide resolved
@teh-cmc teh-cmc marked this pull request as ready for review January 3, 2023 11:29
@Wumpf Wumpf self-requested a review January 3, 2023 12:46
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

git(hub) did interesting things to the commit list here

crates/re_viewer/src/ui/view_text/scene.rs Outdated Show resolved Hide resolved

#[derive(Debug, clap::Parser)]
#[clap(author, version, about)]
struct Args {
Copy link
Member

Choose a reason for hiding this comment

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

I'm new to our rust samples. Seems like a little bit too much boiler plate right now that we need to reduce?
Hiding stuff away in a example-only helper library if ofc problematic as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

I basically just mirrored the only existing Rust example as I haven't been involved with that SDK effort yet, so right now I just want to leave things in a consistent state for those who are! :)

crates/rerun_sdk/examples/log_text_entries.rs Show resolved Hide resolved
crates/rerun_sdk/examples/log_text_entries.rs Show resolved Hide resolved
@teh-cmc
Copy link
Member Author

teh-cmc commented Jan 3, 2023

git(hub) did interesting things to the commit list here

This is the result of a 4-layer deep PR-ception, I'm just happy everyone made it out alive 😄

@teh-cmc teh-cmc merged commit 2dff240 into main Jan 3, 2023
@teh-cmc teh-cmc deleted the cmc/arrowification/text_logs branch January 3, 2023 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

re_viewer: arrowification of text scenes
2 participants