From dc462a8fad01ec6cff46c7ff097ee188a751d0b3 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Wed, 28 Aug 2024 08:38:37 -0700 Subject: [PATCH] Pass `ShowLabels` default through component fallback provider system. (#7266) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What * Followup to #7249 * Fixes #3465 The default value for the `ShowLabels` component now passes through `ComponentFallbackProvider`, so the viewer UI will properly show the fallback value derived from the number of instances. Further possible work in this area would be to define “number of instances” as something that can be generically fetched based on an archetype, rather than requiring each visualizer to communicate how it counts that. ### Checklist * [X] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [X] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7266?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7266?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [X] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [X] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [X] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7266) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --- .../src/visualizers/arrows2d.rs | 10 +++- .../src/visualizers/arrows3d.rs | 10 +++- .../src/visualizers/boxes2d.rs | 10 +++- .../src/visualizers/boxes3d.rs | 10 +++- .../src/visualizers/ellipsoids.rs | 10 +++- .../src/visualizers/lines2d.rs | 10 +++- .../src/visualizers/lines3d.rs | 10 +++- .../src/visualizers/points2d.rs | 10 +++- .../src/visualizers/points3d.rs | 10 +++- .../src/visualizers/utilities/labels.rs | 49 +++++++++++++++---- .../src/visualizers/utilities/mod.rs | 3 +- 11 files changed, 113 insertions(+), 29 deletions(-) diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/arrows2d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/arrows2d.rs index 295103cd34ee..285d65179f5a 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/arrows2d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/arrows2d.rs @@ -137,7 +137,7 @@ impl Arrows2DVisualizer { }, labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, world_from_obj, @@ -304,4 +304,10 @@ impl TypedComponentFallbackProvider for Arrows2DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Arrows2DVisualizer => [Color, DrawOrder]); +impl TypedComponentFallbackProvider for Arrows2DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Arrows2DVisualizer => [Color, DrawOrder, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/arrows3d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/arrows3d.rs index 84b09a0dbb47..2bd3c8d45c86 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/arrows3d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/arrows3d.rs @@ -139,7 +139,7 @@ impl Arrows3DVisualizer { instance_positions, labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, world_from_obj, @@ -301,4 +301,10 @@ impl TypedComponentFallbackProvider for Arrows3DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Arrows3DVisualizer => [Color]); +impl TypedComponentFallbackProvider for Arrows3DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Arrows3DVisualizer => [Color, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/boxes2d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/boxes2d.rs index 47e2a60d831c..c96d7f30a2ee 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/boxes2d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/boxes2d.rs @@ -142,7 +142,7 @@ impl Boxes2DVisualizer { }), labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, std::convert::identity, @@ -313,4 +313,10 @@ impl TypedComponentFallbackProvider for Boxes2DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Boxes2DVisualizer => [Color, DrawOrder]); +impl TypedComponentFallbackProvider for Boxes2DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Boxes2DVisualizer => [Color, DrawOrder, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/boxes3d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/boxes3d.rs index 7c5303429ff2..96b17c0206f8 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/boxes3d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/boxes3d.rs @@ -165,7 +165,7 @@ impl Boxes3DVisualizer { .map(|t| t.translation.into()), labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, glam::Affine3A::IDENTITY, @@ -378,4 +378,10 @@ impl TypedComponentFallbackProvider for Boxes3DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Boxes3DVisualizer => [Color]); +impl TypedComponentFallbackProvider for Boxes3DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Boxes3DVisualizer => [Color, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/ellipsoids.rs b/crates/viewer/re_space_view_spatial/src/visualizers/ellipsoids.rs index ad77722ee317..3ee132c2c5f4 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/ellipsoids.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/ellipsoids.rs @@ -206,7 +206,7 @@ impl Ellipsoids3DVisualizer { .map(|t| t.translation.into()), labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, glam::Affine3A::IDENTITY, @@ -411,4 +411,10 @@ impl TypedComponentFallbackProvider for Ellipsoids3DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Ellipsoids3DVisualizer => [Color]); +impl TypedComponentFallbackProvider for Ellipsoids3DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Ellipsoids3DVisualizer => [Color, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/lines2d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/lines2d.rs index 9e38eaf674b5..85d8d6f50e12 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/lines2d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/lines2d.rs @@ -122,7 +122,7 @@ impl Lines2DVisualizer { }), labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, world_from_obj, @@ -294,4 +294,10 @@ impl TypedComponentFallbackProvider for Lines2DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Lines2DVisualizer => [Color, DrawOrder]); +impl TypedComponentFallbackProvider for Lines2DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Lines2DVisualizer => [Color, DrawOrder, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/lines3d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/lines3d.rs index b0c4cd23505b..f7c310d6212c 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/lines3d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/lines3d.rs @@ -129,7 +129,7 @@ impl Lines3DVisualizer { }), labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, world_from_obj, @@ -294,4 +294,10 @@ impl TypedComponentFallbackProvider for Lines3DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Lines3DVisualizer => [Color]); +impl TypedComponentFallbackProvider for Lines3DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Lines3DVisualizer => [Color, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/points2d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/points2d.rs index 20b4401fde53..f83a3c23eb78 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/points2d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/points2d.rs @@ -137,7 +137,7 @@ impl Points2DVisualizer { instance_positions: data.positions.iter().map(|p| glam::vec2(p.x(), p.y())), labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, world_from_obj, @@ -320,4 +320,10 @@ impl TypedComponentFallbackProvider for Points2DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Points2DVisualizer => [Color, DrawOrder]); +impl TypedComponentFallbackProvider for Points2DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Points2DVisualizer => [Color, DrawOrder, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/points3d.rs b/crates/viewer/re_space_view_spatial/src/visualizers/points3d.rs index f149cd751b78..35a801aa7026 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/points3d.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/points3d.rs @@ -145,7 +145,7 @@ impl Points3DVisualizer { instance_positions: positions.iter().copied(), labels: &data.labels, colors: &colors, - show_labels: data.show_labels, + show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)), annotation_infos: &annotation_infos, }, world_from_obj, @@ -305,4 +305,10 @@ impl TypedComponentFallbackProvider for Points3DVisualizer { } } -re_viewer_context::impl_component_fallback_provider!(Points3DVisualizer => [Color]); +impl TypedComponentFallbackProvider for Points3DVisualizer { + fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels { + super::utilities::show_labels_fallback::(ctx) + } +} + +re_viewer_context::impl_component_fallback_provider!(Points3DVisualizer => [Color, ShowLabels]); diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/utilities/labels.rs b/crates/viewer/re_space_view_spatial/src/visualizers/utilities/labels.rs index 075f869c7cd7..dc9ccdf9c490 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/utilities/labels.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/utilities/labels.rs @@ -4,9 +4,13 @@ use itertools::{izip, Either}; use re_entity_db::InstancePathHash; use re_log_types::{EntityPath, Instance}; -use re_types::components::ShowLabels; +use re_types::components::{ShowLabels, Text}; +use re_types::{Component, Loggable as _}; use re_viewer_context::ResolvedAnnotationInfos; +#[cfg(doc)] +use re_viewer_context::ComponentFallbackProvider; + use crate::visualizers::entity_iterator::clamped_or; #[derive(Clone)] @@ -66,15 +70,44 @@ pub struct LabeledBatch<'a, P: 'a, I: Iterator + 'a> { /// Length 1 is treated as a color for the whole batch. pub colors: &'a [egui::Color32], - pub show_labels: Option, + /// The [`ShowLabels`] component value. + /// + /// If no value is available from the data, use [`show_labels_fallback`] to obtain it. + pub show_labels: re_types::components::ShowLabels, pub annotation_infos: &'a ResolvedAnnotationInfos, } -/// Maximum number of labels after which we stop displaying labels for that entity all together. +/// Maximum number of labels after which we stop displaying labels for that entity all together, +/// unless overridden by a [`ShowLabels`] component. +const MAX_NUM_LABELS_PER_ENTITY: usize = 30; + +/// Given a visualizer’s query context, compute its [`ShowLabels`] fallback value +/// (used when neither the logged data nor the blueprint provides a value). +/// +/// Assumes that the visualizer reads the [`Text`] component for components. +/// The type parameter `C` must be the component type that defines the number of instances +/// in the batch. /// -/// TODO(#4451): Hiding of labels should be configurable. This can be the heuristic for it. -pub const MAX_NUM_LABELS_PER_ENTITY: usize = 30; +// TODO(kpreid): This component type (or the length directly) should be gotten from some kind of +// general mechanism of "how big is this batch?" rather than requiring the caller to specify it, +// possibly incorrectly. +/// +/// This function is normally used to implement the [`ComponentFallbackProvider`] +/// that will be used in a [`LabeledBatch`]. +pub fn show_labels_fallback(ctx: &re_viewer_context::QueryContext<'_>) -> ShowLabels { + let results = + ctx.recording() + .latest_at(ctx.query, ctx.target_entity_path, [C::name(), Text::name()]); + let num_instances = results + .component_batch_raw(&C::name()) + .map_or(0, |array| array.len()); + let num_labels = results + .component_batch_raw(&Text::name()) + .map_or(0, |array| array.len()); + + ShowLabels::from(num_labels == 1 || num_instances < MAX_NUM_LABELS_PER_ENTITY) +} /// Produces 3D ui labels from component data. /// @@ -119,12 +152,8 @@ pub fn process_labels<'a, P: 'a>( show_labels, annotation_infos, } = batch; + let show_labels = bool::from(show_labels.0); - let show_labels = match show_labels { - Some(ShowLabels(value)) => bool::from(value), - // Choose based on automatic policy. - None => labels.len() == 1 || num_instances < MAX_NUM_LABELS_PER_ENTITY, - }; if !show_labels { return Either::Left(iter::empty()); } diff --git a/crates/viewer/re_space_view_spatial/src/visualizers/utilities/mod.rs b/crates/viewer/re_space_view_spatial/src/visualizers/utilities/mod.rs index e75d545450b7..fe0a5c7312ac 100644 --- a/crates/viewer/re_space_view_spatial/src/visualizers/utilities/mod.rs +++ b/crates/viewer/re_space_view_spatial/src/visualizers/utilities/mod.rs @@ -4,7 +4,8 @@ mod spatial_view_visualizer; mod textured_rect; pub use labels::{ - process_labels, process_labels_2d, process_labels_3d, LabeledBatch, UiLabel, UiLabelTarget, + process_labels, process_labels_2d, process_labels_3d, show_labels_fallback, LabeledBatch, + UiLabel, UiLabelTarget, }; pub use spatial_view_visualizer::SpatialViewVisualizerData; pub use textured_rect::textured_rect_from_image;