Skip to content

Commit db6fea3

Browse files
grtlrWumpf
andauthored
Set timeline with most events as default timeline (#11217)
### Related * Closes https://linear.app/rerun/issue/RR-2282/better-default-timeline-when-loading-mcap. * Related: #10739 ### What This PR changes the `default_timeline` logic (when no timeline was selected by the user) to always switch to the timeline with the most events on it. It also adds the number of events to the timeline selector in the time panel. > [!IMPORTANT] > It would be very nice if we could right-align the subdued text. That way it would be much easier to see, at a glance, which of the timelines has the most events (because the text would be the longest). **If someone knows how to do this quickly, please point out in the review.** | dark | light | |--------|--------| | <img width="233" height="199" alt="image" src="https://github.com/user-attachments/assets/2c3eacf0-2a5e-4ffb-b334-a8ba3af28159" /> | <img width="233" height="201" alt="image" src="https://github.com/user-attachments/assets/96d094cc-596d-4a2e-ae95-4a55201fe0b1" /> | --------- Co-authored-by: Andreas Reich <[email protected]>
1 parent 3c88bd3 commit db6fea3

File tree

4 files changed

+106
-39
lines changed

4 files changed

+106
-39
lines changed

crates/store/re_entity_db/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub use self::{
2020
instance_path::{InstancePath, InstancePathHash},
2121
store_bundle::{StoreBundle, StoreLoadError},
2222
time_histogram_per_timeline::{TimeHistogram, TimeHistogramPerTimeline},
23-
times_per_timeline::{TimeCounts, TimesPerTimeline},
23+
times_per_timeline::{TimeCounts, TimelineStats, TimesPerTimeline},
2424
versioned_instance_path::{VersionedInstancePath, VersionedInstancePathHash},
2525
};
2626

crates/store/re_entity_db/src/times_per_timeline.rs

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,30 @@ use re_log_types::{TimeInt, Timeline};
88

99
pub type TimeCounts = BTreeMap<TimeInt, u64>;
1010

11-
#[derive(Clone)]
11+
#[derive(Clone, Debug)]
1212
pub struct TimelineStats {
1313
pub timeline: Timeline,
1414
pub per_time: TimeCounts,
15+
pub total_count: u64,
1516
}
1617

1718
impl TimelineStats {
1819
pub fn new(timeline: Timeline) -> Self {
1920
Self {
2021
timeline,
2122
per_time: Default::default(),
23+
total_count: 0,
2224
}
2325
}
26+
27+
pub fn num_events(&self) -> u64 {
28+
debug_assert_eq!(
29+
self.per_time.values().sum::<u64>(),
30+
self.total_count,
31+
"[DEBUG ASSERT] book keeping drifted"
32+
);
33+
self.total_count
34+
}
2435
}
2536

2637
/// A [`ChunkStoreSubscriber`] that keeps track of all unique timestamps on each [`Timeline`].
@@ -43,6 +54,11 @@ impl TimesPerTimeline {
4354
pub fn timelines(&self) -> impl ExactSizeIterator<Item = &Timeline> + '_ {
4455
self.0.values().map(|stats| &stats.timeline)
4556
}
57+
58+
#[inline]
59+
pub fn timelines_with_stats(&self) -> impl ExactSizeIterator<Item = &TimelineStats> + '_ {
60+
self.0.values()
61+
}
4662
}
4763

