From 0f103c99df587ce7472e09b5ef999954c1cf939d Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 29 Nov 2023 19:04:22 +0100 Subject: [PATCH] improve time panel time_ctrl overwrite behavior --- crates/re_time_panel/src/lib.rs | 20 +++++++++++++------- crates/re_viewer_context/src/time_control.rs | 6 +++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 77ec5263f7ae..d96b8a97b898 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -67,8 +67,9 @@ impl TimePanel { time_panel_expanded: bool, ) { // Naturally, many parts of the time panel need the time control. - // Copy it once, read/edit, and then write back at the end. - let mut time_ctrl = ctx.rec_cfg.time_ctrl.read().clone(); + // Copy it once, read/edit, and then write back at the end if there was a change. + let time_ctrl_before = ctx.rec_cfg.time_ctrl.read().clone(); + let mut time_ctrl_after = time_ctrl_before.clone(); // this is the size of everything above the central panel (window title bar, top bar on web, // etc.) @@ -115,7 +116,7 @@ impl TimePanel { ui.horizontal(|ui| { ui.spacing_mut().interact_size = Vec2::splat(top_bar_height); ui.visuals_mut().button_frame = true; - self.collapsed_ui(ctx, ui, &mut time_ctrl); + self.collapsed_ui(ctx, ui, &mut time_ctrl_after); }); } else { // Expanded: @@ -129,7 +130,7 @@ impl TimePanel { ui.horizontal(|ui| { ui.spacing_mut().interact_size = Vec2::splat(top_bar_height); ui.visuals_mut().button_frame = true; - self.top_row_ui(ctx, ui, &mut time_ctrl); + self.top_row_ui(ctx, ui, &mut time_ctrl_after); }); }) .response @@ -148,15 +149,20 @@ impl TimePanel { let mut streams_frame = egui::Frame::default(); streams_frame.inner_margin.left = margin.x; streams_frame.show(ui, |ui| { - self.expanded_ui(ctx, ui, &mut time_ctrl); + self.expanded_ui(ctx, ui, &mut time_ctrl_after); }); }); } }, ); - // Apply potential changes of the time control: - *ctx.rec_cfg.time_ctrl.write() = time_ctrl; + // Apply time control if there were any changes. + // This means that if anyone else meanwhile changed the time control, these changes are lost now. + // At least though we don't overwrite them if we didn't change anything at all. + // Since changes on the time control via the time panel are rare, this should be fine. + if time_ctrl_before != time_ctrl_after { + *ctx.rec_cfg.time_ctrl.write() = time_ctrl_after; + } } #[allow(clippy::unused_self)] diff --git a/crates/re_viewer_context/src/time_control.rs b/crates/re_viewer_context/src/time_control.rs index 302266dae9e7..1ffa239cd95b 100644 --- a/crates/re_viewer_context/src/time_control.rs +++ b/crates/re_viewer_context/src/time_control.rs @@ -6,7 +6,7 @@ use re_log_types::{Duration, TimeInt, TimeRange, TimeRangeF, TimeReal, TimeType, use crate::NeedsRepaint; /// The time range we are currently zoomed in on. -#[derive(Clone, Copy, Debug, serde::Deserialize, serde::Serialize)] +#[derive(Clone, Copy, Debug, serde::Deserialize, serde::Serialize, PartialEq)] pub struct TimeView { /// Where start of the range. pub min: TimeReal, @@ -20,7 +20,7 @@ pub struct TimeView { } /// State per timeline. -#[derive(Clone, Copy, Debug, serde::Deserialize, serde::Serialize)] +#[derive(Clone, Copy, Debug, serde::Deserialize, serde::Serialize, PartialEq)] struct TimeState { /// The current time (play marker). time: TimeReal, @@ -79,7 +79,7 @@ pub enum PlayState { } /// Controls the global view and progress of the time. -#[derive(serde::Deserialize, serde::Serialize, Clone)] +#[derive(serde::Deserialize, serde::Serialize, Clone, PartialEq)] #[serde(default)] pub struct TimeControl { /// Name of the timeline (e.g. "log_time").