Skip to content

Commit

Permalink
Configurable dynamic plot aggregation based on zoom-level (#4865)
Browse files Browse the repository at this point in the history
⚠️ [Try it
live!](https://app.rerun.io/pr/4865/index.html?url=https://storage.googleapis.com/rerun-builds/pull_request/4865/plot_gauss2.rrd)
:warning:

Make it so users can configure an aggregation strategy in the rare case
where they either have so much data or are so zoomed out that most of
their plot results in an overdraw blurb.

Because this builds on top of the range cache, the data is neatly laid
out in a memory slice already so this is very cheap to compute.

In my tests, the `MinMax` strategy has worked so well that I've decided
to make it the default in the end... That might be controversial
:no_mouth:.

`Off` vs. `MinMax`, using the [new gaussian walk
benchmark](#4903):
![image
(26)](https://github.com/rerun-io/rerun/assets/2910679/1811becb-d213-44bb-87ea-0e4a7fa058ad)
![image
(27)](https://github.com/rerun-io/rerun/assets/2910679/b8d66c92-8719-4de5-a3cb-72c2ea4b1e96)
 


- Fixes #4271 
- DNR: requires #4856
  • Loading branch information
teh-cmc authored Jan 25, 2024
1 parent 7426951 commit 027bbe1
Show file tree
Hide file tree
Showing 6 changed files with 484 additions and 38 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

94 changes: 94 additions & 0 deletions crates/re_entity_db/src/entity_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ pub struct EntityProperties {
/// This is an Option instead of an EditableAutoValue to let each space view class decide on
/// what's the best default.
pub legend_location: Option<LegendCorner>,

/// What kind of data aggregation to perform (for plot space views).
pub time_series_aggregator: EditableAutoValue<TimeSeriesAggregator>,
}

#[cfg(feature = "serde")]
Expand All @@ -153,6 +156,7 @@ impl Default for EntityProperties {
transform_3d_size: EditableAutoValue::Auto(1.0),
show_legend: EditableAutoValue::Auto(true),
legend_location: None,
time_series_aggregator: EditableAutoValue::Auto(TimeSeriesAggregator::default()),
}
}
}
Expand Down Expand Up @@ -191,6 +195,10 @@ impl EntityProperties {

show_legend: self.show_legend.or(&child.show_legend).clone(),
legend_location: self.legend_location.or(child.legend_location),
time_series_aggregator: self
.time_series_aggregator
.or(&child.time_series_aggregator)
.clone(),
}
}

Expand Down Expand Up @@ -232,6 +240,10 @@ impl EntityProperties {

show_legend: other.show_legend.or(&self.show_legend).clone(),
legend_location: other.legend_location.or(self.legend_location),
time_series_aggregator: other
.time_series_aggregator
.or(&self.time_series_aggregator)
.clone(),
}
}

Expand All @@ -250,6 +262,7 @@ impl EntityProperties {
transform_3d_size,
show_legend,
legend_location,
time_series_aggregator,
} = self;

visible != &other.visible
Expand All @@ -264,6 +277,7 @@ impl EntityProperties {
|| transform_3d_size.has_edits(&other.transform_3d_size)
|| show_legend.has_edits(&other.show_legend)
|| *legend_location != other.legend_location
|| time_series_aggregator.has_edits(&other.time_series_aggregator)
}
}

Expand Down Expand Up @@ -355,3 +369,83 @@ impl From<LegendCorner> for egui_plot::Corner {
}
}
}

// ----------------------------------------------------------------------------

/// What kind of aggregation should be performed when the zoom-level on the X axis goes below 1.0?
///
/// Aggregation affects the points' values and radii.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub enum TimeSeriesAggregator {
/// No aggregation.
Off,

/// Average all points in the range together.
Average,

/// Keep only the maximum values in the range.
Max,

/// Keep only the minimum values in the range.
Min,

/// Keep both the minimum and maximum values in the range.
///
/// This will yield two aggregated points instead of one, effectively creating a vertical line.
#[default]
MinMax,

/// Find both the minimum and maximum values in the range, then use the average of those.
MinMaxAverage,
}