4864
// Always ensure we have a default "log_time" timeline.
@@ -85,6 +101,7 @@ impl ChunkStoreSubscriber for TimesPerTimeline {
85101

86102
for time in time_column.times() {
87103
let count = stats.per_time.entry(time).or_default();
104+
let total_count = &mut stats.total_count;
88105

89106
let delta = event.delta();
90107

@@ -95,21 +112,45 @@ impl ChunkStoreSubscriber for TimesPerTimeline {
95112
entity_path = %event.chunk.entity_path(),
96113
current = count,
97114
removed = delta.unsigned_abs(),
98-
"book keeping underflowed"
115+
"per `TimeInt` book keeping underflowed"
99116
);
100117
u64::MIN
101118
});
119+
*total_count = total_count
120+
.checked_sub(delta.unsigned_abs())
121+
.unwrap_or_else(|| {
122+
re_log::debug!(
123+
store_id = ?event.store_id,
124+
entity_path = %event.chunk.entity_path(),
125+
current = total_count,
126+
removed = delta.unsigned_abs(),
127+
"total book keeping underflowed"
128+
);
129+
u64::MIN
130+
});
102131
} else {
103132
*count = count.checked_add(delta.unsigned_abs()).unwrap_or_else(|| {
104133
re_log::debug!(
105134
store_id = ?event.store_id,
106135
entity_path = %event.chunk.entity_path(),
107136
current = count,
108137
removed = delta.unsigned_abs(),
109-
"book keeping overflowed"
138+
"per `TimeInt` book keeping overflowed"
110139
);
111140
u64::MAX
112141
});
142+
*total_count = total_count
143+
.checked_add(delta.unsigned_abs())
144+
.unwrap_or_else(|| {
145+
re_log::debug!(
146+
store_id = ?event.store_id,
147+
entity_path = %event.chunk.entity_path(),
148+
current = total_count,
149+
removed = delta.unsigned_abs(),
150+
"total book keeping underflowed"
151+
);
152+
u64::MAX
153+
});
113154
}
114155

