rerun-io
Replace TimesPerTimeline with TimeHistogramPerTimeline for time-stepp…
- Closes #7084.
Changes:
- Added next_key_after() and prev_key_before() methods to Int64Histogram to support time-stepping functionality
- Updated TimeControl and related UI components to use TimeHistogramPerTimeline
- Updated EntityDb to remove times_per_timeline
- Added tests for new histogram methods and time-stepping
why did any screenshots change at all if this is supposed to an internal refactor? A lot of them seem to be missing the time display which is obviously a bug
I had AI analyze the snapshot differences. Here's the summary, after commit 7041436...
All 18 snapshot changes relate to the time control panel fix. The fix ensures play/pause/follow buttons show the correct selected state even when no time histogram data is available.
Visual changes:
- Button highlighting: buttons now correctly show selected (brighter/highlighted) vs unselected (darker) states
- Panel area updates: the entire time control panel area reflects the correct play state
- Consistency: all snapshots now show consistent button states matching the actual playback state
Root cause:
Previously, when time_histogram_per_timeline was None, the play state flags weren't updated, causing incorrect button states. The fix ensures set_play_state is called even without histogram data, using an empty histogram as a fallback.
More specifically:
add_container_from_blueprint_panel_menu_3.png
- Change: Small button state change in time control panel
- Location: Bottom panel area (y=934)
- Size: 7×6px region
- Visual: Button appears brighter/highlighted (selected state)
- Brightness increase: +25.3
- Overall impact: 852 pixels changed (0.08% of image)
change_container_type_2.png
- Change: Large region change
- Location: (30, 30) to (908, 398) — 879×369px region
- Visual: Significant changes across a large area (13.51% of image)
- Impact: 141,705 pixels changed, max difference 255/255
drag_view_to_other_view_bottom_1.png
- Change: Time control panel area change
- Location: Bottom panel (y=724, spans most of width)
- Size: 1010×299px region
- Visual: Changes in the entire bottom panel area
- Overall impact: 451,106 pixels changed (43.02% of image)
drag_view_to_other_view_left_1.png
- Change: Same as bottom_1 — time control panel area
- Location: Bottom panel (y=724)
- Size: 1010×299px region
- Visual: Changes in the entire bottom panel area
- Overall impact: 452,210 pixels changed (43.13% of image)
drag_view_to_other_view_right_1.png
- Change: Same pattern — time control panel area
- Location: Bottom panel (y=724)
- Size: 1010×299px region
- Visual: Changes in the entire bottom panel area
- Overall impact: 451,009 pixels changed (43.01% of image)
resize_view_horizontal_1.png
- Change: Time control panel button area
- Location: Bottom panel (x=212, y=724)
- Size: 606×50px region (button row width)
- Visual: Changes in the time control button area
- Overall impact: 433,710 pixels changed (41.36% of image)
resize_view_horizontal_2.png
- Change: Same as resize_1 — time control panel button area
- Location: Bottom panel (x=212, y=724)
- Size: 606×50px region
- Visual: Changes in the time control button area
- Overall impact: 454,404 pixels changed (43.34% of image)
resize_view_horizontal_3.png
- Change: Same as resize_1 and resize_2
- Location: Bottom panel (x=212, y=724)
- Size: 606×50px region
- Visual: Changes in the time control button area
- Overall impact: 454,014 pixels changed (43.30% of image)
It's like to justify updating snapshots with this analysis of test_single_text_document. The same reasoning applies to the other failing snapshots.
Internal changes
Old code (TimesPerTimeline):
impl Default for TimesPerTimeline {
fn default() -> Self {
let timeline = Timeline::log_time();
Self(BTreeMap::from([(
*timeline.name(),
TimelineStats::new(timeline), // ← ALWAYS has log_time!
)]))
}
}
→ Timeline selector always shows log_time even with no data
New code (TimeHistogramPerTimeline):
#[derive(Default, Clone)] // ← derives Default
pub struct TimeHistogramPerTimeline {
times: BTreeMap<TimelineName, TimeHistogram>, // ← EMPTY BTreeMap
num_static_messages: u64,
}
→ Timeline selector shows no timelines with no data
UI rendering changes:
Old timeline selector loop:
for timeline_stats in times_per_timeline.timelines_with_stats() {
// Always iterates at least once (log_time)
}
New timeline selector loop:
for (timeline_name, hist) in time_histogram_per_timeline.iter() {
if let Some(timeline) = timelines.get(timeline_name) {
// With empty data: NEVER enters this loop
}
}
Conclusion:
test_single_text_document has no timelines at the first snapshot. The old code artificially showed a log_time timeline by default; the new code correctly shows nothing. The snapshot visual difference is the timeline selector dropdown rendering empty instead of showing log_time.
So the snapshot needs to be updated. This is not a bug, it's the correct behavior for an empty recording.
Crap, thanks for pointing it out!
Will fix and watch out for this going forward.
Sorry about that! Should be fixed now.
It's not.
Converted to draft until the diff is clean and CI green
Something has gone wrong with some rebase/merge:
Where are you looking?
I checked c765a7a and the Compare button and they show me a small subset of things changed.
Comparing against main should show 11 files changed, 619 insertions(+), 456 deletions now.