Skip to content

Commit 5e91a30

Browse files
authored
TimeSeriesAggregation can now be set per SeriesLine (and as blueprint default per View) (#6558)
### What Adds `TimeSeriesAggreation` component to `SeriesLine`, allowing to set it both per line and as a component default. example: ```python rr.log("trig/sin", rr.SeriesLine(color=[255, 0, 0], name="sin(0.01t)", aggregator="minmax"), static=True) ``` This is good step forward overall but there's problems with this PR * ui is a bit worse #6556 * one would expect this setting still on the view since most of the time one wants to change the aggregation for all lines (i.e. the default): #6557 -- * Part of #4442 * If not already fixed, fixes #6460 ### 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/6558?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/6558?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/6558) - [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`.
1 parent 8382c27 commit 5e91a30

32 files changed

Lines changed: 729 additions & 187 deletions

File tree

crates/re_edit_ui/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ use datatype_editors::edit_enum;
1313
use re_types::{
1414
blueprint::components::{BackgroundKind, Corner2D, LockRangeDuringZoom, Visible},
1515
components::{
16-
AxisLength, Color, Colormap, ImagePlaneDistance, MarkerSize, Name, Radius, StrokeWidth,
17-
Text,
16+
AggregationPolicy, AxisLength, Color, Colormap, ImagePlaneDistance, MarkerSize, Name,
17+
Radius, StrokeWidth, Text,
1818
},
1919
};
2020
use re_viewer_context::ViewerContext;
@@ -67,6 +67,9 @@ pub fn register_editors(registry: &mut re_viewer_context::ComponentUiRegistry) {
6767
registry.add_singleline_editor_ui(|_ctx, ui, value| {
6868
edit_enum(ui, "corner2d", value, &Corner2D::ALL)
6969
});
70+
registry.add_singleline_editor_ui(|_ctx, ui, value| {
71+
edit_enum(ui, "tseriesaggregator", value, &AggregationPolicy::ALL)
72+
});
7073

7174
registry.add_multiline_editor_ui(visual_bounds2d::multiline_edit_visual_bounds2d);
7275
registry.add_singleline_editor_ui(visual_bounds2d::singleline_edit_visual_bounds2d);

crates/re_entity_db/src/entity_properties.rs

Lines changed: 0 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::fmt::Formatter;
2-
31
#[cfg(feature = "serde")]
42
use re_log_types::EntityPath;
53

@@ -99,9 +97,6 @@ pub struct EntityProperties {
9997

10098
/// Used to scale the radii of the points in the resulting point cloud.
10199
pub backproject_radius_scale: EditableAutoValue<f32>, // TODO(andreas): should be a component on the DepthImage archetype.
102-
103-
/// What kind of data aggregation to perform (for plot space views).
104-
pub time_series_aggregator: EditableAutoValue<TimeSeriesAggregator>, // TODO(andreas): Should be a component probably on SeriesLine, but today it would become a view property.
105100
}
106101

107102
#[cfg(feature = "serde")]
@@ -110,7 +105,6 @@ impl Default for EntityProperties {
110105
Self {
111106
test_property: true,
112107
backproject_radius_scale: EditableAutoValue::Auto(1.0),
113-
time_series_aggregator: EditableAutoValue::Auto(TimeSeriesAggregator::default()),
114108
}
115109
}
116110
}
@@ -126,11 +120,6 @@ impl EntityProperties {
126120
.backproject_radius_scale
127121
.or(&child.backproject_radius_scale)
128122
.clone(),
129-
130-
time_series_aggregator: self
131-
.time_series_aggregator
132-
.or(&child.time_series_aggregator)
133-
.clone(),
134123
}
135124
}
136125

@@ -149,11 +138,6 @@ impl EntityProperties {
149138
.backproject_radius_scale
150139
.or(&self.backproject_radius_scale)
151140
.clone(),
152-
153-
time_series_aggregator: other
154-
.time_series_aggregator
155-
.or(&self.time_series_aggregator)
156-
.clone(),
157141
}
158142
}
159143

