diff --git a/.typos.toml b/.typos.toml index 0db77cd3215a..72249872a9f7 100644 --- a/.typos.toml +++ b/.typos.toml @@ -34,6 +34,7 @@ armour = "armor" artefact = "artifact" authorise = "authorize" behaviour = "behavior" +behavioural = "behavioral" British = "American" calibre = "caliber" # allow 'cancelled' since Arrow uses it. diff --git a/Cargo.lock b/Cargo.lock index c3767eabd3c2..5ce727803dc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1777,7 +1777,7 @@ checksum = "68b0cf012f1230e43cd00ebb729c6bb58707ecfa8ad08b52ef3a4ccd2697fc30" [[package]] name = "ecolor" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "bytemuck", "emath", @@ -1787,7 +1787,7 @@ dependencies = [ [[package]] name = "eframe" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "ahash", "bytemuck", @@ -1824,7 +1824,7 @@ dependencies = [ [[package]] name = "egui" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "accesskit", "ahash", @@ -1841,7 +1841,7 @@ dependencies = [ [[package]] name = "egui-wgpu" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "ahash", "bytemuck", @@ -1860,7 +1860,7 @@ dependencies = [ [[package]] name = "egui-winit" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "accesskit_winit", "ahash", @@ -1900,7 +1900,7 @@ dependencies = [ [[package]] name = "egui_extras" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "ahash", "egui", @@ -1916,7 +1916,7 @@ dependencies = [ [[package]] name = "egui_glow" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "ahash", "bytemuck", @@ -1989,7 +1989,7 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "emath" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "bytemuck", "serde", @@ -2090,7 +2090,7 @@ dependencies = [ [[package]] name = "epaint" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" dependencies = [ "ab_glyph", "ahash", @@ -2109,7 +2109,7 @@ dependencies = [ [[package]] name = "epaint_default_fonts" version = "0.28.1" -source = "git+https://github.com/emilk/egui.git?rev=66076101e12eee01dec374285521b0bed4ecc40a#66076101e12eee01dec374285521b0bed4ecc40a" +source = "git+https://github.com/emilk/egui.git?rev=1191d9fa86bcb33b57b264b0105f986005f6a6c6#1191d9fa86bcb33b57b264b0105f986005f6a6c6" [[package]] name = "equivalent" @@ -5478,6 +5478,7 @@ dependencies = [ "re_data_ui", "re_dataframe", "re_entity_db", + "re_format", "re_log", "re_log_types", "re_renderer", diff --git a/Cargo.toml b/Cargo.toml index 66e2f4e619a3..129c6521ade2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -514,12 +514,12 @@ missing_errors_doc = "allow" # As a last resport, patch with a commit to our own repository. # ALWAYS document what PR the commit hash is part of, or when it was merged into the upstream trunk. -ecolor = { git = "https://github.com/emilk/egui.git", rev = "66076101e12eee01dec374285521b0bed4ecc40a" } # egui master 2024-09-06 -eframe = { git = "https://github.com/emilk/egui.git", rev = "66076101e12eee01dec374285521b0bed4ecc40a" } # egui master 2024-09-06 -egui = { git = "https://github.com/emilk/egui.git", rev = "66076101e12eee01dec374285521b0bed4ecc40a" } # egui master 2024-09-06 -egui_extras = { git = "https://github.com/emilk/egui.git", rev = "66076101e12eee01dec374285521b0bed4ecc40a" } # egui master 2024-09-06 -egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "66076101e12eee01dec374285521b0bed4ecc40a" } # egui master 2024-09-06 -emath = { git = "https://github.com/emilk/egui.git", rev = "66076101e12eee01dec374285521b0bed4ecc40a" } # egui master 2024-09-06 +ecolor = { git = "https://github.com/emilk/egui.git", rev = "1191d9fa86bcb33b57b264b0105f986005f6a6c6" } # egui master 2024-09-17 +eframe = { git = "https://github.com/emilk/egui.git", rev = "1191d9fa86bcb33b57b264b0105f986005f6a6c6" } # egui master 2024-09-17 +egui = { git = "https://github.com/emilk/egui.git", rev = "1191d9fa86bcb33b57b264b0105f986005f6a6c6" } # egui master 2024-09-17 +egui_extras = { git = "https://github.com/emilk/egui.git", rev = "1191d9fa86bcb33b57b264b0105f986005f6a6c6" } # egui master 2024-09-17 +egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "1191d9fa86bcb33b57b264b0105f986005f6a6c6" } # egui master 2024-09-17 +emath = { git = "https://github.com/emilk/egui.git", rev = "1191d9fa86bcb33b57b264b0105f986005f6a6c6" } # egui master 2024-09-17 # Useful while developing: # ecolor = { path = "../../egui/crates/ecolor" } diff --git a/crates/viewer/re_component_ui/src/datatype_uis/float_drag.rs b/crates/viewer/re_component_ui/src/datatype_uis/float_drag.rs index e75fdcad6e33..e1910e7c8121 100644 --- a/crates/viewer/re_component_ui/src/datatype_uis/float_drag.rs +++ b/crates/viewer/re_component_ui/src/datatype_uis/float_drag.rs @@ -40,7 +40,7 @@ pub fn edit_f32_float_raw_impl( let speed = (value.abs() * 0.01).at_least(0.001); ui.add( egui::DragValue::new(value) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(range) .speed(speed), ) @@ -67,7 +67,7 @@ fn edit_f32_zero_to_one_raw(ui: &mut egui::Ui, value: &mut MaybeMutRef<'_, f32>) if let Some(value) = value.as_mut() { ui.add( egui::DragValue::new(value) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(0.0..=1.0) .speed(0.005) .fixed_decimals(2), diff --git a/crates/viewer/re_component_ui/src/range1d.rs b/crates/viewer/re_component_ui/src/range1d.rs index ce3390b48d68..5c1adef2817e 100644 --- a/crates/viewer/re_component_ui/src/range1d.rs +++ b/crates/viewer/re_component_ui/src/range1d.rs @@ -15,14 +15,14 @@ pub fn edit_range1d( let response_min = ui.add( egui::DragValue::new(min) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(f64::NEG_INFINITY..=*max) .speed(speed), ); ui.label("-"); let response_max = ui.add( egui::DragValue::new(max) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(*min..=f64::INFINITY) .speed(speed), ); diff --git a/crates/viewer/re_component_ui/src/visual_bounds2d.rs b/crates/viewer/re_component_ui/src/visual_bounds2d.rs index 7aec58b1b901..47c1a121e9c1 100644 --- a/crates/viewer/re_component_ui/src/visual_bounds2d.rs +++ b/crates/viewer/re_component_ui/src/visual_bounds2d.rs @@ -56,7 +56,7 @@ fn range_mut_ui(ui: &mut egui::Ui, [start, end]: &mut [f64; 2]) -> egui::Respons ui.horizontal_centered(|ui| { let response_min = ui.add( egui::DragValue::new(start) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(f64::MIN..=*end) .max_decimals(2) .speed(speed), @@ -66,7 +66,7 @@ fn range_mut_ui(ui: &mut egui::Ui, [start, end]: &mut [f64; 2]) -> egui::Respons let response_max = ui.add( egui::DragValue::new(end) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(*start..=f64::MAX) .max_decimals(2) .speed(speed), @@ -94,7 +94,7 @@ pub fn singleline_edit_visual_bounds2d( let response_width = ui.add( egui::DragValue::new(&mut width_edit) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(0.001..=f64::MAX) .max_decimals(1) .speed(speed_func(width)), @@ -102,7 +102,7 @@ pub fn singleline_edit_visual_bounds2d( ui.label("×"); let response_height = ui.add( egui::DragValue::new(&mut height_edit) - .clamp_to_range(false) + .clamp_existing_to_range(false) .range(0.001..=f64::MAX) .max_decimals(1) .speed(speed_func(height)), diff --git a/crates/viewer/re_space_view_dataframe/Cargo.toml b/crates/viewer/re_space_view_dataframe/Cargo.toml index 9394d9ebcaa7..11a80b39a69b 100644 --- a/crates/viewer/re_space_view_dataframe/Cargo.toml +++ b/crates/viewer/re_space_view_dataframe/Cargo.toml @@ -23,6 +23,7 @@ re_chunk_store.workspace = true re_data_ui.workspace = true re_dataframe.workspace = true re_entity_db.workspace = true +re_format.workspace = true re_log.workspace = true re_log_types.workspace = true re_renderer.workspace = true diff --git a/crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs b/crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs index 245fd91cbd27..d6c854474e5e 100644 --- a/crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/dataframe_ui.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use std::ops::Range; use anyhow::Context; -use egui::NumExt as _; +use egui::{NumExt as _, Ui}; use itertools::Itertools; use re_chunk_store::{ColumnDescriptor, LatestAtQuery, RowId}; @@ -13,14 +13,16 @@ use re_ui::UiExt as _; use re_viewer_context::ViewerContext; use crate::display_record_batch::{DisplayRecordBatch, DisplayRecordBatchError}; +use crate::expanded_rows::{ExpandedRows, ExpandedRowsCache}; /// Display a dataframe table for the provided query. pub(crate) fn dataframe_ui<'a>( ctx: &ViewerContext<'_>, ui: &mut egui::Ui, query: impl Into>, + expanded_rows_cache: &mut ExpandedRowsCache, ) { - dataframe_ui_impl(ctx, ui, &query.into()); + dataframe_ui_impl(ctx, ui, &query.into(), expanded_rows_cache); } /// A query handle for either a latest-at or range query. @@ -165,6 +167,8 @@ struct DataframeTableDelegate<'a> { header_entity_paths: Vec>, display_data: anyhow::Result, + expanded_rows: ExpandedRows<'a>, + num_rows: u64, } @@ -173,10 +177,6 @@ impl DataframeTableDelegate<'_> { } impl<'a> egui_table::TableDelegate for DataframeTableDelegate<'a> { - fn default_row_height(&self) -> f32 { - re_ui::DesignTokens::table_line_height() - } - fn prepare(&mut self, info: &egui_table::PrefetchInfo) { re_tracing::profile_function!(); @@ -235,6 +235,7 @@ impl<'a> egui_table::TableDelegate for DataframeTableDelegate<'a> { fn cell_ui(&mut self, ui: &mut egui::Ui, cell: &egui_table::CellInfo) { re_tracing::profile_function!(); + //TODO(ab): paint subcell as well if cell.row_nr % 2 == 1 { // Paint stripes ui.painter() @@ -243,6 +244,27 @@ impl<'a> egui_table::TableDelegate for DataframeTableDelegate<'a> { debug_assert!(cell.row_nr < self.num_rows, "Bug in egui_table"); + egui::Frame::none() + .inner_margin(egui::Margin::symmetric(Self::LEFT_RIGHT_MARGIN, 0.0)) + .show(ui, |ui| { + self.cell_ui_impl(ui, cell); + }); + } + + fn row_top_offset(&self, _ctx: &egui::Context, _table_id: egui::Id, row_nr: u64) -> f32 { + self.expanded_rows.row_top_offset(row_nr) + } + + fn default_row_height(&self) -> f32 { + re_ui::DesignTokens::table_line_height() + } +} + +impl DataframeTableDelegate<'_> { + /// Draw the content of a cell. + fn cell_ui_impl(&mut self, ui: &mut Ui, cell: &egui_table::CellInfo) { + re_tracing::profile_function!(); + let display_data = match &self.display_data { Ok(display_data) => display_data, Err(err) => { @@ -251,55 +273,241 @@ impl<'a> egui_table::TableDelegate for DataframeTableDelegate<'a> { } }; - let cell_ui = |ui: &mut egui::Ui| { - if let Some(BatchRef { batch_idx, row_idx }) = - display_data.batch_ref_from_row.get(&cell.row_nr).copied() - { - let batch = &display_data.display_record_batches[batch_idx]; - let column = &batch.columns()[cell.col_nr]; - - // compute the latest-at query for this row (used to display tooltips) - let timestamp = display_data - .query_time_column_index - .and_then(|col_idx| { - display_data.display_record_batches[batch_idx].columns()[col_idx] - .try_decode_time(row_idx) - }) - .unwrap_or(TimeInt::MAX); - let latest_at_query = LatestAtQuery::new(self.query_handle.timeline(), timestamp); - let row_id = display_data - .row_id_column_index - .and_then(|col_idx| { - display_data.display_record_batches[batch_idx].columns()[col_idx] - .try_decode_row_id(row_idx) - }) - .unwrap_or(RowId::ZERO); + let Some(BatchRef { + batch_idx, + row_idx: batch_row_idx, + }) = display_data.batch_ref_from_row.get(&cell.row_nr).copied() + else { + error_ui( + ui, + "Bug in egui_table: we didn't prefetch what was rendered!", + ); + + return; + }; - if ui.is_sizing_pass() { - ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); - } else { - ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Truncate); - } - column.data_ui(self.ctx, ui, row_id, &latest_at_query, row_idx); - } else { - error_ui( + let batch = &display_data.display_record_batches[batch_idx]; + let column = &batch.columns()[cell.col_nr]; + + // compute the latest-at query for this row (used to display tooltips) + + // TODO(ab): this is done for every cell but really should be done only once per row + let timestamp = display_data + .query_time_column_index + .and_then(|col_idx| { + display_data.display_record_batches[batch_idx].columns()[col_idx] + .try_decode_time(batch_row_idx) + }) + .unwrap_or(TimeInt::MAX); + let latest_at_query = LatestAtQuery::new(self.query_handle.timeline(), timestamp); + let row_id = display_data + .row_id_column_index + .and_then(|col_idx| { + display_data.display_record_batches[batch_idx].columns()[col_idx] + .try_decode_row_id(batch_row_idx) + }) + .unwrap_or(RowId::ZERO); + + if ui.is_sizing_pass() { + ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); + } else { + ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Truncate); + } + + let instance_count = column.instance_count(batch_row_idx); + let row_expansion = self.expanded_rows.additional_lines_for_row(cell.row_nr); + + // Iterate over the lines for this cell. The initial `None` value corresponds to the summary + // line. + let instance_indices = std::iter::once(None) + .chain((0..instance_count).map(Option::Some)) + .take(row_expansion as usize + 1); + + { + re_tracing::profile_scope!("subcells"); + + // how the line is drawn + let line_content = |ui: &mut egui::Ui, + expanded_rows: &mut ExpandedRows<'_>, + line_index: usize, + instance_index: Option| { + // This is called when data actually needs to be drawn (as opposed to summaries like + // "N instances" or "N more…"). + let data_content = |ui: &mut egui::Ui| { + column.data_ui( + self.ctx, + ui, + row_id, + &latest_at_query, + batch_row_idx, + instance_index, + ); + }; + + line_ui( ui, - "Bug in egui_table: we didn't prefetch what was rendered!", + expanded_rows, + line_index, + instance_index, + instance_count, + cell, + data_content, ); + }; + + split_ui_vertically(ui, &mut self.expanded_rows, instance_indices, line_content); + } + } +} + +/// Draw a single line in a table. +/// +/// This deals with the row expansion interaction and logic, as well as summarizing the data when +/// necessary. The actual data drawing is delegated to the `data_content` closure. +fn line_ui( + ui: &mut egui::Ui, + expanded_rows: &mut ExpandedRows<'_>, + line_index: usize, + instance_index: Option, + instance_count: u64, + cell: &egui_table::CellInfo, + data_content: impl Fn(&mut egui::Ui), +) { + re_tracing::profile_function!(); + + let row_expansion = expanded_rows.additional_lines_for_row(cell.row_nr); + + /// What kinds of lines might we encounter here? + enum SubcellKind { + /// Summary line with content that as zero or one instances, so cannot be expanded. + Summary, + + /// Summary line with >1 instances, so can be expanded. + SummaryWithExpand, + + /// A particular instance + Instance, + + /// There are more instances than available lines, so this is a summary of how many + /// there are left. + MoreInstancesSummary { remaining_instances: u64 }, + + /// Not enough instances to fill this line. + Blank, + } + + // The truth table that determines what kind of line we are dealing with. + let subcell_kind = match instance_index { + // First row with >1 instances. + None if { instance_count > 1 } => SubcellKind::SummaryWithExpand, + + // First row with 0 or 1 instances. + None => SubcellKind::Summary, + + // Last line and possibly too many instances to display. + Some(instance_index) + if { line_index as u64 == row_expansion && instance_index < instance_count } => + { + let remaining = instance_count + .saturating_sub(instance_index) + .saturating_sub(1); + if remaining > 0 { + // +1 is because the "X more…" line takes one instance spot + SubcellKind::MoreInstancesSummary { + remaining_instances: remaining + 1, + } + } else { + SubcellKind::Instance } - }; + } - egui::Frame::none() - .inner_margin(egui::Margin::symmetric(Self::LEFT_RIGHT_MARGIN, 0.0)) - .show(ui, cell_ui); + // Some line for which an instance exists. + Some(instance_index) if { instance_index < instance_count } => SubcellKind::Instance, + + // Some line for which no instance exists. + Some(_) => SubcellKind::Blank, + }; + + match subcell_kind { + SubcellKind::Summary => { + data_content(ui); + } + + SubcellKind::SummaryWithExpand => { + let cell_clicked = cell_with_hover_button_ui(ui, &re_ui::icons::EXPAND, |ui| { + ui.label(format!( + "{} instances", + re_format::format_uint(instance_count) + )); + }); + + if cell_clicked { + if instance_count == row_expansion { + expanded_rows.remove_additional_lines_for_row(cell.row_nr); + } else { + expanded_rows.set_additional_lines_for_row(cell.row_nr, instance_count); + } + } + } + + SubcellKind::Instance => { + let cell_clicked = cell_with_hover_button_ui(ui, &re_ui::icons::COLLAPSE, data_content); + + if cell_clicked { + expanded_rows.remove_additional_lines_for_row(cell.row_nr); + } + } + + SubcellKind::MoreInstancesSummary { + remaining_instances, + } => { + let cell_clicked = cell_with_hover_button_ui(ui, &re_ui::icons::EXPAND, |ui| { + ui.label(format!( + "{} more…", + re_format::format_uint(remaining_instances) + )); + }); + + if cell_clicked { + expanded_rows.set_additional_lines_for_row(cell.row_nr, instance_count); + } + } + + SubcellKind::Blank => { /* nothing to show */ } } } /// Display the result of a [`QueryHandle`] in a table. -fn dataframe_ui_impl(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, query_handle: &QueryHandle<'_>) { +fn dataframe_ui_impl( + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + query_handle: &QueryHandle<'_>, + expanded_rows_cache: &mut ExpandedRowsCache, +) { re_tracing::profile_function!(); let schema = query_handle.schema(); + + // The table id mainly drives column widths, so it should be stable across queries leading to + // the same schema. + let table_id_salt = egui::Id::new("__dataframe__").with(schema); + + // It's trickier for the row expansion cache. + // + // For latest-at view, there is always a single row, so it's ok to validate the cache against + // the schema. This means that changing the latest-at time stamp does _not_ invalidate, which is + // desirable. Otherwise, it would be impossible to expand a row when tracking the time panel + // while it is playing. + // + // For range queries, the row layout can change drastically when the query min/max times are + // modified, so in that case we invalidate against the query expression. This means that the + // expanded-ness is reset as soon as the min/max boundaries are changed in the selection panel, + // which is acceptable. + let row_expansion_id_salt = match query_handle { + QueryHandle::LatestAt(_) => egui::Id::new("__dataframe_row_exp__").with(schema), + QueryHandle::Range(query) => egui::Id::new("__dataframe_row_exp__").with(query.query()), + }; + let (header_groups, header_entity_paths) = column_groups_for_entity(schema); let num_rows = query_handle.num_rows(); @@ -313,6 +521,12 @@ fn dataframe_ui_impl(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, query_handle: & display_data: Err(anyhow::anyhow!( "No row data, `fetch_columns_and_rows` not called." )), + expanded_rows: ExpandedRows::new( + ui.ctx().clone(), + ui.make_persistent_id(row_expansion_id_salt), + expanded_rows_cache, + re_ui::DesignTokens::table_line_height(), + ), }; let num_sticky_cols = schema @@ -322,6 +536,7 @@ fn dataframe_ui_impl(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, query_handle: & egui::Frame::none().inner_margin(5.0).show(ui, |ui| { egui_table::Table::new() + .id_salt(table_id_salt) .columns( schema .iter() @@ -378,3 +593,112 @@ fn error_ui(ui: &mut egui::Ui, error: impl AsRef) { ui.error_label(error); re_log::warn_once!("{error}"); } + +/// Draw some cell content with an optional, right-aligned, on-hover button. +/// +/// Returns true if the button was clicked. +// TODO(ab, emilk): ideally, egui::Sides should work for that, but it doesn't yet support the +// asymmetric behavior (left variable width, right fixed width). +// See https://github.com/emilk/egui/issues/5116 +fn cell_with_hover_button_ui( + ui: &mut egui::Ui, + icon: &'static re_ui::Icon, + cell_content: impl FnOnce(&mut egui::Ui), +) -> bool { + if ui.is_sizing_pass() { + // we don't need space for the icon since it only shows on hover + cell_content(ui); + return false; + } + + let is_hovering_cell = ui.rect_contains_pointer(ui.max_rect()); + let is_clicked = + is_hovering_cell && ui.input(|i| i.pointer.button_clicked(egui::PointerButton::Primary)); + + if is_hovering_cell { + let mut content_rect = ui.max_rect(); + content_rect.max.x = (content_rect.max.x + - re_ui::DesignTokens::small_icon_size().x + - re_ui::DesignTokens::text_to_icon_padding()) + .at_least(content_rect.min.x); + + let button_rect = egui::Rect::from_x_y_ranges( + (content_rect.max.x + re_ui::DesignTokens::text_to_icon_padding()) + ..=ui.max_rect().max.x, + ui.max_rect().y_range(), + ); + + let mut content_ui = ui.new_child(egui::UiBuilder::new().max_rect(content_rect)); + cell_content(&mut content_ui); + + let mut button_ui = ui.new_child(egui::UiBuilder::new().max_rect(button_rect)); + button_ui.visuals_mut().widgets.hovered.weak_bg_fill = egui::Color32::TRANSPARENT; + button_ui.visuals_mut().widgets.active.weak_bg_fill = egui::Color32::TRANSPARENT; + button_ui.add(egui::Button::image( + icon.as_image() + .fit_to_exact_size(re_ui::DesignTokens::small_icon_size()) + .tint(button_ui.visuals().widgets.noninteractive.text_color()), + )); + + is_clicked + } else { + cell_content(ui); + false + } +} + +/// Helper to draw individual lines into an expanded cell in a table. +/// +/// `context`: whatever mutable context is necessary for the `line_content_ui` +/// `line_data`: the data to be displayed in each line +/// `line_content_ui`: the function to draw the content of each line +fn split_ui_vertically( + ui: &mut egui::Ui, + context: &mut Ctx, + line_data: impl Iterator, + line_content_ui: impl Fn(&mut egui::Ui, &mut Ctx, usize, Item), +) { + re_tracing::profile_function!(); + + // Empirical testing shows that iterating over all instances can take multiple tens of ms + // when the instance count is very large (which is common). So we use the clip rectangle to + // determine exactly which instances are visible and iterate only over those. + let visible_y_range = ui.clip_rect().y_range(); + let total_y_range = ui.max_rect().y_range(); + + // Note: converting float to unsigned ints implicitly saturate negative values to 0 + let start_row = ((visible_y_range.min - total_y_range.min) + / re_ui::DesignTokens::table_line_height()) + .floor() as usize; + + let end_row = ((visible_y_range.max - total_y_range.min) + / re_ui::DesignTokens::table_line_height()) + .ceil() as usize; + + for (line_index, item_data) in line_data + .enumerate() + .skip(start_row) + .take(end_row.saturating_sub(start_row)) + { + let line_rect = egui::Rect::from_min_size( + ui.cursor().min + + egui::vec2( + 0.0, + line_index as f32 * re_ui::DesignTokens::table_line_height(), + ), + egui::vec2( + ui.available_width(), + re_ui::DesignTokens::table_line_height(), + ), + ); + + // During animation, there may be more lines than can possibly fit. If so, no point in + // continuing to draw them. + if !ui.max_rect().intersects(line_rect) { + return; + } + + let mut line_ui = ui.new_child(egui::UiBuilder::new().max_rect(line_rect)); + line_content_ui(&mut line_ui, context, line_index, item_data); + } +} diff --git a/crates/viewer/re_space_view_dataframe/src/display_record_batch.rs b/crates/viewer/re_space_view_dataframe/src/display_record_batch.rs index 98cb31e59e3a..86bf8e149619 100644 --- a/crates/viewer/re_space_view_dataframe/src/display_record_batch.rs +++ b/crates/viewer/re_space_view_dataframe/src/display_record_batch.rs @@ -31,6 +31,9 @@ pub(crate) enum DisplayRecordBatchError { UnexpectedComponentColumnDataType(String, ArrowDataType), } +/// A single column of component data. +/// +/// Abstracts over the different possible arrow representation of component data. pub(crate) enum ComponentData { Null, ListArray(ArrowListArray), @@ -76,6 +79,39 @@ impl ComponentData { } } + /// Returns the number of instances for the given row index. + /// + /// For [`Self::Null`] columns, or for invalid `row_index`, this will return 0. + fn instance_count(&self, row_index: usize) -> u64 { + match self { + Self::Null => 0, + Self::ListArray(list_array) => { + if list_array.is_valid(row_index) { + list_array.value(row_index).len() as u64 + } else { + 0 + } + } + Self::DictionaryArray { dict, values } => { + if dict.is_valid(row_index) { + values.value(dict.key_value(row_index)).len() as u64 + } else { + 0 + } + } + } + } + + /// Display some data from the column. + /// + /// - Argument `row_index` is the row index within the batch column. + /// - Argument `instance_index` is the specific instance within the specified row. If `None`, a + /// summary of all existing instances is displayed. + /// + /// # Panic + /// + /// Panics if `instance_index` is out-of-bound. Use [`Self::instance_count`] to ensure + /// correctness. #[allow(clippy::too_many_arguments)] fn data_ui( &self, @@ -85,11 +121,15 @@ impl ComponentData { latest_at_query: &LatestAtQuery, entity_path: &EntityPath, component_name: ComponentName, - row_index: usize, // index within the batch column + row_index: usize, + instance_index: Option, ) { let data = match self { Self::Null => { - ui.label("null"); + // don't repeat the null value when expanding instances + if instance_index.is_none() { + ui.label("null"); + } return; } Self::ListArray(list_array) => list_array @@ -101,6 +141,14 @@ impl ComponentData { }; if let Some(data) = data { + let data_to_display = if let Some(instance_index) = instance_index { + // Panics if the instance index is out of bound. This is checked in + // `DisplayColumn::data_ui`. + data.sliced(instance_index as usize, 1) + } else { + data + }; + ctx.component_ui_registry.ui_raw( ctx, ui, @@ -110,7 +158,7 @@ impl ComponentData { entity_path, component_name, Some(row_id), - &*data, + &*data_to_display, ); } else { ui.label("-"); @@ -118,6 +166,7 @@ impl ComponentData { } } +/// A single column of data in a record batch. pub(crate) enum DisplayColumn { RowId { row_id_times: ArrowPrimitiveArray, @@ -201,23 +250,49 @@ impl DisplayColumn { } } + pub(crate) fn instance_count(&self, row_index: usize) -> u64 { + match self { + Self::RowId { .. } | Self::Timeline { .. } => 1, + Self::Component { component_data, .. } => component_data.instance_count(row_index), + } + } + + /// Display some data in the column. + /// + /// - Argument `row_index` is the row index within the batch column. + /// - Argument `instance_index` is the specific instance within the row to display. If `None`, + /// a summary of all instances is displayed. If the instance is out-of-bound (aka greater than + /// [`Self::instance_count`]), nothing is displayed. pub(crate) fn data_ui( &self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, row_id: RowId, latest_at_query: &LatestAtQuery, - index: usize, + row_index: usize, + instance_index: Option, ) { + if let Some(instance_index) = instance_index { + if instance_index >= self.instance_count(row_index) { + // do not display anything for out-of-bound instance index + return; + } + } + match self { Self::RowId { row_id_times, row_id_counters, .. } => { + if instance_index.is_some() { + // we only ever display the row id on the summary line + return; + } + let row_id = RowId::from_u128( - (row_id_times.value(index) as u128) << 64 - | (row_id_counters.value(index) as u128), + (row_id_times.value(row_index) as u128) << 64 + | (row_id_counters.value(row_index) as u128), ); row_id_ui(ctx, ui, &row_id); } @@ -225,7 +300,12 @@ impl DisplayColumn { timeline, time_data, } => { - let timestamp = TimeInt::try_from(time_data.value(index)); + if instance_index.is_some() { + // we only ever display the row id on the summary line + return; + } + + let timestamp = TimeInt::try_from(time_data.value(row_index)); match timestamp { Ok(timestamp) => { ui.label(timeline.typ().format(timestamp, ctx.app_options.time_zone)); @@ -247,7 +327,8 @@ impl DisplayColumn { latest_at_query, entity_path, *component_name, - index, + row_index, + instance_index, ); } } diff --git a/crates/viewer/re_space_view_dataframe/src/expanded_rows.rs b/crates/viewer/re_space_view_dataframe/src/expanded_rows.rs new file mode 100644 index 000000000000..a3523dd81820 --- /dev/null +++ b/crates/viewer/re_space_view_dataframe/src/expanded_rows.rs @@ -0,0 +1,137 @@ +use std::collections::BTreeMap; + +/// Storage for [`ExpandedRows`], which should be persisted across frames. +/// +/// Note: each view should store its own cache. Using a [`re_viewer_context::SpaceViewState`] is a +/// good way to do this. +#[derive(Debug, Clone)] +pub(crate) struct ExpandedRowsCache { + /// Maps "table row number" to "additional lines". + /// + /// When expanded, the base space is still used for the summary, while the additional lines are + /// used for instances. + expanded_rows: BTreeMap, + + /// ID used to invalidate the cache. + valid_for: egui::Id, +} + +impl Default for ExpandedRowsCache { + fn default() -> Self { + Self { + expanded_rows: BTreeMap::default(), + valid_for: egui::Id::new(""), + } + } +} + +impl ExpandedRowsCache { + /// This sets the query used for cache invalidation. + /// + /// If the query doesn't match the cached one, the state will be reset. + fn validate_id(&mut self, id: egui::Id) { + if id != self.valid_for { + self.valid_for = id; + self.expanded_rows = BTreeMap::default(); + } + } +} + +/// Helper to keep track of row expansion. +/// +/// This is a short-lived struct to be created every frame. The persistent state is stored in +/// [`ExpandedRowsCache`]. +/// +/// Uses egui's animation support to animate the row expansion/contraction. For this to work: +/// - When collapsed, the row entry must be set to 0 instead of being removed. Otherwise, it will no +/// longer be "seen" by the animation code. Technically, it could be removed _after_ the +/// animation completes, but it's not worth the complexity. +/// - When the row is first expanded, for the animation to work, it must be immediately seeded to 0 +/// for the animation to have a starting point. +pub(crate) struct ExpandedRows<'a> { + /// Base row height. + row_height: f32, + + /// Cache containing the row expanded-ness. + cache: &'a mut ExpandedRowsCache, + + /// [`egui::Context`] used to animate the row expansion. + egui_ctx: egui::Context, + + /// [`egui::Id`] used to store the animation state. + id: egui::Id, +} + +impl<'a> ExpandedRows<'a> { + /// Create a new [`ExpandedRows`] instance. + /// + /// `egui_ctx` is used to animate the row expansion + /// `id` is used to store the animation state and invalidate the cache, make it persistent and + /// unique + pub(crate) fn new( + egui_ctx: egui::Context, + id: egui::Id, + cache: &'a mut ExpandedRowsCache, + row_height: f32, + ) -> Self { + // (in-)validate the cache + cache.validate_id(id); + + Self { + row_height, + cache, + egui_ctx, + id, + } + } + + /// Implementation for [`egui_table::TableDelegate::row_top_offset`]. + pub(crate) fn row_top_offset(&self, row_nr: u64) -> f32 { + self.cache + .expanded_rows + .range(0..row_nr) + .map(|(expanded_row_nr, additional_lines)| { + self.egui_ctx.animate_value_with_time( + self.row_id(*expanded_row_nr), + *additional_lines as f32 * self.row_height, + self.egui_ctx.style().animation_time, + ) + }) + .sum::() + + row_nr as f32 * self.row_height + } + + /// Return by how many additional lines this row is expended. + pub(crate) fn additional_lines_for_row(&self, row_nr: u64) -> u64 { + self.cache.expanded_rows.get(&row_nr).copied().unwrap_or(0) + } + + /// Set the expansion of a row. + /// + /// Units are in extra row heights. + pub(crate) fn set_additional_lines_for_row(&mut self, row_nr: u64, additional_lines: u64) { + // Note: don't delete the entry when set to 0, this breaks animation. + + // If this is the first time this row is expended, we must seed the corresponding animation + // cache. + if !self.cache.expanded_rows.contains_key(&row_nr) { + self.egui_ctx.animate_value_with_time( + self.row_id(row_nr), + 0.0, + self.egui_ctx.style().animation_time, + ); + } + + self.cache.expanded_rows.insert(row_nr, additional_lines); + } + + /// Collapse a row. + pub(crate) fn remove_additional_lines_for_row(&mut self, row_nr: u64) { + self.set_additional_lines_for_row(row_nr, 0); + } + + #[inline] + fn row_id(&self, row_nr: u64) -> egui::Id { + self.id.with(row_nr) + } +} diff --git a/crates/viewer/re_space_view_dataframe/src/lib.rs b/crates/viewer/re_space_view_dataframe/src/lib.rs index f7093d3a94dc..e25c7b8ed0c8 100644 --- a/crates/viewer/re_space_view_dataframe/src/lib.rs +++ b/crates/viewer/re_space_view_dataframe/src/lib.rs @@ -4,6 +4,7 @@ mod dataframe_ui; mod display_record_batch; +mod expanded_rows; mod query_kind; mod space_view_class; mod view_query; diff --git a/crates/viewer/re_space_view_dataframe/src/space_view_class.rs b/crates/viewer/re_space_view_dataframe/src/space_view_class.rs index 9e1c9dacb402..5fc62bdafa7d 100644 --- a/crates/viewer/re_space_view_dataframe/src/space_view_class.rs +++ b/crates/viewer/re_space_view_dataframe/src/space_view_class.rs @@ -1,16 +1,34 @@ use egui::Ui; +use std::any::Any; use re_log_types::{EntityPath, EntityPathFilter, ResolvedTimeRange}; use re_types_core::SpaceViewClassIdentifier; use re_viewer_context::{ - SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId, SpaceViewState, + SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId, SpaceViewState, SpaceViewStateExt, SpaceViewSystemExecutionError, SystemExecutionOutput, ViewQuery, ViewerContext, }; use re_viewport_blueprint::SpaceViewContents; use crate::dataframe_ui::dataframe_ui; +use crate::expanded_rows::ExpandedRowsCache; use crate::{query_kind::QueryKind, visualizer_system::EmptySystem}; +#[derive(Default)] +struct DataframeSpaceViewState { + /// Cache for the expanded rows. + expended_rows_cache: ExpandedRowsCache, +} + +impl SpaceViewState for DataframeSpaceViewState { + fn as_any(&self) -> &dyn Any { + self + } + + fn as_any_mut(&mut self) -> &mut dyn Any { + self + } +} + #[derive(Default)] pub struct DataframeSpaceView; @@ -57,7 +75,7 @@ mode sets the default time range to _everything_. You can override this in the s } fn new_state(&self) -> Box { - Box::<()>::default() + Box::::default() } fn preferred_tile_aspect_ratio(&self, _state: &dyn SpaceViewState) -> Option { @@ -91,11 +109,12 @@ mode sets the default time range to _everything_. You can override this in the s &self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, - _state: &mut dyn SpaceViewState, + state: &mut dyn SpaceViewState, query: &ViewQuery<'_>, _system_output: SystemExecutionOutput, ) -> Result<(), SpaceViewSystemExecutionError> { re_tracing::profile_function!(); + let state = state.downcast_mut::()?; let view_query = super::view_query::Query::try_from_blueprint(ctx, query.space_view_id)?; let timeline_name = view_query.timeline_name(ctx); @@ -127,7 +146,7 @@ mode sets the default time range to _everything_. You can override this in the s //TODO(ab): specify which columns let query_handle = query_engine.latest_at(&query, None); - dataframe_ui(ctx, ui, query_handle); + dataframe_ui(ctx, ui, query_handle, &mut state.expended_rows_cache); } QueryKind::Range { pov_entity, @@ -152,7 +171,12 @@ mode sets the default time range to _everything_. You can override this in the s }; //TODO(ab): specify which columns should be displayed or not - dataframe_ui(ctx, ui, query_engine.range(&query, None)); + dataframe_ui( + ctx, + ui, + query_engine.range(&query, None), + &mut state.expended_rows_cache, + ); } }; diff --git a/crates/viewer/re_ui/data/icons/collapse.png b/crates/viewer/re_ui/data/icons/collapse.png new file mode 100644 index 000000000000..f0983e45834a Binary files /dev/null and b/crates/viewer/re_ui/data/icons/collapse.png differ diff --git a/crates/viewer/re_ui/data/icons/expand.png b/crates/viewer/re_ui/data/icons/expand.png new file mode 100644 index 000000000000..3d9ae99ae7c5 Binary files /dev/null and b/crates/viewer/re_ui/data/icons/expand.png differ diff --git a/crates/viewer/re_ui/src/design_tokens.rs b/crates/viewer/re_ui/src/design_tokens.rs index 53c90d8449a6..37504e8df0fd 100644 --- a/crates/viewer/re_ui/src/design_tokens.rs +++ b/crates/viewer/re_ui/src/design_tokens.rs @@ -289,7 +289,7 @@ impl DesignTokens { } pub fn table_line_height() -> f32 { - 16.0 // should be big enough to contain buttons, i.e. egui_style.spacing.interact_size.y + 20.0 // should be big enough to contain buttons, i.e. egui_style.spacing.interact_size.y } pub fn table_header_height() -> f32 { diff --git a/crates/viewer/re_ui/src/icons.rs b/crates/viewer/re_ui/src/icons.rs index 7a03285b816b..da9849aed9bb 100644 --- a/crates/viewer/re_ui/src/icons.rs +++ b/crates/viewer/re_ui/src/icons.rs @@ -57,6 +57,9 @@ pub const LEFT_PANEL_TOGGLE: Icon = icon_from_path!("../data/icons/left_panel_to pub const MINIMIZE: Icon = icon_from_path!("../data/icons/minimize.png"); pub const MAXIMIZE: Icon = icon_from_path!("../data/icons/maximize.png"); +pub const COLLAPSE: Icon = icon_from_path!("../data/icons/collapse.png"); +pub const EXPAND: Icon = icon_from_path!("../data/icons/expand.png"); + pub const VISIBLE: Icon = icon_from_path!("../data/icons/visible.png"); pub const INVISIBLE: Icon = icon_from_path!("../data/icons/invisible.png"); diff --git a/crates/viewer/re_ui/src/list_item/list_item.rs b/crates/viewer/re_ui/src/list_item/list_item.rs index f004bede9a7e..a6805054a46e 100644 --- a/crates/viewer/re_ui/src/list_item/list_item.rs +++ b/crates/viewer/re_ui/src/list_item/list_item.rs @@ -103,7 +103,7 @@ impl ListItem { /// Highlight the item as the current drop target. /// /// Use this while dragging, to highlight which container will receive the drop at any given time. - /// **Note**: this flag has otherwise no behavioural effect. It's up to the caller to set it when the item is + /// **Note**: this flag has otherwise no behavioral effect. It's up to the caller to set it when the item is /// being hovered (or otherwise selected as drop target) while a drag is in progress. #[inline] pub fn drop_target_style(mut self, drag_target: bool) -> Self { diff --git a/tests/python/chunk_zoo/chunk_zoo.py b/tests/python/chunk_zoo/chunk_zoo.py index b6e760131c3b..347a406595e0 100644 --- a/tests/python/chunk_zoo/chunk_zoo.py +++ b/tests/python/chunk_zoo/chunk_zoo.py @@ -11,6 +11,7 @@ import argparse from typing import Sequence +import numpy as np import rerun as rr import rerun.components as rrc @@ -114,6 +115,37 @@ def specimen_archetype_without_clamp_join_semantics(): ) +def specimen_many_rows_with_mismatched_instance_count(): + """Points2D across many timestamps with varying and mismatch instance counts.""" + + # Useful for dataframe view row expansion testing. + + np.random.seed(0) + positions_partitions = np.random.randint( + 3, + 15, + size=100, + ) + batch_size = np.sum(positions_partitions) + + # Shuffle the color partitions to induce the mismatch + colors_partitions = positions_partitions.copy() + np.random.shuffle(colors_partitions) + + positions = np.random.rand(batch_size, 2) + colors = np.random.randint(0, 255, size=(batch_size, 4)) + + rr.send_columns( + "/many_rows_with_mismatched_instance_count", + times(range(len(positions_partitions))), + [ + rrc.Position2DBatch(positions).partition(positions_partitions), + rrc.ColorBatch(colors).partition(colors_partitions), + ], + ) + rr.log_components("/many_rows_with_mismatched_instance_count", [rr.Points2D.indicator()], static=True) + + # TODO(ab): add variants (unordered, overlapping timestamps, etc.) def specimen_scalars_interlaced_in_two_chunks(): """Scalar column stored in two chunks, with interlaced timestamps."""