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

Time control is now behind a RwLock, making recording config access non-mutable everywhere #4389

Merged
merged 4 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions crates/re_data_ui/src/item_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
Expand All @@ -269,14 +274,15 @@ pub fn timeline_button_to(
text: impl Into<egui::WidgetText>,
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
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_space_view_spatial/src/contexts/depth_offsets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<DrawOrder>(
&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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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(
Expand Down
7 changes: 5 additions & 2 deletions crates/re_space_view_text_log/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ impl SpaceViewClass for TextSpaceView {
let time = ctx
.rec_cfg
.time_ctrl
.read()
.time_i64()
.unwrap_or(state.latest_time);

Expand Down Expand Up @@ -276,8 +277,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)
Expand Down
23 changes: 14 additions & 9 deletions crates/re_space_view_time_series/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,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();

Expand Down Expand Up @@ -235,12 +239,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 {
Expand Down Expand Up @@ -300,7 +305,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();

Expand Down
12 changes: 7 additions & 5 deletions crates/re_time_panel/src/data_density_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -368,6 +368,7 @@ fn smooth(density: &[f32]) -> Vec<f32> {
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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
}
Loading
Loading