Skip to content

Fix play/pause toggle annoyance #1066

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

Merged
emilk merged 2 commits into from
Feb 3, 2023
Merged

Fix play/pause toggle annoyance #1066

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eeab418
Fix play/pause toggle annoyance
emilk Feb 2, 2023
bae242b
wording
emilk Feb 2, 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
36 changes: 35 additions & 1 deletion crates/re_viewer/src/misc/time_control.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl TimeControl {
// 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 state.time >= max(time_points) {
if max(time_points) <= state.time {
state.time = min(time_points).into();
}
} else {
Expand Down Expand Up @@ -286,6 +286,40 @@ impl TimeControl {
if self.playing {
self.pause();
} else {
// If we are in follow-mode (but paused), what should toggling play/pause do?
//
// There are two cases to consider:
// * We are looking at a file
// * We are following a stream
//
// If we are watching a stream, it makes sense to keep following:
// you paused to look at something, now you're done, so keep following.
//
// If you are watching a file: if the file has finished loading, then
// it can still make sense to go to the end of it.
// But if you're already at the end, then staying at "follow" makes little sense,
// as repeated toggling will just go between paused and follow at the latest data.
// This is made worse by Follow being our default mode (even for files).
//
// As of writing (2023-02) we don't know if we are watching a file or a stream
// (after all, files are also streamed).
//
// So we use a heuristic:
// If we are at the end of the file and unpause, we always start from
// 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 max(time_points) <= state.time {
state.time = min(time_points).into();
self.playing = true;
self.following = false;
Comment on lines +315 to +317
Copy link
Member

@teh-cmc teh-cmc Feb 3, 2023

Choose a reason for hiding this comment

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

Suggested change
state.time = min(time_points).into();
self.playing = true;
self.following = false;
self.set_play_state(times_per_timeline, PlayState::Playing);

Copy link
Member Author

@emilk emilk Feb 3, 2023

Choose a reason for hiding this comment

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

I kind of liked being explicit here

return;
}
}
}

if self.following {
self.set_play_state(times_per_timeline, PlayState::Following);
} else {
Expand Down