Replace TimesPerTimeline with TimeHistogramPerTimeline for time-stepp…

[joelreymont]

• Closes Speed up ingestion: get rid of TimesPerTimeline #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

[emilk]

Thanks for the PR, but I'm pretty sure prev_key_before breaks for large histograms

[joelreymont]

Will fix!

[Wumpf]

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

[joelreymont]

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:

1. 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)

2. 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

3. 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)

4. 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)

5. 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)

6. 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)

7. 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)

8. 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)

[emilk]

Getting close!

[joelreymont]

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.

[emilk]

Something has gone wrong with some rebase/merge:

[joelreymont]

Crap, thanks for pointing it out!

Will fix and watch out for this going forward.

[joelreymont]

Sorry about that! Should be fixed now.

[emilk]

Sorry about that! Should be fixed now.

It's not.

Converted to draft until the diff is clean and CI green

[joelreymont]

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.

[joelreymont]

Will take care of CI tomorrow.

[joelreymont]

Comparing against main should show 11 files changed, 619 insertions(+), 456 deletions now.