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

Bug fix: always show latest data in follow-mode #7425

Merged
merged 1 commit into from
Sep 16, 2024
Merged
Changes from all commits
Commits
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
72 changes: 37 additions & 35 deletions crates/viewer/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ impl AppState {
focused_item,
};

// We move the time at the very start of the frame,
// so that we always show the latest data when we're in "follow" mode.
move_time(&ctx, recording, rx);

// Update the viewport. May spawn new views and handle queued requests (like screenshots).
viewport.on_frame_start(&ctx);

Expand Down Expand Up @@ -466,41 +470,6 @@ impl AppState {
// Process deferred layout operations and apply updates back to blueprint
viewport.update_and_sync_tile_tree_to_blueprint(&ctx);

{
// We move the time at the very end of the frame,
// so we have one frame to see the first data before we move the time.
Comment on lines -470 to -471
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 don't quite understand this, even though I suspect I wrote it.

The "first" data will only be visible if we move/update the current time so that the latest-at query includes the first data.

Sure, if we receive a few video frames during one app update, the new code will not show the first frame of the video, but I don't think that's wrong, nor do I think the old code would have shown the first frame of the video either.

let dt = ui.ctx().input(|i| i.stable_dt);

// Are we still connected to the data source for the current store?
let more_data_is_coming = if let Some(store_source) = &recording.data_source {
rx.sources().iter().any(|s| s.as_ref() == store_source)
} else {
false
};

let recording_needs_repaint = ctx.rec_cfg.time_ctrl.write().update(
recording.times_per_timeline(),
dt,
more_data_is_coming,
);

let blueprint_needs_repaint = if ctx.app_options.inspect_blueprint_timeline {
ctx.blueprint_cfg.time_ctrl.write().update(
ctx.store_context.blueprint.times_per_timeline(),
dt,
more_data_is_coming,
)
} else {
re_viewer_context::NeedsRepaint::No
};

if recording_needs_repaint == re_viewer_context::NeedsRepaint::Yes
|| blueprint_needs_repaint == re_viewer_context::NeedsRepaint::Yes
{
ui.ctx().request_repaint();
}
}

if WATERMARK {
ui.ctx().paint_watermark();
}
Expand Down Expand Up @@ -537,6 +506,39 @@ impl AppState {
}
}

fn move_time(ctx: &ViewerContext<'_>, recording: &EntityDb, rx: &ReceiveSet<LogMsg>) {
let dt = ctx.egui_ctx.input(|i| i.stable_dt);

// Are we still connected to the data source for the current store?
let more_data_is_coming = if let Some(store_source) = &recording.data_source {
rx.sources().iter().any(|s| s.as_ref() == store_source)
} else {
false
};

let recording_needs_repaint = ctx.rec_cfg.time_ctrl.write().update(
recording.times_per_timeline(),
dt,
more_data_is_coming,
);

let blueprint_needs_repaint = if ctx.app_options.inspect_blueprint_timeline {
ctx.blueprint_cfg.time_ctrl.write().update(
ctx.store_context.blueprint.times_per_timeline(),
dt,
more_data_is_coming,
)
} else {
re_viewer_context::NeedsRepaint::No
};

if recording_needs_repaint == re_viewer_context::NeedsRepaint::Yes
|| blueprint_needs_repaint == re_viewer_context::NeedsRepaint::Yes
{
ctx.egui_ctx.request_repaint();
}
}

fn recording_config_entry<'cfgs>(
configs: &'cfgs mut HashMap<StoreId, RecordingConfig>,
id: StoreId,
Expand Down
Loading