impl std::fmt::Display for TimeSeriesAggregator {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
TimeSeriesAggregator::Off => write!(f, "Off"),
TimeSeriesAggregator::Average => write!(f, "Average"),
TimeSeriesAggregator::Max => write!(f, "Max"),
TimeSeriesAggregator::Min => write!(f, "Min"),
TimeSeriesAggregator::MinMax => write!(f, "MinMax"),
TimeSeriesAggregator::MinMaxAverage => write!(f, "MinMaxAverage"),
}
}
}

impl TimeSeriesAggregator {
#[inline]
pub fn variants() -> [TimeSeriesAggregator; 6] {
// Just making sure this method won't compile if the enum gets modified.
#[allow(clippy::match_same_arms)]
match Self::default() {
TimeSeriesAggregator::Off => {}
TimeSeriesAggregator::Average => {}
TimeSeriesAggregator::Max => {}
TimeSeriesAggregator::Min => {}
TimeSeriesAggregator::MinMax => {}
TimeSeriesAggregator::MinMaxAverage => {}
}

[
TimeSeriesAggregator::Off,
TimeSeriesAggregator::Average,
TimeSeriesAggregator::Max,
TimeSeriesAggregator::Min,
TimeSeriesAggregator::MinMax,
TimeSeriesAggregator::MinMaxAverage,
]
}

#[inline]
pub fn description(&self) -> &'static str {
match self {
TimeSeriesAggregator::Off => "No aggregation.",
TimeSeriesAggregator::Average => "Average all points in the range together.",
TimeSeriesAggregator::Max => "Keep only the maximum values in the range.",
TimeSeriesAggregator::Min => "Keep only the minimum values in the range.",
TimeSeriesAggregator::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.",
TimeSeriesAggregator::MinMaxAverage => "Find both the minimum and maximum values in the range, then use the average of those",
}
}
}
4 changes: 3 additions & 1 deletion crates/re_space_view_time_series/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ all-features = true
[dependencies]
re_data_store.workspace = true
re_format.workspace = true
re_log.workspace = true
re_log_types.workspace = true
re_query.workspace = true
re_query_cache.workspace = true
Expand All @@ -28,6 +29,7 @@ re_types = { workspace = true, features = ["egui_plot"] }
re_ui.workspace = true
re_viewer_context.workspace = true

egui_plot.workspace = true
egui.workspace = true
egui_plot.workspace = true
itertools.workspace = true
parking_lot.workspace = true
12 changes: 12 additions & 0 deletions crates/re_space_view_time_series/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,15 @@ mod space_view_class;
mod visualizer_system;

pub use space_view_class::TimeSeriesSpaceView;

/// Computes a deterministic, globally unique ID for the plot based on the ID of the space view
/// itself.
///
/// Use it to access the plot's state from anywhere, e.g.:
/// ```ignore
/// let plot_mem = egui_plot::PlotMemory::load(egui_ctx, crate::plot_id(query.space_view_id));
/// ```
#[inline]
pub(crate) fn plot_id(space_view_id: re_viewer_context::SpaceViewId) -> egui::Id {
egui::Id::new(("plot", space_view_id))
}
71 changes: 57 additions & 14 deletions crates/re_space_view_time_series/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@ use re_format::next_grid_tick_magnitude_ns;
use re_log_types::{EntityPath, TimeZone};
use re_query::query_archetype;
use re_space_view::controls;
use re_viewer_context::external::re_entity_db::EntityProperties;
use re_viewer_context::external::re_entity_db::{
EditableAutoValue, EntityProperties, TimeSeriesAggregator,
};
use re_viewer_context::{
SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId, SpaceViewState,
SpaceViewSystemExecutionError, SystemExecutionOutput, ViewQuery, ViewerContext,
};

use crate::visualizer_system::{PlotSeriesKind, TimeSeriesSystem};

// ---

