From 9ad5a0d2e3857526708216208a0d908df18382b3 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Wed, 6 Dec 2023 13:26:04 +0100 Subject: [PATCH 1/4] Improve the Selection Panel UI for components when a single item is selected (#4416) ### What This PR: - adds a new UI verbosity level to distinguish between single- and multi-selection in the Selection Panel; - adjusts `DataUi` impls accordingly; - update the UI of for `AnnotationContext` to use collapsible headers instead of inner scroll areas (there can be *many* tables for one instance, so inner scroll bars are really annoying); - adds a script to log very long `AnnotationContext` for UI test purposes. * Is affected by (and doesn't fix): https://github.com/rerun-io/rerun/issues/4367 * Follow-up to #4370 * Fixes #4375 ### Screenshot New collapsible-header-based UI for annotation context: https://github.com/rerun-io/rerun/assets/49431240/435566a0-420b-48d7-8ea4-026d02d903a2 Also fix this spurious separator (and the related sizing issue) at the top of the hover box: image ### 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): * Full build: [app.rerun.io](https://app.rerun.io/pr/4416/index.html) * Partial build: [app.rerun.io](https://app.rerun.io/pr/4416/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) - Useful for quick testing when changes do not affect examples in any way * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/4416) - [Docs preview](https://rerun.io/preview/83f1bdcdcd055dd46658343a44f9ae7022c64f45/docs) - [Examples preview](https://rerun.io/preview/83f1bdcdcd055dd46658343a44f9ae7022c64f45/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --------- Co-authored-by: Emil Ernerfeldt --- clippy.toml | 12 +- crates/re_data_ui/src/annotation_context.rs | 229 ++++++++++-------- crates/re_data_ui/src/component.rs | 28 +-- .../re_data_ui/src/component_ui_registry.rs | 5 +- crates/re_data_ui/src/data.rs | 26 +- crates/re_data_ui/src/image.rs | 2 +- crates/re_data_ui/src/instance_path.rs | 15 +- crates/re_data_ui/src/lib.rs | 41 +++- crates/re_data_ui/src/transform3d.rs | 8 +- crates/re_ui/src/lib.rs | 27 +++ crates/re_viewer/src/ui/selection_panel.rs | 7 +- .../src/component_ui_registry.rs | 14 +- scripts/clippy_wasm/clippy.toml | 10 +- .../annotation_context_ui_stress.py | 33 +++ 14 files changed, 285 insertions(+), 172 deletions(-) create mode 100644 tests/python/annotation_context_ui_stress/annotation_context_ui_stress.py diff --git a/clippy.toml b/clippy.toml index 2278963d009b..0734059a8391 100644 --- a/clippy.toml +++ b/clippy.toml @@ -40,17 +40,15 @@ disallowed-macros = [ # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods disallowed-methods = [ - "std::env::temp_dir", # Use the tempdir crate instead + { path = "egui_extras::TableBody::row", reason = "`row` doesn't scale. Use `rows` instead." }, + { path = "sha1::Digest::new", reason = "SHA1 is cryptographically broken" }, + { path = "std::env::temp_dir", reason = "Use the tempdir crate instead" }, + { path = "std::panic::catch_unwind", reason = "We compile with `panic = 'abort'`" }, + { path = "std::thread::spawn", reason = "Use `std::thread::Builder` and name the thread" }, # There are many things that aren't allowed on wasm, # but we cannot disable them all here (because of e.g. https://github.com/rust-lang/rust-clippy/issues/10406) # so we do that in `clipppy_wasm.toml` instead. - - "std::thread::spawn", # Use `std::thread::Builder` and name the thread - - "sha1::Digest::new", # SHA1 is cryptographically broken - - "std::panic::catch_unwind", # We compile with `panic = "abort"` ] # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names diff --git a/crates/re_data_ui/src/annotation_context.rs b/crates/re_data_ui/src/annotation_context.rs index b088af08b6ec..4ccf79be1980 100644 --- a/crates/re_data_ui/src/annotation_context.rs +++ b/crates/re_data_ui/src/annotation_context.rs @@ -7,9 +7,7 @@ use re_types::datatypes::{ }; use re_viewer_context::{auto_color, UiVerbosity, ViewerContext}; -use super::DataUi; - -const TABLE_SCROLL_AREA_HEIGHT: f32 = 500.0; // add scroll-bars when we get to this height +use super::{table_for_verbosity, DataUi}; impl crate::EntityDataUi for re_types::components::ClassId { fn entity_data_ui( @@ -41,12 +39,13 @@ impl crate::EntityDataUi for re_types::components::ClassId { || !class.keypoint_annotations.is_empty() { response.response.on_hover_ui(|ui| { - class_description_ui(ui, class, id); + class_description_ui(ctx, ui, verbosity, class, id); }); } } - UiVerbosity::Reduced | UiVerbosity::All => { - class_description_ui(ui, class, id); + UiVerbosity::Reduced | UiVerbosity::Full | UiVerbosity::LimitHeight => { + ui.separator(); + class_description_ui(ctx, ui, verbosity, class, id); } } } else { @@ -97,7 +96,7 @@ fn annotation_info( impl DataUi for AnnotationContext { fn data_ui( &self, - _ctx: &ViewerContext<'_>, + ctx: &ViewerContext<'_>, ui: &mut egui::Ui, verbosity: UiVerbosity, _query: &re_arrow_store::LatestAtQuery, @@ -111,22 +110,25 @@ impl DataUi for AnnotationContext { ui.label(format!("AnnotationContext with {} classes", self.0.len())); } } - UiVerbosity::All => { + UiVerbosity::LimitHeight | UiVerbosity::Full => { ui.vertical(|ui| { - annotation_info_table_ui( - ui, - self.0 - .iter() - .map(|class| &class.class_description.info) - .sorted_by_key(|info| info.id), - ); + ctx.re_ui + .maybe_collapsing_header(ui, true, "Classes", true, |ui| { + let annotation_infos = self + .0 + .iter() + .map(|class| &class.class_description.info) + .sorted_by_key(|info| info.id) + .collect_vec(); + annotation_info_table_ui(ui, verbosity, &annotation_infos); + }); for ClassDescriptionMapElem { class_id, class_description, } in &self.0 { - class_description_ui(ui, class_description, *class_id); + class_description_ui(ctx, ui, verbosity, class_description, *class_id); } }); } @@ -135,7 +137,9 @@ impl DataUi for AnnotationContext { } fn class_description_ui( + ctx: &re_viewer_context::ViewerContext<'_>, ui: &mut egui::Ui, + mut verbosity: UiVerbosity, class: &ClassDescription, id: re_types::datatypes::ClassId, ) { @@ -143,97 +147,119 @@ fn class_description_ui( return; } + re_tracing::profile_function!(); + + let use_collapsible = verbosity == UiVerbosity::LimitHeight || verbosity == UiVerbosity::Full; + + // We use collapsible header as a means for the user to limit the height, so the annotation info + // tables can be fully unrolled. + if verbosity == UiVerbosity::LimitHeight { + verbosity = UiVerbosity::Full; + } + let row_height = re_ui::ReUi::table_line_height(); - ui.separator(); - ui.strong(format!("Keypoints for Class {}", id.0)); if !class.keypoint_annotations.is_empty() { - ui.add_space(8.0); - ui.strong("Keypoints Annotations"); - ui.push_id(format!("keypoint_annotations_{}", id.0), |ui| { - annotation_info_table_ui( - ui, - class + ctx.re_ui.maybe_collapsing_header( + ui, + use_collapsible, + &format!("Keypoints Annotation for Class {}", id.0), + true, + |ui| { + let annotation_infos = class .keypoint_annotations .iter() - .sorted_by_key(|annotation| annotation.id), - ); - }); + .sorted_by_key(|annotation| annotation.id) + .collect_vec(); + ui.push_id(format!("keypoint_annotations_{}", id.0), |ui| { + annotation_info_table_ui(ui, verbosity, &annotation_infos); + }); + }, + ); } if !class.keypoint_connections.is_empty() { - ui.add_space(8.0); - ui.strong("Keypoint Connections"); - ui.push_id(format!("keypoints_connections_{}", id.0), |ui| { - use egui_extras::{Column, TableBuilder}; + ctx.re_ui.maybe_collapsing_header( + ui, + use_collapsible, + &format!("Keypoint Connections for Class {}", id.0), + true, + |ui| { + ui.push_id(format!("keypoints_connections_{}", id.0), |ui| { + use egui_extras::Column; - let table = TableBuilder::new(ui) - .min_scrolled_height(TABLE_SCROLL_AREA_HEIGHT) - .max_scroll_height(TABLE_SCROLL_AREA_HEIGHT) - .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) - .column(Column::auto().clip(true).at_least(40.0)) - .column(Column::auto().clip(true).at_least(40.0)); - table - .header(re_ui::ReUi::table_header_height(), |mut header| { - re_ui::ReUi::setup_table_header(&mut header); - header.col(|ui| { - ui.strong("From"); - }); - header.col(|ui| { - ui.strong("To"); - }); - }) - .body(|mut body| { - re_ui::ReUi::setup_table_body(&mut body); + let table = table_for_verbosity(verbosity, ui) + .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) + .column(Column::auto().clip(true).at_least(40.0)) + .column(Column::auto().clip(true).at_least(40.0)); + table + .header(re_ui::ReUi::table_header_height(), |mut header| { + re_ui::ReUi::setup_table_header(&mut header); + header.col(|ui| { + ui.strong("From"); + }); + header.col(|ui| { + ui.strong("To"); + }); + }) + .body(|mut body| { + re_ui::ReUi::setup_table_body(&mut body); - // TODO(jleibs): Helper to do this with caching somewhere - let keypoint_map: ahash::HashMap = { - re_tracing::profile_scope!("build_annotation_map"); - class - .keypoint_annotations - .iter() - .map(|kp| (kp.id.into(), kp.clone())) - .collect() - }; + // TODO(jleibs): Helper to do this with caching somewhere + let keypoint_map: ahash::HashMap = { + re_tracing::profile_scope!("build_annotation_map"); + class + .keypoint_annotations + .iter() + .map(|kp| (kp.id.into(), kp.clone())) + .collect() + }; - for KeypointPair { - keypoint0: from, - keypoint1: to, - } in &class.keypoint_connections - { - body.row(row_height, |mut row| { - for id in [from, to] { - row.col(|ui| { - ui.label( - keypoint_map - .get(id) - .and_then(|info| info.label.as_ref()) - .map_or_else( - || format!("id {}", id.0), - |label| label.to_string(), - ), - ); - }); - } + body.rows( + row_height, + class.keypoint_connections.len(), + |row_idx, mut row| { + let pair = &class.keypoint_connections[row_idx]; + let KeypointPair { + keypoint0, + keypoint1, + } = pair; + + for id in [keypoint0, keypoint1] { + row.col(|ui| { + ui.label( + keypoint_map + .get(id) + .and_then(|info| info.label.as_ref()) + .map_or_else( + || format!("id {}", id.0), + |label| label.to_string(), + ), + ); + }); + } + }, + ); }); - } }); - }); + }, + ); } } -fn annotation_info_table_ui<'a>( +fn annotation_info_table_ui( ui: &mut egui::Ui, - annotation_infos: impl Iterator, + verbosity: UiVerbosity, + annotation_infos: &[&AnnotationInfo], ) { + re_tracing::profile_function!(); + let row_height = re_ui::ReUi::table_line_height(); ui.spacing_mut().item_spacing.x = 20.0; // column spacing. - use egui_extras::{Column, TableBuilder}; + use egui_extras::Column; - let table = TableBuilder::new(ui) - .min_scrolled_height(TABLE_SCROLL_AREA_HEIGHT) - .max_scroll_height(TABLE_SCROLL_AREA_HEIGHT) + let table = table_for_verbosity(verbosity, ui) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .column(Column::auto()) // id .column(Column::auto().clip(true).at_least(40.0)) // label @@ -255,24 +281,23 @@ fn annotation_info_table_ui<'a>( .body(|mut body| { re_ui::ReUi::setup_table_body(&mut body); - for info in annotation_infos { - body.row(row_height, |mut row| { - row.col(|ui| { - ui.label(info.id.to_string()); - }); - row.col(|ui| { - let label = if let Some(label) = &info.label { - label.as_str() - } else { - "" - }; - ui.label(label); - }); - row.col(|ui| { - color_ui(ui, info, Vec2::new(64.0, row_height)); - }); + body.rows(row_height, annotation_infos.len(), |row_idx, mut row| { + let info = &annotation_infos[row_idx]; + row.col(|ui| { + ui.label(info.id.to_string()); }); - } + row.col(|ui| { + let label = if let Some(label) = &info.label { + label.as_str() + } else { + "" + }; + ui.label(label); + }); + row.col(|ui| { + color_ui(ui, info, Vec2::new(64.0, row_height)); + }); + }); }); } diff --git a/crates/re_data_ui/src/component.rs b/crates/re_data_ui/src/component.rs index c1b0d17a4078..14d29eab07e7 100644 --- a/crates/re_data_ui/src/component.rs +++ b/crates/re_data_ui/src/component.rs @@ -4,7 +4,7 @@ use re_query::ComponentWithInstances; use re_types::ComponentName; use re_viewer_context::{UiVerbosity, ViewerContext}; -use super::DataUi; +use super::{table_for_verbosity, DataUi}; use crate::item_ui; // We do NOT implement `DataUi` for just `ComponentWithInstances` @@ -41,14 +41,14 @@ impl DataUi for EntityComponentWithInstances { let one_line = match verbosity { UiVerbosity::Small => true, - UiVerbosity::Reduced | UiVerbosity::All => false, + UiVerbosity::Reduced | UiVerbosity::LimitHeight | UiVerbosity::Full => false, }; // in some cases, we don't want to display all instances let max_row = match verbosity { UiVerbosity::Small => 0, UiVerbosity::Reduced => num_instances.at_most(4), // includes "…x more" if any - UiVerbosity::All => num_instances, + UiVerbosity::LimitHeight | UiVerbosity::Full => num_instances, }; // Here we enforce that exactly `max_row` rows are displayed, which means that: @@ -97,28 +97,10 @@ impl DataUi for EntityComponentWithInstances { re_format::format_number(num_instances) )); } else { - let mut table = egui_extras::TableBuilder::new(ui) + table_for_verbosity(verbosity, ui) .resizable(false) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) - .columns(egui_extras::Column::auto(), 2); - - if verbosity == UiVerbosity::All { - // Use full width in the selection panel. - table = table.auto_shrink([false, true]); - } else { - // Be as small as possible in the hover tooltips. - table = table.auto_shrink([true, true]); - } - - if verbosity == UiVerbosity::All && ctx.selection().len() <= 1 { - // We're alone in the selection panel. Let the outer ScrollArea do the work. - table = table.vscroll(false); - } else { - // Don't take too much vertical space to leave room for other selected items. - table = table.vscroll(true).max_scroll_height(100.0); - } - - table + .columns(egui_extras::Column::auto(), 2) .header(re_ui::ReUi::table_header_height(), |mut header| { re_ui::ReUi::setup_table_header(&mut header); header.col(|ui| { diff --git a/crates/re_data_ui/src/component_ui_registry.rs b/crates/re_data_ui/src/component_ui_registry.rs index 8ccc41e819fa..5ec1100cd8a7 100644 --- a/crates/re_data_ui/src/component_ui_registry.rs +++ b/crates/re_data_ui/src/component_ui_registry.rs @@ -126,10 +126,13 @@ fn text_ui(string: &str, ui: &mut egui::Ui, verbosity: UiVerbosity) { UiVerbosity::Reduced => { layout_job.wrap.max_rows = 3; } - UiVerbosity::All => { + UiVerbosity::LimitHeight => { let num_newlines = string.chars().filter(|&c| c == '\n').count(); needs_scroll_area = 10 < num_newlines || 300 < string.len(); } + UiVerbosity::Full => { + needs_scroll_area = false; + } } if needs_scroll_area { diff --git a/crates/re_data_ui/src/data.rs b/crates/re_data_ui/src/data.rs index 22e37e4b07d0..ab4913dcfed4 100644 --- a/crates/re_data_ui/src/data.rs +++ b/crates/re_data_ui/src/data.rs @@ -6,7 +6,7 @@ use re_types::components::{ }; use re_viewer_context::{UiVerbosity, ViewerContext}; -use super::DataUi; +use super::{table_for_verbosity, DataUi}; /// Default number of ui points to show a number. const DEFAULT_NUMBER_WIDTH: f32 = 52.0; @@ -63,7 +63,7 @@ impl DataUi for ViewCoordinates { UiVerbosity::Small => { ui.label(format!("ViewCoordinates: {}", self.describe())); } - UiVerbosity::All | UiVerbosity::Reduced => { + UiVerbosity::Full | UiVerbosity::LimitHeight | UiVerbosity::Reduced => { ui.label(self.describe()); } } @@ -133,13 +133,10 @@ impl DataUi for LineStrip2D { UiVerbosity::Small | UiVerbosity::Reduced => { ui.label(format!("{} positions", self.0.len())); } - UiVerbosity::All => { - use egui_extras::{Column, TableBuilder}; - TableBuilder::new(ui) + UiVerbosity::LimitHeight | UiVerbosity::Full => { + use egui_extras::Column; + table_for_verbosity(verbosity, ui) .resizable(true) - .vscroll(true) - .auto_shrink([false, true]) - .max_scroll_height(100.0) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .columns(Column::initial(DEFAULT_NUMBER_WIDTH).clip(true), 2) .header(re_ui::ReUi::table_header_height(), |mut header| { @@ -182,13 +179,10 @@ impl DataUi for LineStrip3D { UiVerbosity::Small | UiVerbosity::Reduced => { ui.label(format!("{} positions", self.0.len())); } - UiVerbosity::All => { - use egui_extras::{Column, TableBuilder}; - TableBuilder::new(ui) + UiVerbosity::Full | UiVerbosity::LimitHeight => { + use egui_extras::Column; + table_for_verbosity(verbosity, ui) .resizable(true) - .vscroll(true) - .auto_shrink([false, true]) - .max_scroll_height(100.0) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) .columns(Column::initial(DEFAULT_NUMBER_WIDTH).clip(true), 3) .header(re_ui::ReUi::table_header_height(), |mut header| { @@ -245,7 +239,7 @@ impl DataUi for Material { UiVerbosity::Small | UiVerbosity::Reduced => { show_optional_albedo_factor(ui); } - UiVerbosity::All => { + UiVerbosity::Full | UiVerbosity::LimitHeight => { egui::Grid::new("material").num_columns(2).show(ui, |ui| { ui.label("albedo_factor"); show_optional_albedo_factor(ui); @@ -279,7 +273,7 @@ impl DataUi for MeshProperties { UiVerbosity::Small | UiVerbosity::Reduced => { show_optional_indices(ui); } - UiVerbosity::All => { + UiVerbosity::Full | UiVerbosity::LimitHeight => { egui::Grid::new("material").num_columns(2).show(ui, |ui| { ui.label("triangles"); show_optional_indices(ui); diff --git a/crates/re_data_ui/src/image.rs b/crates/re_data_ui/src/image.rs index 4ee9c7975c11..de8c7f7f3fba 100644 --- a/crates/re_data_ui/src/image.rs +++ b/crates/re_data_ui/src/image.rs @@ -170,7 +170,7 @@ fn tensor_ui( }); } - UiVerbosity::All | UiVerbosity::Reduced => { + UiVerbosity::Full | UiVerbosity::LimitHeight | UiVerbosity::Reduced => { ui.vertical(|ui| { ui.set_min_width(100.0); tensor_summary_ui( diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 70d2e8b34b38..6683a73033f5 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -52,12 +52,15 @@ impl DataUi for InstancePath { .num_columns(2) .show(ui, |ui| { for &component_name in crate::ui_visible_components(&components) { - if verbosity != UiVerbosity::All - && component_name.is_indicator_component() - && !all_are_indicators - { - // Skip indicator components in hover ui (unless there are no other types of components). - continue; + match verbosity { + UiVerbosity::Small | UiVerbosity::Reduced => { + // Skip indicator components in hover ui (unless there are no other + // types of components). + if component_name.is_indicator_component() && !all_are_indicators { + continue; + } + } + UiVerbosity::LimitHeight | UiVerbosity::Full => {} } let Some((_, component_data)) = diff --git a/crates/re_data_ui/src/lib.rs b/crates/re_data_ui/src/lib.rs index cfaa7390af09..0e98a3b8ef6b 100644 --- a/crates/re_data_ui/src/lib.rs +++ b/crates/re_data_ui/src/lib.rs @@ -111,10 +111,14 @@ where ctx: &ViewerContext<'_>, ui: &mut egui::Ui, verbosity: UiVerbosity, - _entity: &EntityPath, + entity: &EntityPath, query: &re_arrow_store::LatestAtQuery, ) { - self.data_ui(ctx, ui, verbosity, query); + // This ensures that UI state is maintained per entity. For example, the collapsed state for + // `AnnotationContext` component is not saved by all instances of the component. + ui.push_id(entity.hash(), |ui| { + self.data_ui(ctx, ui, verbosity, query); + }); } } @@ -157,7 +161,7 @@ impl DataUi for [DataCell] { ui.label(sorted.iter().map(format_cell).join(", ")); } - UiVerbosity::All | UiVerbosity::Reduced => { + UiVerbosity::Full | UiVerbosity::LimitHeight | UiVerbosity::Reduced => { ui.vertical(|ui| { for component_bundle in &sorted { ui.label(format_cell(component_bundle)); @@ -189,3 +193,34 @@ pub fn annotations( annotation_map.load(ctx, query, std::iter::once(entity_path)); annotation_map.find(entity_path) } + +// --------------------------------------------------------------------------- + +/// Build an egui table and configure it for the given verbosity. +/// +/// Note that the caller is responsible for strictly limiting the number of displayed rows for +/// [`UiVerbosity::Small`] and [`UiVerbosity::Reduced`], as the table will not scroll. +pub fn table_for_verbosity( + verbosity: UiVerbosity, + ui: &mut egui::Ui, +) -> egui_extras::TableBuilder<'_> { + let table = egui_extras::TableBuilder::new(ui); + match verbosity { + UiVerbosity::Small | UiVerbosity::Reduced => { + // Be as small as possible in the hover tooltips. No scrolling related configuration, as + // the content itself must be limited (scrolling is not possible in tooltips). + table.auto_shrink([true, true]) + } + UiVerbosity::LimitHeight => { + // Don't take too much vertical space to leave room for other selected items. + table + .auto_shrink([false, true]) + .vscroll(true) + .max_scroll_height(100.0) + } + UiVerbosity::Full => { + // We're alone in the selection panel. Let the outer ScrollArea do the work. + table.auto_shrink([false, true]).vscroll(false) + } + } +} diff --git a/crates/re_data_ui/src/transform3d.rs b/crates/re_data_ui/src/transform3d.rs index 2ba11badfb86..0e262359c5e8 100644 --- a/crates/re_data_ui/src/transform3d.rs +++ b/crates/re_data_ui/src/transform3d.rs @@ -16,11 +16,11 @@ impl DataUi for re_types::components::Transform3D { UiVerbosity::Small => { // TODO(andreas): Preview some information instead of just a label with hover ui. ui.label("3D transform").on_hover_ui(|ui| { - self.data_ui(ctx, ui, UiVerbosity::All, query); + self.data_ui(ctx, ui, UiVerbosity::LimitHeight, query); }); } - UiVerbosity::All | UiVerbosity::Reduced => { + UiVerbosity::Full | UiVerbosity::LimitHeight | UiVerbosity::Reduced => { let from_parent = match &self.0 { Transform3D::TranslationRotationScale(t) => t.from_parent, Transform3D::TranslationAndMat3x3(t) => t.from_parent, @@ -68,11 +68,11 @@ impl DataUi for Transform3D { match verbosity { UiVerbosity::Small => { ui.label("3D transform").on_hover_ui(|ui| { - self.data_ui(ctx, ui, UiVerbosity::All, query); + self.data_ui(ctx, ui, UiVerbosity::LimitHeight, query); }); } - UiVerbosity::All | UiVerbosity::Reduced => match self { + UiVerbosity::Full | UiVerbosity::LimitHeight | UiVerbosity::Reduced => match self { Transform3D::TranslationAndMat3x3(translation_matrix) => { translation_matrix.data_ui(ctx, ui, verbosity, query); } diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index 96d694766ea8..b36b6a8298a5 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -633,6 +633,33 @@ impl ReUi { } } + /// Conditionally collapsing header. + /// + /// Display content under a header that is conditionally collapsible. If `collapsing` is `true`, + /// this is equivalent to [`ReUi::collapsing_header`]. If `collapsing` is `false`, the content + /// is displayed under a static, non-collapsible header. + #[allow(clippy::unused_self)] + pub fn maybe_collapsing_header( + &self, + ui: &mut egui::Ui, + collapsing: bool, + label: &str, + default_open: bool, + add_body: impl FnOnce(&mut egui::Ui) -> R, + ) -> egui::CollapsingResponse { + if collapsing { + self.collapsing_header(ui, label, default_open, add_body) + } else { + let response = ui.strong(label); + CollapsingResponse { + header_response: response, + body_response: None, + body_returned: None, + openness: 1.0, + } + } + } + /// Show a prominent collapsing header to be used as section delimitation in side panels. /// /// Note that a clip rect must be set (typically by the panel) to avoid any overdraw. diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index c2bd66c08ffe..2b07eb8e7eb5 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -113,6 +113,11 @@ impl SelectionPanel { ui.add_space(-ui.spacing().item_spacing.y); let selection = ctx.selection().to_vec(); + let multi_selection_verbosity = if selection.len() > 1 { + UiVerbosity::LimitHeight + } else { + UiVerbosity::Full + }; for (i, item) in selection.iter().enumerate() { ui.push_id(i, |ui| { what_is_selected_ui(ui, ctx, &mut viewport.blueprint, item); @@ -136,7 +141,7 @@ impl SelectionPanel { if has_data_section(item) { ctx.re_ui.large_collapsing_header(ui, "Data", true, |ui| { - item.data_ui(ctx, ui, UiVerbosity::All, &query); + item.data_ui(ctx, ui, multi_selection_verbosity, &query); }); } diff --git a/crates/re_viewer_context/src/component_ui_registry.rs b/crates/re_viewer_context/src/component_ui_registry.rs index 0c9043094c12..10ac0a96d9a7 100644 --- a/crates/re_viewer_context/src/component_ui_registry.rs +++ b/crates/re_viewer_context/src/component_ui_registry.rs @@ -18,8 +18,18 @@ pub enum UiVerbosity { /// Keep it under a half-dozen lines. Reduced, - /// Display everything, as large as you want. Used for selection panel. - All, + /// Display everything as wide as available but limit height. + /// + /// This is used for example in the selection panel when multiple items are selected. When using + /// a Table, use the `re_data_ui::table_for_verbosity` function. + LimitHeight, + + /// Display everything as wide as available, without height restrictions. + /// + /// This is used for example in the selection panel when only one item is selected. In this + /// case, any scrolling is handled by the selection panel itself. When using a Table, use the + /// `re_data_ui::table_for_verbosity` function. + Full, } type ComponentUiCallback = Box< diff --git a/scripts/clippy_wasm/clippy.toml b/scripts/clippy_wasm/clippy.toml index 35c016649717..eb4cb7c2ac33 100644 --- a/scripts/clippy_wasm/clippy.toml +++ b/scripts/clippy_wasm/clippy.toml @@ -27,12 +27,10 @@ too-many-lines-threshold = 600 # TODO(emilk): decrease this # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods disallowed-methods = [ - "std::time::Instant::now", # use `web-time` crate instead for wasm/web compatibility - "std::time::Duration::elapsed", # use `web-time` crate instead for wasm/web compatibility - "std::time::SystemTime::now", # use `web-time` or `time` crates instead for wasm/web compatibility - - # Cannot spawn threads on wasm: - "std::thread::spawn", + { path = "std::thread::spawn", reason = "Cannot spawn threads on wasm" }, + { path = "std::time::Duration::elapsed", reason = "use `web-time` crate instead for wasm/web compatibility" }, + { path = "std::time::Instant::now", reason = "use `web-time` crate instead for wasm/web compatibility" }, + { path = "std::time::SystemTime::now", reason = "use `web-time` or `time` crates instead for wasm/web compatibility" }, ] # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types diff --git a/tests/python/annotation_context_ui_stress/annotation_context_ui_stress.py b/tests/python/annotation_context_ui_stress/annotation_context_ui_stress.py new file mode 100644 index 000000000000..829afac6dc20 --- /dev/null +++ b/tests/python/annotation_context_ui_stress/annotation_context_ui_stress.py @@ -0,0 +1,33 @@ +"""Logs a very complex/long Annotation context for the purpose of testing/debugging the related Selection Panel UI.""" + +from __future__ import annotations + +import argparse + +import rerun as rr +from rerun.datatypes import ClassDescription + +parser = argparse.ArgumentParser() +rr.script_add_args(parser) +args = parser.parse_args() +rr.script_setup(args, "rerun_example_annotation_context_ui_stress") + + +annotation_context = rr.AnnotationContext( + [ + ClassDescription( + info=(0, "class_info", (255, 0, 0)), + keypoint_annotations=[(i, f"keypoint {i}", (255, 255 - i, 0)) for i in range(100)], + keypoint_connections=[(i, 99 - i) for i in range(50)], + ), + ClassDescription( + info=(1, "another_class_info", (255, 0, 255)), + keypoint_annotations=[(i, f"keypoint {i}", (255, 255, i)) for i in range(100)], + keypoint_connections=[(0, 2), (1, 2), (2, 3)], + ), + ] +) + +# log two of those to test multi-selection +rr.log("annotation1", annotation_context) +rr.log("annotation2", annotation_context) From 2d5eaa23c2e692d1c3ab8717f05b481c5ea59735 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 6 Dec 2023 14:12:03 +0100 Subject: [PATCH 2/4] Rename `RowId::random` to `RowId::new` (#4445) ### What The `RowId` is time-based, and so not random. ### 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): * Full build: [app.rerun.io](https://app.rerun.io/pr/4445/index.html) * Partial build: [app.rerun.io](https://app.rerun.io/pr/4445/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) - Useful for quick testing when changes do not affect examples in any way * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/4445) - [Docs preview](https://rerun.io/preview/273089f0048b2a73b242717fd7d612fdb10a7912/docs) - [Examples preview](https://rerun.io/preview/273089f0048b2a73b242717fd7d612fdb10a7912/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- crates/re_arrow_store/benches/data_store.rs | 2 +- crates/re_arrow_store/benches/gc.rs | 2 +- crates/re_arrow_store/src/store_dump.rs | 8 ++-- crates/re_arrow_store/src/store_event.rs | 10 ++--- crates/re_arrow_store/src/store_helpers.rs | 4 +- crates/re_arrow_store/src/store_subscriber.rs | 6 +-- crates/re_arrow_store/src/test_util.rs | 4 +- crates/re_arrow_store/tests/correctness.rs | 20 ++++----- crates/re_arrow_store/tests/data_store.rs | 16 +++---- crates/re_arrow_store/tests/dump.rs | 6 +-- crates/re_arrow_store/tests/internals.rs | 6 +-- .../re_data_source/src/load_file_contents.rs | 7 ++- crates/re_data_source/src/load_file_path.rs | 7 ++- crates/re_data_store/examples/memory_usage.rs | 4 +- crates/re_data_store/src/store_db.rs | 2 +- crates/re_data_store/tests/clear.rs | 22 ++++----- crates/re_data_store/tests/time_histograms.rs | 10 ++--- crates/re_log_encoding/src/decoder.rs | 2 +- crates/re_log_types/src/data_row.rs | 12 ++--- crates/re_log_types/src/data_table.rs | 32 ++++++------- crates/re_log_types/src/data_table_batcher.rs | 2 +- crates/re_query/benches/query_benchmark.rs | 4 +- crates/re_query/src/query.rs | 4 +- .../re_query/tests/archetype_query_tests.rs | 26 +++++------ .../re_query/tests/archetype_range_tests.rs | 45 +++++++++---------- crates/re_sdk/src/recording_stream.rs | 8 ++-- .../re_space_view/src/data_query_blueprint.rs | 4 +- .../benches/bench_points.rs | 2 +- crates/re_tuid/benches/bench_tuid.rs | 9 ++-- crates/re_tuid/src/lib.rs | 10 +++-- crates/re_viewer/src/app_blueprint.rs | 7 ++- crates/re_viewer/src/store_hub.rs | 2 +- crates/re_viewer/src/ui/selection_panel.rs | 2 +- .../src/ui/welcome_screen/welcome_page.rs | 2 +- .../src/space_view/view_query.rs | 2 +- crates/re_viewport/src/space_view.rs | 7 ++- crates/re_viewport/src/viewport_blueprint.rs | 4 +- crates/rerun_c/src/lib.rs | 2 +- rerun_py/src/arrow.rs | 2 +- rerun_py/src/python_bridge.rs | 6 +-- 40 files changed, 159 insertions(+), 173 deletions(-) diff --git a/crates/re_arrow_store/benches/data_store.rs b/crates/re_arrow_store/benches/data_store.rs index e7074c684350..a1057964e1cd 100644 --- a/crates/re_arrow_store/benches/data_store.rs +++ b/crates/re_arrow_store/benches/data_store.rs @@ -333,7 +333,7 @@ fn build_table(n: usize, packed: bool) -> DataTable { TableId::ZERO, (0..NUM_ROWS).map(move |frame_idx| { DataRow::from_cells2( - RowId::random(), + RowId::new(), "large_structs", [build_frame_nr(frame_idx.into())], n as _, diff --git a/crates/re_arrow_store/benches/gc.rs b/crates/re_arrow_store/benches/gc.rs index 6700869972c9..c9d79ebe3615 100644 --- a/crates/re_arrow_store/benches/gc.rs +++ b/crates/re_arrow_store/benches/gc.rs @@ -277,7 +277,7 @@ where TableId::ZERO, (0..NUM_ROWS_PER_ENTITY_PATH).map(move |i| { DataRow::from_component_batches( - RowId::random(), + RowId::new(), timegen(i), entity_path.clone(), datagen(i) diff --git a/crates/re_arrow_store/src/store_dump.rs b/crates/re_arrow_store/src/store_dump.rs index 904950794b25..97a06ed8adb7 100644 --- a/crates/re_arrow_store/src/store_dump.rs +++ b/crates/re_arrow_store/src/store_dump.rs @@ -42,7 +42,7 @@ impl DataStore { rows.sort_by_key(|row| (row.timepoint.clone(), row.row_id)); Ok(re_log_types::DataTable::from_rows( - re_log_types::TableId::random(), + re_log_types::TableId::new(), rows, )) } @@ -84,7 +84,7 @@ impl DataStore { } = inner; DataTable { - table_id: TableId::random(), + table_id: TableId::new(), col_row_id: col_row_id.clone(), col_timelines: Default::default(), col_entity_path: std::iter::repeat_with(|| ent_path.clone()) @@ -124,7 +124,7 @@ impl DataStore { } = &*inner.read(); DataTable { - table_id: TableId::random(), + table_id: TableId::new(), col_row_id: col_row_id.clone(), col_timelines: [(*timeline, col_time.iter().copied().map(Some).collect())] .into(), @@ -210,7 +210,7 @@ impl DataStore { } Some(DataTable { - table_id: TableId::random(), + table_id: TableId::new(), col_row_id, col_timelines, col_entity_path, diff --git a/crates/re_arrow_store/src/store_event.rs b/crates/re_arrow_store/src/store_event.rs index 2e7c5b8f6667..aa6064d9acaf 100644 --- a/crates/re_arrow_store/src/store_event.rs +++ b/crates/re_arrow_store/src/store_event.rs @@ -295,7 +295,7 @@ mod tests { let timeline_other = Timeline::new_temporal("other"); let timeline_yet_another = Timeline::new_sequence("yet_another"); - let row_id1 = RowId::random(); + let row_id1 = RowId::new(); let timepoint1 = TimePoint::from_iter([ (timeline_frame, 42.into()), // (timeline_other, 666.into()), // @@ -337,7 +337,7 @@ mod tests { view, ); - let row_id2 = RowId::random(); + let row_id2 = RowId::new(); let timepoint2 = TimePoint::from_iter([ (timeline_frame, 42.into()), // (timeline_yet_another, 1.into()), // @@ -389,7 +389,7 @@ mod tests { view, ); - let row_id3 = RowId::random(); + let row_id3 = RowId::new(); let timepoint3 = TimePoint::timeless(); let row3 = { let num_instances = 6; @@ -486,7 +486,7 @@ mod tests { let timeline_frame = Timeline::new_sequence("frame"); let row1 = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([(timeline_frame, 42.into())]), "entity_a".into(), [&InstanceKey::from_iter(0..10) as _], @@ -503,7 +503,7 @@ mod tests { } let row2 = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([(timeline_frame, 42.into())]), "entity_b".into(), [&[MyColor::from(0xAABBCCDD)] as _], diff --git a/crates/re_arrow_store/src/store_helpers.rs b/crates/re_arrow_store/src/store_helpers.rs index 76be013cc9a9..48c4a95c8d04 100644 --- a/crates/re_arrow_store/src/store_helpers.rs +++ b/crates/re_arrow_store/src/store_helpers.rs @@ -203,7 +203,7 @@ impl DataStore { re_tracing::profile_function!(); let mut row = match DataRow::from_cells1( - RowId::random(), + RowId::new(), entity_path.clone(), timepoint.clone(), 1, @@ -243,7 +243,7 @@ impl DataStore { let cell = DataCell::from_arrow_empty(component, datatype.clone()); let mut row = match DataRow::from_cells1( - RowId::random(), + RowId::new(), entity_path.clone(), timepoint.clone(), cell.num_instances(), diff --git a/crates/re_arrow_store/src/store_subscriber.rs b/crates/re_arrow_store/src/store_subscriber.rs index 0aa454453c32..998eb482ff10 100644 --- a/crates/re_arrow_store/src/store_subscriber.rs +++ b/crates/re_arrow_store/src/store_subscriber.rs @@ -212,7 +212,7 @@ mod tests { let timeline_yet_another = Timeline::new_sequence("yet_another"); let row = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_frame, 42.into()), // (timeline_other, 666.into()), // @@ -231,7 +231,7 @@ mod tests { .collect(); let colors = vec![MyColor::from(0xFF0000FF)]; DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_frame, 42.into()), // (timeline_yet_another, 1.into()), // @@ -247,7 +247,7 @@ mod tests { let num_instances = 6; let colors = vec![MyColor::from(0x00DD00FF); num_instances]; DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::timeless(), "entity_b".into(), [ diff --git a/crates/re_arrow_store/src/test_util.rs b/crates/re_arrow_store/src/test_util.rs index 58f53e4c362c..bfdad6b8130b 100644 --- a/crates/re_arrow_store/src/test_util.rs +++ b/crates/re_arrow_store/src/test_util.rs @@ -7,7 +7,7 @@ use crate::{DataStore, DataStoreConfig}; macro_rules! test_row { ($entity:ident @ $frames:tt => $n:expr; [$c0:expr $(,)*]) => {{ ::re_log_types::DataRow::from_cells1_sized( - ::re_log_types::RowId::random(), + ::re_log_types::RowId::new(), $entity.clone(), $frames, $n, @@ -17,7 +17,7 @@ macro_rules! test_row { }}; ($entity:ident @ $frames:tt => $n:expr; [$c0:expr, $c1:expr $(,)*]) => {{ ::re_log_types::DataRow::from_cells2_sized( - ::re_log_types::RowId::random(), + ::re_log_types::RowId::new(), $entity.clone(), $frames, $n, diff --git a/crates/re_arrow_store/tests/correctness.rs b/crates/re_arrow_store/tests/correctness.rs index 32111ac5f526..12ed9c0e1c3e 100644 --- a/crates/re_arrow_store/tests/correctness.rs +++ b/crates/re_arrow_store/tests/correctness.rs @@ -42,7 +42,7 @@ fn row_id_ordering_semantics() -> anyhow::Result<()> { Default::default(), ); - let row_id = RowId::random(); + let row_id = RowId::new(); let row = DataRow::from_component_batches( row_id, timepoint.clone(), @@ -51,7 +51,7 @@ fn row_id_ordering_semantics() -> anyhow::Result<()> { )?; store.insert_row(&row)?; - let row_id = RowId::random(); + let row_id = RowId::new(); let row = DataRow::from_component_batches( row_id, timepoint.clone(), @@ -83,7 +83,7 @@ fn row_id_ordering_semantics() -> anyhow::Result<()> { Default::default(), ); - let row_id = RowId::random(); + let row_id = RowId::new(); let row = DataRow::from_component_batches( row_id, @@ -114,7 +114,7 @@ fn row_id_ordering_semantics() -> anyhow::Result<()> { Default::default(), ); - let row_id1 = RowId::random(); + let row_id1 = RowId::new(); let row_id2 = row_id1.next(); let row = DataRow::from_component_batches( @@ -159,7 +159,7 @@ fn row_id_ordering_semantics() -> anyhow::Result<()> { Default::default(), ); - let row_id1 = RowId::random(); + let row_id1 = RowId::new(); let row_id2 = row_id1.next(); let row = DataRow::from_component_batches( @@ -266,7 +266,7 @@ fn write_errors() { build_log_time(Time::now()), ] => 1; [ build_some_positions2d(1) ]); - row.row_id = re_log_types::RowId::random(); + row.row_id = re_log_types::RowId::new(); store.insert_row(&row).unwrap(); row.row_id = row.row_id.next(); @@ -574,7 +574,7 @@ fn gc_metadata_size() -> anyhow::Result<()> { for _ in 0..3 { let row = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::timeless(), "xxx".into(), [&[point] as _], @@ -650,7 +650,7 @@ fn entity_min_time_correct_impl(store: &mut DataStore) -> anyhow::Result<()> { let now_minus_one = now - Duration::from_secs(1.0); let row = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_log_time, now.into()), (timeline_frame_nr, 42.into()), @@ -681,7 +681,7 @@ fn entity_min_time_correct_impl(store: &mut DataStore) -> anyhow::Result<()> { // insert row in the future, these shouldn't be visible let row = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_log_time, now_plus_one.into()), (timeline_frame_nr, 54.into()), @@ -711,7 +711,7 @@ fn entity_min_time_correct_impl(store: &mut DataStore) -> anyhow::Result<()> { // insert row in the past, these should be visible let row = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_log_time, now_minus_one.into()), (timeline_frame_nr, 32.into()), diff --git a/crates/re_arrow_store/tests/data_store.rs b/crates/re_arrow_store/tests/data_store.rs index e3bdd80602c5..e22a5278d072 100644 --- a/crates/re_arrow_store/tests/data_store.rs +++ b/crates/re_arrow_store/tests/data_store.rs @@ -329,7 +329,7 @@ fn latest_at_impl(store: &mut DataStore) { insert_table( store, &DataTable::from_rows( - TableId::random(), + TableId::new(), [row1.clone(), row2.clone(), row3.clone(), row4.clone()], ), ); @@ -1000,10 +1000,8 @@ fn protected_gc_impl(store: &mut DataStore) { store.insert_row(&row4).unwrap(); // Re-insert row1 and row2 as timeless data as well - let mut table_timeless = DataTable::from_rows( - TableId::random(), - [row1.clone().next(), row2.clone().next()], - ); + let mut table_timeless = + DataTable::from_rows(TableId::new(), [row1.clone().next(), row2.clone().next()]); table_timeless.col_timelines = Default::default(); insert_table_with_retries(store, &table_timeless); @@ -1098,10 +1096,8 @@ fn protected_gc_clear_impl(store: &mut DataStore) { let row4 = test_row!(ent_path @ [build_frame_nr(frame4)] => 0; [points4]); // Insert the 3 rows as timeless - let mut table_timeless = DataTable::from_rows( - TableId::random(), - [row1.clone(), row2.clone(), row3.clone()], - ); + let mut table_timeless = + DataTable::from_rows(TableId::new(), [row1.clone(), row2.clone(), row3.clone()]); table_timeless.col_timelines = Default::default(); insert_table_with_retries(store, &table_timeless); @@ -1145,7 +1141,7 @@ fn protected_gc_clear_impl(store: &mut DataStore) { assert_eq!(stats.timeless.num_rows, 2); // Now erase points and GC again - let mut table_timeless = DataTable::from_rows(TableId::random(), [row4]); + let mut table_timeless = DataTable::from_rows(TableId::new(), [row4]); table_timeless.col_timelines = Default::default(); insert_table_with_retries(store, &table_timeless); diff --git a/crates/re_arrow_store/tests/dump.rs b/crates/re_arrow_store/tests/dump.rs index a2284599f8bb..67ed29594325 100644 --- a/crates/re_arrow_store/tests/dump.rs +++ b/crates/re_arrow_store/tests/dump.rs @@ -270,7 +270,7 @@ fn create_insert_table(ent_path: impl Into) -> DataTable { build_frame_nr(frame4), ] => 5; [colors4]); - let mut table = DataTable::from_rows(TableId::random(), [row1, row2, row3, row4]); + let mut table = DataTable::from_rows(TableId::new(), [row1, row2, row3, row4]); table.compute_all_size_bytes(); table @@ -314,7 +314,7 @@ fn data_store_dump_empty_column_impl(store: &mut DataStore) { let row2 = test_row!(ent_path @ [ build_frame_nr(frame2), ] => 3; [instances2, positions2]); - let mut table = DataTable::from_rows(TableId::random(), [row1, row2]); + let mut table = DataTable::from_rows(TableId::new(), [row1, row2]); table.compute_all_size_bytes(); insert_table_with_retries(store, &table); } @@ -325,7 +325,7 @@ fn data_store_dump_empty_column_impl(store: &mut DataStore) { let row3 = test_row!(ent_path @ [ build_frame_nr(frame3), ] => 3; [instances3, positions3]); - let mut table = DataTable::from_rows(TableId::random(), [row3]); + let mut table = DataTable::from_rows(TableId::new(), [row3]); table.compute_all_size_bytes(); insert_table_with_retries(store, &table); } diff --git a/crates/re_arrow_store/tests/internals.rs b/crates/re_arrow_store/tests/internals.rs index 345946a49efd..05f8abf7b513 100644 --- a/crates/re_arrow_store/tests/internals.rs +++ b/crates/re_arrow_store/tests/internals.rs @@ -53,7 +53,7 @@ fn pathological_bucket_topology() { let timepoint = TimePoint::from([build_frame_nr(frame_nr.into())]); for _ in 0..num { let row = DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint.clone(), num_instances, @@ -63,7 +63,7 @@ fn pathological_bucket_topology() { store_forward.insert_row(&row).unwrap(); let row = DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint.clone(), num_instances, @@ -86,7 +86,7 @@ fn pathological_bucket_topology() { .map(|frame_nr| { let timepoint = TimePoint::from([build_frame_nr(frame_nr.into())]); DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint, num_instances, diff --git a/crates/re_data_source/src/load_file_contents.rs b/crates/re_data_source/src/load_file_contents.rs index cea4642347c2..06dd8f8302d6 100644 --- a/crates/re_data_source/src/load_file_contents.rs +++ b/crates/re_data_source/src/load_file_contents.rs @@ -55,7 +55,7 @@ fn load_and_send( // First, set a store info since this is the first thing the application expects. tx.send(LogMsg::SetStoreInfo(SetStoreInfo { - row_id: re_log_types::RowId::random(), + row_id: re_log_types::RowId::new(), info: re_log_types::StoreInfo { application_id: re_log_types::ApplicationId(file_contents.name.clone()), store_id: store_id.clone(), @@ -89,15 +89,14 @@ fn log_msg_from_file_contents( let timepoint = re_log_types::TimePoint::default(); let data_row = re_log_types::DataRow::from_cells( - re_log_types::RowId::random(), + re_log_types::RowId::new(), timepoint, entity_path, num_instances, cells, )?; - let data_table = - re_log_types::DataTable::from_rows(re_log_types::TableId::random(), [data_row]); + let data_table = re_log_types::DataTable::from_rows(re_log_types::TableId::new(), [data_row]); let arrow_msg = data_table.to_arrow_msg()?; Ok(LogMsg::ArrowMsg(store_id, arrow_msg)) } diff --git a/crates/re_data_source/src/load_file_path.rs b/crates/re_data_source/src/load_file_path.rs index c9ebcd4c527d..00999447b40c 100644 --- a/crates/re_data_source/src/load_file_path.rs +++ b/crates/re_data_source/src/load_file_path.rs @@ -53,7 +53,7 @@ fn load_and_send( // First, set a store info since this is the first thing the application expects. tx.send(LogMsg::SetStoreInfo(SetStoreInfo { - row_id: re_log_types::RowId::random(), + row_id: re_log_types::RowId::new(), info: re_log_types::StoreInfo { application_id: re_log_types::ApplicationId(path.display().to_string()), store_id: store_id.clone(), @@ -85,15 +85,14 @@ fn log_msg_from_file_path( let timepoint = re_log_types::TimePoint::default(); let data_row = re_log_types::DataRow::from_cells( - re_log_types::RowId::random(), + re_log_types::RowId::new(), timepoint, entity_path, num_instances, cells, )?; - let data_table = - re_log_types::DataTable::from_rows(re_log_types::TableId::random(), [data_row]); + let data_table = re_log_types::DataTable::from_rows(re_log_types::TableId::new(), [data_row]); let arrow_msg = data_table.to_arrow_msg()?; Ok(LogMsg::ArrowMsg(store_id, arrow_msg)) } diff --git a/crates/re_data_store/examples/memory_usage.rs b/crates/re_data_store/examples/memory_usage.rs index b04d279b6e97..50ec9ef490e3 100644 --- a/crates/re_data_store/examples/memory_usage.rs +++ b/crates/re_data_store/examples/memory_usage.rs @@ -109,7 +109,7 @@ fn log_messages() { let used_bytes_start = live_bytes(); let table = Box::new( DataRow::from_cells1( - RowId::random(), + RowId::new(), entity_path!("points"), [build_frame_nr(0.into())], 1, @@ -136,7 +136,7 @@ fn log_messages() { let used_bytes_start = live_bytes(); let table = Box::new( DataRow::from_cells1( - RowId::random(), + RowId::new(), entity_path!("points"), [build_frame_nr(0.into())], NUM_POINTS as _, diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index bc9a919a072d..fec198fcf6f2 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -309,7 +309,7 @@ impl StoreDb { let mut store_db = StoreDb::new(store_info.store_id.clone()); store_db.set_store_info(SetStoreInfo { - row_id: RowId::random(), + row_id: RowId::new(), info: store_info, }); for row in rows { diff --git a/crates/re_data_store/tests/clear.rs b/crates/re_data_store/tests/clear.rs index 8cfc19af8fca..82c08814b952 100644 --- a/crates/re_data_store/tests/clear.rs +++ b/crates/re_data_store/tests/clear.rs @@ -27,7 +27,7 @@ fn clears() -> anyhow::Result<()> { // * Insert a 2D point & color for 'parent' at frame #10. // * Query 'parent' at frame #11 and make sure we find everything back. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 10.into())]); let point = MyPoint::new(1.0, 2.0); let color = MyColor::from(0xFF0000FF); @@ -65,7 +65,7 @@ fn clears() -> anyhow::Result<()> { // * Insert a 2D point for 'child1' at frame #10. // * Query 'child1' at frame #11 and make sure we find everything back. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 10.into())]); let point = MyPoint::new(42.0, 43.0); let row = DataRow::from_component_batches( @@ -96,7 +96,7 @@ fn clears() -> anyhow::Result<()> { // * Insert a color for 'child2' at frame #10. // * Query 'child2' at frame #11 and make sure we find everything back. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 10.into())]); let color = MyColor::from(0x00AA00DD); let row = DataRow::from_component_batches( @@ -129,7 +129,7 @@ fn clears() -> anyhow::Result<()> { // * Query 'child1' at frame #11 and make sure we find everything back. // * Query 'child2' at frame #11 and make sure we find everything back. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 10.into())]); let clear = Clear::flat(); let row = DataRow::from_component_batches( @@ -183,7 +183,7 @@ fn clears() -> anyhow::Result<()> { // * Query 'child1' at frame #11 and make sure we find nothing. // * Query 'child2' at frame #11 and make sure we find nothing. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 10.into())]); let clear = Clear::recursive(); let row = DataRow::from_component_batches( @@ -236,7 +236,7 @@ fn clears() -> anyhow::Result<()> { // * Query 'parent' at frame #9 and make sure we find it back. // * Query 'parent' at frame #11 and make sure we do _not_ find it. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 9.into())]); let instance_key = InstanceKey(0); let row = DataRow::from_component_batches( @@ -280,7 +280,7 @@ fn clears() -> anyhow::Result<()> { // * Query 'child1' at frame #9 and make sure we find everything back. // * Query 'child1' at frame #11 and make sure we do _not_ find anything. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 9.into())]); let point = MyPoint::new(42.0, 43.0); let color = MyColor::from(0xBBBBBBBB); @@ -336,7 +336,7 @@ fn clears() -> anyhow::Result<()> { // * Query 'child2' at frame #9 and make sure we find everything back. // * Query 'child2' at frame #11 and make sure we do _not_ find anything. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 9.into())]); let color = MyColor::from(0x00AA00DD); let point = MyPoint::new(66.0, 666.0); @@ -391,7 +391,7 @@ fn clears() -> anyhow::Result<()> { // * Query 'grandchild' at frame #9 and make sure we find everything back. // * Query 'grandchild' at frame #11 and make sure we do _not_ find anything. { - let row_id = RowId::random(); + let row_id = RowId::new(); let timepoint = TimePoint::from_iter([(timeline_frame, 9.into())]); let color = MyColor::from(0x00AA00DD); let row = DataRow::from_component_batches( @@ -452,7 +452,7 @@ fn clear_and_gc() -> anyhow::Result<()> { let point = MyPoint::new(1.0, 2.0); let row = DataRow::from_component_batches( - RowId::random(), + RowId::new(), timepoint.clone(), entity_path.clone(), [&[point] as _], @@ -466,7 +466,7 @@ fn clear_and_gc() -> anyhow::Result<()> { assert_eq!(stats.timeless.num_rows, 1); let clear = DataRow::from_component_batches( - RowId::random(), + RowId::new(), timepoint.clone(), entity_path.clone(), Clear::recursive() diff --git a/crates/re_data_store/tests/time_histograms.rs b/crates/re_data_store/tests/time_histograms.rs index 085cb5b60685..667bcdb0833d 100644 --- a/crates/re_data_store/tests/time_histograms.rs +++ b/crates/re_data_store/tests/time_histograms.rs @@ -34,7 +34,7 @@ fn time_histograms() -> anyhow::Result<()> { // Single top-level entity, explicitly logged `InstanceKey`s. { let row = DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_frame, 42.into()), // (timeline_other, 666.into()), // @@ -90,7 +90,7 @@ fn time_histograms() -> anyhow::Result<()> { .collect(); let colors = vec![MyColor::from(0xFF0000FF)]; DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_frame, 42.into()), // (timeline_yet_another, 1.into()), // @@ -241,7 +241,7 @@ fn time_histograms() -> anyhow::Result<()> { let num_instances = 6; let colors = vec![MyColor::from(0x00DD00FF); num_instances]; DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::timeless(), "entity".into(), [ @@ -331,7 +331,7 @@ fn time_histograms() -> anyhow::Result<()> { .collect(); let colors = vec![MyColor::from(0xFF0000FF)]; DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_frame, 1234.into()), // (timeline_other, 1235.into()), // @@ -457,7 +457,7 @@ fn time_histograms() -> anyhow::Result<()> { { let row = { DataRow::from_component_batches( - RowId::random(), + RowId::new(), TimePoint::from_iter([ (timeline_frame, 1000.into()), // ]), diff --git a/crates/re_log_encoding/src/decoder.rs b/crates/re_log_encoding/src/decoder.rs index 433179a49181..48eb1f42fb44 100644 --- a/crates/re_log_encoding/src/decoder.rs +++ b/crates/re_log_encoding/src/decoder.rs @@ -228,7 +228,7 @@ fn test_encode_decode() { }; let messages = vec![LogMsg::SetStoreInfo(SetStoreInfo { - row_id: RowId::random(), + row_id: RowId::new(), info: StoreInfo { application_id: ApplicationId("test".to_owned()), store_id: StoreId::random(StoreKind::Recording), diff --git a/crates/re_log_types/src/data_row.rs b/crates/re_log_types/src/data_row.rs index c5d31be9ad9d..8b0eabf39eeb 100644 --- a/crates/re_log_types/src/data_row.rs +++ b/crates/re_log_types/src/data_row.rs @@ -156,15 +156,17 @@ impl std::fmt::Display for RowId { impl RowId { pub const ZERO: Self = Self(re_tuid::Tuid::ZERO); + /// Create a new unique [`RowId`] based on the current time. + #[allow(clippy::new_without_default)] #[inline] - pub fn random() -> Self { - Self(re_tuid::Tuid::random()) + pub fn new() -> Self { + Self(re_tuid::Tuid::new()) } /// Returns the next logical [`RowId`]. /// /// Beware: wrong usage can easily lead to conflicts. - /// Prefer [`RowId::random`] when unsure. + /// Prefer [`RowId::new`] when unsure. #[inline] pub fn next(&self) -> Self { Self(self.0.next()) @@ -176,7 +178,7 @@ impl RowId { /// Wraps the monotonically increasing back to zero on overflow. /// /// Beware: wrong usage can easily lead to conflicts. - /// Prefer [`RowId::random`] when unsure. + /// Prefer [`RowId::new`] when unsure. #[inline] pub fn increment(&self, n: u64) -> Self { Self(self.0.increment(n)) @@ -409,7 +411,7 @@ impl DataRow { /// Turns the `DataRow` into a single-row [`DataTable`]. #[inline] pub fn into_table(self) -> DataTable { - DataTable::from_rows(TableId::random(), [self]) + DataTable::from_rows(TableId::new(), [self]) } } diff --git a/crates/re_log_types/src/data_table.rs b/crates/re_log_types/src/data_table.rs index 284060382575..56f9a1a3f8e1 100644 --- a/crates/re_log_types/src/data_table.rs +++ b/crates/re_log_types/src/data_table.rs @@ -145,15 +145,17 @@ impl std::fmt::Display for TableId { impl TableId { pub const ZERO: Self = Self(re_tuid::Tuid::ZERO); + /// Create a new unique [`TableId`] based on the current time. + #[allow(clippy::new_without_default)] #[inline] - pub fn random() -> Self { - Self(re_tuid::Tuid::random()) + pub fn new() -> Self { + Self(re_tuid::Tuid::new()) } /// Returns the next logical [`TableId`]. /// /// Beware: wrong usage can easily lead to conflicts. - /// Prefer [`TableId::random`] when unsure. + /// Prefer [`TableId::new`] when unsure. #[inline] pub fn next(&self) -> Self { Self(self.0.next()) @@ -165,7 +167,7 @@ impl TableId { /// Wraps the monotonically increasing back to zero on overflow. /// /// Beware: wrong usage can easily lead to conflicts. - /// Prefer [`TableId::random`] when unsure. + /// Prefer [`TableId::new`] when unsure. #[inline] pub fn increment(&self, n: u64) -> Self { Self(self.0.increment(n)) @@ -235,18 +237,18 @@ re_tuid::delegate_arrow_tuid!(TableId as "rerun.controls.TableId"); /// let points: &[MyPoint] = &[[10.0, 10.0].into(), [20.0, 20.0].into()]; /// let colors: &[_] = &[MyColor::from_rgb(128, 128, 128)]; /// let labels: &[Label] = &[]; -/// DataRow::from_cells3(RowId::random(), "a", timepoint(1, 1), num_instances, (points, colors, labels))? +/// DataRow::from_cells3(RowId::new(), "a", timepoint(1, 1), num_instances, (points, colors, labels))? /// }; /// let row1 = { /// let num_instances = 0; /// let colors: &[MyColor] = &[]; -/// DataRow::from_cells1(RowId::random(), "b", timepoint(1, 2), num_instances, colors)? +/// DataRow::from_cells1(RowId::new(), "b", timepoint(1, 2), num_instances, colors)? /// }; /// let row2 = { /// let num_instances = 1; /// let colors: &[_] = &[MyColor::from_rgb(255, 255, 255)]; /// let labels: &[_] = &[Label("hey".into())]; -/// DataRow::from_cells2(RowId::random(), "c", timepoint(2, 1), num_instances, (colors, labels))? +/// DataRow::from_cells2(RowId::new(), "c", timepoint(2, 1), num_instances, (colors, labels))? /// }; /// let table = DataTable::from_rows(table_id, [row0, row1, row2]); /// ``` @@ -275,7 +277,7 @@ re_tuid::delegate_arrow_tuid!(TableId as "rerun.controls.TableId"); /// # DataRow, DataTable, RowId, TableId, Timeline, TimePoint, /// # }; /// # -/// # let table_id = TableId::random(); +/// # let table_id = TableId::new(); /// # /// # let timepoint = |frame_nr: i64, clock: i64| { /// # TimePoint::from([ @@ -291,7 +293,7 @@ re_tuid::delegate_arrow_tuid!(TableId as "rerun.controls.TableId"); /// let labels: &[MyLabel] = &[]; /// /// DataRow::from_cells3( -/// RowId::random(), +/// RowId::new(), /// "a", /// timepoint(1, 1), /// num_instances, @@ -303,7 +305,7 @@ re_tuid::delegate_arrow_tuid!(TableId as "rerun.controls.TableId"); /// let num_instances = 0; /// let colors: &[MyColor] = &[]; /// -/// DataRow::from_cells1(RowId::random(), "b", timepoint(1, 2), num_instances, colors).unwrap() +/// DataRow::from_cells1(RowId::new(), "b", timepoint(1, 2), num_instances, colors).unwrap() /// }; /// /// let row2 = { @@ -312,7 +314,7 @@ re_tuid::delegate_arrow_tuid!(TableId as "rerun.controls.TableId"); /// let labels: &[_] = &[MyLabel("hey".into())]; /// /// DataRow::from_cells2( -/// RowId::random(), +/// RowId::new(), /// "c", /// timepoint(2, 1), /// num_instances, @@ -1299,7 +1301,7 @@ impl DataTable { Time, }; - let table_id = TableId::random(); + let table_id = TableId::new(); let mut tick = 0i64; let mut timepoint = |frame_nr: i64| { @@ -1323,7 +1325,7 @@ impl DataTable { let labels: &[MyLabel] = &[]; DataRow::from_cells3( - RowId::random(), + RowId::new(), "a", timepoint(1), num_instances, @@ -1336,7 +1338,7 @@ impl DataTable { let num_instances = 0; let colors: &[MyColor] = &[]; - DataRow::from_cells1(RowId::random(), "b", timepoint(1), num_instances, colors).unwrap() + DataRow::from_cells1(RowId::new(), "b", timepoint(1), num_instances, colors).unwrap() }; let row2 = { @@ -1345,7 +1347,7 @@ impl DataTable { let labels: &[_] = &[MyLabel("hey".into())]; DataRow::from_cells2( - RowId::random(), + RowId::new(), "c", timepoint(2), num_instances, diff --git a/crates/re_log_types/src/data_table_batcher.rs b/crates/re_log_types/src/data_table_batcher.rs index dbf52ccd489c..89cf50e40e3e 100644 --- a/crates/re_log_types/src/data_table_batcher.rs +++ b/crates/re_log_types/src/data_table_batcher.rs @@ -415,7 +415,7 @@ fn batching_thread( re_log::trace!(reason, "flushing tables"); - let table = DataTable::from_rows(TableId::random(), rows.drain(..)); + let table = DataTable::from_rows(TableId::new(), rows.drain(..)); // TODO(#1981): efficient table sorting here, following the same rules as the store's. // table.sort(); diff --git a/crates/re_query/benches/query_benchmark.rs b/crates/re_query/benches/query_benchmark.rs index 27f0ff56bf6d..bb01848f9ac9 100644 --- a/crates/re_query/benches/query_benchmark.rs +++ b/crates/re_query/benches/query_benchmark.rs @@ -195,7 +195,7 @@ fn build_points_rows(paths: &[EntityPath], num_points: usize) -> Vec { .flat_map(move |frame_idx| { paths.iter().map(move |path| { let mut row = DataRow::from_cells2( - RowId::random(), + RowId::new(), path.clone(), [build_frame_nr((frame_idx as i64).into())], num_points as _, @@ -221,7 +221,7 @@ fn build_strings_rows(paths: &[EntityPath], num_strings: usize) -> Vec .flat_map(move |frame_idx| { paths.iter().map(move |path| { let mut row = DataRow::from_cells2( - RowId::random(), + RowId::new(), path.clone(), [build_frame_nr((frame_idx as i64).into())], num_strings as _, diff --git a/crates/re_query/src/query.rs b/crates/re_query/src/query.rs index f62db528bb2f..319ac4d3af0c 100644 --- a/crates/re_query/src/query.rs +++ b/crates/re_query/src/query.rs @@ -182,7 +182,7 @@ pub fn __populate_example_store() -> DataStore { let positions = vec![MyPoint::new(1.0, 2.0), MyPoint::new(3.0, 4.0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path, timepoint, instances.len() as _, @@ -195,7 +195,7 @@ pub fn __populate_example_store() -> DataStore { let colors = vec![MyColor::from(0xff000000)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path, timepoint, instances.len() as _, diff --git a/crates/re_query/tests/archetype_query_tests.rs b/crates/re_query/tests/archetype_query_tests.rs index 5121a03efb64..15e62c681b49 100644 --- a/crates/re_query/tests/archetype_query_tests.rs +++ b/crates/re_query/tests/archetype_query_tests.rs @@ -22,15 +22,14 @@ fn simple_query() { // Create some positions with implicit instances let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; - let row = - DataRow::from_cells1_sized(RowId::random(), ent_path, timepoint, 2, positions).unwrap(); + let row = DataRow::from_cells1_sized(RowId::new(), ent_path, timepoint, 2, positions).unwrap(); store.insert_row(&row).unwrap(); // Assign one of them a color with an explicit instance let color_instances = vec![InstanceKey(1)]; let colors = vec![Color::from_rgb(255, 0, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path, timepoint, 1, @@ -93,16 +92,14 @@ fn timeless_query() { // Create some positions with implicit instances let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; - let row = - DataRow::from_cells1_sized(RowId::random(), ent_path, timepoint, 2, positions).unwrap(); + let row = DataRow::from_cells1_sized(RowId::new(), ent_path, timepoint, 2, positions).unwrap(); store.insert_row(&row).unwrap(); // Assign one of them a color with an explicit instance.. timelessly! let color_instances = vec![InstanceKey(1)]; let colors = vec![Color::from_rgb(255, 0, 0)]; - let row = - DataRow::from_cells2_sized(RowId::random(), ent_path, [], 1, (color_instances, colors)) - .unwrap(); + let row = DataRow::from_cells2_sized(RowId::new(), ent_path, [], 1, (color_instances, colors)) + .unwrap(); store.insert_row(&row).unwrap(); // Retrieve the view @@ -159,13 +156,12 @@ fn no_instance_join_query() { // Create some positions with an implicit instance let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; - let row = - DataRow::from_cells1_sized(RowId::random(), ent_path, timepoint, 2, positions).unwrap(); + let row = DataRow::from_cells1_sized(RowId::new(), ent_path, timepoint, 2, positions).unwrap(); store.insert_row(&row).unwrap(); // Assign them colors with explicit instances let colors = vec![Color::from_rgb(255, 0, 0), Color::from_rgb(0, 255, 0)]; - let row = DataRow::from_cells1_sized(RowId::random(), ent_path, timepoint, 2, colors).unwrap(); + let row = DataRow::from_cells1_sized(RowId::new(), ent_path, timepoint, 2, colors).unwrap(); store.insert_row(&row).unwrap(); // Retrieve the view @@ -225,8 +221,7 @@ fn missing_column_join_query() { // Create some positions with an implicit instance let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; - let row = - DataRow::from_cells1_sized(RowId::random(), ent_path, timepoint, 2, positions).unwrap(); + let row = DataRow::from_cells1_sized(RowId::new(), ent_path, timepoint, 2, positions).unwrap(); store.insert_row(&row).unwrap(); // Retrieve the view @@ -282,15 +277,14 @@ fn splatted_query() { // Create some positions with implicit instances let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; - let row = - DataRow::from_cells1_sized(RowId::random(), ent_path, timepoint, 2, positions).unwrap(); + let row = DataRow::from_cells1_sized(RowId::new(), ent_path, timepoint, 2, positions).unwrap(); store.insert_row(&row).unwrap(); // Assign all of them a color via splat let color_instances = vec![InstanceKey::SPLAT]; let colors = vec![Color::from_rgb(255, 0, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path, timepoint, 1, diff --git a/crates/re_query/tests/archetype_range_tests.rs b/crates/re_query/tests/archetype_range_tests.rs index 0b6995867da4..ceb539181c6a 100644 --- a/crates/re_query/tests/archetype_range_tests.rs +++ b/crates/re_query/tests/archetype_range_tests.rs @@ -24,7 +24,7 @@ fn simple_range() { // Create some Positions with implicit instances let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; let row = - DataRow::from_cells1_sized(RowId::random(), ent_path.clone(), timepoint1, 2, positions) + DataRow::from_cells1_sized(RowId::new(), ent_path.clone(), timepoint1, 2, positions) .unwrap(); store.insert_row(&row).unwrap(); @@ -32,7 +32,7 @@ fn simple_range() { let color_instances = vec![InstanceKey(1)]; let colors = vec![Color::from_rgb(255, 0, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint1, 1, @@ -48,7 +48,7 @@ fn simple_range() { let color_instances = vec![InstanceKey(0)]; let colors = vec![Color::from_rgb(255, 0, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint2, 1, @@ -63,7 +63,7 @@ fn simple_range() { // Create some Positions with implicit instances let positions = vec![Position2D::new(10.0, 20.0), Position2D::new(30.0, 40.0)]; let row = - DataRow::from_cells1_sized(RowId::random(), ent_path.clone(), timepoint3, 2, positions) + DataRow::from_cells1_sized(RowId::new(), ent_path.clone(), timepoint3, 2, positions) .unwrap(); store.insert_row(&row).unwrap(); } @@ -252,21 +252,21 @@ fn timeless_range() { // Create some Positions with implicit instances let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; let mut row = - DataRow::from_cells1(RowId::random(), ent_path.clone(), timepoint1, 2, &positions) + DataRow::from_cells1(RowId::new(), ent_path.clone(), timepoint1, 2, &positions) .unwrap(); row.compute_all_size_bytes(); store.insert_row(&row).unwrap(); // Insert timelessly too! - let row = DataRow::from_cells1_sized(RowId::random(), ent_path.clone(), [], 2, &positions) - .unwrap(); + let row = + DataRow::from_cells1_sized(RowId::new(), ent_path.clone(), [], 2, &positions).unwrap(); store.insert_row(&row).unwrap(); // Assign one of them a color with an explicit instance let color_instances = vec![InstanceKey(1)]; let colors = vec![Color::from_rgb(255, 0, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint1, 1, @@ -277,7 +277,7 @@ fn timeless_range() { // Insert timelessly too! let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), [], 1, @@ -293,7 +293,7 @@ fn timeless_range() { let color_instances = vec![InstanceKey(0)]; let colors = vec![Color::from_rgb(255, 0, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint2, 1, @@ -304,7 +304,7 @@ fn timeless_range() { // Insert timelessly too! let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint2, 1, @@ -318,19 +318,14 @@ fn timeless_range() { { // Create some Positions with implicit instances let positions = vec![Position2D::new(10.0, 20.0), Position2D::new(30.0, 40.0)]; - let row = DataRow::from_cells1_sized( - RowId::random(), - ent_path.clone(), - timepoint3, - 2, - &positions, - ) - .unwrap(); + let row = + DataRow::from_cells1_sized(RowId::new(), ent_path.clone(), timepoint3, 2, &positions) + .unwrap(); store.insert_row(&row).unwrap(); // Insert timelessly too! - let row = DataRow::from_cells1_sized(RowId::random(), ent_path.clone(), [], 2, &positions) - .unwrap(); + let row = + DataRow::from_cells1_sized(RowId::new(), ent_path.clone(), [], 2, &positions).unwrap(); store.insert_row(&row).unwrap(); } @@ -705,7 +700,7 @@ fn simple_splatted_range() { // Create some Positions with implicit instances let positions = vec![Position2D::new(1.0, 2.0), Position2D::new(3.0, 4.0)]; let row = - DataRow::from_cells1_sized(RowId::random(), ent_path.clone(), timepoint1, 2, positions) + DataRow::from_cells1_sized(RowId::new(), ent_path.clone(), timepoint1, 2, positions) .unwrap(); store.insert_row(&row).unwrap(); @@ -713,7 +708,7 @@ fn simple_splatted_range() { let color_instances = vec![InstanceKey(1)]; let colors = vec![Color::from_rgb(255, 0, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint1, 1, @@ -729,7 +724,7 @@ fn simple_splatted_range() { let color_instances = vec![InstanceKey::SPLAT]; let colors = vec![Color::from_rgb(0, 255, 0)]; let row = DataRow::from_cells2_sized( - RowId::random(), + RowId::new(), ent_path.clone(), timepoint2, 1, @@ -744,7 +739,7 @@ fn simple_splatted_range() { // Create some Positions with implicit instances let positions = vec![Position2D::new(10.0, 20.0), Position2D::new(30.0, 40.0)]; let row = - DataRow::from_cells1_sized(RowId::random(), ent_path.clone(), timepoint3, 2, positions) + DataRow::from_cells1_sized(RowId::new(), ent_path.clone(), timepoint3, 2, positions) .unwrap(); store.insert_row(&row).unwrap(); } diff --git a/crates/re_sdk/src/recording_stream.rs b/crates/re_sdk/src/recording_stream.rs index 04b0f96a4786..20c7b7a743f8 100644 --- a/crates/re_sdk/src/recording_stream.rs +++ b/crates/re_sdk/src/recording_stream.rs @@ -588,7 +588,7 @@ impl RecordingStreamInner { ); sink.send( re_log_types::SetStoreInfo { - row_id: re_log_types::RowId::random(), + row_id: re_log_types::RowId::new(), info: info.clone(), } .into(), @@ -854,7 +854,7 @@ impl RecordingStream { None } else { Some(DataRow::from_cells( - RowId::random(), + RowId::new(), timepoint.clone(), ent_path.clone(), num_instances as _, @@ -868,7 +868,7 @@ impl RecordingStream { } else { splatted.push(DataCell::from_native([InstanceKey::SPLAT])); Some(DataRow::from_cells( - RowId::random(), + RowId::new(), timepoint, ent_path, 1, @@ -926,7 +926,7 @@ fn forwarding_thread( ); new_sink.send( re_log_types::SetStoreInfo { - row_id: re_log_types::RowId::random(), + row_id: re_log_types::RowId::new(), info: info.clone(), } .into(), diff --git a/crates/re_space_view/src/data_query_blueprint.rs b/crates/re_space_view/src/data_query_blueprint.rs index 5be1cdfe7f87..60ea2d80cc0f 100644 --- a/crates/re_space_view/src/data_query_blueprint.rs +++ b/crates/re_space_view/src/data_query_blueprint.rs @@ -113,7 +113,7 @@ impl DataQueryBlueprint { }; let row = DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), self.id.as_entity_path(), timepoint.clone(), 1, @@ -595,7 +595,7 @@ mod tests { // Set up a store DB with some entities for entity_path in ["parent", "parent/skipped/child1", "parent/skipped/child2"] { - let row_id = RowId::random(); + let row_id = RowId::new(); let point = MyPoint::new(1.0, 2.0); let row = DataRow::from_component_batches( row_id, diff --git a/crates/re_space_view_spatial/benches/bench_points.rs b/crates/re_space_view_spatial/benches/bench_points.rs index 5176e3e44c4b..87289bbbf491 100644 --- a/crates/re_space_view_spatial/benches/bench_points.rs +++ b/crates/re_space_view_spatial/benches/bench_points.rs @@ -45,7 +45,7 @@ fn bench_points(c: &mut criterion::Criterion) { let mut timepoint = TimePoint::default(); timepoint.insert(timeline, TimeInt::from_seconds(0)); let data_row = - DataRow::from_archetype(RowId::random(), timepoint, ent_path.clone(), &points).unwrap(); + DataRow::from_archetype(RowId::new(), timepoint, ent_path.clone(), &points).unwrap(); store.insert_row(&data_row).unwrap(); store }; diff --git a/crates/re_tuid/benches/bench_tuid.rs b/crates/re_tuid/benches/bench_tuid.rs index b37b92dcf4b6..d2c4e5784cf0 100644 --- a/crates/re_tuid/benches/bench_tuid.rs +++ b/crates/re_tuid/benches/bench_tuid.rs @@ -3,8 +3,8 @@ use criterion::{criterion_group, criterion_main, Criterion}; fn bench_tuid(c: &mut Criterion) { let mut group = c.benchmark_group("tuid"); group.throughput(criterion::Throughput::Elements(1)); - group.bench_function("Tuid::random", |b| { - b.iter(|| criterion::black_box(re_tuid::Tuid::random())); + group.bench_function("Tuid::new", |b| { + b.iter(|| criterion::black_box(re_tuid::Tuid::new())); }); } @@ -18,7 +18,7 @@ fn bench_arrow(c: &mut Criterion) { let mut group = c.benchmark_group(format!("arrow/serialize/elem_count={elem_count}")); group.throughput(criterion::Throughput::Elements(elem_count)); - let tuids = vec![re_tuid::Tuid::random(); elem_count as usize]; + let tuids = vec![re_tuid::Tuid::new(); elem_count as usize]; group.bench_function("arrow2", |b| { b.iter(|| { @@ -33,8 +33,7 @@ fn bench_arrow(c: &mut Criterion) { group.throughput(criterion::Throughput::Elements(elem_count)); let data: Box = - re_tuid::Tuid::to_arrow(vec![re_tuid::Tuid::random(); elem_count as usize]) - .unwrap(); + re_tuid::Tuid::to_arrow(vec![re_tuid::Tuid::new(); elem_count as usize]).unwrap(); group.bench_function("arrow2", |b| { b.iter(|| { diff --git a/crates/re_tuid/src/lib.rs b/crates/re_tuid/src/lib.rs index fb89b2dd1207..0cef44c97e58 100644 --- a/crates/re_tuid/src/lib.rs +++ b/crates/re_tuid/src/lib.rs @@ -47,8 +47,10 @@ impl Tuid { inc: u64::MAX, }; + /// Create a new unique [`Tuid`] based on the current time. + #[allow(clippy::new_without_default)] #[inline] - pub fn random() -> Self { + pub fn new() -> Self { use std::cell::RefCell; thread_local! { @@ -89,7 +91,7 @@ impl Tuid { /// Wraps the monotonically increasing back to zero on overflow. /// /// Beware: wrong usage can easily lead to conflicts. - /// Prefer [`Tuid::random`] when unsure. + /// Prefer [`Tuid::new`] when unsure. #[inline] pub fn next(&self) -> Self { let Self { time_ns, inc } = *self; @@ -106,7 +108,7 @@ impl Tuid { /// Wraps the monotonically increasing back to zero on overflow. /// /// Beware: wrong usage can easily lead to conflicts. - /// Prefer [`Tuid::random`] when unsure. + /// Prefer [`Tuid::new`] when unsure. #[inline] pub fn increment(&self, n: u64) -> Self { let Self { time_ns, inc } = *self; @@ -181,7 +183,7 @@ fn test_tuid() { } let num = 100_000; - let ids: Vec = (0..num).map(|_| Tuid::random()).collect(); + let ids: Vec = (0..num).map(|_| Tuid::new()).collect(); assert!(is_sorted(&ids)); assert_eq!(ids.iter().cloned().collect::>().len(), num); assert_eq!(ids.iter().cloned().collect::>().len(), num); diff --git a/crates/re_viewer/src/app_blueprint.rs b/crates/re_viewer/src/app_blueprint.rs index 6f659dba087c..e098d0c102d3 100644 --- a/crates/re_viewer/src/app_blueprint.rs +++ b/crates/re_viewer/src/app_blueprint.rs @@ -90,9 +90,8 @@ pub fn setup_welcome_screen_blueprint(welcome_screen_blueprint: &mut StoreDb) { let component = PanelView { is_expanded }; - let row = - DataRow::from_cells1_sized(RowId::random(), entity_path, timepoint, 1, [component]) - .unwrap(); // Can only fail if we have the wrong number of instances for the component, and we don't + let row = DataRow::from_cells1_sized(RowId::new(), entity_path, timepoint, 1, [component]) + .unwrap(); // Can only fail if we have the wrong number of instances for the component, and we don't welcome_screen_blueprint.add_data_row(row).unwrap(); // Can only fail if we have the wrong number of instances for the component, and we don't } @@ -115,7 +114,7 @@ impl<'a> AppBlueprint<'a> { let component = PanelView { is_expanded }; let row = - DataRow::from_cells1_sized(RowId::random(), entity_path, timepoint, 1, [component]) + DataRow::from_cells1_sized(RowId::new(), entity_path, timepoint, 1, [component]) .unwrap(); // Can only fail if we have the wrong number of instances for the component, and we don't command_sender.send_system(SystemCommand::UpdateBlueprint( diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index b3e95af5bf3a..414a7d925ff6 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -576,7 +576,7 @@ impl StoreBundle { let mut blueprint_db = StoreDb::new(id.clone()); blueprint_db.set_store_info(re_log_types::SetStoreInfo { - row_id: re_log_types::RowId::random(), + row_id: re_log_types::RowId::new(), info: re_log_types::StoreInfo { application_id: id.as_str().into(), store_id: id.clone(), diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 2b07eb8e7eb5..a6aaf59b0021 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -549,7 +549,7 @@ fn blueprint_ui( }; let row = DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), query.id.as_entity_path(), timepoint, 1, diff --git a/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs b/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs index 5d2f912e3cc1..e1a6aef30c31 100644 --- a/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs +++ b/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs @@ -348,7 +348,7 @@ fn open_markdown_recording( .with_media_type(re_types::components::MediaType::markdown()); let row = DataRow::from_archetype( - RowId::random(), + RowId::new(), TimePoint::timeless(), EntityPath::from(entity_path), &text_doc, diff --git a/crates/re_viewer_context/src/space_view/view_query.rs b/crates/re_viewer_context/src/space_view/view_query.rs index a472e3cc8f0c..9b7e0283f358 100644 --- a/crates/re_viewer_context/src/space_view/view_query.rs +++ b/crates/re_viewer_context/src/space_view/view_query.rs @@ -90,7 +90,7 @@ impl DataResult { }; let row = DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), self.override_path.clone(), TimePoint::timeless(), 1, diff --git a/crates/re_viewport/src/space_view.rs b/crates/re_viewport/src/space_view.rs index a1644aff5fb6..509c7daa719a 100644 --- a/crates/re_viewport/src/space_view.rs +++ b/crates/re_viewport/src/space_view.rs @@ -424,7 +424,7 @@ mod tests { fn save_override(props: EntityProperties, path: &EntityPath, store: &mut StoreDb) { let component = EntityPropertiesComponent { props }; let row = DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), path.clone(), TimePoint::timeless(), 1, @@ -447,9 +447,8 @@ mod tests { "parent/skip/child1".into(), "parent/skip/child2".into(), ] { - let row = - DataRow::from_archetype(RowId::random(), TimePoint::timeless(), path, &points) - .unwrap(); + let row = DataRow::from_archetype(RowId::new(), TimePoint::timeless(), path, &points) + .unwrap(); recording.add_data_row(row).ok(); } diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index 5971796aaaed..4ef1c6c9cdd2 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -333,7 +333,7 @@ fn add_delta_from_single_component<'a, C>( std::borrow::Cow<'a, C>: std::convert::From, { let row = DataRow::from_cells1_sized( - RowId::random(), + RowId::new(), entity_path.clone(), timepoint.clone(), 1, @@ -464,7 +464,7 @@ pub fn clear_space_view(deltas: &mut Vec, space_view_id: &SpaceViewId) let timepoint = TimePoint::timeless(); if let Ok(row) = DataRow::from_component_batches( - RowId::random(), + RowId::new(), timepoint, space_view_id.as_entity_path(), Clear::recursive() diff --git a/crates/rerun_c/src/lib.rs b/crates/rerun_c/src/lib.rs index 33667d5dff88..e0c608c34f36 100644 --- a/crates/rerun_c/src/lib.rs +++ b/crates/rerun_c/src/lib.rs @@ -649,7 +649,7 @@ fn rr_log_impl( } let data_row = DataRow::from_cells( - re_sdk::log::RowId::random(), + re_sdk::log::RowId::new(), TimePoint::default(), // we use the one in the recording stream for now entity_path, num_instances, diff --git a/rerun_py/src/arrow.rs b/rerun_py/src/arrow.rs index 421a54734b01..b52da072fdc4 100644 --- a/rerun_py/src/arrow.rs +++ b/rerun_py/src/arrow.rs @@ -66,7 +66,7 @@ pub fn build_data_row_from_components( let num_instances = cells.first().map_or(0, |cell| cell.num_instances()); let row = DataRow::from_cells( - RowId::random(), + RowId::new(), time_point.clone(), entity_path.clone(), num_instances, diff --git a/rerun_py/src/python_bridge.rs b/rerun_py/src/python_bridge.rs index 456a2efba6db..95c7f796cef9 100644 --- a/rerun_py/src/python_bridge.rs +++ b/rerun_py/src/python_bridge.rs @@ -748,7 +748,7 @@ fn set_panel(entity_path: &str, is_expanded: bool, blueprint: Option<&PyRecordin let panel_state = PanelView { is_expanded }; let row = DataRow::from_cells1( - RowId::random(), + RowId::new(), entity_path, TimePoint::default(), 1, @@ -794,7 +794,7 @@ fn add_space_view( let space_view = SpaceViewComponent { space_view }; let row = DataRow::from_cells1( - RowId::random(), + RowId::new(), entity_path, TimePoint::default(), 1, @@ -820,7 +820,7 @@ fn set_auto_space_views(enabled: bool, blueprint: Option<&PyRecordingStream>) { let enable_auto_space = AutoSpaceViews(enabled); let row = DataRow::from_cells1( - RowId::random(), + RowId::new(), VIEWPORT_PATH, TimePoint::default(), 1, From 556c0b22f06d1cc0a1908ba3b3040cf0debec4c0 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 6 Dec 2023 14:24:33 +0100 Subject: [PATCH 3/4] Show connection status in top bar (#4443) ### What * Closes https://github.com/rerun-io/rerun/issues/3046 This replaces the loading screen with a connection status that's always shown in the top panel. image ### 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): * Full build: [app.rerun.io](https://app.rerun.io/pr/4443/index.html) * Partial build: [app.rerun.io](https://app.rerun.io/pr/4443/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) - Useful for quick testing when changes do not affect examples in any way * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/4443) - [Docs preview](https://rerun.io/preview/426dea12d09df75a35829394c210a346d6de8e6c/docs) - [Examples preview](https://rerun.io/preview/426dea12d09df75a35829394c210a346d6de8e6c/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- crates/re_viewer/src/app.rs | 22 +- crates/re_viewer/src/ui/mod.rs | 2 +- crates/re_viewer/src/ui/recordings_panel.rs | 2 +- crates/re_viewer/src/ui/top_panel.rs | 201 +++++++++++++----- crates/re_viewer/src/ui/welcome_screen/mod.rs | 102 +-------- .../src/ui/welcome_screen/welcome_page.rs | 20 +- 6 files changed, 168 insertions(+), 181 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 3d03edb6d6dc..23800a15bc2e 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -678,7 +678,7 @@ impl App { crate::ui::mobile_warning_ui(&self.re_ui, ui); - crate::ui::top_panel(app_blueprint, store_context, ui, self, gpu_resource_stats); + crate::ui::top_panel(self, app_blueprint, store_context, gpu_resource_stats, ui); self.memory_panel_ui(ui, gpu_resource_stats, store_stats); @@ -731,10 +731,11 @@ impl App { render_ctx.before_submit(); } } else { - // This is part of the loading vs. welcome screen UI logic. The loading screen - // is displayed when no app ID is set. This is e.g. the initial state for the - // web demos. - crate::ui::loading_ui(ui, &self.rx); + // There's nothing to show. + // We get here when + // A) there is nothing loaded + // B) we decided not to show the welcome screen, presumably because data is expected at any time now. + // The user can see the connection status in the top bar. } }); } @@ -941,12 +942,13 @@ impl App { } } - /// This function will create an empty blueprint whenever the welcome screen should be - /// displayed. - /// - /// The welcome screen can be displayed only when a blueprint is available (and no recording is - /// loaded). This function implements the heuristic which determines when the welcome screen + /// This function implements a heuristic which determines when the welcome screen /// should show up. + /// + /// Why not always show it when no data is loaded? + /// Because sometimes we expect data to arrive at any moment, + /// and showing the wlecome screen for a few frames will just be an annoying flash + /// in the users face. fn should_show_welcome_screen(&mut self, store_hub: &StoreHub) -> bool { // Don't show the welcome screen if we have actual data to display. if store_hub.current_recording().is_some() || store_hub.selected_application_id().is_some() diff --git a/crates/re_viewer/src/ui/mod.rs b/crates/re_viewer/src/ui/mod.rs index 075eb1d4aafc..53eeb8b238fd 100644 --- a/crates/re_viewer/src/ui/mod.rs +++ b/crates/re_viewer/src/ui/mod.rs @@ -17,5 +17,5 @@ pub use recordings_panel::recordings_panel_ui; pub(crate) use { self::mobile_warning_ui::mobile_warning_ui, self::top_panel::top_panel, - self::welcome_screen::loading_ui, self::welcome_screen::WelcomeScreen, + self::welcome_screen::WelcomeScreen, }; diff --git a/crates/re_viewer/src/ui/recordings_panel.rs b/crates/re_viewer/src/ui/recordings_panel.rs index 42dc8abbe761..876b7751165e 100644 --- a/crates/re_viewer/src/ui/recordings_panel.rs +++ b/crates/re_viewer/src/ui/recordings_panel.rs @@ -69,7 +69,7 @@ fn loading_receivers_ui( | SmartChannelSource::Sdk | SmartChannelSource::WsClient { .. } | SmartChannelSource::TcpServer { .. } => { - // TODO(#3046): show these in status bar + // These show up in the top panel - see `top_panel.rs`. continue; } }; diff --git a/crates/re_viewer/src/ui/top_panel.rs b/crates/re_viewer/src/ui/top_panel.rs index 660895ecec39..dfc2eae5e1d1 100644 --- a/crates/re_viewer/src/ui/top_panel.rs +++ b/crates/re_viewer/src/ui/top_panel.rs @@ -1,17 +1,19 @@ use egui::NumExt as _; +use itertools::Itertools; use re_format::format_number; use re_renderer::WgpuResourcePoolStatistics; +use re_smart_channel::{ReceiveSet, SmartChannelSource}; use re_ui::UICommand; use re_viewer_context::StoreContext; use crate::{app_blueprint::AppBlueprint, App}; pub fn top_panel( + app: &mut App, app_blueprint: &AppBlueprint<'_>, store_context: Option<&StoreContext<'_>>, - ui: &mut egui::Ui, - app: &mut App, gpu_resource_stats: &WgpuResourcePoolStatistics, + ui: &mut egui::Ui, ) { re_tracing::profile_function!(); @@ -26,10 +28,11 @@ pub fn top_panel( ui.set_height(top_bar_style.height); ui.add_space(top_bar_style.indent); - top_bar_ui(app_blueprint, store_context, ui, app, gpu_resource_stats); + top_bar_ui(app, app_blueprint, store_context, ui, gpu_resource_stats); }) .response; + // React to dragging and double-clicking the top bar: #[cfg(not(target_arch = "wasm32"))] if !re_ui::NATIVE_WINDOW_BAR { let title_bar_response = _response.interact(egui::Sense::click()); @@ -45,10 +48,10 @@ pub fn top_panel( } fn top_bar_ui( + app: &mut App, app_blueprint: &AppBlueprint<'_>, store_context: Option<&StoreContext<'_>>, ui: &mut egui::Ui, - app: &mut App, gpu_resource_stats: &WgpuResourcePoolStatistics, ) { app.rerun_menu_button_ui(store_context, ui); @@ -76,58 +79,11 @@ fn top_bar_ui( ui.add_space(extra_margin); } - let mut selection_panel_expanded = app_blueprint.selection_panel_expanded; - if app - .re_ui() - .medium_icon_toggle_button( - ui, - &re_ui::icons::RIGHT_PANEL_TOGGLE, - &mut selection_panel_expanded, - ) - .on_hover_text(format!( - "Toggle Selection View{}", - UICommand::ToggleSelectionPanel.format_shortcut_tooltip_suffix(ui.ctx()) - )) - .clicked() - { - app_blueprint.toggle_selection_panel(&app.command_sender); - } - - let mut time_panel_expanded = app_blueprint.time_panel_expanded; - if app - .re_ui() - .medium_icon_toggle_button( - ui, - &re_ui::icons::BOTTOM_PANEL_TOGGLE, - &mut time_panel_expanded, - ) - .on_hover_text(format!( - "Toggle Timeline View{}", - UICommand::ToggleTimePanel.format_shortcut_tooltip_suffix(ui.ctx()) - )) - .clicked() - { - app_blueprint.toggle_time_panel(&app.command_sender); - } + panel_buttons_r2l(app, app_blueprint, ui); - let mut blueprint_panel_expanded = app_blueprint.blueprint_panel_expanded; - if app - .re_ui() - .medium_icon_toggle_button( - ui, - &re_ui::icons::LEFT_PANEL_TOGGLE, - &mut blueprint_panel_expanded, - ) - .on_hover_text(format!( - "Toggle Blueprint View{}", - UICommand::ToggleBlueprintPanel.format_shortcut_tooltip_suffix(ui.ctx()) - )) - .clicked() - { - app_blueprint.toggle_blueprint_panel(&app.command_sender); - } + connection_status_ui(ui, app.msg_receive_set()); - if cfg!(debug_assertions) && app.app_options().show_metrics { + if cfg!(debug_assertions) { ui.vertical_centered(|ui| { ui.style_mut().wrap = Some(false); ui.add_space(6.0); // TODO(emilk): in egui, add a proper way of centering a single widget in a UI. @@ -137,6 +93,143 @@ fn top_bar_ui( }); } +fn connection_status_ui(ui: &mut egui::Ui, rx: &ReceiveSet) { + let sources = rx + .sources() + .into_iter() + .filter(|source| { + match source.as_ref() { + SmartChannelSource::File(_) | SmartChannelSource::RrdHttpStream { .. } => { + false // These show up in the recordings panel as a "Loading…" in `recordings_panel.rs` + } + + re_smart_channel::SmartChannelSource::RrdWebEventListener + | re_smart_channel::SmartChannelSource::Sdk + | re_smart_channel::SmartChannelSource::WsClient { .. } + | re_smart_channel::SmartChannelSource::TcpServer { .. } => true, + } + }) + .collect_vec(); + + match sources.len() { + 0 => return, + 1 => { + source_label(ui, sources[0].as_ref()); + } + n => { + // In practice we never get here + ui.label(format!("{n} sources connected")) + .on_hover_ui(|ui| { + ui.vertical(|ui| { + for source in &sources { + source_label(ui, source.as_ref()); + } + }); + }); + } + } + + fn source_label(ui: &mut egui::Ui, source: &SmartChannelSource) -> egui::Response { + let response = ui.label(status_string(source)); + + let tooltip = match source { + SmartChannelSource::File(_) + | SmartChannelSource::RrdHttpStream { .. } + | SmartChannelSource::RrdWebEventListener + | SmartChannelSource::Sdk + | SmartChannelSource::WsClient { .. } => None, + + SmartChannelSource::TcpServer { .. } => { + Some("Waiting for an SDK to connect".to_owned()) + } + }; + + if let Some(tooltip) = tooltip { + response.on_hover_text(tooltip) + } else { + response + } + } + + fn status_string(source: &SmartChannelSource) -> String { + match source { + re_smart_channel::SmartChannelSource::File(path) => { + format!("Loading {}…", path.display()) + } + re_smart_channel::SmartChannelSource::RrdHttpStream { url } => { + format!("Loading {url}…") + } + re_smart_channel::SmartChannelSource::RrdWebEventListener => { + "Waiting for logging data…".to_owned() + } + re_smart_channel::SmartChannelSource::Sdk => { + "Waiting for logging data from SDK".to_owned() + } + re_smart_channel::SmartChannelSource::WsClient { ws_server_url } => { + // TODO(emilk): it would be even better to know whether or not we are connected, or are attempting to connect + format!("Waiting for data from {ws_server_url}") + } + re_smart_channel::SmartChannelSource::TcpServer { port } => { + format!("Listening on TCP port {port}") + } + } + } +} + +/// Lay out the panel button right-to-left +fn panel_buttons_r2l(app: &App, app_blueprint: &AppBlueprint<'_>, ui: &mut egui::Ui) { + let mut selection_panel_expanded = app_blueprint.selection_panel_expanded; + if app + .re_ui() + .medium_icon_toggle_button( + ui, + &re_ui::icons::RIGHT_PANEL_TOGGLE, + &mut selection_panel_expanded, + ) + .on_hover_text(format!( + "Toggle Selection View{}", + UICommand::ToggleSelectionPanel.format_shortcut_tooltip_suffix(ui.ctx()) + )) + .clicked() + { + app_blueprint.toggle_selection_panel(&app.command_sender); + } + + let mut time_panel_expanded = app_blueprint.time_panel_expanded; + if app + .re_ui() + .medium_icon_toggle_button( + ui, + &re_ui::icons::BOTTOM_PANEL_TOGGLE, + &mut time_panel_expanded, + ) + .on_hover_text(format!( + "Toggle Timeline View{}", + UICommand::ToggleTimePanel.format_shortcut_tooltip_suffix(ui.ctx()) + )) + .clicked() + { + app_blueprint.toggle_time_panel(&app.command_sender); + } + + let mut blueprint_panel_expanded = app_blueprint.blueprint_panel_expanded; + if app + .re_ui() + .medium_icon_toggle_button( + ui, + &re_ui::icons::LEFT_PANEL_TOGGLE, + &mut blueprint_panel_expanded, + ) + .on_hover_text(format!( + "Toggle Blueprint View{}", + UICommand::ToggleBlueprintPanel.format_shortcut_tooltip_suffix(ui.ctx()) + )) + .clicked() + { + app_blueprint.toggle_blueprint_panel(&app.command_sender); + } +} + /// Shows clickable website link as an image (text doesn't look as nice) fn website_link_ui(ui: &mut egui::Ui) { let desired_height = ui.max_rect().height(); diff --git a/crates/re_viewer/src/ui/welcome_screen/mod.rs b/crates/re_viewer/src/ui/welcome_screen/mod.rs index f2391cd16a77..319b0fd8dfa7 100644 --- a/crates/re_viewer/src/ui/welcome_screen/mod.rs +++ b/crates/re_viewer/src/ui/welcome_screen/mod.rs @@ -1,12 +1,14 @@ mod example_page; mod welcome_page; +use std::hash::Hash; + use egui::Widget; +use welcome_page::welcome_page_ui; + use re_log_types::LogMsg; -use re_smart_channel::{ReceiveSet, SmartChannelSource}; +use re_smart_channel::ReceiveSet; use re_ui::ReUi; -use std::hash::Hash; -use welcome_page::welcome_page_ui; #[derive(Debug, Default, PartialEq, Hash)] enum WelcomeScreenPage { @@ -104,39 +106,6 @@ impl WelcomeScreen { } } -/// Full-screen UI shown while in loading state. -pub fn loading_ui(ui: &mut egui::Ui, rx: &ReceiveSet) { - let status_strings = status_strings(rx); - if status_strings.is_empty() { - return; - } - - ui.centered_and_justified(|ui| { - for status_string in status_strings { - let style = ui.style(); - let mut layout_job = egui::text::LayoutJob::default(); - layout_job.append( - status_string.status, - 0.0, - egui::TextFormat::simple( - egui::TextStyle::Heading.resolve(style), - style.visuals.strong_text_color(), - ), - ); - layout_job.append( - &format!("\n\n{}", status_string.source), - 0.0, - egui::TextFormat::simple( - egui::TextStyle::Body.resolve(style), - style.visuals.text_color(), - ), - ); - layout_job.halign = egui::Align::Center; - ui.label(layout_job); - } - }); -} - fn set_large_button_style(ui: &mut egui::Ui) { ui.style_mut().spacing.button_padding = egui::vec2(10.0, 7.0); let visuals = ui.visuals_mut(); @@ -182,64 +151,3 @@ fn large_text_button(ui: &mut egui::Ui, text: impl Into) -> eg }) .inner } - -/// Describes the current state of the Rerun viewer. -struct StatusString { - /// General status string (e.g. "Ready", "Loading…", etc.). - status: &'static str, - - /// Source string (e.g. listening IP, file path, etc.). - source: String, - - /// Whether or not the status is valid once data loading is completed, i.e. if data may still - /// be received later. - long_term: bool, -} - -impl StatusString { - fn new(status: &'static str, source: String, long_term: bool) -> Self { - Self { - status, - source, - long_term, - } - } -} - -/// Returns the status strings to be displayed by the loading and welcome screen. -fn status_strings(rx: &ReceiveSet) -> Vec { - rx.sources() - .into_iter() - .map(|s| status_string(&s)) - .collect() -} - -fn status_string(source: &SmartChannelSource) -> StatusString { - match source { - re_smart_channel::SmartChannelSource::File(path) => { - StatusString::new("Loading…", path.display().to_string(), false) - } - re_smart_channel::SmartChannelSource::RrdHttpStream { url } => { - StatusString::new("Loading…", url.clone(), false) - } - re_smart_channel::SmartChannelSource::RrdWebEventListener => { - StatusString::new("Ready", "Waiting for logging data…".to_owned(), true) - } - re_smart_channel::SmartChannelSource::Sdk => StatusString::new( - "Ready", - "Waiting for logging data from SDK".to_owned(), - true, - ), - re_smart_channel::SmartChannelSource::WsClient { ws_server_url } => { - // TODO(emilk): it would be even better to know whether or not we are connected, or are attempting to connect - StatusString::new( - "Ready", - format!("Waiting for data from {ws_server_url}"), - true, - ) - } - re_smart_channel::SmartChannelSource::TcpServer { port } => { - StatusString::new("Ready", format!("Listening on port {port}"), true) - } - } -} diff --git a/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs b/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs index e1a6aef30c31..06472c53ae4f 100644 --- a/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs +++ b/crates/re_viewer/src/ui/welcome_screen/welcome_page.rs @@ -1,4 +1,4 @@ -use super::{large_text_button, status_strings, url_large_text_button, WelcomeScreenResponse}; +use super::{large_text_button, url_large_text_button, WelcomeScreenResponse}; use egui::{NumExt, Ui}; use re_data_store::StoreDb; use re_log_types::{ @@ -46,23 +46,7 @@ pub(super) fn welcome_page_ui( ) -> WelcomeScreenResponse { ui.vertical(|ui| { let accepts_connections = rx.accepts_tcp_connections(); - - let show_example = onboarding_content_ui(ui, command_sender, accepts_connections); - - for status_strings in status_strings(rx) { - if status_strings.long_term { - ui.add_space(55.0); - ui.vertical_centered(|ui| { - ui.label(status_strings.status); - ui.label( - egui::RichText::new(status_strings.source) - .color(ui.visuals().weak_text_color()), - ); - }); - } - } - - show_example + onboarding_content_ui(ui, command_sender, accepts_connections) }) .inner } From fa25e536abdb96516eb3269b2f49cfaa9dde39ae Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 6 Dec 2023 15:38:04 +0100 Subject: [PATCH 4/4] RenderContext usage cleanup (#4446) ### What Some cleanup related to recent interior mutability changes: In a lot of places we can now pass just the RenderContext instead of several arguments, making a lot of code less complex. Also, "shared renderer data" is no longer needed either and is flattened out into the RenderContext. ### 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): * Full build: [app.rerun.io](https://app.rerun.io/pr/4446/index.html) * Partial build: [app.rerun.io](https://app.rerun.io/pr/4446/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) - Useful for quick testing when changes do not affect examples in any way * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/4446) - [Docs preview](https://rerun.io/preview/032f676245da9bbb08d2f6db1dd7ee33ccd09428/docs) - [Examples preview](https://rerun.io/preview/032f676245da9bbb08d2f6db1dd7ee33ccd09428/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- crates/re_renderer/src/context.rs | 61 +++++----------- .../re_renderer/src/draw_phases/outlines.rs | 38 ++++------ .../src/draw_phases/picking_layer.rs | 26 ++----- crates/re_renderer/src/renderer/compositor.rs | 52 ++++++-------- .../re_renderer/src/renderer/debug_overlay.rs | 38 ++++------ .../re_renderer/src/renderer/depth_cloud.rs | 54 +++++---------- .../src/renderer/generic_skybox.rs | 32 +++------ crates/re_renderer/src/renderer/lines.rs | 65 +++++++---------- .../re_renderer/src/renderer/mesh_renderer.rs | 54 +++++---------- crates/re_renderer/src/renderer/mod.rs | 23 ++----- .../re_renderer/src/renderer/point_cloud.rs | 69 +++++++------------ crates/re_renderer/src/renderer/rectangles.rs | 59 ++++++---------- .../re_renderer/src/renderer/test_triangle.rs | 32 +++------ .../src/resource_managers/mesh_manager.rs | 7 +- crates/re_renderer/src/view_builder.rs | 4 +- .../wgpu_resources/pipeline_layout_pool.rs | 28 ++++---- .../wgpu_resources/render_pipeline_pool.rs | 14 ++-- .../src/wgpu_resources/shader_module_pool.rs | 13 ++-- 18 files changed, 244 insertions(+), 425 deletions(-) diff --git a/crates/re_renderer/src/context.rs b/crates/re_renderer/src/context.rs index 18a33bfdbeb5..6a8cc4f13231 100644 --- a/crates/re_renderer/src/context.rs +++ b/crates/re_renderer/src/context.rs @@ -10,7 +10,7 @@ use crate::{ renderer::Renderer, resource_managers::{MeshManager, TextureManager2D}, wgpu_resources::{GpuRenderPipelinePoolMoveAccessor, WgpuResourcePools}, - FileResolver, FileServer, FileSystem, RecommendedFileResolver, + FileServer, RecommendedFileResolver, }; /// Any resource involving wgpu rendering which can be re-used across different scenes. @@ -19,7 +19,12 @@ pub struct RenderContext { pub device: Arc, pub queue: Arc, - pub(crate) shared_renderer_data: SharedRendererData, + pub(crate) config: RenderContextConfig, + + /// Global bindings, always bound to 0 bind group slot zero. + /// [`Renderer`] are not allowed to use bind group 0 themselves! + pub(crate) global_bindings: GlobalBindings, + renderers: RwLock, pub(crate) resolver: RecommendedFileResolver, #[cfg(all(not(target_arch = "wasm32"), debug_assertions))] // native debug build @@ -41,15 +46,6 @@ pub struct RenderContext { pub gpu_resources: WgpuResourcePools, // Last due to drop order. } -/// Immutable data that is shared between all [`Renderer`] -pub struct SharedRendererData { - pub(crate) config: RenderContextConfig, - - /// Global bindings, always bound to 0 bind group slot zero. - /// [`Renderer`] are not allowed to use bind group 0 themselves! - pub(crate) global_bindings: GlobalBindings, -} - /// Struct owning *all* [`Renderer`]. /// [`Renderer`] are created lazily and stay around indefinitely. pub(crate) struct Renderers { @@ -57,16 +53,13 @@ pub(crate) struct Renderers { } impl Renderers { - pub fn get_or_create( + pub fn get_or_create( &mut self, - shared_data: &SharedRendererData, - resource_pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, + ctx: &RenderContext, ) -> &R { self.renderers.entry().or_insert_with(|| { re_tracing::profile_scope!("create_renderer", std::any::type_name::()); - R::create_renderer(shared_data, resource_pools, device, resolver) + R::create_renderer(ctx) }) } @@ -165,22 +158,8 @@ impl RenderContext { err_tracker }; - let shared_renderer_data = SharedRendererData { - config, - global_bindings, - }; - let resolver = crate::new_recommended_file_resolver(); - let mut renderers = RwLock::new(Renderers { - renderers: TypeMap::new(), - }); - - let mesh_manager = RwLock::new(MeshManager::new(renderers.get_mut().get_or_create( - &shared_renderer_data, - &gpu_resources, - &device, - &resolver, - ))); + let mesh_manager = RwLock::new(MeshManager::new()); let texture_manager_2d = TextureManager2D::new(device.clone(), queue.clone(), &gpu_resources.textures); @@ -209,9 +188,12 @@ impl RenderContext { device, queue, - shared_renderer_data, + config, + global_bindings, - renderers, + renderers: RwLock::new(Renderers { + renderers: TypeMap::new(), + }), gpu_resources, @@ -250,9 +232,7 @@ impl RenderContext { // knowing that we're not _actually_ blocking. // // For more details check https://github.com/gfx-rs/wgpu/issues/3601 - if cfg!(target_arch = "wasm32") - && self.shared_renderer_data.config.device_caps.tier == DeviceTier::Gles - { + if cfg!(target_arch = "wasm32") && self.config.device_caps.tier == DeviceTier::Gles { self.device.poll(wgpu::Maintain::Wait); return; } @@ -405,12 +385,7 @@ impl RenderContext { // If it wasn't there we have to add it. // This path is rare since it happens only once per renderer type in the lifetime of the ctx. // (we don't discard renderers ever) - self.renderers.write().get_or_create::<_, R>( - &self.shared_renderer_data, - &self.gpu_resources, - &self.device, - &self.resolver, - ); + self.renderers.write().get_or_create::(self); // Release write lock again and only take a read lock. // safe to unwrap since we just created it and nobody removes elements from the renderer. diff --git a/crates/re_renderer/src/draw_phases/outlines.rs b/crates/re_renderer/src/draw_phases/outlines.rs index 615519b7edce..a2bd6ebfecd9 100644 --- a/crates/re_renderer/src/draw_phases/outlines.rs +++ b/crates/re_renderer/src/draw_phases/outlines.rs @@ -208,8 +208,7 @@ impl OutlineMaskProcessor { // ------------- Textures ------------- let texture_pool = &ctx.gpu_resources.textures; - let mask_sample_count = - Self::mask_sample_count(&ctx.shared_renderer_data.config.device_caps); + let mask_sample_count = Self::mask_sample_count(&ctx.config.device_caps); let mask_texture_desc = crate::wgpu_resources::TextureDesc { label: format!("{instance_label}::mask_texture").into(), size: wgpu::Extent3d { @@ -264,8 +263,7 @@ impl OutlineMaskProcessor { // ------------- Render Pipelines ------------- - let screen_triangle_vertex_shader = - screen_triangle_vertex_shader(&ctx.gpu_resources, &ctx.device, &ctx.resolver); + let screen_triangle_vertex_shader = screen_triangle_vertex_shader(ctx); let jumpflooding_init_shader_module = if mask_sample_count == 1 { include_shader_module!("../../shader/outlines/jumpflooding_init.wgsl") } else { @@ -274,54 +272,46 @@ impl OutlineMaskProcessor { let jumpflooding_init_desc = RenderPipelineDesc { label: "OutlineMaskProcessor::jumpflooding_init".into(), pipeline_layout: ctx.gpu_resources.pipeline_layouts.get_or_create( - &ctx.device, + ctx, &PipelineLayoutDesc { label: "OutlineMaskProcessor::jumpflooding_init".into(), entries: vec![bind_group_layout_jumpflooding_init], }, - &ctx.gpu_resources.bind_group_layouts, ), vertex_entrypoint: "main".into(), vertex_handle: screen_triangle_vertex_shader, fragment_entrypoint: "main".into(), - fragment_handle: ctx.gpu_resources.shader_modules.get_or_create( - &ctx.device, - &ctx.resolver, - &jumpflooding_init_shader_module, - ), + fragment_handle: ctx + .gpu_resources + .shader_modules + .get_or_create(ctx, &jumpflooding_init_shader_module), vertex_buffers: smallvec![], render_targets: smallvec![Some(Self::VORONOI_FORMAT.into())], primitive: wgpu::PrimitiveState::default(), depth_stencil: None, multisample: wgpu::MultisampleState::default(), }; - let render_pipeline_jumpflooding_init = ctx.gpu_resources.render_pipelines.get_or_create( - &ctx.device, - &jumpflooding_init_desc, - &ctx.gpu_resources.pipeline_layouts, - &ctx.gpu_resources.shader_modules, - ); + let render_pipeline_jumpflooding_init = ctx + .gpu_resources + .render_pipelines + .get_or_create(ctx, &jumpflooding_init_desc); let render_pipeline_jumpflooding_step = ctx.gpu_resources.render_pipelines.get_or_create( - &ctx.device, + ctx, &RenderPipelineDesc { label: "OutlineMaskProcessor::jumpflooding_step".into(), pipeline_layout: ctx.gpu_resources.pipeline_layouts.get_or_create( - &ctx.device, + ctx, &PipelineLayoutDesc { label: "OutlineMaskProcessor::jumpflooding_step".into(), entries: vec![bind_group_layout_jumpflooding_step], }, - &ctx.gpu_resources.bind_group_layouts, ), fragment_handle: ctx.gpu_resources.shader_modules.get_or_create( - &ctx.device, - &ctx.resolver, + ctx, &include_shader_module!("../../shader/outlines/jumpflooding_step.wgsl"), ), ..jumpflooding_init_desc }, - &ctx.gpu_resources.pipeline_layouts, - &ctx.gpu_resources.shader_modules, ); Self { diff --git a/crates/re_renderer/src/draw_phases/picking_layer.rs b/crates/re_renderer/src/draw_phases/picking_layer.rs index 52053c9d951d..0904e0f6d2e8 100644 --- a/crates/re_renderer/src/draw_phases/picking_layer.rs +++ b/crates/re_renderer/src/draw_phases/picking_layer.rs @@ -204,11 +204,7 @@ impl PickingLayerProcessor { }, ); - let direct_depth_readback = ctx - .shared_renderer_data - .config - .device_caps - .support_depth_readback(); + let direct_depth_readback = ctx.config.device_caps.support_depth_readback(); let picking_depth_target = ctx.gpu_resources.textures.alloc( &ctx.device, @@ -259,7 +255,7 @@ impl PickingLayerProcessor { frame_uniform_buffer_content, ); - let bind_group_0 = ctx.shared_renderer_data.global_bindings.create_bind_group( + let bind_group_0 = ctx.global_bindings.create_bind_group( &ctx.gpu_resources, &ctx.device, frame_uniform_buffer, @@ -535,30 +531,24 @@ impl DepthReadbackWorkaround { ); let render_pipeline = ctx.gpu_resources.render_pipelines.get_or_create( - &ctx.device, + ctx, &RenderPipelineDesc { label: "DepthCopyWorkaround::render_pipeline".into(), pipeline_layout: ctx.gpu_resources.pipeline_layouts.get_or_create( - &ctx.device, + ctx, &PipelineLayoutDesc { label: "DepthCopyWorkaround::render_pipeline".into(), - entries: vec![ - ctx.shared_renderer_data.global_bindings.layout, - bind_group_layout, - ], + entries: vec![ctx.global_bindings.layout, bind_group_layout], }, - &ctx.gpu_resources.bind_group_layouts, ), vertex_entrypoint: "main".into(), vertex_handle: ctx.gpu_resources.shader_modules.get_or_create( - &ctx.device, - &ctx.resolver, + ctx, &include_shader_module!("../../shader/screen_triangle.wgsl"), ), fragment_entrypoint: "main".into(), fragment_handle: ctx.gpu_resources.shader_modules.get_or_create( - &ctx.device, - &ctx.resolver, + ctx, &include_shader_module!("../../shader/copy_texture.wgsl"), ), vertex_buffers: smallvec![], @@ -571,8 +561,6 @@ impl DepthReadbackWorkaround { depth_stencil: None, multisample: wgpu::MultisampleState::default(), }, - &ctx.gpu_resources.pipeline_layouts, - &ctx.gpu_resources.shader_modules, ); Self { diff --git a/crates/re_renderer/src/renderer/compositor.rs b/crates/re_renderer/src/renderer/compositor.rs index 9310502a57da..f1872e14b16d 100644 --- a/crates/re_renderer/src/renderer/compositor.rs +++ b/crates/re_renderer/src/renderer/compositor.rs @@ -1,18 +1,17 @@ use crate::{ allocator::create_and_fill_uniform_buffer, - context::SharedRendererData, include_shader_module, renderer::{screen_triangle_vertex_shader, DrawData, DrawError, Renderer}, view_builder::ViewBuilder, wgpu_resources::{ BindGroupDesc, BindGroupEntry, BindGroupLayoutDesc, GpuBindGroup, GpuBindGroupLayoutHandle, GpuRenderPipelineHandle, GpuRenderPipelinePoolAccessor, GpuTexture, PipelineLayoutDesc, - RenderPipelineDesc, WgpuResourcePools, + RenderPipelineDesc, }, OutlineConfig, Rgba, }; -use crate::{DrawPhase, FileResolver, FileSystem, RenderContext}; +use crate::{DrawPhase, RenderContext}; use smallvec::smallvec; @@ -99,14 +98,9 @@ impl CompositorDrawData { impl Renderer for Compositor { type RendererDrawData = CompositorDrawData; - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { - let bind_group_layout = pools.bind_group_layouts.get_or_create( - device, + fn create_renderer(ctx: &RenderContext) -> Self { + let bind_group_layout = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "Compositor::bind_group_layout".into(), entries: vec![ @@ -148,47 +142,41 @@ impl Renderer for Compositor { }, ); - let vertex_handle = screen_triangle_vertex_shader(pools, device, resolver); + let vertex_handle = screen_triangle_vertex_shader(ctx); let render_pipeline_descriptor = RenderPipelineDesc { label: "CompositorDrawData::render_pipeline_regular".into(), - pipeline_layout: pools.pipeline_layouts.get_or_create( - device, + pipeline_layout: ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "compositor".into(), - entries: vec![shared_data.global_bindings.layout, bind_group_layout], + entries: vec![ctx.global_bindings.layout, bind_group_layout], }, - &pools.bind_group_layouts, ), vertex_entrypoint: "main".into(), vertex_handle, fragment_entrypoint: "main".into(), - fragment_handle: pools.shader_modules.get_or_create( - device, - resolver, - &include_shader_module!("../../shader/composite.wgsl"), - ), + fragment_handle: ctx + .gpu_resources + .shader_modules + .get_or_create(ctx, &include_shader_module!("../../shader/composite.wgsl")), vertex_buffers: smallvec![], - render_targets: smallvec![Some(shared_data.config.output_format_color.into())], + render_targets: smallvec![Some(ctx.config.output_format_color.into())], primitive: wgpu::PrimitiveState::default(), depth_stencil: None, multisample: wgpu::MultisampleState::default(), }; - let render_pipeline_regular = pools.render_pipelines.get_or_create( - device, - &render_pipeline_descriptor, - &pools.pipeline_layouts, - &pools.shader_modules, - ); - let render_pipeline_screenshot = pools.render_pipelines.get_or_create( - device, + let render_pipeline_regular = ctx + .gpu_resources + .render_pipelines + .get_or_create(ctx, &render_pipeline_descriptor); + let render_pipeline_screenshot = ctx.gpu_resources.render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "CompositorDrawData::render_pipeline_screenshot".into(), render_targets: smallvec![Some(ViewBuilder::SCREENSHOT_COLOR_FORMAT.into())], ..render_pipeline_descriptor }, - &pools.pipeline_layouts, - &pools.shader_modules, ); Compositor { diff --git a/crates/re_renderer/src/renderer/debug_overlay.rs b/crates/re_renderer/src/renderer/debug_overlay.rs index d19de8319a3f..6b73b987df4f 100644 --- a/crates/re_renderer/src/renderer/debug_overlay.rs +++ b/crates/re_renderer/src/renderer/debug_overlay.rs @@ -1,17 +1,16 @@ use crate::{ allocator::create_and_fill_uniform_buffer, - context::SharedRendererData, draw_phases::DrawPhase, include_shader_module, wgpu_resources::{ BindGroupDesc, BindGroupEntry, BindGroupLayoutDesc, GpuBindGroup, GpuBindGroupLayoutHandle, GpuRenderPipelineHandle, GpuRenderPipelinePoolAccessor, GpuTexture, PipelineLayoutDesc, - RenderPipelineDesc, WgpuResourcePools, + RenderPipelineDesc, }, RectInt, }; -use super::{DrawData, DrawError, FileResolver, FileSystem, RenderContext, Renderer}; +use super::{DrawData, DrawError, RenderContext, Renderer}; use smallvec::smallvec; @@ -142,14 +141,11 @@ impl DebugOverlayDrawData { impl Renderer for DebugOverlayRenderer { type RendererDrawData = DebugOverlayDrawData; - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { - let bind_group_layout = pools.bind_group_layouts.get_or_create( - device, + fn create_renderer(ctx: &RenderContext) -> Self { + re_tracing::profile_function!(); + + let bind_group_layout = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "DebugOverlay::bind_group_layout".into(), entries: vec![ @@ -191,29 +187,27 @@ impl Renderer for DebugOverlayRenderer { }, ); - let shader_module = pools.shader_modules.get_or_create( - device, - resolver, + let shader_module = ctx.gpu_resources.shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/debug_overlay.wgsl"), ); - let render_pipeline = pools.render_pipelines.get_or_create( - device, + let render_pipeline = ctx.gpu_resources.render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "DebugOverlayDrawData::render_pipeline_regular".into(), - pipeline_layout: pools.pipeline_layouts.get_or_create( - device, + pipeline_layout: ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "DebugOverlay".into(), - entries: vec![shared_data.global_bindings.layout, bind_group_layout], + entries: vec![ctx.global_bindings.layout, bind_group_layout], }, - &pools.bind_group_layouts, ), vertex_entrypoint: "main_vs".into(), vertex_handle: shader_module, fragment_entrypoint: "main_fs".into(), fragment_handle: shader_module, vertex_buffers: smallvec![], - render_targets: smallvec![Some(shared_data.config.output_format_color.into())], + render_targets: smallvec![Some(ctx.config.output_format_color.into())], primitive: wgpu::PrimitiveState { topology: wgpu::PrimitiveTopology::TriangleStrip, cull_mode: None, @@ -222,8 +216,6 @@ impl Renderer for DebugOverlayRenderer { depth_stencil: None, multisample: wgpu::MultisampleState::default(), }, - &pools.pipeline_layouts, - &pools.shader_modules, ); DebugOverlayRenderer { render_pipeline, diff --git a/crates/re_renderer/src/renderer/depth_cloud.rs b/crates/re_renderer/src/renderer/depth_cloud.rs index 0cb3d6480598..4d24320bed0b 100644 --- a/crates/re_renderer/src/renderer/depth_cloud.rs +++ b/crates/re_renderer/src/renderer/depth_cloud.rs @@ -28,10 +28,7 @@ use crate::{ Colormap, OutlineMaskPreference, PickingLayerObjectId, PickingLayerProcessor, }; -use super::{ - DrawData, DrawError, FileResolver, FileSystem, RenderContext, Renderer, SharedRendererData, - WgpuResourcePools, -}; +use super::{DrawData, DrawError, RenderContext, Renderer}; // --- @@ -348,16 +345,13 @@ impl Renderer for DepthCloudRenderer { ] } - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { + fn create_renderer(ctx: &RenderContext) -> Self { re_tracing::profile_function!(); - let bind_group_layout = pools.bind_group_layouts.get_or_create( - device, + let render_pipelines = &ctx.gpu_resources.render_pipelines; + + let bind_group_layout = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "depth_cloud_bg_layout".into(), entries: vec![ @@ -411,18 +405,16 @@ impl Renderer for DepthCloudRenderer { }, ); - let pipeline_layout = pools.pipeline_layouts.get_or_create( - device, + let pipeline_layout = ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "depth_cloud_rp_layout".into(), - entries: vec![shared_data.global_bindings.layout, bind_group_layout], + entries: vec![ctx.global_bindings.layout, bind_group_layout], }, - &pools.bind_group_layouts, ); - let shader_module = pools.shader_modules.get_or_create( - device, - resolver, + let shader_module = ctx.gpu_resources.shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/depth_cloud.wgsl"), ); @@ -447,14 +439,10 @@ impl Renderer for DepthCloudRenderer { ..ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE }, }; - let render_pipeline_color = pools.render_pipelines.get_or_create( - device, - &render_pipeline_desc_color, - &pools.pipeline_layouts, - &pools.shader_modules, - ); - let render_pipeline_picking_layer = pools.render_pipelines.get_or_create( - device, + let render_pipeline_color = + render_pipelines.get_or_create(ctx, &render_pipeline_desc_color); + let render_pipeline_picking_layer = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "DepthCloudRenderer::render_pipeline_picking_layer".into(), fragment_entrypoint: "fs_main_picking_layer".into(), @@ -463,24 +451,18 @@ impl Renderer for DepthCloudRenderer { multisample: PickingLayerProcessor::PICKING_LAYER_MSAA_STATE, ..render_pipeline_desc_color.clone() }, - &pools.pipeline_layouts, - &pools.shader_modules, ); - let render_pipeline_outline_mask = pools.render_pipelines.get_or_create( - device, + let render_pipeline_outline_mask = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "DepthCloudRenderer::render_pipeline_outline_mask".into(), fragment_entrypoint: "fs_main_outline_mask".into(), render_targets: smallvec![Some(OutlineMaskProcessor::MASK_FORMAT.into())], depth_stencil: OutlineMaskProcessor::MASK_DEPTH_STATE, // Alpha to coverage doesn't work with the mask integer target. - multisample: OutlineMaskProcessor::mask_default_msaa_state( - &shared_data.config.device_caps, - ), + multisample: OutlineMaskProcessor::mask_default_msaa_state(&ctx.config.device_caps), ..render_pipeline_desc_color }, - &pools.pipeline_layouts, - &pools.shader_modules, ); DepthCloudRenderer { diff --git a/crates/re_renderer/src/renderer/generic_skybox.rs b/crates/re_renderer/src/renderer/generic_skybox.rs index 494ed22c8f4b..5b278e79aaa3 100644 --- a/crates/re_renderer/src/renderer/generic_skybox.rs +++ b/crates/re_renderer/src/renderer/generic_skybox.rs @@ -1,18 +1,17 @@ use smallvec::smallvec; use crate::{ - context::SharedRendererData, draw_phases::DrawPhase, include_shader_module, renderer::screen_triangle_vertex_shader, view_builder::ViewBuilder, wgpu_resources::{ GpuRenderPipelineHandle, GpuRenderPipelinePoolAccessor, PipelineLayoutDesc, - RenderPipelineDesc, WgpuResourcePools, + RenderPipelineDesc, }, }; -use super::{DrawData, DrawError, FileResolver, FileSystem, RenderContext, Renderer}; +use super::{DrawData, DrawError, RenderContext, Renderer}; /// Renders a generated skybox from a color gradient /// @@ -39,34 +38,27 @@ impl GenericSkyboxDrawData { impl Renderer for GenericSkybox { type RendererDrawData = GenericSkyboxDrawData; - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { + fn create_renderer(ctx: &RenderContext) -> Self { re_tracing::profile_function!(); - let vertex_handle = screen_triangle_vertex_shader(pools, device, resolver); - let render_pipeline = pools.render_pipelines.get_or_create( - device, + let vertex_handle = screen_triangle_vertex_shader(ctx); + let render_pipeline = ctx.gpu_resources.render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "GenericSkybox::render_pipeline".into(), - pipeline_layout: pools.pipeline_layouts.get_or_create( - device, + pipeline_layout: ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "GenericSkybox::render_pipeline".into(), - entries: vec![shared_data.global_bindings.layout], + entries: vec![ctx.global_bindings.layout], }, - &pools.bind_group_layouts, ), vertex_entrypoint: "main".into(), vertex_handle, fragment_entrypoint: "main".into(), - fragment_handle: pools.shader_modules.get_or_create( - device, - resolver, + fragment_handle: ctx.gpu_resources.shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/generic_skybox.wgsl"), ), vertex_buffers: smallvec![], @@ -83,8 +75,6 @@ impl Renderer for GenericSkybox { }), multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }, - &pools.pipeline_layouts, - &pools.shader_modules, ); GenericSkybox { render_pipeline } } diff --git a/crates/re_renderer/src/renderer/lines.rs b/crates/re_renderer/src/renderer/lines.rs index ed180106c30a..126f94652f01 100644 --- a/crates/re_renderer/src/renderer/lines.rs +++ b/crates/re_renderer/src/renderer/lines.rs @@ -109,6 +109,7 @@ use std::{num::NonZeroU64, ops::Range}; use bitflags::bitflags; use bytemuck::Zeroable; use enumset::{enum_set, EnumSet}; +use re_tracing::profile_function; use smallvec::smallvec; use crate::{ @@ -126,10 +127,7 @@ use crate::{ PickingLayerObjectId, PickingLayerProcessor, }; -use super::{ - DrawData, DrawError, FileResolver, FileSystem, LineVertex, RenderContext, Renderer, - SharedRendererData, WgpuResourcePools, -}; +use super::{DrawData, DrawError, LineVertex, RenderContext, Renderer}; pub mod gpu_data { // Don't use `wgsl_buffer_types` since none of this data goes into a buffer, so its alignment rules don't apply. @@ -786,14 +784,13 @@ impl Renderer for LineRenderer { ] } - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { - let bind_group_layout_all_lines = pools.bind_group_layouts.get_or_create( - device, + fn create_renderer(ctx: &RenderContext) -> Self { + profile_function!(); + + let render_pipelines = &ctx.gpu_resources.render_pipelines; + + let bind_group_layout_all_lines = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "LineRenderer::bind_group_layout_all_lines".into(), entries: vec![ @@ -843,8 +840,8 @@ impl Renderer for LineRenderer { }, ); - let bind_group_layout_batch = pools.bind_group_layouts.get_or_create( - device, + let bind_group_layout_batch = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "LineRenderer::bind_group_layout_batch".into(), entries: vec![wgpu::BindGroupLayoutEntry { @@ -862,24 +859,22 @@ impl Renderer for LineRenderer { }, ); - let pipeline_layout = pools.pipeline_layouts.get_or_create( - device, + let pipeline_layout = ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "LineRenderer::pipeline_layout".into(), entries: vec![ - shared_data.global_bindings.layout, + ctx.global_bindings.layout, bind_group_layout_all_lines, bind_group_layout_batch, ], }, - &pools.bind_group_layouts, ); - let shader_module = pools.shader_modules.get_or_create( - device, - resolver, - &include_shader_module!("../../shader/lines.wgsl"), - ); + let shader_module = ctx + .gpu_resources + .shader_modules + .get_or_create(ctx, &include_shader_module!("../../shader/lines.wgsl")); let render_pipeline_desc_color = RenderPipelineDesc { label: "LineRenderer::render_pipeline_color".into(), @@ -901,14 +896,10 @@ impl Renderer for LineRenderer { ..ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE }, }; - let render_pipeline_color = pools.render_pipelines.get_or_create( - device, - &render_pipeline_desc_color, - &pools.pipeline_layouts, - &pools.shader_modules, - ); - let render_pipeline_picking_layer = pools.render_pipelines.get_or_create( - device, + let render_pipeline_color = + render_pipelines.get_or_create(ctx, &render_pipeline_desc_color); + let render_pipeline_picking_layer = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "LineRenderer::render_pipeline_picking_layer".into(), fragment_entrypoint: "fs_main_picking_layer".into(), @@ -917,11 +908,9 @@ impl Renderer for LineRenderer { multisample: PickingLayerProcessor::PICKING_LAYER_MSAA_STATE, ..render_pipeline_desc_color.clone() }, - &pools.pipeline_layouts, - &pools.shader_modules, ); - let render_pipeline_outline_mask = pools.render_pipelines.get_or_create( - device, + let render_pipeline_outline_mask = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "LineRenderer::render_pipeline_outline_mask".into(), pipeline_layout, @@ -937,12 +926,8 @@ impl Renderer for LineRenderer { }, depth_stencil: OutlineMaskProcessor::MASK_DEPTH_STATE, // Alpha to coverage doesn't work with the mask integer target. - multisample: OutlineMaskProcessor::mask_default_msaa_state( - &shared_data.config.device_caps, - ), + multisample: OutlineMaskProcessor::mask_default_msaa_state(&ctx.config.device_caps), }, - &pools.pipeline_layouts, - &pools.shader_modules, ); LineRenderer { diff --git a/crates/re_renderer/src/renderer/mesh_renderer.rs b/crates/re_renderer/src/renderer/mesh_renderer.rs index f77aeda056bc..346fa0cc8b7a 100644 --- a/crates/re_renderer/src/renderer/mesh_renderer.rs +++ b/crates/re_renderer/src/renderer/mesh_renderer.rs @@ -22,10 +22,7 @@ use crate::{ Color32, OutlineMaskPreference, PickingLayerId, PickingLayerProcessor, }; -use super::{ - DrawData, DrawError, FileResolver, FileSystem, RenderContext, Renderer, SharedRendererData, - WgpuResourcePools, -}; +use super::{DrawData, DrawError, RenderContext, Renderer}; mod gpu_data { use ecolor::Color32; @@ -290,16 +287,13 @@ impl Renderer for MeshRenderer { ] } - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { + fn create_renderer(ctx: &RenderContext) -> Self { re_tracing::profile_function!(); - let bind_group_layout = pools.bind_group_layouts.get_or_create( - device, + let render_pipelines = &ctx.gpu_resources.render_pipelines; + + let bind_group_layout = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "MeshRenderer::bind_group_layout".into(), entries: vec![ @@ -328,18 +322,16 @@ impl Renderer for MeshRenderer { ], }, ); - let pipeline_layout = pools.pipeline_layouts.get_or_create( - device, + let pipeline_layout = ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "MeshRenderer::pipeline_layout".into(), - entries: vec![shared_data.global_bindings.layout, bind_group_layout], + entries: vec![ctx.global_bindings.layout, bind_group_layout], }, - &pools.bind_group_layouts, ); - let shader_module = pools.shader_modules.get_or_create( - device, - resolver, + let shader_module = ctx.gpu_resources.shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/instanced_mesh.wgsl"), ); @@ -367,14 +359,10 @@ impl Renderer for MeshRenderer { depth_stencil: ViewBuilder::MAIN_TARGET_DEFAULT_DEPTH_STATE, multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }; - let render_pipeline_shaded = pools.render_pipelines.get_or_create( - device, - &render_pipeline_shaded_desc, - &pools.pipeline_layouts, - &pools.shader_modules, - ); - let render_pipeline_picking_layer = pools.render_pipelines.get_or_create( - device, + let render_pipeline_shaded = + render_pipelines.get_or_create(ctx, &render_pipeline_shaded_desc); + let render_pipeline_picking_layer = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "MeshRenderer::render_pipeline_picking_layer".into(), fragment_entrypoint: "fs_main_picking_layer".into(), @@ -383,23 +371,17 @@ impl Renderer for MeshRenderer { multisample: PickingLayerProcessor::PICKING_LAYER_MSAA_STATE, ..render_pipeline_shaded_desc.clone() }, - &pools.pipeline_layouts, - &pools.shader_modules, ); - let render_pipeline_outline_mask = pools.render_pipelines.get_or_create( - device, + let render_pipeline_outline_mask = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "MeshRenderer::render_pipeline_outline_mask".into(), fragment_entrypoint: "fs_main_outline_mask".into(), render_targets: smallvec![Some(OutlineMaskProcessor::MASK_FORMAT.into())], depth_stencil: OutlineMaskProcessor::MASK_DEPTH_STATE, - multisample: OutlineMaskProcessor::mask_default_msaa_state( - &shared_data.config.device_caps, - ), + multisample: OutlineMaskProcessor::mask_default_msaa_state(&ctx.config.device_caps), ..render_pipeline_shaded_desc }, - &pools.pipeline_layouts, - &pools.shader_modules, ); MeshRenderer { diff --git a/crates/re_renderer/src/renderer/mod.rs b/crates/re_renderer/src/renderer/mod.rs index 1e2acda5b722..76003439db40 100644 --- a/crates/re_renderer/src/renderer/mod.rs +++ b/crates/re_renderer/src/renderer/mod.rs @@ -36,11 +36,10 @@ mod debug_overlay; pub use debug_overlay::{DebugOverlayDrawData, DebugOverlayError, DebugOverlayRenderer}; use crate::{ - context::{RenderContext, SharedRendererData}, + context::RenderContext, draw_phases::DrawPhase, include_shader_module, - wgpu_resources::{GpuRenderPipelinePoolAccessor, PoolError, WgpuResourcePools}, - FileResolver, FileSystem, + wgpu_resources::{GpuRenderPipelinePoolAccessor, PoolError}, }; /// GPU sided data used by a [`Renderer`] to draw things to the screen. @@ -64,12 +63,7 @@ pub enum DrawError { pub trait Renderer { type RendererDrawData: DrawData; - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self; + fn create_renderer(ctx: &RenderContext) -> Self; // TODO(andreas): Some Renderers need to create their own passes, need something like this for that. @@ -87,14 +81,11 @@ pub trait Renderer { } /// Gets or creates a vertex shader module for drawing a screen filling triangle. -pub fn screen_triangle_vertex_shader( - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, +pub fn screen_triangle_vertex_shader( + ctx: &RenderContext, ) -> crate::wgpu_resources::GpuShaderModuleHandle { - pools.shader_modules.get_or_create( - device, - resolver, + ctx.gpu_resources.shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/screen_triangle.wgsl"), ) } diff --git a/crates/re_renderer/src/renderer/point_cloud.rs b/crates/re_renderer/src/renderer/point_cloud.rs index d0a5d9e8b755..77ed7c2f693f 100644 --- a/crates/re_renderer/src/renderer/point_cloud.rs +++ b/crates/re_renderer/src/renderer/point_cloud.rs @@ -36,10 +36,7 @@ use crate::{ }, }; -use super::{ - DrawData, DrawError, FileResolver, FileSystem, RenderContext, Renderer, SharedRendererData, - WgpuResourcePools, -}; +use super::{DrawData, DrawError, RenderContext, Renderer}; bitflags! { /// Property flags for a point batch @@ -533,16 +530,13 @@ impl Renderer for PointCloudRenderer { ] } - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { + fn create_renderer(ctx: &RenderContext) -> Self { re_tracing::profile_function!(); - let bind_group_layout_all_points = pools.bind_group_layouts.get_or_create( - device, + let render_pipelines = &ctx.gpu_resources.render_pipelines; + + let bind_group_layout_all_points = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "PointCloudRenderer::bind_group_layout_all_points".into(), entries: vec![ @@ -592,8 +586,8 @@ impl Renderer for PointCloudRenderer { }, ); - let bind_group_layout_batch = pools.bind_group_layouts.get_or_create( - device, + let bind_group_layout_batch = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &BindGroupLayoutDesc { label: "PointCloudRenderer::bind_group_layout_batch".into(), entries: vec![wgpu::BindGroupLayoutEntry { @@ -611,33 +605,32 @@ impl Renderer for PointCloudRenderer { }, ); - let pipeline_layout = pools.pipeline_layouts.get_or_create( - device, + let pipeline_layout = ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "PointCloudRenderer::pipeline_layout".into(), entries: vec![ - shared_data.global_bindings.layout, + ctx.global_bindings.layout, bind_group_layout_all_points, bind_group_layout_batch, ], }, - &pools.bind_group_layouts, ); let shader_module_desc = include_shader_module!("../../shader/point_cloud.wgsl"); - let shader_module = - pools - .shader_modules - .get_or_create(device, resolver, &shader_module_desc); + let shader_module = ctx + .gpu_resources + .shader_modules + .get_or_create(ctx, &shader_module_desc); // WORKAROUND for https://github.com/gfx-rs/naga/issues/1743 let mut shader_module_desc_vertex = shader_module_desc.clone(); shader_module_desc_vertex.extra_workaround_replacements = vec![("fwidth(".to_owned(), "f32(".to_owned())]; - let shader_module_vertex = - pools - .shader_modules - .get_or_create(device, resolver, &shader_module_desc_vertex); + let shader_module_vertex = ctx + .gpu_resources + .shader_modules + .get_or_create(ctx, &shader_module_desc_vertex); let render_pipeline_desc_color = RenderPipelineDesc { label: "PointCloudRenderer::render_pipeline_color".into(), @@ -660,14 +653,10 @@ impl Renderer for PointCloudRenderer { ..ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE }, }; - let render_pipeline_color = pools.render_pipelines.get_or_create( - device, - &render_pipeline_desc_color, - &pools.pipeline_layouts, - &pools.shader_modules, - ); - let render_pipeline_picking_layer = pools.render_pipelines.get_or_create( - device, + let render_pipeline_color = + render_pipelines.get_or_create(ctx, &render_pipeline_desc_color); + let render_pipeline_picking_layer = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "PointCloudRenderer::render_pipeline_picking_layer".into(), fragment_entrypoint: "fs_main_picking_layer".into(), @@ -676,24 +665,18 @@ impl Renderer for PointCloudRenderer { multisample: PickingLayerProcessor::PICKING_LAYER_MSAA_STATE, ..render_pipeline_desc_color.clone() }, - &pools.pipeline_layouts, - &pools.shader_modules, ); - let render_pipeline_outline_mask = pools.render_pipelines.get_or_create( - device, + let render_pipeline_outline_mask = render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "PointCloudRenderer::render_pipeline_outline_mask".into(), fragment_entrypoint: "fs_main_outline_mask".into(), render_targets: smallvec![Some(OutlineMaskProcessor::MASK_FORMAT.into())], depth_stencil: OutlineMaskProcessor::MASK_DEPTH_STATE, // Alpha to coverage doesn't work with the mask integer target. - multisample: OutlineMaskProcessor::mask_default_msaa_state( - &shared_data.config.device_caps, - ), + multisample: OutlineMaskProcessor::mask_default_msaa_state(&ctx.config.device_caps), ..render_pipeline_desc_color }, - &pools.pipeline_layouts, - &pools.shader_modules, ); PointCloudRenderer { diff --git a/crates/re_renderer/src/renderer/rectangles.rs b/crates/re_renderer/src/renderer/rectangles.rs index 59eea5e43024..72b75854a80d 100644 --- a/crates/re_renderer/src/renderer/rectangles.rs +++ b/crates/re_renderer/src/renderer/rectangles.rs @@ -28,10 +28,7 @@ use crate::{ Colormap, OutlineMaskPreference, PickingLayerProcessor, Rgba, }; -use super::{ - DrawData, DrawError, FileResolver, FileSystem, RenderContext, Renderer, SharedRendererData, - WgpuResourcePools, -}; +use super::{DrawData, DrawError, RenderContext, Renderer}; /// Texture filter setting for magnification (a texel covers several pixels). #[derive(Debug, Clone, Copy)] @@ -496,16 +493,13 @@ pub struct RectangleRenderer { impl Renderer for RectangleRenderer { type RendererDrawData = RectangleDrawData; - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { + fn create_renderer(ctx: &RenderContext) -> Self { re_tracing::profile_function!(); - let bind_group_layout = pools.bind_group_layouts.get_or_create( - device, + let render_pipelines = &ctx.gpu_resources.render_pipelines; + + let bind_group_layout = ctx.gpu_resources.bind_group_layouts.get_or_create( + &ctx.device, &(BindGroupLayoutDesc { label: "RectangleRenderer::bind_group_layout".into(), entries: vec![ @@ -572,23 +566,20 @@ impl Renderer for RectangleRenderer { }), ); - let pipeline_layout = pools.pipeline_layouts.get_or_create( - device, + let pipeline_layout = ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &(PipelineLayoutDesc { label: "RectangleRenderer::pipeline_layout".into(), - entries: vec![shared_data.global_bindings.layout, bind_group_layout], + entries: vec![ctx.global_bindings.layout, bind_group_layout], }), - &pools.bind_group_layouts, ); - let shader_module_vs = pools.shader_modules.get_or_create( - device, - resolver, + let shader_module_vs = ctx.gpu_resources.shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/rectangle_vs.wgsl"), ); - let shader_module_fs = pools.shader_modules.get_or_create( - device, - resolver, + let shader_module_fs = ctx.gpu_resources.shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/rectangle_fs.wgsl"), ); @@ -614,14 +605,10 @@ impl Renderer for RectangleRenderer { depth_stencil: ViewBuilder::MAIN_TARGET_DEFAULT_DEPTH_STATE, multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }; - let render_pipeline_color = pools.render_pipelines.get_or_create( - device, - &render_pipeline_desc_color, - &pools.pipeline_layouts, - &pools.shader_modules, - ); - let render_pipeline_picking_layer = pools.render_pipelines.get_or_create( - device, + let render_pipeline_color = + render_pipelines.get_or_create(ctx, &render_pipeline_desc_color); + let render_pipeline_picking_layer = render_pipelines.get_or_create( + ctx, &(RenderPipelineDesc { label: "RectangleRenderer::render_pipeline_picking_layer".into(), fragment_entrypoint: "fs_main_picking_layer".into(), @@ -630,23 +617,17 @@ impl Renderer for RectangleRenderer { multisample: PickingLayerProcessor::PICKING_LAYER_MSAA_STATE, ..render_pipeline_desc_color.clone() }), - &pools.pipeline_layouts, - &pools.shader_modules, ); - let render_pipeline_outline_mask = pools.render_pipelines.get_or_create( - device, + let render_pipeline_outline_mask = render_pipelines.get_or_create( + ctx, &(RenderPipelineDesc { label: "RectangleRenderer::render_pipeline_outline_mask".into(), fragment_entrypoint: "fs_main_outline_mask".into(), render_targets: smallvec![Some(OutlineMaskProcessor::MASK_FORMAT.into())], depth_stencil: OutlineMaskProcessor::MASK_DEPTH_STATE, - multisample: OutlineMaskProcessor::mask_default_msaa_state( - &shared_data.config.device_caps, - ), + multisample: OutlineMaskProcessor::mask_default_msaa_state(&ctx.config.device_caps), ..render_pipeline_desc_color }), - &pools.pipeline_layouts, - &pools.shader_modules, ); RectangleRenderer { diff --git a/crates/re_renderer/src/renderer/test_triangle.rs b/crates/re_renderer/src/renderer/test_triangle.rs index 092fb2ed28d2..226622b301f1 100644 --- a/crates/re_renderer/src/renderer/test_triangle.rs +++ b/crates/re_renderer/src/renderer/test_triangle.rs @@ -1,7 +1,6 @@ use smallvec::smallvec; use crate::{ - context::SharedRendererData, include_shader_module, view_builder::ViewBuilder, wgpu_resources::{GpuRenderPipelineHandle, PipelineLayoutDesc, RenderPipelineDesc}, @@ -30,34 +29,27 @@ impl TestTriangleDrawData { impl Renderer for TestTriangle { type RendererDrawData = TestTriangleDrawData; - fn create_renderer( - shared_data: &SharedRendererData, - pools: &WgpuResourcePools, - device: &wgpu::Device, - resolver: &FileResolver, - ) -> Self { - let render_pipeline = pools.render_pipelines.get_or_create( - device, + fn create_renderer(ctx: &RenderContext) -> Self { + let shader_modules = &ctx.gpu_resources.shader_modules; + let render_pipeline = ctx.gpu_resources.render_pipelines.get_or_create( + ctx, &RenderPipelineDesc { label: "TestTriangle::render_pipeline".into(), - pipeline_layout: pools.pipeline_layouts.get_or_create( - device, + pipeline_layout: ctx.gpu_resources.pipeline_layouts.get_or_create( + ctx, &PipelineLayoutDesc { label: "global only".into(), - entries: vec![shared_data.global_bindings.layout], + entries: vec![ctx.global_bindings.layout], }, - &pools.bind_group_layouts, ), vertex_entrypoint: "vs_main".into(), - vertex_handle: pools.shader_modules.get_or_create( - device, - resolver, + vertex_handle: shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/test_triangle.wgsl"), ), fragment_entrypoint: "fs_main".into(), - fragment_handle: pools.shader_modules.get_or_create( - device, - resolver, + fragment_handle: shader_modules.get_or_create( + ctx, &include_shader_module!("../../shader/test_triangle.wgsl"), ), vertex_buffers: smallvec![], @@ -72,8 +64,6 @@ impl Renderer for TestTriangle { }), multisample: ViewBuilder::MAIN_TARGET_DEFAULT_MSAA_STATE, }, - &pools.pipeline_layouts, - &pools.shader_modules, ); TestTriangle { render_pipeline } diff --git a/crates/re_renderer/src/resource_managers/mesh_manager.rs b/crates/re_renderer/src/resource_managers/mesh_manager.rs index 649978bf408a..ece7fe172731 100644 --- a/crates/re_renderer/src/resource_managers/mesh_manager.rs +++ b/crates/re_renderer/src/resource_managers/mesh_manager.rs @@ -1,7 +1,6 @@ use crate::{ mesh::{GpuMesh, Mesh}, renderer::MeshRenderer, - wgpu_resources::GpuBindGroupLayoutHandle, RenderContext, }; @@ -15,14 +14,12 @@ pub type GpuMeshHandle = ResourceHandle; pub struct MeshManager { manager: ResourceManager, - mesh_bind_group_layout: GpuBindGroupLayoutHandle, } impl MeshManager { - pub(crate) fn new(mesh_renderer: &MeshRenderer) -> Self { + pub(crate) fn new() -> Self { MeshManager { manager: Default::default(), - mesh_bind_group_layout: mesh_renderer.bind_group_layout, } } @@ -35,7 +32,7 @@ impl MeshManager { ) -> Result { re_tracing::profile_function!(); Ok(self.manager.store_resource( - GpuMesh::new(ctx, self.mesh_bind_group_layout, mesh)?, + GpuMesh::new(ctx, ctx.renderer::().bind_group_layout, mesh)?, lifetime, )) } diff --git a/crates/re_renderer/src/view_builder.rs b/crates/re_renderer/src/view_builder.rs index 907a55679606..05d0ea104ef9 100644 --- a/crates/re_renderer/src/view_builder.rs +++ b/crates/re_renderer/src/view_builder.rs @@ -449,7 +449,7 @@ impl ViewBuilder { auto_size_points: auto_size_points.0, auto_size_lines: auto_size_lines.0, - device_tier: (ctx.shared_renderer_data.config.device_caps.tier as u32).into(), + device_tier: (ctx.config.device_caps.tier as u32).into(), }; let frame_uniform_buffer = create_and_fill_uniform_buffer( ctx, @@ -457,7 +457,7 @@ impl ViewBuilder { frame_uniform_buffer_content, ); - let bind_group_0 = ctx.shared_renderer_data.global_bindings.create_bind_group( + let bind_group_0 = ctx.global_bindings.create_bind_group( &ctx.gpu_resources, &ctx.device, frame_uniform_buffer, diff --git a/crates/re_renderer/src/wgpu_resources/pipeline_layout_pool.rs b/crates/re_renderer/src/wgpu_resources/pipeline_layout_pool.rs index 3178196d8720..a4363716e397 100644 --- a/crates/re_renderer/src/wgpu_resources/pipeline_layout_pool.rs +++ b/crates/re_renderer/src/wgpu_resources/pipeline_layout_pool.rs @@ -1,7 +1,7 @@ -use crate::debug_label::DebugLabel; +use crate::{debug_label::DebugLabel, RenderContext}; use super::{ - bind_group_layout_pool::{GpuBindGroupLayoutHandle, GpuBindGroupLayoutPool}, + bind_group_layout_pool::GpuBindGroupLayoutHandle, static_resource_pool::{ StaticResourcePool, StaticResourcePoolAccessor as _, StaticResourcePoolReadLockAccessor, }, @@ -25,24 +25,24 @@ pub struct GpuPipelineLayoutPool { impl GpuPipelineLayoutPool { pub fn get_or_create( &self, - device: &wgpu::Device, + ctx: &RenderContext, desc: &PipelineLayoutDesc, - bind_group_layout_pool: &GpuBindGroupLayoutPool, ) -> GpuPipelineLayoutHandle { self.pool.get_or_create(desc, |desc| { // TODO(andreas): error handling - let bind_groups = bind_group_layout_pool.resources(); + let bind_groups = ctx.gpu_resources.bind_group_layouts.resources(); - device.create_pipeline_layout(&wgpu::PipelineLayoutDescriptor { - label: desc.label.get(), - bind_group_layouts: &desc - .entries - .iter() - .map(|handle| bind_groups.get(*handle).unwrap()) - .collect::>(), - push_constant_ranges: &[], // Sadly not widely supported - }) + ctx.device + .create_pipeline_layout(&wgpu::PipelineLayoutDescriptor { + label: desc.label.get(), + bind_group_layouts: &desc + .entries + .iter() + .map(|handle| bind_groups.get(*handle).unwrap()) + .collect::>(), + push_constant_ranges: &[], // Sadly not widely supported + }) }) } diff --git a/crates/re_renderer/src/wgpu_resources/render_pipeline_pool.rs b/crates/re_renderer/src/wgpu_resources/render_pipeline_pool.rs index a3727298975a..457118d87e26 100644 --- a/crates/re_renderer/src/wgpu_resources/render_pipeline_pool.rs +++ b/crates/re_renderer/src/wgpu_resources/render_pipeline_pool.rs @@ -1,6 +1,6 @@ use smallvec::SmallVec; -use crate::debug_label::DebugLabel; +use crate::{debug_label::DebugLabel, RenderContext}; use super::{ pipeline_layout_pool::{GpuPipelineLayoutHandle, GpuPipelineLayoutPool}, @@ -181,17 +181,19 @@ pub struct GpuRenderPipelinePool { impl GpuRenderPipelinePool { pub fn get_or_create( &self, - device: &wgpu::Device, + ctx: &RenderContext, desc: &RenderPipelineDesc, - pipeline_layout_pool: &GpuPipelineLayoutPool, - shader_module_pool: &GpuShaderModulePool, ) -> GpuRenderPipelineHandle { self.pool.get_or_create(desc, |desc| { sanity_check_vertex_buffers(&desc.vertex_buffers); // TODO(cmc): certainly not unwrapping here - desc.create_render_pipeline(device, pipeline_layout_pool, shader_module_pool) - .unwrap() + desc.create_render_pipeline( + &ctx.device, + &ctx.gpu_resources.pipeline_layouts, + &ctx.gpu_resources.shader_modules, + ) + .unwrap() }) } diff --git a/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs b/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs index 68b980fdf6d8..03edd081c72a 100644 --- a/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs +++ b/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs @@ -3,7 +3,7 @@ use std::{hash::Hash, path::PathBuf}; use ahash::HashSet; use anyhow::Context as _; -use crate::{debug_label::DebugLabel, FileResolver, FileSystem}; +use crate::{debug_label::DebugLabel, FileResolver, FileSystem, RenderContext}; use super::static_resource_pool::{StaticResourcePool, StaticResourcePoolReadLockAccessor}; @@ -113,14 +113,17 @@ pub struct GpuShaderModulePool { } impl GpuShaderModulePool { - pub fn get_or_create( + pub fn get_or_create( &self, - device: &wgpu::Device, - resolver: &FileResolver, + ctx: &RenderContext, desc: &ShaderModuleDesc, ) -> GpuShaderModuleHandle { self.pool.get_or_create(desc, |desc| { - desc.create_shader_module(device, resolver, &self.shader_text_workaround_replacements) + desc.create_shader_module( + &ctx.device, + &ctx.resolver, + &self.shader_text_workaround_replacements, + ) }) }