From 3f116396b77b04504f58d8166658d69332379249 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 30 Nov 2023 11:08:58 +0100 Subject: [PATCH] Time control is now behind a RwLock, making recording config access non-mutable everywhere (#4389) ### What * Another step towards #1325 * Follow-up to #4387 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [app.rerun.io](https://app.rerun.io/pr/4389) (if applicable) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/4389) - [Docs preview](https://rerun.io/preview/405ddca2f8389bb62e1d1e65ef59a3d211361968/docs) - [Examples preview](https://rerun.io/preview/405ddca2f8389bb62e1d1e65ef59a3d211361968/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- Cargo.lock | 1 + crates/re_data_ui/src/item_ui.rs | 16 +- .../src/contexts/depth_offsets.rs | 2 +- .../src/contexts/transform_context.rs | 3 +- .../src/space_view_class.rs | 7 +- .../src/space_view_class.rs | 23 ++- .../re_time_panel/src/data_density_graph.rs | 12 +- crates/re_time_panel/src/lib.rs | 168 ++++++++++++------ crates/re_time_panel/src/time_ranges_ui.rs | 12 +- crates/re_viewer/src/app.rs | 2 +- crates/re_viewer/src/app_state.rs | 11 +- crates/re_viewer/src/ui/selection_panel.rs | 2 +- crates/re_viewer/src/ui/visible_history.rs | 25 ++- crates/re_viewer_context/Cargo.toml | 4 +- crates/re_viewer_context/src/time_control.rs | 13 +- .../re_viewer_context/src/viewer_context.rs | 33 ++-- crates/re_viewport/src/space_view.rs | 2 +- crates/re_viewport/src/viewport.rs | 2 +- 18 files changed, 202 insertions(+), 136 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 458fa86c31d0..2cd55ccdb56c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2781,6 +2781,7 @@ checksum = "435011366fe56583b16cf956f9df0095b405b82d76425bc8981c0e22e60ec4df" dependencies = [ "autocfg", "scopeguard", + "serde", ] [[package]] diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index f95062d9c590..884834b982b9 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -238,7 +238,11 @@ pub fn time_button( timeline: &Timeline, value: TimeInt, ) -> egui::Response { - let is_selected = ctx.rec_cfg.time_ctrl.is_time_selected(timeline, value); + let is_selected = ctx + .rec_cfg + .time_ctrl + .read() + .is_time_selected(timeline, value); let response = ui.selectable_label( is_selected, @@ -249,8 +253,9 @@ pub fn time_button( if response.clicked() { ctx.rec_cfg .time_ctrl + .write() .set_timeline_and_time(*timeline, value); - ctx.rec_cfg.time_ctrl.pause(); + ctx.rec_cfg.time_ctrl.write().pause(); } response } @@ -269,14 +274,15 @@ pub fn timeline_button_to( text: impl Into, timeline: &Timeline, ) -> egui::Response { - let is_selected = ctx.rec_cfg.time_ctrl.timeline() == timeline; + let is_selected = ctx.rec_cfg.time_ctrl.read().timeline() == timeline; let response = ui .selectable_label(is_selected, text) .on_hover_text("Click to switch to this timeline"); if response.clicked() { - ctx.rec_cfg.time_ctrl.set_timeline(*timeline); - ctx.rec_cfg.time_ctrl.pause(); + let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); + time_ctrl.set_timeline(*timeline); + time_ctrl.pause(); } response } diff --git a/crates/re_space_view_spatial/src/contexts/depth_offsets.rs b/crates/re_space_view_spatial/src/contexts/depth_offsets.rs index d8ec3aefc708..00db1cf4e4be 100644 --- a/crates/re_space_view_spatial/src/contexts/depth_offsets.rs +++ b/crates/re_space_view_spatial/src/contexts/depth_offsets.rs @@ -51,7 +51,7 @@ impl ViewContextSystem for EntityDepthOffsets { for data_result in query.iter_visible_data_results(Self::name()) { if let Some(draw_order) = store.query_latest_component::( &data_result.entity_path, - &ctx.rec_cfg.time_ctrl.current_query(), + &ctx.rec_cfg.time_ctrl.read().current_query(), ) { entities_per_draw_order .entry(draw_order.value) diff --git a/crates/re_space_view_spatial/src/contexts/transform_context.rs b/crates/re_space_view_spatial/src/contexts/transform_context.rs index 97ae3bded4a5..b052fe208d42 100644 --- a/crates/re_space_view_spatial/src/contexts/transform_context.rs +++ b/crates/re_space_view_spatial/src/contexts/transform_context.rs @@ -87,7 +87,6 @@ impl ViewContextSystem for TransformContext { re_tracing::profile_function!(); let entity_db = ctx.store_db.entity_db(); - let time_ctrl = &ctx.rec_cfg.time_ctrl; // TODO(jleibs): The need to do this hints at a problem with how we think about // the interaction between properties and "context-systems". @@ -114,7 +113,7 @@ impl ViewContextSystem for TransformContext { return; }; - let time_query = time_ctrl.current_query(); + let time_query = ctx.rec_cfg.time_ctrl.read().current_query(); // Child transforms of this space self.gather_descendants_transforms( diff --git a/crates/re_space_view_text_log/src/space_view_class.rs b/crates/re_space_view_text_log/src/space_view_class.rs index fd47580dbe1f..fedeaf07889b 100644 --- a/crates/re_space_view_text_log/src/space_view_class.rs +++ b/crates/re_space_view_text_log/src/space_view_class.rs @@ -167,6 +167,7 @@ impl SpaceViewClass for TextSpaceView { let time = ctx .rec_cfg .time_ctrl + .read() .time_i64() .unwrap_or(state.latest_time); @@ -277,8 +278,10 @@ fn table_ui( use egui_extras::Column; - let global_timeline = *ctx.rec_cfg.time_ctrl.timeline(); - let global_time = ctx.rec_cfg.time_ctrl.time_int(); + let (global_timeline, global_time) = { + let time_ctrl = ctx.rec_cfg.time_ctrl.read(); + (*time_ctrl.timeline(), time_ctrl.time_int()) + }; let mut table_builder = egui_extras::TableBuilder::new(ui) .resizable(true) diff --git a/crates/re_space_view_time_series/src/space_view_class.rs b/crates/re_space_view_time_series/src/space_view_class.rs index 8e4bc882c697..9942bc0f1977 100644 --- a/crates/re_space_view_time_series/src/space_view_class.rs +++ b/crates/re_space_view_time_series/src/space_view_class.rs @@ -160,10 +160,14 @@ impl SpaceViewClass for TimeSeriesSpaceView { ) -> Result<(), SpaceViewSystemExecutionError> { re_tracing::profile_function!(); - let time_ctrl = &ctx.rec_cfg.time_ctrl; - let current_time = time_ctrl.time_i64(); - let time_type = time_ctrl.time_type(); - let timeline = time_ctrl.timeline(); + let (current_time, time_type, timeline) = { + // Avoid holding the lock for long + let time_ctrl = ctx.rec_cfg.time_ctrl.read(); + let current_time = time_ctrl.time_i64(); + let time_type = time_ctrl.time_type(); + let timeline = *time_ctrl.timeline(); + (current_time, time_type, timeline) + }; let timeline_name = timeline.name().to_string(); @@ -236,12 +240,13 @@ impl SpaceViewClass for TimeSeriesSpaceView { transform, } = plot.show(ui, |plot_ui| { if plot_ui.response().secondary_clicked() { - let timeline = ctx.rec_cfg.time_ctrl.timeline(); - ctx.rec_cfg.time_ctrl.set_timeline_and_time( - *timeline, + let mut time_ctrl_write = ctx.rec_cfg.time_ctrl.write(); + let timeline = *time_ctrl_write.timeline(); + time_ctrl_write.set_timeline_and_time( + timeline, plot_ui.pointer_coordinate().unwrap().x as i64 + time_offset, ); - ctx.rec_cfg.time_ctrl.pause(); + time_ctrl_write.pause(); } for line in &time_series.lines { @@ -301,7 +306,7 @@ impl SpaceViewClass for TimeSeriesSpaceView { let time = time_offset + transform.value_from_position(pointer_pos).x.round() as i64; - let time_ctrl = &mut ctx.rec_cfg.time_ctrl; + let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); time_ctrl.set_time(time); time_ctrl.pause(); diff --git a/crates/re_time_panel/src/data_density_graph.rs b/crates/re_time_panel/src/data_density_graph.rs index e6b0b540b2c6..b24b0f379e47 100644 --- a/crates/re_time_panel/src/data_density_graph.rs +++ b/crates/re_time_panel/src/data_density_graph.rs @@ -11,7 +11,7 @@ use egui::{epaint::Vertex, lerp, pos2, remap, Color32, NumExt as _, Rect, Shape} use re_data_store::TimeHistogram; use re_data_ui::{item_ui, DataUi}; use re_log_types::{TimeInt, TimeRange, TimeReal}; -use re_viewer_context::{Item, UiVerbosity, ViewerContext}; +use re_viewer_context::{Item, TimeControl, UiVerbosity, ViewerContext}; use super::time_ranges_ui::TimeRangesUi; @@ -368,6 +368,7 @@ fn smooth(density: &[f32]) -> Vec { pub fn data_density_graph_ui( data_dentity_graph_painter: &mut DataDensityGraphPainter, ctx: &mut ViewerContext<'_>, + time_ctrl: &mut TimeControl, time_area_response: &egui::Response, time_area_painter: &egui::Painter, ui: &egui::Ui, @@ -486,11 +487,12 @@ pub fn data_density_graph_ui( if time_area_response.clicked_by(egui::PointerButton::Primary) { ctx.set_single_selection(item); - ctx.rec_cfg.time_ctrl.set_time(hovered_time_range.min); - ctx.rec_cfg.time_ctrl.pause(); + time_ctrl.set_time(hovered_time_range.min); + time_ctrl.pause(); } else if !ui.ctx().memory(|mem| mem.is_anything_being_dragged()) { show_row_ids_tooltip( ctx, + time_ctrl, ui.ctx(), item, hovered_time_range, @@ -521,6 +523,7 @@ fn make_brighter(color: Color32) -> Color32 { fn show_row_ids_tooltip( ctx: &mut ViewerContext<'_>, + time_ctrl: &mut TimeControl, egui_ctx: &egui::Context, item: &Item, time_range: TimeRange, @@ -552,8 +555,7 @@ fn show_row_ids_tooltip( ui.add_space(8.0); - let timeline = *ctx.rec_cfg.time_ctrl.timeline(); - let query = re_arrow_store::LatestAtQuery::new(timeline, time_range.max); + let query = re_arrow_store::LatestAtQuery::new(*time_ctrl.timeline(), time_range.max); item.data_ui(ctx, ui, UiVerbosity::Reduced, &query); }); } diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 89188405918a..d96b8a97b898 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -66,6 +66,11 @@ impl TimePanel { ui: &mut egui::Ui, 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 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.) let screen_header_height = ui.cursor().top(); @@ -111,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); + self.collapsed_ui(ctx, ui, &mut time_ctrl_after); }); } else { // Expanded: @@ -125,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); + self.top_row_ui(ctx, ui, &mut time_ctrl_after); }); }) .response @@ -144,16 +149,29 @@ 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); + self.expanded_ui(ctx, ui, &mut time_ctrl_after); }); }); } }, ); + + // 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)] - fn collapsed_ui(&mut self, ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { + fn collapsed_ui( + &mut self, + ctx: &mut ViewerContext<'_>, + ui: &mut egui::Ui, + time_ctrl: &mut TimeControl, + ) { ui.spacing_mut().item_spacing.x = 18.0; // from figma if ui.max_rect().width() < 600.0 { @@ -161,7 +179,6 @@ impl TimePanel { ui.vertical(|ui| { ui.horizontal(|ui| { let re_ui = &ctx.re_ui; - let time_ctrl = &mut ctx.rec_cfg.time_ctrl; let times_per_timeline = ctx.store_db.times_per_timeline(); self.time_control_ui .play_pause_ui(time_ctrl, re_ui, times_per_timeline, ui); @@ -169,19 +186,17 @@ impl TimePanel { self.time_control_ui.fps_ui(time_ctrl, ui); }); ui.horizontal(|ui| { - let time_ctrl = &mut ctx.rec_cfg.time_ctrl; self.time_control_ui.timeline_selector_ui( time_ctrl, ctx.store_db.times_per_timeline(), ui, ); - collapsed_time_marker_and_time(ui, ctx); + collapsed_time_marker_and_time(ui, ctx, time_ctrl); }); }); } else { // One row: let re_ui = &ctx.re_ui; - let time_ctrl = &mut ctx.rec_cfg.time_ctrl; let times_per_timeline = ctx.store_db.times_per_timeline(); self.time_control_ui .play_pause_ui(time_ctrl, re_ui, times_per_timeline, ui); @@ -190,11 +205,16 @@ impl TimePanel { self.time_control_ui.playback_speed_ui(time_ctrl, ui); self.time_control_ui.fps_ui(time_ctrl, ui); - collapsed_time_marker_and_time(ui, ctx); + collapsed_time_marker_and_time(ui, ctx, time_ctrl); } } - fn expanded_ui(&mut self, ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { + fn expanded_ui( + &mut self, + ctx: &mut ViewerContext<'_>, + ui: &mut egui::Ui, + time_ctrl: &mut TimeControl, + ) { re_tracing::profile_function!(); self.data_density_graph_painter.begin_frame(ui.ctx()); @@ -226,11 +246,12 @@ impl TimePanel { let side_margin = 26.0; // chosen so that the scroll bar looks approximately centered in the default gap self.time_ranges_ui = initialize_time_ranges_ui( ctx, + time_ctrl, Rangef::new( time_fg_x_range.min + side_margin, time_fg_x_range.max - side_margin, ), - ctx.rec_cfg.time_ctrl.time_view(), + time_ctrl.time_view(), ); let full_y_range = Rangef::new(ui.min_rect().bottom(), ui.max_rect().bottom()); @@ -266,7 +287,14 @@ impl TimePanel { let time_bg_area_painter = ui.painter().with_clip_rect(time_bg_area_rect); let time_area_painter = ui.painter().with_clip_rect(time_fg_area_rect); - paint_visible_history_range(&self.time_ranges_ui, ctx, ui.painter(), time_fg_area_rect); + if let Some(highlighted_range) = time_ctrl.highlighted_range { + paint_range_highlight( + highlighted_range, + &self.time_ranges_ui, + ui.painter(), + time_fg_area_rect, + ); + } ui.painter().hline( 0.0..=ui.max_rect().right(), @@ -279,7 +307,7 @@ impl TimePanel { ui, &time_area_painter, timeline_rect.top()..=timeline_rect.bottom(), - ctx.rec_cfg.time_ctrl.time_type(), + time_ctrl.time_type(), ctx.app_options.time_zone_for_timestamps, ); paint_time_ranges_gaps( @@ -291,7 +319,7 @@ impl TimePanel { ); time_selection_ui::loop_selection_ui( ctx.store_db, - &mut ctx.rec_cfg.time_ctrl, + time_ctrl, &self.time_ranges_ui, ui, &time_bg_area_painter, @@ -299,7 +327,7 @@ impl TimePanel { ); let time_area_response = interact_with_streams_rect( &self.time_ranges_ui, - &mut ctx.rec_cfg.time_ctrl, + time_ctrl, ui, &time_bg_area_rect, &streams_rect, @@ -314,6 +342,7 @@ impl TimePanel { // All the entity rows and their data density graphs: self.tree_ui( ctx, + time_ctrl, &time_area_response, &lower_time_area_painter, time_x_left, @@ -345,7 +374,7 @@ impl TimePanel { // Put time-marker on top and last, so that you can always drag it time_marker_ui( &self.time_ranges_ui, - &mut ctx.rec_cfg.time_ctrl, + time_ctrl, ctx.re_ui, ui, &time_area_painter, @@ -362,6 +391,7 @@ impl TimePanel { fn tree_ui( &mut self, ctx: &mut ViewerContext<'_>, + time_ctrl: &mut TimeControl, time_area_response: &egui::Response, time_area_painter: &egui::Painter, tree_max_y: f32, @@ -388,6 +418,7 @@ impl TimePanel { if show_root { self.show_tree( ctx, + time_ctrl, time_area_response, time_area_painter, tree_max_y, @@ -399,6 +430,7 @@ impl TimePanel { } else { self.show_children( ctx, + time_ctrl, time_area_response, time_area_painter, tree_max_y, @@ -409,6 +441,7 @@ impl TimePanel { if ctx.app_options.show_blueprint_in_timeline { self.show_tree( ctx, + time_ctrl, time_area_response, time_area_painter, tree_max_y, @@ -425,6 +458,7 @@ impl TimePanel { fn show_tree( &mut self, ctx: &mut ViewerContext<'_>, + time_ctrl: &mut TimeControl, time_area_response: &egui::Response, time_area_painter: &egui::Painter, tree_max_y: f32, @@ -469,6 +503,7 @@ impl TimePanel { .show_collapsing(ui, collapsing_header_id, default_open, |_, ui| { self.show_children( ctx, + time_ctrl, time_area_response, time_area_painter, tree_max_y, @@ -510,12 +545,13 @@ impl TimePanel { let empty = re_data_store::TimeHistogram::default(); let num_messages_at_time = tree .recursive_time_histogram - .get(ctx.rec_cfg.time_ctrl.timeline()) + .get(time_ctrl.timeline()) .unwrap_or(&empty); data_density_graph::data_density_graph_ui( &mut self.data_density_graph_painter, ctx, + time_ctrl, time_area_response, time_area_painter, ui, @@ -529,9 +565,11 @@ impl TimePanel { } } + #[allow(clippy::too_many_arguments)] fn show_children( &mut self, ctx: &mut ViewerContext<'_>, + time_ctrl: &mut TimeControl, time_area_response: &egui::Response, time_area_painter: &egui::Painter, tree_max_y: f32, @@ -541,6 +579,7 @@ impl TimePanel { for (last_component, child) in &tree.children { self.show_tree( ctx, + time_ctrl, time_area_response, time_area_painter, tree_max_y, @@ -596,10 +635,10 @@ impl TimePanel { let response_rect = response.rect; + let timeline = time_ctrl.timeline(); + let empty_messages_over_time = TimeHistogram::default(); - let messages_over_time = data - .get(ctx.rec_cfg.time_ctrl.timeline()) - .unwrap_or(&empty_messages_over_time); + let messages_over_time = data.get(timeline).unwrap_or(&empty_messages_over_time); // `data.times` does not contain timeless. Need to add those manually: let total_num_messages = @@ -608,7 +647,7 @@ impl TimePanel { if total_num_messages == 0 { ui.label(ctx.re_ui.warning_text(format!( "No event logged on timeline {:?}", - ctx.rec_cfg.time_ctrl.timeline().name() + timeline.name() ))); } else { ui.label(format!("Number of events: {total_num_messages}")); @@ -635,6 +674,7 @@ impl TimePanel { data_density_graph::data_density_graph_ui( &mut self.data_density_graph_painter, ctx, + time_ctrl, time_area_response, time_area_painter, ui, @@ -649,7 +689,12 @@ impl TimePanel { } } - fn top_row_ui(&mut self, ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) { + fn top_row_ui( + &mut self, + ctx: &mut ViewerContext<'_>, + ui: &mut egui::Ui, + time_ctrl: &mut TimeControl, + ) { ui.spacing_mut().item_spacing.x = 18.0; // from figma if ui.max_rect().width() < 600.0 { @@ -657,7 +702,6 @@ impl TimePanel { ui.vertical(|ui| { ui.horizontal(|ui| { let re_ui = &ctx.re_ui; - let time_ctrl = &mut ctx.rec_cfg.time_ctrl; let times_per_timeline = ctx.store_db.times_per_timeline(); self.time_control_ui .play_pause_ui(time_ctrl, re_ui, times_per_timeline, ui); @@ -665,14 +709,13 @@ impl TimePanel { self.time_control_ui.fps_ui(time_ctrl, ui); }); ui.horizontal(|ui| { - let time_ctrl = &mut ctx.rec_cfg.time_ctrl; self.time_control_ui.timeline_selector_ui( time_ctrl, ctx.store_db.times_per_timeline(), ui, ); - current_time_ui(ctx, ui); + current_time_ui(ctx, ui, time_ctrl); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { help_button(ui); @@ -682,7 +725,6 @@ impl TimePanel { } else { // One row: let re_ui = &ctx.re_ui; - let time_ctrl = &mut ctx.rec_cfg.time_ctrl; let times_per_timeline = ctx.store_db.times_per_timeline(); self.time_control_ui @@ -691,7 +733,7 @@ impl TimePanel { .timeline_selector_ui(time_ctrl, times_per_timeline, ui); self.time_control_ui.playback_speed_ui(time_ctrl, ui); self.time_control_ui.fps_ui(time_ctrl, ui); - current_time_ui(ctx, ui); + current_time_ui(ctx, ui, time_ctrl); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { help_button(ui); @@ -729,8 +771,12 @@ fn highlight_timeline_row( } } -fn collapsed_time_marker_and_time(ui: &mut egui::Ui, ctx: &mut ViewerContext<'_>) { - let space_needed_for_current_time = match ctx.rec_cfg.time_ctrl.timeline().typ() { +fn collapsed_time_marker_and_time( + ui: &mut egui::Ui, + ctx: &mut ViewerContext<'_>, + time_ctrl: &mut TimeControl, +) { + let space_needed_for_current_time = match time_ctrl.timeline().typ() { re_arrow_store::TimeType::Time => 220.0, re_arrow_store::TimeType::Sequence => 100.0, }; @@ -740,12 +786,20 @@ fn collapsed_time_marker_and_time(ui: &mut egui::Ui, ctx: &mut ViewerContext<'_> time_range_rect.max.x -= space_needed_for_current_time; if time_range_rect.width() > 50.0 { - let time_ranges_ui = initialize_time_ranges_ui(ctx, time_range_rect.x_range(), None); + let time_ranges_ui = + initialize_time_ranges_ui(ctx, time_ctrl, time_range_rect.x_range(), None); time_ranges_ui.snap_time_control(ctx); let painter = ui.painter_at(time_range_rect.expand(4.0)); - paint_visible_history_range(&time_ranges_ui, ctx, &painter, time_range_rect); + if let Some(highlighted_range) = time_ctrl.highlighted_range { + paint_range_highlight( + highlighted_range, + &time_ranges_ui, + &painter, + time_range_rect, + ); + } painter.hline( time_range_rect.x_range(), @@ -754,7 +808,7 @@ fn collapsed_time_marker_and_time(ui: &mut egui::Ui, ctx: &mut ViewerContext<'_> ); time_marker_ui( &time_ranges_ui, - &mut ctx.rec_cfg.time_ctrl, + time_ctrl, ctx.re_ui, ui, &painter, @@ -765,30 +819,28 @@ fn collapsed_time_marker_and_time(ui: &mut egui::Ui, ctx: &mut ViewerContext<'_> } } - current_time_ui(ctx, ui); + current_time_ui(ctx, ui, time_ctrl); } -fn paint_visible_history_range( +fn paint_range_highlight( + highlighted_range: TimeRange, time_ranges_ui: &TimeRangesUi, - ctx: &mut ViewerContext<'_>, painter: &egui::Painter, rect: Rect, ) { - if let Some(time_range) = ctx.rec_cfg.visible_history_highlight { - let x_from = time_ranges_ui.x_from_time_f32(time_range.min.into()); - let x_to = time_ranges_ui.x_from_time_f32(time_range.max.into()); - - if let (Some(x_from), Some(x_to)) = (x_from, x_to) { - let visible_history_area_rect = - Rect::from_x_y_ranges(x_from..=x_to, rect.y_range()).intersect(rect); - - painter.rect( - visible_history_area_rect, - 0.0, - egui::Color32::WHITE.gamma_multiply(0.1), - egui::Stroke::NONE, - ); - } + let x_from = time_ranges_ui.x_from_time_f32(highlighted_range.min.into()); + let x_to = time_ranges_ui.x_from_time_f32(highlighted_range.max.into()); + + if let (Some(x_from), Some(x_to)) = (x_from, x_to) { + let visible_history_area_rect = + Rect::from_x_y_ranges(x_from..=x_to, rect.y_range()).intersect(rect); + + painter.rect( + visible_history_area_rect, + 0.0, + egui::Color32::WHITE.gamma_multiply(0.1), + egui::Stroke::NONE, + ); } } @@ -841,11 +893,11 @@ fn is_time_safe_to_show( TimeInt::BEGINNING < time } -fn current_time_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { - if let Some(time_int) = ctx.rec_cfg.time_ctrl.time_int() { - let timeline = ctx.rec_cfg.time_ctrl.timeline(); +fn current_time_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, time_ctrl: &TimeControl) { + if let Some(time_int) = time_ctrl.time_int() { + let timeline = time_ctrl.timeline(); if is_time_safe_to_show(ctx.store_db, timeline, time_int.into()) { - let time_type = ctx.rec_cfg.time_ctrl.time_type(); + let time_type = time_ctrl.time_type(); ui.monospace(time_type.format(time_int, ctx.app_options.time_zone_for_timestamps)); } } @@ -855,6 +907,7 @@ fn current_time_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { fn initialize_time_ranges_ui( ctx: &ViewerContext<'_>, + time_ctrl: &TimeControl, time_x_range: Rangef, mut time_view: Option, ) -> TimeRangesUi { @@ -870,13 +923,10 @@ fn initialize_time_ranges_ui( Vec::new() }; - if let Some(times) = ctx - .store_db - .time_histogram(ctx.rec_cfg.time_ctrl.timeline()) - { + if let Some(times) = ctx.store_db.time_histogram(time_ctrl.timeline()) { // NOTE: `times` can be empty if a GC wiped everything. if !times.is_empty() { - let timeline_axis = TimelineAxis::new(ctx.rec_cfg.time_ctrl.time_type(), times); + let timeline_axis = TimelineAxis::new(time_ctrl.time_type(), times); time_view = time_view.or_else(|| Some(view_everything(&time_x_range, &timeline_axis))); time_range.extend(timeline_axis.ranges); } diff --git a/crates/re_time_panel/src/time_ranges_ui.rs b/crates/re_time_panel/src/time_ranges_ui.rs index 480a239b62f0..8c9e989bae4b 100644 --- a/crates/re_time_panel/src/time_ranges_ui.rs +++ b/crates/re_time_panel/src/time_ranges_ui.rs @@ -242,15 +242,17 @@ impl TimeRangesUi { // Make sure playback time doesn't get stuck between non-continuous regions: pub fn snap_time_control(&self, ctx: &mut ViewerContext<'_>) { - if ctx.rec_cfg.time_ctrl.play_state() != PlayState::Playing { + let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); + + if time_ctrl.play_state() != PlayState::Playing { return; } // Make sure time doesn't get stuck between non-continuous regions: - if let Some(time) = ctx.rec_cfg.time_ctrl.time() { + if let Some(time) = time_ctrl.time() { let time = self.snap_time_to_segments(time); - ctx.rec_cfg.time_ctrl.set_time(time); - } else if let Some(selection) = ctx.rec_cfg.time_ctrl.loop_selection() { + time_ctrl.set_time(time); + } else if let Some(selection) = time_ctrl.loop_selection() { let snapped_min = self.snap_time_to_segments(selection.min); let snapped_max = self.snap_time_to_segments(selection.max); @@ -262,7 +264,7 @@ impl TimeRangesUi { } // Keeping max works better when looping - ctx.rec_cfg.time_ctrl.set_loop_selection(TimeRangeF::new( + time_ctrl.set_loop_selection(TimeRangeF::new( snapped_max - selection.length(), snapped_max, )); diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index bc23bef590a3..dd3a897859db 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -543,7 +543,7 @@ impl App { let Some(rec_cfg) = self.state.recording_config_mut(rec_id) else { return; }; - let time_ctrl = &mut rec_cfg.time_ctrl; + let time_ctrl = rec_cfg.time_ctrl.get_mut(); let times_per_timeline = store_db.times_per_timeline(); diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 2ceec4bf5eec..2245d647796b 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -66,10 +66,10 @@ impl AppState { .get(rec_id) // is there an active loop selection? .and_then(|rec_cfg| { - rec_cfg - .time_ctrl + let time_ctrl = rec_cfg.time_ctrl.read(); + time_ctrl .loop_selection() - .map(|q| (*rec_cfg.time_ctrl.timeline(), q)) + .map(|q| (*time_ctrl.timeline(), q)) }) }) } @@ -112,7 +112,7 @@ impl AppState { let entities_per_system_per_class = identify_entities_per_system_per_class( space_view_class_registry, store_db, - &rec_cfg.time_ctrl.current_query(), + &rec_cfg.time_ctrl.get_mut().current_query(), ); // Execute the queries for every `SpaceView` @@ -276,7 +276,7 @@ impl AppState { false }; - let needs_repaint = ctx.rec_cfg.time_ctrl.update( + let needs_repaint = ctx.rec_cfg.time_ctrl.write().update( store_db.times_per_timeline(), dt, more_data_is_coming, @@ -333,6 +333,7 @@ fn recording_config_entry<'cfgs>( rec_cfg .time_ctrl + .get_mut() .set_play_state(store_db.times_per_timeline(), play_state); rec_cfg diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 7459e86521d4..1bb8e7da185a 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -52,7 +52,7 @@ impl SelectionPanel { }); // Always reset the VH highlight, and let the UI re-set it if needed. - ctx.rec_cfg.visible_history_highlight = None; + ctx.rec_cfg.time_ctrl.write().highlighted_range = None; panel.show_animated_inside(ui, expanded, |ui: &mut egui::Ui| { // Set the clip rectangle to the panel for the benefit of nested, "full span" widgets diff --git a/crates/re_viewer/src/ui/visible_history.rs b/crates/re_viewer/src/ui/visible_history.rs index 7cc929e0a6d2..c125938cc4ad 100644 --- a/crates/re_viewer/src/ui/visible_history.rs +++ b/crates/re_viewer/src/ui/visible_history.rs @@ -8,7 +8,7 @@ use re_log_types::{EntityPath, TimeRange, TimeType, TimeZone}; use re_space_view_spatial::{SpatialSpaceView2D, SpatialSpaceView3D}; use re_space_view_time_series::TimeSeriesSpaceView; use re_types_core::ComponentName; -use re_viewer_context::{SpaceViewClass, SpaceViewClassName, ViewerContext}; +use re_viewer_context::{SpaceViewClass, SpaceViewClassName, TimeControl, ViewerContext}; /// These space views support the Visible History feature. static VISIBLE_HISTORY_SUPPORTED_SPACE_VIEWS: once_cell::sync::Lazy> = @@ -43,7 +43,8 @@ static VISIBLE_HISTORY_SUPPORTED_COMPONENT_NAMES: once_cell::sync::Lazy, + ctx: &ViewerContext<'_>, + time_ctrl: &TimeControl, space_view_class: &SpaceViewClassName, entity_path: Option<&EntityPath>, ) -> bool { @@ -53,7 +54,7 @@ fn has_visible_history( if let Some(entity_path) = entity_path { let store = ctx.store_db.store(); - let component_names = store.all_components(ctx.rec_cfg.time_ctrl.timeline(), entity_path); + let component_names = store.all_components(time_ctrl.timeline(), entity_path); if let Some(component_names) = component_names { if !component_names .iter() @@ -78,13 +79,14 @@ pub fn visible_history_ui( visible_history_prop: &mut ExtraQueryHistory, resolved_visible_history_prop: &ExtraQueryHistory, ) { - if !has_visible_history(ctx, space_view_class, entity_path) { + let time_ctrl = ctx.rec_cfg.time_ctrl.read().clone(); + if !has_visible_history(ctx, &time_ctrl, space_view_class, entity_path) { return; } let re_ui = ctx.re_ui; - let is_sequence_timeline = matches!(ctx.rec_cfg.time_ctrl.timeline().typ(), TimeType::Sequence); + let is_sequence_timeline = matches!(time_ctrl.timeline().typ(), TimeType::Sequence); let mut interacting_with_controls = false; @@ -109,18 +111,13 @@ pub fn visible_history_ui( }); }); - let timeline_spec = if let Some(times) = ctx - .store_db - .time_histogram(ctx.rec_cfg.time_ctrl.timeline()) - { + let timeline_spec = if let Some(times) = ctx.store_db.time_histogram(time_ctrl.timeline()) { TimelineSpec::from_time_histogram(times) } else { TimelineSpec::from_time_range(0..=0) }; - let current_time = ctx - .rec_cfg - .time_ctrl + let current_time = time_ctrl .time_i64() .unwrap_or_default() .at_least(*timeline_spec.range.start()); // accounts for timeless time (TimeInt::BEGINNING) @@ -219,7 +216,7 @@ pub fn visible_history_ui( .map_or(false, |r| r.hovered()); if should_display_visible_history { - if let Some(current_time) = ctx.rec_cfg.time_ctrl.time_int() { + if let Some(current_time) = time_ctrl.time_int() { let visible_history = match (visible_history_prop.enabled, is_sequence_timeline) { (true, true) => visible_history_prop.sequences, (true, false) => visible_history_prop.nanos, @@ -227,7 +224,7 @@ pub fn visible_history_ui( (false, false) => resolved_visible_history_prop.nanos, }; - ctx.rec_cfg.visible_history_highlight = Some(TimeRange::new( + ctx.rec_cfg.time_ctrl.write().highlighted_range = Some(TimeRange::new( visible_history.from(current_time), visible_history.to(current_time), )); diff --git a/crates/re_viewer_context/Cargo.toml b/crates/re_viewer_context/Cargo.toml index 8531cb052f6f..20bc77a88e0f 100644 --- a/crates/re_viewer_context/Cargo.toml +++ b/crates/re_viewer_context/Cargo.toml @@ -41,8 +41,8 @@ macaw.workspace = true ndarray.workspace = true nohash-hasher.workspace = true once_cell.workspace = true -parking_lot.workspace = true -serde = "1" +parking_lot = { workspace = true, features = ["serde"] } +serde.workspace = true slotmap.workspace = true smallvec.workspace = true thiserror.workspace = true diff --git a/crates/re_viewer_context/src/time_control.rs b/crates/re_viewer_context/src/time_control.rs index e8bd42603062..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)] +#[derive(serde::Deserialize, serde::Serialize, Clone, PartialEq)] #[serde(default)] pub struct TimeControl { /// Name of the timeline (e.g. "log_time"). @@ -97,6 +97,12 @@ pub struct TimeControl { speed: f32, looping: Looping, + + /// Range with special highlight. + /// + /// This is used during UI interactions. E.g. to show visual history range that's highlighted. + #[serde(skip)] + pub highlighted_range: Option, } impl Default for TimeControl { @@ -108,6 +114,7 @@ impl Default for TimeControl { following: true, speed: 1.0, looping: Looping::Off, + highlighted_range: None, } } } diff --git a/crates/re_viewer_context/src/viewer_context.rs b/crates/re_viewer_context/src/viewer_context.rs index 710bb888a5ae..6d9b9fd9c103 100644 --- a/crates/re_viewer_context/src/viewer_context.rs +++ b/crates/re_viewer_context/src/viewer_context.rs @@ -1,14 +1,13 @@ use ahash::HashMap; -use re_data_store::store_db::StoreDb; -use re_data_store::{EntityTree, TimeHistogramPerTimeline}; -use re_log_types::TimeRange; +use parking_lot::RwLock; + +use re_data_store::{store_db::StoreDb, EntityTree, TimeHistogramPerTimeline}; -use crate::query_context::DataQueryResult; use crate::{ - item::resolve_mono_instance_path_item, AppOptions, Caches, CommandSender, ComponentUiRegistry, - Item, ItemCollection, SelectionState, SpaceViewClassRegistry, StoreContext, TimeControl, + item::resolve_mono_instance_path_item, query_context::DataQueryResult, AppOptions, Caches, + CommandSender, ComponentUiRegistry, DataQueryId, EntitiesPerSystemPerClass, Item, + ItemCollection, SelectionState, SpaceViewClassRegistry, StoreContext, TimeControl, }; -use crate::{DataQueryId, EntitiesPerSystemPerClass}; /// Common things needed by many parts of the viewer. pub struct ViewerContext<'a> { @@ -40,7 +39,7 @@ pub struct ViewerContext<'a> { pub query_results: &'a HashMap, /// UI config for the current recording (found in [`StoreDb`]). - pub rec_cfg: &'a mut RecordingConfig, + pub rec_cfg: &'a RecordingConfig, /// The look and feel of the UI. pub re_ui: &'a re_ui::ReUi, @@ -58,7 +57,7 @@ impl<'a> ViewerContext<'a> { self.rec_cfg .selection_state .set_single_selection(resolve_mono_instance_path_item( - &self.rec_cfg.time_ctrl.current_query(), + &self.rec_cfg.time_ctrl.read().current_query(), self.store_db.store(), item, )); @@ -80,7 +79,7 @@ impl<'a> ViewerContext<'a> { .selection_state .set_hovered(hovered.map(|item| { resolve_mono_instance_path_item( - &self.rec_cfg.time_ctrl.current_query(), + &self.rec_cfg.time_ctrl.read().current_query(), self.store_db.store(), item, ) @@ -93,13 +92,13 @@ impl<'a> ViewerContext<'a> { /// The current time query, based on the current time control. pub fn current_query(&self) -> re_arrow_store::LatestAtQuery { - self.rec_cfg.time_ctrl.current_query() + self.rec_cfg.time_ctrl.read().current_query() } /// Returns whether the given tree has any data logged in the current timeline. pub fn tree_has_data_in_current_timeline(&self, tree: &EntityTree) -> bool { tree.recursive_time_histogram - .has_timeline(self.rec_cfg.time_ctrl.timeline()) + .has_timeline(self.rec_cfg.time_ctrl.read().timeline()) || tree.num_timeless_messages() > 0 } @@ -108,7 +107,7 @@ impl<'a> ViewerContext<'a> { &self, component_stat: &TimeHistogramPerTimeline, ) -> bool { - component_stat.has_timeline(self.rec_cfg.time_ctrl.timeline()) + component_stat.has_timeline(self.rec_cfg.time_ctrl.read().timeline()) || component_stat.num_timeless_messages() > 0 } } @@ -120,14 +119,8 @@ impl<'a> ViewerContext<'a> { #[serde(default)] pub struct RecordingConfig { /// The current time of the time panel, how fast it is moving, etc. - pub time_ctrl: TimeControl, + pub time_ctrl: RwLock, /// Selection & hovering state. pub selection_state: SelectionState, - - /// Should the visible history time range be highlighted? - /// - /// This is used during UI interactions to show the range of time that is being edited. - #[serde(skip)] - pub visible_history_highlight: Option, } diff --git a/crates/re_viewport/src/space_view.rs b/crates/re_viewport/src/space_view.rs index e412bdb78752..bbb54d0aa98b 100644 --- a/crates/re_viewport/src/space_view.rs +++ b/crates/re_viewport/src/space_view.rs @@ -271,7 +271,7 @@ impl SpaceViewBlueprint { space_view_id: self.id, space_origin: &self.space_origin, per_system_data_results: &per_system_data_results, - timeline: *ctx.rec_cfg.time_ctrl.timeline(), + timeline: *ctx.rec_cfg.time_ctrl.read().timeline(), latest_at, highlights, }; diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index f47b7c6f56b0..561ed1b16563 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -475,7 +475,7 @@ fn space_view_ui( space_view_state: &mut dyn SpaceViewState, space_view_highlights: &SpaceViewHighlights, ) { - let Some(latest_at) = ctx.rec_cfg.time_ctrl.time_int() else { + let Some(latest_at) = ctx.rec_cfg.time_ctrl.read().time_int() else { ui.centered_and_justified(|ui| { ui.weak("No time selected"); });