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] 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)