From be6d679777791ff5567778da953f6caa48c29f40 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Thu, 23 May 2024 13:12:19 +0200 Subject: [PATCH 01/10] Remove the ability to display multiple tensors in a single space view (#6392) ### What - Fixes https://github.com/rerun-io/rerun/issues/6387 Configuring a tensor space view with a query returning more than 1 tensor is now displayed as follows: image

This is consistent with the existing behaviour of TextDocument: image

This PR is very minimal and aims at status quo consistency. This could be further improved: - https://github.com/rerun-io/rerun/issues/6393 ### 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/6392?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/6392?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/6392) - [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`. --- .../src/space_view_class.rs | 204 +++++++----------- .../src/tensor_slice_to_gpu.rs | 12 +- .../src/visualizer_system.rs | 5 +- .../src/space_view_class.rs | 4 +- docs/snippets/all/views/tensor.py | 10 +- .../rerun/blueprint/views/tensor_view.py | 10 +- .../check_mono_entity_views.py | 59 +++++ 7 files changed, 153 insertions(+), 151 deletions(-) create mode 100644 tests/python/release_checklist/check_mono_entity_views.py diff --git a/crates/re_space_view_tensor/src/space_view_class.rs b/crates/re_space_view_tensor/src/space_view_class.rs index e2b6d57008c6..50af4a4557b5 100644 --- a/crates/re_space_view_tensor/src/space_view_class.rs +++ b/crates/re_space_view_tensor/src/space_view_class.rs @@ -32,10 +32,20 @@ type ViewType = re_types::blueprint::views::TensorView; #[derive(Default)] pub struct ViewTensorState { - /// Selects in [`Self::state_tensors`]. - pub selected_tensor: Option, + /// What slice are we viewing? + /// + /// This get automatically reset if/when the current tensor shape changes. + pub(crate) slice: SliceSelection, - pub state_tensors: ahash::HashMap, + /// How we map values to colors. + pub(crate) color_mapping: ColorMapping, + + /// Scaling, filtering, aspect ratio, etc for the rendered texture. + texture_settings: TextureSettings, + + /// Last viewed tensor, copied each frame. + /// Used for the selection view. + tensor: Option<(RowId, DecodedTensor)>, } impl SpaceViewState for ViewTensorState { @@ -58,88 +68,6 @@ pub struct SliceSelection { pub selector_values: BTreeMap, } -pub struct PerTensorState { - /// What slice are we vieiwing? - slice: SliceSelection, - - /// How we map values to colors. - color_mapping: ColorMapping, - - /// Scaling, filtering, aspect ratio, etc for the rendered texture. - texture_settings: TextureSettings, - - /// Last viewed tensor, copied each frame. - /// Used for the selection view. - tensor: Option<(RowId, DecodedTensor)>, -} - -impl PerTensorState { - pub fn create(tensor_data_row_id: RowId, tensor: &DecodedTensor) -> Self { - Self { - slice: SliceSelection { - dim_mapping: DimensionMapping::create(tensor.shape()), - selector_values: Default::default(), - }, - color_mapping: ColorMapping::default(), - texture_settings: TextureSettings::default(), - tensor: Some((tensor_data_row_id, tensor.clone())), - } - } - - pub fn slice(&self) -> &SliceSelection { - &self.slice - } - - pub fn color_mapping(&self) -> &ColorMapping { - &self.color_mapping - } - - pub fn ui(&mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { - let Some((tensor_data_row_id, tensor)) = &self.tensor else { - ui.label("No Tensor shown in this Space View."); - return; - }; - - let tensor_stats = ctx - .cache - .entry(|c: &mut TensorStatsCache| c.entry(*tensor_data_row_id, tensor)); - ctx.re_ui - .selection_grid(ui, "tensor_selection_ui") - .show(ui, |ui| { - // We are in a bare Tensor view -- meaning / meter is unknown. - let meaning = TensorDataMeaning::Unknown; - let meter = None; - tensor_summary_ui_grid_contents( - ctx.re_ui, - ui, - tensor, - tensor, - meaning, - meter, - &tensor_stats, - ); - self.texture_settings.ui(ctx.re_ui, ui); - self.color_mapping.ui(ctx.render_ctx, ctx.re_ui, ui); - }); - - ui.separator(); - ui.strong("Dimension Mapping"); - dimension_mapping_ui(ctx.re_ui, ui, &mut self.slice.dim_mapping, tensor.shape()); - let default_mapping = DimensionMapping::create(tensor.shape()); - if ui - .add_enabled( - self.slice.dim_mapping != default_mapping, - egui::Button::new("Reset mapping"), - ) - .on_disabled_hover_text("The default is already set up") - .on_hover_text("Reset dimension mapping to the default") - .clicked() - { - self.slice.dim_mapping = DimensionMapping::create(tensor.shape()); - } - } -} - impl SpaceViewClass for TensorSpaceView { fn identifier() -> SpaceViewClassIdentifier { ViewType::identifier() @@ -207,11 +135,51 @@ impl SpaceViewClass for TensorSpaceView { _root_entity_properties: &mut EntityProperties, ) -> Result<(), SpaceViewSystemExecutionError> { let state = state.downcast_mut::()?; - if let Some(selected_tensor) = &state.selected_tensor { - if let Some(state_tensor) = state.state_tensors.get_mut(selected_tensor) { - state_tensor.ui(ctx, ui); + + ctx.re_ui + .selection_grid(ui, "tensor_selection_ui") + .show(ui, |ui| { + if let Some((tensor_data_row_id, tensor)) = &state.tensor { + let tensor_stats = ctx + .cache + .entry(|c: &mut TensorStatsCache| c.entry(*tensor_data_row_id, tensor)); + + // We are in a bare Tensor view -- meaning / meter is unknown. + let meaning = TensorDataMeaning::Unknown; + let meter = None; + tensor_summary_ui_grid_contents( + ctx.re_ui, + ui, + tensor, + tensor, + meaning, + meter, + &tensor_stats, + ); + } + + state.texture_settings.ui(ctx.re_ui, ui); + state.color_mapping.ui(ctx.render_ctx, ctx.re_ui, ui); + }); + + if let Some((_, tensor)) = &state.tensor { + ui.separator(); + ui.strong("Dimension Mapping"); + dimension_mapping_ui(ctx.re_ui, ui, &mut state.slice.dim_mapping, tensor.shape()); + let default_mapping = DimensionMapping::create(tensor.shape()); + if ui + .add_enabled( + state.slice.dim_mapping != default_mapping, + egui::Button::new("Reset mapping"), + ) + .on_disabled_hover_text("The default is already set up") + .on_hover_text("Reset dimension mapping to the default") + .clicked() + { + state.slice.dim_mapping = DimensionMapping::create(tensor.shape()); } } + Ok(()) } @@ -238,40 +206,26 @@ impl SpaceViewClass for TensorSpaceView { let tensors = &system_output.view_systems.get::()?.tensors; - if tensors.is_empty() { - ui.centered_and_justified(|ui| ui.label("(empty)")); - state.selected_tensor = None; - } else { - if let Some(selected_tensor) = &state.selected_tensor { - if !tensors.contains_key(selected_tensor) { - state.selected_tensor = None; - } - } - if state.selected_tensor.is_none() { - state.selected_tensor = Some(tensors.iter().next().unwrap().0.clone()); - } - - if tensors.len() > 1 { - // Show radio buttons for the different tensors we have in this view - better than nothing! - ui.horizontal(|ui| { - for instance_path in tensors.keys() { - let is_selected = state.selected_tensor.as_ref() == Some(instance_path); - if ui.radio(is_selected, instance_path.to_string()).clicked() { - state.selected_tensor = Some(instance_path.clone()); - } - } - }); - } + if tensors.len() > 1 { + state.tensor = None; - if let Some(selected_tensor) = &state.selected_tensor { - if let Some((tensor_data_row_id, tensor)) = tensors.get(selected_tensor) { - let state_tensor = state - .state_tensors - .entry(selected_tensor.clone()) - .or_insert_with(|| PerTensorState::create(*tensor_data_row_id, tensor)); - view_tensor(ctx, ui, state_tensor, *tensor_data_row_id, tensor); - } + egui::Frame { + inner_margin: re_ui::ReUi::view_padding().into(), + ..egui::Frame::default() } + .show(ui, |ui| { + ui.label(format!( + "Can only show one tensor at a time; was given {}. Update the query so that it \ + returns a single tensor entity and create additional views for the others.", + tensors.len() + )); + }); + } else if let Some((tensor_data_row_id, tensor)) = tensors.first() { + state.tensor = Some((*tensor_data_row_id, tensor.clone())); + view_tensor(ctx, ui, state, *tensor_data_row_id, tensor); + } else { + state.tensor = None; + ui.centered_and_justified(|ui| ui.label("(empty)")); } Ok(()) @@ -281,14 +235,12 @@ impl SpaceViewClass for TensorSpaceView { fn view_tensor( ctx: &ViewerContext<'_>, ui: &mut egui::Ui, - state: &mut PerTensorState, + state: &mut ViewTensorState, tensor_data_row_id: RowId, tensor: &DecodedTensor, ) { re_tracing::profile_function!(); - state.tensor = Some((tensor_data_row_id, tensor.clone())); - if !state.slice.dim_mapping.is_valid(tensor.num_dim()) { state.slice.dim_mapping = DimensionMapping::create(tensor.shape()); } @@ -339,7 +291,7 @@ fn view_tensor( fn tensor_slice_ui( ctx: &ViewerContext<'_>, ui: &mut egui::Ui, - state: &PerTensorState, + state: &ViewTensorState, tensor_data_row_id: RowId, tensor: &DecodedTensor, dimension_labels: [(String, bool); 2], @@ -358,7 +310,7 @@ fn tensor_slice_ui( fn paint_tensor_slice( ctx: &ViewerContext<'_>, ui: &mut egui::Ui, - state: &PerTensorState, + state: &ViewTensorState, tensor_data_row_id: RowId, tensor: &DecodedTensor, ) -> anyhow::Result<(egui::Response, egui::Painter, egui::Rect)> { @@ -750,7 +702,7 @@ fn paint_axis_names( } } -fn selectors_ui(ui: &mut egui::Ui, state: &mut PerTensorState, tensor: &TensorData) { +fn selectors_ui(ui: &mut egui::Ui, state: &mut ViewTensorState, tensor: &TensorData) { for selector in &state.slice.dim_mapping.selectors { if !selector.visible { continue; diff --git a/crates/re_space_view_tensor/src/tensor_slice_to_gpu.rs b/crates/re_space_view_tensor/src/tensor_slice_to_gpu.rs index 0f124d92dc40..71bddf04d4e7 100644 --- a/crates/re_space_view_tensor/src/tensor_slice_to_gpu.rs +++ b/crates/re_space_view_tensor/src/tensor_slice_to_gpu.rs @@ -12,7 +12,7 @@ use re_viewer_context::{ TensorStats, }; -use crate::space_view_class::{selected_tensor_slice, PerTensorState, SliceSelection}; +use crate::space_view_class::{selected_tensor_slice, SliceSelection, ViewTensorState}; #[derive(thiserror::Error, Debug, PartialEq)] pub enum TensorUploadError { @@ -31,24 +31,22 @@ pub fn colormapped_texture( tensor_data_row_id: RowId, tensor: &DecodedTensor, tensor_stats: &TensorStats, - state: &PerTensorState, + state: &ViewTensorState, ) -> Result> { re_tracing::profile_function!(); let range = tensor_data_range_heuristic(tensor_stats, tensor.dtype()) .map_err(|err| TextureManager2DError::DataCreation(err.into()))?; let texture = - upload_texture_slice_to_gpu(render_ctx, tensor_data_row_id, tensor, state.slice())?; - - let color_mapping = state.color_mapping(); + upload_texture_slice_to_gpu(render_ctx, tensor_data_row_id, tensor, &state.slice)?; Ok(ColormappedTexture { texture, range, decode_srgb: false, multiply_rgb_with_alpha: false, - gamma: color_mapping.gamma, - color_mapper: re_renderer::renderer::ColorMapper::Function(color_mapping.map), + gamma: state.color_mapping.gamma, + color_mapper: re_renderer::renderer::ColorMapper::Function(state.color_mapping.map), shader_decoding: match tensor.buffer { TensorBuffer::Nv12(_) => Some(ShaderDecoding::Nv12), TensorBuffer::Yuy2(_) => Some(ShaderDecoding::Yuy2), diff --git a/crates/re_space_view_tensor/src/visualizer_system.rs b/crates/re_space_view_tensor/src/visualizer_system.rs index c0f0bfb05018..f1c2bc55cff9 100644 --- a/crates/re_space_view_tensor/src/visualizer_system.rs +++ b/crates/re_space_view_tensor/src/visualizer_system.rs @@ -9,7 +9,7 @@ use re_viewer_context::{ #[derive(Default)] pub struct TensorSystem { - pub tensors: std::collections::BTreeMap, + pub tensors: Vec<(RowId, DecodedTensor)>, } impl IdentifiedViewSystem for TensorSystem { @@ -64,8 +64,7 @@ impl TensorSystem { .entry(|c: &mut TensorDecodeCache| c.entry(row_id, tensor.value.0)) { Ok(decoded_tensor) => { - self.tensors - .insert(ent_path.clone(), (row_id, decoded_tensor)); + self.tensors.push((row_id, decoded_tensor)); } Err(err) => { re_log::warn_once!("Failed to decode decoding tensor at path {ent_path}: {err}"); diff --git a/crates/re_space_view_text_document/src/space_view_class.rs b/crates/re_space_view_text_document/src/space_view_class.rs index 529a348a1b9a..2158c2e717f0 100644 --- a/crates/re_space_view_text_document/src/space_view_class.rs +++ b/crates/re_space_view_text_document/src/space_view_class.rs @@ -177,7 +177,9 @@ impl SpaceViewClass for TextDocumentSpaceView { } else { // TODO(jleibs): better handling for multiple results ui.label(format!( - "Can only show one text document at a time; was given {}.", + "Can only show one text document at a time; was given {}. Update \ + the query so that it returns a single text document and create \ + additional views for the others.", text_document.text_entries.len() )); } diff --git a/docs/snippets/all/views/tensor.py b/docs/snippets/all/views/tensor.py index 5f9d3c98c31c..8b31e6e69794 100644 --- a/docs/snippets/all/views/tensor.py +++ b/docs/snippets/all/views/tensor.py @@ -6,12 +6,8 @@ rr.init("rerun_example_tensor", spawn=True) -tensor_one = np.random.randint(0, 256, (8, 6, 3, 5), dtype=np.uint8) -rr.log("tensors/one", rr.Tensor(tensor_one, dim_names=("width", "height", "channel", "batch"))) -tensor_two = np.random.random_sample((10, 20, 30)) -rr.log("tensors/two", rr.Tensor(tensor_two)) - -# Create a tensor view that displays both tensors (you can switch between them inside the view). -blueprint = rrb.Blueprint(rrb.TensorView(origin="/tensors", name="Tensors"), collapse_panels=True) +tensor = np.random.randint(0, 256, (8, 6, 3, 5), dtype=np.uint8) +rr.log("tensor", rr.Tensor(tensor, dim_names=("width", "height", "channel", "batch"))) +blueprint = rrb.Blueprint(rrb.TensorView(origin="tensor", name="Tensor"), collapse_panels=True) rr.send_blueprint(blueprint) diff --git a/rerun_py/rerun_sdk/rerun/blueprint/views/tensor_view.py b/rerun_py/rerun_sdk/rerun/blueprint/views/tensor_view.py index c1a08ca4a252..66241d5d6aef 100644 --- a/rerun_py/rerun_sdk/rerun/blueprint/views/tensor_view.py +++ b/rerun_py/rerun_sdk/rerun/blueprint/views/tensor_view.py @@ -26,14 +26,10 @@ class TensorView(SpaceView): rr.init("rerun_example_tensor", spawn=True) - tensor_one = np.random.randint(0, 256, (8, 6, 3, 5), dtype=np.uint8) - rr.log("tensors/one", rr.Tensor(tensor_one, dim_names=("width", "height", "channel", "batch"))) - tensor_two = np.random.random_sample((10, 20, 30)) - rr.log("tensors/two", rr.Tensor(tensor_two)) - - # Create a tensor view that displays both tensors (you can switch between them inside the view). - blueprint = rrb.Blueprint(rrb.TensorView(origin="/tensors", name="Tensors"), collapse_panels=True) + tensor = np.random.randint(0, 256, (8, 6, 3, 5), dtype=np.uint8) + rr.log("tensor", rr.Tensor(tensor, dim_names=("width", "height", "channel", "batch"))) + blueprint = rrb.Blueprint(rrb.TensorView(origin="tensor", name="Tensor"), collapse_panels=True) rr.send_blueprint(blueprint) ```
diff --git a/tests/python/release_checklist/check_mono_entity_views.py b/tests/python/release_checklist/check_mono_entity_views.py new file mode 100644 index 000000000000..676777b97e37 --- /dev/null +++ b/tests/python/release_checklist/check_mono_entity_views.py @@ -0,0 +1,59 @@ +from __future__ import annotations + +import os +from argparse import Namespace +from uuid import uuid4 + +import numpy as np +import rerun as rr +import rerun.blueprint as rrb + +README = """ +# Mono-entity views + +This test checks that mono-entity views work as expected. + +- Reset the blueprint to default +- Check each space view: when titled `ERROR`, they should display an error, and when titled `OK`, they should display the tensor or text document correctly. + +""" + + +def log_readme() -> None: + rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True) + + +def log_data() -> None: + rr.log("tensor/one", rr.Tensor(np.random.rand(10, 10, 3, 5))) + rr.log("tensor/two", rr.Tensor(np.random.rand(3, 5, 7, 5))) + + rr.log("txt/one", rr.TextDocument("Hello")) + rr.log("txt/two", rr.TextDocument("World")) + + +def blueprint() -> rrb.BlueprintLike: + return rrb.Grid( + rrb.TextDocumentView(origin="readme"), + rrb.TensorView(origin="/tensor", name="ERROR"), + rrb.TensorView(origin="/tensor/one", name="OK"), + rrb.TensorView(origin="/tensor/two", name="OK"), + rrb.TextDocumentView(origin="/txt", name="ERROR"), + rrb.TextDocumentView(origin="/txt/one", name="OK"), + rrb.TextDocumentView(origin="/txt/two", name="OK"), + ) + + +def run(args: Namespace) -> None: + rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4(), default_blueprint=blueprint()) + + log_readme() + log_data() + + +if __name__ == "__main__": + import argparse + + parser = argparse.ArgumentParser(description="Interactive release checklist") + rr.script_add_args(parser) + args = parser.parse_args() + run(args) From 0f3cfbfff03125339dc081ea0781371308b03ce5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 24 May 2024 11:00:08 +0200 Subject: [PATCH 02/10] Smooth scrolling in 2D space views (#6422) When scrolling with a discreet mouse wheel, the scrolling is now smoothed. ### 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/6422?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/6422?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/6422) - [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`. --- crates/re_space_view_spatial/src/ui_2d.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/re_space_view_spatial/src/ui_2d.rs b/crates/re_space_view_spatial/src/ui_2d.rs index e406167b3e1a..3a24f7444bf8 100644 --- a/crates/re_space_view_spatial/src/ui_2d.rs +++ b/crates/re_space_view_spatial/src/ui_2d.rs @@ -80,12 +80,7 @@ fn ui_from_scene( let mut pan_delta_in_ui = response.drag_delta(); if response.hovered() { - // NOTE: we use `raw_scroll` instead of `smooth_scroll_delta` to avoid the - // added latency of smoothing, which is really annoying on Mac trackpads. - // The smoothing is only useful for users with discreet scroll wheels, - // and they are likely to pan with dragging instead. - // TODO(egui#4401): https://github.com/emilk/egui/issues/4401 - pan_delta_in_ui += response.ctx.input(|i| i.raw_scroll_delta); + pan_delta_in_ui += response.ctx.input(|i| i.smooth_scroll_delta); } if pan_delta_in_ui != Vec2::ZERO { *visual_bounds = From a6042f9a7c054b898f697524e136eae47fc7cebb Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Fri, 24 May 2024 12:27:45 +0200 Subject: [PATCH 03/10] Improve styling of list item's collapsing triangle (#6426) ### What - Part of #6418 Before: ![collapse-ui](https://github.com/rerun-io/rerun/assets/1148717/5bd9f55a-69a0-4d27-8393-26b8b376cf5d) After: ![Export-1716534172555](https://github.com/rerun-io/rerun/assets/49431240/bc915eb9-9c45-4295-91f7-3ec889e957bc) ### 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/6426?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/6426?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/6426) - [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`. --- crates/re_ui/src/list_item/list_item.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/re_ui/src/list_item/list_item.rs b/crates/re_ui/src/list_item/list_item.rs index 1798a4307dba..df4a21e1922c 100644 --- a/crates/re_ui/src/list_item/list_item.rs +++ b/crates/re_ui/src/list_item/list_item.rs @@ -288,7 +288,12 @@ impl<'a> ListItem<'a> { id.unwrap_or(ui.id()).with("collapsing_triangle"), egui::Sense::click(), ); - ReUi::paint_collapsing_triangle(ui, openness, triangle_rect.center(), &visuals); + ReUi::paint_collapsing_triangle( + ui, + openness, + triangle_rect.center(), + ui.style().interact(&triangle_response), + ); collapse_response = Some(triangle_response); } From 70dae59637a47566cd5b66ab37515687d9f7f15b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 24 May 2024 13:29:56 +0200 Subject: [PATCH 04/10] Fix `ListItem` bugs (#6398) ### What First bug: scrolling would make columns expand/collapse Second bug: id clashes (found by selecting multiple space views) ### 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/6398?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/6398?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/6398) - [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`. --- crates/re_space_view/src/view_property_ui.rs | 2 +- .../src/space_view_class.rs | 4 +- crates/re_time_panel/src/lib.rs | 6 +- .../hierarchical_drag_and_drop.rs | 6 +- .../examples/re_ui_example/right_panel.rs | 8 +- crates/re_ui/src/list_item/list_item.rs | 85 ++++---- crates/re_ui/src/list_item/scope.rs | 4 +- crates/re_viewer/src/ui/selection_panel.rs | 2 +- .../re_viewport/src/viewport_blueprint_ui.rs | 191 +++++++++--------- 9 files changed, 154 insertions(+), 154 deletions(-) diff --git a/crates/re_space_view/src/view_property_ui.rs b/crates/re_space_view/src/view_property_ui.rs index 5801147cccf5..3b5a5ca70fe2 100644 --- a/crates/re_space_view/src/view_property_ui.rs +++ b/crates/re_space_view/src/view_property_ui.rs @@ -73,7 +73,7 @@ pub fn view_property_ui( .interactive(false) .show_hierarchical_with_children( ui, - A::name().full_name(), + ui.make_persistent_id(A::name().full_name()), true, list_item::LabelContent::new(A::display_name()), sub_prop_ui, diff --git a/crates/re_space_view_time_series/src/space_view_class.rs b/crates/re_space_view_time_series/src/space_view_class.rs index e7c0bba54e3a..8af064c0340e 100644 --- a/crates/re_space_view_time_series/src/space_view_class.rs +++ b/crates/re_space_view_time_series/src/space_view_class.rs @@ -724,9 +724,9 @@ fn axis_ui( .interactive(false) .show_hierarchical_with_children( ui, - "time_series_selection_ui_y_axis", + ui.make_persistent_id("time_series_selection_ui_y_axis"), true, - list_item::LabelContent::new("Y Axis"), + list_item::LabelContent::new("Y axis"), sub_prop_ui, ); } diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 00ed32636674..e13b0a24a64c 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -605,6 +605,10 @@ impl TimePanel { .set_open(ui.ctx(), true); } + // Globally unique id - should only be one of these in view at one time. + // We do this so that we can support "collapse/expand all" command. + let id = egui::Id::new(CollapseScope::StreamsTree.entity(tree.path.clone())); + let list_item::ShowCollapsingResponse { item_response: response, body_response, @@ -613,7 +617,7 @@ impl TimePanel { .force_hovered(is_item_hovered) .show_hierarchical_with_children( ui, - CollapseScope::StreamsTree.entity(tree.path.clone()), + id, default_open, list_item::LabelContent::new(text) .with_icon(guess_instance_path_icon( diff --git a/crates/re_ui/examples/re_ui_example/hierarchical_drag_and_drop.rs b/crates/re_ui/examples/re_ui_example/hierarchical_drag_and_drop.rs index 74d6ee43c9b0..a31793a582d0 100644 --- a/crates/re_ui/examples/re_ui_example/hierarchical_drag_and_drop.rs +++ b/crates/re_ui/examples/re_ui_example/hierarchical_drag_and_drop.rs @@ -276,13 +276,17 @@ impl HierarchicalDragAndDrop { item_id: ItemId, children: &Vec, ) { + // Globally unique id - should only be one of these in view at one time. + // We do this so that we can support "collapse/expand all" command. + let id = egui::Id::new(item_id); + let response = list_item::ListItem::new(re_ui) .selected(self.selected(item_id)) .draggable(true) .drop_target_style(self.target_container == Some(item_id)) .show_hierarchical_with_children( ui, - item_id, + id, true, list_item::LabelContent::new(format!("Container {item_id:?}")).subdued(true), |re_ui, ui| { diff --git a/crates/re_ui/examples/re_ui_example/right_panel.rs b/crates/re_ui/examples/re_ui_example/right_panel.rs index 5aff241633ea..e9bf12da0dc8 100644 --- a/crates/re_ui/examples/re_ui_example/right_panel.rs +++ b/crates/re_ui/examples/re_ui_example/right_panel.rs @@ -131,7 +131,7 @@ impl RightPanel { re_ui.list_item().show_hierarchical_with_children( ui, - "label content features", + ui.make_persistent_id("label content features"), true, list_item::LabelContent::new("LabelContent features:"), |re_ui, ui| { @@ -203,7 +203,7 @@ impl RightPanel { re_ui.list_item().show_hierarchical_with_children( ui, - "property content features", + ui.make_persistent_id("property content features"), true, list_item::PropertyContent::new("PropertyContent features:") .value_text("bunch of properties"), @@ -258,7 +258,7 @@ impl RightPanel { re_ui.list_item().show_hierarchical_with_children( ui, - "property content right button reserve", + ui.make_persistent_id("property content right button reserve"), true, list_item::PropertyContent::new("PropertyContent action button:") .value_text("demo of right gutter"), @@ -301,7 +301,7 @@ impl RightPanel { re_ui.list_item().show_hierarchical_with_children( ui, - "other features", + ui.make_persistent_id("other features"), true, list_item::LabelContent::new("Other contents:"), |re_ui, ui| { diff --git a/crates/re_ui/src/list_item/list_item.rs b/crates/re_ui/src/list_item/list_item.rs index df4a21e1922c..837f2492824a 100644 --- a/crates/re_ui/src/list_item/list_item.rs +++ b/crates/re_ui/src/list_item/list_item.rs @@ -146,17 +146,18 @@ impl<'a> ListItem<'a> { /// Draw the item as a non-leaf node from a hierarchical list. /// + /// The `id` should be globally unique! + /// You can use `ui.make_persistent_id(…)` for that. + /// /// *Important*: must be called while nested in a [`super::list_item_scope`]. pub fn show_hierarchical_with_children( mut self, ui: &mut Ui, - id: impl Into, + id: egui::Id, default_open: bool, content: impl ListItemContent + 'a, add_childrens: impl FnOnce(&ReUi, &mut egui::Ui) -> R, ) -> ShowCollapsingResponse { - let id = id.into(); - let mut state = egui::collapsing_header::CollapsingState::load_with_default_open( ui.ctx(), id, @@ -270,48 +271,48 @@ impl<'a> ListItem<'a> { let mut collapse_response = None; - if ui.is_rect_visible(bg_rect) { - let visuals = ui.style().interact_selectable(&style_response, selected); - - let background_frame = ui.painter().add(egui::Shape::Noop); - - // Draw collapsing triangle - if let Some(openness) = collapse_openness { - let triangle_pos = ui.painter().round_pos_to_pixels(egui::pos2( - rect.min.x, - rect.center().y - 0.5 * ReUi::collapsing_triangle_area().y, - )); - let triangle_rect = - egui::Rect::from_min_size(triangle_pos, ReUi::collapsing_triangle_area()); - let triangle_response = ui.interact( - triangle_rect.expand(3.0), // make it easier to click - id.unwrap_or(ui.id()).with("collapsing_triangle"), - egui::Sense::click(), - ); - ReUi::paint_collapsing_triangle( - ui, - openness, - triangle_rect.center(), - ui.style().interact(&triangle_response), - ); - collapse_response = Some(triangle_response); - } + let visuals = ui.style().interact_selectable(&style_response, selected); + + let background_frame = ui.painter().add(egui::Shape::Noop); + + // Draw collapsing triangle + if let Some(openness) = collapse_openness { + let triangle_pos = ui.painter().round_pos_to_pixels(egui::pos2( + rect.min.x, + rect.center().y - 0.5 * ReUi::collapsing_triangle_area().y, + )); + let triangle_rect = + egui::Rect::from_min_size(triangle_pos, ReUi::collapsing_triangle_area()); + let triangle_response = ui.interact( + triangle_rect.expand(3.0), // make it easier to click + id.unwrap_or(ui.id()).with("collapsing_triangle"), + egui::Sense::click(), + ); + ReUi::paint_collapsing_triangle( + ui, + openness, + triangle_rect.center(), + ui.style().interact(&triangle_response), + ); + collapse_response = Some(triangle_response); + } - // Draw content - let mut content_rect = rect; - if collapse_openness.is_some() { - content_rect.min.x += extra_indent + collapse_extra; - } + // Draw content + let mut content_rect = rect; + if collapse_openness.is_some() { + content_rect.min.x += extra_indent + collapse_extra; + } - let content_ctx = ContentContext { - rect: content_rect, - bg_rect, - response: &style_response, - list_item: &self, - layout_info, - }; - content.ui(re_ui, ui, &content_ctx); + let content_ctx = ContentContext { + rect: content_rect, + bg_rect, + response: &style_response, + list_item: &self, + layout_info, + }; + content.ui(re_ui, ui, &content_ctx); + if ui.is_rect_visible(bg_rect) { // Ensure the background highlight is drawn over round pixel coordinates. Otherwise, // there could be artifact between consecutive highlighted items when drawn on // fractional pixels. diff --git a/crates/re_ui/src/list_item/scope.rs b/crates/re_ui/src/list_item/scope.rs index 669cb5b95b84..ac4798a709c0 100644 --- a/crates/re_ui/src/list_item/scope.rs +++ b/crates/re_ui/src/list_item/scope.rs @@ -252,10 +252,10 @@ impl LayoutInfoStack { /// list items. pub fn list_item_scope( ui: &mut egui::Ui, - id_source: impl Into, + id_source: impl std::hash::Hash, content: impl FnOnce(&mut egui::Ui) -> R, ) -> R { - let scope_id = ui.id().with(id_source.into()); + let scope_id = ui.id().with(id_source); // read last frame layout statistics and reset for the new frame let layout_stats = LayoutStatistics::read(ui.ctx(), scope_id); diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 78fc653a5efc..2eff873a5023 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -124,7 +124,7 @@ impl SelectionPanel { UiLayout::SelectionPanelFull }; for (i, item) in selection.iter_items().enumerate() { - ui.push_id(i, |ui| { + ui.push_id(item, |ui| { what_is_selected_ui(ui, ctx, viewport.blueprint, item); match item { diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 6310bc5ccd80..c5638edbd800 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -338,6 +338,10 @@ impl Viewport<'_, '_> { remove_response | vis_response }); + // Globally unique id - should only be one of these in view at one time. + // We do this so that we can support "collapse/expand all" command. + let id = egui::Id::new(CollapseScope::BlueprintTree.container(*container_id)); + let list_item::ShowCollapsingResponse { item_response: response, body_response, @@ -345,17 +349,11 @@ impl Viewport<'_, '_> { .selected(ctx.selection().contains_item(&item)) .draggable(true) .drop_target_style(self.state.is_candidate_drop_parent_container(container_id)) - .show_hierarchical_with_children( - ui, - CollapseScope::BlueprintTree.container(*container_id), - default_open, - item_content, - |_, ui| { - for child in &container_blueprint.contents { - self.contents_ui(ctx, ui, child, container_visible); - } - }, - ); + .show_hierarchical_with_children(ui, id, default_open, item_content, |_, ui| { + for child in &container_blueprint.contents { + self.contents_ui(ctx, ui, child, container_visible); + } + }); context_menu_ui_for_item( ctx, @@ -424,6 +422,11 @@ impl Viewport<'_, '_> { response | vis_response }); + + // Globally unique id - should only be one of these in view at one time. + // We do this so that we can support "collapse/expand all" command. + let id = egui::Id::new(CollapseScope::BlueprintTree.space_view(*space_view_id)); + let list_item::ShowCollapsingResponse { item_response: mut response, body_response, @@ -431,65 +434,56 @@ impl Viewport<'_, '_> { .selected(ctx.selection().contains_item(&item)) .draggable(true) .force_hovered(is_item_hovered) - .show_hierarchical_with_children( - ui, - CollapseScope::BlueprintTree.space_view(*space_view_id), - default_open, - item_content, - |_, ui| { - // Always show the origin hierarchy first. - self.space_view_entity_hierarchy_ui( - ctx, - ui, - query_result, - &DataResultNodeOrPath::from_path_lookup( - result_tree, - &space_view.space_origin, - ), - space_view, - space_view_visible, - false, - ); - - // Show 'projections' if there's any items that weren't part of the tree under origin but are directly included. - // The latter is important since `+ image/camera/**` necessarily has `image` and `image/camera` in the data result tree. - let mut projections = Vec::new(); - result_tree.visit(&mut |node| { - if node - .data_result - .entity_path - .starts_with(&space_view.space_origin) - { - false // If it's under the origin, we're not interested, stop recursing. - } else if node.data_result.tree_prefix_only { - true // Keep recursing until we find a projection. - } else { - projections.push(node); - false // We found a projection, stop recursing as everything below is now included in the projections. - } - }); - if !projections.is_empty() { - list_item::ListItem::new(ctx.re_ui) - .interactive(false) - .show_flat( - ui, - list_item::LabelContent::new("Projections:").italics(true), - ); + .show_hierarchical_with_children(ui, id, default_open, item_content, |_, ui| { + // Always show the origin hierarchy first. + self.space_view_entity_hierarchy_ui( + ctx, + ui, + query_result, + &DataResultNodeOrPath::from_path_lookup(result_tree, &space_view.space_origin), + space_view, + space_view_visible, + false, + ); - for projection in projections { - self.space_view_entity_hierarchy_ui( - ctx, - ui, - query_result, - &DataResultNodeOrPath::DataResultNode(projection), - space_view, - space_view_visible, - true, - ); - } + // Show 'projections' if there's any items that weren't part of the tree under origin but are directly included. + // The latter is important since `+ image/camera/**` necessarily has `image` and `image/camera` in the data result tree. + let mut projections = Vec::new(); + result_tree.visit(&mut |node| { + if node + .data_result + .entity_path + .starts_with(&space_view.space_origin) + { + false // If it's under the origin, we're not interested, stop recursing. + } else if node.data_result.tree_prefix_only { + true // Keep recursing until we find a projection. + } else { + projections.push(node); + false // We found a projection, stop recursing as everything below is now included in the projections. } - }, - ); + }); + if !projections.is_empty() { + list_item::ListItem::new(ctx.re_ui) + .interactive(false) + .show_flat( + ui, + list_item::LabelContent::new("Projections:").italics(true), + ); + + for projection in projections { + self.space_view_entity_hierarchy_ui( + ctx, + ui, + query_result, + &DataResultNodeOrPath::DataResultNode(projection), + space_view, + space_view_visible, + true, + ); + } + } + }); response = response.on_hover_text("Space view"); @@ -634,39 +628,36 @@ impl Viewport<'_, '_> { let default_open = entity_path.starts_with(&space_view.space_origin) && Self::default_open_for_data_result(node); + // Globally unique id - should only be one of these in view at one time. + // We do this so that we can support "collapse/expand all" command. + let id = egui::Id::new( + CollapseScope::BlueprintTree.data_result(space_view.id, entity_path.clone()), + ); + list_item - .show_hierarchical_with_children( - ui, - CollapseScope::BlueprintTree.data_result(space_view.id, entity_path.clone()), - default_open, - item_content, - |_, ui| { - for child in node.children.iter().sorted_by_key(|c| { - query_result - .tree - .lookup_result(**c) - .map_or(&space_view.space_origin, |c| &c.entity_path) - }) { - let Some(child_node) = query_result.tree.lookup_node(*child) else { - debug_assert!( - false, - "DataResultNode {node:?} has an invalid child" - ); - continue; - }; - - self.space_view_entity_hierarchy_ui( - ctx, - ui, - query_result, - &DataResultNodeOrPath::DataResultNode(child_node), - space_view, - space_view_visible, - projection_mode, - ); - } - }, - ) + .show_hierarchical_with_children(ui, id, default_open, item_content, |_, ui| { + for child in node.children.iter().sorted_by_key(|c| { + query_result + .tree + .lookup_result(**c) + .map_or(&space_view.space_origin, |c| &c.entity_path) + }) { + let Some(child_node) = query_result.tree.lookup_node(*child) else { + debug_assert!(false, "DataResultNode {node:?} has an invalid child"); + continue; + }; + + self.space_view_entity_hierarchy_ui( + ctx, + ui, + query_result, + &DataResultNodeOrPath::DataResultNode(child_node), + space_view, + space_view_visible, + projection_mode, + ); + } + }) .item_response } else { list_item.show_hierarchical(ui, item_content) From 317dcda0bdad5b1e553ba856645fffc60b278fa0 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 24 May 2024 15:40:14 +0200 Subject: [PATCH 05/10] Improve welcome screen for small screens (#6421) Makes the welcome screen use more of the available space. Important for small screens, mobile, or embedded web viewers. ### What * Hide blueprint panel * Reduce vertical space * Fix title casing #### Before (0.16 viewer) Screenshot 2024-05-23 at 15 14 14 #### After Screenshot 2024-05-23 at 15 14 24 #### Mobile ![IMG_7872](https://github.com/rerun-io/rerun/assets/1148717/f4ba5a95-0c6e-43f0-aadd-b17f768ebaec) ### 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/6421?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/6421?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/6421) - [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`. --- crates/re_time_panel/src/lib.rs | 8 +++++--- crates/re_viewer/src/app_blueprint.rs | 11 ++++++----- .../src/ui/welcome_screen/example_section.rs | 5 ++++- .../src/ui/welcome_screen/welcome_section.rs | 4 +--- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index e13b0a24a64c..414a322255f1 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -264,7 +264,9 @@ impl TimePanel { ) { ui.spacing_mut().item_spacing.x = 18.0; // from figma - if ui.max_rect().width() < 600.0 { + let has_any_data_on_timeline = entity_db.has_any_data_on_timeline(time_ctrl.timeline()); + + if ui.max_rect().width() < 600.0 && has_any_data_on_timeline { // Responsive ui for narrow screens, e.g. mobile. Split the controls into two rows. ui.vertical(|ui| { ui.horizontal(|ui| { @@ -273,7 +275,7 @@ impl TimePanel { self.time_control_ui .play_pause_ui(time_ctrl, re_ui, times_per_timeline, ui); - if entity_db.has_any_data_on_timeline(time_ctrl.timeline()) { + if has_any_data_on_timeline { self.time_control_ui.playback_speed_ui(time_ctrl, ui); self.time_control_ui.fps_ui(time_ctrl, ui); } @@ -296,7 +298,7 @@ impl TimePanel { self.time_control_ui .timeline_selector_ui(time_ctrl, times_per_timeline, ui); - if entity_db.has_any_data_on_timeline(time_ctrl.timeline()) { + if has_any_data_on_timeline { self.time_control_ui.playback_speed_ui(time_ctrl, ui); self.time_control_ui.fps_ui(time_ctrl, ui); } diff --git a/crates/re_viewer/src/app_blueprint.rs b/crates/re_viewer/src/app_blueprint.rs index 088c2e6e0177..debfcf31cc72 100644 --- a/crates/re_viewer/src/app_blueprint.rs +++ b/crates/re_viewer/src/app_blueprint.rs @@ -4,10 +4,10 @@ use re_log_types::{DataRow, EntityPath, RowId}; use re_types::blueprint::components::PanelState; use re_viewer_context::{CommandSender, StoreContext, SystemCommand, SystemCommandSender}; -pub const TOP_PANEL_PATH: &str = "top_panel"; -pub const BLUEPRINT_PANEL_PATH: &str = "blueprint_panel"; -pub const SELECTION_PANEL_PATH: &str = "selection_panel"; -pub const TIME_PANEL_PATH: &str = "time_panel"; +const TOP_PANEL_PATH: &str = "top_panel"; +const BLUEPRINT_PANEL_PATH: &str = "blueprint_panel"; +const SELECTION_PANEL_PATH: &str = "selection_panel"; +const TIME_PANEL_PATH: &str = "time_panel"; /// Blueprint for top-level application pub struct AppBlueprint<'a> { @@ -98,9 +98,10 @@ impl<'a> AppBlueprint<'a> { } pub fn setup_welcome_screen_blueprint(welcome_screen_blueprint: &mut EntityDb) { + // Most things are hidden in the welcome screen: for (panel_name, value) in [ (TOP_PANEL_PATH, PanelState::Expanded), - (BLUEPRINT_PANEL_PATH, PanelState::Expanded), + (BLUEPRINT_PANEL_PATH, PanelState::Hidden), (SELECTION_PANEL_PATH, PanelState::Hidden), (TIME_PANEL_PATH, PanelState::Hidden), ] { diff --git a/crates/re_viewer/src/ui/welcome_screen/example_section.rs b/crates/re_viewer/src/ui/welcome_screen/example_section.rs index 2007c4ba3067..5d333aa24fad 100644 --- a/crates/re_viewer/src/ui/welcome_screen/example_section.rs +++ b/crates/re_viewer/src/ui/welcome_screen/example_section.rs @@ -34,7 +34,8 @@ pub(super) const MIN_COLUMN_WIDTH: f32 = 250.0; const MAX_COLUMN_WIDTH: f32 = 337.0; const MAX_COLUMN_COUNT: usize = 3; const COLUMN_HSPACE: f32 = 20.0; -const TITLE_TO_GRID_VSPACE: f32 = 32.0; +const AFTER_HEADER_VSPACE: f32 = 48.0; +const TITLE_TO_GRID_VSPACE: f32 = 24.0; const ROW_VSPACE: f32 = 20.0; const THUMBNAIL_RADIUS: f32 = 12.0; @@ -274,6 +275,8 @@ impl ExampleSection { ui.vertical(|ui| { header_ui(ui); + ui.add_space(AFTER_HEADER_VSPACE); + let Some(examples) = examples.ready_mut() else { // Still waiting for example to load ui.separator(); diff --git a/crates/re_viewer/src/ui/welcome_screen/welcome_section.rs b/crates/re_viewer/src/ui/welcome_screen/welcome_section.rs index 4aa6251d725c..a28873a9f443 100644 --- a/crates/re_viewer/src/ui/welcome_screen/welcome_section.rs +++ b/crates/re_viewer/src/ui/welcome_screen/welcome_section.rs @@ -1,7 +1,7 @@ use egui::Ui; pub(super) const DOCS_URL: &str = "https://www.rerun.io/docs"; -pub(super) const WELCOME_SCREEN_TITLE: &str = "Visualize Multimodal Data"; +pub(super) const WELCOME_SCREEN_TITLE: &str = "Visualize multimodal data"; pub(super) const WELCOME_SCREEN_BULLET_TEXT: &[&str] = &[ "Log data with the Rerun SDK in C++, Python, or Rust", "Visualize and explore live or recorded data", @@ -65,7 +65,5 @@ pub(super) fn welcome_section_ui(ui: &mut egui::Ui) { new_tab: true, }); } - - ui.add_space(83.0); }); } From 2e494b311d958672103caf62a9f86adb8ebdc093 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Sun, 26 May 2024 20:52:42 +0200 Subject: [PATCH 06/10] Move context menu to new `re_context_menu` crate (#6423) ### What - Fixes #6414 Mostly pure refactor, except for moving `determine_visualizable_entities` to a `SpaceViewClass` extension trait. ### 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/6423?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/6423?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/6423) - [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`. --- ARCHITECTURE.md | 28 +++++++----- Cargo.lock | 29 +++++++++--- Cargo.toml | 1 + crates/re_context_menu/Cargo.toml | 37 +++++++++++++++ crates/re_context_menu/README.md | 10 +++++ .../src}/actions/add_container.rs | 2 +- .../actions/add_entities_to_new_space_view.rs | 8 ++-- .../src}/actions/add_space_view.rs | 2 +- .../src}/actions/clone_space_view.rs | 2 +- .../src}/actions/collapse_expand_all.rs | 2 +- .../src}/actions/mod.rs | 0 .../actions/move_contents_to_new_container.rs | 2 +- .../src}/actions/remove.rs | 2 +- .../src}/actions/show_hide.rs | 2 +- .../mod.rs => re_context_menu/src/lib.rs} | 2 + .../src}/sub_menu.rs | 2 +- crates/re_space_view/Cargo.toml | 1 - crates/re_space_view/src/lib.rs | 2 - crates/re_space_view/src/visualizable.rs | 38 ---------------- crates/re_time_panel/Cargo.toml | 2 +- crates/re_time_panel/src/lib.rs | 2 +- crates/re_viewer/Cargo.toml | 2 +- crates/re_viewer/src/app_state.rs | 39 ++++++++-------- crates/re_viewer/src/ui/override_ui.rs | 22 ++++----- crates/re_viewer/src/ui/selection_panel.rs | 6 +-- crates/re_viewer_context/src/lib.rs | 2 +- .../re_viewer_context/src/space_view/mod.rs | 4 +- .../src/space_view/space_view_class.rs | 45 +++++++++++++++++-- crates/re_viewport/Cargo.toml | 3 +- crates/re_viewport/src/lib.rs | 2 - .../src/space_view_entity_picker.rs | 6 +-- crates/re_viewport/src/viewport.rs | 9 ++-- .../re_viewport/src/viewport_blueprint_ui.rs | 4 +- crates/re_viewport_blueprint/Cargo.toml | 2 +- 34 files changed, 195 insertions(+), 127 deletions(-) create mode 100644 crates/re_context_menu/Cargo.toml create mode 100644 crates/re_context_menu/README.md rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/add_container.rs (96%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/add_entities_to_new_space_view.rs (96%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/add_space_view.rs (95%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/clone_space_view.rs (93%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/collapse_expand_all.rs (98%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/mod.rs (100%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/move_contents_to_new_container.rs (97%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/remove.rs (96%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/actions/show_hide.rs (98%) rename crates/{re_viewport/src/context_menu/mod.rs => re_context_menu/src/lib.rs} (99%) rename crates/{re_viewport/src/context_menu => re_context_menu/src}/sub_menu.rs (94%) delete mode 100644 crates/re_space_view/src/visualizable.rs diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 665144876cc5..6388b7dd05ce 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -88,13 +88,14 @@ Of course, this will only take us so far. In the future we plan on caching queri Here is an overview of the crates included in the project: - - - - - + + + + + + ### What Part of #6330 Removes or explicitly allows unwrap() where it makes sense. Affected crates: `re_entity_db`, `re_format_arrow`, `and re_log_types` ### 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/6380?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/6380?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/6380) - [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`. --------- Co-authored-by: Emil Ernerfeldt --- crates/re_entity_db/src/entity_tree.rs | 3 +- crates/re_entity_db/src/lib.rs | 3 -- crates/re_format_arrow/src/lib.rs | 8 +-- crates/re_log_types/src/arrow_msg.rs | 1 + crates/re_log_types/src/data_cell.rs | 15 +++++- crates/re_log_types/src/data_table.rs | 39 ++++++++------ crates/re_log_types/src/data_table_batcher.rs | 1 + crates/re_log_types/src/example_components.rs | 14 +++-- crates/re_log_types/src/lib.rs | 3 -- crates/re_log_types/src/time.rs | 51 +++++++++++-------- crates/re_types_core/src/result.rs | 14 ++++- 11 files changed, 97 insertions(+), 55 deletions(-) diff --git a/crates/re_entity_db/src/entity_tree.rs b/crates/re_entity_db/src/entity_tree.rs index c26fe215eab6..5b7aebc803ec 100644 --- a/crates/re_entity_db/src/entity_tree.rs +++ b/crates/re_entity_db/src/entity_tree.rs @@ -362,8 +362,7 @@ impl EntityTree { if cell.component_name() == ClearIsRecursive::name() { let is_recursive = cell .try_to_native_mono::() - .unwrap() - .map_or(false, |settings| settings.0); + .map_or(false, |opt| opt.map_or(false, |settings| settings.0)); self.on_added_clear(clear_cascade, store_diff, is_recursive); } diff --git a/crates/re_entity_db/src/lib.rs b/crates/re_entity_db/src/lib.rs index fbcd2ee60ca1..6295ba7181d5 100644 --- a/crates/re_entity_db/src/lib.rs +++ b/crates/re_entity_db/src/lib.rs @@ -4,9 +4,6 @@ #![doc = document_features::document_features!()] //! -// TODO(#3408): remove unwrap() -#![allow(clippy::unwrap_used)] - pub mod entity_db; pub mod entity_properties; pub mod entity_tree; diff --git a/crates/re_format_arrow/src/lib.rs b/crates/re_format_arrow/src/lib.rs index 573d924a9c89..e7f64b061b79 100644 --- a/crates/re_format_arrow/src/lib.rs +++ b/crates/re_format_arrow/src/lib.rs @@ -1,8 +1,5 @@ //! Formatting for tables of Arrow arrays -// TODO(#3408): remove unwrap() -#![allow(clippy::unwrap_used)] - use std::fmt::Formatter; use arrow2::{ @@ -241,7 +238,10 @@ where .iter() .map(|disp| { let mut string = String::new(); - (disp)(&mut string, row).unwrap(); + if (disp)(&mut string, row).is_err() { + // Seems to be okay to silently ignore errors here, but reset the string just in case + string.clear(); + } let chars: Vec<_> = string.chars().collect(); if chars.len() > WIDTH_UPPER_BOUNDARY as usize { Cell::new( diff --git a/crates/re_log_types/src/arrow_msg.rs b/crates/re_log_types/src/arrow_msg.rs index c62f60eed676..ea5e9059b4f7 100644 --- a/crates/re_log_types/src/arrow_msg.rs +++ b/crates/re_log_types/src/arrow_msg.rs @@ -187,6 +187,7 @@ impl<'de> serde::Deserialize<'de> for ArrowMsg { chunks.len() ))); } + #[allow(clippy::unwrap_used)] // is_empty check above let chunk = chunks.into_iter().next().unwrap(); Ok(ArrowMsg { diff --git a/crates/re_log_types/src/data_cell.rs b/crates/re_log_types/src/data_cell.rs index fa10673455b7..f0a9cdec1345 100644 --- a/crates/re_log_types/src/data_cell.rs +++ b/crates/re_log_types/src/data_cell.rs @@ -231,6 +231,8 @@ impl DataCell { where C: Component + Clone + 'a, { + // NOTE: see function description why it's okay here + #[allow(clippy::unwrap_used)] Self::try_from_native(values).unwrap() } @@ -246,6 +248,8 @@ impl DataCell { where C: Component + Clone + 'a, { + // NOTE: see function description why it's okay here + #[allow(clippy::unwrap_used)] Self::try_from_native_sparse(values).unwrap() } @@ -295,6 +299,8 @@ impl DataCell { /// See [`Self::try_from_arrow`] for the fallible alternative. #[inline] pub fn from_arrow(name: ComponentName, values: Box) -> Self { + // NOTE: see function description why it's okay here + #[allow(clippy::unwrap_used)] Self::try_from_arrow(name, values).unwrap() } @@ -333,6 +339,8 @@ impl DataCell { /// See [`Self::try_from_arrow_empty`] for a fallible alternative. #[inline] pub fn from_arrow_empty(name: ComponentName, datatype: arrow2::datatypes::DataType) -> Self { + // NOTE: see function description why it's okay here + #[allow(clippy::unwrap_used)] Self::try_from_arrow_empty(name, datatype).unwrap() } @@ -381,7 +389,7 @@ impl DataCell { let datatype = ListArray::::default_datatype(datatype); let offsets = Offsets::try_from_lengths(std::iter::once(self.num_instances() as usize)) - .unwrap() + .unwrap_or_default() .into(); let validity = None; @@ -436,6 +444,8 @@ impl DataCell { /// See [`Self::try_to_native`] for a fallible alternative. #[inline] pub fn to_native<'a, C: Component + 'a>(&'a self) -> Vec { + // NOTE: see function description why it's okay here + #[allow(clippy::unwrap_used)] self.try_to_native().unwrap() } @@ -456,6 +466,8 @@ impl DataCell { /// See [`Self::try_to_native_opt`] for a fallible alternative. #[inline] pub fn to_native_opt<'a, C: Component + 'a>(&'a self) -> Vec> { + // NOTE: see function description why it's okay here + #[allow(clippy::unwrap_used)] self.try_to_native_opt().unwrap() } } @@ -510,6 +522,7 @@ impl DataCell { fn is_sorted_and_unique_primitive(arr: &dyn Array) -> bool { // NOTE: unwrap cannot fail, checked by caller just below + #[allow(clippy::unwrap_used)] let values = arr.as_any().downcast_ref::>().unwrap(); values.values().windows(2).all(|v| v[0] < v[1]) } diff --git a/crates/re_log_types/src/data_table.rs b/crates/re_log_types/src/data_table.rs index 6c1d3bcec26d..43b556317f0d 100644 --- a/crates/re_log_types/src/data_table.rs +++ b/crates/re_log_types/src/data_table.rs @@ -437,7 +437,9 @@ impl DataTable { for cell in cells.0 { let component = cell.component_name(); // NOTE: unwrap cannot fail, all arrays pre-allocated above. - columns.get_mut(&component).unwrap()[i] = Some(cell); + #[allow(clippy::unwrap_used)] + let column = columns.get_mut(&component).unwrap(); + column[i] = Some(cell); } } @@ -790,10 +792,10 @@ impl DataTable { let offsets = Offsets::try_from_lengths(column.iter().map(|cell| { cell.as_ref() .map_or(0, |cell| cell.num_instances() as usize) - })) + })); // NOTE: cannot fail, `data` has as many instances as `column` - .unwrap() - .into(); + #[allow(clippy::unwrap_used)] + let offsets = offsets.unwrap().into(); #[allow(clippy::from_iter_instead_of_collect)] let validity = Bitmap::from_iter(column.iter().map(|cell| cell.is_some())); @@ -911,12 +913,14 @@ impl DataTable { }; // NOTE: the unwrappings cannot fail since control_index() makes sure the index is valid + #[allow(clippy::unwrap_used)] let col_row_id = RowId::from_arrow( chunk .get(control_index(RowId::name().as_str())?) .unwrap() .as_ref(), )?; + #[allow(clippy::unwrap_used)] let col_entity_path = EntityPath::from_arrow( chunk .get(control_index(EntityPath::name().as_str())?) @@ -976,10 +980,11 @@ impl DataTable { } }; + // NOTE: unwrap cannot fail here, datatype checked above + #[allow(clippy::unwrap_used)] let col_time = column .as_any() .downcast_ref::>() - // NOTE: cannot fail, datatype checked above .unwrap(); let col_time: TimeOptVec = col_time.into_iter().map(|time| time.copied()).collect(); @@ -1100,7 +1105,8 @@ impl DataTable { rows_by_timeline2.remove(&Timeline::log_time()); for (timeline, rows1) in &mut rows_by_timeline1 { - let rows2 = rows_by_timeline2.get_mut(timeline).unwrap(); // safe + #[allow(clippy::unwrap_used)] // safe, the keys are checked above + let rows2 = rows_by_timeline2.get_mut(timeline).unwrap(); // NOTE: We need both sets of rows to follow a common natural order for the comparison // to make sense. @@ -1196,30 +1202,29 @@ impl DataTable { c2.component_name(), )); - fn cell_to_bytes(cell: DataCell) -> Vec { + fn cell_to_bytes(cell: DataCell) -> anyhow::Result> { let row = DataRow::from_cells1( RowId::ZERO, "cell", TimePoint::default(), cell, - ) - .unwrap(); + )?; let table = DataTable::from_rows(TableId::ZERO, [row]); - let msg = table.to_arrow_msg().unwrap(); + let msg = table.to_arrow_msg()?; use arrow2::io::ipc::write::StreamWriter; let mut buf = Vec::::new(); let mut writer = StreamWriter::new(&mut buf, Default::default()); - writer.start(&msg.schema, None).unwrap(); - writer.write(&msg.chunk, None).unwrap(); - writer.finish().unwrap(); + writer.start(&msg.schema, None)?; + writer.write(&msg.chunk, None)?; + writer.finish()?; - buf + Ok(buf) } - let c1_bytes = cell_to_bytes(c1.clone()); - let c2_bytes = cell_to_bytes(c2.clone()); + let c1_bytes = cell_to_bytes(c1.clone())?; + let c2_bytes = cell_to_bytes(c2.clone())?; size_mismatches.push(format!( "IPC size is {} vs {} bytes", @@ -1267,6 +1272,8 @@ impl DataTable { #[cfg(not(target_arch = "wasm32"))] impl DataTable { pub fn example(timeless: bool) -> Self { + // NOTE: because everything here is predetermined and there is no input we assume it's safe here + #![allow(clippy::unwrap_used)] use crate::{ example_components::{MyColor, MyLabel, MyPoint}, Time, diff --git a/crates/re_log_types/src/data_table_batcher.rs b/crates/re_log_types/src/data_table_batcher.rs index 0a419c60c3a8..f45de80f89cc 100644 --- a/crates/re_log_types/src/data_table_batcher.rs +++ b/crates/re_log_types/src/data_table_batcher.rs @@ -405,6 +405,7 @@ impl DataTableBatcher { pub fn tables(&self) -> Receiver { // NOTE: `rx_tables` is only ever taken when the batcher as a whole is dropped, at which // point it is impossible to call this method. + #![allow(clippy::unwrap_used)] self.inner.rx_tables.clone().unwrap() } } diff --git a/crates/re_log_types/src/example_components.rs b/crates/re_log_types/src/example_components.rs index e47c0edc46c5..1a9615bc7803 100644 --- a/crates/re_log_types/src/example_components.rs +++ b/crates/re_log_types/src/example_components.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use re_types_core::{Loggable, SizeBytes}; +use re_types_core::{DeserializationError, Loggable, SizeBytes}; // ---------------------------------------------------------------------------- @@ -116,7 +116,9 @@ impl Loggable for MyPoint { let array = data .as_any() .downcast_ref::() - .unwrap(); + .ok_or(DeserializationError::downcast_error::< + arrow2::array::StructArray, + >())?; let x_array = array.values()[0].as_ref(); let y_array = array.values()[1].as_ref(); @@ -124,11 +126,15 @@ impl Loggable for MyPoint { let xs = x_array .as_any() .downcast_ref::() - .unwrap(); + .ok_or(DeserializationError::downcast_error::< + arrow2::array::Float32Array, + >())?; let ys = y_array .as_any() .downcast_ref::() - .unwrap(); + .ok_or(DeserializationError::downcast_error::< + arrow2::array::Float32Array, + >())?; Ok(xs .values_iter() diff --git a/crates/re_log_types/src/lib.rs b/crates/re_log_types/src/lib.rs index 57200b6138f8..57ecc4b6037a 100644 --- a/crates/re_log_types/src/lib.rs +++ b/crates/re_log_types/src/lib.rs @@ -17,9 +17,6 @@ //! e.g. the entity `foo/bar/baz` is has the transform that is the product of //! `foo.transform * foo/bar.transform * foo/bar/baz.transform`. -// TODO(#3408): remove unwrap() -#![allow(clippy::unwrap_used)] - pub mod arrow_msg; pub mod example_components; pub mod hash; diff --git a/crates/re_log_types/src/time.rs b/crates/re_log_types/src/time.rs index 1fea5960f20a..6963a8c9c789 100644 --- a/crates/re_log_types/src/time.rs +++ b/crates/re_log_types/src/time.rs @@ -1,3 +1,5 @@ +use anyhow::Result; +use re_log::ResultExt; use std::ops::RangeInclusive; use time::{format_description::FormatItem, OffsetDateTime, UtcOffset}; @@ -77,25 +79,26 @@ impl Time { parsed_format: &Vec>, time_zone_for_timestamps: TimeZone, ) -> String { - match time_zone_for_timestamps { - TimeZone::Local => { - if let Ok(local_offset) = UtcOffset::current_local_offset() { - // Return in the local timezone. - let local_datetime = datetime.to_offset(local_offset); - local_datetime.format(&parsed_format).unwrap() - } else { - // Fallback to UTC. - // Skipping `err` description from logging because as of writing it doesn't add much, see - // https://github.com/time-rs/time/blob/v0.3.29/time/src/error/indeterminate_offset.rs - re_log::warn_once!("Failed to access local timezone offset to UTC."); - format!("{}Z", datetime.format(&parsed_format).unwrap()) + let r = (|| -> Result { + match time_zone_for_timestamps { + TimeZone::Local => { + if let Ok(local_offset) = UtcOffset::current_local_offset() { + // Return in the local timezone. + let local_datetime = datetime.to_offset(local_offset); + local_datetime.format(&parsed_format) + } else { + // Fallback to UTC. + // Skipping `err` description from logging because as of writing it doesn't add much, see + // https://github.com/time-rs/time/blob/v0.3.29/time/src/error/indeterminate_offset.rs + re_log::warn_once!("Failed to access local timezone offset to UTC."); + Ok(format!("{}Z", datetime.format(&parsed_format)?)) + } } + TimeZone::Utc => Ok(format!("{}Z", datetime.format(&parsed_format)?)), + TimeZone::UnixEpoch => datetime.format(&parsed_format), } - TimeZone::Utc => { - format!("{}Z", datetime.format(&parsed_format).unwrap()) - } - TimeZone::UnixEpoch => datetime.format(&parsed_format).unwrap(), - } + })(); + r.ok_or_log_error().unwrap_or_default() } /// Human-readable formatting @@ -120,6 +123,7 @@ impl Time { let date_is_today = datetime.date() == OffsetDateTime::now_utc().date(); let date_format = format!("[year]-[month]-[day] {time_format}"); + #[allow(clippy::unwrap_used)] // date_format is okay! let parsed_format = if date_is_today { time::format_description::parse(&time_format).unwrap() } else { @@ -167,6 +171,7 @@ impl Time { TimeZone::Utc | TimeZone::Local => "[hour]:[minute]:[second]", } }; + #[allow(clippy::unwrap_used)] // time_format is okay! let parsed_format = time::format_description::parse(time_format).unwrap(); return Self::time_string(datetime, &parsed_format, time_zone_for_timestamps); @@ -202,10 +207,14 @@ impl Time { time_format: &str, time_zone_for_timestamps: TimeZone, ) -> Option { - self.to_datetime().map(|datetime| { - let parsed_format = time::format_description::parse(time_format).unwrap(); - Self::time_string(datetime, &parsed_format, time_zone_for_timestamps) - }) + let datetime = self.to_datetime()?; + let parsed_format = time::format_description::parse(time_format).ok()?; + + Some(Self::time_string( + datetime, + &parsed_format, + time_zone_for_timestamps, + )) } #[inline] diff --git a/crates/re_types_core/src/result.rs b/crates/re_types_core/src/result.rs index 02960ca87a09..098d1edf20f7 100644 --- a/crates/re_types_core/src/result.rs +++ b/crates/re_types_core/src/result.rs @@ -1,4 +1,4 @@ -use std::{fmt::Display, ops::Deref}; +use std::{any, fmt::Display, ops::Deref}; use crate::ComponentName; @@ -208,6 +208,9 @@ pub enum DeserializationError { backtrace: _Backtrace, }, + #[error("Downcast to {to} failed")] + DowncastError { to: String, backtrace: _Backtrace }, + #[error("serde-based deserialization (`attr.rust.serde_type`) failed: {reason}")] SerdeFailure { reason: String, @@ -318,6 +321,14 @@ impl DeserializationError { } } + #[inline] + pub fn downcast_error() -> Self { + Self::DowncastError { + to: any::type_name::().to_owned(), + backtrace: ::backtrace::Backtrace::new_unresolved(), + } + } + #[inline] pub fn serde_failure(reason: impl AsRef) -> Self { Self::SerdeFailure { @@ -345,6 +356,7 @@ impl DeserializationError { | Self::DatatypeMismatch { backtrace, .. } | Self::OffsetOutOfBounds { backtrace, .. } | Self::OffsetSliceOutOfBounds { backtrace, .. } + | Self::DowncastError { backtrace, .. } | Self::SerdeFailure { backtrace, .. } => Some(backtrace.clone()), Self::DataCellError(_) | Self::ValidationError(_) => None, } From a4f0e7219f98cb13248aee6383f3c23325d0e948 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Mon, 27 May 2024 11:59:46 +0200 Subject: [PATCH 08/10] Move blueprint tree UI to new `re_blueprint_tree` crate (#6427) ### What - Fixes #6415 Straightforward refactor overall. Made two calls: - Create a `ui` sub-module in `re_viewport_blueprint` to host the `add_space_view_or_container` modal (used by both the blueprint tree and the selection panel). - Moved down two small utility functions in `re_viewer_context` (in `lib.rs`). - There was a weird double-buffering of the drop target container ID going through `TreeAction`. This is gone in favour of a much simpler approach (still double-buffered though). (Chained to #6423) ![](https://static.rerun.io/crates/1b123b5323a7d1366bdbd6c2fd5a788f46d20caf/1200w.png) ### 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/6427?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/6427?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/6427) - [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`. --- ARCHITECTURE.md | 11 +- Cargo.lock | 24 +- Cargo.toml | 1 + crates/re_blueprint_tree/Cargo.toml | 35 ++ crates/re_blueprint_tree/README.md | 10 + .../src/blueprint_tree.rs} | 505 +++++++++++------- crates/re_blueprint_tree/src/lib.rs | 5 + crates/re_viewer/Cargo.toml | 1 + crates/re_viewer/src/app_state.rs | 15 +- crates/re_viewer/src/ui/blueprint_panel.rs | 68 --- crates/re_viewer/src/ui/mod.rs | 2 - crates/re_viewer/src/ui/selection_panel.rs | 15 +- crates/re_viewer_context/src/lib.rs | 21 + crates/re_viewport/Cargo.toml | 1 - crates/re_viewport/src/lib.rs | 16 - crates/re_viewport/src/viewport.rs | 44 +- crates/re_viewport_blueprint/Cargo.toml | 3 + crates/re_viewport_blueprint/src/lib.rs | 1 + .../re_viewport_blueprint/src/tree_actions.rs | 6 - .../ui}/add_space_view_or_container_modal.rs | 7 +- crates/re_viewport_blueprint/src/ui/mod.rs | 33 ++ .../src/viewport_blueprint.rs | 8 - 22 files changed, 479 insertions(+), 353 deletions(-) create mode 100644 crates/re_blueprint_tree/Cargo.toml create mode 100644 crates/re_blueprint_tree/README.md rename crates/{re_viewport/src/viewport_blueprint_ui.rs => re_blueprint_tree/src/blueprint_tree.rs} (68%) create mode 100644 crates/re_blueprint_tree/src/lib.rs delete mode 100644 crates/re_viewer/src/ui/blueprint_panel.rs rename crates/{re_viewport/src => re_viewport_blueprint/src/ui}/add_space_view_or_container_modal.rs (97%) create mode 100644 crates/re_viewport_blueprint/src/ui/mod.rs diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6388b7dd05ce..6be030512871 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -88,11 +88,11 @@ Of course, this will only take us so far. In the future we plan on caching queri Here is an overview of the crates included in the project: - - - - - + + + + + @@ -127,6 +127,7 @@ Update instructions: | Crate | Description | |-----------------------------|----------------------------------------------------------------------------------------| +| re_blueprint_tree | The UI for the blueprint tree in the left panel. | | re_viewer | The Rerun Viewer | | re_viewport | The central viewport panel of the Rerun viewer. | | re_time_panel | The time panel of the Rerun Viewer, allowing to control the displayed timeline & time. | diff --git a/Cargo.lock b/Cargo.lock index 96b1878451fb..8328078fed7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4150,6 +4150,25 @@ dependencies = [ "simdutf8", ] +[[package]] +name = "re_blueprint_tree" +version = "0.17.0-alpha.2" +dependencies = [ + "egui", + "itertools 0.12.0", + "re_context_menu", + "re_data_ui", + "re_entity_db", + "re_log", + "re_log_types", + "re_tracing", + "re_types", + "re_ui", + "re_viewer_context", + "re_viewport_blueprint", + "smallvec", +] + [[package]] name = "re_build_info" version = "0.17.0-alpha.2" @@ -5025,6 +5044,7 @@ dependencies = [ "once_cell", "poll-promise", "re_analytics", + "re_blueprint_tree", "re_build_info", "re_build_tools", "re_context_menu", @@ -5141,7 +5161,6 @@ dependencies = [ "re_ui", "re_viewer_context", "re_viewport_blueprint", - "smallvec", ] [[package]] @@ -5153,6 +5172,8 @@ dependencies = [ "egui_tiles", "itertools 0.12.0", "nohash-hasher", + "once_cell", + "parking_lot", "re_data_store", "re_entity_db", "re_log", @@ -5161,6 +5182,7 @@ dependencies = [ "re_types", "re_types_blueprint", "re_types_core", + "re_ui", "re_viewer_context", "slotmap", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 31c6add8698f..7c1cbdd8641d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ version = "0.17.0-alpha.2" # In particular: if we compile rerun 0.3.0-alpha.0 we only want it to use # re_log_types 0.3.0-alpha.0, NOT 0.3.0-alpha.4 even though it is newer and semver-compatible. re_analytics = { path = "crates/re_analytics", version = "=0.17.0-alpha.2", default-features = false } +re_blueprint_tree = { path = "crates/re_blueprint_tree", version = "=0.17.0-alpha.2", default-features = false } re_build_info = { path = "crates/re_build_info", version = "=0.17.0-alpha.2", default-features = false } re_build_tools = { path = "crates/re_build_tools", version = "=0.17.0-alpha.2", default-features = false } re_crash_handler = { path = "crates/re_crash_handler", version = "=0.17.0-alpha.2", default-features = false } diff --git a/crates/re_blueprint_tree/Cargo.toml b/crates/re_blueprint_tree/Cargo.toml new file mode 100644 index 000000000000..f383ed3d0517 --- /dev/null +++ b/crates/re_blueprint_tree/Cargo.toml @@ -0,0 +1,35 @@ +[package] +authors.workspace = true +description = "The UI for the blueprint tree in the left panel." +edition.workspace = true +homepage.workspace = true +license.workspace = true +name = "re_blueprint_tree" +publish = true +readme = "README.md" +repository.workspace = true +rust-version.workspace = true +version.workspace = true +include = ["../../LICENSE-APACHE", "../../LICENSE-MIT", "**/*.rs", "Cargo.toml"] + +[lints] +workspace = true + +[package.metadata.docs.rs] +all-features = true + +[dependencies] +re_context_menu.workspace = true +re_data_ui.workspace = true +re_entity_db = { workspace = true, features = ["serde"] } +re_log_types.workspace = true +re_log.workspace = true +re_tracing.workspace = true +re_types.workspace = true +re_ui.workspace = true +re_viewer_context.workspace = true +re_viewport_blueprint.workspace = true + +egui.workspace = true +itertools.workspace = true +smallvec.workspace = true diff --git a/crates/re_blueprint_tree/README.md b/crates/re_blueprint_tree/README.md new file mode 100644 index 000000000000..aab22dc3fa01 --- /dev/null +++ b/crates/re_blueprint_tree/README.md @@ -0,0 +1,10 @@ +# re_blueprint_tree + +Part of the [`rerun`](https://github.com/rerun-io/rerun) family of crates. + +[![Latest version](https://img.shields.io/crates/v/re_blueprint_tree.svg)](https://crates.io/crates/re_blueprint_tree?speculative-link) +[![Documentation](https://docs.rs/re_blueprint_tree/badge.svg)](https://docs.rs/re_blueprint_tree?speculative-link) +![MIT](https://img.shields.io/badge/license-MIT-blue.svg) +![Apache](https://img.shields.io/badge/license-Apache-blue.svg) + +The UI for the blueprint tree in the left panel. diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_blueprint_tree/src/blueprint_tree.rs similarity index 68% rename from crates/re_viewport/src/viewport_blueprint_ui.rs rename to crates/re_blueprint_tree/src/blueprint_tree.rs index 805913026910..cc07858f26ed 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_blueprint_tree/src/blueprint_tree.rs @@ -8,21 +8,15 @@ use re_entity_db::InstancePath; use re_log_types::EntityPath; use re_types::blueprint::components::Visible; use re_ui::{drag_and_drop::DropTarget, list_item, ReUi}; -use re_viewer_context::{CollapseScope, Contents, ContentsName, DataResultTree}; +use re_viewer_context::{ + contents_name_style, icon_for_container_kind, CollapseScope, Contents, DataResultTree, + SystemCommandSender, +}; use re_viewer_context::{ ContainerId, DataQueryResult, DataResultNode, HoverHighlight, Item, SpaceViewId, ViewerContext, }; -use re_viewport_blueprint::SpaceViewBlueprint; - -use crate::Viewport; - -/// The style to use for displaying this space view name in the UI. -pub fn contents_name_style(name: &ContentsName) -> re_ui::LabelStyle { - match name { - ContentsName::Named(_) => re_ui::LabelStyle::Normal, - ContentsName::Placeholder(_) => re_ui::LabelStyle::Unnamed, - } -} +use re_viewport_blueprint::ui::show_add_space_view_or_container_modal; +use re_viewport_blueprint::{SpaceViewBlueprint, ViewportBlueprint}; enum DataResultNodeOrPath<'a> { Path(&'a EntityPath), @@ -53,23 +47,78 @@ impl<'a> DataResultNodeOrPath<'a> { } } -impl Viewport<'_, '_> { +/// Holds the state of the blueprint tree UI. +#[derive(Default)] +pub struct BlueprintTree { + /// The item that should be focused on in the blueprint tree. + /// + /// Set at each frame by [`Self::tree_ui`]. This is similar to + /// [`ViewerContext::focused_item`] but account for how specifically the blueprint tree should + /// handle the focused item. + blueprint_tree_scroll_to_item: Option, + + /// Current candidate parent container for the ongoing drop. Should be drawn with special + /// highlight. + /// + /// See [`Self::is_candidate_drop_parent_container`] for details. + candidate_drop_parent_container_id: Option, + + /// Candidate parent container to be drawn on next frame. + /// + /// We double-buffer this value to deal with ordering constraints. + next_candidate_drop_parent_container_id: Option, +} + +impl BlueprintTree { + /// Show the Blueprint section of the left panel based on the current [`ViewportBlueprint`] + pub fn show( + &mut self, + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, + ) { + ctx.re_ui.panel_content(ui, |_, ui| { + ctx.re_ui.panel_title_bar_with_buttons( + ui, + "Blueprint", + Some("The blueprint is where you can configure the Rerun Viewer"), + |ui| { + self.add_new_spaceview_button_ui(ctx, blueprint, ui); + reset_blueprint_button_ui(ctx, ui); + }, + ); + }); + + // This call is excluded from `panel_content` because it has a ScrollArea, which should not be + // inset. Instead, it calls panel_content itself inside the ScrollArea. + self.tree_ui(ctx, blueprint, ui); + } + /// Show the blueprint panel tree view. - pub fn tree_ui(&mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { + fn tree_ui( + &mut self, + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, + ) { re_tracing::profile_function!(); + // The candidate drop parent container is double-buffered, so here we have the buffer swap. + self.candidate_drop_parent_container_id = self.next_candidate_drop_parent_container_id; + self.next_candidate_drop_parent_container_id = None; + egui::ScrollArea::both() .id_source("blueprint_tree_scroll_area") .auto_shrink([true, false]) .show(ui, |ui| { ctx.re_ui.panel_content(ui, |_, ui| { - self.state.blueprint_tree_scroll_to_item = ctx + self.blueprint_tree_scroll_to_item = ctx .focused_item .as_ref() - .and_then(|item| self.handle_focused_item(ctx, ui, item)); + .and_then(|item| handle_focused_item(ctx, blueprint, ui, item)); list_item::list_item_scope(ui, "blueprint tree", |ui| { - self.root_container_tree_ui(ctx, ui); + self.root_container_tree_ui(ctx, blueprint, ui); }); let empty_space_response = @@ -82,6 +131,7 @@ impl Viewport<'_, '_> { // handle drag and drop interaction on empty space self.handle_empty_space_drag_and_drop_interaction( + blueprint, ui, empty_space_response.rect, ); @@ -89,131 +139,9 @@ impl Viewport<'_, '_> { }); } - /// Expand all required items and compute which item we should scroll to. - fn handle_focused_item( - &self, - ctx: &ViewerContext<'_>, - ui: &egui::Ui, - focused_item: &Item, - ) -> Option { - match focused_item { - Item::AppId(_) | Item::DataSource(_) | Item::StoreId(_) => None, - - Item::Container(container_id) => { - self.expand_all_contents_until(ui.ctx(), &Contents::Container(*container_id)); - Some(focused_item.clone()) - } - Item::SpaceView(space_view_id) => { - self.expand_all_contents_until(ui.ctx(), &Contents::SpaceView(*space_view_id)); - ctx.focused_item.clone() - } - Item::DataResult(space_view_id, instance_path) => { - self.expand_all_contents_until(ui.ctx(), &Contents::SpaceView(*space_view_id)); - self.expand_all_data_results_until( - ctx, - ui.ctx(), - space_view_id, - &instance_path.entity_path, - ); - - ctx.focused_item.clone() - } - Item::InstancePath(instance_path) => { - let space_view_ids = - self.list_space_views_with_entity(ctx, &instance_path.entity_path); - - // focus on the first matching data result - let res = space_view_ids - .first() - .map(|id| Item::DataResult(*id, instance_path.clone())); - - for space_view_id in space_view_ids { - self.expand_all_contents_until(ui.ctx(), &Contents::SpaceView(space_view_id)); - self.expand_all_data_results_until( - ctx, - ui.ctx(), - &space_view_id, - &instance_path.entity_path, - ); - } - - res - } - Item::ComponentPath(component_path) => self.handle_focused_item( - ctx, - ui, - &Item::InstancePath(InstancePath::entity_all(component_path.entity_path.clone())), - ), - } - } - - /// Expand all containers until reaching the provided content. - fn expand_all_contents_until(&self, egui_ctx: &egui::Context, focused_contents: &Contents) { - //TODO(ab): this could look nicer if `Contents` was declared in re_view_context :) - let expend_contents = |contents: &Contents| match contents { - Contents::Container(container_id) => CollapseScope::BlueprintTree - .container(*container_id) - .set_open(egui_ctx, true), - Contents::SpaceView(space_view_id) => CollapseScope::BlueprintTree - .space_view(*space_view_id) - .set_open(egui_ctx, true), - }; - - self.blueprint.visit_contents(&mut |contents, hierarchy| { - if contents == focused_contents { - expend_contents(contents); - for parent in hierarchy { - expend_contents(&Contents::Container(*parent)); - } - } - }); - } - - /// List all space views that have the provided entity as data result. - #[inline] - fn list_space_views_with_entity( - &self, - ctx: &ViewerContext<'_>, - entity_path: &EntityPath, - ) -> SmallVec<[SpaceViewId; 4]> { - let mut space_view_ids = SmallVec::new(); - self.blueprint.visit_contents(&mut |contents, _| { - if let Contents::SpaceView(space_view_id) = contents { - let result_tree = &ctx.lookup_query_result(*space_view_id).tree; - if result_tree.lookup_node_by_path(entity_path).is_some() { - space_view_ids.push(*space_view_id); - } - } - }); - space_view_ids - } - - /// Expand data results of the provided space view all the way to the provided entity. - #[allow(clippy::unused_self)] - fn expand_all_data_results_until( - &self, - ctx: &ViewerContext<'_>, - egui_ctx: &egui::Context, - space_view_id: &SpaceViewId, - entity_path: &EntityPath, - ) { - let result_tree = &ctx.lookup_query_result(*space_view_id).tree; - if result_tree.lookup_node_by_path(entity_path).is_some() { - if let Some(root_node) = result_tree.root_node() { - EntityPath::incremental_walk(Some(&root_node.data_result.entity_path), entity_path) - .chain(std::iter::once(root_node.data_result.entity_path.clone())) - .for_each(|entity_path| { - CollapseScope::BlueprintTree - .data_result(*space_view_id, entity_path) - .set_open(egui_ctx, true); - }); - } - } - } - /// Check if the provided item should be scrolled to. fn scroll_to_me_if_needed(&self, ui: &egui::Ui, item: &Item, response: &egui::Response) { - if Some(item) == self.state.blueprint_tree_scroll_to_item.as_ref() { + if Some(item) == self.blueprint_tree_scroll_to_item.as_ref() { // Scroll only if the entity isn't already visible. This is important because that's what // happens when double-clicking an entity _in the blueprint tree_. In such case, it would be // annoying to induce a scroll motion. @@ -230,18 +158,19 @@ impl Viewport<'_, '_> { } fn contents_ui( - &self, + &mut self, ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, ui: &mut egui::Ui, contents: &Contents, parent_visible: bool, ) { match contents { Contents::Container(container_id) => { - self.container_tree_ui(ctx, ui, container_id, parent_visible); + self.container_tree_ui(ctx, blueprint, ui, container_id, parent_visible); } Contents::SpaceView(space_view_id) => { - self.space_view_entry_ui(ctx, ui, space_view_id, parent_visible); + self.space_view_entry_ui(ctx, blueprint, ui, space_view_id, parent_visible); } }; } @@ -250,13 +179,18 @@ impl Viewport<'_, '_> { /// /// The root container is different from other containers in that it cannot be removed or dragged, and it cannot be /// collapsed, so it's drawn without a collapsing triangle. - fn root_container_tree_ui(&self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { - let Some(container_id) = self.blueprint.root_container else { + fn root_container_tree_ui( + &mut self, + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, + ) { + let Some(container_id) = blueprint.root_container else { // nothing to draw if there is no root container return; }; - let Some(container_blueprint) = self.blueprint.containers.get(&container_id) else { + let Some(container_blueprint) = blueprint.containers.get(&container_id) else { re_log::warn_once!("Cannot find root container {container_id}"); return; }; @@ -267,23 +201,21 @@ impl Viewport<'_, '_> { let item_response = list_item::ListItem::new(ctx.re_ui) .selected(ctx.selection().contains_item(&item)) .draggable(false) - .drop_target_style(self.state.is_candidate_drop_parent_container(&container_id)) + .drop_target_style(self.is_candidate_drop_parent_container(&container_id)) .show_flat( ui, list_item::LabelContent::new(format!("Viewport ({})", container_name.as_ref())) .label_style(contents_name_style(&container_name)) - .with_icon(crate::icon_for_container_kind( - &container_blueprint.container_kind, - )), + .with_icon(icon_for_container_kind(&container_blueprint.container_kind)), ); for child in &container_blueprint.contents { - self.contents_ui(ctx, ui, child, true); + self.contents_ui(ctx, blueprint, ui, child, true); } context_menu_ui_for_item( ctx, - self.blueprint, + blueprint, &item, &item_response, SelectionUpdateBehavior::UseSelection, @@ -292,6 +224,7 @@ impl Viewport<'_, '_> { ctx.select_hovered_on_click(&item_response, item); self.handle_root_container_drag_and_drop_interaction( + blueprint, ui, Contents::Container(container_id), &item_response, @@ -299,8 +232,9 @@ impl Viewport<'_, '_> { } fn container_tree_ui( - &self, + &mut self, ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, ui: &mut egui::Ui, container_id: &ContainerId, parent_visible: bool, @@ -308,7 +242,7 @@ impl Viewport<'_, '_> { let item = Item::Container(*container_id); let content = Contents::Container(*container_id); - let Some(container_blueprint) = self.blueprint.containers.get(container_id) else { + let Some(container_blueprint) = blueprint.containers.get(container_id) else { re_log::warn_once!("Ignoring unknown container {container_id}"); return; }; @@ -323,16 +257,14 @@ impl Viewport<'_, '_> { let item_content = list_item::LabelContent::new(container_name.as_ref()) .subdued(!container_visible) .label_style(contents_name_style(&container_name)) - .with_icon(crate::icon_for_container_kind( - &container_blueprint.container_kind, - )) + .with_icon(icon_for_container_kind(&container_blueprint.container_kind)) .with_buttons(|re_ui, ui| { let vis_response = visibility_button_ui(re_ui, ui, parent_visible, &mut visible); let remove_response = remove_button_ui(re_ui, ui, "Remove container"); if remove_response.clicked() { - self.blueprint.mark_user_interaction(ctx); - self.blueprint.remove_contents(content); + blueprint.mark_user_interaction(ctx); + blueprint.remove_contents(content); } remove_response | vis_response @@ -348,16 +280,16 @@ impl Viewport<'_, '_> { } = list_item::ListItem::new(ctx.re_ui) .selected(ctx.selection().contains_item(&item)) .draggable(true) - .drop_target_style(self.state.is_candidate_drop_parent_container(container_id)) + .drop_target_style(self.is_candidate_drop_parent_container(container_id)) .show_hierarchical_with_children(ui, id, default_open, item_content, |_, ui| { for child in &container_blueprint.contents { - self.contents_ui(ctx, ui, child, container_visible); + self.contents_ui(ctx, blueprint, ui, child, container_visible); } }); context_menu_ui_for_item( ctx, - self.blueprint, + blueprint, &item, &response, SelectionUpdateBehavior::UseSelection, @@ -365,11 +297,11 @@ impl Viewport<'_, '_> { self.scroll_to_me_if_needed(ui, &item, &response); ctx.select_hovered_on_click(&response, item); - self.blueprint - .set_content_visibility(ctx, &content, visible); + blueprint.set_content_visibility(ctx, &content, visible); self.handle_drag_and_drop_interaction( ctx, + blueprint, ui, content, &response, @@ -378,14 +310,15 @@ impl Viewport<'_, '_> { } fn space_view_entry_ui( - &self, + &mut self, ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, ui: &mut egui::Ui, space_view_id: &SpaceViewId, container_visible: bool, ) { - let Some(space_view) = self.blueprint.space_views.get(space_view_id) else { - re_log::warn_once!("Bug: asked to show a ui for a space view that doesn't exist"); + let Some(space_view) = blueprint.space_views.get(space_view_id) else { + re_log::warn_once!("Bug: asked to show a UI for a space view that doesn't exist"); return; }; debug_assert_eq!(space_view.id, *space_view_id); @@ -415,9 +348,8 @@ impl Viewport<'_, '_> { let response = remove_button_ui(re_ui, ui, "Remove space view from the viewport"); if response.clicked() { - self.blueprint.mark_user_interaction(ctx); - self.blueprint - .remove_contents(Contents::SpaceView(*space_view_id)); + blueprint.mark_user_interaction(ctx); + blueprint.remove_contents(Contents::SpaceView(*space_view_id)); } response | vis_response @@ -438,6 +370,7 @@ impl Viewport<'_, '_> { // Always show the origin hierarchy first. self.space_view_entity_hierarchy_ui( ctx, + blueprint, ui, query_result, &DataResultNodeOrPath::from_path_lookup(result_tree, &space_view.space_origin), @@ -474,6 +407,7 @@ impl Viewport<'_, '_> { for projection in projections { self.space_view_entity_hierarchy_ui( ctx, + blueprint, ui, query_result, &DataResultNodeOrPath::DataResultNode(projection), @@ -488,12 +422,12 @@ impl Viewport<'_, '_> { response = response.on_hover_text("Space view"); if response.clicked() { - self.blueprint.focus_tab(space_view.id); + blueprint.focus_tab(space_view.id); } context_menu_ui_for_item( ctx, - self.blueprint, + blueprint, &item, &response, SelectionUpdateBehavior::UseSelection, @@ -503,10 +437,10 @@ impl Viewport<'_, '_> { let content = Contents::SpaceView(*space_view_id); - self.blueprint - .set_content_visibility(ctx, &content, visible); + blueprint.set_content_visibility(ctx, &content, visible); self.handle_drag_and_drop_interaction( ctx, + blueprint, ui, content, &response, @@ -518,6 +452,7 @@ impl Viewport<'_, '_> { fn space_view_entity_hierarchy_ui( &self, ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, ui: &mut egui::Ui, query_result: &DataQueryResult, node_or_path: &DataResultNodeOrPath<'_>, @@ -649,6 +584,7 @@ impl Viewport<'_, '_> { self.space_view_entity_hierarchy_ui( ctx, + blueprint, ui, query_result, &DataResultNodeOrPath::DataResultNode(child_node), @@ -682,7 +618,7 @@ impl Viewport<'_, '_> { context_menu_ui_for_item( ctx, - self.blueprint, + blueprint, &item, &response, SelectionUpdateBehavior::UseSelection, @@ -691,7 +627,14 @@ impl Viewport<'_, '_> { ctx.select_hovered_on_click(&response, item); } - pub fn add_new_spaceview_button_ui(&mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { + /// Add a button to trigger the addition of a new space view or container. + #[allow(clippy::unused_self)] + fn add_new_spaceview_button_ui( + &self, + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, + ) { if ctx .re_ui .small_icon_button(ui, &re_ui::icons::ADD) @@ -704,11 +647,11 @@ impl Viewport<'_, '_> { if let Some(Item::Container(container_id)) = ctx.selection().single_item() { Some(*container_id) } else { - self.blueprint.root_container + blueprint.root_container }; if let Some(target_container_id) = target_container_id { - self.show_add_space_view_or_container_modal(target_container_id); + show_add_space_view_or_container_modal(target_container_id); } } } @@ -717,7 +660,8 @@ impl Viewport<'_, '_> { // drag and drop support fn handle_root_container_drag_and_drop_interaction( - &self, + &mut self, + blueprint: &ViewportBlueprint, ui: &egui::Ui, contents: Contents, response: &egui::Response, @@ -755,13 +699,14 @@ impl Viewport<'_, '_> { ); if let Some(drop_target) = drop_target { - self.handle_drop_target(ui, dragged_item_id, &drop_target); + self.handle_drop_target(blueprint, ui, dragged_item_id, &drop_target); } } fn handle_drag_and_drop_interaction( - &self, + &mut self, ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, ui: &egui::Ui, contents: Contents, response: &egui::Response, @@ -793,13 +738,13 @@ impl Viewport<'_, '_> { // let Some((parent_container_id, position_index_in_parent)) = - self.blueprint.find_parent_and_position_index(&contents) + blueprint.find_parent_and_position_index(&contents) else { return; }; let previous_container = if position_index_in_parent > 0 { - self.blueprint + blueprint .container(&parent_container_id) .map(|container| container.contents[position_index_in_parent - 1]) .filter(|contents| matches!(contents, Contents::Container(_))) @@ -838,11 +783,16 @@ impl Viewport<'_, '_> { ); if let Some(drop_target) = drop_target { - self.handle_drop_target(ui, dragged_item_id, &drop_target); + self.handle_drop_target(blueprint, ui, dragged_item_id, &drop_target); } } - fn handle_empty_space_drag_and_drop_interaction(&self, ui: &egui::Ui, empty_space: egui::Rect) { + fn handle_empty_space_drag_and_drop_interaction( + &mut self, + blueprint: &ViewportBlueprint, + ui: &egui::Ui, + empty_space: egui::Rect, + ) { // // check if a drag is in progress and set the cursor accordingly // @@ -861,7 +811,7 @@ impl Viewport<'_, '_> { // TODO(ab): this is a rather primitive behavior. Ideally we should allow dropping in the last container based // on the horizontal position of the cursor. - let Some(root_container_id) = self.blueprint.root_container else { + let Some(root_container_id) = blueprint.root_container else { return; }; @@ -873,12 +823,13 @@ impl Viewport<'_, '_> { usize::MAX, ); - self.handle_drop_target(ui, dragged_item_id, &drop_target); + self.handle_drop_target(blueprint, ui, dragged_item_id, &drop_target); } } fn handle_drop_target( - &self, + &mut self, + blueprint: &ViewportBlueprint, ui: &Ui, dragged_item_id: Contents, drop_target: &DropTarget, @@ -886,8 +837,7 @@ impl Viewport<'_, '_> { // We cannot allow the target location to be "inside" the dragged item, because that would amount moving // myself inside of me. if let Contents::Container(dragged_container_id) = &dragged_item_id { - if self - .blueprint + if blueprint .is_contents_in_container(&drop_target.target_parent_id, dragged_container_id) { return; @@ -906,7 +856,7 @@ impl Viewport<'_, '_> { }; if ui.input(|i| i.pointer.any_released()) { - self.blueprint.move_contents( + blueprint.move_contents( dragged_item_id, target_container_id, drop_target.target_position_index, @@ -914,13 +864,184 @@ impl Viewport<'_, '_> { egui::DragAndDrop::clear_payload(ui.ctx()); } else { - self.blueprint.set_drop_target(&target_container_id); + self.next_candidate_drop_parent_container_id = Some(target_container_id); } } + + /// Is the provided container the current candidate parent container for the ongoing drag? + /// + /// When a drag is in progress, the candidate parent container for the dragged item should be highlighted. Note that + /// this can happen when hovering said container, its direct children, or even the item just after it. + fn is_candidate_drop_parent_container(&self, container_id: &ContainerId) -> bool { + self.candidate_drop_parent_container_id.as_ref() == Some(container_id) + } } // ---------------------------------------------------------------------------- +fn reset_blueprint_button_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { + let default_blueprint_id = ctx + .store_context + .hub + .default_blueprint_id_for_app(&ctx.store_context.app_id); + + let default_blueprint = default_blueprint_id.and_then(|id| ctx.store_context.bundle.get(id)); + + let mut disabled_reason = None; + + if let Some(default_blueprint) = default_blueprint { + let active_is_clone_of_default = + Some(default_blueprint.store_id()) == ctx.store_context.blueprint.cloned_from(); + let last_modified_at_the_same_time = + default_blueprint.latest_row_id() == ctx.store_context.blueprint.latest_row_id(); + if active_is_clone_of_default && last_modified_at_the_same_time { + disabled_reason = Some("No modifications has been made"); + } + } + + let enabled = disabled_reason.is_none(); + let response = ui.add_enabled( + enabled, + ctx.re_ui.small_icon_button_widget(ui, &re_ui::icons::RESET), + ); + + let response = if let Some(disabled_reason) = disabled_reason { + response.on_disabled_hover_text(disabled_reason) + } else { + let hover_text = if default_blueprint_id.is_some() { + "Reset to the default blueprint for this app" + } else { + "Re-populate viewport with automatically chosen space views" + }; + response.on_hover_text(hover_text) + }; + + if response.clicked() { + ctx.command_sender + .send_system(re_viewer_context::SystemCommand::ClearActiveBlueprint); + } +} + +/// Expand all required items and compute which item we should scroll to. +fn handle_focused_item( + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + ui: &egui::Ui, + focused_item: &Item, +) -> Option { + match focused_item { + Item::AppId(_) | Item::DataSource(_) | Item::StoreId(_) => None, + + Item::Container(container_id) => { + expand_all_contents_until(blueprint, ui.ctx(), &Contents::Container(*container_id)); + Some(focused_item.clone()) + } + Item::SpaceView(space_view_id) => { + expand_all_contents_until(blueprint, ui.ctx(), &Contents::SpaceView(*space_view_id)); + ctx.focused_item.clone() + } + Item::DataResult(space_view_id, instance_path) => { + expand_all_contents_until(blueprint, ui.ctx(), &Contents::SpaceView(*space_view_id)); + expand_all_data_results_until(ctx, ui.ctx(), space_view_id, &instance_path.entity_path); + + ctx.focused_item.clone() + } + Item::InstancePath(instance_path) => { + let space_view_ids = + list_space_views_with_entity(ctx, blueprint, &instance_path.entity_path); + + // focus on the first matching data result + let res = space_view_ids + .first() + .map(|id| Item::DataResult(*id, instance_path.clone())); + + for space_view_id in space_view_ids { + expand_all_contents_until(blueprint, ui.ctx(), &Contents::SpaceView(space_view_id)); + expand_all_data_results_until( + ctx, + ui.ctx(), + &space_view_id, + &instance_path.entity_path, + ); + } + + res + } + Item::ComponentPath(component_path) => handle_focused_item( + ctx, + blueprint, + ui, + &Item::InstancePath(InstancePath::entity_all(component_path.entity_path.clone())), + ), + } +} + +/// Expand all containers until reaching the provided content. +fn expand_all_contents_until( + blueprint: &ViewportBlueprint, + egui_ctx: &egui::Context, + focused_contents: &Contents, +) { + //TODO(ab): this could look nicer if `Contents` was declared in re_view_context :) + let expend_contents = |contents: &Contents| match contents { + Contents::Container(container_id) => CollapseScope::BlueprintTree + .container(*container_id) + .set_open(egui_ctx, true), + Contents::SpaceView(space_view_id) => CollapseScope::BlueprintTree + .space_view(*space_view_id) + .set_open(egui_ctx, true), + }; + + blueprint.visit_contents(&mut |contents, hierarchy| { + if contents == focused_contents { + expend_contents(contents); + for parent in hierarchy { + expend_contents(&Contents::Container(*parent)); + } + } + }); +} + +/// List all space views that have the provided entity as data result. +#[inline] +fn list_space_views_with_entity( + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + entity_path: &EntityPath, +) -> SmallVec<[SpaceViewId; 4]> { + let mut space_view_ids = SmallVec::new(); + blueprint.visit_contents(&mut |contents, _| { + if let Contents::SpaceView(space_view_id) = contents { + let result_tree = &ctx.lookup_query_result(*space_view_id).tree; + if result_tree.lookup_node_by_path(entity_path).is_some() { + space_view_ids.push(*space_view_id); + } + } + }); + space_view_ids +} + +/// Expand data results of the provided space view all the way to the provided entity. +fn expand_all_data_results_until( + ctx: &ViewerContext<'_>, + egui_ctx: &egui::Context, + space_view_id: &SpaceViewId, + entity_path: &EntityPath, +) { + let result_tree = &ctx.lookup_query_result(*space_view_id).tree; + if result_tree.lookup_node_by_path(entity_path).is_some() { + if let Some(root_node) = result_tree.root_node() { + EntityPath::incremental_walk(Some(&root_node.data_result.entity_path), entity_path) + .chain(std::iter::once(root_node.data_result.entity_path.clone())) + .for_each(|entity_path| { + CollapseScope::BlueprintTree + .data_result(*space_view_id, entity_path) + .set_open(egui_ctx, true); + }); + } + } +} + fn remove_button_ui(re_ui: &ReUi, ui: &mut Ui, tooltip: &str) -> Response { re_ui .small_icon_button(ui, &re_ui::icons::REMOVE) diff --git a/crates/re_blueprint_tree/src/lib.rs b/crates/re_blueprint_tree/src/lib.rs new file mode 100644 index 000000000000..843349464447 --- /dev/null +++ b/crates/re_blueprint_tree/src/lib.rs @@ -0,0 +1,5 @@ +//! This crate implements the UI for the blueprint tree in the left panel. + +mod blueprint_tree; + +pub use blueprint_tree::BlueprintTree; diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index a99ebad1d8fc..56b341432d91 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -40,6 +40,7 @@ analytics = ["dep:re_analytics"] # Internal: re_context_menu.workspace = true re_build_info.workspace = true +re_blueprint_tree.workspace = true re_data_loader.workspace = true re_data_source.workspace = true re_data_store.workspace = true diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index be0c5f150826..6775f4a05deb 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -11,10 +11,11 @@ use re_viewer_context::{ SpaceViewClassRegistry, StoreContext, StoreHub, SystemCommandSender as _, ViewerContext, }; use re_viewport::{Viewport, ViewportState}; +use re_viewport_blueprint::ui::add_space_view_or_container_modal_ui; use re_viewport_blueprint::ViewportBlueprint; +use crate::app_blueprint::AppBlueprint; use crate::ui::recordings_panel_ui; -use crate::{app_blueprint::AppBlueprint, ui::blueprint_panel_ui}; const WATERMARK: bool = false; // Nice for recording media material @@ -35,6 +36,8 @@ pub struct AppState { selection_panel: crate::selection_panel::SelectionPanel, time_panel: re_time_panel::TimePanel, blueprint_panel: re_time_panel::TimePanel, + #[serde(skip)] + blueprint_tree: re_blueprint_tree::BlueprintTree, #[serde(skip)] welcome_screen: crate::ui::WelcomeScreen, @@ -65,6 +68,7 @@ impl Default for AppState { selection_panel: Default::default(), time_panel: Default::default(), blueprint_panel: re_time_panel::TimePanel::new_blueprint_panel(), + blueprint_tree: Default::default(), welcome_screen: Default::default(), viewport_state: Default::default(), selection_state: Default::default(), @@ -137,6 +141,7 @@ impl AppState { selection_panel, time_panel, blueprint_panel, + blueprint_tree, welcome_screen, viewport_state, selection_state, @@ -403,7 +408,7 @@ impl AppState { } if !show_welcome { - blueprint_panel_ui(&mut viewport, &ctx, ui); + blueprint_tree.show(&ctx, &viewport_blueprint, ui); } }); }, @@ -428,6 +433,12 @@ impl AppState { } }); + // + // Other UI things + // + + add_space_view_or_container_modal_ui(&ctx, &viewport_blueprint, ui); + // Process deferred layout operations and apply updates back to blueprint viewport.update_and_sync_tile_tree_to_blueprint(&ctx); diff --git a/crates/re_viewer/src/ui/blueprint_panel.rs b/crates/re_viewer/src/ui/blueprint_panel.rs deleted file mode 100644 index 823acee982ec..000000000000 --- a/crates/re_viewer/src/ui/blueprint_panel.rs +++ /dev/null @@ -1,68 +0,0 @@ -use re_viewer_context::{SystemCommandSender as _, ViewerContext}; -use re_viewport::Viewport; - -/// Show the Blueprint section of the left panel based on the current [`Viewport`] -pub fn blueprint_panel_ui( - viewport: &mut Viewport<'_, '_>, - ctx: &ViewerContext<'_>, - ui: &mut egui::Ui, -) { - ctx.re_ui.panel_content(ui, |_, ui| { - ctx.re_ui.panel_title_bar_with_buttons( - ui, - "Blueprint", - Some("The blueprint is where you can configure the Rerun Viewer"), - |ui| { - viewport.add_new_spaceview_button_ui(ctx, ui); - reset_blueprint_button_ui(ctx, ui); - }, - ); - }); - - // This call is excluded from `panel_content` because it has a ScrollArea, which should not be - // inset. Instead, it calls panel_content itself inside the ScrollArea. - viewport.tree_ui(ctx, ui); -} - -fn reset_blueprint_button_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { - let default_blueprint_id = ctx - .store_context - .hub - .default_blueprint_id_for_app(&ctx.store_context.app_id); - - let default_blueprint = default_blueprint_id.and_then(|id| ctx.store_context.bundle.get(id)); - - let mut disabled_reason = None; - - if let Some(default_blueprint) = default_blueprint { - let active_is_clone_of_default = - Some(default_blueprint.store_id()) == ctx.store_context.blueprint.cloned_from(); - let last_modified_at_the_same_time = - default_blueprint.latest_row_id() == ctx.store_context.blueprint.latest_row_id(); - if active_is_clone_of_default && last_modified_at_the_same_time { - disabled_reason = Some("No modifications has been made"); - } - } - - let enabled = disabled_reason.is_none(); - let response = ui.add_enabled( - enabled, - ctx.re_ui.small_icon_button_widget(ui, &re_ui::icons::RESET), - ); - - let response = if let Some(disabled_reason) = disabled_reason { - response.on_disabled_hover_text(disabled_reason) - } else { - let hover_text = if default_blueprint_id.is_some() { - "Reset to the default blueprint for this app" - } else { - "Re-populate viewport with automatically chosen space views" - }; - response.on_hover_text(hover_text) - }; - - if response.clicked() { - ctx.command_sender - .send_system(re_viewer_context::SystemCommand::ClearActiveBlueprint); - } -} diff --git a/crates/re_viewer/src/ui/mod.rs b/crates/re_viewer/src/ui/mod.rs index 4865f62983d8..802406d84163 100644 --- a/crates/re_viewer/src/ui/mod.rs +++ b/crates/re_viewer/src/ui/mod.rs @@ -1,4 +1,3 @@ -mod blueprint_panel; mod mobile_warning_ui; mod override_ui; mod recordings_panel; @@ -12,7 +11,6 @@ pub(crate) mod query_range_ui; pub(crate) mod selection_panel; pub(crate) mod space_view_space_origin_ui; -pub use blueprint_panel::blueprint_panel_ui; pub use recordings_panel::recordings_panel_ui; // ---- diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index a6bf739245df..5327477da7a5 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -18,11 +18,12 @@ use re_types::{ }; use re_ui::{icons, list_item, ReUi, SyntaxHighlighting as _}; use re_viewer_context::{ - gpu_bridge::colormap_dropdown_button_ui, ContainerId, Contents, DataQueryResult, - HoverHighlight, Item, SpaceViewClass, SpaceViewId, UiLayout, ViewerContext, + contents_name_style, gpu_bridge::colormap_dropdown_button_ui, icon_for_container_kind, + ContainerId, Contents, DataQueryResult, HoverHighlight, Item, SpaceViewClass, SpaceViewId, + UiLayout, ViewerContext, }; -use re_viewport::{contents_name_style, icon_for_container_kind, Viewport}; -use re_viewport_blueprint::ViewportBlueprint; +use re_viewport::Viewport; +use re_viewport_blueprint::{ui::show_add_space_view_or_container_modal, ViewportBlueprint}; use crate::ui::override_ui::override_visualizer_ui; use crate::{app_state::default_selection_panel_width, ui::override_ui::override_ui}; @@ -194,7 +195,7 @@ impl SelectionPanel { fn container_children( ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - viewport: &mut Viewport<'_, '_>, + viewport: &Viewport<'_, '_>, container_id: &ContainerId, ) { let Some(container) = viewport.blueprint.container(container_id) else { @@ -206,7 +207,7 @@ fn container_children( ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { if ctx.re_ui.small_icon_button(ui, &icons::ADD).clicked() { - viewport.show_add_space_view_or_container_modal(*container_id); + show_add_space_view_or_container_modal(*container_id); } }); }); @@ -362,7 +363,7 @@ fn what_is_selected_ui( ctx.re_ui, ui, container_name.as_ref(), - Some(re_viewport::icon_for_container_kind( + Some(re_viewer_context::icon_for_container_kind( &container_blueprint.container_kind, )), Some(contents_name_style(&container_name)), diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index a5229c8d7f11..67dca7747eee 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -87,3 +87,24 @@ pub enum NeedsRepaint { Yes, No, } + +// --- + +/// Determines the icon to use for a given container kind. +#[inline] +pub fn icon_for_container_kind(kind: &egui_tiles::ContainerKind) -> &'static re_ui::Icon { + match kind { + egui_tiles::ContainerKind::Tabs => &re_ui::icons::CONTAINER_TABS, + egui_tiles::ContainerKind::Horizontal => &re_ui::icons::CONTAINER_HORIZONTAL, + egui_tiles::ContainerKind::Vertical => &re_ui::icons::CONTAINER_VERTICAL, + egui_tiles::ContainerKind::Grid => &re_ui::icons::CONTAINER_GRID, + } +} + +/// The style to use for displaying this space view name in the UI. +pub fn contents_name_style(name: &ContentsName) -> re_ui::LabelStyle { + match name { + ContentsName::Named(_) => re_ui::LabelStyle::Normal, + ContentsName::Placeholder(_) => re_ui::LabelStyle::Unnamed, + } +} diff --git a/crates/re_viewport/Cargo.toml b/crates/re_viewport/Cargo.toml index 5bfeff8c338a..d862760ebe52 100644 --- a/crates/re_viewport/Cargo.toml +++ b/crates/re_viewport/Cargo.toml @@ -47,4 +47,3 @@ itertools.workspace = true nohash-hasher.workspace = true once_cell.workspace = true rayon.workspace = true -smallvec.workspace = true diff --git a/crates/re_viewport/src/lib.rs b/crates/re_viewport/src/lib.rs index a58c6d6ff85c..1ef82670ae1a 100644 --- a/crates/re_viewport/src/lib.rs +++ b/crates/re_viewport/src/lib.rs @@ -5,7 +5,6 @@ // TODO(#3408): remove unwrap() #![allow(clippy::unwrap_used)] -mod add_space_view_or_container_modal; mod auto_layout; mod screenshot; mod space_view_entity_picker; @@ -13,25 +12,10 @@ pub mod space_view_heuristics; mod space_view_highlights; mod system_execution; mod viewport; -mod viewport_blueprint_ui; pub use self::viewport::{Viewport, ViewportState}; -pub use self::viewport_blueprint_ui::contents_name_style; pub mod external { pub use re_space_view; pub use re_types_blueprint; } - -// --- - -/// Determines the icon to use for a given container kind. -#[inline] -pub fn icon_for_container_kind(kind: &egui_tiles::ContainerKind) -> &'static re_ui::Icon { - match kind { - egui_tiles::ContainerKind::Tabs => &re_ui::icons::CONTAINER_TABS, - egui_tiles::ContainerKind::Horizontal => &re_ui::icons::CONTAINER_HORIZONTAL, - egui_tiles::ContainerKind::Vertical => &re_ui::icons::CONTAINER_VERTICAL, - egui_tiles::ContainerKind::Grid => &re_ui::icons::CONTAINER_GRID, - } -} diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index 751904115055..27c2f256a316 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -12,13 +12,13 @@ use re_renderer::ScreenshotProcessor; use re_types::SpaceViewClassIdentifier; use re_ui::{Icon, ReUi}; use re_viewer_context::{ - blueprint_id_to_tile_id, ContainerId, Contents, Item, SpaceViewClassRegistry, SpaceViewId, - SpaceViewState, SystemExecutionOutput, ViewQuery, ViewerContext, + blueprint_id_to_tile_id, icon_for_container_kind, ContainerId, Contents, Item, + SpaceViewClassRegistry, SpaceViewId, SpaceViewState, SystemExecutionOutput, ViewQuery, + ViewerContext, }; use re_viewport_blueprint::{TreeAction, ViewportBlueprint}; use crate::{ - add_space_view_or_container_modal::AddSpaceViewOrContainerModal, icon_for_container_kind, screenshot::handle_pending_space_view_screenshots, space_view_entity_picker::SpaceViewEntityPicker, system_execution::execute_systems_for_all_space_views, @@ -39,22 +39,7 @@ pub struct ViewportState { /// State for the "Add entity" modal. space_view_entity_modal: SpaceViewEntityPicker, - /// State for the "Add space view or container" modal. - add_space_view_container_modal: AddSpaceViewOrContainerModal, - space_view_states: HashMap, - - /// Current candidate parent container for the ongoing drop. - /// - /// See [`ViewportState::is_candidate_drop_parent_container`] for details. - candidate_drop_parent_container_id: Option, - - /// The item that should be focused on in the blueprint tree. - /// - /// Set at each frame by [`Viewport::tree_ui`]. This is similar to - /// [`ViewerContext::focused_item`] but account for how specifically the blueprint tree should - /// handle the focused item. - pub(crate) blueprint_tree_scroll_to_item: Option, } static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); @@ -81,14 +66,6 @@ impl ViewportState { .get(&space_view_id) .map_or(&DEFAULT_PROPS, |state| &state.auto_properties) } - - /// Is the provided container the current candidate parent container for the ongoing drag? - /// - /// When a drag is in progress, the candidate parent container for the dragged item should be highlighted. Note that - /// this can happen when hovering said container, its direct children, or even the item just after it. - pub fn is_candidate_drop_parent_container(&self, container_id: &ContainerId) -> bool { - self.candidate_drop_parent_container_id.as_ref() == Some(container_id) - } } fn tree_simplification_options() -> egui_tiles::SimplificationOptions { @@ -172,20 +149,11 @@ impl<'a, 'b> Viewport<'a, 'b> { self.state.space_view_entity_modal.open(space_view_id); } - pub fn show_add_space_view_or_container_modal(&mut self, target_container: ContainerId) { - self.state - .add_space_view_container_modal - .open(target_container); - } - pub fn viewport_ui(&mut self, ui: &mut egui::Ui, ctx: &'a ViewerContext<'_>) { // run modals (these are noop if the modals are not active) self.state .space_view_entity_modal .ui(ui.ctx(), ctx, self.blueprint); - self.state - .add_space_view_container_modal - .ui(ui.ctx(), ctx, self.blueprint); let Viewport { blueprint, state, .. @@ -302,9 +270,6 @@ impl<'a, 'b> Viewport<'a, 'b> { let mut reset = false; - // always reset the drop target - self.state.candidate_drop_parent_container_id = None; - // TODO(#4687): Be extra careful here. If we mark edited inappropriately we can create an infinite edit loop. for tree_action in self.tree_action_receiver.try_iter() { re_log::trace!("Processing tree action: {tree_action:?}"); @@ -486,9 +451,6 @@ impl<'a, 'b> Viewport<'a, 'b> { self.tree_edited = true; } - TreeAction::SetDropTarget(container_id) => { - self.state.candidate_drop_parent_container_id = Some(container_id); - } } } diff --git a/crates/re_viewport_blueprint/Cargo.toml b/crates/re_viewport_blueprint/Cargo.toml index fc8d923a9420..6e784151fa80 100644 --- a/crates/re_viewport_blueprint/Cargo.toml +++ b/crates/re_viewport_blueprint/Cargo.toml @@ -27,6 +27,7 @@ re_tracing.workspace = true re_types.workspace = true re_types_blueprint.workspace = true re_types_core.workspace = true +re_ui.workspace = true re_viewer_context.workspace = true ahash.workspace = true @@ -34,5 +35,7 @@ egui_tiles.workspace = true egui.workspace = true itertools.workspace = true nohash-hasher.workspace = true +once_cell.workspace = true +parking_lot.workspace = true slotmap.workspace = true smallvec.workspace = true diff --git a/crates/re_viewport_blueprint/src/lib.rs b/crates/re_viewport_blueprint/src/lib.rs index 66ec113ae9b1..28f1e699c469 100644 --- a/crates/re_viewport_blueprint/src/lib.rs +++ b/crates/re_viewport_blueprint/src/lib.rs @@ -6,6 +6,7 @@ mod container; mod space_view; mod space_view_contents; mod tree_actions; +pub mod ui; mod view_properties; mod viewport_blueprint; diff --git a/crates/re_viewport_blueprint/src/tree_actions.rs b/crates/re_viewport_blueprint/src/tree_actions.rs index db71f6bec05a..6e9ab96f4ed7 100644 --- a/crates/re_viewport_blueprint/src/tree_actions.rs +++ b/crates/re_viewport_blueprint/src/tree_actions.rs @@ -39,10 +39,4 @@ pub enum TreeAction { target_container: ContainerId, target_position_in_container: usize, }, - - /// Set the container that is currently identified as the drop target of an ongoing drag. - /// - /// This is used for highlighting the drop target in the UI. Note that the drop target container is reset at every - /// frame, so this command must be re-sent every frame as long as a drop target is identified. - SetDropTarget(ContainerId), } diff --git a/crates/re_viewport/src/add_space_view_or_container_modal.rs b/crates/re_viewport_blueprint/src/ui/add_space_view_or_container_modal.rs similarity index 97% rename from crates/re_viewport/src/add_space_view_or_container_modal.rs rename to crates/re_viewport_blueprint/src/ui/add_space_view_or_container_modal.rs index 68d30458d3f1..112eb1eb8550 100644 --- a/crates/re_viewport/src/add_space_view_or_container_modal.rs +++ b/crates/re_viewport_blueprint/src/ui/add_space_view_or_container_modal.rs @@ -1,12 +1,11 @@ //! Modal for adding a new space view of container to an existing target container. +use crate::{SpaceViewBlueprint, ViewportBlueprint}; use re_ui::ReUi; use re_viewer_context::{ - blueprint_id_to_tile_id, ContainerId, RecommendedSpaceView, ViewerContext, + blueprint_id_to_tile_id, icon_for_container_kind, ContainerId, RecommendedSpaceView, + ViewerContext, }; -use re_viewport_blueprint::{SpaceViewBlueprint, ViewportBlueprint}; - -use crate::icon_for_container_kind; #[derive(Default)] pub struct AddSpaceViewOrContainerModal { diff --git a/crates/re_viewport_blueprint/src/ui/mod.rs b/crates/re_viewport_blueprint/src/ui/mod.rs new file mode 100644 index 000000000000..a7e55e7976cc --- /dev/null +++ b/crates/re_viewport_blueprint/src/ui/mod.rs @@ -0,0 +1,33 @@ +//! UI utilities related to the viewport blueprint. +//! +//! Current this is mainly the add space view or container modal. + +use parking_lot::Mutex; + +use re_viewer_context::{ContainerId, ViewerContext}; + +use crate::ViewportBlueprint; +mod add_space_view_or_container_modal; + +use add_space_view_or_container_modal::AddSpaceViewOrContainerModal; + +static ADD_SPACE_VIEW_OR_CONTAINER_MODAL: once_cell::sync::Lazy< + Mutex, +> = once_cell::sync::Lazy::new(|| Mutex::new(AddSpaceViewOrContainerModal::default())); + +pub fn add_space_view_or_container_modal_ui( + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + ui: &egui::Ui, +) { + // give a chance to the modal to be drawn + ADD_SPACE_VIEW_OR_CONTAINER_MODAL + .lock() + .ui(ui.ctx(), ctx, blueprint); +} + +pub fn show_add_space_view_or_container_modal(target_container: ContainerId) { + ADD_SPACE_VIEW_OR_CONTAINER_MODAL + .lock() + .open(target_container); +} diff --git a/crates/re_viewport_blueprint/src/viewport_blueprint.rs b/crates/re_viewport_blueprint/src/viewport_blueprint.rs index f24361de8767..a17274ae98ba 100644 --- a/crates/re_viewport_blueprint/src/viewport_blueprint.rs +++ b/crates/re_viewport_blueprint/src/viewport_blueprint.rs @@ -673,14 +673,6 @@ impl ViewportBlueprint { self.send_tree_action(TreeAction::MakeAllChildrenSameSize(*container_id)); } - /// Set the container that is currently identified as the drop target of an ongoing drag. - /// - /// This is used for highlighting the drop target in the UI. Note that the drop target container is reset at every - /// frame, so this command must be re-sent every frame as long as a drop target is identified. - pub fn set_drop_target(&self, container_id: &ContainerId) { - self.send_tree_action(TreeAction::SetDropTarget(*container_id)); - } - /// Check the visibility of the provided content. /// /// This function may be called from UI code. From b0b49aaff96f9909ae7ccdda0968ae6aea144b90 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Mon, 27 May 2024 12:18:43 +0200 Subject: [PATCH 09/10] Move selection panel UI to new `re_selection_panel` crate (#6431) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What ☝🏻 Chained to #6427 I made the following calls (along with @Wumpf): - Moved the `SpaceViewStates` to `re_view_context` (along with the space view class stuff). - Very partially renamed "space view" -> "view" for the stuff that I moved around. - Attempted to bring consistency to argument order in selection panel (ViewerContext -> ViewportBlueprint [-> ViewStates] -> egui::Ui -> other args). ### 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/6431?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/6431?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/6431) - [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`. --- ARCHITECTURE.md | 11 +- Cargo.lock | 35 +- Cargo.toml | 1 + crates/re_selection_panel/Cargo.toml | 44 ++ crates/re_selection_panel/README.md | 10 + crates/re_selection_panel/src/lib.rs | 10 + .../src}/override_ui.rs | 0 .../src}/query_range_ui.rs | 0 .../src}/selection_history_ui.rs | 5 +- .../src}/selection_panel.rs | 645 +++++++++--------- .../src/space_view_entity_picker.rs | 3 +- .../src}/space_view_space_origin_ui.rs | 18 +- crates/re_viewer/Cargo.toml | 5 +- crates/re_viewer/src/app_state.rs | 33 +- crates/re_viewer/src/lib.rs | 5 +- crates/re_viewer/src/ui/mod.rs | 5 - crates/re_viewer_context/src/lib.rs | 15 +- .../re_viewer_context/src/space_view/mod.rs | 2 + .../src/space_view/view_states.rs | 53 ++ crates/re_viewport/Cargo.toml | 2 - crates/re_viewport/src/lib.rs | 4 +- .../re_viewport/src/space_view_heuristics.rs | 22 - crates/re_viewport/src/viewport.rs | 100 +-- crates/re_viewport_blueprint/src/lib.rs | 20 + 24 files changed, 565 insertions(+), 483 deletions(-) create mode 100644 crates/re_selection_panel/Cargo.toml create mode 100644 crates/re_selection_panel/README.md create mode 100644 crates/re_selection_panel/src/lib.rs rename crates/{re_viewer/src/ui => re_selection_panel/src}/override_ui.rs (100%) rename crates/{re_viewer/src/ui => re_selection_panel/src}/query_range_ui.rs (100%) rename crates/{re_viewer/src/ui => re_selection_panel/src}/selection_history_ui.rs (98%) rename crates/{re_viewer/src/ui => re_selection_panel/src}/selection_panel.rs (79%) rename crates/{re_viewport => re_selection_panel}/src/space_view_entity_picker.rs (99%) rename crates/{re_viewer/src/ui => re_selection_panel/src}/space_view_space_origin_ui.rs (94%) create mode 100644 crates/re_viewer_context/src/space_view/view_states.rs delete mode 100644 crates/re_viewport/src/space_view_heuristics.rs diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6be030512871..8c364bda6cf3 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -88,11 +88,11 @@ Of course, this will only take us so far. In the future we plan on caching queri Here is an overview of the crates included in the project: - - - - - + + + + + @@ -131,6 +131,7 @@ Update instructions: | re_viewer | The Rerun Viewer | | re_viewport | The central viewport panel of the Rerun viewer. | | re_time_panel | The time panel of the Rerun Viewer, allowing to control the displayed timeline & time. | +| re_selection_panel | The UI for the selection panel. | | re_space_view | Types & utilities for defining Space View classes and communicating with the Viewport. | | re_space_view_bar_chart | A Space View that shows a single bar chart. | | re_space_view_dataframe | A Space View that shows the data contained in entities in a table. | diff --git a/Cargo.lock b/Cargo.lock index 8328078fed7e..f009ea39d439 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4668,6 +4668,34 @@ dependencies = [ "thiserror", ] +[[package]] +name = "re_selection_panel" +version = "0.17.0-alpha.2" +dependencies = [ + "egui", + "egui_tiles", + "itertools 0.12.0", + "nohash-hasher", + "once_cell", + "re_context_menu", + "re_data_store", + "re_data_ui", + "re_entity_db", + "re_log", + "re_log_types", + "re_renderer", + "re_space_view_spatial", + "re_space_view_time_series", + "re_tracing", + "re_types", + "re_types_core", + "re_ui", + "re_viewer_context", + "re_viewport_blueprint", + "serde", + "static_assertions", +] + [[package]] name = "re_smart_channel" version = "0.17.0-alpha.2" @@ -5036,18 +5064,15 @@ dependencies = [ "egui", "egui-wgpu", "egui_plot", - "egui_tiles", "ehttp", "image", "itertools 0.12.0", "js-sys", - "once_cell", "poll-promise", "re_analytics", "re_blueprint_tree", "re_build_info", "re_build_tools", - "re_context_menu", "re_data_loader", "re_data_source", "re_data_store", @@ -5062,6 +5087,7 @@ dependencies = [ "re_query", "re_renderer", "re_sdk_comms", + "re_selection_panel", "re_smart_channel", "re_space_view_bar_chart", "re_space_view_dataframe", @@ -5084,7 +5110,6 @@ dependencies = [ "ron", "serde", "serde_json", - "static_assertions", "thiserror", "time", "wasm-bindgen", @@ -5146,10 +5171,8 @@ dependencies = [ "image", "itertools 0.12.0", "nohash-hasher", - "once_cell", "rayon", "re_context_menu", - "re_data_ui", "re_entity_db", "re_log", "re_log_types", diff --git a/Cargo.toml b/Cargo.toml index 7c1cbdd8641d..400ec03eac0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ re_log_types = { path = "crates/re_log_types", version = "=0.17.0-alpha.2", defa re_memory = { path = "crates/re_memory", version = "=0.17.0-alpha.2", default-features = false } re_query = { path = "crates/re_query", version = "=0.17.0-alpha.2", default-features = false } re_renderer = { path = "crates/re_renderer", version = "=0.17.0-alpha.2", default-features = false } +re_selection_panel = { path = "crates/re_selection_panel", version = "=0.17.0-alpha.2", default-features = false } re_sdk = { path = "crates/re_sdk", version = "=0.17.0-alpha.2", default-features = false } re_sdk_comms = { path = "crates/re_sdk_comms", version = "=0.17.0-alpha.2", default-features = false } re_smart_channel = { path = "crates/re_smart_channel", version = "=0.17.0-alpha.2", default-features = false } diff --git a/crates/re_selection_panel/Cargo.toml b/crates/re_selection_panel/Cargo.toml new file mode 100644 index 000000000000..203a1717fa06 --- /dev/null +++ b/crates/re_selection_panel/Cargo.toml @@ -0,0 +1,44 @@ +[package] +authors.workspace = true +description = "The UI for the selection panel." +edition.workspace = true +homepage.workspace = true +license.workspace = true +name = "re_selection_panel" +publish = true +readme = "README.md" +repository.workspace = true +rust-version.workspace = true +version.workspace = true +include = ["../../LICENSE-APACHE", "../../LICENSE-MIT", "**/*.rs", "Cargo.toml"] + +[lints] +workspace = true + +[package.metadata.docs.rs] +all-features = true + +[dependencies] +re_context_menu.workspace = true +re_data_store.workspace = true +re_data_ui.workspace = true +re_entity_db = { workspace = true, features = ["serde"] } +re_log.workspace = true +re_log_types.workspace = true +re_renderer.workspace = true +re_space_view_time_series.workspace = true +re_space_view_spatial.workspace = true +re_tracing.workspace = true +re_types_core.workspace = true +re_types.workspace = true +re_ui.workspace = true +re_viewer_context.workspace = true +re_viewport_blueprint.workspace = true + +egui_tiles.workspace = true +egui.workspace = true +itertools.workspace = true +once_cell.workspace = true +nohash-hasher.workspace = true +serde = { workspace = true, features = ["derive"] } +static_assertions.workspace = true diff --git a/crates/re_selection_panel/README.md b/crates/re_selection_panel/README.md new file mode 100644 index 000000000000..8ea22245eeea --- /dev/null +++ b/crates/re_selection_panel/README.md @@ -0,0 +1,10 @@ +# re_selection_panel + +Part of the [`rerun`](https://github.com/rerun-io/rerun) family of crates. + +[![Latest version](https://img.shields.io/crates/v/re_selection_panel.svg)](https://crates.io/crates/re_selection_panel?speculative-link) +[![Documentation](https://docs.rs/re_selection_panel/badge.svg)](https://docs.rs/re_selection_panel?speculative-link) +![MIT](https://img.shields.io/badge/license-MIT-blue.svg) +![Apache](https://img.shields.io/badge/license-Apache-blue.svg) + +The UI for the selection panel. diff --git a/crates/re_selection_panel/src/lib.rs b/crates/re_selection_panel/src/lib.rs new file mode 100644 index 000000000000..9b5ff318510a --- /dev/null +++ b/crates/re_selection_panel/src/lib.rs @@ -0,0 +1,10 @@ +//! The UI for the selection panel. + +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; + +pub use selection_panel::SelectionPanel; diff --git a/crates/re_viewer/src/ui/override_ui.rs b/crates/re_selection_panel/src/override_ui.rs similarity index 100% rename from crates/re_viewer/src/ui/override_ui.rs rename to crates/re_selection_panel/src/override_ui.rs diff --git a/crates/re_viewer/src/ui/query_range_ui.rs b/crates/re_selection_panel/src/query_range_ui.rs similarity index 100% rename from crates/re_viewer/src/ui/query_range_ui.rs rename to crates/re_selection_panel/src/query_range_ui.rs diff --git a/crates/re_viewer/src/ui/selection_history_ui.rs b/crates/re_selection_panel/src/selection_history_ui.rs similarity index 98% rename from crates/re_viewer/src/ui/selection_history_ui.rs rename to crates/re_selection_panel/src/selection_history_ui.rs index f4264f3505d4..14c35eae0c99 100644 --- a/crates/re_viewer/src/ui/selection_history_ui.rs +++ b/crates/re_selection_panel/src/selection_history_ui.rs @@ -1,4 +1,5 @@ use egui::RichText; + use re_ui::UICommand; use re_viewer_context::{Item, ItemCollection, SelectionHistory}; use re_viewport_blueprint::ViewportBlueprint; @@ -143,7 +144,9 @@ impl SelectionHistoryUi { } } if sel.iter_items().count() == 1 { - item_kind_ui(ui, sel.iter_items().next().unwrap()); + if let Some(item) = sel.iter_items().next() { + item_kind_ui(ui, item); + } } }); } diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_selection_panel/src/selection_panel.rs similarity index 79% rename from crates/re_viewer/src/ui/selection_panel.rs rename to crates/re_selection_panel/src/selection_panel.rs index 5327477da7a5..602991000a6b 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_selection_panel/src/selection_panel.rs @@ -20,34 +20,40 @@ use re_ui::{icons, list_item, ReUi, SyntaxHighlighting as _}; use re_viewer_context::{ contents_name_style, gpu_bridge::colormap_dropdown_button_ui, icon_for_container_kind, ContainerId, Contents, DataQueryResult, HoverHighlight, Item, SpaceViewClass, SpaceViewId, - UiLayout, ViewerContext, + UiLayout, ViewStates, ViewerContext, }; -use re_viewport::Viewport; use re_viewport_blueprint::{ui::show_add_space_view_or_container_modal, ViewportBlueprint}; -use crate::ui::override_ui::override_visualizer_ui; -use crate::{app_state::default_selection_panel_width, ui::override_ui::override_ui}; - -use super::{ +use crate::override_ui::{override_ui, override_visualizer_ui}; +use crate::space_view_entity_picker::SpaceViewEntityPicker; +use crate::{ query_range_ui::query_range_ui_data_result, query_range_ui::query_range_ui_space_view, selection_history_ui::SelectionHistoryUi, }; // --- +fn default_selection_panel_width(screen_width: f32) -> f32 { + (0.45 * screen_width).min(300.0).round() +} /// The "Selection view" sidebar. #[derive(Default, serde::Deserialize, serde::Serialize)] #[serde(default)] -pub(crate) struct SelectionPanel { +pub struct SelectionPanel { selection_state_ui: SelectionHistoryUi, + + #[serde(skip)] + /// State for the "Add entity" modal. + space_view_entity_modal: SpaceViewEntityPicker, } impl SelectionPanel { pub fn show_panel( &mut self, ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + view_states: &mut ViewStates, ui: &mut egui::Ui, - viewport: &mut Viewport<'_, '_>, expanded: bool, ) { let screen_width = ui.ctx().screen_rect().width(); @@ -76,7 +82,7 @@ impl SelectionPanel { if let Some(selection) = self.selection_state_ui.selection_ui( ctx.re_ui, ui, - viewport.blueprint, + blueprint, &mut history, ) { ctx.selection_state().set_selection(selection); @@ -93,19 +99,23 @@ impl SelectionPanel { .show(ui, |ui| { ui.add_space(ui.spacing().item_spacing.y); ctx.re_ui.panel_content(ui, |_, ui| { - self.contents(ctx, ui, viewport); + self.contents(ctx, blueprint, view_states, ui); }); }); }); }); + + // run modals (these are noop if the modals are not active) + self.space_view_entity_modal.ui(ui.ctx(), ctx, blueprint); } #[allow(clippy::unused_self)] fn contents( &mut self, ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + view_states: &mut ViewStates, ui: &mut egui::Ui, - viewport: &mut Viewport<'_, '_>, ) { re_tracing::profile_function!(); @@ -124,17 +134,17 @@ impl SelectionPanel { }; for (i, item) in selection.iter_items().enumerate() { ui.push_id(item, |ui| { - what_is_selected_ui(ui, ctx, viewport.blueprint, item); + what_is_selected_ui(ctx, blueprint, ui, item); match item { Item::Container(container_id) => { - container_top_level_properties(ui, ctx, viewport, container_id); + container_top_level_properties(ctx, blueprint, ui, container_id); ui.add_space(12.0); - container_children(ui, ctx, viewport, container_id); + container_children(ctx, blueprint, ui, container_id); } Item::SpaceView(space_view_id) => { - space_view_top_level_properties(ui, ctx, viewport.blueprint, space_view_id); + space_view_top_level_properties(ctx, blueprint, ui, space_view_id); } _ => {} @@ -153,7 +163,7 @@ impl SelectionPanel { // Special override section for space-view-entities if let Item::DataResult(space_view_id, instance_path) = item { - if let Some(space_view) = viewport.blueprint.space_views.get(space_view_id) { + if let Some(space_view) = blueprint.space_views.get(space_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. @@ -179,7 +189,7 @@ impl SelectionPanel { if has_blueprint_section(item) { ctx.re_ui .large_collapsing_header(ui, "Blueprint", true, |ui| { - blueprint_ui(ui, ctx, viewport, item); + self.blueprint_ui(ctx, blueprint, view_states, ui, item); }); } @@ -190,15 +200,287 @@ impl SelectionPanel { }); } } + + /// What is the blueprint stuff for this item? + fn blueprint_ui( + &mut self, + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + view_states: &mut ViewStates, + ui: &mut egui::Ui, + item: &Item, + ) { + match item { + Item::AppId(_) + | Item::DataSource(_) + | Item::StoreId(_) + | Item::ComponentPath(_) + | Item::Container(_) + | Item::InstancePath(_) => {} + + Item::SpaceView(space_view_id) => { + self.blueprint_ui_for_space_view(ctx, blueprint, view_states, ui, *space_view_id); + } + + Item::DataResult(space_view_id, instance_path) => { + blueprint_ui_for_data_result(ctx, blueprint, ui, *space_view_id, instance_path); + } + } + } + + fn blueprint_ui_for_space_view( + &mut self, + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + view_states: &mut ViewStates, + ui: &mut Ui, + space_view_id: SpaceViewId, + ) { + if let Some(space_view) = blueprint.space_view(&space_view_id) { + if let Some(new_entity_path_filter) = self.entity_path_filter_ui( + ctx, + ui, + space_view_id, + &space_view.contents.entity_path_filter, + &space_view.space_origin, + ) { + space_view + .contents + .set_entity_path_filter(ctx, &new_entity_path_filter); + } + + ui.add_space(ui.spacing().item_spacing.y); + } + + if ui + .button("Clone space view") + .on_hover_text( + "Create an exact duplicate of this space view including all blueprint settings", + ) + .clicked() + { + if let Some(new_space_view_id) = blueprint.duplicate_space_view(&space_view_id, ctx) { + ctx.selection_state() + .set_selection(Item::SpaceView(new_space_view_id)); + blueprint.mark_user_interaction(ctx); + } + } + + ui.add_space(ui.spacing().item_spacing.y / 2.0); + ReUi::full_span_separator(ui); + ui.add_space(ui.spacing().item_spacing.y / 2.0); + + if let Some(space_view) = blueprint.space_view(&space_view_id) { + let class_identifier = *space_view.class_identifier(); + + let space_view_state = view_states.view_state_mut( + ctx.space_view_class_registry, + space_view.id, + &class_identifier, + ); + + query_range_ui_space_view(ctx, ui, space_view); + + // Space View don't inherit (legacy) properties. + let mut props = + space_view.legacy_properties(ctx.store_context.blueprint, ctx.blueprint_query); + let props_before = props.clone(); + + let space_view_class = space_view.class(ctx.space_view_class_registry); + if let Err(err) = space_view_class.selection_ui( + ctx, + ui, + space_view_state.view_state.as_mut(), + &space_view.space_origin, + space_view.id, + &mut props, + ) { + re_log::error!( + "Error in space view selection UI (class: {}, display name: {}): {err}", + space_view.class_identifier(), + space_view_class.display_name(), + ); + } + + if props_before != props { + space_view.save_legacy_properties(ctx, props); + } + } + } + + /// Returns a new filter when the editing is done, and there has been a change. + fn entity_path_filter_ui( + &mut self, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + space_view_id: SpaceViewId, + filter: &EntityPathFilter, + origin: &EntityPath, + ) -> Option { + fn entity_path_filter_help_ui(ui: &mut egui::Ui) { + let markdown = r#" +# Entity path query syntax + +Entity path queries are described as a list of include/exclude rules that act on paths: + +```diff ++ /world/** # add everything… +- /world/roads/** # …but remove all roads… ++ /world/roads/main # …but show main road +``` + +If there are multiple matching rules, the most specific rule wins. +If there are multiple rules of the same specificity, the last one wins. +If no rules match, the path is excluded. + +The `/**` suffix matches the whole subtree, i.e. self and any child, recursively +(`/world/**` matches both `/world` and `/world/car/driver`). +Other uses of `*` are not (yet) supported. + +`EntityPathFilter` sorts the rule by entity path, with recursive coming before non-recursive. +This means the last matching rule is also the most specific one. +For instance: + +```diff ++ /world/** +- /world +- /world/car/** ++ /world/car/driver +``` + +The last rule matching `/world/car/driver` is `+ /world/car/driver`, so it is included. +The last rule matching `/world/car/hood` is `- /world/car/**`, so it is excluded. +The last rule matching `/world` is `- /world`, so it is excluded. +The last rule matching `/world/house` is `+ /world/**`, so it is included. + "# + .trim(); + + re_ui::markdown_ui(ui, egui::Id::new("entity_path_filter_help_ui"), markdown); + } + + fn syntax_highlight_entity_path_filter( + style: &egui::Style, + mut string: &str, + ) -> egui::text::LayoutJob { + let font_id = egui::TextStyle::Body.resolve(style); + + let mut job = egui::text::LayoutJob::default(); + + while !string.is_empty() { + let newline = string.find('\n').unwrap_or(string.len() - 1); + let line = &string[..=newline]; + string = &string[newline + 1..]; + let is_exclusion = line.trim_start().starts_with('-'); + + let color = if is_exclusion { + egui::Color32::LIGHT_RED + } else { + egui::Color32::LIGHT_GREEN + }; + + let text_format = egui::TextFormat { + font_id: font_id.clone(), + color, + ..Default::default() + }; + + job.append(line, 0.0, text_format); + } + + job + } + + fn text_layouter( + ui: &egui::Ui, + string: &str, + wrap_width: f32, + ) -> std::sync::Arc { + let mut layout_job = syntax_highlight_entity_path_filter(ui.style(), string); + layout_job.wrap.max_width = wrap_width; + ui.fonts(|f| f.layout_job(layout_job)) + } + + // We store the string we are temporarily editing in the `Ui`'s temporary data storage. + // This is so it can contain invalid rules while the user edits it, and it's only normalized + // when they press enter, or stops editing. + let filter_text_id = ui.id().with("filter_text"); + + let mut filter_string = ui.data_mut(|data| { + data.get_temp_mut_or_insert_with::(filter_text_id, || filter.formatted()) + .clone() + }); + + let rightmost_x = ui.cursor().min.x; + ui.horizontal(|ui| { + ui.label("Entity path query").on_hover_text( + "The entity path query consists of a list of include/exclude rules \ + that determines what entities are part of this space view", + ); + + let current_x = ui.cursor().min.x; + // Compute a width that results in these things to be right-aligned with the following text edit. + let desired_width = (ui.available_width() - ui.spacing().item_spacing.x) + .at_most(ui.spacing().text_edit_width - (current_x - rightmost_x)); + + ui.allocate_ui_with_layout( + egui::vec2(desired_width, ui.available_height()), + egui::Layout::right_to_left(egui::Align::Center), + |ui| { + re_ui::help_hover_button(ui).on_hover_ui(entity_path_filter_help_ui); + if ui + .button("Edit") + .on_hover_text("Modify the entity query using the editor") + .clicked() + { + self.space_view_entity_modal.open(space_view_id); + } + }, + ); + }); + + let response = + ui.add(egui::TextEdit::multiline(&mut filter_string).layouter(&mut text_layouter)); + + if response.has_focus() { + ui.data_mut(|data| data.insert_temp::(filter_text_id, filter_string.clone())); + } else { + // Reconstruct it from the filter next frame + ui.data_mut(|data| data.remove::(filter_text_id)); + } + + // Show some statistics about the query, print a warning text if something seems off. + let query = ctx.lookup_query_result(space_view_id); + if query.num_matching_entities == 0 { + ui.label(ctx.re_ui.warning_text("Does not match any entity")); + } else if query.num_matching_entities == 1 { + ui.label("Matches 1 entity"); + } else { + ui.label(format!("Matches {} entities", query.num_matching_entities)); + } + if query.num_matching_entities != 0 && query.num_visualized_entities == 0 { + // TODO(andreas): Talk about this root bit only if it's a spatial view. + ui.label(ctx.re_ui.warning_text( + format!("This space view is not able to visualize any of the matched entities using the current root \"{origin:?}\"."), + )); + } + + // Apply the edit. + let new_filter = EntityPathFilter::parse_forgiving(&filter_string, &Default::default()); + if &new_filter == filter { + None // no change + } else { + Some(new_filter) + } + } } fn container_children( - ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - viewport: &Viewport<'_, '_>, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, container_id: &ContainerId, ) { - let Some(container) = viewport.blueprint.container(container_id) else { + let Some(container) = blueprint.container(container_id) else { return; }; @@ -215,7 +497,7 @@ fn container_children( let show_content = |ui: &mut egui::Ui| { let mut has_child = false; for child_contents in &container.contents { - has_child |= show_list_item_for_container_child(ui, ctx, viewport, child_contents); + has_child |= show_list_item_for_container_child(ctx, blueprint, ui, child_contents); } if !has_child { @@ -291,9 +573,9 @@ fn space_view_button( /// /// This includes a title bar and contextual information about there this item is located. fn what_is_selected_ui( - ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - viewport: &ViewportBlueprint, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, item: &Item, ) { match item { @@ -347,7 +629,7 @@ fn what_is_selected_ui( } Item::Container(container_id) => { - if let Some(container_blueprint) = viewport.container(container_id) { + if let Some(container_blueprint) = blueprint.container(container_id) { let hover_text = if let Some(display_name) = container_blueprint.display_name.as_ref() { format!( @@ -405,11 +687,11 @@ fn what_is_selected_ui( item_ui::entity_path_button(ctx, &query, db, ui, None, entity_path); }); - list_existing_data_blueprints(ui, ctx, &entity_path.clone().into(), viewport); + list_existing_data_blueprints(ctx, blueprint, ui, &entity_path.clone().into()); } Item::SpaceView(space_view_id) => { - if let Some(space_view) = viewport.space_view(space_view_id) { + if let Some(space_view) = blueprint.space_view(space_view_id) { let space_view_class = space_view.class(ctx.space_view_class_registry); let hover_text = if let Some(display_name) = space_view.display_name.as_ref() { @@ -467,13 +749,13 @@ fn what_is_selected_ui( } } - list_existing_data_blueprints(ui, ctx, instance_path, viewport); + list_existing_data_blueprints(ctx, blueprint, ui, instance_path); } Item::DataResult(space_view_id, instance_path) => { let name = instance_path.syntax_highlighted(ui.style()); - if let Some(space_view) = viewport.space_view(space_view_id) { + if let Some(space_view) = blueprint.space_view(space_view_id) { let typ = item.kind(); item_title_ui( ctx.re_ui, @@ -553,10 +835,10 @@ fn item_title_ui( /// Display a list of all the space views an entity appears in. fn list_existing_data_blueprints( - ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - instance_path: &InstancePath, blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, + instance_path: &InstancePath, ) { let space_views_with_path = blueprint.space_views_containing_entity_path(ctx, &instance_path.entity_path); @@ -592,9 +874,9 @@ fn list_existing_data_blueprints( /// out as needing to be edited in most case when creating a new space view, which is why they are /// shown at the very top. fn space_view_top_level_properties( - ui: &mut egui::Ui, ctx: &ViewerContext<'_>, viewport: &ViewportBlueprint, + ui: &mut egui::Ui, space_view_id: &SpaceViewId, ) { if let Some(space_view) = viewport.space_view(space_view_id) { @@ -637,12 +919,12 @@ fn space_view_top_level_properties( } fn container_top_level_properties( - ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - viewport: &Viewport<'_, '_>, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, container_id: &ContainerId, ) { - let Some(container) = viewport.blueprint.container(container_id) else { + let Some(container) = blueprint.container(container_id) else { return; }; @@ -663,9 +945,7 @@ fn container_top_level_properties( let mut container_kind = container.container_kind; container_kind_selection_ui(ctx, ui, &mut container_kind); - viewport - .blueprint - .set_container_kind(*container_id, container_kind); + blueprint.set_container_kind(*container_id, container_kind); ui.end_row(); @@ -710,7 +990,7 @@ fn container_top_level_properties( .on_hover_text("Simplify this container and its children") .clicked() { - viewport.blueprint.simplify_container( + blueprint.simplify_container( container_id, egui_tiles::SimplificationOptions { prune_empty_tabs: true, @@ -748,7 +1028,7 @@ fn container_top_level_properties( .on_hover_text("Make all children the same size") .clicked() { - viewport.blueprint.make_all_children_same_size(container_id); + blueprint.make_all_children_same_size(container_id); } ui.end_row(); }); @@ -793,15 +1073,15 @@ fn container_kind_selection_ui( /// /// Return true if successful. fn show_list_item_for_container_child( - ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - viewport: &Viewport<'_, '_>, + blueprint: &ViewportBlueprint, + ui: &mut egui::Ui, child_contents: &Contents, ) -> bool { let mut remove_contents = false; let (item, list_item_content) = match child_contents { Contents::SpaceView(space_view_id) => { - let Some(space_view) = viewport.blueprint.space_view(space_view_id) else { + let Some(space_view) = blueprint.space_view(space_view_id) else { re_log::warn_once!("Could not find space view with ID {space_view_id:?}",); return false; }; @@ -826,7 +1106,7 @@ fn show_list_item_for_container_child( ) } Contents::Container(container_id) => { - let Some(container) = viewport.blueprint.container(container_id) else { + let Some(container) = blueprint.container(container_id) else { re_log::warn_once!("Could not find container with ID {container_id:?}",); return false; }; @@ -862,7 +1142,7 @@ fn show_list_item_for_container_child( context_menu_ui_for_item( ctx, - viewport.blueprint, + blueprint, &item, &response, SelectionUpdateBehavior::Ignore, @@ -870,8 +1150,8 @@ fn show_list_item_for_container_child( ctx.select_hovered_on_click(&response, item); if remove_contents { - viewport.blueprint.mark_user_interaction(ctx); - viewport.blueprint.remove_contents(*child_contents); + blueprint.mark_user_interaction(ctx); + blueprint.remove_contents(*child_contents); } true @@ -890,120 +1170,14 @@ fn has_blueprint_section(item: &Item) -> bool { } } -/// What is the blueprint stuff for this item? -fn blueprint_ui( - ui: &mut egui::Ui, - ctx: &ViewerContext<'_>, - viewport: &mut Viewport<'_, '_>, - item: &Item, -) { - match item { - Item::AppId(_) - | Item::DataSource(_) - | Item::StoreId(_) - | Item::ComponentPath(_) - | Item::Container(_) - | Item::InstancePath(_) => {} - - Item::SpaceView(space_view_id) => { - blueprint_ui_for_space_view(ui, ctx, viewport, *space_view_id); - } - - Item::DataResult(space_view_id, instance_path) => { - blueprint_ui_for_data_result(ui, ctx, viewport, *space_view_id, instance_path); - } - } -} - -fn blueprint_ui_for_space_view( - ui: &mut Ui, - ctx: &ViewerContext<'_>, - viewport: &mut Viewport<'_, '_>, - space_view_id: SpaceViewId, -) { - if let Some(space_view) = viewport.blueprint.space_view(&space_view_id) { - if let Some(new_entity_path_filter) = entity_path_filter_ui( - ui, - ctx, - viewport, - space_view_id, - &space_view.contents.entity_path_filter, - &space_view.space_origin, - ) { - space_view - .contents - .set_entity_path_filter(ctx, &new_entity_path_filter); - } - - ui.add_space(ui.spacing().item_spacing.y); - } - - if ui - .button("Clone space view") - .on_hover_text( - "Create an exact duplicate of this space view including all blueprint settings", - ) - .clicked() - { - if let Some(new_space_view_id) = - viewport.blueprint.duplicate_space_view(&space_view_id, ctx) - { - ctx.selection_state() - .set_selection(Item::SpaceView(new_space_view_id)); - viewport.blueprint.mark_user_interaction(ctx); - } - } - - ui.add_space(ui.spacing().item_spacing.y / 2.0); - ReUi::full_span_separator(ui); - ui.add_space(ui.spacing().item_spacing.y / 2.0); - - if let Some(space_view) = viewport.blueprint.space_view(&space_view_id) { - let class_identifier = *space_view.class_identifier(); - - let space_view_state = viewport.state.space_view_state_mut( - ctx.space_view_class_registry, - space_view.id, - &class_identifier, - ); - - query_range_ui_space_view(ctx, ui, space_view); - - // Space View don't inherit (legacy) properties. - let mut props = - space_view.legacy_properties(ctx.store_context.blueprint, ctx.blueprint_query); - let props_before = props.clone(); - - let space_view_class = space_view.class(ctx.space_view_class_registry); - if let Err(err) = space_view_class.selection_ui( - ctx, - ui, - space_view_state.space_view_state.as_mut(), - &space_view.space_origin, - space_view.id, - &mut props, - ) { - re_log::error!( - "Error in space view selection UI (class: {}, display name: {}): {err}", - space_view.class_identifier(), - space_view_class.display_name(), - ); - } - - if props_before != props { - space_view.save_legacy_properties(ctx, props); - } - } -} - fn blueprint_ui_for_data_result( - ui: &mut Ui, ctx: &ViewerContext<'_>, - viewport: &Viewport<'_, '_>, + blueprint: &ViewportBlueprint, + ui: &mut Ui, space_view_id: SpaceViewId, instance_path: &InstancePath, ) { - if let Some(space_view) = viewport.blueprint.space_view(&space_view_id) { + if let Some(space_view) = blueprint.space_view(&space_view_id) { if instance_path.instance.is_all() { // the whole entity let entity_path = &instance_path.entity_path; @@ -1033,167 +1207,6 @@ fn blueprint_ui_for_data_result( } } -/// Returns a new filter when the editing is done, and there has been a change. -fn entity_path_filter_ui( - ui: &mut egui::Ui, - ctx: &ViewerContext<'_>, - viewport: &mut Viewport<'_, '_>, - space_view_id: SpaceViewId, - filter: &EntityPathFilter, - origin: &EntityPath, -) -> Option { - fn entity_path_filter_help_ui(ui: &mut egui::Ui) { - let markdown = r#" -# Entity path query syntax - -Entity path queries are described as a list of include/exclude rules that act on paths: - -```diff -+ /world/** # add everything… -- /world/roads/** # …but remove all roads… -+ /world/roads/main # …but show main road -``` - -If there are multiple matching rules, the most specific rule wins. -If there are multiple rules of the same specificity, the last one wins. -If no rules match, the path is excluded. - -The `/**` suffix matches the whole subtree, i.e. self and any child, recursively -(`/world/**` matches both `/world` and `/world/car/driver`). -Other uses of `*` are not (yet) supported. - -`EntityPathFilter` sorts the rule by entity path, with recursive coming before non-recursive. -This means the last matching rule is also the most specific one. -For instance: - -```diff -+ /world/** -- /world -- /world/car/** -+ /world/car/driver -``` - -The last rule matching `/world/car/driver` is `+ /world/car/driver`, so it is included. -The last rule matching `/world/car/hood` is `- /world/car/**`, so it is excluded. -The last rule matching `/world` is `- /world`, so it is excluded. -The last rule matching `/world/house` is `+ /world/**`, so it is included. - "# - .trim(); - - re_ui::markdown_ui(ui, egui::Id::new("entity_path_filter_help_ui"), markdown); - } - - fn syntax_highlight_entity_path_filter( - style: &egui::Style, - mut string: &str, - ) -> egui::text::LayoutJob { - let font_id = egui::TextStyle::Body.resolve(style); - - let mut job = egui::text::LayoutJob::default(); - - while !string.is_empty() { - let newline = string.find('\n').unwrap_or(string.len() - 1); - let line = &string[..=newline]; - string = &string[newline + 1..]; - let is_exclusion = line.trim_start().starts_with('-'); - - let color = if is_exclusion { - egui::Color32::LIGHT_RED - } else { - egui::Color32::LIGHT_GREEN - }; - - let text_format = egui::TextFormat { - font_id: font_id.clone(), - color, - ..Default::default() - }; - - job.append(line, 0.0, text_format); - } - - job - } - - fn text_layouter(ui: &egui::Ui, string: &str, wrap_width: f32) -> std::sync::Arc { - let mut layout_job = syntax_highlight_entity_path_filter(ui.style(), string); - layout_job.wrap.max_width = wrap_width; - ui.fonts(|f| f.layout_job(layout_job)) - } - - // We store the string we are temporarily editing in the `Ui`'s temporary data storage. - // This is so it can contain invalid rules while the user edits it, and it's only normalized - // when they press enter, or stops editing. - let filter_text_id = ui.id().with("filter_text"); - - let mut filter_string = ui.data_mut(|data| { - data.get_temp_mut_or_insert_with::(filter_text_id, || filter.formatted()) - .clone() - }); - - let rightmost_x = ui.cursor().min.x; - ui.horizontal(|ui| { - ui.label("Entity path query").on_hover_text( - "The entity path query consists of a list of include/exclude rules \ - that determines what entities are part of this space view", - ); - - let current_x = ui.cursor().min.x; - // Compute a width that results in these things to be right-aligned with the following text edit. - let desired_width = (ui.available_width() - ui.spacing().item_spacing.x) - .at_most(ui.spacing().text_edit_width - (current_x - rightmost_x)); - - ui.allocate_ui_with_layout( - egui::vec2(desired_width, ui.available_height()), - egui::Layout::right_to_left(egui::Align::Center), - |ui| { - re_ui::help_hover_button(ui).on_hover_ui(entity_path_filter_help_ui); - if ui - .button("Edit") - .on_hover_text("Modify the entity query using the editor") - .clicked() - { - viewport.show_add_remove_entities_modal(space_view_id); - } - }, - ); - }); - - let response = - ui.add(egui::TextEdit::multiline(&mut filter_string).layouter(&mut text_layouter)); - - if response.has_focus() { - ui.data_mut(|data| data.insert_temp::(filter_text_id, filter_string.clone())); - } else { - // Reconstruct it from the filter next frame - ui.data_mut(|data| data.remove::(filter_text_id)); - } - - // Show some statistics about the query, print a warning text if something seems off. - let query = ctx.lookup_query_result(space_view_id); - if query.num_matching_entities == 0 { - ui.label(ctx.re_ui.warning_text("Does not match any entity")); - } else if query.num_matching_entities == 1 { - ui.label("Matches 1 entity"); - } else { - ui.label(format!("Matches {} entities", query.num_matching_entities)); - } - if query.num_matching_entities != 0 && query.num_visualized_entities == 0 { - // TODO(andreas): Talk about this root bit only if it's a spatial view. - ui.label(ctx.re_ui.warning_text( - format!("This space view is not able to visualize any of the matched entities using the current root \"{origin:?}\"."), - )); - } - - // Apply the edit. - let new_filter = EntityPathFilter::parse_forgiving(&filter_string, &Default::default()); - if &new_filter == filter { - None // no change - } else { - Some(new_filter) - } -} - fn entity_props_ui( ctx: &ViewerContext<'_>, ui: &mut egui::Ui, diff --git a/crates/re_viewport/src/space_view_entity_picker.rs b/crates/re_selection_panel/src/space_view_entity_picker.rs similarity index 99% rename from crates/re_viewport/src/space_view_entity_picker.rs rename to crates/re_selection_panel/src/space_view_entity_picker.rs index 2d86da761a84..dc1183b86e75 100644 --- a/crates/re_viewport/src/space_view_entity_picker.rs +++ b/crates/re_selection_panel/src/space_view_entity_picker.rs @@ -11,7 +11,7 @@ use re_viewport_blueprint::{SpaceViewBlueprint, ViewportBlueprint}; /// /// Delegates to [`re_ui::modal::ModalHandler`] #[derive(Default)] -pub struct SpaceViewEntityPicker { +pub(crate) struct SpaceViewEntityPicker { space_view_id: Option, modal_handler: re_ui::modal::ModalHandler, } @@ -156,6 +156,7 @@ fn add_entities_line_ui( ui.horizontal(|ui| { let entity_path = &entity_tree.path; + #[allow(clippy::unwrap_used)] let add_info = entities_add_info.get(entity_path).unwrap(); let is_explicitly_excluded = entity_path_filter.is_explicitly_excluded(entity_path); diff --git a/crates/re_viewer/src/ui/space_view_space_origin_ui.rs b/crates/re_selection_panel/src/space_view_space_origin_ui.rs similarity index 94% rename from crates/re_viewer/src/ui/space_view_space_origin_ui.rs rename to crates/re_selection_panel/src/space_view_space_origin_ui.rs index 6830c3e83199..27d19c46b0ed 100644 --- a/crates/re_viewer/src/ui/space_view_space_origin_ui.rs +++ b/crates/re_selection_panel/src/space_view_space_origin_ui.rs @@ -1,12 +1,11 @@ use std::ops::ControlFlow; -use eframe::emath::NumExt; -use egui::{Key, Ui}; +use egui::{Key, NumExt as _, Ui}; use re_log_types::EntityPath; use re_ui::{list_item, ReUi, SyntaxHighlighting}; use re_viewer_context::ViewerContext; -use re_viewport_blueprint::SpaceViewBlueprint; +use re_viewport_blueprint::{default_created_space_views, SpaceViewBlueprint}; /// State of the space origin widget. #[derive(Default, Clone)] @@ -92,13 +91,12 @@ fn space_view_space_origin_widget_editing_ui( // All suggestions for this class of space views. // TODO(#4895): we should have/use a much simpler heuristic API to get a list of compatible entity sub-tree - let space_view_suggestions = - re_viewport::space_view_heuristics::default_created_space_views(ctx) - .into_iter() - .filter(|this_space_view| { - this_space_view.class_identifier() == space_view.class_identifier() - }) - .collect::>(); + let space_view_suggestions = default_created_space_views(ctx) + .into_iter() + .filter(|this_space_view| { + this_space_view.class_identifier() == space_view.class_identifier() + }) + .collect::>(); // Filtered suggestions based on the current text edit content. let filtered_space_view_suggestions = space_view_suggestions diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index 56b341432d91..d9c82e49ead5 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -38,7 +38,6 @@ analytics = ["dep:re_analytics"] [dependencies] # Internal: -re_context_menu.workspace = true re_build_info.workspace = true re_blueprint_tree.workspace = true re_data_loader.workspace = true @@ -58,6 +57,7 @@ re_log_types.workspace = true re_memory.workspace = true re_query.workspace = true re_renderer = { workspace = true, default-features = false } +re_selection_panel.workspace = true re_sdk_comms.workspace = true re_smart_channel.workspace = true re_space_view_bar_chart.workspace = true @@ -96,17 +96,14 @@ eframe = { workspace = true, default-features = false, features = [ egui_plot.workspace = true egui-wgpu.workspace = true egui.workspace = true -egui_tiles.workspace = true ehttp.workspace = true image = { workspace = true, default-features = false, features = ["png"] } itertools = { workspace = true } -once_cell = { workspace = true } poll-promise = { workspace = true, features = ["web"] } rfd.workspace = true ron.workspace = true serde = { workspace = true, features = ["derive"] } serde_json.workspace = true -static_assertions.workspace = true thiserror.workspace = true time = { workspace = true, features = ["formatting"] } web-time.workspace = true diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 6775f4a05deb..83d7e921da82 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -8,9 +8,10 @@ use re_types::blueprint::components::PanelState; use re_viewer_context::{ blueprint_timeline, AppOptions, ApplicationSelectionState, Caches, CommandSender, ComponentUiRegistry, PlayState, RecordingConfig, SpaceViewClassExt as _, - SpaceViewClassRegistry, StoreContext, StoreHub, SystemCommandSender as _, ViewerContext, + SpaceViewClassRegistry, StoreContext, StoreHub, SystemCommandSender as _, ViewStates, + ViewerContext, }; -use re_viewport::{Viewport, ViewportState}; +use re_viewport::Viewport; use re_viewport_blueprint::ui::add_space_view_or_container_modal_ui; use re_viewport_blueprint::ViewportBlueprint; @@ -33,7 +34,7 @@ pub struct AppState { recording_configs: HashMap, blueprint_cfg: RecordingConfig, - selection_panel: crate::selection_panel::SelectionPanel, + selection_panel: re_selection_panel::SelectionPanel, time_panel: re_time_panel::TimePanel, blueprint_panel: re_time_panel::TimePanel, #[serde(skip)] @@ -42,10 +43,12 @@ pub struct AppState { #[serde(skip)] welcome_screen: crate::ui::WelcomeScreen, - // TODO(jleibs): This is sort of a weird place to put this but makes more - // sense than the blueprint + /// Storage for the state of each `SpaceView` + /// + /// This is stored here for simplicity. An exclusive reference for that is passed to the users, + /// such as [`Viewport`] and [`re_selection_panel::SelectionPanel`]. #[serde(skip)] - viewport_state: ViewportState, + view_states: ViewStates, /// Selection & hovering state. pub selection_state: ApplicationSelectionState, @@ -70,7 +73,7 @@ impl Default for AppState { blueprint_panel: re_time_panel::TimePanel::new_blueprint_panel(), blueprint_tree: Default::default(), welcome_screen: Default::default(), - viewport_state: Default::default(), + view_states: Default::default(), selection_state: Default::default(), focused_item: Default::default(), } @@ -143,7 +146,7 @@ impl AppState { blueprint_panel, blueprint_tree, welcome_screen, - viewport_state, + view_states, selection_state, focused_item, } = self; @@ -160,7 +163,6 @@ impl AppState { ); let mut viewport = Viewport::new( &viewport_blueprint, - viewport_state, space_view_class_registry, receiver, sender, @@ -257,7 +259,7 @@ impl AppState { // First update the viewport and thus all active space views. // This may update their heuristics, so that all panels that are shown in this frame, // have the latest information. - viewport.on_frame_start(&ctx); + viewport.on_frame_start(&ctx, view_states); { re_tracing::profile_scope!("updated_query_results"); @@ -289,7 +291,7 @@ impl AppState { &blueprint_query, rec_cfg.time_ctrl.read().timeline(), space_view_class_registry, - viewport.state.legacy_auto_properties(space_view.id), + view_states.legacy_auto_properties(space_view.id), query_result, ); } @@ -351,8 +353,9 @@ impl AppState { selection_panel.show_panel( &ctx, + &viewport_blueprint, + view_states, ui, - &mut viewport, app_blueprint.selection_panel_state.is_expanded(), ); @@ -429,7 +432,7 @@ impl AppState { if show_welcome { welcome_screen.ui(ui, re_ui, command_sender, welcome_screen_state); } else { - viewport.viewport_ui(ui, &ctx); + viewport.viewport_ui(ui, &ctx, view_states); } }); @@ -592,7 +595,3 @@ fn check_for_clicked_hyperlinks( pub fn default_blueprint_panel_width(screen_width: f32) -> f32 { (0.35 * screen_width).min(200.0).round() } - -pub fn default_selection_panel_width(screen_width: f32) -> f32 { - (0.45 * screen_width).min(300.0).round() -} diff --git a/crates/re_viewer/src/lib.rs b/crates/re_viewer/src/lib.rs index f5519a34176c..3017d5489ccb 100644 --- a/crates/re_viewer/src/lib.rs +++ b/crates/re_viewer/src/lib.rs @@ -25,10 +25,7 @@ mod viewer_analytics; /// Unstable. Used for the ongoing blueprint experimentations. pub mod blueprint; -pub(crate) use { - app_state::AppState, - ui::{memory_panel, selection_panel}, -}; +pub(crate) use {app_state::AppState, ui::memory_panel}; pub use app::{App, StartupOptions}; diff --git a/crates/re_viewer/src/ui/mod.rs b/crates/re_viewer/src/ui/mod.rs index 802406d84163..8a4f36bde88c 100644 --- a/crates/re_viewer/src/ui/mod.rs +++ b/crates/re_viewer/src/ui/mod.rs @@ -1,15 +1,10 @@ mod mobile_warning_ui; -mod override_ui; mod recordings_panel; mod rerun_menu; -mod selection_history_ui; mod top_panel; mod welcome_screen; pub(crate) mod memory_panel; -pub(crate) mod query_range_ui; -pub(crate) mod selection_panel; -pub(crate) mod space_view_space_origin_ui; pub use recordings_panel::recordings_panel_ui; // ---- diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index 67dca7747eee..4dad50df7018 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -51,13 +51,14 @@ pub use selection_state::{ }; pub use space_view::{ DataResult, IdentifiedViewSystem, OverridePath, PerSystemDataResults, PerSystemEntities, - PropertyOverrides, RecommendedSpaceView, SmallVisualizerSet, SpaceViewClass, SpaceViewClassExt, - SpaceViewClassLayoutPriority, SpaceViewClassRegistry, SpaceViewClassRegistryError, - SpaceViewEntityHighlight, SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewSpawnHeuristics, - SpaceViewState, SpaceViewStateExt, SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, - SystemExecutionOutput, ViewContextCollection, ViewContextSystem, ViewQuery, - ViewSystemIdentifier, VisualizableFilterContext, VisualizerAdditionalApplicabilityFilter, - VisualizerCollection, VisualizerQueryInfo, VisualizerSystem, + PerViewState, PropertyOverrides, RecommendedSpaceView, SmallVisualizerSet, SpaceViewClass, + SpaceViewClassExt, SpaceViewClassLayoutPriority, SpaceViewClassRegistry, + SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights, + SpaceViewOutlineMasks, SpaceViewSpawnHeuristics, SpaceViewState, SpaceViewStateExt, + SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, SystemExecutionOutput, + ViewContextCollection, ViewContextSystem, ViewQuery, ViewStates, ViewSystemIdentifier, + VisualizableFilterContext, VisualizerAdditionalApplicabilityFilter, VisualizerCollection, + VisualizerQueryInfo, VisualizerSystem, }; pub use store_context::StoreContext; pub use store_hub::StoreHub; diff --git a/crates/re_viewer_context/src/space_view/mod.rs b/crates/re_viewer_context/src/space_view/mod.rs index df04e0e9522a..31003ddf662e 100644 --- a/crates/re_viewer_context/src/space_view/mod.rs +++ b/crates/re_viewer_context/src/space_view/mod.rs @@ -13,6 +13,7 @@ mod spawn_heuristics; mod system_execution_output; mod view_context_system; mod view_query; +mod view_states; mod visualizer_entity_subscriber; mod visualizer_system; @@ -32,6 +33,7 @@ pub use view_query::{ DataResult, OverridePath, PerSystemDataResults, PropertyOverrides, SmallVisualizerSet, ViewQuery, }; +pub use view_states::{PerViewState, ViewStates}; pub use visualizer_entity_subscriber::VisualizerAdditionalApplicabilityFilter; pub use visualizer_system::{VisualizerCollection, VisualizerQueryInfo, VisualizerSystem}; diff --git a/crates/re_viewer_context/src/space_view/view_states.rs b/crates/re_viewer_context/src/space_view/view_states.rs new file mode 100644 index 000000000000..3fa1add2d17e --- /dev/null +++ b/crates/re_viewer_context/src/space_view/view_states.rs @@ -0,0 +1,53 @@ +//! Storage for the state of each `SpaceView`. +//! +//! The `Viewer` has ownership of this state and pass it around to users (mainly viewport and +//! selection panel). + +use ahash::HashMap; +use once_cell::sync::Lazy; + +use re_entity_db::EntityPropertyMap; +use re_log_types::external::re_types_core::SpaceViewClassIdentifier; + +use crate::{SpaceViewClassRegistry, SpaceViewId, SpaceViewState}; + +// State for each `SpaceView` including both the auto properties and +// the internal state of the space view itself. +pub struct PerViewState { + pub auto_properties: EntityPropertyMap, + pub view_state: Box, +} + +// ---------------------------------------------------------------------------- +/// State for the `SpaceView`s that persists across frames but otherwise +/// is not saved. +#[derive(Default)] +pub struct ViewStates { + states: HashMap, +} + +static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); + +impl ViewStates { + pub fn view_state_mut( + &mut self, + space_view_class_registry: &SpaceViewClassRegistry, + space_view_id: SpaceViewId, + space_view_class: &SpaceViewClassIdentifier, + ) -> &mut PerViewState { + self.states + .entry(space_view_id) + .or_insert_with(|| PerViewState { + auto_properties: Default::default(), + view_state: space_view_class_registry + .get_class_or_log_error(space_view_class) + .new_state(), + }) + } + + pub fn legacy_auto_properties(&self, space_view_id: SpaceViewId) -> &EntityPropertyMap { + self.states + .get(&space_view_id) + .map_or(&DEFAULT_PROPS, |state| &state.auto_properties) + } +} diff --git a/crates/re_viewport/Cargo.toml b/crates/re_viewport/Cargo.toml index d862760ebe52..5c1cc575ef14 100644 --- a/crates/re_viewport/Cargo.toml +++ b/crates/re_viewport/Cargo.toml @@ -20,7 +20,6 @@ all-features = true [dependencies] re_context_menu.workspace = true -re_data_ui.workspace = true re_entity_db = { workspace = true, features = ["serde"] } re_log_types.workspace = true re_log.workspace = true @@ -45,5 +44,4 @@ glam.workspace = true image = { workspace = true, default-features = false, features = ["png"] } itertools.workspace = true nohash-hasher.workspace = true -once_cell.workspace = true rayon.workspace = true diff --git a/crates/re_viewport/src/lib.rs b/crates/re_viewport/src/lib.rs index 1ef82670ae1a..2f96ed237402 100644 --- a/crates/re_viewport/src/lib.rs +++ b/crates/re_viewport/src/lib.rs @@ -7,13 +7,11 @@ mod auto_layout; mod screenshot; -mod space_view_entity_picker; -pub mod space_view_heuristics; mod space_view_highlights; mod system_execution; mod viewport; -pub use self::viewport::{Viewport, ViewportState}; +pub use self::viewport::Viewport; pub mod external { pub use re_space_view; diff --git a/crates/re_viewport/src/space_view_heuristics.rs b/crates/re_viewport/src/space_view_heuristics.rs deleted file mode 100644 index 5185143e0ce9..000000000000 --- a/crates/re_viewport/src/space_view_heuristics.rs +++ /dev/null @@ -1,22 +0,0 @@ -use re_viewer_context::ViewerContext; - -use re_viewport_blueprint::SpaceViewBlueprint; - -/// List out all space views we generate by default for the available data. -/// -/// TODO(andreas): This is transitional. We want to pass on the space view spawn heuristics -/// directly and make more high level decisions with it. -pub fn default_created_space_views(ctx: &ViewerContext<'_>) -> Vec { - re_tracing::profile_function!(); - - ctx.space_view_class_registry - .iter_registry() - .flat_map(|entry| { - let spawn_heuristics = entry.class.spawn_heuristics(ctx); - spawn_heuristics - .into_vec() - .into_iter() - .map(|recommendation| SpaceViewBlueprint::new(entry.identifier, recommendation)) - }) - .collect() -} diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index 27c2f256a316..ae760e465caa 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -4,70 +4,22 @@ use ahash::HashMap; use egui_tiles::{Behavior as _, EditAction}; -use once_cell::sync::Lazy; use re_context_menu::{context_menu_ui_for_item, SelectionUpdateBehavior}; -use re_entity_db::EntityPropertyMap; use re_renderer::ScreenshotProcessor; -use re_types::SpaceViewClassIdentifier; use re_ui::{Icon, ReUi}; use re_viewer_context::{ - blueprint_id_to_tile_id, icon_for_container_kind, ContainerId, Contents, Item, - SpaceViewClassRegistry, SpaceViewId, SpaceViewState, SystemExecutionOutput, ViewQuery, + blueprint_id_to_tile_id, icon_for_container_kind, ContainerId, Contents, Item, PerViewState, + SpaceViewClassRegistry, SpaceViewId, SystemExecutionOutput, ViewQuery, ViewStates, ViewerContext, }; use re_viewport_blueprint::{TreeAction, ViewportBlueprint}; use crate::{ screenshot::handle_pending_space_view_screenshots, - space_view_entity_picker::SpaceViewEntityPicker, system_execution::execute_systems_for_all_space_views, }; -// State for each `SpaceView` including both the auto properties and -// the internal state of the space view itself. -pub struct PerSpaceViewState { - pub auto_properties: EntityPropertyMap, - pub space_view_state: Box, -} - -// ---------------------------------------------------------------------------- -/// State for the [`Viewport`] that persists across frames but otherwise -/// is not saved. -#[derive(Default)] -pub struct ViewportState { - /// State for the "Add entity" modal. - space_view_entity_modal: SpaceViewEntityPicker, - - space_view_states: HashMap, -} - -static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); - -impl ViewportState { - pub fn space_view_state_mut( - &mut self, - space_view_class_registry: &SpaceViewClassRegistry, - space_view_id: SpaceViewId, - space_view_class: &SpaceViewClassIdentifier, - ) -> &mut PerSpaceViewState { - self.space_view_states - .entry(space_view_id) - .or_insert_with(|| PerSpaceViewState { - auto_properties: Default::default(), - space_view_state: space_view_class_registry - .get_class_or_log_error(space_view_class) - .new_state(), - }) - } - - pub fn legacy_auto_properties(&self, space_view_id: SpaceViewId) -> &EntityPropertyMap { - self.space_view_states - .get(&space_view_id) - .map_or(&DEFAULT_PROPS, |state| &state.auto_properties) - } -} - fn tree_simplification_options() -> egui_tiles::SimplificationOptions { egui_tiles::SimplificationOptions { prune_empty_tabs: false, @@ -82,15 +34,11 @@ fn tree_simplification_options() -> egui_tiles::SimplificationOptions { // ---------------------------------------------------------------------------- /// Defines the layout of the Viewport -pub struct Viewport<'a, 'b> { +pub struct Viewport<'a> { /// The blueprint that drives this viewport. This is the source of truth from the store /// for this frame. pub blueprint: &'a ViewportBlueprint, - /// The persistent state of the viewport that is not saved to the store but otherwise - /// persis frame-to-frame. - pub state: &'b mut ViewportState, - /// The [`egui_tiles::Tree`] tree that actually manages blueprint layout. This tree needs /// to be mutable for things like drag-and-drop and is ultimately saved back to the store. /// at the end of the frame if edited. @@ -112,10 +60,9 @@ pub struct Viewport<'a, 'b> { tree_action_sender: std::sync::mpsc::Sender, } -impl<'a, 'b> Viewport<'a, 'b> { +impl<'a> Viewport<'a> { pub fn new( blueprint: &'a ViewportBlueprint, - state: &'b mut ViewportState, space_view_class_registry: &SpaceViewClassRegistry, tree_action_receiver: std::sync::mpsc::Receiver, tree_action_sender: std::sync::mpsc::Sender, @@ -137,7 +84,6 @@ impl<'a, 'b> Viewport<'a, 'b> { Self { blueprint, - state, tree, tree_edited: edited, tree_action_receiver, @@ -145,19 +91,13 @@ impl<'a, 'b> Viewport<'a, 'b> { } } - pub fn show_add_remove_entities_modal(&mut self, space_view_id: SpaceViewId) { - self.state.space_view_entity_modal.open(space_view_id); - } - - pub fn viewport_ui(&mut self, ui: &mut egui::Ui, ctx: &'a ViewerContext<'_>) { - // run modals (these are noop if the modals are not active) - self.state - .space_view_entity_modal - .ui(ui.ctx(), ctx, self.blueprint); - - let Viewport { - blueprint, state, .. - } = self; + pub fn viewport_ui( + &mut self, + ui: &mut egui::Ui, + ctx: &'a ViewerContext<'_>, + view_states: &mut ViewStates, + ) { + let Viewport { blueprint, .. } = self; let is_zero_sized_viewport = ui.available_size().min_elem() <= 0.0; if is_zero_sized_viewport { @@ -201,7 +141,7 @@ impl<'a, 'b> Viewport<'a, 'b> { re_tracing::profile_scope!("tree.ui"); let mut tab_viewer = TabViewer { - viewport_state: state, + view_states, ctx, viewport_blueprint: blueprint, maximized: &mut maximized, @@ -234,14 +174,14 @@ impl<'a, 'b> Viewport<'a, 'b> { self.blueprint.set_maximized(maximized, ctx); } - pub fn on_frame_start(&mut self, ctx: &ViewerContext<'_>) { + pub fn on_frame_start(&mut self, ctx: &ViewerContext<'_>, view_states: &mut ViewStates) { re_tracing::profile_function!(); for space_view in self.blueprint.space_views.values() { - let PerSpaceViewState { + let PerViewState { auto_properties, - space_view_state, - } = self.state.space_view_state_mut( + view_state: space_view_state, + } = view_states.view_state_mut( ctx.space_view_class_registry, space_view.id, space_view.class_identifier(), @@ -495,7 +435,7 @@ impl<'a, 'b> Viewport<'a, 'b> { /// In our case, each pane is a space view, /// while containers are just groups of things. struct TabViewer<'a, 'b> { - viewport_state: &'a mut ViewportState, + view_states: &'a mut ViewStates, ctx: &'a ViewerContext<'b>, viewport_blueprint: &'a ViewportBlueprint, maximized: &'a mut Option, @@ -563,10 +503,10 @@ impl<'a, 'b> egui_tiles::Behavior for TabViewer<'a, 'b> { ) }); - let PerSpaceViewState { + let PerViewState { auto_properties: _, - space_view_state, - } = self.viewport_state.space_view_state_mut( + view_state: space_view_state, + } = self.view_states.view_state_mut( self.ctx.space_view_class_registry, space_view_blueprint.id, space_view_blueprint.class_identifier(), diff --git a/crates/re_viewport_blueprint/src/lib.rs b/crates/re_viewport_blueprint/src/lib.rs index 28f1e699c469..e29a9d0d9f01 100644 --- a/crates/re_viewport_blueprint/src/lib.rs +++ b/crates/re_viewport_blueprint/src/lib.rs @@ -11,6 +11,7 @@ mod view_properties; mod viewport_blueprint; pub use container::ContainerBlueprint; +use re_viewer_context::ViewerContext; pub use space_view::SpaceViewBlueprint; pub use space_view_contents::SpaceViewContents; pub use tree_actions::TreeAction; @@ -55,3 +56,22 @@ pub fn container_kind_from_egui( egui_tiles::ContainerKind::Grid => ContainerKind::Grid, } } + +/// List out all space views we generate by default for the available data. +/// +/// TODO(andreas): This is transitional. We want to pass on the space view spawn heuristics +/// directly and make more high level decisions with it. +pub fn default_created_space_views(ctx: &ViewerContext<'_>) -> Vec { + re_tracing::profile_function!(); + + ctx.space_view_class_registry + .iter_registry() + .flat_map(|entry| { + let spawn_heuristics = entry.class.spawn_heuristics(ctx); + spawn_heuristics + .into_vec() + .into_iter() + .map(|recommendation| SpaceViewBlueprint::new(entry.identifier, recommendation)) + }) + .collect() +} From 30ec825711beb1410f768bf6330d050e300d638b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Proch=C3=A1zka?= Date: Mon, 27 May 2024 13:14:34 +0200 Subject: [PATCH 10/10] Allow hiding top panel via blueprint (#6409) ### What - `rrb.TopPanel` is now exposed, works similarly to `rrb.TimePanel` - `TimePanel` hidden state now fully hides the time panel ### 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/6409?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/6409?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/6409) - [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`. --------- Co-authored-by: Andreas Reich --- crates/re_time_panel/src/lib.rs | 4 ++ crates/re_viewer/src/app.rs | 19 +++--- crates/re_viewer/src/ui/top_panel.rs | 65 +++++++++++-------- .../rerun_sdk/rerun/blueprint/__init__.py | 1 + rerun_py/rerun_sdk/rerun/blueprint/api.py | 22 ++++++- 5 files changed, 72 insertions(+), 39 deletions(-) diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 5ea499cf3edb..b1d77a79f32f 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -147,6 +147,10 @@ impl TimePanel { ui: &mut egui::Ui, state: PanelState, ) { + if state == PanelState::Hidden { + return; + } + // Naturally, many parts of the time panel need the time control. // Copy it once, read/edit, and then write back at the end if there was a change. let time_ctrl_before = rec_cfg.time_ctrl.read().clone(); diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 4a4720d969b6..680a53eed624 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -4,7 +4,6 @@ use re_entity_db::entity_db::EntityDb; use re_log_types::{ApplicationId, FileSource, LogMsg, StoreKind}; use re_renderer::WgpuResourcePoolStatistics; use re_smart_channel::{ReceiveSet, SmartChannelSource}; -use re_types::blueprint::components::PanelState; use re_ui::{toasts, UICommand, UICommandSender}; use re_viewer_context::{ command_channel, @@ -884,16 +883,14 @@ impl App { crate::ui::mobile_warning_ui(&self.re_ui, ui); - if app_blueprint.top_panel_state != PanelState::Hidden { - crate::ui::top_panel( - frame, - self, - app_blueprint, - store_context, - gpu_resource_stats, - ui, - ); - } + crate::ui::top_panel( + frame, + self, + app_blueprint, + store_context, + gpu_resource_stats, + ui, + ); self.memory_panel_ui(ui, gpu_resource_stats, store_stats); diff --git a/crates/re_viewer/src/ui/top_panel.rs b/crates/re_viewer/src/ui/top_panel.rs index 06e7a7a591cd..f118501ffd77 100644 --- a/crates/re_viewer/src/ui/top_panel.rs +++ b/crates/re_viewer/src/ui/top_panel.rs @@ -20,34 +20,33 @@ pub fn top_panel( let style_like_web = app.is_screenshotting(); let top_bar_style = app.re_ui().top_bar_style(style_like_web); - - egui::TopBottomPanel::top("top_bar") - .frame(app.re_ui().top_panel_frame()) - .exact_height(top_bar_style.height) - .show_inside(ui, |ui| { - // React to dragging and double-clicking the top bar: - #[cfg(not(target_arch = "wasm32"))] - if !re_ui::NATIVE_WINDOW_BAR { - // Interact with background first, so that buttons in the top bar gets input priority - // (last added widget has priority for input). - let title_bar_response = ui.interact( - ui.max_rect(), - ui.id().with("background"), - egui::Sense::click(), - ); - if title_bar_response.double_clicked() { - let maximized = ui.input(|i| i.viewport().maximized.unwrap_or(false)); - ui.ctx() - .send_viewport_cmd(egui::ViewportCommand::Maximized(!maximized)); - } else if title_bar_response.is_pointer_button_down_on() { - ui.ctx().send_viewport_cmd(egui::ViewportCommand::StartDrag); - } + let top_panel_frame = app.re_ui().top_panel_frame(); + + let mut content = |ui: &mut egui::Ui, show_content: bool| { + // React to dragging and double-clicking the top bar: + #[cfg(not(target_arch = "wasm32"))] + if !re_ui::NATIVE_WINDOW_BAR { + // Interact with background first, so that buttons in the top bar gets input priority + // (last added widget has priority for input). + let title_bar_response = ui.interact( + ui.max_rect(), + ui.id().with("background"), + egui::Sense::click(), + ); + if title_bar_response.double_clicked() { + let maximized = ui.input(|i| i.viewport().maximized.unwrap_or(false)); + ui.ctx() + .send_viewport_cmd(egui::ViewportCommand::Maximized(!maximized)); + } else if title_bar_response.is_pointer_button_down_on() { + ui.ctx().send_viewport_cmd(egui::ViewportCommand::StartDrag); } + } - egui::menu::bar(ui, |ui| { - ui.set_height(top_bar_style.height); - ui.add_space(top_bar_style.indent); + egui::menu::bar(ui, |ui| { + ui.set_height(top_bar_style.height); + ui.add_space(top_bar_style.indent); + if show_content { top_bar_ui( frame, app, @@ -56,8 +55,22 @@ pub fn top_panel( ui, gpu_resource_stats, ); - }); + } }); + }; + + let panel = egui::TopBottomPanel::top("top_bar") + .frame(top_panel_frame) + .exact_height(top_bar_style.height); + let is_expanded = app_blueprint.top_panel_state.is_expanded(); + + // On MacOS, we show the close/minimize/maximize buttons in the top panel. + // We _always_ want to show the top panel in that case, and only hide its content. + if !re_ui::NATIVE_WINDOW_BAR { + panel.show_inside(ui, |ui| content(ui, is_expanded)); + } else { + panel.show_animated_inside(ui, is_expanded, |ui| content(ui, is_expanded)); + } } fn top_bar_ui( diff --git a/rerun_py/rerun_sdk/rerun/blueprint/__init__.py b/rerun_py/rerun_sdk/rerun/blueprint/__init__.py index a325c83ee7a2..0b0443378383 100644 --- a/rerun_py/rerun_sdk/rerun/blueprint/__init__.py +++ b/rerun_py/rerun_sdk/rerun/blueprint/__init__.py @@ -18,6 +18,7 @@ SelectionPanel, SpaceView, TimePanel, + TopPanel, ) from .archetypes import ( Background, diff --git a/rerun_py/rerun_sdk/rerun/blueprint/api.py b/rerun_py/rerun_sdk/rerun/blueprint/api.py index fc934e7423a3..eb557f1682a3 100644 --- a/rerun_py/rerun_sdk/rerun/blueprint/api.py +++ b/rerun_py/rerun_sdk/rerun/blueprint/api.py @@ -317,6 +317,8 @@ def __init__(self, *, expanded: bool | None = None, state: PanelStateLike | None state: Whether the panel is expanded, collapsed, or hidden. + Collapsed and hidden both fully hide the top panel. + """ super().__init__(blueprint_path="top_panel", expanded=expanded, state=state) @@ -335,6 +337,8 @@ def __init__(self, *, expanded: bool | None = None, state: PanelStateLike | None state: Whether the panel is expanded, collapsed, or hidden. + Collapsed and hidden both fully hide the blueprint panel. + """ super().__init__(blueprint_path="blueprint_panel", expanded=expanded, state=state) @@ -353,6 +357,8 @@ def __init__(self, *, expanded: bool | None = None, state: PanelStateLike | None state: Whether the panel is expanded, collapsed, or hidden. + Collapsed and hidden both fully hide the selection panel. + """ super().__init__(blueprint_path="selection_panel", expanded=expanded, state=state) @@ -371,6 +377,9 @@ def __init__(self, *, expanded: bool | None = None, state: PanelStateLike | None state: Whether the panel is expanded, collapsed, or hidden. + Expanded fully shows the panel, collapsed shows a simplified panel, + hidden fully hides the panel. + """ super().__init__(blueprint_path="time_panel", expanded=expanded, state=state) @@ -383,7 +392,7 @@ def __init__(self, *, expanded: bool | None = None, state: PanelStateLike | None helper classes. """ -BlueprintPart = Union[ContainerLike, BlueprintPanel, SelectionPanel, TimePanel] +BlueprintPart = Union[ContainerLike, TopPanel, BlueprintPanel, SelectionPanel, TimePanel] """ The types that make up a blueprint. """ @@ -430,7 +439,9 @@ def __init__( Defaults to `False` unless no Containers or SpaceViews are provided, in which case it defaults to `True`. If you want to create a completely empty Blueprint, you must explicitly set this to `False`. collapse_panels: - Whether to collapse the panels in the viewer. Defaults to `False`. + Whether to collapse panels in the viewer. Defaults to `False`. + + This fully hides the blueprint/selection panels, and shows the simplified time panel. """ from .containers import Tabs @@ -442,6 +453,10 @@ def __init__( for part in parts: if isinstance(part, (Container, SpaceView)): contents.append(part) + elif isinstance(part, TopPanel): + if hasattr(self, "top_panel"): + raise ValueError("Only one top panel can be provided") + self.top_panel = part elif isinstance(part, BlueprintPanel): if hasattr(self, "blueprint_panel"): raise ValueError("Only one blueprint panel can be provided") @@ -492,6 +507,9 @@ def _log_to_stream(self, stream: RecordingStream) -> None: stream.log("viewport", viewport_arch) # type: ignore[attr-defined] + if hasattr(self, "top_panel"): + self.top_panel._log_to_stream(stream) + if hasattr(self, "blueprint_panel"): self.blueprint_panel._log_to_stream(stream) elif self.collapse_panels: