Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Scalar, SeriesLine, and SeriesPoint archetypes with their own visualizers #4875

Merged
merged 37 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
11cc4f2
Introduce SeriesLine and SeriesPoint archetypes
jleibs Jan 19, 2024
aacd520
Add a trimmed down Scalar archetype
jleibs Jan 30, 2024
9d49bcd
codegen
jleibs Jan 19, 2024
a385c17
Refactor aggregation out of visualizer_system
jleibs Jan 30, 2024
2825e1c
Refactor override utilities out of visualizer_system
jleibs Jan 30, 2024
3dfd3bb
Make the existing TimeSeriesSystem legacy
jleibs Jan 30, 2024
a1529ac
Copy-pasta a LineVisualizerSystem
jleibs Jan 30, 2024
0916895
defend against inverted time ranges
teh-cmc Jan 31, 2024
8c8ff28
More copy-pasta
jleibs Jan 31, 2024
37d89ba
New blueprint component for specifying visualizers
jleibs Jan 31, 2024
9e40916
codegen
jleibs Jan 31, 2024
1a99f36
Use the Visualizers override to drive with visualizers are run
jleibs Jan 31, 2024
20446a4
Don't show components that aren't relevant to the visualizers
jleibs Jan 31, 2024
bc9391b
dont forbid it entirely, there are still good reasons to do this
teh-cmc Feb 1, 2024
24967b7
pushing boundaries, literally
teh-cmc Feb 1, 2024
c099d5a
review
teh-cmc Feb 1, 2024
c0acdee
fmt
teh-cmc Feb 1, 2024
507e716
Pre-merge #4994
jleibs Feb 1, 2024
1824fd5
Factor out common plot operations to utils
jleibs Feb 1, 2024
e3e84d5
Merge branch 'main' into jleibs/series_style
jleibs Feb 1, 2024
a0f20dd
Fix codegen
jleibs Feb 1, 2024
cacd6e2
Naming the legacy_visualizer_system back to visualizer_system to avoi…
jleibs Feb 1, 2024
60530c3
Fix use statements from rename
jleibs Feb 1, 2024
4b7aef9
Lots of lines->series renames
jleibs Feb 1, 2024
b5ccb44
Clean up and move around archetypes
jleibs Feb 1, 2024
697030c
codegen
jleibs Feb 1, 2024
b959504
Factor out determination of plot bounds and delta
jleibs Feb 1, 2024
faafcbe
Reference code that handles the overriding
jleibs Feb 1, 2024
6bf2f99
Merge branch 'main' into jleibs/series_style
jleibs Feb 1, 2024
c6756b2
More doc cleanup
jleibs Feb 1, 2024
7687334
Merge branch 'main' into jleibs/series_style
jleibs Feb 1, 2024
009b153
spell
jleibs Feb 1, 2024
457842b
Fix the spawn heuristic
jleibs Feb 1, 2024
2ecadab
lint
jleibs Feb 1, 2024
4fbb760
pydoc index
jleibs Feb 1, 2024
edadbb8
Opt out of roundtrips
jleibs Feb 1, 2024
ff87c9e
Awkward doc reference
jleibs Feb 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions crates/re_space_view_time_series/src/aggregation.rs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just moving all the aggregation pieces out of visualizer_system.

Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use crate::{PlotPoint, PlotPointAttrs};

/// Implements aggregation behavior corresponding to [`TimeSeriesAggregator::Average`].
/// Implements aggregation behaviors corresponding to [`TimeSeriesAggregator`][re_viewer_context::external::re_entity_db::TimeSeriesAggregator]:
/// `Average`
pub struct AverageAggregator;

