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.

Sign up for GitHub

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

Automatically select user timeline if no timeline was explicitly selected yet #2986

Merged
merged 3 commits into from
Aug 16, 2023
+65 −39
Merged

Automatically select user timeline if no timeline was explicitly selected yet #2986

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cd470e
Automatically select user timeline if no timeline was explicitly sele…
Wumpf Aug 15, 2023
17cb433
clarify code that sets default timeline
Wumpf Aug 15, 2023
5e49b00
implement deref for EditableAutoValue
Wumpf Aug 15, 2023
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
94 changes: 53 additions & 41 deletions crates/re_viewer_context/src/time_control.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::collections::{BTreeMap, BTreeSet};

use re_data_store::TimesPerTimeline;
use re_data_store::{EditableAutoValue, TimesPerTimeline};
use re_log_types::{Duration, TimeInt, TimeRange, TimeRangeF, TimeReal, TimeType, Timeline};

use crate::NeedsRepaint;
Expand Down Expand Up @@ -83,7 +83,7 @@ pub enum PlayState {
#[serde(default)]
pub struct TimeControl {
/// Name of the timeline (e.g. "log_time").
timeline: Timeline,
timeline: EditableAutoValue<Timeline>,

states: BTreeMap<Timeline, TimeState>,

Expand Down Expand Up @@ -127,13 +127,15 @@ impl TimeControl {
return NeedsRepaint::No; // we have no data on this timeline yet, so bail
};

let timeline = *self.timeline.get();

match self.play_state() {
PlayState::Paused => {
// It's possible that the playback is paused because e.g. it reached its end, but
// then the user decides to switch timelines.
// When they do so, it might be the case that they switch to a timeline they've
// never interacted with before, in which case we don't even have a time state yet.
self.states.entry(self.timeline).or_insert_with(|| {
self.states.entry(timeline).or_insert_with(|| {
TimeState::new(if self.following {
full_range.max
} else {
Expand All @@ -147,7 +149,7 @@ impl TimeControl {

let state = self
.states
.entry(self.timeline)
.entry(timeline)
.or_insert_with(|| TimeState::new(full_range.min));

if self.looping == Looping::Off && full_range.max <= state.time {
Expand All @@ -173,7 +175,7 @@ impl TimeControl {
state.time = state.time.max(loop_range.min);
}

match self.timeline.typ() {
match timeline.typ() {
TimeType::Sequence => {
state.time += TimeReal::from(state.fps * dt);
}
Expand All @@ -190,7 +192,7 @@ impl TimeControl {
}
PlayState::Following => {
// Set the time to the max:
match self.states.entry(self.timeline) {
match self.states.entry(timeline) {
std::collections::btree_map::Entry::Vacant(entry) => {
entry.insert(TimeState::new(full_range.max));
}
Expand Down Expand Up @@ -232,6 +234,8 @@ impl TimeControl {
}

pub fn set_play_state(&mut self, times_per_timeline: &TimesPerTimeline, play_state: PlayState) {
let timeline = self.timeline.get();

match play_state {
PlayState::Paused => {
self.playing = false;
Expand All @@ -241,24 +245,24 @@ impl TimeControl {
self.following = false;

// Start from beginning if we are at the end:
if let Some(time_points) = times_per_timeline.get(&self.timeline) {
if let Some(state) = self.states.get_mut(&self.timeline) {
if let Some(time_points) = times_per_timeline.get(timeline) {
if let Some(state) = self.states.get_mut(timeline) {
if max(time_points) <= state.time {
state.time = min(time_points).into();
}
} else {
self.states
.insert(self.timeline, TimeState::new(min(time_points)));
.insert(*timeline, TimeState::new(min(time_points)));
}
}
}
PlayState::Following => {
self.playing = true;
self.following = true;

if let Some(time_points) = times_per_timeline.get(&self.timeline) {
if let Some(time_points) = times_per_timeline.get(timeline) {
// Set the time to the max:
match self.states.entry(self.timeline) {
match self.states.entry(*timeline) {
std::collections::btree_map::Entry::Vacant(entry) => {
entry.insert(TimeState::new(max(time_points)));
}
Expand Down Expand Up @@ -308,8 +312,8 @@ impl TimeControl {
}

pub fn restart(&mut self, times_per_timeline: &TimesPerTimeline) {
if let Some(time_points) = times_per_timeline.get(&self.timeline) {
if let Some(state) = self.states.get_mut(&self.timeline) {
if let Some(time_points) = times_per_timeline.get(self.timeline.get()) {
if let Some(state) = self.states.get_mut(self.timeline.get()) {
state.time = min(time_points).into();
self.following = false;
}
Expand Down Expand Up @@ -344,8 +348,8 @@ impl TimeControl {
// the beginning in play mode.

// Start from beginning if we are at the end:
if let Some(time_points) = times_per_timeline.get(&self.timeline) {
if let Some(state) = self.states.get_mut(&self.timeline) {
if let Some(time_points) = times_per_timeline.get(self.timeline.get()) {
if let Some(state) = self.states.get_mut(self.timeline.get()) {
if max(time_points) <= state.time {
state.time = min(time_points).into();
self.playing = true;
Expand Down Expand Up @@ -375,47 +379,53 @@ impl TimeControl {

/// playback fps
pub fn fps(&self) -> Option<f32> {
self.states.get(&self.timeline).map(|state| state.fps)
self.states.get(self.timeline.get()).map(|state| state.fps)
}

/// playback fps
pub fn set_fps(&mut self, fps: f32) {
if let Some(state) = self.states.get_mut(&self.timeline) {
if let Some(state) = self.states.get_mut(self.timeline.get()) {
state.fps = fps;
}
}

/// Make sure the selected timeline is a valid one
pub fn select_a_valid_timeline(&mut self, times_per_timeline: &TimesPerTimeline) {
for timeline in times_per_timeline.timelines() {
if &self.timeline == timeline {
return; // it's valid
fn is_timeline_valid(selected: &Timeline, times_per_timeline: &TimesPerTimeline) -> bool {
for timeline in times_per_timeline.timelines() {
if selected == timeline {
return true; // it's valid
}
}
false
}
if let Some(timeline) = default_timeline(times_per_timeline.timelines()) {
self.timeline = *timeline;
} else {
self.timeline = Default::default();

if !self.timeline.is_auto() && is_timeline_valid(self.timeline.get(), times_per_timeline) {
return;
Wumpf marked this conversation as resolved.
Show resolved Hide resolved
}

self.timeline = EditableAutoValue::Auto(
default_timeline(times_per_timeline.timelines()).map_or(Default::default(), |t| *t),
);
}

/// The currently selected timeline
pub fn timeline(&self) -> &Timeline {
&self.timeline
self.timeline.get()
}

/// The time type of the currently selected timeline
pub fn time_type(&self) -> TimeType {
self.timeline.typ()
self.timeline.get().typ()
}

pub fn set_timeline(&mut self, timeline: Timeline) {
self.timeline = timeline;
self.timeline = EditableAutoValue::UserEdited(timeline);
}

/// The current time.
pub fn time(&self) -> Option<TimeReal> {
self.states.get(&self.timeline).map(|state| state.time)
self.states.get(self.timeline.get()).map(|state| state.time)
}

/// The current time.
Expand All @@ -431,43 +441,43 @@ impl TimeControl {
/// Query for latest value at the currently selected time on the currently selected timeline.
pub fn current_query(&self) -> re_arrow_store::LatestAtQuery {
re_arrow_store::LatestAtQuery::new(
self.timeline,
*self.timeline.get(),
self.time().map_or(TimeInt::MAX, |t| t.floor()),
)
}

/// The current loop range, iff selection looping is turned on.
pub fn active_loop_selection(&self) -> Option<TimeRangeF> {
if self.looping == Looping::Selection {
self.states.get(&self.timeline)?.loop_selection
self.states.get(self.timeline.get())?.loop_selection
} else {
None
}
}

/// The full range of times for the current timeline
pub fn full_range(&self, times_per_timeline: &TimesPerTimeline) -> Option<TimeRange> {
times_per_timeline.get(&self.timeline).map(range)
times_per_timeline.get(self.timeline.get()).map(range)
}

/// The selected slice of time that is called the "loop selection".
///
/// This can still return `Some` even if looping is currently off.
pub fn loop_selection(&self) -> Option<TimeRangeF> {
self.states.get(&self.timeline)?.loop_selection
self.states.get(self.timeline.get())?.loop_selection
}

/// Set the current loop selection without enabling looping.
pub fn set_loop_selection(&mut self, selection: TimeRangeF) {
self.states
.entry(self.timeline)
.entry(*self.timeline.get())
.or_insert_with(|| TimeState::new(selection.min))
.loop_selection = Some(selection);
}

/// Remove the current loop selection.
pub fn remove_loop_selection(&mut self) {
if let Some(state) = self.states.get_mut(&self.timeline) {
if let Some(state) = self.states.get_mut(self.timeline.get()) {
state.loop_selection = None;
}
if self.looping() == Looping::Selection {
Expand All @@ -477,47 +487,49 @@ impl TimeControl {

/// Is the current time in the selection range (if any), or at the current time mark?
pub fn is_time_selected(&self, timeline: &Timeline, needle: TimeInt) -> bool {
if timeline != &self.timeline {
if timeline != self.timeline.get() {
return false;
}

if let Some(state) = self.states.get(&self.timeline) {
if let Some(state) = self.states.get(self.timeline.get()) {
state.time.floor() == needle
} else {
false
}
}

pub fn set_timeline_and_time(&mut self, timeline: Timeline, time: impl Into<TimeReal>) {
self.timeline = timeline;
self.timeline = EditableAutoValue::UserEdited(timeline);
self.set_time(time);
}

pub fn set_time(&mut self, time: impl Into<TimeReal>) {
let time = time.into();

self.states
.entry(self.timeline)
.entry(*self.timeline.get())
.or_insert_with(|| TimeState::new(time))
.time = time;
}

/// The range of time we are currently zoomed in on.
pub fn time_view(&self) -> Option<TimeView> {
self.states.get(&self.timeline).and_then(|state| state.view)
self.states
.get(self.timeline.get())
.and_then(|state| state.view)
}

/// The range of time we are currently zoomed in on.
pub fn set_time_view(&mut self, view: TimeView) {
self.states
.entry(self.timeline)
.entry(*self.timeline.get())
.or_insert_with(|| TimeState::new(view.min))
.view = Some(view);
}

/// The range of time we are currently zoomed in on.
pub fn reset_time_view(&mut self) {
if let Some(state) = self.states.get_mut(&self.timeline) {
if let Some(state) = self.states.get_mut(self.timeline.get()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of changing all &foo to foo.get(), just implement Deref for EditableAutoValue!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for those cases I still need get() due to the way Deref works in rust: *self.timeline gives the underlying Timeline by value. But with Deref in place *self.timeline.get() becomes *self.timeline and self.timeline.get().typ() becomes self.timeline.typ()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could ofc also do &*self.timeline.get() if that's preferred

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean do &*self.timeline

state.view = None;
}
}
Expand Down