Fix name editing for point series, fix fallbacks for point/line serie…

…s display (#6666) ### What * Fixes #6613 https://github.com/rerun-io/rerun/assets/1220815/8f47caf9-ee7f-408d-b596-e06ff2565d5b commit by commit (yes it's just two :)) ### 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/6666?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/6666?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/6666) - [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`.
rerun-io · Jun 27, 2024 · e325f79 · e325f79
1 parent 65e6474
commit e325f79
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 63 deletions.
diff --git a/crates/re_space_view_time_series/src/aggregation.rs b/crates/re_space_view_time_series/src/aggregation.rs
@@ -194,7 +194,6 @@ fn are_aggregatable(point1: &PlotPoint, point2: &PlotPoint, window_size: usize)
         attrs,
     } = point1;
     let PlotPointAttrs {
-        label,
         color,
         radius_ui: _,
         kind,
@@ -203,7 +202,6 @@ fn are_aggregatable(point1: &PlotPoint, point2: &PlotPoint, window_size: usize)
     // We cannot aggregate two points that don't live in the same aggregation window to start with.
     // This is very common with e.g. sparse datasets.
     time.abs_diff(point2.time) <= window_size as u64
-        && *label == point2.attrs.label
         && *color == point2.attrs.color
         && *kind == point2.attrs.kind
 }