@@ -162,91 +146,9 @@ impl EntityProperties {
162146
let Self {
163147
test_property,
164148
backproject_radius_scale,
165-
time_series_aggregator,
166149
} = self;
167150

168151
test_property != &other.test_property
169152
|| backproject_radius_scale.has_edits(&other.backproject_radius_scale)
170-
|| time_series_aggregator.has_edits(&other.time_series_aggregator)
171-
}
172-
}
173-
174-
// ----------------------------------------------------------------------------
175-
176-
/// What kind of aggregation should be performed when the zoom-level on the X axis goes below 1.0?
177-
///
178-
/// Aggregation affects the points' values and radii.
179-
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
180-
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
181-
pub enum TimeSeriesAggregator {
182-
/// No aggregation.
183-
Off,
184-
185-
/// Average all points in the range together.
186-
Average,
187-
188-
/// Keep only the maximum values in the range.
189-
Max,
190-
191-
/// Keep only the minimum values in the range.
192-
Min,
193-
194-
/// Keep both the minimum and maximum values in the range.
195-
///
196-
/// This will yield two aggregated points instead of one, effectively creating a vertical line.
197-
#[default]
198-
MinMax,
199-
200-
/// Find both the minimum and maximum values in the range, then use the average of those.
201-
MinMaxAverage,
202-
}
203-
204-
impl std::fmt::Display for TimeSeriesAggregator {
205-
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
206-
match self {
207-
Self::Off => write!(f, "Off"),
208-
Self::Average => write!(f, "Average"),
209-
Self::Max => write!(f, "Max"),
210-
Self::Min => write!(f, "Min"),
211-
Self::MinMax => write!(f, "MinMax"),
212-
Self::MinMaxAverage => write!(f, "MinMaxAverage"),
213-
}
214-
}
215-
}
216-
217-
impl TimeSeriesAggregator {
218-
#[inline]
219-
pub fn variants() -> [Self; 6] {
220-
// Just making sure this method won't compile if the enum gets modified.
221-
#[allow(clippy::match_same_arms)]
222-
match Self::default() {
223-
Self::Off => {}
224-
Self::Average => {}
225-
Self::Max => {}
226-
Self::Min => {}
227-
Self::MinMax => {}
228-
Self::MinMaxAverage => {}
229-
}
230-
231-
[
232-
Self::Off,
233-
Self::Average,
234-
Self::Max,
235-
Self::Min,
236-
Self::MinMax,
237-
Self::MinMaxAverage,
238-
]
239-
}
240-
241-
#[inline]
242-
pub fn description(&self) -> &'static str {
243-
match self {
244-
Self::Off => "No aggregation.",
245-
Self::Average => "Average all points in the range together.",
246-
Self::Max => "Keep only the maximum values in the range.",
247-
Self::Min => "Keep only the minimum values in the range.",
248-
Self::MinMax => "Keep both the minimum and maximum values in the range.\nThis will yield two aggregated points instead of one, effectively creating a vertical line.",
249-
Self::MinMaxAverage => "Find both the minimum and maximum values in the range, then use the average of those",
250-
}
251153
}
252154
}

crates/re_entity_db/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
#![doc = document_features::document_features!()]
55
//!
66
7+
#![allow(unused_imports)] // TODO(andreas): temporary until entity_properties is removed.
8+
79
pub mod entity_db;
810
pub mod entity_properties;
911
pub mod entity_tree;

crates/re_space_view_time_series/src/aggregation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{PlotPoint, PlotPointAttrs};
22

3-
/// Implements aggregation behaviors corresponding to [`TimeSeriesAggregator`][re_viewer_context::external::re_entity_db::TimeSeriesAggregator]:
3+
/// Implements aggregation behaviors corresponding to [`AggregationPolicy`][re_viewer_context::external::re_entity_db::AggregationPolicy]:
44
/// `Average`
55
pub struct AverageAggregator;
66

@@ -76,7 +76,7 @@ impl AverageAggregator {
7676
}
7777
}
7878

