Add support to expand rows to show instances in the dataframe view #7400

abey79 · 2024-09-11T12:45:45Z

What

Closes Dataframe view: add an Index column for instances and support un/collapsing #7066

This PR introduces the ability to expend dataframe view rows to inspect individual instances.

Note: expanding instance can potentially lead to very loooong tables. This doesn't affect performance as we only query/draw what is visible on screen, it means we reaching the limits of egui's f32 numerical precision. This can lead to uneven scrolling or even visual glitches if the number of instance is >>1M.

Follow-up todo:

Have alternating background stripes also in expanded rows in the dataframe view #7412

row_expand.mp4

Checklist

I have read and agree to Contributor Guide and the Code of Conduct
I've included a screenshot or gif (if applicable)
I have tested the web demo (if applicable):
- Using examples from latest main build: rerun.io/viewer
- Using full set of examples from nightly build: rerun.io/viewer
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
If applicable, add a new check to the release checklist!
If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

emilk

Haven't had time for a proper review yet; just a quick look

crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs

[skip ci]

abey79 · 2024-09-12T15:01:20Z

For some reason, the view stop drawing while the time panel is playing 🤔

edit: fixed by f5786b8

Export-1726153168328.mp4

abey79 · 2024-09-12T16:50:08Z

edit: fixed in 89313e5 (#7400)

This PR triggers a panic in egui via egui_table.

Repro:

open arkit example
create a dataframe view with origin = /world/mesh
keep the default "Follow timeline" setting
pause the timeline
expend the /world/mesh:Position3D or TriangleIndices column
crash? sometime it doesn't reproduce immediately
modify the origin (e.g. /world/**) making sure /world/mesh is still included
back to (5), rinse and repeat until crash happens.

backtrace:

thread 'main' panicked at 'assertion failed: (rect.height() - size.y).abs() < 1.0 || size.y == f32::INFINITY'
egui/src/layout.rs:664
stack backtrace:
   8: core::panicking::panic_fmt
             at core/src/panicking.rs:72:14
   9: core::panicking::panic
             at core/src/panicking.rs:144:5
  10: egui::layout::Layout::next_widget_space_ignore_wrap_justify
             at egui/src/layout.rs:664:9
  11: egui::placer::Placer::next_widget_space_ignore_wrap_justify
             at egui/src/placer.rs:220:9
      egui::placer::Placer::set_min_height
             at egui/src/placer.rs:271:20
      egui::ui::Ui::set_min_height
             at egui/src/ui.rs:824:9
      egui::ui::Ui::set_min_size
             at egui/src/ui.rs:810:9
  12: egui_table::split_scroll::SplitScroll::show::{{closure}}::{{closure}}
             at egui_table/src/split_scroll.rs:91:25
      core::ops::function::FnOnce::call_once{{vtable.shim}}
             at core/src/ops/function.rs:250:5
  13: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
      egui::containers::scroll_area::ScrollArea::show_viewport_dyn
             at egui/src/containers/scroll_area.rs:786:21
  14: core::ops::function::FnOnce::call_once{{vtable.shim}}
  15: egui::ui::Ui::scope_dyn
  16: egui::ui::Ui::scope
             at egui/src/ui.rs:2132:9
  17: core::ops::function::FnOnce::call_once{{vtable.shim}}
  18: egui::ui::Ui::scope_dyn
  19: egui_table::table::Table::show
             at egui_table/src/table.rs:409:9
  20: core::ops::function::FnOnce::call_once{{vtable.shim}}
  21: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
      egui::containers::frame::Frame::show_dyn
             at egui/src/containers/frame.rs:283:19
  22: egui::containers::frame::Frame::show
             at egui/src/containers/frame.rs:273:9
      re_space_view_dataframe::dataframe_ui::dataframe_ui_impl
             at re_space_view_dataframe/src/dataframe_ui.rs:520:5
  23: re_space_view_dataframe::dataframe_ui::dataframe_ui
             at re_space_view_dataframe/src/dataframe_ui.rs:25:5
      <re_space_view_dataframe::space_view_class::DataframeSpaceView as re_viewer_context::space_view::space_view_class::SpaceViewClass>::ui
             at re_space_view_dataframe/src/space_view_class.rs:171:17
  24: core::ops::function::FnOnce::call_once{{vtable.shim}}
  25: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
  26: egui::ui::Ui::scope_dyn
             at egui/src/ui.rs:2153:19
  27: egui::ui::Ui::scope
             at egui/src/ui.rs:2132:9
  28: <re_viewport::viewport::TabViewer as egui_tiles::behavior::Behavior<re_viewer_context::blueprint_id::BlueprintId<re_viewer_context::blueprint_id::SpaceViewIdRegistry>>>::pane_ui
             at re_viewport/src/viewport.rs:545:9
  29: core::ops::function::FnOnce::call_once{{vtable.shim}}
  30: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
  31: egui::ui::Ui::scope_dyn
             at egui/src/ui.rs:2153:19
  32: egui::ui::Ui::scope
             at egui/src/ui.rs:2132:9
  33: egui::ui::Ui::add_enabled_ui
             at egui/src/ui.rs:1537:9
      egui_tiles::tree::Tree<Pane>::tile_ui
             at b2f5e23/src/tree.rs:347:12
  34: egui_tiles::container::tabs::Tabs::ui
             at b2f5e23/src/container/tabs.rs:203:13
  35: core::ops::function::FnOnce::call_once{{vtable.shim}}
  36: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
  37: egui::ui::Ui::scope_dyn
             at egui/src/ui.rs:2153:19
  38: egui::ui::Ui::scope
             at egui/src/ui.rs:2132:9
  39: egui::ui::Ui::add_enabled_ui
             at egui/src/ui.rs:1537:9
      egui_tiles::tree::Tree<Pane>::tile_ui
             at b2f5e23/src/tree.rs:347:12
  40: egui_tiles::tree::Tree<Pane>::ui
             at b2f5e23/src/tree.rs:279:13
  41: core::ops::function::FnOnce::call_once{{vtable.shim}}
  42: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
  43: egui::ui::Ui::scope_dyn
             at egui/src/ui.rs:2153:19
  44: egui::ui::Ui::scope
             at egui/src/ui.rs:2132:9
  45: re_viewport::viewport::Viewport::viewport_ui
             at re_viewport/src/viewport.rs:138:9
  46: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
  47: egui::containers::panel::CentralPanel::show_inside_dyn::{{closure}}
             at egui/src/containers/panel.rs:1118:13
      core::ops::function::FnOnce::call_once{{vtable.shim}}
             at core/src/ops/function.rs:250:5
  48: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
      egui::containers::frame::Frame::show_dyn
             at egui/src/containers/frame.rs:283:19
  49: egui::containers::frame::Frame::show
             at egui/src/containers/frame.rs:273:9
      egui::containers::panel::CentralPanel::show_inside_dyn
             at egui/src/containers/panel.rs:1116:15
  50: egui::containers::panel::CentralPanel::show_inside
             at egui/src/containers/panel.rs:1095:9
      re_viewer::app_state::AppState::show
             at re_viewer/src/app_state.rs:444:13
  51: core::ops::function::FnOnce::call_once{{vtable.shim}}
  52: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
  53: egui::containers::panel::CentralPanel::show_inside_dyn::{{closure}}
             at egui/src/containers/panel.rs:1118:13
      core::ops::function::FnOnce::call_once{{vtable.shim}}
             at core/src/ops/function.rs:250:5
  54: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:2015:9
      egui::containers::frame::Frame::show_dyn
             at egui/src/containers/frame.rs:283:19
  55: egui::containers::frame::Frame::show
             at egui/src/containers/frame.rs:273:9
      egui::containers::panel::CentralPanel::show_inside_dyn
             at egui/src/containers/panel.rs:1116:15
  56: egui::containers::panel::CentralPanel::show_dyn
             at egui/src/containers/panel.rs:1149:30
  57: egui::containers::panel::CentralPanel::show
             at egui/src/containers/panel.rs:1128:9
      re_viewer::app::App::ui
             at re_viewer/src/app.rs:934:9
      <re_viewer::app::App as eframe::epi::App>::update
             at re_viewer/src/app.rs:1612:9
  58: eframe::native::epi_integration::EpiIntegration::update::{{closure}}
             at eframe/src/native/epi_integration.rs:283:17
      egui::context::Context::run
             at egui/src/context.rs:767:9
  59: eframe::native::epi_integration::EpiIntegration::update
             at eframe/src/native/epi_integration.rs:276:27
  60: eframe::native::wgpu_integration::WgpuWinitRunning::run_ui_and_paint
             at eframe/src/native/wgpu_integration.rs:605:27
      <eframe::native::wgpu_integration::WgpuWinitApp as eframe::native::winit_integration::WinitApp>::run_ui_and_paint
             at eframe/src/native/wgpu_integration.rs:374:13
  61: <eframe::native::run::WinitAppWrapper<T> as winit::application::ApplicationHandler<eframe::native::winit_integration::UserEvent>>::window_event::{{closure}}
             at eframe/src/native/run.rs:278:21
      eframe::native::event_loop_context::with_event_loop_context
             at eframe/src/native/event_loop_context.rs:53:5
  62: <eframe::native::run::WinitAppWrapper<T> as winit::application::ApplicationHandler<eframe::native::winit_integration::UserEvent>>::window_event
             at eframe/src/native/run.rs:275:9
  63: winit::event_loop::dispatch_event_for_app
             at winit-0.30.5/src/event_loop.rs:642:52
      winit::event_loop::EventLoop<T>::run_app::{{closure}}
             at winit-0.30.5/src/event_loop.rs:265:49
  64: winit::platform_impl::macos::event_loop::map_user_event::{{closure}}
             at winit-0.30.5/src/platform_impl/macos/event_loop.rs:174:22
  65: <alloc::boxed::Box<F,A> as core::ops::function::FnMut<Args>>::call_mut
             at alloc/src/boxed.rs:2022:9
      winit::platform_impl::macos::event_handler::EventHandler::handle_event
             at winit-0.30.5/src/platform_impl/macos/event_handler.rs:125:17
  66: winit::platform_impl::macos::app_state::ApplicationDelegate::handle_event
             at winit-0.30.5/src/platform_impl/macos/app_state.rs:303:9
  67: winit::platform_impl::macos::app_state::ApplicationDelegate::cleared
             at winit-0.30.5/src/platform_impl/macos/app_state.rs:365:13
  68: winit::platform_impl::macos::observer::control_flow_end_handler::{{closure}}
             at winit-0.30.5/src/platform_impl/macos/observer.rs:84:21
      winit::platform_impl::macos::observer::control_flow_handler::{{closure}}
             at winit-0.30.5/src/platform_impl/macos/observer.rs:46:9
      std::panicking::try::do_call
             at std/src/panicking.rs:552:40
      std::panicking::try
             at std/src/panicking.rs:516:19
  69: std::panic::catch_unwind
             at std/src/panic.rs:142:14
      winit::platform_impl::macos::event_loop::stop_app_on_panic
             at winit-0.30.5/src/platform_impl/macos/event_loop.rs:435:11
      winit::platform_impl::macos::observer::control_flow_handler
             at winit-0.30.5/src/platform_impl/macos/observer.rs:44:5
      winit::platform_impl::macos::observer::control_flow_end_handler
             at winit-0.30.5/src/platform_impl/macos/observer.rs:79:9
  70: <unknown>
  71: <unknown>
  72: <unknown>
  73: <unknown>
  74: <unknown>
  75: <unknown>
  76: <unknown>
  77: <unknown>
  78: <unknown>
  79: <unknown>
  80: winit::platform_impl::macos::event_loop::EventLoop<T>::run_on_demand::{{closure}}::{{closure}}
             at winit-0.30.5/src/platform_impl/macos/event_loop.rs:306:26
      objc2::rc::autorelease::autoreleasepool
             at objc2-0.5.2/src/rc/autorelease.rs:438:15
      winit::platform_impl::macos::event_loop::EventLoop<T>::run_on_demand::{{closure}}
             at winit-0.30.5/src/platform_impl/macos/event_loop.rs:292:13
      winit::platform_impl::macos::event_handler::EventHandler::set
             at winit-0.30.5/src/platform_impl/macos/event_handler.rs:98:9
  81: winit::platform_impl::macos::app_state::ApplicationDelegate::set_event_handler
             at winit-0.30.5/src/platform_impl/macos/app_state.rs:172:9
      winit::platform_impl::macos::event_loop::EventLoop<T>::run_on_demand
             at winit-0.30.5/src/platform_impl/macos/event_loop.rs:291:9
  82: <winit::event_loop::EventLoop<T> as winit::platform::run_on_demand::EventLoopExtRunOnDemand>::run_on_demand
             at winit-0.30.5/src/platform/run_on_demand.rs:89:9
      winit::platform::run_on_demand::EventLoopExtRunOnDemand::run_app_on_demand
             at winit-0.30.5/src/platform/run_on_demand.rs:75:9
      eframe::native::run::run_and_return
             at eframe/src/native/run.rs:295:16
      eframe::native::run::run_wgpu::{{closure}}
             at eframe/src/native/run.rs:352:13
      eframe::native::run::with_event_loop::{{closure}}
             at eframe/src/native/run.rs:52:12
      std::thread::local::LocalKey<T>::try_with
             at std/src/thread/local.rs:270:16
      std::thread::local::LocalKey<T>::with
             at std/src/thread/local.rs:246:9
      eframe::native::run::with_event_loop
             at eframe/src/native/run.rs:42:16
      eframe::native::run::run_wgpu
             at eframe/src/native/run.rs:350:16
      eframe::run_native
             at eframe/src/lib.rs:265:13

cc @emilk

…alidation

… tens of ms frame time)

emilk

I think we should introduce a second word for these "sub-rows", like "line". As in: "Most table rows consist of a single line, but rows can sometimes be expanded to cover multiple lines"

crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs

emilk · 2024-09-16T08:40:40Z

crates/viewer/re_space_view_dataframe/src/expanded_rows.rs

+
+/// Storage for [`ExpandedRows`], which should be persisted across frames.
+#[derive(Debug, Clone)]
+pub(crate) struct ExpandedRowsCache {


Is there one of these per view, or what?

Yes. They are stored in the view state. (Alternatively could use egui memory with view id as key, but the view state works well and doesn't require id shenanigans.)

(Added a clarifying comment)

crates/viewer/re_space_view_dataframe/src/expanded_rows.rs

crates/viewer/re_ui/src/list_item/list_item.rs

emilk · 2024-09-16T08:47:26Z

crates/viewer/re_ui/src/design_tokens.rs

@@ -289,7 +289,7 @@ impl DesignTokens {
    }

    pub fn table_line_height() -> f32 {


We should probably note that this excludes margins

It doesn't, though. We don't have any explicit vertical margin, we just center stuff within the line space.

emilk

I think we should introduce a second word for these "sub-rows", like "line". As in: "Most table rows consist of a single line, but rows can sometimes be expanded to cover multiple lines"

# Conflicts: # Cargo.lock # crates/viewer/re_space_view_dataframe/Cargo.toml # crates/viewer/re_space_view_dataframe/src/space_view_class.rs

### What This PR adds zebra stripes to all dataframe view lines. Before, when a row was extend to show instances (#7400), those additional lines weren't striped. Also, the alternating colour is now a bit more visible. <img width="1789" alt="image" src="https://github.com/user-attachments/assets/95da89a8-d132-4b10-97fd-4f2ce20f40bc"> ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7434?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7434?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7434) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`.

abey79 added 5 commits September 11, 2024 14:45

First working prototype

327791b

Proper UI for collapsing/uncollapsing instances

d80ef25

Merge branch 'main' into antoine/dataframe-instance-expansion

008f8b6

Proper handling of "xx more…" instances

b4b0d5c

Fix animation + some code cleaning

1fa7fb5

abey79 added ui concerns graphical user interface include in changelog feat-dataframe-view Everything related to the dataframe view labels Sep 12, 2024

abey79 added 3 commits September 12, 2024 12:22

More cleaning and lint fix

69f394a

Merge branch 'main' into antoine/dataframe-instance-expansion

d07cce2

Make the collapse/expand button the same color as inactive text

ec8efd4

abey79 changed the title ~~WIP: instance expansion~~ Add support to expand rows to show instances in the dataframe view Sep 12, 2024

abey79 marked this pull request as ready for review September 12, 2024 10:28

emilk reviewed Sep 12, 2024

View reviewed changes

crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs Outdated Show resolved Hide resolved

crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs Outdated Show resolved Hide resolved

crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs Outdated Show resolved Hide resolved

US spelling

6d46025

abey79 marked this pull request as draft September 12, 2024 12:53

abey79 added 5 commits September 12, 2024 15:06

Rewrite of the sub-cell logic, reads like a child book now

f63728f

[skip ci]

Extract part of cell_ui into cell_ui_impl

4b13469

Further method extraction to fight against right drift

45ddb3f

Minor cleaning

c197bab

Merge branch 'main' into antoine/dataframe-instance-expansion

b5c8c98

abey79 marked this pull request as ready for review September 12, 2024 13:50

Tiny cleanup

ee80456

Improve salting, which fixes the refresh bug

f5786b8

abey79 added 4 commits September 12, 2024 19:08

Much improved, simplified, and documented the row expansion cache inv…

91d19a6

…alidation

Merge branch 'main' into antoine/dataframe-instance-expansion

07c6bf3

Replace the test script with a (simpler) zoo specimen

500939a

Only iterate on visible instances based on clip rect (shaved multiple…

49fef4f

… tens of ms frame time)

emilk approved these changes Sep 16, 2024

View reviewed changes

abey79 added 5 commits September 17, 2024 09:25

Merge branch 'main' into antoine/dataframe-instance-expansion

3abee41

# Conflicts: # Cargo.lock # crates/viewer/re_space_view_dataframe/Cargo.toml # crates/viewer/re_space_view_dataframe/src/space_view_class.rs

Review comments

1d76790

Some more renaming

51716df

Updated egui commit to last version (which removed the panic)

89313e5

Merge branch 'main' into antoine/dataframe-instance-expansion

1f25fb6

abey79 merged commit 16cc253 into main Sep 17, 2024
34 checks passed

abey79 deleted the antoine/dataframe-instance-expansion branch September 17, 2024 14:37

abey79 mentioned this pull request Sep 17, 2024

Dataframe view now has zebra stripes on every line #7434

Merged

6 tasks

emilk removed the include in changelog label Oct 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to expand rows to show instances in the dataframe view #7400

Add support to expand rows to show instances in the dataframe view #7400

abey79 commented Sep 11, 2024 •

edited by github-actions bot

Loading

emilk left a comment

abey79 commented Sep 12, 2024 •

edited

Loading

abey79 commented Sep 12, 2024 •

edited

Loading

emilk left a comment

emilk Sep 16, 2024

abey79 Sep 17, 2024

abey79 Sep 17, 2024

emilk Sep 16, 2024

abey79 Sep 17, 2024

emilk left a comment

		@@ -289,7 +289,7 @@ impl DesignTokens {
		}

		pub fn table_line_height() -> f32 {

Add support to expand rows to show instances in the dataframe view #7400

Add support to expand rows to show instances in the dataframe view #7400

Conversation

abey79 commented Sep 11, 2024 • edited by github-actions bot Loading

What

Checklist

emilk left a comment

Choose a reason for hiding this comment

abey79 commented Sep 12, 2024 • edited Loading

abey79 commented Sep 12, 2024 • edited Loading

emilk left a comment

Choose a reason for hiding this comment

emilk Sep 16, 2024

Choose a reason for hiding this comment

abey79 Sep 17, 2024

Choose a reason for hiding this comment

abey79 Sep 17, 2024

Choose a reason for hiding this comment

emilk Sep 16, 2024

Choose a reason for hiding this comment

abey79 Sep 17, 2024

Choose a reason for hiding this comment

emilk left a comment

Choose a reason for hiding this comment

abey79 commented Sep 11, 2024 •

edited by github-actions bot

Loading

abey79 commented Sep 12, 2024 •

edited

Loading

abey79 commented Sep 12, 2024 •

edited

Loading