Make time and timeline controlled by blueprint #11405

IsseW · 2025-10-02T10:11:19Z

What

Makes time cursor and current timeline be controlled by the blueprint.

Details

We log the time as static for blueprints for it not to affect undo state.

This includes some refactors that felt necessary to make tests still work and not be overly complicated:

TestContext doesn't require a mutable reference to run.
Can update TestContext blueprints via TestContext::with_blueprint_ctx.
The blueprint ctx is a struct that implements a helper trait for interacting with blueprints. App also has an equivalent AppBlueprintCtx, and ViewerContext also implements this trait.

The time panel was refactored a bit to allow the normal time panel and blueprint inspector time panel to still share code. This is mostly changing &mut TimeControl to &mut impl TimeControlCtx which handles writing to blueprints/just updating time control whether this is the blueprint inspector time panel or not.

TODO

Fix times per timeline with blueprint stored time, probably not store time per timeline in blueprint now and instead "switch out" what time to use.
Fix snapshot tests, the current produced results are most likely wrong.

github-actions · 2025-10-02T10:11:43Z

Web viewer built successfully.

Result	Commit	Link	Manifest
✅	`d84ada0`	https://rerun.io/viewer/pr/11405	`+nightly` `+main`

^{Note: This comment is updated whenever you push a commit.}

Copilot

Pull Request Overview

This PR implements time and timeline control via blueprints, allowing the time cursor and current timeline to be persisted in the blueprint store instead of just being ephemeral UI state.

Key changes:

Time cursor and timeline selection are now stored in blueprints using a new TimePanelBlueprint archetype
Time is logged as static data to avoid affecting undo state
Added blueprint context helpers for reading/writing time-related blueprint components

Reviewed Changes

Copilot reviewed 84 out of 84 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`rerun_py/rerun_sdk/rerun/recording_stream.py`	Move import to TYPE_CHECKING block to avoid runtime import
`rerun_py/rerun_sdk/rerun/blueprint/components/time_cell.py`	New auto-generated component for storing time references in blueprints
`rerun_py/rerun_sdk/rerun/blueprint/archetypes/time_panel_blueprint.py`	New auto-generated archetype for time panel blueprint state including timeline and time cursor
`rerun_py/rerun_sdk/rerun/blueprint/api.py`	Enhanced TimePanel class with timeline and time cursor parameters
`crates/viewer/re_viewer_context/src/time_control.rs`	Major refactor to integrate blueprint-based time control with helper traits and context
`crates/viewer/re_viewer/src/app.rs`	Updated to use blueprint context for time control operations
`crates/store/re_types/definitions/rerun/blueprint/archetypes/panel_blueprint.fbs`	Added TimePanelBlueprint definition and fixed documentation

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

rerun_py/rerun_sdk/rerun/recording_stream.py

crates/viewer/re_viewer_context/src/time_control.rs

Copilot · 2025-10-03T09:21:06Z

crates/viewer/re_viewer_context/src/time_control.rs

+use crate::{NeedsRepaint, blueprint_helpers::BlueprintContext};
+
+pub const TIME_PANEL_PATH: &str = "time_panel";
+


[nitpick] This constant is duplicated in crates/viewer/re_viewer/src/app_blueprint.rs. Consider moving this to a shared location to avoid duplication and potential inconsistencies.

Suggested change

use crate::{NeedsRepaint, blueprint_helpers::BlueprintContext};

pub const TIME_PANEL_PATH: &str = "time_panel";

use crate::{NeedsRepaint, blueprint_helpers::{BlueprintContext, TIME_PANEL_PATH}};

It's not duplicated. And is moved to a lower crate, possibly there could be some better place to have all of these constants though?

crates/viewer/re_time_panel/src/time_panel.rs

emilk

Wow, this didn't look easy 😆 Good job on wading through all this mess.

I'm worried we're making already complicated code even more complicated.
I never liked how TimeControl was behind an RwLock. Ideally, all our UI code reads immutable state and then emits commands, and TimeControl is and remains the main holdout in all this (not coincidentally, it is also among the oldest code in the codebase).

Is there a design we could have that is more data-centric and less reliant on traits?