#[derive(Clone, Default)]
pub struct TimeSeriesSpaceViewState {
/// Is the user dragging the cursor this frame?
Expand Down Expand Up @@ -101,7 +105,7 @@ impl SpaceViewClass for TimeSeriesSpaceView {
_state: &mut Self::State,
_space_origin: &EntityPath,
space_view_id: SpaceViewId,
_root_entity_properties: &mut EntityProperties,
root_entity_properties: &mut EntityProperties,
) {
let re_types::blueprint::archetypes::TimeSeries { legend } = query_archetype(
ctx.store_context.blueprint.store(),
Expand All @@ -112,7 +116,32 @@ impl SpaceViewClass for TimeSeriesSpaceView {
.unwrap_or_default();

ctx.re_ui
.selection_grid(ui, "time_series_selection_ui")
.selection_grid(ui, "time_series_selection_ui_aggregation")
.show(ui, |ui| {
ctx.re_ui
.grid_left_hand_label(ui, "Aggregation")
.on_hover_text("Configures the aggregation behavior of the plot when the zoom-level on the X axis goes below 1.0, i.e. a single pixel covers more than one tick worth of data.\nThis can greatly improve performance (and readability) in such situations as it prevents overdraw.");

let mut agg_mode = *root_entity_properties.time_series_aggregator.get();

egui::ComboBox::from_id_source("aggregation_mode")
.selected_text(agg_mode.to_string())
.show_ui(ui, |ui| {
ui.style_mut().wrap = Some(false);
ui.set_min_width(64.0);

for variant in TimeSeriesAggregator::variants() {
ui.selectable_value(&mut agg_mode, variant, variant.to_string())
.on_hover_text(variant.description());
}
});

root_entity_properties.time_series_aggregator =
EditableAutoValue::UserEdited(agg_mode);
});

ctx.re_ui
.selection_grid(ui, "time_series_selection_ui_legend")
.show(ui, |ui| {
ctx.re_ui.grid_left_hand_label(ui, "Legend");

Expand Down Expand Up @@ -166,6 +195,7 @@ impl SpaceViewClass for TimeSeriesSpaceView {
ctx.save_blueprint_component(&space_view_id.as_entity_path(), edit_legend);
}
});

ui.end_row();
});
}
Expand All @@ -176,15 +206,15 @@ impl SpaceViewClass for TimeSeriesSpaceView {
ui: &mut egui::Ui,
state: &mut Self::State,
_root_entity_properties: &EntityProperties,
_query: &ViewQuery<'_>,
query: &ViewQuery<'_>,
system_output: SystemExecutionOutput,
) -> Result<(), SpaceViewSystemExecutionError> {
re_tracing::profile_function!();

let re_types::blueprint::archetypes::TimeSeries { legend } = query_archetype(
ctx.store_context.blueprint.store(),
ctx.blueprint_query,
&_query.space_view_id.as_entity_path(),
&query.space_view_id.as_entity_path(),
)
.and_then(|arch| arch.to_archetype())
.unwrap_or_default();
Expand All @@ -202,6 +232,9 @@ impl SpaceViewClass for TimeSeriesSpaceView {

let time_series = system_output.view_systems.get::<TimeSeriesSystem>()?;

let aggregator = time_series.aggregator;
let aggregation_factor = time_series.aggregation_factor;

// Get the minimum time/X value for the entire plot…
let min_time = time_series.min_time.unwrap_or(0);

Expand All @@ -222,6 +255,7 @@ impl SpaceViewClass for TimeSeriesSpaceView {

let time_zone_for_timestamps = ctx.app_options.time_zone_for_timestamps;
let mut plot = Plot::new(plot_id_src)
.id(crate::plot_id(query.space_view_id))
.allow_zoom([true, zoom_both_axis])
.x_axis_formatter(move |time, _, _| {
format_time(
Expand All @@ -232,17 +266,26 @@ impl SpaceViewClass for TimeSeriesSpaceView {
})
.label_formatter(move |name, value| {
let name = if name.is_empty() { "y" } else { name };
let label = time_type.format(
(value.x as i64 + time_offset).into(),
time_zone_for_timestamps
);

let is_integer = value.y.round() == value.y;
let decimals = if is_integer { 0 } else { 5 };
format!(
"{timeline_name}: {}\n{name}: {:.*}",
time_type.format(
(value.x as i64 + time_offset).into(),
time_zone_for_timestamps
),
decimals,
value.y,
)

let agg_range_is_integer = aggregation_factor.round() == aggregation_factor;
let agg_range_decimals = if agg_range_is_integer { 0 } else { 5 };

if aggregator == TimeSeriesAggregator::Off || aggregation_factor <= 1.0 {
format!("{timeline_name}: {label}\n{name}: {:.decimals$}", value.y)
} else {
format!(
"{timeline_name}: {label}\n{name}: {:.decimals$}\n\
Y value aggregated using {aggregator} over {aggregation_factor:.agg_range_decimals$} X increments",
value.y,
)
}
});

if legend.visible {
Expand Down
Loading

0 comments on commit 027bbe1

Please sign in to comment.