79-
/// Implements aggregation behaviors corresponding to [`TimeSeriesAggregator`][re_viewer_context::external::re_entity_db::TimeSeriesAggregator]:
79+
/// Implements aggregation behaviors corresponding to [`AggregationPolicy`][re_viewer_context::external::re_entity_db::AggregationPolicy]:
8080
/// `Min`, `Max`, `MinMax`, and `MinMaxAverage`.
8181
pub enum MinMaxAggregator {
8282
/// Keep only the maximum values in the range.

crates/re_space_view_time_series/src/lib.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ mod space_view_class;
1313
mod util;
1414

1515
use re_log_types::EntityPath;
16-
use re_types::{components::MarkerShape, datatypes::Utf8};
17-
use re_viewer_context::external::re_entity_db::TimeSeriesAggregator;
16+
use re_types::{
17+
components::{AggregationPolicy, MarkerShape},
18+
datatypes::Utf8,
19+
};
1820
pub use space_view_class::TimeSeriesSpaceView;
1921

2022
/// Computes a deterministic, globally unique ID for the plot based on the ID of the space view
@@ -88,7 +90,7 @@ pub struct PlotSeries {
8890
pub min_time: i64,
8991

9092
/// What kind of aggregation was used to compute the graph?
91-
pub aggregator: TimeSeriesAggregator,
93+
pub aggregator: AggregationPolicy,
9294

9395
/// `1.0` for raw data.
9496
///

crates/re_space_view_time_series/src/line_visualizer_system.rs

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use itertools::Itertools as _;
22
use re_query::{PromiseResult, QueryError};
33
use re_space_view::range_with_blueprint_resolved_data;
44
use re_types::archetypes;
5+
use re_types::components::AggregationPolicy;
56
use re_types::{
67
archetypes::SeriesLine,
78
components::{Color, Name, Scalar, StrokeWidth},
@@ -152,15 +153,8 @@ impl SeriesLineSystem {
152153
let current_query = ctx.current_query();
153154
let query_ctx = ctx.query_context(data_result, &current_query);
154155

155-
let fallback_color =
156-
re_viewer_context::TypedComponentFallbackProvider::<Color>::fallback_for(
157-
self, &query_ctx,
158-
);
159-
160-
let fallback_stroke =
161-
re_viewer_context::TypedComponentFallbackProvider::<StrokeWidth>::fallback_for(
162-
self, &query_ctx,
163-
);
156+
let fallback_color: Color = self.fallback_for(&query_ctx);
157+
let fallback_stroke: StrokeWidth = self.fallback_for(&query_ctx);
164158

165159
// All the default values for a `PlotPoint`, accounting for both overrides and default
166160
// values.
@@ -202,6 +196,7 @@ impl SeriesLineSystem {
202196
Color::name(),
203197
StrokeWidth::name(),
204198
Name::name(),
199+
AggregationPolicy::name(),
205200
],
206201
);
207202

@@ -337,13 +332,26 @@ impl SeriesLineSystem {
337332
}
338333

339334
// Now convert the `PlotPoints` into `Vec<PlotSeries>`
335+
let aggregator = results
336+
.get_or_empty_dense::<AggregationPolicy>(resolver)
337+
.ok()
338+
.and_then(|result| {
339+
result
340+
.range_data(all_scalars_entry_range.clone())
341+
.next()
342+
.and_then(|aggregator| aggregator.first().copied())
343+
})
344+
// TODO(andreas): Relying on the default==placeholder here instead of going through a fallback provider.
345+
// This is fine, because we know there's no `TypedFallbackProvider`, but wrong if one were to be added.
346+
.unwrap_or_default();
340347
points_to_series(
341-
data_result,
348+
&data_result.entity_path,
342349
time_per_pixel,
343350
points,
344351
ctx.recording_store(),
345352
view_query,
346353
series_name,
354+
aggregator,
347355
all_series,
348356
);
349357
}

crates/re_space_view_time_series/src/point_visualizer_system.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,14 @@ impl SeriesPointSystem {
327327

328328
// Now convert the `PlotPoints` into `Vec<PlotSeries>`
329329
points_to_series(
330-
data_result,
330+
&data_result.entity_path,
331331
time_per_pixel,
332332
points,
333333
ctx.recording_store(),
334334
view_query,
335335
series_name,
336+
// Aggregation for points is not supported.
337+
re_types::components::AggregationPolicy::Off,
336338
&mut self.all_series,
337339
);
338340
}

crates/re_space_view_time_series/src/space_view_class.rs

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@ use re_log_types::{EntityPath, TimeInt, TimeZone};
88
use re_space_view::{controls, view_property_ui};
99
use re_types::blueprint::archetypes::{PlotLegend, ScalarAxis};
1010
use re_types::blueprint::components::{Corner2D, LockRangeDuringZoom, Visible};
11+
use re_types::components::AggregationPolicy;
1112
use re_types::{components::Range1D, datatypes::TimeRange, SpaceViewClassIdentifier, View};
1213
use re_ui::{list_item, UiExt as _};
13-
use re_viewer_context::external::re_entity_db::{
14-
EditableAutoValue, EntityProperties, TimeSeriesAggregator,
15-
};
14+
use re_viewer_context::external::re_entity_db::EntityProperties;
1615
use re_viewer_context::{
1716
ApplicableEntities, IdentifiedViewSystem, IndicatedEntities, PerVisualizer, QueryRange,
1817
RecommendedSpaceView, SmallVisualizerSet, SpaceViewClass, SpaceViewClassRegistryError,
@@ -144,42 +143,11 @@ impl SpaceViewClass for TimeSeriesSpaceView {
144143
state: &mut dyn SpaceViewState,
145144
_space_origin: &EntityPath,
146145
space_view_id: SpaceViewId,
147-
root_entity_properties: &mut EntityProperties,
146+
_root_entity_properties: &mut EntityProperties,
148147
) -> Result<(), SpaceViewSystemExecutionError> {
149148
let state = state.downcast_mut::<TimeSeriesSpaceViewState>()?;
150149

151150
list_item::list_item_scope(ui, "time_series_selection_ui", |ui| {
152-
ui.list_item()
153-
.interactive(false)
154-
.show_hierarchical(
155-
ui,
156-
list_item::PropertyContent::new("Zoom aggregation").value_fn(|ui, _| {
157-
let mut agg_mode = *root_entity_properties.time_series_aggregator.get();
158-
159-
egui::ComboBox::from_id_source("aggregation_mode")
160-
.selected_text(agg_mode.to_string())
161-
.show_ui(ui, |ui| {
162-
for variant in TimeSeriesAggregator::variants() {
163-
ui.selectable_value(
164-
&mut agg_mode,
165-
variant,
166-
variant.to_string(),
167-
)
168-
.on_hover_text(variant.description());
169-
}
170-
});
171-
172-
root_entity_properties.time_series_aggregator =
173-
EditableAutoValue::UserEdited(agg_mode);
174-
}),
175-
)
176-
.on_hover_text(
177-
"Configures the zoom-dependent scalar aggregation.\n\
178-
This is done only if steps on the X axis go below 1.0, i.e. a single pixel \
179-
covers more than one tick worth of data. It can greatly improve performance \
180-
(and readability) in such situations as it prevents overdraw.",
181-
);
182-
183151
view_property_ui::<PlotLegend>(ctx, ui, space_view_id, self, state);
184152
view_property_ui::<ScalarAxis>(ctx, ui, space_view_id, self, state);
185153
});
@@ -398,7 +366,7 @@ impl SpaceViewClass for TimeSeriesSpaceView {
398366

399367
let y_value = re_format::format_f64(value.y);
400368

401-
if aggregator == TimeSeriesAggregator::Off || aggregation_factor <= 1.0 {
369+
if aggregator == AggregationPolicy::Off || aggregation_factor <= 1.0 {
402370
format!("{timeline_name}: {label}\n{name}: {y_value}")
403371
} else {
404372
format!(

0 commit comments

Comments
 (0)