From 4b6bc9e75539e597c81912dcb8ff0739da2766eb Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 20 Jun 2024 11:55:28 +0200 Subject: [PATCH 1/2] Unified visualizer & override ui, enabled on all entities (#6599) ### What Previously, we had ui for adding component overrides, under an experimental feature everywhere except the time series view + some hardcoded ui for manipulating overrides (those that were `EntityProperties` in 0.16). This PR removes both the component override ui and the hardcoded property ui in favor of a new overview + override ui showing all currently used values by all active visualizers + ability to add/remove visualizers. https://github.com/rerun-io/rerun/assets/1220815/425a9406-9eb2-4319-846b-fe0d0eee1a42 --- Noteworthy "sideeffects": * `ComponentUiCallback` now uses raw arrow arrays, allowing it to show fallbacks * kills lots'a old code :) * `VisualizerQueryInfo::queried` keeps now archetype component ordering instead of alphabetical (not in above video yet) * lots of inconsistency in our data flow are now visible --- Planned direct follow-ups: * allow various reset / override menus * particularly important: remove override, add (single) override on multi-value * Would be great to do this with a menu button, will have to be a context menu until https://github.com/emilk/egui/issues/4607 is handled * allow expanding to see the override/store/default/fallback stack * original plan was to skip on this, but the method has all the data layed out neatly now, so might as well! * get the "Add" button integrated into the top (unless this is hard?) * show better docs: https://github.com/rerun-io/rerun/issues/6556 * Find a better name for the "blueprint section" below --- Also needed but not planned short term: * inspect multivalues * we could do so for store multivalues already, but everything else we can't yet select, so let's tackle this later * ui polish --- Part of: * #4888 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6599?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6599?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/6599) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --- Cargo.lock | 1 + crates/re_data_ui/src/component.rs | 27 +- crates/re_data_ui/src/component_path.rs | 5 +- .../re_data_ui/src/component_ui_registry.rs | 49 +- crates/re_data_ui/src/instance_path.rs | 3 +- crates/re_edit_ui/src/lib.rs | 3 +- crates/re_query/src/latest_at/helpers.rs | 11 + crates/re_selection_panel/src/defaults_ui.rs | 2 +- crates/re_selection_panel/src/lib.rs | 2 +- crates/re_selection_panel/src/override_ui.rs | 448 ------------------ .../re_selection_panel/src/selection_panel.rs | 292 +----------- .../re_selection_panel/src/visualizer_ui.rs | 339 +++++++++++++ crates/re_space_view/src/query.rs | 4 +- crates/re_space_view/src/results_ext.rs | 6 +- .../src/contexts/transform_context.rs | 3 +- crates/re_space_view_spatial/src/lib.rs | 3 +- .../src/visualizers/transform3d_arrows.rs | 7 +- .../src/line_visualizer_system.rs | 10 +- .../src/point_visualizer_system.rs | 10 +- crates/re_viewer/src/ui/rerun_menu.rs | 8 - crates/re_viewer_context/Cargo.toml | 1 + crates/re_viewer_context/src/app_options.rs | 5 - .../src/component_ui_registry.rs | 159 ++++--- .../src/space_view/view_query.rs | 3 +- .../src/space_view/visualizer_system.rs | 39 +- crates/re_viewer_context/src/test_context.rs | 2 +- 26 files changed, 561 insertions(+), 881 deletions(-) delete mode 100644 crates/re_selection_panel/src/override_ui.rs create mode 100644 crates/re_selection_panel/src/visualizer_ui.rs diff --git a/Cargo.lock b/Cargo.lock index 70a50bb9945a..934b411fb514 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5266,6 +5266,7 @@ dependencies = [ "half 2.3.1", "indexmap 2.1.0", "itertools 0.13.0", + "linked-hash-map", "macaw", "ndarray", "nohash-hasher", diff --git a/crates/re_data_ui/src/component.rs b/crates/re_data_ui/src/component.rs index 4e77f1b31521..8398811528ae 100644 --- a/crates/re_data_ui/src/component.rs +++ b/crates/re_data_ui/src/component.rs @@ -1,10 +1,7 @@ -use std::sync::Arc; - use egui::NumExt; use re_entity_db::{external::re_query::LatestAtComponentResults, EntityPath, InstancePath}; use re_log_types::Instance; -use re_types::ComponentName; use re_ui::{ContextExt as _, SyntaxHighlighting as _}; use re_viewer_context::{UiLayout, ViewerContext}; @@ -12,13 +9,12 @@ use super::DataUi; use crate::item_ui; /// All the values of a specific [`re_log_types::ComponentPath`]. -pub struct EntityLatestAtResults { +pub struct EntityLatestAtResults<'a> { pub entity_path: EntityPath, - pub component_name: ComponentName, - pub results: Arc, + pub results: &'a LatestAtComponentResults, } -impl DataUi for EntityLatestAtResults { +impl<'a> DataUi for EntityLatestAtResults<'a> { fn data_ui( &self, ctx: &ViewerContext<'_>, @@ -27,12 +23,17 @@ impl DataUi for EntityLatestAtResults { query: &re_data_store::LatestAtQuery, db: &re_entity_db::EntityDb, ) { - re_tracing::profile_function!(self.component_name); + let Some(component_name) = self.results.component_name(db.resolver()) else { + // TODO(#5607): what should happen if the promise is still pending? + return; + }; + + re_tracing::profile_function!(component_name); // TODO(#5607): what should happen if the promise is still pending? let Some(num_instances) = self .results - .raw(db.resolver(), self.component_name) + .raw(db.resolver(), component_name) .map(|data| data.len()) else { ui.weak(""); @@ -74,7 +75,7 @@ impl DataUi for EntityLatestAtResults { if let Some(histogram) = db .tree() .subtree(&self.entity_path) - .and_then(|tree| tree.entity.components.get(&self.component_name)) + .and_then(|tree| tree.entity.components.get(&component_name)) { if histogram.num_static_messages() > 1 { ui.label(ui.ctx().warning_text(format!( @@ -139,7 +140,7 @@ impl DataUi for EntityLatestAtResults { query, db, &self.entity_path, - &self.results, + self.results, &Instance::from(0), ); } else if one_line { @@ -157,7 +158,7 @@ impl DataUi for EntityLatestAtResults { ui.label("Index"); }); header.col(|ui| { - ui.label(self.component_name.short_name()); + ui.label(component_name.short_name()); }); }) .body(|mut body| { @@ -186,7 +187,7 @@ impl DataUi for EntityLatestAtResults { query, db, &self.entity_path, - &self.results, + self.results, &instance, ); }); diff --git a/crates/re_data_ui/src/component_path.rs b/crates/re_data_ui/src/component_path.rs index 8878622aef6f..b59e971c9141 100644 --- a/crates/re_data_ui/src/component_path.rs +++ b/crates/re_data_ui/src/component_path.rs @@ -1,5 +1,3 @@ -use std::sync::Arc; - use re_log_types::ComponentPath; use re_ui::ContextExt as _; use re_viewer_context::{UiLayout, ViewerContext}; @@ -31,8 +29,7 @@ impl DataUi for ComponentPath { if let Some(results) = results.components.get(component_name) { crate::EntityLatestAtResults { entity_path: entity_path.clone(), - component_name: *component_name, - results: Arc::clone(results), + results: results.as_ref(), } .data_ui(ctx, ui, ui_layout, query, db); } else if let Some(entity_tree) = ctx.recording().tree().subtree(entity_path) { diff --git a/crates/re_data_ui/src/component_ui_registry.rs b/crates/re_data_ui/src/component_ui_registry.rs index 3aca3eb25fba..f586d53709d8 100644 --- a/crates/re_data_ui/src/component_ui_registry.rs +++ b/crates/re_data_ui/src/component_ui_registry.rs @@ -1,7 +1,8 @@ use re_data_store::LatestAtQuery; -use re_entity_db::{external::re_query::LatestAtComponentResults, EntityDb}; -use re_log_types::{external::arrow2, EntityPath, Instance}; +use re_entity_db::EntityDb; +use re_log_types::{external::arrow2, EntityPath}; use re_types::external::arrow2::array::Utf8Array; +use re_ui::UiExt; use re_viewer_context::{ComponentUiRegistry, UiLayout, ViewerContext}; use super::EntityDataUi; @@ -39,43 +40,39 @@ pub fn add_to_registry(registry: &mut Com registry.add_display_ui( C::name(), Box::new( - |ctx, ui, ui_layout, query, db, entity_path, component, instance| { - // TODO(#5607): what should happen if the promise is still pending? - if let Some(component) = component.instance::(db.resolver(), instance.get() as _) - { - component.entity_data_ui(ctx, ui, ui_layout, entity_path, query, db); - } else { - ui.weak("(not found)"); + |ctx, ui, ui_layout, query, db, entity_path, component_raw| match C::from_arrow( + component_raw, + ) { + Ok(components) => match components.len() { + 0 => { + ui.weak("(empty)"); + } + 1 => { + components[0].entity_data_ui(ctx, ui, ui_layout, entity_path, query, db); + } + i => { + ui.label(format!("{} values", re_format::format_uint(i))); + } + }, + Err(err) => { + ui.error_label("(failed to deserialize)") + .on_hover_text(err.to_string()); } }, ), ); } -#[allow(clippy::too_many_arguments)] fn fallback_component_ui( _ctx: &ViewerContext<'_>, ui: &mut egui::Ui, ui_layout: UiLayout, _query: &LatestAtQuery, - db: &EntityDb, + _db: &EntityDb, _entity_path: &EntityPath, - component: &LatestAtComponentResults, - instance: &Instance, + component: &dyn arrow2::array::Array, ) { - // TODO(#5607): what should happen if the promise is still pending? - let value = component - .component_name(db.resolver()) - .and_then(|component_name| { - component.instance_raw(db.resolver(), component_name, instance.get() as _) - }); - - // No special ui implementation - use a generic one: - if let Some(value) = value { - arrow_ui(ui, ui_layout, &*value); - } else { - ui.weak("(null)"); - } + arrow_ui(ui, ui_layout, component); } fn arrow_ui(ui: &mut egui::Ui, ui_layout: UiLayout, array: &dyn arrow2::array::Array) { diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 788cdc2d3f8b..c21e344cb142 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -121,8 +121,7 @@ impl DataUi for InstancePath { if instance.is_all() { crate::EntityLatestAtResults { entity_path: entity_path.clone(), - component_name, - results: std::sync::Arc::clone(results), + results: results.as_ref(), } .data_ui( ctx, diff --git a/crates/re_edit_ui/src/lib.rs b/crates/re_edit_ui/src/lib.rs index 7a4693af2e5f..e0a6f9a1c0f4 100644 --- a/crates/re_edit_ui/src/lib.rs +++ b/crates/re_edit_ui/src/lib.rs @@ -16,7 +16,7 @@ use datatype_editors::{ use re_types::{ blueprint::components::{BackgroundKind, Corner2D, LockRangeDuringZoom, ViewFit, Visible}, components::{ - AggregationPolicy, AxisLength, Color, Colormap, FillRatio, GammaCorrection, + AggregationPolicy, AxisLength, Color, Colormap, DepthMeter, FillRatio, GammaCorrection, ImagePlaneDistance, MagnificationFilter, MarkerSize, Name, Radius, StrokeWidth, Text, }, }; @@ -59,6 +59,7 @@ pub fn register_editors(registry: &mut re_viewer_context::ComponentUiRegistry) { registry.add_singleline_editor_ui::(edit_singleline_string); registry.add_singleline_editor_ui::(edit_singleline_string); + registry.add_singleline_editor_ui::(edit_f32_zero_to_max_float_raw); registry.add_singleline_editor_ui::(edit_f32_zero_to_max_float_raw); registry.add_singleline_editor_ui::(edit_f32_zero_to_max_float_raw); registry.add_singleline_editor_ui::(edit_f32_zero_to_max_float_raw); diff --git a/crates/re_query/src/latest_at/helpers.rs b/crates/re_query/src/latest_at/helpers.rs index 29dfc1751bad..5d501c43d166 100644 --- a/crates/re_query/src/latest_at/helpers.rs +++ b/crates/re_query/src/latest_at/helpers.rs @@ -60,6 +60,17 @@ impl LatestAtComponentResults { } } + /// Tries to return the component data as an arrow array. + /// + /// Logs a warning and returns `None` if the component is missing or cannot be deserialized. + #[inline] + pub fn try_raw(&self, resolver: &PromiseResolver) -> Option> { + match self.resolved(resolver) { + PromiseResult::Pending | PromiseResult::Error(_) => None, + PromiseResult::Ready(cell) => Some(cell.to_arrow()), + } + } + /// Returns true if the component is missing, an empty array or still pending. pub fn is_empty(&self, resolver: &PromiseResolver) -> bool { match self.resolved(resolver) { diff --git a/crates/re_selection_panel/src/defaults_ui.rs b/crates/re_selection_panel/src/defaults_ui.rs index 5e7be83b9045..a383603e9343 100644 --- a/crates/re_selection_panel/src/defaults_ui.rs +++ b/crates/re_selection_panel/src/defaults_ui.rs @@ -45,7 +45,7 @@ pub fn defaults_ui(ctx: &ViewContext<'_>, space_view: &SpaceViewBlueprint, ui: & // TODO(jleibs): We can do something fancier in the future such as presenting both // options once we have a motivating use-case. for (id, vis) in ctx.visualizer_collection.iter_with_identifiers() { - for component in vis.visualizer_query_info().queried { + for &component in vis.visualizer_query_info().queried.iter() { component_to_vis.entry(component).or_insert_with(|| id); } } diff --git a/crates/re_selection_panel/src/lib.rs b/crates/re_selection_panel/src/lib.rs index b6906336f32b..b17c8c002d67 100644 --- a/crates/re_selection_panel/src/lib.rs +++ b/crates/re_selection_panel/src/lib.rs @@ -1,12 +1,12 @@ //! The UI for the selection panel. mod defaults_ui; -mod override_ui; mod query_range_ui; mod selection_history_ui; mod selection_panel; mod space_view_entity_picker; mod space_view_space_origin_ui; +mod visualizer_ui; pub use selection_panel::SelectionPanel; diff --git a/crates/re_selection_panel/src/override_ui.rs b/crates/re_selection_panel/src/override_ui.rs deleted file mode 100644 index 7b385c49b44e..000000000000 --- a/crates/re_selection_panel/src/override_ui.rs +++ /dev/null @@ -1,448 +0,0 @@ -use std::collections::{BTreeMap, BTreeSet}; - -use itertools::Itertools; - -use re_data_store::LatestAtQuery; -use re_entity_db::{EntityDb, InstancePath}; -use re_log_types::{DataCell, DataRow, RowId, StoreKind}; -use re_types_core::{components::VisualizerOverrides, ComponentName}; -use re_ui::{ContextExt as _, UiExt as _}; -use re_viewer_context::{ - ComponentUiTypes, DataResult, OverridePath, SpaceViewClassExt as _, SystemCommand, - SystemCommandSender as _, ViewContext, ViewSystemIdentifier, ViewerContext, -}; -use re_viewport_blueprint::SpaceViewBlueprint; - -pub fn override_ui( - ctx: &ViewContext<'_>, - space_view: &SpaceViewBlueprint, - instance_path: &InstancePath, - ui: &mut egui::Ui, -) { - let InstancePath { - entity_path, - instance: _, // Override ui only works on the first instance of an entity. - } = instance_path; - - // Because of how overrides are implemented the overridden-data must be an entity - // in the real store. We would never show an override UI for a selected blueprint - // entity from the blueprint-inspector since it isn't "part" of a space-view to provide - // the overrides. - let query = ctx.current_query(); - - let query_result = ctx.lookup_query_result(space_view.id); - let Some(data_result) = query_result - .tree - .lookup_result_by_path(entity_path) - .cloned() - else { - ui.label(ui.ctx().error_text("Entity not found in view.")); - return; - }; - - let active_overrides: BTreeSet = data_result - .property_overrides - .as_ref() - .map(|props| props.resolved_component_overrides.keys().copied().collect()) - .unwrap_or_default(); - - let mut component_to_vis: BTreeMap = Default::default(); - - // Accumulate the components across all visualizers and track which visualizer - // each component came from so we can use it for fallbacks later. - // - // If two visualizers have the same component, the first one wins. - // TODO(jleibs): We can do something fancier in the future such as presenting both - // options once we have a motivating use-case. - for vis in &data_result.visualizers { - let Some(queried) = ctx - .visualizer_collection - .get_by_identifier(*vis) - .ok() - .map(|vis| vis.visualizer_query_info().queried) - else { - continue; - }; - - for component in queried { - component_to_vis.entry(component).or_insert_with(|| *vis); - } - } - - add_new_override( - ctx, - &query, - ctx.recording(), - ui, - &component_to_vis, - &active_overrides, - &data_result, - ); - - // TODO(jleibs): This clone is annoying, but required because QueryContext wants - // a reference to the EntityPath. We could probably refactor this to avoid the clone. - let Some(overrides) = data_result.property_overrides.clone() else { - return; - }; - - let sorted_overrides = overrides - .resolved_component_overrides - .into_iter() - .sorted_by_key(|(c, _)| *c); - - let query_context = ctx.query_context(&data_result, &query); - - re_ui::list_item::list_item_scope(ui, "overrides", |ui| { - ui.spacing_mut().item_spacing.y = 0.0; - for ( - ref component_name, - OverridePath { - ref store_kind, - path: ref entity_path_overridden, - }, - ) in sorted_overrides - { - let Some(visualizer_identifier) = component_to_vis.get(component_name) else { - continue; - }; - let Ok(visualizer) = ctx - .visualizer_collection - .get_by_identifier(*visualizer_identifier) - else { - re_log::warn!( - "Failed to resolve visualizer identifier {visualizer_identifier}, to a visualizer implementation" - ); - continue; - }; - - let value_fn = |ui: &mut egui::Ui| { - let (origin_db, query) = match store_kind { - StoreKind::Blueprint => { - (ctx.blueprint_db(), ctx.viewer_ctx.blueprint_query.clone()) - } - StoreKind::Recording => (ctx.recording(), ctx.current_query()), - }; - let component_data = origin_db - .query_caches() - .latest_at( - origin_db.store(), - &query, - entity_path_overridden, - [*component_name], - ) - .components - .get(component_name) - .cloned(); /* arc */ - - if let Some(results) = component_data { - ctx.viewer_ctx.component_ui_registry.singleline_edit_ui( - &query_context, - ui, - origin_db, - entity_path_overridden, - *component_name, - &results, - visualizer.as_fallback_provider(), - ); - } else { - // TODO(jleibs): Is it possible to set an override to empty and not confuse - // the situation with "not-overridden?". Maybe we hit this in cases of `[]` vs `[null]`. - ui.weak("(empty)"); - } - }; - - ui.list_item() - .interactive(false) - .show_flat( - ui, - re_ui::list_item::PropertyContent::new(component_name.short_name()) - .min_desired_width(150.0) - .action_button(&re_ui::icons::CLOSE, || { - ctx.save_empty_blueprint_component_by_name( - &overrides.individual_override_path, - *component_name, - ); - }) - .value_fn(|ui, _| value_fn(ui)), - ) - .on_hover_text(component_name.full_name()); - } - }); -} - -#[allow(clippy::too_many_arguments)] -pub fn add_new_override( - ctx: &ViewContext<'_>, - query: &LatestAtQuery, - db: &EntityDb, - ui: &mut egui::Ui, - component_to_vis: &BTreeMap, - active_overrides: &BTreeSet, - data_result: &DataResult, -) { - let remaining_components = component_to_vis - .keys() - .filter(|c| !active_overrides.contains(*c)) - .collect::>(); - - let enabled = !remaining_components.is_empty(); - - ui.add_enabled_ui(enabled, |ui| { - let mut opened = false; - let menu = ui - .menu_button("Add", |ui| { - opened = true; - ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); - - let query_context = ctx.query_context(data_result, query); - - // Present the option to add new components for each component that doesn't - // already have an active override. - for (component, viz) in component_to_vis { - if active_overrides.contains(component) { - continue; - } - // If we don't have an override_path we can't set up an initial override - // this shouldn't happen if the `DataResult` is valid. - let Some(override_path) = data_result.individual_override_path() else { - if cfg!(debug_assertions) { - re_log::error!("No override path for: {}", component); - } - continue; - }; - - // If there is no registered editor, don't let the user create an override - // TODO(andreas): Can only handle single line editors right now. - if !ctx - .viewer_ctx - .component_ui_registry - .registered_ui_types(*component) - .contains(ComponentUiTypes::SingleLineEditor) - { - continue; - } - - if ui.button(component.short_name()).clicked() { - // We are creating a new override. We need to decide what initial value to give it. - // - First see if there's an existing splat in the recording. - // - Next see if visualizer system wants to provide a value. - // - Finally, fall back on the default value from the component registry. - - let components = [*component]; - - let Some(mut initial_data) = db - .store() - .latest_at(query, &data_result.entity_path, *component, &components) - .and_then(|result| result.2[0].clone()) - .or_else(|| { - ctx.visualizer_collection - .get_by_identifier(*viz) - .ok() - .and_then(|sys| { - sys.fallback_for(&query_context, *component) - .map(|fallback| { - DataCell::from_arrow(*component, fallback) - }) - .ok() - }) - }) - else { - re_log::warn!("Could not identify an initial value for: {}", component); - return; - }; - - initial_data.compute_size_bytes(); - - match DataRow::from_cells( - RowId::new(), - ctx.blueprint_timepoint_for_writes(), - override_path.clone(), - [initial_data], - ) { - Ok(row) => { - ctx.viewer_ctx.command_sender.send_system( - SystemCommand::UpdateBlueprint( - ctx.blueprint_db().store_id().clone(), - vec![row], - ), - ); - } - Err(err) => { - re_log::warn!( - "Failed to create DataRow for blueprint component: {}", - err - ); - } - } - - ui.close_menu(); - } - } - }) - .response - .on_disabled_hover_text("No additional components available."); - if !opened { - menu.on_hover_text("Choose a component to specify an override value.".to_owned()); - } - }); -} - -// --- - -pub fn override_visualizer_ui( - ctx: &ViewerContext<'_>, - space_view: &SpaceViewBlueprint, - instance_path: &InstancePath, - ui: &mut egui::Ui, -) { - ui.push_id("visualizer_overrides", |ui| { - let InstancePath { - entity_path, - instance: _, - } = instance_path; - - let recording = ctx.recording(); - - let query_result = ctx.lookup_query_result(space_view.id); - let Some(data_result) = query_result - .tree - .lookup_result_by_path(entity_path) - .cloned() - else { - ui.label(ui.ctx().error_text("Entity not found in view.")); - return; - }; - - let Some(override_path) = data_result.individual_override_path() else { - if cfg!(debug_assertions) { - re_log::error!("No override path for entity: {}", data_result.entity_path); - } - return; - }; - - let active_visualizers: Vec<_> = data_result.visualizers.iter().sorted().copied().collect(); - - add_new_visualizer( - ctx, - recording, - ui, - space_view, - &data_result, - &active_visualizers, - ); - - re_ui::list_item::list_item_scope(ui, "visualizers", |ui| { - ui.spacing_mut().item_spacing.y = 0.0; - - for viz_name in &active_visualizers { - ui.list_item().interactive(false).show_flat( - ui, - re_ui::list_item::LabelContent::new(viz_name.as_str()) - .min_desired_width(150.0) - .with_buttons(|ui| { - let response = ui.small_icon_button(&re_ui::icons::CLOSE); - if response.clicked() { - let component = VisualizerOverrides::from( - active_visualizers - .iter() - .filter(|v| *v != viz_name) - .map(|v| re_types_core::ArrowString::from(v.as_str())) - .collect::>(), - ); - - ctx.save_blueprint_component(override_path, &component); - } - response - }) - .always_show_buttons(true), - ); - } - }); - }); -} - -pub fn add_new_visualizer( - ctx: &ViewerContext<'_>, - entity_db: &EntityDb, - ui: &mut egui::Ui, - space_view: &SpaceViewBlueprint, - data_result: &DataResult, - active_visualizers: &[ViewSystemIdentifier], -) { - // If we don't have an override_path we can't set up an initial override - // this shouldn't happen if the `DataResult` is valid. - let Some(override_path) = data_result.individual_override_path() else { - if cfg!(debug_assertions) { - re_log::error!("No override path for entity: {}", data_result.entity_path); - } - return; - }; - - // TODO(jleibs): This has already been computed for the SpaceView this frame. Maybe We - // should do this earlier and store it with the SpaceView? - let applicable_entities_per_visualizer = ctx - .space_view_class_registry - .applicable_entities_for_visualizer_systems(entity_db.store_id()); - - let visualizable_entities = space_view - .class(ctx.space_view_class_registry) - .determine_visualizable_entities( - &applicable_entities_per_visualizer, - entity_db, - &ctx.space_view_class_registry - .new_visualizer_collection(space_view.class_identifier()), - &space_view.space_origin, - ); - - let visualizer_options = visualizable_entities - .iter() - .filter(|(vis, ents)| { - ents.contains(&data_result.entity_path) && !active_visualizers.contains(vis) - }) - .map(|(vis, _)| vis) - .sorted() - .collect::>(); - - let enabled = !visualizer_options.is_empty(); - - let mut opened = false; - - ui.add_enabled_ui(enabled, |ui| { - let menu = ui - .menu_button("Add", |ui| { - ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); - opened = true; - - if visualizer_options.is_empty() { - ui.close_menu(); - } - - // Present the option to add new components for each component that doesn't - // already have an active override. - for viz in visualizer_options { - if ui.button(viz.as_str()).clicked() { - let component = VisualizerOverrides::from( - active_visualizers - .iter() - .chain(std::iter::once(viz)) - .map(|v| { - let arrow_str: re_types_core::ArrowString = v.as_str().into(); - arrow_str - }) - .collect::>(), - ); - - ctx.save_blueprint_component(override_path, &component); - - ui.close_menu(); - } - } - }) - .response - .on_disabled_hover_text("No additional visualizers available."); - - if !opened { - menu.on_hover_text("Choose a component to specify an override value.".to_owned()); - } - }); -} diff --git a/crates/re_selection_panel/src/selection_panel.rs b/crates/re_selection_panel/src/selection_panel.rs index 5ed9c331b462..2d66f6f6bc0f 100644 --- a/crates/re_selection_panel/src/selection_panel.rs +++ b/crates/re_selection_panel/src/selection_panel.rs @@ -3,38 +3,25 @@ use egui_tiles::ContainerKind; use re_context_menu::{context_menu_ui_for_item, SelectionUpdateBehavior}; use re_data_ui::{ - image_meaning_for_entity, item_ui, + item_ui, item_ui::{guess_instance_path_icon, guess_query_and_db_for_selected_entity}, DataUi, }; use re_entity_db::{EntityPath, InstancePath}; use re_log_types::EntityPathFilter; -use re_space_view::{DataResultQuery as _, HybridLatestAtResults}; -use re_space_view_time_series::TimeSeriesSpaceView; -use re_types::{ - archetypes::{Axes3D, DepthImage, Pinhole}, - blueprint::components::Interactive, - components::{ - AxisLength, Colormap, DepthMeter, FillRatio, ImagePlaneDistance, PinholeProjection, - Transform3D, VisualizerOverrides, - }, - tensor_data::TensorDataMeaning, -}; +use re_types::blueprint::components::Interactive; use re_ui::{icons, list_item, ContextExt as _, DesignTokens, SyntaxHighlighting as _, UiExt as _}; use re_viewer_context::{ - contents_name_style, gpu_bridge::colormap_dropdown_button_ui, icon_for_container_kind, - ContainerId, Contents, DataQueryResult, DataResult, HoverHighlight, Item, SpaceViewClass, - SpaceViewId, UiLayout, ViewContext, ViewStates, ViewerContext, + contents_name_style, icon_for_container_kind, ContainerId, Contents, DataQueryResult, + DataResult, HoverHighlight, Item, SpaceViewId, UiLayout, ViewContext, ViewStates, + ViewerContext, }; use re_viewport_blueprint::{ ui::show_add_space_view_or_container_modal, SpaceViewBlueprint, ViewportBlueprint, }; use crate::space_view_entity_picker::SpaceViewEntityPicker; -use crate::{ - defaults_ui::defaults_ui, - override_ui::{override_ui, override_visualizer_ui}, -}; +use crate::{defaults_ui::defaults_ui, visualizer_ui::visualizer_ui}; use crate::{ query_range_ui::query_range_ui_data_result, query_range_ui::query_range_ui_space_view, selection_history_ui::SelectionHistoryUi, @@ -165,23 +152,15 @@ impl SelectionPanel { }); } - // Special override section for space-view-entities + // Special override section for view-entities if let Item::DataResult(view_id, instance_path) = item { - if let Some(view) = blueprint.space_views.get(view_id) { - // TODO(jleibs): Overrides still require special handling inside the visualizers. - // For now, only show the override section for TimeSeries until support is implemented - // generically. - if view.class_identifier() == TimeSeriesSpaceView::identifier() - || ctx.app_options.experimental_visualizer_selection - { - ui.large_collapsing_header("Visualizers", true, |ui| { - override_visualizer_ui(ctx, view, instance_path, ui); - }); - + // Only show visualizer selection when the entire entity is selected. + // (showing it for instances gives the wrong impression) + if instance_path.is_all() { + if let Some(view) = blueprint.space_views.get(view_id) { let view_ctx = view.bundle_context_with_states(ctx, view_states); - - ui.large_collapsing_header("Component Overrides", true, |ui| { - override_ui(&view_ctx, view, instance_path, ui); + ui.large_collapsing_header("Visualizers", true, |ui| { + visualizer_ui(&view_ctx, view, &instance_path.entity_path, ui); }); } } @@ -1160,8 +1139,6 @@ fn entity_props_ui( use re_types::blueprint::components::Visible; use re_types::Loggable as _; - let entity_path = &data_result.entity_path; - list_item::list_item_scope(ui, "entity_props", |ui| { { let visible_before = data_result.is_visible(ctx.viewer_ctx); @@ -1222,247 +1199,4 @@ fn entity_props_ui( }); query_range_ui_data_result(ctx.viewer_ctx, ui, data_result); - - egui::Grid::new("entity_properties") - .num_columns(2) - .show(ui, |ui| { - // TODO(wumpf): It would be nice to only show pinhole & depth properties in the context of a 3D view. - // if *view_state.state_spatial.nav_mode.get() == SpatialNavigationMode::ThreeD { - pinhole_props_ui(ctx, ui, data_result); - depth_props_ui(ctx, ui, data_result, entity_path); - transform3d_visualization_ui(ctx, ui, data_result); - }); -} - -fn colormap_props_ui( - ctx: &ViewContext<'_>, - ui: &mut egui::Ui, - data_result: &DataResult, - depth_image_results: &HybridLatestAtResults<'_>, -) { - let colormap = depth_image_results.get_mono_with_fallback::(); - let mut new_colormap = colormap; - - ui.label("Color map"); - colormap_dropdown_button_ui(ctx.viewer_ctx.render_ctx, ui, &mut new_colormap); - - if new_colormap != colormap { - data_result.save_individual_override(ctx.viewer_ctx, &new_colormap); - } - - ui.end_row(); -} - -fn pinhole_props_ui(ctx: &ViewContext<'_>, ui: &mut egui::Ui, data_result: &DataResult) { - let (query, store) = - guess_query_and_db_for_selected_entity(ctx.viewer_ctx, &data_result.entity_path); - - if store - .latest_at_component::(&data_result.entity_path, &query) - .is_some() - { - let results = data_result.latest_at_with_overrides::(ctx, &query); - - let mut image_plane_value: f32 = results - .get_mono_with_fallback::() - .into(); - - ui.label("Image plane distance"); - let speed = (image_plane_value * 0.05).at_least(0.01); - if ui - .add( - egui::DragValue::new(&mut image_plane_value) - .clamp_range(0.0..=1.0e8) - .speed(speed), - ) - .on_hover_text("Controls how far away the image plane is") - .changed() - { - let new_image_plane: ImagePlaneDistance = image_plane_value.into(); - - data_result.save_individual_override(ctx.viewer_ctx, &new_image_plane); - } - ui.end_row(); - } -} - -fn transform3d_visualization_ui( - ctx: &ViewContext<'_>, - ui: &mut egui::Ui, - data_result: &DataResult, -) { - re_tracing::profile_function!(); - - let (query, store) = - guess_query_and_db_for_selected_entity(ctx.viewer_ctx, &data_result.entity_path); - - if store - .latest_at_component::(&data_result.entity_path, &query) - .is_none() - { - return; - } - - let arrow_viz = "Transform3DArrows".into(); - - let mut show_arrows = data_result.visualizers.contains(&arrow_viz); - - let results = data_result.latest_at_with_overrides::(ctx, &query); - - let mut arrow_length: f32 = results.get_mono_with_fallback::().into(); - - { - let response = ui.re_checkbox( &mut show_arrows, "Show transform").on_hover_text( - "Enables/disables the display of three arrows to visualize the (accumulated) transform at this entity. Red/green/blue show the x/y/z axis respectively."); - if response.changed() { - let component = if show_arrows { - VisualizerOverrides::from( - data_result - .visualizers - .iter() - .chain(std::iter::once(&arrow_viz)) - .map(|v| re_types_core::ArrowString::from(v.as_str())) - .collect::>(), - ) - } else { - VisualizerOverrides::from( - data_result - .visualizers - .iter() - .filter(|v| **v != arrow_viz) - .map(|v| re_types_core::ArrowString::from(v.as_str())) - .collect::>(), - ) - }; - - data_result.save_individual_override(ctx.viewer_ctx, &component); - } - } - - if show_arrows { - ui.end_row(); - ui.label("Transform-arrow length"); - let speed = (arrow_length * 0.05).at_least(0.001); - let response = ui - .add( - egui::DragValue::new(&mut arrow_length) - .clamp_range(0.0..=1.0e8) - .speed(speed), - ) - .on_hover_text( - "How long the arrows should be in the entity's own coordinate system. Double-click to reset to auto.", - ); - if response.double_clicked() { - data_result.clear_individual_override::(ctx.viewer_ctx); - response.surrender_focus(); - } else if response.changed() { - data_result - .save_individual_override::(ctx.viewer_ctx, &arrow_length.into()); - } - } - - ui.end_row(); -} - -fn depth_props_ui( - ctx: &ViewContext<'_>, - ui: &mut egui::Ui, - data_result: &DataResult, - entity_path: &EntityPath, -) -> Option<()> { - re_tracing::profile_function!(); - - let (query, db) = guess_query_and_db_for_selected_entity(ctx.viewer_ctx, entity_path); - - let meaning = image_meaning_for_entity(entity_path, &query, db.store()); - - if meaning != TensorDataMeaning::Depth { - return Some(()); - } - let image_projection_ent_path = db - .latest_at_component_at_closest_ancestor::(entity_path, &query)? - .0; - - ui.label("Pinhole"); - item_ui::entity_path_button( - ctx.viewer_ctx, - &query, - db, - ui, - None, - &image_projection_ent_path, - ) - .on_hover_text("The entity path of the pinhole transform being used to do the backprojection."); - ui.end_row(); - - let (query, _store) = - guess_query_and_db_for_selected_entity(ctx.viewer_ctx, &data_result.entity_path); - let depth_image_results = data_result.latest_at_with_overrides::(ctx, &query); - - depth_from_world_scale_ui(ctx, ui, data_result, &depth_image_results); - backproject_radius_scale_ui(ctx, ui, data_result, &depth_image_results); - colormap_props_ui(ctx, ui, data_result, &depth_image_results); - - Some(()) -} - -fn depth_from_world_scale_ui( - ctx: &ViewContext<'_>, - ui: &mut egui::Ui, - data_result: &DataResult, - depth_image_results: &HybridLatestAtResults<'_>, -) { - let depth_meter = depth_image_results.get_mono_with_fallback::(); - - ui.label("Backproject meter"); - let mut value = *depth_meter; - let speed = (value * 0.05).at_least(0.01); - let response = ui - .add( - egui::DragValue::new(&mut value) - .clamp_range(0.0..=1.0e8) - .speed(speed), - ) - .on_hover_text("How many steps in the depth image correspond to one world-space unit. For instance, 1000 means millimeters.\n\ - Double-click to reset."); - if response.double_clicked() { - data_result.clear_individual_override::(ctx.viewer_ctx); - response.surrender_focus(); - } else if response.changed() { - data_result.save_individual_override(ctx.viewer_ctx, &DepthMeter(value)); - } - - ui.end_row(); -} - -fn backproject_radius_scale_ui( - ctx: &ViewContext<'_>, - ui: &mut egui::Ui, - data_result: &DataResult, - depth_image_results: &HybridLatestAtResults<'_>, -) { - let radius_scale = depth_image_results.get_mono_with_fallback::(); - - ui.label("Backproject radius scale"); - let mut value = *radius_scale.0; - let speed = (value * 0.01).at_least(0.001); - let response = ui - .add( - egui::DragValue::new(&mut value) - .clamp_range(0.0..=1.0e8) - .speed(speed), - ) - .on_hover_text( - "Scales the radii of the points in the backprojected point cloud.\n\ - This is a factor of the projected pixel diameter. \ - This means a scale of 0.5 will leave adjacent pixels at the same depth value just touching.\n\ - Double-click to reset.", - ); - if response.double_clicked() { - data_result.clear_individual_override::(ctx.viewer_ctx); - response.surrender_focus(); - } else if response.changed() { - data_result.save_individual_override(ctx.viewer_ctx, &FillRatio(value.into())); - } - ui.end_row(); } diff --git a/crates/re_selection_panel/src/visualizer_ui.rs b/crates/re_selection_panel/src/visualizer_ui.rs new file mode 100644 index 000000000000..617a54c3e243 --- /dev/null +++ b/crates/re_selection_panel/src/visualizer_ui.rs @@ -0,0 +1,339 @@ +use itertools::Itertools; + +use re_data_ui::DataUi; +use re_entity_db::EntityDb; +use re_log_types::EntityPath; +use re_space_view::latest_at_with_blueprint_resolved_data; +use re_types_core::components::VisualizerOverrides; +use re_ui::{list_item, ContextExt as _, UiExt as _}; +use re_viewer_context::{ + DataResult, SpaceViewClassExt as _, UiLayout, ViewContext, ViewSystemIdentifier, +}; +use re_viewport_blueprint::SpaceViewBlueprint; + +pub fn visualizer_ui( + ctx: &ViewContext<'_>, + space_view: &SpaceViewBlueprint, + entity_path: &EntityPath, + ui: &mut egui::Ui, +) { + let recording = ctx.recording(); + + let query_result = ctx.lookup_query_result(space_view.id); + let Some(data_result) = query_result + .tree + .lookup_result_by_path(entity_path) + .cloned() + else { + ui.label(ui.ctx().error_text("Entity not found in view.")); + return; + }; + + let Some(override_path) = data_result.individual_override_path() else { + if cfg!(debug_assertions) { + re_log::error!("No override path for entity: {}", data_result.entity_path); + } + return; + }; + + let active_visualizers: Vec<_> = data_result.visualizers.iter().sorted().copied().collect(); + + add_new_visualizer( + ctx, + recording, + ui, + space_view, + &data_result, + &active_visualizers, + ); + + let remove_visualizer_button = |ui: &mut egui::Ui, vis_name: ViewSystemIdentifier| { + let response = ui.small_icon_button(&re_ui::icons::CLOSE); + if response.clicked() { + let component = VisualizerOverrides::from( + active_visualizers + .iter() + .filter(|v| *v != &vis_name) + .map(|v| re_types_core::ArrowString::from(v.as_str())) + .collect::>(), + ); + + ctx.save_blueprint_component(override_path, &component); + } + response + }; + + list_item::list_item_scope(ui, "visualizers", |ui| { + ui.spacing_mut().item_spacing.y = 0.0; + + for &visualizer_id in &active_visualizers { + let default_open = true; + ui.list_item() + .interactive(false) + .show_hierarchical_with_children( + ui, + ui.make_persistent_id(visualizer_id), + default_open, + list_item::LabelContent::new(visualizer_id.as_str()) + .min_desired_width(150.0) + .with_buttons(|ui| remove_visualizer_button(ui, visualizer_id)) + .always_show_buttons(true), + |ui| visualizer_components(ctx, ui, &data_result, visualizer_id), + ); + } + }); +} + +/// Possible sources for a value in the component resolve stack. +/// +/// Mostly for convenience and readability. +enum ValueSource { + Override, + Store, + Default, + FallbackOrPlaceholder, +} + +fn visualizer_components( + ctx: &ViewContext<'_>, + ui: &mut egui::Ui, + data_result: &DataResult, + visualizer_id: ViewSystemIdentifier, +) { + // List all components that the visualizer may consume. + let Ok(visualizer) = ctx.visualizer_collection.get_by_identifier(visualizer_id) else { + re_log::warn!( + "Failed to resolve visualizer identifier {visualizer_id}, to a visualizer implementation" + ); + return; + }; + + let query_info = visualizer.visualizer_query_info(); + + let store_query = ctx.current_query(); + let query_ctx = ctx.query_context(data_result, &store_query); + + // Query fully resolved data. + let query_result = latest_at_with_blueprint_resolved_data( + ctx, + None, // TODO(andreas): Figure out how to deal with annotation context here. + &store_query, + data_result, + query_info.queried.iter().copied(), + ); + + // TODO(andreas): Should we show required components in a special way? + for &component in query_info.queried.iter() { + if component.is_indicator_component() { + continue; + } + + // TODO(andreas): What about annotation context? + + // Query all the sources for our value. + // (technically we only need to query those that are shown, but rolling this out makes things easier). + let result_override = query_result.overrides.get(component); + let raw_override = result_override.and_then(|r| r.try_raw(&query_result.resolver)); + let non_empty_override = raw_override.as_ref().map_or(false, |r| !r.is_empty()); + + let result_store = query_result.results.get(component); + let raw_store = result_store.and_then(|r| r.try_raw(&query_result.resolver)); + let non_empty_store = raw_store.as_ref().map_or(false, |r| !r.is_empty()); + + let result_default = query_result.defaults.get(component); + let raw_default = result_default.and_then(|r| r.try_raw(&query_result.resolver)); + let non_empty_default = raw_default.as_ref().map_or(false, |r| !r.is_empty()); + + let raw_fallback = match visualizer.fallback_for(&query_ctx, component) { + Ok(fallback) => fallback, + Err(err) => { + re_log::warn_once!("Failed to get fallback for component {component}: {err}"); + continue; // TODO(andreas): Don't give up on the entire component because of this. Show an error instead. + } + }; + + // Determine where the final value comes from. + // Putting this into an enum makes it easier to reason about the next steps. + let value_source = match (non_empty_override, non_empty_store, non_empty_default) { + (true, _, _) => ValueSource::Override, + (false, true, _) => ValueSource::Store, + (false, false, true) => ValueSource::Default, + (false, false, false) => ValueSource::FallbackOrPlaceholder, + }; + + #[allow(clippy::unwrap_used)] // We checked earlier that these values are valid! + let raw_current_value = match value_source { + ValueSource::Override => raw_override.as_ref().unwrap(), + ValueSource::Store => raw_store.as_ref().unwrap(), + ValueSource::Default => raw_default.as_ref().unwrap(), + ValueSource::FallbackOrPlaceholder => &raw_fallback, + } + .as_ref(); + + let Some(override_path) = data_result.individual_override_path() else { + // This shouldn't the `DataResult` is valid. + if cfg!(debug_assertions) { + re_log::error!("No override path for entity: {}", data_result.entity_path); + } + return; + }; + + let value_fn = |ui: &mut egui::Ui, _style| { + // Edit ui can only handle a single value. + let multiline = false; + if raw_current_value.len() > 1 + || !ctx.viewer_ctx.component_ui_registry.try_show_edit_ui( + ctx.viewer_ctx, + ui, + raw_current_value, + override_path, + component, + multiline, + ) + { + // TODO(andreas): Unfortunately, display ui needs db & query. (fix that!) + // In fact some display UIs will struggle since they try to query additional data from the store. + // so we have to figure out what store and path things come from. + let bp_query = ctx.viewer_ctx.blueprint_query; + + #[allow(clippy::unwrap_used)] // We checked earlier that these values are valid! + let (query, db, entity_path, latest_at_results) = match value_source { + ValueSource::Override => ( + bp_query, + ctx.blueprint_db(), + override_path, + result_override.unwrap(), + ), + ValueSource::Store => ( + &store_query, + ctx.recording(), + &data_result.entity_path, + result_store.unwrap(), + ), + ValueSource::Default => ( + bp_query, + ctx.blueprint_db(), + ctx.defaults_path, + result_default.unwrap(), + ), + ValueSource::FallbackOrPlaceholder => { + // Fallback values are always single values, so we can directly go to the component ui. + // TODO(andreas): db & entity path don't make sense here. + ctx.viewer_ctx.component_ui_registry.ui_raw( + ctx.viewer_ctx, + ui, + UiLayout::List, + &store_query, + ctx.recording(), + &data_result.entity_path, + component, + raw_current_value, + ); + return; + } + }; + + re_data_ui::EntityLatestAtResults { + entity_path: entity_path.clone(), + results: latest_at_results, + } + .data_ui(ctx.viewer_ctx, ui, UiLayout::List, query, db); + } + }; + + // TODO(andreas): Add a "more" button for options like "remove override" etc. + // TODO(andreas): Add subitems for showing override/store/default/fallback values + easy removal etc. + ui.list_item() + .interactive(false) + .show_flat( + ui, + list_item::PropertyContent::new(component.short_name()).value_fn(value_fn), + ) + .on_hover_text(component.full_name()); + } +} + +fn add_new_visualizer( + ctx: &ViewContext<'_>, + entity_db: &EntityDb, + ui: &mut egui::Ui, + space_view: &SpaceViewBlueprint, + data_result: &DataResult, + active_visualizers: &[ViewSystemIdentifier], +) { + // If we don't have an override_path we can't set up an initial override + // this shouldn't happen if the `DataResult` is valid. + let Some(override_path) = data_result.individual_override_path() else { + if cfg!(debug_assertions) { + re_log::error!("No override path for entity: {}", data_result.entity_path); + } + return; + }; + + // TODO(jleibs): This has already been computed for the SpaceView this frame. Maybe We + // should do this earlier and store it with the SpaceView? + let applicable_entities_per_visualizer = ctx + .viewer_ctx + .space_view_class_registry + .applicable_entities_for_visualizer_systems(entity_db.store_id()); + + let visualizable_entities = space_view + .class(ctx.viewer_ctx.space_view_class_registry) + .determine_visualizable_entities( + &applicable_entities_per_visualizer, + entity_db, + &ctx.visualizer_collection, + &space_view.space_origin, + ); + + let visualizer_options = visualizable_entities + .iter() + .filter(|(vis, ents)| { + ents.contains(&data_result.entity_path) && !active_visualizers.contains(vis) + }) + .map(|(vis, _)| vis) + .sorted() + .collect::>(); + + let enabled = !visualizer_options.is_empty(); + + let mut opened = false; + + ui.add_enabled_ui(enabled, |ui| { + let menu = ui + .menu_button("Add", |ui| { + ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); + opened = true; + + if visualizer_options.is_empty() { + ui.close_menu(); + } + + // Present an option to enable any visualizer that isn't already enabled. + for viz in visualizer_options { + if ui.button(viz.as_str()).clicked() { + let component = VisualizerOverrides::from( + active_visualizers + .iter() + .chain(std::iter::once(viz)) + .map(|v| { + let arrow_str: re_types_core::ArrowString = v.as_str().into(); + arrow_str + }) + .collect::>(), + ); + + ctx.save_blueprint_component(override_path, &component); + + ui.close_menu(); + } + } + }) + .response + .on_disabled_hover_text("No additional visualizers available."); + + if !opened { + menu.on_hover_text("Choose a component to specify an override value.".to_owned()); + } + }); +} diff --git a/crates/re_space_view/src/query.rs b/crates/re_space_view/src/query.rs index 65b7a5e165d1..f8549d8d54eb 100644 --- a/crates/re_space_view/src/query.rs +++ b/crates/re_space_view/src/query.rs @@ -172,7 +172,7 @@ fn query_overrides<'a>( } pub trait DataResultQuery { - fn latest_at_with_overrides<'a, A: re_types_core::Archetype>( + fn latest_at_with_blueprint_resolved_data<'a, A: re_types_core::Archetype>( &'a self, ctx: &'a ViewContext<'a>, latest_at_query: &'a LatestAtQuery, @@ -186,7 +186,7 @@ pub trait DataResultQuery { } impl DataResultQuery for DataResult { - fn latest_at_with_overrides<'a, A: re_types_core::Archetype>( + fn latest_at_with_blueprint_resolved_data<'a, A: re_types_core::Archetype>( &'a self, ctx: &'a ViewContext<'a>, latest_at_query: &'a LatestAtQuery, diff --git a/crates/re_space_view/src/results_ext.rs b/crates/re_space_view/src/results_ext.rs index 6a400964abe8..32c07e152c18 100644 --- a/crates/re_space_view/src/results_ext.rs +++ b/crates/re_space_view/src/results_ext.rs @@ -16,9 +16,9 @@ use crate::DataResultQuery as _; /// Although overrides are never temporal, when accessed via the [`crate::RangeResultsExt`] trait /// they will be merged into the results appropriately. pub struct HybridLatestAtResults<'a> { - pub(crate) overrides: LatestAtResults, - pub(crate) results: LatestAtResults, - pub(crate) defaults: LatestAtResults, + pub overrides: LatestAtResults, + pub results: LatestAtResults, + pub defaults: LatestAtResults, pub ctx: &'a ViewContext<'a>, pub query: LatestAtQuery, pub data_result: &'a DataResult, diff --git a/crates/re_space_view_spatial/src/contexts/transform_context.rs b/crates/re_space_view_spatial/src/contexts/transform_context.rs index fe7ccededb49..5899a24e22bb 100644 --- a/crates/re_space_view_spatial/src/contexts/transform_context.rs +++ b/crates/re_space_view_spatial/src/contexts/transform_context.rs @@ -211,7 +211,8 @@ impl TransformContext { .lookup_result_by_path(p) .cloned() .map(|data_result| { - let results = data_result.latest_at_with_overrides::(ctx, query); + let results = data_result + .latest_at_with_blueprint_resolved_data::(ctx, query); results.get_mono_with_fallback::() }) diff --git a/crates/re_space_view_spatial/src/lib.rs b/crates/re_space_view_spatial/src/lib.rs index 0a9d0e500223..8883973dd040 100644 --- a/crates/re_space_view_spatial/src/lib.rs +++ b/crates/re_space_view_spatial/src/lib.rs @@ -66,7 +66,8 @@ fn query_pinhole( query: &re_data_store::LatestAtQuery, data_result: &re_viewer_context::DataResult, ) -> Option { - let results = data_result.latest_at_with_overrides::(ctx, query); + let results = data_result + .latest_at_with_blueprint_resolved_data::(ctx, query); let image_from_camera = results.get_mono()?; diff --git a/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs b/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs index ab2a7e2e4e77..67705c00546d 100644 --- a/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs +++ b/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs @@ -84,7 +84,8 @@ impl VisualizerSystem for Transform3DArrowsVisualizer { world_from_obj, ); - let results = data_result.latest_at_with_overrides::(ctx, &latest_at_query); + let results = + data_result.latest_at_with_blueprint_resolved_data::(ctx, &latest_at_query); let axis_length = results.get_mono_with_fallback::().into(); add_axis_arrows( @@ -178,8 +179,8 @@ impl TypedComponentFallbackProvider for Transform3DArrowsVisualizer .visualizers .contains(&CamerasVisualizer::identifier()) { - let results = - data_result.latest_at_with_overrides::(view_ctx, ctx.query); + let results = data_result + .latest_at_with_blueprint_resolved_data::(view_ctx, ctx.query); Some(results.get_mono_with_fallback::()) } else { diff --git a/crates/re_space_view_time_series/src/line_visualizer_system.rs b/crates/re_space_view_time_series/src/line_visualizer_system.rs index 55641b804365..421c8cc26c3e 100644 --- a/crates/re_space_view_time_series/src/line_visualizer_system.rs +++ b/crates/re_space_view_time_series/src/line_visualizer_system.rs @@ -6,7 +6,7 @@ use re_types::components::AggregationPolicy; use re_types::{ archetypes::SeriesLine, components::{Color, Name, Scalar, StrokeWidth}, - Archetype as _, ComponentNameSet, Loggable, + Archetype as _, Loggable, }; use re_viewer_context::{ IdentifiedViewSystem, QueryContext, SpaceViewSystemExecutionError, @@ -36,11 +36,9 @@ const DEFAULT_STROKE_WIDTH: f32 = 0.75; impl VisualizerSystem for SeriesLineSystem { fn visualizer_query_info(&self) -> VisualizerQueryInfo { let mut query_info = VisualizerQueryInfo::from_archetype::(); - let mut series_line_queried: ComponentNameSet = SeriesLine::all_components() - .iter() - .map(ToOwned::to_owned) - .collect::(); - query_info.queried.append(&mut series_line_queried); + query_info + .queried + .extend(SeriesLine::all_components().iter().map(ToOwned::to_owned)); query_info.indicators = std::iter::once(SeriesLine::indicator().name()).collect(); query_info } diff --git a/crates/re_space_view_time_series/src/point_visualizer_system.rs b/crates/re_space_view_time_series/src/point_visualizer_system.rs index 010ed6908ee4..a6c19b31d9bb 100644 --- a/crates/re_space_view_time_series/src/point_visualizer_system.rs +++ b/crates/re_space_view_time_series/src/point_visualizer_system.rs @@ -5,7 +5,7 @@ use re_space_view::range_with_blueprint_resolved_data; use re_types::{ archetypes::{self, SeriesPoint}, components::{Color, MarkerShape, MarkerSize, Name, Scalar}, - Archetype as _, ComponentNameSet, Loggable, + Archetype as _, Loggable, }; use re_viewer_context::{ IdentifiedViewSystem, QueryContext, SpaceViewSystemExecutionError, @@ -38,11 +38,9 @@ const DEFAULT_MARKER_SIZE: f32 = 3.0; impl VisualizerSystem for SeriesPointSystem { fn visualizer_query_info(&self) -> VisualizerQueryInfo { let mut query_info = VisualizerQueryInfo::from_archetype::(); - let mut series_point_queried: ComponentNameSet = SeriesPoint::all_components() - .iter() - .map(ToOwned::to_owned) - .collect::(); - query_info.queried.append(&mut series_point_queried); + query_info + .queried + .extend(SeriesPoint::all_components().iter().map(ToOwned::to_owned)); query_info.indicators = std::iter::once(SeriesPoint::indicator().name()).collect(); query_info } diff --git a/crates/re_viewer/src/ui/rerun_menu.rs b/crates/re_viewer/src/ui/rerun_menu.rs index 197c0fb6586a..fa6fea0a61bf 100644 --- a/crates/re_viewer/src/ui/rerun_menu.rs +++ b/crates/re_viewer/src/ui/rerun_menu.rs @@ -371,14 +371,6 @@ fn experimental_feature_ui( "Plots: query clamping", ) .on_hover_text("Toggle query clamping for the plot visualizers."); - - ui - .re_checkbox( - - &mut app_options.experimental_visualizer_selection, - "Visualizer selection for all views", - ) - .on_hover_text("Enables explicit visualizer selection for all views, not just Time Series where it's default enabled."); } #[cfg(debug_assertions)] diff --git a/crates/re_viewer_context/Cargo.toml b/crates/re_viewer_context/Cargo.toml index 420bc01c43e7..b889c7a5eadd 100644 --- a/crates/re_viewer_context/Cargo.toml +++ b/crates/re_viewer_context/Cargo.toml @@ -45,6 +45,7 @@ glam = { workspace = true, features = ["serde"] } half.workspace = true indexmap = { workspace = true, features = ["std", "serde"] } itertools.workspace = true +linked-hash-map.workspace = true macaw.workspace = true ndarray.workspace = true nohash-hasher.workspace = true diff --git a/crates/re_viewer_context/src/app_options.rs b/crates/re_viewer_context/src/app_options.rs index 15ee53e05492..73f880f3e3d9 100644 --- a/crates/re_viewer_context/src/app_options.rs +++ b/crates/re_viewer_context/src/app_options.rs @@ -23,9 +23,6 @@ pub struct AppOptions { /// Toggle query clamping for the plot visualizers. pub experimental_plot_query_clamping: bool, - /// Toggle explicit visualizer selection for all views. - pub experimental_visualizer_selection: bool, - /// Displays an overlay for debugging picking. pub show_picking_debug_overlay: bool, @@ -57,8 +54,6 @@ impl Default for AppOptions { experimental_plot_query_clamping: false, - experimental_visualizer_selection: false, - show_picking_debug_overlay: false, inspect_blueprint_timeline: false, diff --git a/crates/re_viewer_context/src/component_ui_registry.rs b/crates/re_viewer_context/src/component_ui_registry.rs index 801271ad329c..0955045ed314 100644 --- a/crates/re_viewer_context/src/component_ui_registry.rs +++ b/crates/re_viewer_context/src/component_ui_registry.rs @@ -154,8 +154,7 @@ type ComponentUiCallback = Box< &LatestAtQuery, &EntityDb, &EntityPath, - &LatestAtComponentResults, - &Instance, + &dyn arrow2::array::Array, ) + Send + Sync, >; @@ -335,7 +334,10 @@ impl ComponentUiRegistry { types } - /// Show a ui for this instance of this component. + /// Show a ui for a component instance. + /// + /// Has a fallback to show an info text if the instance is not specific, + /// but in these cases `LatestAtComponentResults::data_ui` should be used instead! #[allow(clippy::too_many_arguments)] pub fn ui( &self, @@ -353,48 +355,57 @@ impl ComponentUiRegistry { return; }; - re_tracing::profile_function!(component_name.full_name()); - - // Use the ui callback if there is one. - if let Some(ui_callback) = self.component_uis.get(&component_name) { - (*ui_callback)( + if !instance.is_specific() { + ui.label(format!("({instance} values)")); + } else if let Some(component_raw) = + component.instance_raw(db.resolver(), component_name, instance.get() as _) + { + self.ui_raw( ctx, ui, ui_layout, query, db, entity_path, - component, - instance, + component_name, + component_raw.as_ref(), ); + } else { + ui.label("(empty)"); + } + } + + /// Show a ui for a single raw component. + #[allow(clippy::too_many_arguments)] + pub fn ui_raw( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + ui_layout: UiLayout, + query: &LatestAtQuery, + db: &EntityDb, + entity_path: &EntityPath, + component_name: ComponentName, + component_raw: &dyn arrow2::array::Array, + ) { + re_tracing::profile_function!(component_name.full_name()); + + // Use the ui callback if there is one. + if let Some(ui_callback) = self.component_uis.get(&component_name) { + (*ui_callback)(ctx, ui, ui_layout, query, db, entity_path, component_raw); return; } // If there is none but we have a singleline edit_ui and only a single value, use a disabled edit ui. - if instance.is_specific() { - if let Some(edit_ui) = self.component_singleline_editors.get(&component_name) { - if let Some(raw_component) = - component.instance_raw(db.resolver(), component_name, instance.get() as _) - { - ui.scope(|ui| { - ui.disable(); - (*edit_ui)(ctx, ui, raw_component.as_ref()); - }); - return; - } - } + if let Some(edit_ui) = self.component_singleline_editors.get(&component_name) { + ui.scope(|ui| { + ui.disable(); + (*edit_ui)(ctx, ui, component_raw); + }); + return; } - (*self.fallback_ui)( - ctx, - ui, - ui_layout, - query, - db, - entity_path, - component, - instance, - ); + (*self.fallback_ui)(ctx, ui, ui_layout, query, db, entity_path, component_raw); } /// Show a multi-line editor for this instance of this component. @@ -472,6 +483,55 @@ impl ComponentUiRegistry { // TODO(andreas, jleibs): Editors only show & edit the first instance of a component batch. let instance: Instance = 0.into(); + let component_raw = match component_value_or_fallback( + ctx, + component_query_result, + component_name, + instance, + origin_db.resolver(), + fallback_provider, + ) { + Ok(value) => value, + Err(error_text) => { + re_log::error_once!("{error_text}"); + ui.error_label(&error_text); + return; + } + }; + + if !self.try_show_edit_ui( + ctx.viewer_ctx, + ui, + component_raw.as_ref(), + blueprint_write_path, + component_name, + multiline, + ) { + // Even if we can't edit the component, it's still helpful to show what the value is. + self.ui_raw( + ctx.viewer_ctx, + ui, + UiLayout::List, + ctx.query, + origin_db, + ctx.target_entity_path, + component_name, + component_raw.as_ref(), + ); + } + } + + pub fn try_show_edit_ui( + &self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + raw_current_value: &dyn arrow2::array::Array, + blueprint_write_path: &EntityPath, + component_name: ComponentName, + multiline: bool, + ) -> bool { + re_tracing::profile_function!(component_name.full_name()); + let editors = if multiline { &self.component_multiline_editors } else { @@ -479,42 +539,15 @@ impl ComponentUiRegistry { }; if let Some(edit_callback) = editors.get(&component_name) { - let component_value_or_fallback = match component_value_or_fallback( - ctx, - component_query_result, - component_name, - instance, - origin_db.resolver(), - fallback_provider, - ) { - Ok(value) => value, - Err(error_text) => { - re_log::error_once!("{error_text}"); - ui.error_label(&error_text); - return; - } - }; - - if let Some(updated) = - (*edit_callback)(ctx.viewer_ctx, ui, component_value_or_fallback.as_ref()) - { - ctx.viewer_ctx.save_blueprint_data_cell( + if let Some(updated) = (*edit_callback)(ctx, ui, raw_current_value) { + ctx.save_blueprint_data_cell( blueprint_write_path, re_log_types::DataCell::from_arrow(component_name, updated), ); } + true } else { - // Even if we can't edit the component, it's still helpful to show what the value is. - self.ui( - ctx.viewer_ctx, - ui, - UiLayout::List, - ctx.query, - origin_db, - ctx.target_entity_path, - component_query_result, - &instance, - ); + false } } } diff --git a/crates/re_viewer_context/src/space_view/view_query.rs b/crates/re_viewer_context/src/space_view/view_query.rs index 40a63adbd640..c1d3b4f97269 100644 --- a/crates/re_viewer_context/src/space_view/view_query.rs +++ b/crates/re_viewer_context/src/space_view/view_query.rs @@ -238,8 +238,7 @@ impl DataResult { ctx.save_empty_blueprint_component::(individual_override_path); } - #[inline] - pub fn lookup_override( + fn lookup_override( &self, ctx: &ViewerContext<'_>, ) -> Option { diff --git a/crates/re_viewer_context/src/space_view/visualizer_system.rs b/crates/re_viewer_context/src/space_view/visualizer_system.rs index 6b450a401854..de8f54bc0a5a 100644 --- a/crates/re_viewer_context/src/space_view/visualizer_system.rs +++ b/crates/re_viewer_context/src/space_view/visualizer_system.rs @@ -1,6 +1,6 @@ use ahash::HashMap; -use re_types::{Archetype, ComponentNameSet}; +use re_types::{Archetype, ComponentName, ComponentNameSet}; use crate::{ ApplicableEntities, ComponentFallbackProvider, IdentifiedViewSystem, @@ -9,6 +9,33 @@ use crate::{ VisualizerAdditionalApplicabilityFilter, }; +#[derive(Debug, Clone, Default)] +pub struct SortedComponentNameSet(linked_hash_map::LinkedHashMap); + +impl SortedComponentNameSet { + pub fn insert(&mut self, k: ComponentName) -> Option<()> { + self.0.insert(k, ()) + } + + pub fn extend(&mut self, iter: impl IntoIterator) { + self.0.extend(iter.into_iter().map(|k| (k, ()))); + } + + pub fn iter(&self) -> linked_hash_map::Keys<'_, ComponentName, ()> { + self.0.keys() + } + + pub fn contains(&self, k: &ComponentName) -> bool { + self.0.contains_key(k) + } +} + +impl FromIterator for SortedComponentNameSet { + fn from_iter>(iter: I) -> Self { + Self(iter.into_iter().map(|k| (k, ())).collect()) + } +} + pub struct VisualizerQueryInfo { /// These are not required, but if _any_ of these are found, it is a strong indication that this /// system should be active (if also the `required_components` are found). @@ -19,9 +46,11 @@ pub struct VisualizerQueryInfo { /// This does not include indicator components. pub required: ComponentNameSet, - /// Returns the set of components that the system _queries_. - /// Must include required, usually excludes indicators - pub queried: ComponentNameSet, + /// Returns the list of components that the system _queries_. + /// + /// Must include required, usually excludes indicators. + /// Order should reflect order in archetype docs & user code as well as possible. + pub queried: SortedComponentNameSet, } impl VisualizerQueryInfo { @@ -40,7 +69,7 @@ impl VisualizerQueryInfo { Self { indicators: ComponentNameSet::new(), required: ComponentNameSet::new(), - queried: ComponentNameSet::new(), + queried: SortedComponentNameSet::default(), } } } diff --git a/crates/re_viewer_context/src/test_context.rs b/crates/re_viewer_context/src/test_context.rs index a1059d4c659d..81a30b5a26f4 100644 --- a/crates/re_viewer_context/src/test_context.rs +++ b/crates/re_viewer_context/src/test_context.rs @@ -56,7 +56,7 @@ impl TestContext { let blueprint_query = LatestAtQuery::latest(self.active_timeline); let (command_sender, _) = command_channel(); let component_ui_registry = ComponentUiRegistry::new(Box::new( - |_ctx, _ui, _ui_layout, _query, _db, _entity_path, _component, _instance| {}, + |_ctx, _ui, _ui_layout, _query, _db, _entity_path, _component| {}, )); let store_context = StoreContext { From 27a3ebac5a42a76a0f4180a36612b7854a37a53b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 24 Jun 2024 09:33:58 +0200 Subject: [PATCH 2/2] Remove `panic="abort"` from `dev` builds (#6608) The `backtrace` crate isn't compatible with `panic="abort"` (see [docs](https://docs.rs/backtrace/latest/backtrace/) and [issue](https://github.com/rust-lang/backtrace-rs/issues/397)). One great feature we have in egui is pressing down all modifier keys on your keyboard and seeing the backtrace to the widget you hover. This is awesome for when you want to change the code for some specific part of the UI. But it didn't work, at least not always. This PR fixes it. We still use `panic="abort"` in release builds, for the benefit of smaller binaries. * See also https://github.com/emilk/egui/pull/4696 https://github.com/rerun-io/rerun/assets/1148717/cff9ddbe-838c-4e67-937a-e7dd97d1537f ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6608?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6608?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/6608) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --- Cargo.toml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 28ebf99c1bcd..4e96befd78f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -273,8 +273,12 @@ zip = { version = "0.6", default-features = false } # Our dev profile has some optimizations turned on, as well as debug assertions. [profile.dev] -opt-level = 1 # Make debug builds run faster -panic = "abort" # This leads to better optimizations and smaller binaries (and is the default in Wasm anyways). +opt-level = 1 # Make debug builds run faster + +# panic = "abort" leads to better optimizations and smaller binaries (and is the default in Wasm anyways), +# but it also means backtraces don't work with the `backtrace` library (https://github.com/rust-lang/backtrace-rs/issues/397). +# egui has a feature where if you hold down all modifiers keys on your keyboard and hover any UI widget, +# you will see the backtrace to that widget, and we don't want to break that feature in dev builds. [profile.dev.build-override] debug = true # enable debug symbols for build scripts when building in dev (codegen backtraces!)