From fc944fd0edba1f0e72b8595287cded02ec5c603c Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 26 Sep 2024 17:04:58 +0200 Subject: [PATCH 01/10] [dfv2] WIP: wrapper and UI for the new query blueprint --- crates/viewer/re_component_ui/src/timeline.rs | 1 + .../viewer/re_space_view_dataframe/src/lib.rs | 1 + .../src/space_view_class.rs | 9 +- .../src/view_query_v2.rs | 484 ++++++++++++++++++ .../src/view_properties.rs | 5 + 5 files changed, 498 insertions(+), 2 deletions(-) create mode 100644 crates/viewer/re_space_view_dataframe/src/view_query_v2.rs diff --git a/crates/viewer/re_component_ui/src/timeline.rs b/crates/viewer/re_component_ui/src/timeline.rs index df8aeca7581a..0c217c59c221 100644 --- a/crates/viewer/re_component_ui/src/timeline.rs +++ b/crates/viewer/re_component_ui/src/timeline.rs @@ -3,6 +3,7 @@ use re_types_core::LoggableBatch as _; use re_viewer_context::external::re_log_types::TimelineName; use re_viewer_context::{MaybeMutRef, ViewerContext}; +//TODO(#7498): might be unneeded after the dataframe view update pub(crate) fn edit_timeline_name( ctx: &ViewerContext<'_>, ui: &mut egui::Ui, diff --git a/crates/viewer/re_space_view_dataframe/src/lib.rs b/crates/viewer/re_space_view_dataframe/src/lib.rs index e25c7b8ed0c8..5ceda1054647 100644 --- a/crates/viewer/re_space_view_dataframe/src/lib.rs +++ b/crates/viewer/re_space_view_dataframe/src/lib.rs @@ -8,6 +8,7 @@ mod expanded_rows; mod query_kind; mod space_view_class; mod view_query; +mod view_query_v2; mod visualizer_system; pub use space_view_class::DataframeSpaceView; 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 609d8c370bf8..695dda14f830 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 @@ -15,7 +15,7 @@ use re_viewport_blueprint::{SpaceViewContents, ViewProperty}; use crate::dataframe_ui::HideColumnAction; use crate::{ dataframe_ui::dataframe_ui, expanded_rows::ExpandedRowsCache, query_kind::QueryKind, - visualizer_system::EmptySystem, + view_query_v2, visualizer_system::EmptySystem, }; #[derive(Default)] @@ -110,7 +110,12 @@ mode sets the default time range to _everything_. You can override this in the s _space_origin: &EntityPath, space_view_id: SpaceViewId, ) -> Result<(), SpaceViewSystemExecutionError> { - crate::view_query::query_ui(ctx, ui, state, space_view_id) + crate::view_query::query_ui(ctx, ui, state, space_view_id)?; + + //TODO(ab): just display the UI for now, this has no effect on the view itself yet. + ui.separator(); + let view_query = view_query_v2::QueryV2::from_blueprint(ctx, space_view_id); + view_query.selection_panel_ui(ctx, ui, space_view_id) } fn extra_title_bar_ui( diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs new file mode 100644 index 000000000000..dc1f2824dce1 --- /dev/null +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs @@ -0,0 +1,484 @@ +use std::collections::BTreeSet; + +use re_log_types::{ + EntityPath, ResolvedTimeRange, TimeInt, TimeType, TimeZone, Timeline, TimelineName, +}; +use re_types::blueprint::{archetypes, components, datatypes}; +use re_types_core::{ComponentName, ComponentNameSet}; +use re_ui::{list_item, UiExt}; +use re_viewer_context::{SpaceViewId, SpaceViewSystemExecutionError, TimeDragValue, ViewerContext}; +use re_viewport_blueprint::ViewProperty; + +/// Struct to hold the point-of-view column used for the filter by event. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub(crate) struct EventColumn { + pub(crate) entity_path: EntityPath, + pub(crate) component_name: ComponentName, +} + +/// Wrapper over the `DataframeQueryV2` blueprint archetype that can also display some UI. +pub(crate) struct QueryV2 { + query_property: ViewProperty, +} + +// Ctor and accessors +impl QueryV2 { + pub(crate) fn from_blueprint(ctx: &ViewerContext<'_>, space_view_id: SpaceViewId) -> Self { + Self { + query_property: ViewProperty::from_archetype::( + ctx.blueprint_db(), + ctx.blueprint_query, + space_view_id, + ), + } + } + + /// Get the query timeline. + /// + /// This tries to read the timeline name from the blueprint. If missing or invalid, the current + /// timeline is used and saved back to the blueprint. + pub(crate) fn timeline( + &self, + ctx: &ViewerContext<'_>, + ) -> Result { + // read the timeline and make sure it actually exists + let timeline = self + .query_property + .component_or_empty::()? + .and_then(|name| { + ctx.recording() + .timelines() + .find(|timeline| timeline.name() == &TimelineName::from(name.as_str())) + .copied() + }); + + // if the timeline is unset, we "freeze" it to the current time panel timeline + let save_timeline = timeline.is_none(); + let timeline = timeline.unwrap_or_else(|| *ctx.rec_cfg.time_ctrl.read().timeline()); + if save_timeline { + self.set_timeline_name(ctx, timeline.name()); + } + + Ok(timeline) + } + + /// Save the timeline to the one specified. + /// + /// Note: this resets the range filter timestamps to -inf/+inf as any other value might be + /// invalidated. + fn set_timeline_name(&self, ctx: &ViewerContext<'_>, timeline_name: &TimelineName) { + self.query_property + .save_blueprint_component(ctx, &components::TimelineName::from(timeline_name.as_str())); + + // clearing the range filter is equivalent to setting it to the default -inf/+inf + self.query_property + .clear_blueprint_component::(ctx); + } + + pub(crate) fn range_filter(&self) -> Result<(TimeInt, TimeInt), SpaceViewSystemExecutionError> { + #[allow(clippy::map_unwrap_or)] + Ok(self + .query_property + .component_or_empty::()? + .map(|range_filter| (range_filter.start.into(), range_filter.end.into())) + .unwrap_or((TimeInt::MIN, TimeInt::MAX))) + } + + fn set_range_filter(&self, ctx: &ViewerContext<'_>, start: TimeInt, end: TimeInt) { + if (start, end) == (TimeInt::MIN, TimeInt::MAX) { + self.query_property + .clear_blueprint_component::(ctx); + } else { + self.query_property + .save_blueprint_component(ctx, &components::RangeFilter::new(start, end)); + } + } + + pub(crate) fn filter_by_event_active(&self) -> Result { + Ok(self + .query_property + .component_or_empty::()? + .map_or(false, |comp| *comp.0)) + } + + fn set_filter_by_event_active(&self, ctx: &ViewerContext<'_>, active: bool) { + self.query_property + .save_blueprint_component(ctx, &components::FilterByEventActive(active.into())); + } + + pub(crate) fn filter_event_column( + &self, + ) -> Result, SpaceViewSystemExecutionError> { + Ok(self + .query_property + .component_or_empty::()? + .map(|comp| { + let components::ComponentColumnSelector(datatypes::ComponentColumnSelector { + entity_path, + component, + }) = comp; + + EventColumn { + entity_path: EntityPath::from(entity_path.as_str()), + component_name: ComponentName::from(component.as_str()), + } + })) + } + + fn set_filter_event_column(&self, ctx: &ViewerContext<'_>, event_column: EventColumn) { + let EventColumn { + entity_path, + component_name, + } = event_column; + + let component = components::ComponentColumnSelector::new(&entity_path, component_name); + + self.query_property + .save_blueprint_component(ctx, &component); + } +} + +// UI +impl QueryV2 { + /// Display the selection panel ui for this query. + pub(crate) fn selection_panel_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + space_view_id: SpaceViewId, + ) -> Result<(), SpaceViewSystemExecutionError> { + let timeline = self.timeline(ctx)?; + + self.timeline_ui(ctx, ui, &timeline)?; + self.filter_range_ui(ctx, ui, &timeline)?; + self.filter_event_ui(ctx, ui, &timeline, space_view_id)?; + + Ok(()) + } + + fn timeline_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + timeline: &Timeline, + ) -> Result<(), SpaceViewSystemExecutionError> { + let mut timeline_name = *timeline.name(); + egui::Grid::new("dataframe_view_query_ui_timeline") + .num_columns(2) + .spacing(egui::vec2(8.0, 10.0)) + .show(ui, |ui| -> Result<_, SpaceViewSystemExecutionError> { + ui.grid_left_hand_label("Timeline"); + + if edit_timeline_name(ctx, ui, &mut timeline_name).changed() { + self.set_timeline_name(ctx, &timeline_name); + } + + Ok(()) + }) + .inner + } + + fn filter_range_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + timeline: &Timeline, + ) -> Result<(), SpaceViewSystemExecutionError> { + let time_drag_value = if let Some(times) = ctx.recording().time_histogram(timeline) { + TimeDragValue::from_time_histogram(times) + } else { + // This should never happen because `timeline` is guaranteed to be valid by `Self::timeline()` + TimeDragValue::from_time_range(0..=0) + }; + + ui.label("Filter rows by time range:"); + let (mut start, mut end) = self.range_filter()?; + + let mut changed = false; + let mut should_display_time_range = false; + list_item::list_item_scope(ui, "dataframe_view_query_ui_range_filter", |ui| { + let mut reset_start = false; + + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("Start") + .action_button_with_enabled(&re_ui::icons::RESET, start != TimeInt::MIN, || { + reset_start = true; + }) + .value_fn(|ui, _| { + let response = time_boundary_ui( + ui, + &time_drag_value, + None, + timeline.typ(), + ctx.app_options.time_zone, + &mut start, + ); + + changed |= response.changed(); + should_display_time_range |= + response.hovered() || response.dragged() || response.has_focus(); + }), + ); + + if reset_start { + start = TimeInt::MIN; + changed = true; + } + + let mut reset_to = false; + + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("End") + .action_button_with_enabled(&re_ui::icons::RESET, end != TimeInt::MAX, || { + reset_to = true; + }) + .value_fn(|ui, _| { + let response = time_boundary_ui( + ui, + &time_drag_value, + Some(start), + timeline.typ(), + ctx.app_options.time_zone, + &mut end, + ); + + changed |= response.changed(); + should_display_time_range |= + response.hovered() || response.dragged() || response.has_focus(); + }), + ); + + if reset_to { + end = TimeInt::MAX; + changed = true; + } + }); + + if changed { + self.set_range_filter(ctx, start, end); + } + + if should_display_time_range { + let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); + if time_ctrl.timeline() == timeline { + time_ctrl.highlighted_range = Some(ResolvedTimeRange::new(start, end)); + } + } + + Ok(()) + } + + fn filter_event_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + timeline: &Timeline, + space_view_id: SpaceViewId, + ) -> Result<(), SpaceViewSystemExecutionError> { + // + // Read stuff + // + + let mut filter_by_event_active = self.filter_by_event_active()?; + + let original_event_column = self.filter_event_column()?; + let (event_entity, event_component) = + original_event_column.clone().map_or((None, None), |col| { + (Some(col.entity_path), Some(col.component_name)) + }); + + // + // Filter active? + // + + if ui + .re_checkbox(&mut filter_by_event_active, "Filter by event from:") + .changed() + { + self.set_filter_by_event_active(ctx, filter_by_event_active); + } + + // + // Event entity + // + + let all_entities = all_pov_entities_for_space_view(ctx, space_view_id, timeline); + + let mut event_entity = event_entity + .and_then(|entity| all_entities.contains(&entity).then_some(entity)) + .or_else(|| all_entities.iter().next().cloned()) + .unwrap_or_else(|| EntityPath::from("/")); + + ui.add_enabled_ui(filter_by_event_active, |ui| { + ui.list_item_flat_noninteractive(list_item::PropertyContent::new("Entity").value_fn( + |ui, _| { + egui::ComboBox::new("pov_entity", "") + .selected_text(event_entity.to_string()) + .show_ui(ui, |ui| { + for entity in all_entities { + let label = entity.to_string(); + ui.selectable_value(&mut event_entity, entity, label); + } + }); + }, + )); + }); + + // + // Event component + // + + let all_components = ctx + .recording_store() + .all_components_on_timeline(timeline, &event_entity) + .unwrap_or_default(); + + // The list of suggested components is build as follows: + // - consider all indicator components + // - for the matching archetypes, take all required components + // - keep those that are actually present + let suggested_components = || { + all_components + .iter() + .filter_map(|c| { + c.indicator_component_archetype() + .and_then(|archetype_short_name| { + ctx.reflection + .archetype_reflection_from_short_name(&archetype_short_name) + }) + }) + .flat_map(|archetype_reflection| { + archetype_reflection + .required_fields() + .map(|field| field.component_name) + }) + .filter(|c| all_components.contains(c)) + .collect::() + }; + + // If the currently saved component, we auto-switch it to a reasonable one. + let mut event_component = event_component + .and_then(|component| all_components.contains(&component).then_some(component)) + .or_else(|| suggested_components().first().copied()) + .unwrap_or_else(|| ComponentName::from("-")); + + ui.add_enabled_ui(filter_by_event_active, |ui| { + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("Component").value_fn(|ui, _| { + egui::ComboBox::new("pov_component", "") + .selected_text(event_component.short_name()) + .show_ui(ui, |ui| { + for component in all_components { + let label = component.short_name(); + ui.selectable_value(&mut event_component, component, label); + } + }); + }), + ); + }); + + // + // Save event if changed + // + + let event_column = EventColumn { + entity_path: event_entity, + component_name: event_component, + }; + + if original_event_column.as_ref() != Some(&event_column) { + self.set_filter_event_column(ctx, event_column); + } + + Ok(()) + } +} + +/// Gather all entities that can meaningfully be used as point-of-view for this view. +/// +/// Meaning: +/// - the entity is part of this view +/// - the entity has any component on the chosen timeline +fn all_pov_entities_for_space_view( + ctx: &ViewerContext<'_>, + space_view_id: SpaceViewId, + timeline: &Timeline, +) -> BTreeSet { + let mut all_entities = BTreeSet::new(); + ctx.lookup_query_result(space_view_id) + .tree + .visit(&mut |node| { + if !node.data_result.tree_prefix_only { + let comp_for_entity = ctx + .recording_store() + .all_components_on_timeline(timeline, &node.data_result.entity_path); + if comp_for_entity.is_some_and(|components| !components.is_empty()) { + all_entities.insert(node.data_result.entity_path.clone()); + } + } + true + }); + + all_entities +} + +fn time_boundary_ui( + ui: &mut egui::Ui, + time_drag_value: &TimeDragValue, + low_bound_override: Option, + timeline_typ: TimeType, + time_zone: TimeZone, + time: &mut TimeInt, +) -> egui::Response { + if *time == TimeInt::MAX { + let mut response = ui.button("+∞").on_hover_text("Click to edit"); + if response.clicked() { + *time = time_drag_value.max_time(); + response.mark_changed(); + } + response + } else if *time == TimeInt::MIN { + let mut response = ui.button("–∞").on_hover_text("Click to edit"); + if response.clicked() { + *time = time_drag_value.min_time(); + response.mark_changed(); + } + response + } else { + match timeline_typ { + TimeType::Time => { + time_drag_value + .temporal_drag_value_ui(ui, time, true, low_bound_override, time_zone) + .0 + } + + TimeType::Sequence => { + time_drag_value.sequence_drag_value_ui(ui, time, true, low_bound_override) + } + } + } +} + +fn edit_timeline_name( + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + value: &mut TimelineName, +) -> egui::Response { + let mut changed = false; + let mut combobox_response = egui::ComboBox::from_id_salt(&value) + .selected_text(value.as_str()) + .show_ui(ui, |ui| { + for timeline in ctx.recording().timelines() { + let response = + ui.selectable_value(value, *timeline.name(), timeline.name().as_str()); + + changed |= response.changed(); + } + }); + + if changed { + combobox_response.response.mark_changed(); + } + + combobox_response.response +} diff --git a/crates/viewer/re_viewport_blueprint/src/view_properties.rs b/crates/viewer/re_viewport_blueprint/src/view_properties.rs index d5dfb4d9d24c..2894c50b6adb 100644 --- a/crates/viewer/re_viewport_blueprint/src/view_properties.rs +++ b/crates/viewer/re_viewport_blueprint/src/view_properties.rs @@ -175,6 +175,11 @@ impl ViewProperty { ctx.save_blueprint_component(&self.blueprint_store_path, components); } + /// Clears a blueprint component. + pub fn clear_blueprint_component(&self, ctx: &ViewerContext<'_>) { + ctx.clear_blueprint_component_by_name(&self.blueprint_store_path, C::name()); + } + /// Resets a blueprint component to the value it had in the default blueprint. pub fn reset_blueprint_component(&self, ctx: &ViewerContext<'_>) { ctx.reset_blueprint_component_by_name(&self.blueprint_store_path, C::name()); From e0197dd41d59af7c6d60d7ac82716b0ae1e0eb5d Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 26 Sep 2024 18:54:32 +0200 Subject: [PATCH 02/10] Add UI for latest-at on/off --- .../src/view_query_v2.rs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs index dc1f2824dce1..e3e519da876a 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs @@ -136,6 +136,18 @@ impl QueryV2 { self.query_property .save_blueprint_component(ctx, &component); } + + pub(crate) fn latest_at(&self) -> Result { + Ok(self + .query_property + .component_or_empty::()? + .map_or(false, |comp| *comp.0)) + } + + pub(crate) fn set_latest_at(&self, ctx: &ViewerContext<'_>, latest_at: bool) { + self.query_property + .save_blueprint_component(ctx, &components::ApplyLatestAt(latest_at.into())); + } } // UI @@ -150,8 +162,12 @@ impl QueryV2 { let timeline = self.timeline(ctx)?; self.timeline_ui(ctx, ui, &timeline)?; + ui.separator(); self.filter_range_ui(ctx, ui, &timeline)?; + ui.separator(); self.filter_event_ui(ctx, ui, &timeline, space_view_id)?; + ui.separator(); + self.latest_at_ui(ctx, ui)?; Ok(()) } @@ -392,6 +408,29 @@ impl QueryV2 { Ok(()) } + + fn latest_at_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + ) -> Result<(), SpaceViewSystemExecutionError> { + ui.label("Empty cells:"); + + let mut latest_at = self.latest_at()?; + let changed = { + ui.re_radio_value(&mut latest_at, false, "Leave empty") + .changed() + } | { + ui.re_radio_value(&mut latest_at, true, "Fill with latest-at values") + .changed() + }; + + if changed { + self.set_latest_at(ctx, latest_at); + } + + Ok(()) + } } /// Gather all entities that can meaningfully be used as point-of-view for this view. From 7fbc7b6751fe0dc1101bca4616b064bce6ea6626 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 09:39:33 +0200 Subject: [PATCH 03/10] Refactor in multiple files + stub for column visibility UI --- .../src/space_view_class.rs | 8 +- .../src/view_query_v2/blueprint_io.rs | 128 ++++++++++++ .../src/view_query_v2/mod.rs | 61 ++++++ .../{view_query_v2.rs => view_query_v2/ui.rs} | 184 ++---------------- 4 files changed, 213 insertions(+), 168 deletions(-) create mode 100644 crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs create mode 100644 crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs rename crates/viewer/re_space_view_dataframe/src/{view_query_v2.rs => view_query_v2/ui.rs} (67%) 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 695dda14f830..a8c6aa42fe5c 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 @@ -114,8 +114,14 @@ mode sets the default time range to _everything_. You can override this in the s //TODO(ab): just display the UI for now, this has no effect on the view itself yet. ui.separator(); + let state = state.downcast_mut::()?; let view_query = view_query_v2::QueryV2::from_blueprint(ctx, space_view_id); - view_query.selection_panel_ui(ctx, ui, space_view_id) + let Some(schema) = &state.schema else { + // Shouldn't happen, except maybe on the first frame, which is too early + // for the user to click the menu anyway. + return Ok(()); + }; + view_query.selection_panel_ui(ctx, ui, space_view_id, schema) } fn extra_title_bar_ui( diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs new file mode 100644 index 000000000000..607cac8e3355 --- /dev/null +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs @@ -0,0 +1,128 @@ +use crate::view_query_v2::{EventColumn, QueryV2}; +use re_log_types::{EntityPath, TimeInt, TimelineName}; +use re_types::blueprint::{components, datatypes}; +use re_types_core::ComponentName; +use re_viewer_context::{SpaceViewSystemExecutionError, ViewerContext}; + +// Accessors wrapping reads/writes to the blueprint store. +impl QueryV2 { + /// Get the query timeline. + /// + /// This tries to read the timeline name from the blueprint. If missing or invalid, the current + /// timeline is used and saved back to the blueprint. + pub(crate) fn timeline( + &self, + ctx: &ViewerContext<'_>, + ) -> Result { + // read the timeline and make sure it actually exists + let timeline = self + .query_property + .component_or_empty::()? + .and_then(|name| { + ctx.recording() + .timelines() + .find(|timeline| timeline.name() == &TimelineName::from(name.as_str())) + .copied() + }); + + // if the timeline is unset, we "freeze" it to the current time panel timeline + let save_timeline = timeline.is_none(); + let timeline = timeline.unwrap_or_else(|| *ctx.rec_cfg.time_ctrl.read().timeline()); + if save_timeline { + self.set_timeline_name(ctx, timeline.name()); + } + + Ok(timeline) + } + + /// Save the timeline to the one specified. + /// + /// Note: this resets the range filter timestamps to -inf/+inf as any other value might be + /// invalidated. + pub(super) fn set_timeline_name(&self, ctx: &ViewerContext<'_>, timeline_name: &TimelineName) { + self.query_property + .save_blueprint_component(ctx, &components::TimelineName::from(timeline_name.as_str())); + + // clearing the range filter is equivalent to setting it to the default -inf/+inf + self.query_property + .clear_blueprint_component::(ctx); + } + + pub(crate) fn range_filter(&self) -> Result<(TimeInt, TimeInt), SpaceViewSystemExecutionError> { + #[allow(clippy::map_unwrap_or)] + Ok(self + .query_property + .component_or_empty::()? + .map(|range_filter| (range_filter.start.into(), range_filter.end.into())) + .unwrap_or((TimeInt::MIN, TimeInt::MAX))) + } + + pub(super) fn set_range_filter(&self, ctx: &ViewerContext<'_>, start: TimeInt, end: TimeInt) { + if (start, end) == (TimeInt::MIN, TimeInt::MAX) { + self.query_property + .clear_blueprint_component::(ctx); + } else { + self.query_property + .save_blueprint_component(ctx, &components::RangeFilter::new(start, end)); + } + } + + pub(crate) fn filter_by_event_active(&self) -> Result { + Ok(self + .query_property + .component_or_empty::()? + .map_or(false, |comp| *comp.0)) + } + + pub(super) fn set_filter_by_event_active(&self, ctx: &ViewerContext<'_>, active: bool) { + self.query_property + .save_blueprint_component(ctx, &components::FilterByEventActive(active.into())); + } + + pub(crate) fn filter_event_column( + &self, + ) -> Result, SpaceViewSystemExecutionError> { + Ok(self + .query_property + .component_or_empty::()? + .map(|comp| { + let components::ComponentColumnSelector(datatypes::ComponentColumnSelector { + entity_path, + component, + }) = comp; + + EventColumn { + entity_path: EntityPath::from(entity_path.as_str()), + component_name: ComponentName::from(component.as_str()), + } + })) + } + + pub(super) fn set_filter_event_column( + &self, + ctx: &ViewerContext<'_>, + event_column: EventColumn, + ) { + let EventColumn { + entity_path, + component_name, + } = event_column; + + let component = components::ComponentColumnSelector::new(&entity_path, component_name); + + self.query_property + .save_blueprint_component(ctx, &component); + } + + pub(crate) fn latest_at(&self) -> Result { + Ok(self + .query_property + .component_or_empty::()? + .map_or(false, |comp| *comp.0)) + } + + pub(crate) fn set_latest_at(&self, ctx: &ViewerContext<'_>, latest_at: bool) { + self.query_property + .save_blueprint_component(ctx, &components::ApplyLatestAt(latest_at.into())); + } +} diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs new file mode 100644 index 000000000000..ec5cbfc15c8e --- /dev/null +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs @@ -0,0 +1,61 @@ +mod blueprint_io; +mod ui; + +use re_chunk_store::ColumnDescriptor; +use re_log_types::EntityPath; +use re_types::blueprint::archetypes; +use re_types_core::ComponentName; +use re_viewer_context::{SpaceViewId, SpaceViewSystemExecutionError, ViewerContext}; +use re_viewport_blueprint::ViewProperty; + +/// Struct to hold the point-of-view column used for the filter by event. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub(crate) struct EventColumn { + pub(crate) entity_path: EntityPath, + pub(crate) component_name: ComponentName, +} + +/// Wrapper over the `DataframeQueryV2` blueprint archetype that can also display some UI. +pub(crate) struct QueryV2 { + query_property: ViewProperty, +} + +impl QueryV2 { + /// Create a query object from the blueprint store. + /// + /// See the `blueprint_io` module for more related accessors. + pub(crate) fn from_blueprint(ctx: &ViewerContext<'_>, space_view_id: SpaceViewId) -> Self { + Self { + query_property: ViewProperty::from_archetype::( + ctx.blueprint_db(), + ctx.blueprint_query, + space_view_id, + ), + } + } + + /// Display the selection panel ui for this query. + /// + /// Implementation is in the `ui` module. + pub(crate) fn selection_panel_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + space_view_id: SpaceViewId, + schema: &[ColumnDescriptor], + ) -> Result<(), SpaceViewSystemExecutionError> { + let timeline = self.timeline(ctx)?; + + self.timeline_ui(ctx, ui, &timeline)?; + ui.separator(); + self.filter_range_ui(ctx, ui, &timeline)?; + ui.separator(); + self.filter_event_ui(ctx, ui, &timeline, space_view_id)?; + ui.separator(); + self.column_visibility_ui(ctx, ui, &timeline, schema)?; + ui.separator(); + self.latest_at_ui(ctx, ui)?; + + Ok(()) + } +} diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs similarity index 67% rename from crates/viewer/re_space_view_dataframe/src/view_query_v2.rs rename to crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs index e3e519da876a..da3351aedbf1 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs @@ -1,178 +1,18 @@ use std::collections::BTreeSet; +use re_chunk_store::ColumnDescriptor; use re_log_types::{ EntityPath, ResolvedTimeRange, TimeInt, TimeType, TimeZone, Timeline, TimelineName, }; -use re_types::blueprint::{archetypes, components, datatypes}; use re_types_core::{ComponentName, ComponentNameSet}; use re_ui::{list_item, UiExt}; use re_viewer_context::{SpaceViewId, SpaceViewSystemExecutionError, TimeDragValue, ViewerContext}; -use re_viewport_blueprint::ViewProperty; -/// Struct to hold the point-of-view column used for the filter by event. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub(crate) struct EventColumn { - pub(crate) entity_path: EntityPath, - pub(crate) component_name: ComponentName, -} - -/// Wrapper over the `DataframeQueryV2` blueprint archetype that can also display some UI. -pub(crate) struct QueryV2 { - query_property: ViewProperty, -} - -// Ctor and accessors -impl QueryV2 { - pub(crate) fn from_blueprint(ctx: &ViewerContext<'_>, space_view_id: SpaceViewId) -> Self { - Self { - query_property: ViewProperty::from_archetype::( - ctx.blueprint_db(), - ctx.blueprint_query, - space_view_id, - ), - } - } - - /// Get the query timeline. - /// - /// This tries to read the timeline name from the blueprint. If missing or invalid, the current - /// timeline is used and saved back to the blueprint. - pub(crate) fn timeline( - &self, - ctx: &ViewerContext<'_>, - ) -> Result { - // read the timeline and make sure it actually exists - let timeline = self - .query_property - .component_or_empty::()? - .and_then(|name| { - ctx.recording() - .timelines() - .find(|timeline| timeline.name() == &TimelineName::from(name.as_str())) - .copied() - }); - - // if the timeline is unset, we "freeze" it to the current time panel timeline - let save_timeline = timeline.is_none(); - let timeline = timeline.unwrap_or_else(|| *ctx.rec_cfg.time_ctrl.read().timeline()); - if save_timeline { - self.set_timeline_name(ctx, timeline.name()); - } - - Ok(timeline) - } - - /// Save the timeline to the one specified. - /// - /// Note: this resets the range filter timestamps to -inf/+inf as any other value might be - /// invalidated. - fn set_timeline_name(&self, ctx: &ViewerContext<'_>, timeline_name: &TimelineName) { - self.query_property - .save_blueprint_component(ctx, &components::TimelineName::from(timeline_name.as_str())); - - // clearing the range filter is equivalent to setting it to the default -inf/+inf - self.query_property - .clear_blueprint_component::(ctx); - } - - pub(crate) fn range_filter(&self) -> Result<(TimeInt, TimeInt), SpaceViewSystemExecutionError> { - #[allow(clippy::map_unwrap_or)] - Ok(self - .query_property - .component_or_empty::()? - .map(|range_filter| (range_filter.start.into(), range_filter.end.into())) - .unwrap_or((TimeInt::MIN, TimeInt::MAX))) - } - - fn set_range_filter(&self, ctx: &ViewerContext<'_>, start: TimeInt, end: TimeInt) { - if (start, end) == (TimeInt::MIN, TimeInt::MAX) { - self.query_property - .clear_blueprint_component::(ctx); - } else { - self.query_property - .save_blueprint_component(ctx, &components::RangeFilter::new(start, end)); - } - } - - pub(crate) fn filter_by_event_active(&self) -> Result { - Ok(self - .query_property - .component_or_empty::()? - .map_or(false, |comp| *comp.0)) - } - - fn set_filter_by_event_active(&self, ctx: &ViewerContext<'_>, active: bool) { - self.query_property - .save_blueprint_component(ctx, &components::FilterByEventActive(active.into())); - } - - pub(crate) fn filter_event_column( - &self, - ) -> Result, SpaceViewSystemExecutionError> { - Ok(self - .query_property - .component_or_empty::()? - .map(|comp| { - let components::ComponentColumnSelector(datatypes::ComponentColumnSelector { - entity_path, - component, - }) = comp; - - EventColumn { - entity_path: EntityPath::from(entity_path.as_str()), - component_name: ComponentName::from(component.as_str()), - } - })) - } - - fn set_filter_event_column(&self, ctx: &ViewerContext<'_>, event_column: EventColumn) { - let EventColumn { - entity_path, - component_name, - } = event_column; - - let component = components::ComponentColumnSelector::new(&entity_path, component_name); - - self.query_property - .save_blueprint_component(ctx, &component); - } - - pub(crate) fn latest_at(&self) -> Result { - Ok(self - .query_property - .component_or_empty::()? - .map_or(false, |comp| *comp.0)) - } +use crate::view_query_v2::{EventColumn, QueryV2}; - pub(crate) fn set_latest_at(&self, ctx: &ViewerContext<'_>, latest_at: bool) { - self.query_property - .save_blueprint_component(ctx, &components::ApplyLatestAt(latest_at.into())); - } -} - -// UI +// UI implementation impl QueryV2 { - /// Display the selection panel ui for this query. - pub(crate) fn selection_panel_ui( - &self, - ctx: &ViewerContext<'_>, - ui: &mut egui::Ui, - space_view_id: SpaceViewId, - ) -> Result<(), SpaceViewSystemExecutionError> { - let timeline = self.timeline(ctx)?; - - self.timeline_ui(ctx, ui, &timeline)?; - ui.separator(); - self.filter_range_ui(ctx, ui, &timeline)?; - ui.separator(); - self.filter_event_ui(ctx, ui, &timeline, space_view_id)?; - ui.separator(); - self.latest_at_ui(ctx, ui)?; - - Ok(()) - } - - fn timeline_ui( + pub(super) fn timeline_ui( &self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, @@ -194,7 +34,7 @@ impl QueryV2 { .inner } - fn filter_range_ui( + pub(super) fn filter_range_ui( &self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, @@ -284,7 +124,7 @@ impl QueryV2 { Ok(()) } - fn filter_event_ui( + pub(super) fn filter_event_ui( &self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, @@ -409,7 +249,17 @@ impl QueryV2 { Ok(()) } - fn latest_at_ui( + pub(super) fn column_visibility_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + timeline: &Timeline, + schema: &[ColumnDescriptor], + ) -> Result<(), SpaceViewSystemExecutionError> { + Ok(()) + } + + pub(super) fn latest_at_ui( &self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, From acd0f754b27cc4af6a53b9a01e3f1fb408b7c41b Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 14:12:22 +0200 Subject: [PATCH 04/10] WIP column visibility ui --- .../src/space_view_class.rs | 12 +- .../src/view_query_v2/blueprint_io.rs | 102 ++++++++++- .../src/view_query_v2/mod.rs | 4 +- .../src/view_query_v2/ui.rs | 162 ++++++++++++++++-- 4 files changed, 260 insertions(+), 20 deletions(-) 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 a8c6aa42fe5c..f0b935799960 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 @@ -5,6 +5,7 @@ use re_chunk_store::{ColumnDescriptor, ColumnSelector}; use re_log_types::{EntityPath, EntityPathFilter, ResolvedTimeRange, TimelineName}; use re_types::blueprint::{archetypes, components}; use re_types_core::SpaceViewClassIdentifier; +use re_ui::modal::ModalHandler; use re_ui::UiExt as _; use re_viewer_context::{ SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId, SpaceViewState, SpaceViewStateExt, @@ -25,6 +26,9 @@ struct DataframeSpaceViewState { /// Schema for the current query, cached here for the column visibility UI. schema: Option>, + + /// Modal dialog for column visibility. + column_visibility_modal: ModalHandler, } impl SpaceViewState for DataframeSpaceViewState { @@ -121,7 +125,13 @@ mode sets the default time range to _everything_. You can override this in the s // for the user to click the menu anyway. return Ok(()); }; - view_query.selection_panel_ui(ctx, ui, space_view_id, schema) + view_query.selection_panel_ui( + ctx, + ui, + space_view_id, + schema, + &mut state.column_visibility_modal, + ) } fn extra_title_bar_ui( diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs index 607cac8e3355..5c83ffed8b21 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs @@ -1,9 +1,13 @@ -use crate::view_query_v2::{EventColumn, QueryV2}; +use std::collections::HashSet; + +use re_chunk_store::{ColumnDescriptor, ColumnSelector}; use re_log_types::{EntityPath, TimeInt, TimelineName}; use re_types::blueprint::{components, datatypes}; use re_types_core::ComponentName; use re_viewer_context::{SpaceViewSystemExecutionError, ViewerContext}; +use crate::view_query_v2::{EventColumn, QueryV2}; + // Accessors wrapping reads/writes to the blueprint store. impl QueryV2 { /// Get the query timeline. @@ -125,4 +129,100 @@ impl QueryV2 { self.query_property .save_blueprint_component(ctx, &components::ApplyLatestAt(latest_at.into())); } + + /// Returns the currently selected columns. + /// + /// `None` means all columns are selected. + fn selected_columns( + &self, + ) -> Result, SpaceViewSystemExecutionError> { + Ok(self + .query_property + .component_or_empty::()?) + } + + pub(super) fn select_columns(&self, ctx: &ViewerContext<'_>, columns: HashSet) { + let mut selected_columns = datatypes::SelectedColumns::default(); + for column in columns { + match column { + ColumnSelector::Control(_) => {} + ColumnSelector::Time(desc) => { + selected_columns + .time_columns + .push(desc.timeline.as_str().into()); + } + ColumnSelector::Component(desc) => { + let blueprint_component_descriptor = datatypes::ComponentColumnSelector { + entity_path: (&desc.entity_path).into(), + component: desc.component.as_str().into(), + }; + + selected_columns + .component_columns + .push(blueprint_component_descriptor); + } + } + } + + self.query_property + .save_blueprint_component(ctx, &components::SelectedColumns(selected_columns)); + } + + pub(super) fn select_all_columns(&self, ctx: &ViewerContext<'_>) { + self.query_property + .clear_blueprint_component::(ctx); + } + + pub(super) fn unselect_all_columns(&self, ctx: &ViewerContext<'_>) { + self.query_property + .save_blueprint_component(ctx, &components::SelectedColumns::default()); + } + + pub(crate) fn apply_column_visibility_to_schema( + &self, + ctx: &ViewerContext<'_>, + schema: &[ColumnDescriptor], + ) -> Result>, SpaceViewSystemExecutionError> { + let selected_columns = self.selected_columns()?; + + // no selected columns means all columns are visible + let Some(datatypes::SelectedColumns { + time_columns, + component_columns, + }) = selected_columns.as_deref() + else { + return Ok(None); + }; + + let selected_time_columns: HashSet = time_columns + .iter() + .map(|timeline_name| timeline_name.as_str().into()) + .collect(); + let selected_component_columns = component_columns.iter().cloned().collect::>(); + + let query_timeline_name = *self.timeline(ctx)?.name(); + let result = schema + .iter() + .filter(|column| match column { + ColumnDescriptor::Control(_) => true, + ColumnDescriptor::Time(desc) => { + // we always include the query timeline column because we need it for the dataframe ui + desc.timeline.name() == &query_timeline_name + || selected_time_columns.contains(desc.timeline.name()) + } + ColumnDescriptor::Component(desc) => { + let blueprint_component_descriptor = components::ComponentColumnSelector::new( + &desc.entity_path, + desc.component_name, + ); + + selected_component_columns.contains(&blueprint_component_descriptor) + } + }) + .cloned() + .map(ColumnSelector::from) + .collect(); + + Ok(Some(result)) + } } diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs index ec5cbfc15c8e..1088339a72f9 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs @@ -5,6 +5,7 @@ use re_chunk_store::ColumnDescriptor; use re_log_types::EntityPath; use re_types::blueprint::archetypes; use re_types_core::ComponentName; +use re_ui::modal::ModalHandler; use re_viewer_context::{SpaceViewId, SpaceViewSystemExecutionError, ViewerContext}; use re_viewport_blueprint::ViewProperty; @@ -43,6 +44,7 @@ impl QueryV2 { ui: &mut egui::Ui, space_view_id: SpaceViewId, schema: &[ColumnDescriptor], + column_visibility_modal_handler: &mut ModalHandler, ) -> Result<(), SpaceViewSystemExecutionError> { let timeline = self.timeline(ctx)?; @@ -52,7 +54,7 @@ impl QueryV2 { ui.separator(); self.filter_event_ui(ctx, ui, &timeline, space_view_id)?; ui.separator(); - self.column_visibility_ui(ctx, ui, &timeline, schema)?; + self.column_visibility_ui(ctx, ui, &timeline, schema, column_visibility_modal_handler)?; ui.separator(); self.latest_at_ui(ctx, ui)?; diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs index da3351aedbf1..c085ca4126a5 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs @@ -1,10 +1,11 @@ -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashSet}; -use re_chunk_store::ColumnDescriptor; +use re_chunk_store::{ColumnDescriptor, ColumnSelector}; use re_log_types::{ EntityPath, ResolvedTimeRange, TimeInt, TimeType, TimeZone, Timeline, TimelineName, }; use re_types_core::{ComponentName, ComponentNameSet}; +use re_ui::modal::{Modal, ModalHandler}; use re_ui::{list_item, UiExt}; use re_viewer_context::{SpaceViewId, SpaceViewSystemExecutionError, TimeDragValue, ViewerContext}; @@ -165,21 +166,6 @@ impl QueryV2 { .or_else(|| all_entities.iter().next().cloned()) .unwrap_or_else(|| EntityPath::from("/")); - ui.add_enabled_ui(filter_by_event_active, |ui| { - ui.list_item_flat_noninteractive(list_item::PropertyContent::new("Entity").value_fn( - |ui, _| { - egui::ComboBox::new("pov_entity", "") - .selected_text(event_entity.to_string()) - .show_ui(ui, |ui| { - for entity in all_entities { - let label = entity.to_string(); - ui.selectable_value(&mut event_entity, entity, label); - } - }); - }, - )); - }); - // // Event component // @@ -218,7 +204,26 @@ impl QueryV2 { .or_else(|| suggested_components().first().copied()) .unwrap_or_else(|| ComponentName::from("-")); + // + // UI for event entity and component + // + ui.add_enabled_ui(filter_by_event_active, |ui| { + ui.spacing_mut().item_spacing.y = 0.0; + + ui.list_item_flat_noninteractive(list_item::PropertyContent::new("Entity").value_fn( + |ui, _| { + egui::ComboBox::new("pov_entity", "") + .selected_text(event_entity.to_string()) + .show_ui(ui, |ui| { + for entity in all_entities { + let label = entity.to_string(); + ui.selectable_value(&mut event_entity, entity, label); + } + }); + }, + )); + ui.list_item_flat_noninteractive( list_item::PropertyContent::new("Component").value_fn(|ui, _| { egui::ComboBox::new("pov_component", "") @@ -255,7 +260,130 @@ impl QueryV2 { ui: &mut egui::Ui, timeline: &Timeline, schema: &[ColumnDescriptor], + column_visibility_modal_handler: &mut ModalHandler, ) -> Result<(), SpaceViewSystemExecutionError> { + // Gather our selected columns. + let selected_columns: HashSet<_> = self + .apply_column_visibility_to_schema(ctx, schema)? + .map(|columns| columns.into_iter().collect()) + .unwrap_or_else(|| schema.iter().cloned().map(Into::into).collect()); + + let visible_count = selected_columns.len(); + let hidden_count = schema.len() - visible_count; + + ui.label("Columns:"); + + let visible_count_label = format!("{visible_count} visible, {} hidden", hidden_count); + ui.label(&visible_count_label); + + if ui.button("edit").clicked() { + column_visibility_modal_handler.open(); + } + + let mut new_selected_columns = selected_columns.clone(); + column_visibility_modal_handler.ui( + ui.ctx(), + || Modal::new("Column visibility"), + |ui, _| { + // + // Summary toggle + // + + let indeterminate = visible_count != 0 && hidden_count != 0; + let mut all_enabled = hidden_count == 0; + + if ui + .checkbox_indeterminate(&mut all_enabled, visible_count_label, indeterminate) + .changed() + { + if all_enabled { + self.select_all_columns(ctx); + } else { + self.unselect_all_columns(ctx); + } + } + + // + // Time columns + // + + let mut first = true; + for column in schema { + let ColumnDescriptor::Time(time_column_descriptor) = column else { + continue; + }; + + if first { + ui.label("Timelines"); + first = false; + } + + let column_selector: ColumnSelector = column.clone().into(); + + let is_query_timeline = + time_column_descriptor.timeline.name() == timeline.name(); + let is_enabled = !is_query_timeline; + let mut is_visible = + is_query_timeline || selected_columns.contains(&column_selector); + + ui.add_enabled_ui(is_enabled, |ui| { + if ui + .re_checkbox(&mut is_visible, column.short_name()) + .on_disabled_hover_text("The query timeline must always be visible") + .changed() + { + if is_visible { + new_selected_columns.insert(column_selector); + } else { + new_selected_columns.remove(&column_selector); + } + } + }); + } + + // + // Component columns + // + + let mut current_entity = None; + for column in schema { + let ColumnDescriptor::Component(component_column_descriptor) = column else { + continue; + }; + + if Some(&component_column_descriptor.entity_path) != current_entity.as_ref() { + current_entity = Some(component_column_descriptor.entity_path.clone()); + ui.label(component_column_descriptor.entity_path.to_string()); + } + + let column_selector: ColumnSelector = column.clone().into(); + let mut is_visible = selected_columns.contains(&column_selector); + + if ui + .re_checkbox(&mut is_visible, column.short_name()) + .changed() + { + if is_visible { + new_selected_columns.insert(column_selector); + } else { + new_selected_columns.remove(&column_selector); + } + } + } + }, + ); + + // save changes of column visibility + if new_selected_columns != selected_columns { + if new_selected_columns.len() == schema.len() { + // length match is a guaranteed match because the `selected_columns` sets are built + // from filtering out the scheme + self.select_all_columns(ctx); + } else { + self.select_columns(ctx, new_selected_columns); + } + } + Ok(()) } From 9663b73eb43e363e06779f902a3bdc4b93669623 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 15:04:33 +0200 Subject: [PATCH 05/10] Make `short_name()` for RowId control column return `RowId` --- crates/store/re_types_core/src/loggable.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/store/re_types_core/src/loggable.rs b/crates/store/re_types_core/src/loggable.rs index dd09ff2f60bc..cacb8400960a 100644 --- a/crates/store/re_types_core/src/loggable.rs +++ b/crates/store/re_types_core/src/loggable.rs @@ -136,6 +136,8 @@ impl ComponentName { short_name } else if let Some(short_name) = full_name.strip_prefix("rerun.components.") { short_name + } else if let Some(short_name) = full_name.strip_prefix("rerun.controls.") { + short_name } else if let Some(short_name) = full_name.strip_prefix("rerun.") { short_name } else { From bf1972d8c0367592674629b9e1822250c5391cf6 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 15:05:15 +0200 Subject: [PATCH 06/10] Column visibility: make `RowId` explicit and tweak ui --- .../src/view_query_v2/ui.rs | 188 +++++++++++------- 1 file changed, 119 insertions(+), 69 deletions(-) diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs index c085ca4126a5..62d46b8e2c00 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs @@ -274,93 +274,102 @@ impl QueryV2 { ui.label("Columns:"); let visible_count_label = format!("{visible_count} visible, {} hidden", hidden_count); - ui.label(&visible_count_label); + ui.list_item_flat_noninteractive( + list_item::LabelContent::new(&visible_count_label) + .always_show_buttons(true) + .with_buttons(|ui| { + let resp = ui.small_icon_button(&re_ui::icons::EDIT); + + if resp.clicked() { + column_visibility_modal_handler.open(); + } - if ui.button("edit").clicked() { - column_visibility_modal_handler.open(); - } + resp + }), + ); let mut new_selected_columns = selected_columns.clone(); - column_visibility_modal_handler.ui( - ui.ctx(), - || Modal::new("Column visibility"), - |ui, _| { - // - // Summary toggle - // - let indeterminate = visible_count != 0 && hidden_count != 0; - let mut all_enabled = hidden_count == 0; + let modal_ui = |ui: &mut egui::Ui| { + // + // Summary toggle + // - if ui - .checkbox_indeterminate(&mut all_enabled, visible_count_label, indeterminate) - .changed() - { - if all_enabled { - self.select_all_columns(ctx); - } else { - self.unselect_all_columns(ctx); - } + let indeterminate = visible_count != 0 && hidden_count != 0; + let mut all_enabled = hidden_count == 0; + + if ui + .checkbox_indeterminate(&mut all_enabled, visible_count_label, indeterminate) + .changed() + { + if all_enabled { + self.select_all_columns(ctx); + } else { + self.unselect_all_columns(ctx); } + } - // - // Time columns - // + ui.add_space(12.0); - let mut first = true; - for column in schema { - let ColumnDescriptor::Time(time_column_descriptor) = column else { - continue; - }; + // + // Control columns (always selected) + // - if first { - ui.label("Timelines"); - first = false; - } + let mut first = true; + for column in schema { + let ColumnDescriptor::Control(_) = column else { + continue; + }; - let column_selector: ColumnSelector = column.clone().into(); - - let is_query_timeline = - time_column_descriptor.timeline.name() == timeline.name(); - let is_enabled = !is_query_timeline; - let mut is_visible = - is_query_timeline || selected_columns.contains(&column_selector); - - ui.add_enabled_ui(is_enabled, |ui| { - if ui - .re_checkbox(&mut is_visible, column.short_name()) - .on_disabled_hover_text("The query timeline must always be visible") - .changed() - { - if is_visible { - new_selected_columns.insert(column_selector); - } else { - new_selected_columns.remove(&column_selector); - } - } - }); + if first { + ui.label("Control"); + first = false; } - // - // Component columns - // + // Control columns are always shown because: + // - now it's just `RowId` + // - the dataframe UI requires the `RowId` column to be present (for tooltips) + let is_enabled = false; + let mut is_visible = true; - let mut current_entity = None; - for column in schema { - let ColumnDescriptor::Component(component_column_descriptor) = column else { - continue; - }; + ui.add_enabled_ui(is_enabled, |ui| { + if ui + .re_checkbox(&mut is_visible, column.short_name()) + .on_disabled_hover_text("The query timeline must always be visible") + .changed() + { /* cannot happen for now */ } + }); + } - if Some(&component_column_descriptor.entity_path) != current_entity.as_ref() { - current_entity = Some(component_column_descriptor.entity_path.clone()); - ui.label(component_column_descriptor.entity_path.to_string()); - } + // + // Time columns + // - let column_selector: ColumnSelector = column.clone().into(); - let mut is_visible = selected_columns.contains(&column_selector); + let mut first = true; + for column in schema { + let ColumnDescriptor::Time(time_column_descriptor) = column else { + continue; + }; + if first { + ui.add_space(6.0); + ui.label("Timelines"); + first = false; + } + + let column_selector: ColumnSelector = column.clone().into(); + + // The query timeline is always active because it's necessary for the dataframe ui + // (for tooltips). + let is_query_timeline = time_column_descriptor.timeline.name() == timeline.name(); + let is_enabled = !is_query_timeline; + let mut is_visible = + is_query_timeline || selected_columns.contains(&column_selector); + + ui.add_enabled_ui(is_enabled, |ui| { if ui .re_checkbox(&mut is_visible, column.short_name()) + .on_disabled_hover_text("The query timeline must always be visible") .changed() { if is_visible { @@ -369,7 +378,48 @@ impl QueryV2 { new_selected_columns.remove(&column_selector); } } + }); + } + + // + // Component columns + // + + let mut current_entity = None; + for column in schema { + let ColumnDescriptor::Component(component_column_descriptor) = column else { + continue; + }; + + if Some(&component_column_descriptor.entity_path) != current_entity.as_ref() { + current_entity = Some(component_column_descriptor.entity_path.clone()); + ui.add_space(6.0); + ui.label(component_column_descriptor.entity_path.to_string()); } + + let column_selector: ColumnSelector = column.clone().into(); + let mut is_visible = selected_columns.contains(&column_selector); + + if ui + .re_checkbox(&mut is_visible, column.short_name()) + .changed() + { + if is_visible { + new_selected_columns.insert(column_selector); + } else { + new_selected_columns.remove(&column_selector); + } + } + } + }; + + column_visibility_modal_handler.ui( + ui.ctx(), + || Modal::new("Column visibility"), + |ui, _| { + egui::ScrollArea::vertical() + .auto_shrink([false, false]) + .show(ui, modal_ui) }, ); From 3b50e3e61e994c69921f0ef693080f6fdec29b40 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 15:16:03 +0200 Subject: [PATCH 07/10] Added `handle_hide_column_actions` --- .../src/view_query_v2/blueprint_io.rs | 72 ++++++++++++++++++- .../src/view_query_v2/ui.rs | 2 +- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs index 5c83ffed8b21..deeac6c9eb1a 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs @@ -1,13 +1,13 @@ use std::collections::HashSet; +use crate::dataframe_ui::HideColumnAction; +use crate::view_query_v2::{EventColumn, QueryV2}; use re_chunk_store::{ColumnDescriptor, ColumnSelector}; use re_log_types::{EntityPath, TimeInt, TimelineName}; use re_types::blueprint::{components, datatypes}; use re_types_core::ComponentName; use re_viewer_context::{SpaceViewSystemExecutionError, ViewerContext}; -use crate::view_query_v2::{EventColumn, QueryV2}; - // Accessors wrapping reads/writes to the blueprint store. impl QueryV2 { /// Get the query timeline. @@ -178,6 +178,7 @@ impl QueryV2 { .save_blueprint_component(ctx, &components::SelectedColumns::default()); } + /// Given a schema, list the columns that should be visible, according to the blueprint. pub(crate) fn apply_column_visibility_to_schema( &self, ctx: &ViewerContext<'_>, @@ -225,4 +226,71 @@ impl QueryV2 { Ok(Some(result)) } + + pub(crate) fn handle_hide_column_actions( + &self, + ctx: &ViewerContext<'_>, + schema: &[ColumnDescriptor], + actions: Vec, + ) -> Result<(), SpaceViewSystemExecutionError> { + // We are hiding some columns, so if the `SelectedColumns` component is not set (aka all + // columns are visible), we need to create a fully populated version of it. + let mut selected_columns = + self.selected_columns()? + .map(|comp| comp.0) + .unwrap_or_else(|| { + let mut selected_columns = datatypes::SelectedColumns::default(); + for column in schema { + match column { + ColumnDescriptor::Control(_) => {} + ColumnDescriptor::Time(desc) => { + selected_columns + .time_columns + .push(desc.timeline.name().as_str().into()); + } + ColumnDescriptor::Component(desc) => { + let blueprint_component_descriptor = + datatypes::ComponentColumnSelector { + entity_path: (&desc.entity_path).into(), + component: desc.component_name.as_str().into(), + }; + + selected_columns + .component_columns + .push(blueprint_component_descriptor); + } + } + } + + selected_columns + }); + + for action in actions { + match action { + HideColumnAction::HideTimeColumn { timeline_name } => { + selected_columns + .time_columns + .retain(|name| name != &timeline_name.as_str().into()); + } + + HideColumnAction::HideComponentColumn { + entity_path, + component_name, + } => { + let blueprint_component_descriptor = datatypes::ComponentColumnSelector { + entity_path: (&entity_path).into(), + component: component_name.as_str().into(), + }; + selected_columns + .component_columns + .retain(|desc| desc != &blueprint_component_descriptor); + } + } + } + + self.query_property + .save_blueprint_component(ctx, &components::SelectedColumns(selected_columns)); + + Ok(()) + } } diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs index 62d46b8e2c00..9ddddc470515 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs @@ -273,7 +273,7 @@ impl QueryV2 { ui.label("Columns:"); - let visible_count_label = format!("{visible_count} visible, {} hidden", hidden_count); + let visible_count_label = format!("{visible_count} visible, {hidden_count} hidden"); ui.list_item_flat_noninteractive( list_item::LabelContent::new(&visible_count_label) .always_show_buttons(true) From 012ea5fe323bec02c309192cdf12e3661086f845 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 15:25:15 +0200 Subject: [PATCH 08/10] Minor cleanup --- .../src/view_query_v2/blueprint_io.rs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs index deeac6c9eb1a..f2770f2c0e63 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs @@ -152,10 +152,8 @@ impl QueryV2 { .push(desc.timeline.as_str().into()); } ColumnSelector::Component(desc) => { - let blueprint_component_descriptor = datatypes::ComponentColumnSelector { - entity_path: (&desc.entity_path).into(), - component: desc.component.as_str().into(), - }; + let blueprint_component_descriptor = + datatypes::ComponentColumnSelector::new(&desc.entity_path, desc.component); selected_columns .component_columns @@ -250,10 +248,10 @@ impl QueryV2 { } ColumnDescriptor::Component(desc) => { let blueprint_component_descriptor = - datatypes::ComponentColumnSelector { - entity_path: (&desc.entity_path).into(), - component: desc.component_name.as_str().into(), - }; + datatypes::ComponentColumnSelector::new( + &desc.entity_path, + desc.component_name, + ); selected_columns .component_columns @@ -277,10 +275,9 @@ impl QueryV2 { entity_path, component_name, } => { - let blueprint_component_descriptor = datatypes::ComponentColumnSelector { - entity_path: (&entity_path).into(), - component: component_name.as_str().into(), - }; + let blueprint_component_descriptor = + datatypes::ComponentColumnSelector::new(&entity_path, component_name); + selected_columns .component_columns .retain(|desc| desc != &blueprint_component_descriptor); From e625defee9dca596b1e468a2353da96bba0acb07 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 16:16:20 +0200 Subject: [PATCH 09/10] Renames and cleanup in the query blueprint code --- .../src/view_query_v2/blueprint_io.rs | 98 +++++++------------ .../src/view_query_v2/ui.rs | 20 ++-- 2 files changed, 45 insertions(+), 73 deletions(-) diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs index f2770f2c0e63..ed97388ff83e 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/blueprint_io.rs @@ -33,7 +33,7 @@ impl QueryV2 { let save_timeline = timeline.is_none(); let timeline = timeline.unwrap_or_else(|| *ctx.rec_cfg.time_ctrl.read().timeline()); if save_timeline { - self.set_timeline_name(ctx, timeline.name()); + self.save_timeline_name(ctx, timeline.name()); } Ok(timeline) @@ -43,7 +43,7 @@ impl QueryV2 { /// /// Note: this resets the range filter timestamps to -inf/+inf as any other value might be /// invalidated. - pub(super) fn set_timeline_name(&self, ctx: &ViewerContext<'_>, timeline_name: &TimelineName) { + pub(super) fn save_timeline_name(&self, ctx: &ViewerContext<'_>, timeline_name: &TimelineName) { self.query_property .save_blueprint_component(ctx, &components::TimelineName::from(timeline_name.as_str())); @@ -61,7 +61,7 @@ impl QueryV2 { .unwrap_or((TimeInt::MIN, TimeInt::MAX))) } - pub(super) fn set_range_filter(&self, ctx: &ViewerContext<'_>, start: TimeInt, end: TimeInt) { + pub(super) fn save_range_filter(&self, ctx: &ViewerContext<'_>, start: TimeInt, end: TimeInt) { if (start, end) == (TimeInt::MIN, TimeInt::MAX) { self.query_property .clear_blueprint_component::(ctx); @@ -78,7 +78,7 @@ impl QueryV2 { .map_or(false, |comp| *comp.0)) } - pub(super) fn set_filter_by_event_active(&self, ctx: &ViewerContext<'_>, active: bool) { + pub(super) fn save_filter_by_event_active(&self, ctx: &ViewerContext<'_>, active: bool) { self.query_property .save_blueprint_component(ctx, &components::FilterByEventActive(active.into())); } @@ -102,7 +102,7 @@ impl QueryV2 { })) } - pub(super) fn set_filter_event_column( + pub(super) fn save_filter_event_column( &self, ctx: &ViewerContext<'_>, event_column: EventColumn, @@ -118,30 +118,23 @@ impl QueryV2 { .save_blueprint_component(ctx, &component); } - pub(crate) fn latest_at(&self) -> Result { + pub(crate) fn latest_at_enabled(&self) -> Result { Ok(self .query_property .component_or_empty::()? .map_or(false, |comp| *comp.0)) } - pub(crate) fn set_latest_at(&self, ctx: &ViewerContext<'_>, latest_at: bool) { + pub(crate) fn save_latest_at_enabled(&self, ctx: &ViewerContext<'_>, enabled: bool) { self.query_property - .save_blueprint_component(ctx, &components::ApplyLatestAt(latest_at.into())); + .save_blueprint_component(ctx, &components::ApplyLatestAt(enabled.into())); } - /// Returns the currently selected columns. - /// - /// `None` means all columns are selected. - fn selected_columns( + pub(super) fn save_selected_columns( &self, - ) -> Result, SpaceViewSystemExecutionError> { - Ok(self - .query_property - .component_or_empty::()?) - } - - pub(super) fn select_columns(&self, ctx: &ViewerContext<'_>, columns: HashSet) { + ctx: &ViewerContext<'_>, + columns: impl IntoIterator, + ) { let mut selected_columns = datatypes::SelectedColumns::default(); for column in columns { match column { @@ -166,12 +159,12 @@ impl QueryV2 { .save_blueprint_component(ctx, &components::SelectedColumns(selected_columns)); } - pub(super) fn select_all_columns(&self, ctx: &ViewerContext<'_>) { + pub(super) fn save_all_columns_selected(&self, ctx: &ViewerContext<'_>) { self.query_property .clear_blueprint_component::(ctx); } - pub(super) fn unselect_all_columns(&self, ctx: &ViewerContext<'_>) { + pub(super) fn save_all_columns_unselected(&self, ctx: &ViewerContext<'_>) { self.query_property .save_blueprint_component(ctx, &components::SelectedColumns::default()); } @@ -182,7 +175,9 @@ impl QueryV2 { ctx: &ViewerContext<'_>, schema: &[ColumnDescriptor], ) -> Result>, SpaceViewSystemExecutionError> { - let selected_columns = self.selected_columns()?; + let selected_columns = self + .query_property + .component_or_empty::()?; // no selected columns means all columns are visible let Some(datatypes::SelectedColumns { @@ -231,62 +226,39 @@ impl QueryV2 { schema: &[ColumnDescriptor], actions: Vec, ) -> Result<(), SpaceViewSystemExecutionError> { - // We are hiding some columns, so if the `SelectedColumns` component is not set (aka all - // columns are visible), we need to create a fully populated version of it. - let mut selected_columns = - self.selected_columns()? - .map(|comp| comp.0) - .unwrap_or_else(|| { - let mut selected_columns = datatypes::SelectedColumns::default(); - for column in schema { - match column { - ColumnDescriptor::Control(_) => {} - ColumnDescriptor::Time(desc) => { - selected_columns - .time_columns - .push(desc.timeline.name().as_str().into()); - } - ColumnDescriptor::Component(desc) => { - let blueprint_component_descriptor = - datatypes::ComponentColumnSelector::new( - &desc.entity_path, - desc.component_name, - ); - - selected_columns - .component_columns - .push(blueprint_component_descriptor); - } - } - } + if actions.is_empty() { + return Ok(()); + } - selected_columns - }); + let mut selected_columns: Vec<_> = self + .apply_column_visibility_to_schema(ctx, schema)? + .map(|columns| columns.into_iter().collect()) + .unwrap_or_else(|| schema.iter().cloned().map(Into::into).collect()); for action in actions { match action { HideColumnAction::HideTimeColumn { timeline_name } => { - selected_columns - .time_columns - .retain(|name| name != &timeline_name.as_str().into()); + selected_columns.retain(|column| match column { + ColumnSelector::Time(desc) => desc.timeline != timeline_name, + _ => true, + }); } HideColumnAction::HideComponentColumn { entity_path, component_name, } => { - let blueprint_component_descriptor = - datatypes::ComponentColumnSelector::new(&entity_path, component_name); - - selected_columns - .component_columns - .retain(|desc| desc != &blueprint_component_descriptor); + selected_columns.retain(|column| match column { + ColumnSelector::Component(desc) => { + desc.entity_path != entity_path || desc.component != component_name + } + _ => true, + }); } } } - self.query_property - .save_blueprint_component(ctx, &components::SelectedColumns(selected_columns)); + self.save_selected_columns(ctx, selected_columns); Ok(()) } diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs index 9ddddc470515..189c2a025bad 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs @@ -27,7 +27,7 @@ impl QueryV2 { ui.grid_left_hand_label("Timeline"); if edit_timeline_name(ctx, ui, &mut timeline_name).changed() { - self.set_timeline_name(ctx, &timeline_name); + self.save_timeline_name(ctx, &timeline_name); } Ok(()) @@ -112,7 +112,7 @@ impl QueryV2 { }); if changed { - self.set_range_filter(ctx, start, end); + self.save_range_filter(ctx, start, end); } if should_display_time_range { @@ -152,7 +152,7 @@ impl QueryV2 { .re_checkbox(&mut filter_by_event_active, "Filter by event from:") .changed() { - self.set_filter_by_event_active(ctx, filter_by_event_active); + self.save_filter_by_event_active(ctx, filter_by_event_active); } // @@ -248,7 +248,7 @@ impl QueryV2 { }; if original_event_column.as_ref() != Some(&event_column) { - self.set_filter_event_column(ctx, event_column); + self.save_filter_event_column(ctx, event_column); } Ok(()) @@ -303,9 +303,9 @@ impl QueryV2 { .changed() { if all_enabled { - self.select_all_columns(ctx); + self.save_all_columns_selected(ctx); } else { - self.unselect_all_columns(ctx); + self.save_all_columns_unselected(ctx); } } @@ -428,9 +428,9 @@ impl QueryV2 { if new_selected_columns.len() == schema.len() { // length match is a guaranteed match because the `selected_columns` sets are built // from filtering out the scheme - self.select_all_columns(ctx); + self.save_all_columns_selected(ctx); } else { - self.select_columns(ctx, new_selected_columns); + self.save_selected_columns(ctx, new_selected_columns); } } @@ -444,7 +444,7 @@ impl QueryV2 { ) -> Result<(), SpaceViewSystemExecutionError> { ui.label("Empty cells:"); - let mut latest_at = self.latest_at()?; + let mut latest_at = self.latest_at_enabled()?; let changed = { ui.re_radio_value(&mut latest_at, false, "Leave empty") .changed() @@ -454,7 +454,7 @@ impl QueryV2 { }; if changed { - self.set_latest_at(ctx, latest_at); + self.save_latest_at_enabled(ctx, latest_at); } Ok(()) From b043d198809056b5d59bb33cd6e8a58bbf6b56fc Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 27 Sep 2024 19:16:47 +0200 Subject: [PATCH 10/10] Switch to menu instead of modal for column visibility ui --- .../src/space_view_class.rs | 11 +----- .../src/view_query_v2/mod.rs | 4 +-- .../src/view_query_v2/ui.rs | 34 +++++-------------- 3 files changed, 10 insertions(+), 39 deletions(-) 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 f0b935799960..5d68fc6ff34f 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 @@ -26,9 +26,6 @@ struct DataframeSpaceViewState { /// Schema for the current query, cached here for the column visibility UI. schema: Option>, - - /// Modal dialog for column visibility. - column_visibility_modal: ModalHandler, } impl SpaceViewState for DataframeSpaceViewState { @@ -125,13 +122,7 @@ mode sets the default time range to _everything_. You can override this in the s // for the user to click the menu anyway. return Ok(()); }; - view_query.selection_panel_ui( - ctx, - ui, - space_view_id, - schema, - &mut state.column_visibility_modal, - ) + view_query.selection_panel_ui(ctx, ui, space_view_id, schema) } fn extra_title_bar_ui( diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs index 1088339a72f9..ec5cbfc15c8e 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/mod.rs @@ -5,7 +5,6 @@ use re_chunk_store::ColumnDescriptor; use re_log_types::EntityPath; use re_types::blueprint::archetypes; use re_types_core::ComponentName; -use re_ui::modal::ModalHandler; use re_viewer_context::{SpaceViewId, SpaceViewSystemExecutionError, ViewerContext}; use re_viewport_blueprint::ViewProperty; @@ -44,7 +43,6 @@ impl QueryV2 { ui: &mut egui::Ui, space_view_id: SpaceViewId, schema: &[ColumnDescriptor], - column_visibility_modal_handler: &mut ModalHandler, ) -> Result<(), SpaceViewSystemExecutionError> { let timeline = self.timeline(ctx)?; @@ -54,7 +52,7 @@ impl QueryV2 { ui.separator(); self.filter_event_ui(ctx, ui, &timeline, space_view_id)?; ui.separator(); - self.column_visibility_ui(ctx, ui, &timeline, schema, column_visibility_modal_handler)?; + self.column_visibility_ui(ctx, ui, &timeline, schema)?; ui.separator(); self.latest_at_ui(ctx, ui)?; diff --git a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs index 189c2a025bad..4294961ce599 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query_v2/ui.rs @@ -5,7 +5,6 @@ use re_log_types::{ EntityPath, ResolvedTimeRange, TimeInt, TimeType, TimeZone, Timeline, TimelineName, }; use re_types_core::{ComponentName, ComponentNameSet}; -use re_ui::modal::{Modal, ModalHandler}; use re_ui::{list_item, UiExt}; use re_viewer_context::{SpaceViewId, SpaceViewSystemExecutionError, TimeDragValue, ViewerContext}; @@ -260,7 +259,6 @@ impl QueryV2 { ui: &mut egui::Ui, timeline: &Timeline, schema: &[ColumnDescriptor], - column_visibility_modal_handler: &mut ModalHandler, ) -> Result<(), SpaceViewSystemExecutionError> { // Gather our selected columns. let selected_columns: HashSet<_> = self @@ -270,23 +268,7 @@ impl QueryV2 { let visible_count = selected_columns.len(); let hidden_count = schema.len() - visible_count; - - ui.label("Columns:"); - let visible_count_label = format!("{visible_count} visible, {hidden_count} hidden"); - ui.list_item_flat_noninteractive( - list_item::LabelContent::new(&visible_count_label) - .always_show_buttons(true) - .with_buttons(|ui| { - let resp = ui.small_icon_button(&re_ui::icons::EDIT); - - if resp.clicked() { - column_visibility_modal_handler.open(); - } - - resp - }), - ); let mut new_selected_columns = selected_columns.clone(); @@ -299,7 +281,7 @@ impl QueryV2 { let mut all_enabled = hidden_count == 0; if ui - .checkbox_indeterminate(&mut all_enabled, visible_count_label, indeterminate) + .checkbox_indeterminate(&mut all_enabled, &visible_count_label, indeterminate) .changed() { if all_enabled { @@ -413,15 +395,15 @@ impl QueryV2 { } }; - column_visibility_modal_handler.ui( - ui.ctx(), - || Modal::new("Column visibility"), + ui.list_item_flat_noninteractive(list_item::PropertyContent::new("Columns").value_fn( |ui, _| { - egui::ScrollArea::vertical() - .auto_shrink([false, false]) - .show(ui, modal_ui) + egui::menu::menu_button(ui, &visible_count_label, |ui| { + egui::ScrollArea::vertical() + .auto_shrink([false, false]) + .show(ui, modal_ui) + }); }, - ); + )); // save changes of column visibility if new_selected_columns != selected_columns {