115156
if *count == 0 {

crates/viewer/re_time_panel/src/time_control_ui.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,21 @@ impl TimeControlUi {
2828
egui::ComboBox::from_id_salt("timeline")
2929
.selected_text(time_control.timeline().name().as_str())
3030
.show_ui(ui, |ui| {
31-
for timeline in times_per_timeline.timelines() {
31+
for timeline_stats in times_per_timeline.timelines_with_stats() {
32+
let timeline = &timeline_stats.timeline;
3233
if ui
3334
.selectable_label(
3435
timeline == time_control.timeline(),
35-
timeline.name().as_str(),
36+
(
37+
timeline.name().as_str(),
38+
egui::Atom::grow(),
39+
egui::RichText::new(format!(
40+
"{} events",
41+
re_format::format_uint(timeline_stats.num_events())
42+
))
43+
.size(10.0)
44+
.color(ui.tokens().text_subdued),
45+
),
3646
)
3747
.clicked()
3848
{

crates/viewer/re_viewer_context/src/time_control.rs

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::BTreeMap;
22

33
use re_chunk::TimelineName;
4-
use re_entity_db::{TimeCounts, TimesPerTimeline};
4+
use re_entity_db::{TimeCounts, TimelineStats, TimesPerTimeline};
55
use re_log_types::{
66
AbsoluteTimeRange, AbsoluteTimeRangeF, Duration, TimeCell, TimeInt, TimeReal, TimeType,
77
Timeline,
@@ -564,7 +564,8 @@ impl TimeControl {
564564
if matches!(self.timeline, ActiveTimeline::Auto(_))
565565
|| !is_timeline_valid(self.timeline(), times_per_timeline)
566566
{
567-
self.timeline = ActiveTimeline::Auto(default_timeline(times_per_timeline.timelines()));
567+
self.timeline =
568+
ActiveTimeline::Auto(default_timeline(times_per_timeline.timelines_with_stats()));
568569
}
569570
}
570571

@@ -740,23 +741,26 @@ fn range(values: &TimeCounts) -> AbsoluteTimeRange {
740741
AbsoluteTimeRange::new(min(values), max(values))
741742
}
742743

743-
/// Pick the timeline that should be the default, prioritizing user-defined ones.
744-
fn default_timeline<'a>(timelines: impl IntoIterator<Item = &'a Timeline>) -> Timeline {
745-
let mut found_log_tick = false;
746-
let mut found_log_time = false;
744+
/// Pick the timeline that should be the default, by number of elements and prioritizing user-defined ones.
745+
fn default_timeline<'a>(timelines: impl IntoIterator<Item = &'a TimelineStats>) -> Timeline {
746+
re_tracing::profile_function!();
747747

748-
for timeline in timelines {
749-
if timeline == &Timeline::log_tick() {
750-
found_log_tick = true;
751-
} else if timeline == &Timeline::log_time() {
752-
found_log_time = true;
753-
} else {
754-
return *timeline;
748+
// Helper function that acts as a tie-breaker.
749+
fn timeline_priority(timeline: &Timeline) -> u8 {
750+
match timeline {
751+
t if *t == Timeline::log_tick() => 0, // lowest priority
752+
t if *t == Timeline::log_time() => 1, // medium priority
753+
_ => 2, // user-defined, highest priority
755754
}
756755
}
756+
let most_events = timelines.into_iter().max_by(|a, b| {
757+
a.num_events()
758+
.cmp(&b.num_events())
759+
.then_with(|| timeline_priority(&a.timeline).cmp(&timeline_priority(&b.timeline)))
760+
});
757761

758-
if found_log_tick && !found_log_time {
759-
Timeline::log_tick()
762+
if let Some(most_events) = most_events {
763+
most_events.timeline
760764
} else {
761765
Timeline::log_time()
762766
}
@@ -823,50 +827,62 @@ fn step_back_time_looped(
823827
mod tests {
824828
use super::*;
825829

830+
fn with_events(timeline: Timeline, num: u64) -> TimelineStats {
831+
TimelineStats {
832+
timeline,
833+
// Dummy `TimeInt` because were only interested in the counts.
834+
per_time: std::iter::once((TimeInt::ZERO, num)).collect(),
835+
total_count: num,
836+
}
837+
}
838+
826839
#[test]
827840
fn test_default_timeline() {
828-
let log_time = Timeline::log_time();
829-
let log_tick = Timeline::log_tick();
830-
let custom_timeline0 = Timeline::new("my_timeline0", TimeType::DurationNs);
831-
let custom_timeline1 = Timeline::new("my_timeline1", TimeType::DurationNs);
832-
833-
assert_eq!(default_timeline([]), log_time);
834-
assert_eq!(default_timeline([&log_tick]), log_tick);
835-
assert_eq!(default_timeline([&log_time]), log_time);
836-
assert_eq!(default_timeline([&log_time, &log_tick]), log_time);
841+
let log_time = with_events(Timeline::log_time(), 42);
842+
let log_tick = with_events(Timeline::log_tick(), 42);
843+
let custom_timeline0 = with_events(Timeline::new("my_timeline0", TimeType::DurationNs), 42);
844+
let custom_timeline1 = with_events(Timeline::new("my_timeline1", TimeType::DurationNs), 43);
845+
846+
assert_eq!(default_timeline([]), log_time.timeline);
847+
assert_eq!(default_timeline([&log_tick]), log_tick.timeline);
848+
assert_eq!(default_timeline([&log_time]), log_time.timeline);
849+
assert_eq!(default_timeline([&log_time, &log_tick]), log_time.timeline);
837850
assert_eq!(
838851
default_timeline([&log_time, &log_tick, &custom_timeline0]),
839-
custom_timeline0
852+
custom_timeline0.timeline
840853
);
841854
assert_eq!(
842855
default_timeline([&custom_timeline0, &log_time, &log_tick]),
843-
custom_timeline0
856+
custom_timeline0.timeline
844857
);
845858
assert_eq!(
846859
default_timeline([&log_time, &custom_timeline0, &log_tick]),
847-
custom_timeline0
860+
custom_timeline0.timeline
848861
);
849862
assert_eq!(
850863
default_timeline([&custom_timeline0, &log_time]),
851-
custom_timeline0
864+
custom_timeline0.timeline
852865
);
853866
assert_eq!(
854867
default_timeline([&custom_timeline0, &log_tick]),
855-
custom_timeline0
868+
custom_timeline0.timeline
856869
);
857870
assert_eq!(
858871
default_timeline([&log_time, &custom_timeline0]),
859-
custom_timeline0
872+
custom_timeline0.timeline
860873
);
861874
assert_eq!(
862875
default_timeline([&log_tick, &custom_timeline0]),
863-
custom_timeline0
876+
custom_timeline0.timeline
864877
);
865878

866879
assert_eq!(
867880
default_timeline([&custom_timeline0, &custom_timeline1]),
868-
custom_timeline0
881+
custom_timeline1.timeline
882+
);
883+
assert_eq!(
884+
default_timeline([&custom_timeline0]),
885+
custom_timeline0.timeline
869886
);
870-
assert_eq!(default_timeline([&custom_timeline0]), custom_timeline0);
871887
}
872888
}

0 commit comments

Comments
 (0)