diff --git a/crates/re_space_view_time_series/src/lib.rs b/crates/re_space_view_time_series/src/lib.rs
@@ -35,7 +35,6 @@ pub(crate) fn plot_id(space_view_id: re_viewer_context::SpaceViewId) -> egui::Id
 
 #[derive(Clone, Debug)]
 pub struct PlotPointAttrs {
-    pub label: Option<Utf8>,
     pub color: egui::Color32,
 
     /// Radius of markers, or stroke radius for lines.
@@ -52,15 +51,11 @@ pub struct ScatterAttrs {
 impl PartialEq for PlotPointAttrs {
     fn eq(&self, rhs: &Self) -> bool {
         let Self {
-            label,
             color,
             radius_ui,
             kind,
         } = self;
-        label.eq(&rhs.label)
-            && color.eq(&rhs.color)
-            && radius_ui.total_cmp(&rhs.radius_ui).is_eq()
-            && kind.eq(&rhs.kind)
+        color.eq(&rhs.color) && radius_ui.total_cmp(&rhs.radius_ui).is_eq() && kind.eq(&rhs.kind)
     }
 }
 

diff --git a/crates/re_space_view_time_series/src/line_visualizer_system.rs b/crates/re_space_view_time_series/src/line_visualizer_system.rs
@@ -78,7 +78,16 @@ impl TypedComponentFallbackProvider<StrokeWidth> for SeriesLineSystem {
     }
 }
 
-re_viewer_context::impl_component_fallback_provider!(SeriesLineSystem => [Color, StrokeWidth]);
+impl TypedComponentFallbackProvider<Name> for SeriesLineSystem {
+    fn fallback_for(&self, ctx: &QueryContext<'_>) -> Name {
+        ctx.target_entity_path
+            .last()
+            .map(|part| part.ui_string().into())
+            .unwrap_or_default()
+    }
+}
+
+re_viewer_context::impl_component_fallback_provider!(SeriesLineSystem => [Color, StrokeWidth, Name]);
 
 impl SeriesLineSystem {
     fn load_scalars(
@@ -160,15 +169,13 @@ impl SeriesLineSystem {
             time: 0,
             value: 0.0,
             attrs: PlotPointAttrs {
-                label: None,
                 color: fallback_color.into(),
                 radius_ui: 0.5 * fallback_stroke_width.0,
                 kind: PlotSeriesKind::Continuous,
             },
         };
 
         let mut points;
-        let mut series_name = Default::default();
 
         let time_range = determine_time_range(
             view_query.latest_at,
@@ -313,21 +320,16 @@ impl SeriesLineSystem {
             }
 
             // Extract the series name
-            // TODO(jleibs): Handle Err values.
-            if let Ok(all_series_name) = results.get_or_empty_dense::<Name>(resolver) {
-                if !matches!(
-                    all_series_name.status(),
-                    (PromiseResult::Ready(()), PromiseResult::Ready(()))
-                ) {
-                    // TODO(#5607): what should happen if the promise is still pending?
-                }
-
-                series_name = all_series_name
-                    .range_data(all_scalars_entry_range.clone())
-                    .next()
-                    .and_then(|name| name.first())
-                    .map(|name| name.0.clone());
-            }
+            let series_name = results
+                .get_or_empty_dense::<Name>(resolver)
+                .ok()
+                .and_then(|all_series_name| {
+                    all_series_name
+                        .range_data(all_scalars_entry_range.clone())
+                        .next()
+                        .and_then(|name| name.first().cloned())
+                })
+                .unwrap_or_else(|| self.fallback_for(&query_ctx));
 
             // Now convert the `PlotPoints` into `Vec<PlotSeries>`
             let aggregator = results
@@ -348,7 +350,7 @@ impl SeriesLineSystem {
                 points,
                 ctx.recording_store(),
                 view_query,
-                series_name,
+                &series_name,
                 aggregator,
                 all_series,
             );

diff --git a/crates/re_space_view_time_series/src/point_visualizer_system.rs b/crates/re_space_view_time_series/src/point_visualizer_system.rs
@@ -5,7 +5,7 @@ use re_space_view::range_with_blueprint_resolved_data;
 use re_types::{
     archetypes::{self, SeriesPoint},
     components::{Color, MarkerShape, MarkerSize, Name, Scalar},
-    Archetype as _, Loggable,
+    Archetype as _, Loggable as _,
 };
 use re_viewer_context::{
     IdentifiedViewSystem, QueryContext, SpaceViewSystemExecutionError,
@@ -80,7 +80,16 @@ impl TypedComponentFallbackProvider<MarkerSize> for SeriesPointSystem {
     }
 }
 
-re_viewer_context::impl_component_fallback_provider!(SeriesPointSystem => [Color, MarkerSize]);
+impl TypedComponentFallbackProvider<Name> for SeriesPointSystem {
+    fn fallback_for(&self, ctx: &QueryContext<'_>) -> Name {
+        ctx.target_entity_path
+            .last()
+            .map(|part| part.ui_string().into())
+            .unwrap_or_default()
+    }
+}
+
+re_viewer_context::impl_component_fallback_provider!(SeriesPointSystem => [Color, MarkerSize, Name]);
 
 impl SeriesPointSystem {
     fn load_scalars(
@@ -118,7 +127,6 @@ impl SeriesPointSystem {
                 time: 0,
                 value: 0.0,
                 attrs: PlotPointAttrs {
-                    label: None,
                     color: fallback_color.into(),
                     radius_ui: fallback_size.into(),
                     kind: PlotSeriesKind::Scatter(ScatterAttrs {
@@ -128,7 +136,6 @@ impl SeriesPointSystem {
             };
 
             let mut points;
-            let mut series_name = Default::default();
 
             let time_range = determine_time_range(
                 view_query.latest_at,
@@ -151,10 +158,11 @@ impl SeriesPointSystem {
                     &query,
                     data_result,
                     [
-                        Scalar::name(),
                         Color::name(),
-                        MarkerSize::name(),
                         MarkerShape::name(),
+                        MarkerSize::name(),
+                        Name::name(),
+                        Scalar::name(),
                     ],
                 );
 
@@ -307,21 +315,16 @@ impl SeriesPointSystem {
                 }
 
                 // Extract the series name
-                // TODO(jleibs): Handle Err values.
-                if let Ok(all_series_name) = results.get_or_empty_dense::<Name>(resolver) {
-                    if !matches!(
-                        all_series_name.status(),
-                        (PromiseResult::Ready(()), PromiseResult::Ready(()))
-                    ) {
-                        // TODO(#5607): what should happen if the promise is still pending?
-                    }
-
-                    series_name = all_series_name
-                        .range_data(all_scalars_entry_range.clone())
-                        .next()
-                        .and_then(|name| name.first())
-                        .map(|name| name.0.clone());
-                }
+                let series_name = results
+                    .get_or_empty_dense::<Name>(resolver)
+                    .ok()
+                    .and_then(|all_series_name| {
+                        all_series_name
+                            .range_data(all_scalars_entry_range.clone())
+                            .next()
+                            .and_then(|name| name.first().cloned())
+                    })
+                    .unwrap_or_else(|| self.fallback_for(&query_ctx));
 
                 // Now convert the `PlotPoints` into `Vec<PlotSeries>`
                 points_to_series(
@@ -330,7 +333,7 @@ impl SeriesPointSystem {
                     points,
                     ctx.recording_store(),
                     view_query,
-                    series_name,
+                    &series_name,
                     // Aggregation for points is not supported.
                     re_types::components::AggregationPolicy::Off,
                     &mut self.all_series,

diff --git a/crates/re_space_view_time_series/src/util.rs b/crates/re_space_view_time_series/src/util.rs
@@ -1,7 +1,7 @@
 use re_log_types::{EntityPath, ResolvedTimeRange};
 use re_types::{
     components::AggregationPolicy,
-    datatypes::{TimeRange, TimeRangeBoundary, Utf8},
+    datatypes::{TimeRange, TimeRangeBoundary},
 };
 use re_viewer_context::{ViewQuery, ViewerContext};
 
@@ -85,7 +85,7 @@ pub fn points_to_series(
     points: Vec<PlotPoint>,
     store: &re_data_store::DataStore,
     query: &ViewQuery<'_>,
-    series_name: Option<Utf8>,
+    series_name: &re_types::components::Name,
     aggregator: AggregationPolicy,
     all_series: &mut Vec<PlotSeries>,
 ) {
@@ -99,13 +99,6 @@ pub fn points_to_series(
         .entity_min_time(&query.timeline, entity_path)
         .map_or(points.first().map_or(0, |p| p.time), |time| time.as_i64());
 
-    let series_label = series_name.unwrap_or_else(|| {
-        let same_label = |points: &[PlotPoint]| -> Option<Utf8> {
-            let label = points[0].attrs.label.as_ref()?;
-            (points.iter().all(|p| p.attrs.label.as_ref() == Some(label))).then(|| label.clone())
-        };
-        same_label(&points).unwrap_or_else(|| entity_path.to_string().into())
-    });
     if points.len() == 1 {
         // Can't draw a single point as a continuous line, so fall back on scatter
         let mut kind = points[0].attrs.kind;
@@ -114,7 +107,7 @@ pub fn points_to_series(
         }
 
         all_series.push(PlotSeries {
-            label: series_label,
+            label: series_name.0.clone(),
             color: points[0].attrs.color,
             radius_ui: points[0].attrs.radius_ui,
             kind,
@@ -126,7 +119,7 @@ pub fn points_to_series(
         });
     } else {
         add_series_runs(
-            &series_label,
+            series_name,
             points,
             entity_path,
             aggregator,
@@ -205,7 +198,7 @@ pub fn apply_aggregation(
 
 #[inline(never)] // Better callstacks on crashes
 fn add_series_runs(
-    series_label: &Utf8,
+    series_name: &re_types::components::Name,
     points: Vec<PlotPoint>,
     entity_path: &EntityPath,
     aggregator: AggregationPolicy,
@@ -218,7 +211,7 @@ fn add_series_runs(
     let num_points = points.len();
     let mut attrs = points[0].attrs.clone();
     let mut series: PlotSeries = PlotSeries {
-        label: series_label.clone(),
+        label: series_name.0.clone(),
         color: attrs.color,
         radius_ui: attrs.radius_ui,
         points: Vec::with_capacity(num_points),
@@ -242,7 +235,7 @@ fn add_series_runs(
             let prev_series = std::mem::replace(
                 &mut series,
                 PlotSeries {
-                    label: series_label.clone(),
+                    label: series_name.0.clone(),
                     color: attrs.color,
                     radius_ui: attrs.radius_ui,
                     kind: attrs.kind,