impl AverageAggregator {
/// `aggregation_factor`: the width of the aggregation window.
#[inline]
pub fn aggregate(aggregation_factor: f64, points: &[PlotPoint]) -> Vec<PlotPoint> {
let min_time = points.first().map_or(i64::MIN, |p| p.time);
Expand Down Expand Up @@ -72,9 +74,8 @@ impl AverageAggregator {
}
}

/// Implements aggregation behaviors corresponding to [`TimeSeriesAggregator::Max`],
/// [`TimeSeriesAggregator::Min`], [`TimeSeriesAggregator::MinMax`] and
/// [`TimeSeriesAggregator::MinMaxAverage`], .
/// Implements aggregation behaviors corresponding to [`TimeSeriesAggregator`][re_viewer_context::external::re_entity_db::TimeSeriesAggregator]:
/// `Min`, `Max`, `MinMax`, and `MinMaxAverage`.
pub enum MinMaxAggregator {
/// Keep only the maximum values in the range.
Max,
Expand All @@ -93,19 +94,19 @@ pub enum MinMaxAggregator {

impl MinMaxAggregator {
#[inline]
pub fn aggregate(&self, aggregation_factor: f64, points: &[PlotPoint]) -> Vec<PlotPoint> {
pub fn aggregate(&self, aggregation_window_size: f64, points: &[PlotPoint]) -> Vec<PlotPoint> {
// NOTE: `round()` since this can only handle discrete window sizes.
let window_size = usize::max(1, aggregation_window_size.round() as usize);

let min_time = points.first().map_or(i64::MIN, |p| p.time);
let max_time = points.last().map_or(i64::MAX, |p| p.time);

let capacity = (points.len() as f64 / aggregation_factor) as usize;
let capacity = (points.len() as f64 / window_size as f64) as usize;
let mut aggregated = match self {
MinMaxAggregator::MinMax => Vec::with_capacity(capacity * 2),
_ => Vec::with_capacity(capacity),
};

// NOTE: `round()` since this can only handle discrete window sizes.
let window_size = usize::max(1, aggregation_factor.round() as usize);

let mut i = 0;
while i < points.len() {
let mut j = 0;
Expand Down
3 changes: 3 additions & 0 deletions crates/re_space_view_time_series/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,8 @@ pub struct PlotSeries {
pub aggregator: TimeSeriesAggregator,

/// `1.0` for raw data.
///
/// How many raw data points were aggregated into a single step of the graph?
/// This is an average.
pub aggregation_factor: f64,
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ use re_viewer_context::{
};

use crate::overrides::initial_override_color;
use crate::util::{determine_plot_bounds_and_delta, determine_time_range, points_to_series};
use crate::util::{
determine_plot_bounds_and_time_per_pixel, determine_time_range, points_to_series,
};
use crate::{overrides::lookup_override, PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind};

/// The system for rendering [`SeriesLine`] archetypes.
Expand Down Expand Up @@ -91,7 +93,7 @@ impl SeriesLineSystem {
let query_caches = ctx.entity_db.query_caches();
let store = ctx.entity_db.store();

let (plot_bounds, plot_value_delta) = determine_plot_bounds_and_delta(ctx, query);
let (plot_bounds, time_per_pixel) = determine_plot_bounds_and_time_per_pixel(ctx, query);

// TODO(cmc): this should be thread-pooled in case there are a gazillon series in the same plot…
for data_result in query.iter_visible_data_results(Self::identifier()) {
Expand Down Expand Up @@ -191,7 +193,7 @@ impl SeriesLineSystem {
// Now convert the `PlotPoints` into `Vec<PlotSeries>`
points_to_series(
data_result,
plot_value_delta,
time_per_pixel,
points,
store,
query,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ use re_viewer_context::{
};

use crate::overrides::initial_override_color;
use crate::util::{determine_plot_bounds_and_delta, determine_time_range, points_to_series};
use crate::util::{
determine_plot_bounds_and_time_per_pixel, determine_time_range, points_to_series,
};
use crate::{overrides::lookup_override, PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind};

/// The system for rendering [`SeriesPoint`] archetypes.
Expand Down Expand Up @@ -91,7 +93,7 @@ impl SeriesPointSystem {
let query_caches = ctx.entity_db.query_caches();
let store = ctx.entity_db.store();

let (plot_bounds, plot_value_delta) = determine_plot_bounds_and_delta(ctx, query);
let (plot_bounds, time_per_pixel) = determine_plot_bounds_and_time_per_pixel(ctx, query);

// TODO(cmc): this should be thread-pooled in case there are a gazillon series in the same plot…
for data_result in query.iter_visible_data_results(Self::identifier()) {
Expand Down Expand Up @@ -191,7 +193,7 @@ impl SeriesPointSystem {
// Now convert the `PlotPoints` into `Vec<PlotSeries>`
points_to_series(
data_result,
plot_value_delta,
time_per_pixel,
points,
store,
query,
Expand Down
26 changes: 18 additions & 8 deletions crates/re_space_view_time_series/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use re_viewer_context::external::re_entity_db::{
EditableAutoValue, EntityProperties, TimeSeriesAggregator,
};
use re_viewer_context::{
IdentifiedViewSystem, RecommendedSpaceView, SpaceViewClass, SpaceViewClassRegistryError,
SpaceViewId, SpaceViewSpawnHeuristics, SpaceViewState, SpaceViewSystemExecutionError,
SystemExecutionOutput, ViewQuery, ViewerContext,
IdentifiedViewSystem, IndicatedEntities, RecommendedSpaceView, SpaceViewClass,
SpaceViewClassRegistryError, SpaceViewId, SpaceViewSpawnHeuristics, SpaceViewState,
SpaceViewSystemExecutionError, SystemExecutionOutput, ViewQuery, ViewerContext,
};

use crate::line_visualizer_system::SeriesLineSystem;
Expand Down Expand Up @@ -214,12 +214,22 @@ It can greatly improve performance (and readability) in such situations as it pr
re_tracing::profile_function!();

// For all following lookups, checking indicators is enough, since we know that this is enough to infer visualizability here.
let Some(indicated_entities) = ctx
.indicated_entities_per_visualizer
.get(&LegacyTimeSeriesSystem::identifier())
else {
let mut indicated_entities = IndicatedEntities::default();

for indicated in [
LegacyTimeSeriesSystem::identifier(),
SeriesLineSystem::identifier(),
SeriesPointSystem::identifier(),
]
Comment on lines +219 to +223
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm if one would only log the new scalar archetype, then this wouldn't spawn a space view. But then again we also don't know what visualizer to activate right now, so I figure this is correct? All a bit confusing with the Legacy one around right now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually works because the SeriesLineSystem and SeriesPointSystem both accept Scalar as an indicator.

.iter()
.filter_map(|&system_id| ctx.indicated_entities_per_visualizer.get(&system_id))
{
indicated_entities.0.extend(indicated.0.iter().cloned());
}

if indicated_entities.0.is_empty() {
return SpaceViewSpawnHeuristics::default();
};
}

// Spawn time series data at the root if there's time series data either
// directly at the root or one of its children.
Expand Down
58 changes: 41 additions & 17 deletions crates/re_space_view_time_series/src/util.rs
Wumpf marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,23 @@ use crate::{
};

/// Find the plot bounds and the per-ui-point delta from egui.
pub fn determine_plot_bounds_and_delta(
pub fn determine_plot_bounds_and_time_per_pixel(
ctx: &ViewerContext<'_>,
query: &ViewQuery<'_>,
) -> (Option<egui_plot::PlotBounds>, f64) {
let egui_ctx = &ctx.re_ui.egui_ctx;

let plot_mem = egui_plot::PlotMemory::load(egui_ctx, crate::plot_id(query.space_view_id));
let plot_bounds = plot_mem.as_ref().map(|mem| *mem.bounds());
// What's the delta in value of X across two adjacent UI points?
// I.e. think of GLSL's `dpdx()`.
let plot_value_delta = plot_mem.as_ref().map_or(1.0, |mem| {
1.0 / mem.transform().dpos_dvalue_x().max(f64::EPSILON)
});
(plot_bounds, plot_value_delta)

// How many ui points per time unit?
let points_per_time = plot_mem
.as_ref()
.map_or(1.0, |mem| mem.transform().dpos_dvalue_x());
let pixels_per_time = egui_ctx.pixels_per_point() as f64 * points_per_time;
// How many time units per physical pixel?
let time_per_pixel = 1.0 / pixels_per_time.max(f64::EPSILON);
(plot_bounds, time_per_pixel)
}

pub fn determine_time_range(
Expand Down Expand Up @@ -68,7 +71,7 @@ pub fn determine_time_range(
// we notice a change in attributes, we need a new series.
pub fn points_to_series(
data_result: &re_viewer_context::DataResult,
plot_value_delta: f64,
time_per_pixel: f64,
points: Vec<PlotPoint>,
store: &re_data_store::DataStore,
query: &ViewQuery<'_>,
Expand All @@ -83,10 +86,11 @@ pub fn points_to_series(
.accumulated_properties()
.time_series_aggregator
.get();
let (aggregation_factor, points) = apply_aggregation(aggregator, plot_value_delta, points);
let (aggregation_factor, points) = apply_aggregation(aggregator, time_per_pixel, points, query);
let min_time = store
.entity_min_time(&query.timeline, &data_result.entity_path)
.map_or(points.first().map_or(0, |p| p.time), |time| time.as_i64());

let same_label = |points: &[PlotPoint]| -> Option<String> {
let label = points[0].attrs.label.as_ref()?;
(points.iter().all(|p| p.attrs.label.as_ref() == Some(label))).then(|| label.clone())
Expand Down Expand Up @@ -126,39 +130,59 @@ pub fn points_to_series(
/// Apply the given aggregation to the provided points.
pub fn apply_aggregation(
aggregator: TimeSeriesAggregator,
plot_value_delta: f64,
time_per_pixel: f64,
points: Vec<PlotPoint>,
query: &ViewQuery<'_>,
) -> (f64, Vec<PlotPoint>) {
let aggregation_factor = plot_value_delta;
// Aggregate over this many time units.
//
// MinMax does zig-zag between min and max, which causes a very jagged look.
// It can be mitigated by lowering the aggregation duration, but that causes
// a lot more work for the tessellator and renderer.
// TODO(#4969): output a thicker line instead of zig-zagging.
let aggregation_duration = time_per_pixel; // aggregate all points covering one physical pixel

// So it can be displayed in the UI by the SpaceViewClass.
let num_points_before = points.len() as f64;
let points = if aggregation_factor > 2.0 {

let points = if aggregation_duration > 2.0 {
re_tracing::profile_scope!("aggregate", aggregator.to_string());

#[allow(clippy::match_same_arms)] // readability
match aggregator {
TimeSeriesAggregator::Off => points,
TimeSeriesAggregator::Average => {
AverageAggregator::aggregate(aggregation_factor, &points)
AverageAggregator::aggregate(aggregation_duration, &points)
}
TimeSeriesAggregator::Min => {
MinMaxAggregator::Min.aggregate(aggregation_factor, &points)
MinMaxAggregator::Min.aggregate(aggregation_duration, &points)
}
TimeSeriesAggregator::Max => {
MinMaxAggregator::Max.aggregate(aggregation_factor, &points)
MinMaxAggregator::Max.aggregate(aggregation_duration, &points)
}
TimeSeriesAggregator::MinMax => {
MinMaxAggregator::MinMax.aggregate(aggregation_factor, &points)
MinMaxAggregator::MinMax.aggregate(aggregation_duration, &points)
}
TimeSeriesAggregator::MinMaxAverage => {
MinMaxAggregator::MinMaxAverage.aggregate(aggregation_factor, &points)
MinMaxAggregator::MinMaxAverage.aggregate(aggregation_duration, &points)
}
}
} else {
points
};

let num_points_after = points.len() as f64;
let actual_aggregation_factor = num_points_before / num_points_after;

re_log::trace!(
id = %query.space_view_id,
?aggregator,
aggregation_duration,
num_points_before,
num_points_after,
actual_aggregation_factor,
);

(actual_aggregation_factor, points)
}

Expand Down
8 changes: 4 additions & 4 deletions crates/re_space_view_time_series/src/visualizer_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ use re_viewer_context::{

use crate::{
overrides::{initial_override_color, lookup_override},
util::{determine_plot_bounds_and_delta, determine_time_range, points_to_series},
util::{determine_plot_bounds_and_time_per_pixel, determine_time_range, points_to_series},
PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind,
};

/// The legacy system for rendering [`TimeSeriesScalars`] archetypes.
/// The legacy system for rendering [`TimeSeriesScalar`] archetypes.
#[derive(Default, Debug)]
pub struct LegacyTimeSeriesSystem {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename the file as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally did but it made the git diff much more annoying because it failed to track the move

pub annotation_map: AnnotationMap,
Expand Down Expand Up @@ -86,7 +86,7 @@ impl LegacyTimeSeriesSystem {
let query_caches = ctx.entity_db.query_caches();
let store = ctx.entity_db.store();

let (plot_bounds, plot_value_delta) = determine_plot_bounds_and_delta(ctx, query);
let (plot_bounds, time_per_pixel) = determine_plot_bounds_and_time_per_pixel(ctx, query);

// TODO(cmc): this should be thread-pooled in case there are a gazillon series in the same plot…
for data_result in query.iter_visible_data_results(Self::identifier()) {
Expand Down Expand Up @@ -195,7 +195,7 @@ impl LegacyTimeSeriesSystem {
// Now convert the `PlotPoints` into `Vec<PlotSeries>`
points_to_series(
data_result,
plot_value_delta,
time_per_pixel,
points,
store,
query,
Expand Down
7 changes: 4 additions & 3 deletions crates/re_types/definitions/rerun/archetypes/scalar.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ namespace rerun.archetypes;
/// The current timeline value will be used for the time/X-axis, hence scalars
/// cannot be timeless.
///
/// WHen used to produce a plot, this archetytpe is used to provide the data that
/// When used to produce a plot, this archetype is used to provide the data that
/// is referenced by the `SeriesLine` or `SeriesPoint` archetypes. You can do
/// this by logging both archetypes to the same path, or alternatively configuring
/// the plot-specific archetypes through the blueprint.
///
/// \py See also [`SeriesLine`][rerun.archetypes.SeriesLine].
/// \rs See also [`SeriesPoint`][crate::archetypes::SeriesPoint].
/// \py See also [`SeriesPoint`][rerun.archetypes.SeriesPoint], [`SeriesLine`][rerun.archetypes.SeriesLine].
/// \rs See also [`SeriesPoint`][crate::archetypes::SeriesPoint], [`SeriesLine`][crate::archetypes::SeriesLine].
/// \cpp See also `rerun::archetypes::SeriesPoint`, `rerun::archetypes::SeriesLine`.
///
jleibs marked this conversation as resolved.
Show resolved Hide resolved
/// \example scalar_simple title="Simple line plot" image="https://static.rerun.io/scalar_simple/8bcc92f56268739f8cd24d60d1fe72a655f62a46/1200w.png"
/// \example scalar_multiple_plots !api title="Multiple time series plots" image="https://static.rerun.io/scalar_multiple/15845c2a348f875248fbd694e03eabd922741c4c/1200w.png"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ namespace rerun.archetypes;
/// The current simulation time will be used for the time/X-axis, hence scalars
/// cannot be timeless!
///
/// This archetype is in the process of being deprecated. Prefer usage of
/// `Scalar`, `SeriesLine`, and `SeriesPoint` instead.
///
/// \py See also [`Scalar`][rerun.archetypes.Scalar], [`SeriesPoint`][rerun.archetypes.SeriesPoint], [`SeriesLine`][rerun.archetypes.SeriesLine].
/// \rs See also [`Scalar`][crate::archetypes.Scalar], [`SeriesPoint`][crate::archetypes::SeriesPoint], [`SeriesLine`][crate::archetypes::SeriesLine].
/// \cpp See also `rerun::archetypes::Scalar`, `rerun::archetypes::SeriesPoint`, `rerun::archetypes::SeriesLine`.
///
/// \example scalar_simple title="Simple line plot" image="https://static.rerun.io/scalar_simple/8bcc92f56268739f8cd24d60d1fe72a655f62a46/1200w.png"
/// \example scalar_multiple_plots !api title="Multiple time series plots" image="https://static.rerun.io/scalar_multiple/15845c2a348f875248fbd694e03eabd922741c4c/1200w.png"
table TimeSeriesScalar (
Expand Down
4 changes: 2 additions & 2 deletions crates/re_types/src/archetypes/scalar.rs

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

5 changes: 5 additions & 0 deletions crates/re_types/src/archetypes/time_series_scalar.rs

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

2 changes: 1 addition & 1 deletion docs/content/reference/types/archetypes/scalar.md

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

3 changes: 3 additions & 0 deletions docs/content/reference/types/archetypes/time_series_scalar.md

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

Loading
Loading