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

Show meshes and images with rerun foo.obj bar.png #2060

Merged
merged 24 commits into from
May 16, 2023
Merged

Conversation

emilk
Copy link
Member

@emilk emilk commented May 9, 2023

What

I was investigating a performance issue when loading .glb files and wanted a simple way to test this. So now you can run rerun foo.obj to quickly show a glb, gltf, obj, jpeg, or png file. Other image file types will also work if you enable their features in the image crate. You can also load multiple files at once, e.g. rerun *.rrd.

Checklist

PR Build Summary: https://build.rerun.io/pr/2060

@emilk emilk force-pushed the emilk/simple-mesh-loading branch from ce14280 to 62fe8d5 Compare May 9, 2023 08:16
@emilk emilk added enhancement New feature or request 📺 re_viewer affects re_viewer itself labels May 9, 2023
@emilk emilk changed the title Show meshes with rerun mesh.glb Show meshes and images with rerun file May 9, 2023
@emilk emilk changed the title Show meshes and images with rerun file Show meshes and images with rerun foo.obj bar.png May 9, 2023
@emilk emilk force-pushed the emilk/simple-mesh-loading branch 2 times, most recently from a0705c5 to 0e05194 Compare May 10, 2023 16:27
@emilk emilk force-pushed the emilk/simple-mesh-loading branch from 0e05194 to 5c6de58 Compare May 12, 2023 09:21
@emilk emilk marked this pull request as ready for review May 12, 2023 10:53
@teh-cmc teh-cmc self-requested a review May 15, 2023 07:47
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

Nice to see this coming alive.

I find it a bit weird that one cannot load both a file and an HTTP resource at the same time, especially since the HTTP link will be treated as a path which will result in something along the lines of Failed to load "https://app.rerun.io/data/colmap_fiat.rrd": Failed to open file.

More importantly, I only see downsides to using MsgSender here: the feature doesn't seem to rely on sending messages at all, it just needs to crafts rows of data.
Implementing this directly as an extension to DataRow will make it reusable by others and remove the dependency on re_sdk.

crates/re_smart_channel/src/lib.rs Outdated Show resolved Hide resolved
crates/rerun/src/run.rs Outdated Show resolved Hide resolved
crates/rerun/src/run.rs Outdated Show resolved Hide resolved
crates/re_sdk/src/msg_sender.rs Show resolved Hide resolved
crates/re_sdk/src/msg_sender.rs Show resolved Hide resolved
crates/rerun/src/run.rs Outdated Show resolved Hide resolved
emilk and others added 3 commits May 15, 2023 17:06
@emilk
Copy link
Member Author

emilk commented May 15, 2023

I find it a bit weird that one cannot load both a file and an HTTP resource at the same time, especially since the HTTP link will be treated as a path which will result in something along the lines of Failed to load "https://app.rerun.io/data/colmap_fiat.rrd": Failed to open file.

Yeah, I agree it is a bit weird, but implementing this balloons the PR from "Support loading files" to also include things like "Support hosting multiple web viewers at once, all connected to different web socket addresses, while also showing a viewer which loads two different .rrd files at once" etc etc. Hence I am leaning towards leaving that as a ticket:

I will make sure to improve the error message though, and add TODO:s to the code

@emilk
Copy link
Member Author

emilk commented May 15, 2023

Added DataCell::from_file_path

@teh-cmc teh-cmc self-requested a review May 16, 2023 07:04
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

Nice!

Trying to load a non-existing file path results in the websocket path being taken, which might be very confusing to end users not familiar with what's going on internally:

[2023-05-16T07:31:23Z INFO  re_viewer::remote_viewer_app] Connecting to WS server at "ws:///home/cmc/Downloads/i_dont_exist.png"…
[2023-05-16T07:31:23Z INFO  re_ws_comms::client] Connecting to "ws:///home/cmc/Downloads/i_dont_exist.png"…
[2023-05-16T07:31:23Z ERROR re_ws_comms::client] Connection error: HTTP format error: invalid format

crates/re_log_types/src/path/entity_path.rs Outdated Show resolved Hide resolved
Comment on lines +117 to +122
let cell = DataCell::from_file_path(file_path)?;
Ok(Self {
num_instances: Some(cell.num_instances()),
instanced: vec![cell],
..Self::new(ent_path)
})
Copy link
Member

Choose a reason for hiding this comment

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

nice

@emilk
Copy link
Member Author

emilk commented May 16, 2023

Trying to load a non-existing file path results in the websocket path being taken, which might be very confusing to end users not familiar with what's going on internally:

[2023-05-16T07:31:23Z INFO  re_viewer::remote_viewer_app] Connecting to WS server at "ws:///home/cmc/Downloads/i_dont_exist.png"…
[2023-05-16T07:31:23Z INFO  re_ws_comms::client] Connecting to "ws:///home/cmc/Downloads/i_dont_exist.png"…
[2023-05-16T07:31:23Z ERROR re_ws_comms::client] Connection error: HTTP format error: invalid format

Not sure how to fix that easily. It can be really hard to tell a file path from a websocket address. Is example.zip a file or a domain name?

We can add heuristics, like "ends with .png, jpg, …"… I'll do something

@teh-cmc
Copy link
Member

teh-cmc commented May 16, 2023

Just pointing it out, don't think we necessarily have to fix today.

Maybe the easiest way for now is to add some explanation as to what's going on in the error message when everything else failed ("hey we tried opening 'xxx' as X, Y and Z and none of them worked").

@emilk emilk merged commit c8323ac into main May 16, 2023
@emilk emilk deleted the emilk/simple-mesh-loading branch May 16, 2023 10:57
@emilk emilk mentioned this pull request May 25, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 📺 re_viewer affects re_viewer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants