From 4b1fabdee44b26a1115253c92b2a306b2cbf512d Mon Sep 17 00:00:00 2001 From: Isse Date: Tue, 2 Dec 2025 17:36:47 +0100 Subject: [PATCH 1/4] Make transform frame edit not experimental --- .../re_selection_panel/src/selection_panel.rs | 150 ++++++++++++++---- .../re_viewer/src/ui/settings_screen.rs | 20 --- .../re_viewer_context/src/app_options.rs | 6 - 3 files changed, 119 insertions(+), 57 deletions(-) diff --git a/crates/viewer/re_selection_panel/src/selection_panel.rs b/crates/viewer/re_selection_panel/src/selection_panel.rs index bd58730e8ac7..c6f7965c7ab1 100644 --- a/crates/viewer/re_selection_panel/src/selection_panel.rs +++ b/crates/viewer/re_selection_panel/src/selection_panel.rs @@ -8,7 +8,7 @@ use re_data_ui::{ }; use re_entity_db::{EntityPath, InstancePath}; use re_log_types::{ComponentPath, EntityPathFilter, EntityPathSubs, ResolvedEntityPathFilter}; -use re_types::ComponentDescriptor; +use re_types::{ComponentDescriptor, TransformFrameIdHash}; use re_ui::list_item::ListItemContentButtonsExt as _; use re_ui::{ SyntaxHighlighting as _, UiExt as _, icons, @@ -349,12 +349,7 @@ impl SelectionPanel { // TODO(RR-2700): Come up with something non-experimental. let is_spatial_view = view.class_identifier() == "3D" || view.class_identifier() == "2D"; - if ctx - .global_context - .app_options - .experimental_coordinate_frame_display_and_override - && is_spatial_view - { + if is_spatial_view { coordinate_frame_ui(ui, &view_ctx, data_result); } } @@ -521,7 +516,7 @@ The last rule matching `/world/house` is `+ /world/**`, so it is included. } } -/// Shows the active coordinate frame. +/// Shows the active coordinate frame if it isn't the fallback frame. /// /// This is not technically a visualizer, but it affects visualization, so we show it alongside. fn coordinate_frame_ui(ui: &mut egui::Ui, ctx: &ViewContext<'_>, data_result: &DataResult) { @@ -543,29 +538,111 @@ fn coordinate_frame_ui(ui: &mut egui::Ui, ctx: &ViewContext<'_>, data_result: &D let override_path = data_result.override_path(); - let frame_id_before = query_result - .get_mono_with_fallback::(component) - .to_string(); - let mut frame_id = frame_id_before.clone(); + let result_override = query_result.overrides.get(component); + let raw_override = result_override + .and_then(|c| c.non_empty_component_batch_raw(component)) + .map(|(_, array)| array); + + let Some(frame_id_before) = query_result + .get_mono::(component) + .map(|f| f.to_string()) + .or_else(|| { + if raw_override.is_some() { + Some(String::new()) + } else { + None + } + }) + else { + return; + }; - ui.list_item_flat_noninteractive( - list_item::PropertyContent::new("Coordinate frame") - .value_text_mut(&mut frame_id) - .with_menu_button(&re_ui::icons::MORE, "More options", |ui: &mut egui::Ui| { - let result_override = query_result.overrides.get(component); - let raw_override = result_override - .and_then(|c| c.non_empty_component_batch_raw(component)) - .map(|(_, array)| array); - - crate::visualizer_ui::remove_and_reset_override_buttons( - ctx, - ui, - component_descr.clone(), - override_path, - &raw_override, - ); - }), - ) + let mut frame_id = if raw_override.is_some() { + frame_id_before.clone() + } else { + String::new() + }; + + let caches = ctx.viewer_ctx.store_context.caches; + let transform_cache = caches.entry(|c: &mut re_viewer_context::TransformDatabaseStoreCache| { + c.lock_transform_cache(ctx.recording()) + }); + + let property_content = list_item::PropertyContent::new("Coordinate frame") + .value_fn(|ui, _| { + let frame_exists = transform_cache + .frame_id_registry() + .lookup_frame_id(TransformFrameIdHash::from_str(&frame_id)) + .is_some(); + let mut text_edit = + egui::TextEdit::singleline(&mut frame_id).hint_text(&frame_id_before); + if !frame_exists { + text_edit = text_edit.text_color(ui.tokens().error_fg_color); + } + let response = ui.add(text_edit); + + let mut suggestions = transform_cache + .frame_id_registry() + .iter_frame_ids() + // Only show named frames. + .filter(|(_, id)| !id.is_entity_path_derived()) + .filter_map(|(_, id)| id.strip_prefix(&frame_id)) + .filter(|rest| !rest.is_empty()) + .collect::>(); + + suggestions.sort_unstable(); + + let width = response.rect.width(); + egui::Popup::from_response(&response) + .style(re_ui::menu::menu_style()) + .open((response.has_focus() || response.lost_focus()) && !suggestions.is_empty()) + .show(|ui| { + ui.set_width(width); + + egui::ScrollArea::vertical() + .min_scrolled_height(350.0) + .max_height(350.0) + .show(ui, |ui| { + for rest in suggestions { + let mut job = egui::text::LayoutJob::default(); + job.append( + &frame_id, + 0.0, + egui::TextFormat::simple( + ui.style().text_styles[&egui::TextStyle::Body].clone(), + ui.tokens().text_default, + ), + ); + job.append( + rest, + 0.0, + egui::TextFormat::simple( + ui.style().text_styles[&egui::TextStyle::Body].clone(), + ui.tokens().text_subdued, + ), + ); + + if ui + .add(egui::Button::new(job).min_size(egui::vec2(width, 0.0))) + .clicked() + { + frame_id.push_str(rest); + } + } + }); + }); + }) + .with_menu_button(&re_ui::icons::MORE, "More options", |ui: &mut egui::Ui| { + crate::visualizer_ui::remove_and_reset_override_buttons( + ctx, + ui, + component_descr.clone(), + override_path, + &raw_override, + ); + }); + + ui.list_item_flat_noninteractive(property_content) .on_hover_ui(|ui| { ui.markdown_ui( "The coordinate frame this entity is associated with. @@ -574,7 +651,18 @@ To learn more about coordinate frames, see the [Spaces & Transforms](https://rer ); }); - if frame_id_before != frame_id { + if raw_override.is_some() { + if frame_id.is_empty() { + ctx.clear_blueprint_component(override_path.clone(), component_descr); + } else if frame_id_before != frame_id { + // Save as blueprint override. + ctx.save_blueprint_component( + override_path.clone(), + &component_descr, + &TransformFrameId::new(&frame_id), + ); + } + } else if !frame_id.is_empty() { // Save as blueprint override. ctx.save_blueprint_component( override_path.clone(), diff --git a/crates/viewer/re_viewer/src/ui/settings_screen.rs b/crates/viewer/re_viewer/src/ui/settings_screen.rs index 870186097bbb..f0576a170e6c 100644 --- a/crates/viewer/re_viewer/src/ui/settings_screen.rs +++ b/crates/viewer/re_viewer/src/ui/settings_screen.rs @@ -176,26 +176,6 @@ fn settings_screen_ui_impl(ui: &mut egui::Ui, app_options: &mut AppOptions, keep separator_with_some_space(ui); ui.strong("Video"); video_section_ui(ui, app_options); - - // - // Experimental features - // - - //#[cfg(not(target_arch = "wasm32"))] - if true { - separator_with_some_space(ui); - ui.strong("Experimental features"); - - ui.re_checkbox( - &mut app_options.experimental_coordinate_frame_display_and_override, - "Display coordinate frames", - ) - .on_hover_ui(|ui| { - ui.markdown_ui( - "Every entity is associated with a coordinate frame id. Enabling this shows them in 2D & 3D views and allows blueprint overrides for them.", - ); - }); - } } fn video_section_ui(ui: &mut Ui, app_options: &mut AppOptions) { diff --git a/crates/viewer/re_viewer_context/src/app_options.rs b/crates/viewer/re_viewer_context/src/app_options.rs index 09299949c7a4..08679e18412b 100644 --- a/crates/viewer/re_viewer_context/src/app_options.rs +++ b/crates/viewer/re_viewer_context/src/app_options.rs @@ -63,10 +63,6 @@ pub struct AppOptions { /// see [`AppOptions::cache_subdirectory`]. #[cfg(not(target_arch = "wasm32"))] pub cache_directory: Option, - - /// Enables experimental coordinate frame id overrides. - // TODO(RR-2700): Come up with something non-experimental. - pub experimental_coordinate_frame_display_and_override: bool, } impl Default for AppOptions { @@ -94,8 +90,6 @@ impl Default for AppOptions { #[cfg(not(target_arch = "wasm32"))] cache_directory: Self::default_cache_directory(), - - experimental_coordinate_frame_display_and_override: false, } } } From 6b82de065107cdd289c3cc9979bd1378f21dfeb2 Mon Sep 17 00:00:00 2001 From: Isse Date: Tue, 2 Dec 2025 17:50:39 +0100 Subject: [PATCH 2/4] format code --- .../re_selection_panel/src/selection_panel.rs | 146 ++++++++++-------- 1 file changed, 80 insertions(+), 66 deletions(-) diff --git a/crates/viewer/re_selection_panel/src/selection_panel.rs b/crates/viewer/re_selection_panel/src/selection_panel.rs index c6f7965c7ab1..2f92875bc5c7 100644 --- a/crates/viewer/re_selection_panel/src/selection_panel.rs +++ b/crates/viewer/re_selection_panel/src/selection_panel.rs @@ -563,74 +563,9 @@ fn coordinate_frame_ui(ui: &mut egui::Ui, ctx: &ViewContext<'_>, data_result: &D String::new() }; - let caches = ctx.viewer_ctx.store_context.caches; - let transform_cache = caches.entry(|c: &mut re_viewer_context::TransformDatabaseStoreCache| { - c.lock_transform_cache(ctx.recording()) - }); - let property_content = list_item::PropertyContent::new("Coordinate frame") .value_fn(|ui, _| { - let frame_exists = transform_cache - .frame_id_registry() - .lookup_frame_id(TransformFrameIdHash::from_str(&frame_id)) - .is_some(); - let mut text_edit = - egui::TextEdit::singleline(&mut frame_id).hint_text(&frame_id_before); - if !frame_exists { - text_edit = text_edit.text_color(ui.tokens().error_fg_color); - } - let response = ui.add(text_edit); - - let mut suggestions = transform_cache - .frame_id_registry() - .iter_frame_ids() - // Only show named frames. - .filter(|(_, id)| !id.is_entity_path_derived()) - .filter_map(|(_, id)| id.strip_prefix(&frame_id)) - .filter(|rest| !rest.is_empty()) - .collect::>(); - - suggestions.sort_unstable(); - - let width = response.rect.width(); - egui::Popup::from_response(&response) - .style(re_ui::menu::menu_style()) - .open((response.has_focus() || response.lost_focus()) && !suggestions.is_empty()) - .show(|ui| { - ui.set_width(width); - - egui::ScrollArea::vertical() - .min_scrolled_height(350.0) - .max_height(350.0) - .show(ui, |ui| { - for rest in suggestions { - let mut job = egui::text::LayoutJob::default(); - job.append( - &frame_id, - 0.0, - egui::TextFormat::simple( - ui.style().text_styles[&egui::TextStyle::Body].clone(), - ui.tokens().text_default, - ), - ); - job.append( - rest, - 0.0, - egui::TextFormat::simple( - ui.style().text_styles[&egui::TextStyle::Body].clone(), - ui.tokens().text_subdued, - ), - ); - - if ui - .add(egui::Button::new(job).min_size(egui::vec2(width, 0.0))) - .clicked() - { - frame_id.push_str(rest); - } - } - }); - }); + frame_id_edit(ctx, ui, &mut frame_id, &frame_id_before); }) .with_menu_button(&re_ui::icons::MORE, "More options", |ui: &mut egui::Ui| { crate::visualizer_ui::remove_and_reset_override_buttons( @@ -672,6 +607,85 @@ To learn more about coordinate frames, see the [Spaces & Transforms](https://rer } } +fn frame_id_edit( + ctx: &ViewContext<'_>, + ui: &mut egui::Ui, + frame_id: &mut String, + frame_id_before: &String, +) { + let caches = ctx.viewer_ctx.store_context.caches; + let transform_cache = caches.entry(|c: &mut re_viewer_context::TransformDatabaseStoreCache| { + c.lock_transform_cache(ctx.recording()) + }); + + let frame_exists = transform_cache + .frame_id_registry() + .lookup_frame_id(TransformFrameIdHash::from_str(&*frame_id)) + .is_some(); + let mut text_edit = egui::TextEdit::singleline(frame_id).hint_text(frame_id_before); + if !frame_exists { + text_edit = text_edit.text_color(ui.tokens().error_fg_color); + } + let response = ui.add(text_edit); + + let mut suggestions = transform_cache + .frame_id_registry() + .iter_frame_ids() + // Only show named frames. + .filter(|(_, id)| !id.is_entity_path_derived()) + .filter_map(|(_, id)| id.strip_prefix(&*frame_id)) + .filter(|rest| !rest.is_empty()) + .collect::>(); + + let suggestions_open = + (response.has_focus() || response.lost_focus()) && !suggestions.is_empty(); + + suggestions.sort_unstable(); + + let width = response.rect.width(); + + let suggestions_ui = |ui: &mut egui::Ui| { + for rest in suggestions { + let mut job = egui::text::LayoutJob::default(); + job.append( + &*frame_id, + 0.0, + egui::TextFormat::simple( + ui.style().text_styles[&egui::TextStyle::Body].clone(), + ui.tokens().text_default, + ), + ); + job.append( + rest, + 0.0, + egui::TextFormat::simple( + ui.style().text_styles[&egui::TextStyle::Body].clone(), + ui.tokens().text_subdued, + ), + ); + + if ui + .add(egui::Button::new(job).min_size(egui::vec2(width, 0.0))) + .clicked() + { + frame_id.push_str(rest); + } + } + }; + + egui::Popup::from_response(&response) + .style(re_ui::menu::menu_style()) + .open(suggestions_open) + .show(|ui: &mut egui::Ui| { + ui.set_width(width); + + egui::ScrollArea::vertical() + .min_scrolled_height(350.0) + .max_height(350.0) + .show(ui, suggestions_ui); + }); +} + fn show_recording_properties( ctx: &ViewerContext<'_>, db: &re_entity_db::EntityDb, From 6a529dbd011de6f7a8f5e0effbfeff8d0cd1b042 Mon Sep 17 00:00:00 2001 From: Isse Date: Wed, 3 Dec 2025 09:47:57 +0100 Subject: [PATCH 3/4] address review --- .../re_selection_panel/src/selection_panel.rs | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/crates/viewer/re_selection_panel/src/selection_panel.rs b/crates/viewer/re_selection_panel/src/selection_panel.rs index 2f92875bc5c7..3b801b931fbd 100644 --- a/crates/viewer/re_selection_panel/src/selection_panel.rs +++ b/crates/viewer/re_selection_panel/src/selection_panel.rs @@ -346,7 +346,6 @@ impl SelectionPanel { let view_ctx = view.bundle_context_with_states(ctx, view_states); visible_interactive_toggle_ui(&view_ctx, ui, query_result, data_result); - // TODO(RR-2700): Come up with something non-experimental. let is_spatial_view = view.class_identifier() == "3D" || view.class_identifier() == "2D"; if is_spatial_view { @@ -613,41 +612,49 @@ fn frame_id_edit( frame_id: &mut String, frame_id_before: &String, ) { - let caches = ctx.viewer_ctx.store_context.caches; - let transform_cache = caches.entry(|c: &mut re_viewer_context::TransformDatabaseStoreCache| { - c.lock_transform_cache(ctx.recording()) - }); + let (frame_exists, mut suggestions) = { + // In a scope to not hold the lock for longer than needed. + let caches = ctx.viewer_ctx.store_context.caches; + let transform_cache = + caches.entry(|c: &mut re_viewer_context::TransformDatabaseStoreCache| { + c.lock_transform_cache(ctx.recording()) + }); + + let frame_exists = transform_cache + .frame_id_registry() + .lookup_frame_id(TransformFrameIdHash::from_str(&*frame_id)) + .is_some(); + + let suggestions = transform_cache + .frame_id_registry() + .iter_frame_ids() + // Only show named frames. + .filter(|(_, id)| !id.is_entity_path_derived()) + .filter_map(|(_, id)| id.strip_prefix(&*frame_id)) + .filter(|rest| !rest.is_empty()) + .map(|rest| rest.to_owned()) + .collect::>(); + + (frame_exists, suggestions) + }; + + suggestions.sort_unstable(); - let frame_exists = transform_cache - .frame_id_registry() - .lookup_frame_id(TransformFrameIdHash::from_str(&*frame_id)) - .is_some(); let mut text_edit = egui::TextEdit::singleline(frame_id).hint_text(frame_id_before); if !frame_exists { text_edit = text_edit.text_color(ui.tokens().error_fg_color); } let response = ui.add(text_edit); - let mut suggestions = transform_cache - .frame_id_registry() - .iter_frame_ids() - // Only show named frames. - .filter(|(_, id)| !id.is_entity_path_derived()) - .filter_map(|(_, id)| id.strip_prefix(&*frame_id)) - .filter(|rest| !rest.is_empty()) - .collect::>(); - let suggestions_open = (response.has_focus() || response.lost_focus()) && !suggestions.is_empty(); - suggestions.sort_unstable(); - let width = response.rect.width(); let suggestions_ui = |ui: &mut egui::Ui| { for rest in suggestions { - let mut job = egui::text::LayoutJob::default(); - job.append( + let mut layout_job = egui::text::LayoutJob::default(); + layout_job.append( &*frame_id, 0.0, egui::TextFormat::simple( @@ -655,8 +662,8 @@ fn frame_id_edit( ui.tokens().text_default, ), ); - job.append( - rest, + layout_job.append( + &rest, 0.0, egui::TextFormat::simple( ui.style().text_styles[&egui::TextStyle::Body].clone(), @@ -665,10 +672,10 @@ fn frame_id_edit( ); if ui - .add(egui::Button::new(job).min_size(egui::vec2(width, 0.0))) + .add(egui::Button::new(layout_job).min_size(egui::vec2(width, 0.0))) .clicked() { - frame_id.push_str(rest); + frame_id.push_str(&rest); } } }; From a33a88fe793602328f9c846f7afe4ebc7f746955 Mon Sep 17 00:00:00 2001 From: Isse Date: Wed, 3 Dec 2025 09:57:12 +0100 Subject: [PATCH 4/4] update snapshot --- crates/viewer/re_viewer/tests/snapshots/settings_screen.png | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_viewer/tests/snapshots/settings_screen.png b/crates/viewer/re_viewer/tests/snapshots/settings_screen.png index d630c7eb0f56..b45542dc1b98 100644 --- a/crates/viewer/re_viewer/tests/snapshots/settings_screen.png +++ b/crates/viewer/re_viewer/tests/snapshots/settings_screen.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:fbc37db5977d36c660a850abe78460e79a9441db2d2af92325d3a22679603f74 -size 105919 +oid sha256:6f6caa2f778d76f994fd9a6ac07ad4f1c5cfcc4d7b4e1c7373bcad927c1be60b +size 99584