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

2D view of multiple DepthImages doesn't update with time scroll #7679

Closed
eskjorg opened this issue Oct 10, 2024 · 5 comments
Closed

2D view of multiple DepthImages doesn't update with time scroll #7679

eskjorg opened this issue Oct 10, 2024 · 5 comments
Labels
💬 discussion enhancement New feature or request ❓ question Further information is requested ui concerns graphical user interface

Comments

@eskjorg
Copy link

eskjorg commented Oct 10, 2024

Describe the bug
2D view with multiple DepthImages doesn't show the correct image when dragging the slider.

But hovering still works, showing the correct pixel for each layer. This indicates that the correct data is there.

To Reproduce

for i, depth_image in enumerate(depth_arr):
    rr.log(
        f"{phase}/depth_imgs/{i}",
        rr.DepthImage(depth_image, meter=1000.0, colormap=Colormap.Viridis),
    )

Then press play or drag slider through time.

Expected behavior
The 2D view should show each corresponding image when scrolling.

Screenshots

This is bad:

screen_rec.mov

This is good:
screen_shot

Desktop
macOS Sonoma 14.5 (23F79)

Rerun version
rerun-cli 0.18.2 [rustc 1.76.0 (07dca489a 2024-02-04), LLVM 17.0.6] aarch64-apple-darwin release-0.18.2 59ff15b, built 2024-08-29T13:55:42Z

@eskjorg eskjorg added 👀 needs triage This issue needs to be triaged by the Rerun team 🪳 bug Something isn't working labels Oct 10, 2024
@Wumpf
Copy link
Member

Wumpf commented Oct 10, 2024

Interesting find! So actually DepthImage is not supposed to support (transparent) layering at all: this is ever so slightly hinted by the fact that only Image and SegmentationImage come with an optional Opacity component.

This means that a depth image in front will always hide everything behind it. So unless you log new data to the front-most entity, you won't see anything.
Which in turn means that the actual bug here is unfortunately the thing you called out working: Multiple previews should not show the hover interaction is meant to be disabled for fully hidden things (just like a mesh being drawn in front of a point doesn't allow hovering the point).
Note also that the default DrawOrder is stable, but undefined, so you can't be sure which image is in front unless you specify it yourself.

But obviously your intention here is to get information out of several images so I'm wondering what you would like this to look like. Can you elaborate a bit on that?

So far we considered it not meaningful to have transparent layering on depth images - I have a hard time understanding how to get meaningful information out of such a visualization. Typically in those cases you'd either keep logging to the same entity path (then at every point in time you have a different image being shown) or, if all of them should show at once, you'd create more Views via a blueprint.

@eskjorg
Copy link
Author

eskjorg commented Oct 14, 2024

But obviously your intention here is to get information out of several images so I'm wondering what you would like this to look like. Can you elaborate a bit on that?

I was hoping that there was some clever way in which a stack of images would be shown. So they still only take up one "tab" in the viewer. For example another slider separate from time that would scroll among the images here. Maybe I should just stack them into a tensor and log that once. Would that show better?

So far we considered it not meaningful to have transparent layering on depth images - I have a hard time understanding how to get meaningful information out of such a visualization. Typically in those cases you'd either keep logging to the same entity path (then at every point in time you have a different image being shown) or, if all of them should show at once, you'd create more Views via a blueprint.

I see then which 2 alternatives I have. Using DrawOrder for example helped. Thanks!

Note also that the default DrawOrder is stable, but undefined, so you can't be sure which image is in front unless you specify it yourself.

Yes, I've noticed that the draw order here seemed strange but still the same every time when I rerun the code. Why is that?

@Wumpf
Copy link
Member

Wumpf commented Oct 14, 2024

Maybe I should just stack them into a tensor and log that once. Would that show better?

you could do that to get the slider you're looking for, but then you loose all the features that are specific to depth images (like depth cloud generation when viewed in 3D), also the Tensor view has a few shortcomings like more limited hover interaction etc.
Really though the timeline is right now the go-to tool for this kind of interaction. Another orthogonal slider that operates on which entity is shown sounds like a cool idea but a lot more design effort would need to go into making that well defined 🤔
From the pov of the existing systems that would bottom out in "make all entities visible except one, which one controlled by a slider that is somehow tied to entity paths" which is a bit tricky (there's no special concept right now for numbered entitites either, those are just names).

Yes, I've noticed that the draw order here seemed strange but still the same every time when I rerun the code. Why is that?

In a nutshell as long as you don't log draw order, the order is "random but stable". Right now I believe default draw order is determined by alphabetical order of entity names, but this may be subject to change. We can't rely on log order since data may come in different order, and at that point we also don't want to special case on the default timelines to extract this information since there's some paths of data ingestion that don't have them.


Not entirely sure on the actionable things here now: I created a separate bug for the unexpected multihover. But beyond that we'd need a whole new ui concept here to support this. Not saying that wouldn't be a good thing to have, it's just that I can't quite yet get my head around it.

@Wumpf Wumpf added enhancement New feature or request ❓ question Further information is requested 💬 discussion ui concerns graphical user interface and removed 🪳 bug Something isn't working 👀 needs triage This issue needs to be triaged by the Rerun team labels Oct 14, 2024
@eskjorg
Copy link
Author

eskjorg commented Oct 14, 2024

Thanks for all explanations!
I say don't invest time in any new UI for my sake, I barely look at those depth images, just thought I found a bug.
( Would be much more interested in this #2499 ! )

Fine to close this issue now.

@Wumpf
Copy link
Member

Wumpf commented Oct 14, 2024

Alright, let's close this for now then. Definitely a design direction we'll have to get back to though 🤔
cc: @gavrelina

@Wumpf Wumpf closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💬 discussion enhancement New feature or request ❓ question Further information is requested ui concerns graphical user interface
Projects
None yet
Development

No branches or pull requests

2 participants