In my ideal world we'd set up the current state of TimeControl at the start of the frame (based on the blueprint for as much as we can), then in the UI functions we return some delta of what happens as idempotent commands ("change timeline to log_time, turn off looping, …"). It's then up to the caller to pipe those commands to the correct sink (e.g. depending on if this is the real data time panel, or the dev blueprint time panel).
Some commands will end up as writes in the blueprint store, others not (we haven't blueprintified everything yet).

I think such a design would also make testing simpler (we could do a unit-snapshot-test of the time control buttons, without having any larger test context), FWIW.

WDYT? After this PR I'm pretty certain you have the best understanding of this mess :)

emilk · 2025-10-06T10:58:27Z

crates/store/re_types/definitions/rerun/blueprint/archetypes/panel_blueprint.fbs

+  timeline: rerun.blueprint.components.TimelineName ("attr.rerun.component_optional", nullable, order: 2000);
+
+  /// What time the time cursor should be on.
+  time: rerun.blueprint.components.TimeCell ("attr.rerun.component_optional", nullable, order: 2100);


Note for self: I wonder if we should somehow mark its "staticness" here? 🤔

emilk · 2025-10-06T11:00:07Z

crates/store/re_types/definitions/rerun/blueprint/components/time_cell.fbs

+  "attr.rust.repr": "transparent",
+  "attr.rust.tuple_struct"
+) {
+  time: rerun.datatypes.TimeInt (order: 100);


This will be a bit confusing. We already hav e a struct TimeCell that has the TimeType in it as well.

I feel like this should either have a TimeType, or simply be called TimeInt (it's quite often we have a datatype and a component of the same name)

emilk · 2025-10-06T11:02:15Z

crates/viewer/re_test_context/src/lib.rs

+                blueprint_query: &blueprint_query,
+            };
+
+            // ctx.set_timeline("log_tick".into());


emilk · 2025-10-06T11:06:27Z

crates/viewer/re_time_panel/src/time_control_ui.rs

+        time_control: &mut impl TimeControlExt,
        ui: &mut egui::Ui,
    ) {
        ui.horizontal(|ui| {
            ui.spacing_mut().item_spacing.x = 5.0; // from figma
-            self.play_button_ui(ctx, time_control, ui);
-            self.follow_button_ui(ctx, time_control, ui);
-            self.pause_button_ui(ctx, time_control, ui);
+            self.play_button_ui(ctx, time_control.get(), ui);
+            self.follow_button_ui(ctx, time_control.get(), ui);
+            self.pause_button_ui(ctx, time_control.get(), ui);
            self.step_time_button_ui(ctx, ui);
-            self.loop_button_ui(time_control, ui);
+            self.loop_button_ui(time_control.get_mut(), ui);


This smells. Why not just pass in &mut TimeControl directly?

emilk · 2025-10-06T11:07:29Z

crates/viewer/re_time_panel/src/time_control_ui.rs

        &self,
-        time_control: &mut TimeControl,
+        ctx: &ViewerContext<'_>,
+        time_control: &mut impl TimeControlExt,


This will add unnecessary monomorphization, increasing binary size and compilation times.

Suggested change

time_control: &mut impl TimeControlExt,

time_control: &mut dyn TimeControlExt,

here and elsewhere

emilk · 2025-10-06T11:16:27Z

crates/viewer/re_viewer/src/app.rs

    file_handle.write(&bytes).await.context("Failed to save")
 }
+
+pub struct AppBlueprintCtx<'a> {


please move this to its own file. app.rs is already way too big 🙈

emilk · 2025-10-06T11:24:35Z

crates/viewer/re_viewer_context/src/time_control.rs

+    /// Replaces the current timeline with the automatic one.
+    fn clear_timeline(&self);
+
+    fn clear_time(&self);


What does this do?

emilk · 2025-10-06T11:25:36Z

crates/viewer/re_viewer_context/src/time_control.rs

+                &TimePanelBlueprint::descriptor_time(),
+            )?;
+
+        Some(TimeInt::saturated_temporal_i64(time.0.0))


Suggested change

Some(TimeInt::saturated_temporal_i64(time.0.0))

Some(TimeInt::ZERO)

emilk · 2025-10-06T11:26:29Z

crates/viewer/re_viewer_context/src/time_control.rs

    ///
    /// If `should_diff_state` is true, then the response also contains any changes in state
    /// between last frame and the current one.
-    #[must_use]


Intentionally removed? Maybe it should be moved onto TimeControlResponse?

Most likely got lost in copying it, yeah having it on TimeControlResponse makes more sense!

emilk · 2025-10-06T11:27:14Z

crates/viewer/re_viewer_context/src/time_control.rs

+        if let Some(time) = blueprint_ctx.get_time() {
+            if self.time_int() != Some(time) {
+                self.set_time(time.into());
+            }
+        }


Suggested change

if let Some(time) = blueprint_ctx.get_time() {

if self.time_int() != Some(time) {

self.set_time(time.into());

}

}

if let Some(time) = blueprint_ctx.get_time() && self.time_int() != Some(time) {

self.set_time(time.into());

}

emilk

Wow, this didn't look easy 😆 Good job on wading through all this mess.

I'm worried we're making already complicated code even more complicated.
I never liked how TimeControl was behind an RwLock. Ideally, all our UI code reads immutable state and then emits commands, and TimeControl is and remains the main holdout in all this (not coincidentally, it is also among the oldest code in the codebase).

Is there a design we could have that is more data-centric and less reliant on traits?

In my ideal world we'd set up the current state of TimeControl at the start of the frame (based on the blueprint for as much as we can), then in the UI functions we return some delta of what happens as idempotent commands ("change timeline to log_time, turn off looping, …"). It's then up to the caller to pipe those commands to the correct sink (e.g. depending on if this is the real data time panel, or the dev blueprint time panel).
Some commands will end up as writes in the blueprint store, others not (we haven't blueprintified everything yet).

I think such a design would also make testing simpler (we could do a unit-snapshot-test of the time control buttons, without having any larger test context), FWIW.

WDYT? After this PR I'm pretty certain you have the best understanding of this mess :)

… on test context

IsseW added sdk-python Python logging API 📺 re_viewer affects re_viewer itself 🟦 blueprint The data that defines our UI include in changelog labels Oct 2, 2025

IsseW added 8 commits October 2, 2025 17:14

Make time and timeline controlled by blueprint

f96b8a2

test and fix setting timeline/time from python

9f1e37f

do remaining todos

57b35ba

lints

22792b7

py-fmt

6d5f2e7

fix some tests

0714715

fix more functions

7bbbea2

rebase fix

c8610b0

IsseW force-pushed the isse/blueprint-time-panel branch from c446bca to c8610b0 Compare October 2, 2025 15:44

IsseW added 6 commits October 2, 2025 18:03

fix web build

c2f541d

Don't override saved times per timeline

cc16f97

lints

1b10e83

make logging static components not clear redo buffer

3e7ea89

rust lints

859a286

fixing tests

7647309

IsseW marked this pull request as ready for review October 3, 2025 09:20

IsseW requested a review from Copilot October 3, 2025 09:20

Copilot AI reviewed Oct 3, 2025

View reviewed changes

IsseW added 3 commits October 3, 2025 11:26

dedupe floor

e6ca9f6

clean up

b249c43

fix snapshot tests

bd286d2

IsseW force-pushed the isse/blueprint-time-panel branch from 1c132d7 to bd286d2 Compare October 3, 2025 13:46

IsseW added 2 commits October 3, 2025 16:27

revert recording_stream.py

c184f5d

Check if we have the time field

ba81443

IsseW force-pushed the isse/blueprint-time-panel branch from 463d941 to ba81443 Compare October 3, 2025 15:04

emilk self-requested a review October 6, 2025 10:54

emilk reviewed Oct 6, 2025

View reviewed changes

IsseW added 2 commits October 6, 2025 14:41

move must_use to TimeControlResponse

41e1636

Update time control from blueprint after calling with_blueprint_ctx…

d84ada0

… on test context

		use crate::{NeedsRepaint, blueprint_helpers::BlueprintContext};

		pub const TIME_PANEL_PATH: &str = "time_panel";

	time_control: &mut impl TimeControlExt,
	time_control: &mut dyn TimeControlExt,

	Some(TimeInt::saturated_temporal_i64(time.0.0))
	Some(TimeInt::ZERO)

Make time and timeline controlled by blueprint #11405

Are you sure you want to change the base?

Make time and timeline controlled by blueprint #11405

Conversation

IsseW commented Oct 2, 2025 • edited by emilk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related

What

Details

TODO

Uh oh!

github-actions bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emilk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

IsseW commented Oct 2, 2025 •

edited by emilk

Loading

github-actions bot commented Oct 2, 2025 •

edited

Loading