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

Fix precision issue when zooming in on the timeline #1370

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Fix precision issue when zooming in on the timeline #1370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions crates/re_log_types/src/time_range.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,16 @@ impl TimeRangeF {
/// Where in the range is this value? Returns 0-1 if within the range.
///
/// Returns <0 if before and >1 if after.
pub fn inverse_lerp(&self, value: TimeReal) -> f32 {
pub fn inverse_lerp(&self, value: TimeReal) -> f64 {
if self.min == self.max {
0.5
} else {
(value - self.min).as_f32() / (self.max - self.min).as_f32()
(value - self.min).as_f64() / (self.max - self.min).as_f64()
}
}

pub fn lerp(&self, t: f32) -> TimeReal {
self.min + (self.max - self.min) * t as f64
pub fn lerp(&self, t: f64) -> TimeReal {
self.min + (self.max - self.min) * t
}

#[inline]
Expand Down
28 changes: 14 additions & 14 deletions crates/re_viewer/src/ui/time_panel/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,8 +644,8 @@ fn show_data_over_time(

// painting each data point as a separate circle is slow (too many circles!)
// so we join time points that are close together.
let points_per_time = time_ranges_ui.points_per_time().unwrap_or(f32::INFINITY);
let max_stretch_length_in_time = 1.0 / points_per_time as f64; // TODO(emilk)
let points_per_time = time_ranges_ui.points_per_time().unwrap_or(f64::INFINITY);
let max_stretch_length_in_time = 1.0 / points_per_time; // TODO(emilk)

let pointer_pos = ui.input(|i| i.pointer.hover_pos());

Expand Down Expand Up @@ -677,7 +677,7 @@ fn show_data_over_time(

let mut paint_stretch = |stretch: &Stretch| {
let stop_x = time_ranges_ui
.x_from_time(stretch.stop_time.into())
.x_from_time_f32(stretch.stop_time.into())
.unwrap_or(stretch.start_x);

let num_messages: usize = stretch.time_points.iter().map(|(_time, count)| count).sum();
Expand Down Expand Up @@ -722,7 +722,7 @@ fn show_data_over_time(
if num_timeless_messages > 0 {
let time_int = TimeInt::BEGINNING;
let time_real = TimeReal::from(time_int);
if let Some(x) = time_ranges_ui.x_from_time(time_real) {
if let Some(x) = time_ranges_ui.x_from_time_f32(time_real) {
let selected = selected_time_range.map_or(true, |range| range.contains(time_real));
paint_stretch(&Stretch {
start_x: x,
Expand All @@ -739,11 +739,11 @@ fn show_data_over_time(
let margin = 5.0;
let visible_time_range = TimeRange {
min: time_ranges_ui
.time_from_x(time_area_painter.clip_rect().left() - margin)
.time_from_x_f32(time_area_painter.clip_rect().left() - margin)
.map_or(TimeInt::MIN, |tf| tf.floor()),

max: time_ranges_ui
.time_from_x(time_area_painter.clip_rect().right() + margin)
.time_from_x_f32(time_area_painter.clip_rect().right() + margin)
.map_or(TimeInt::MAX, |tf| tf.ceil()),
};

Expand Down Expand Up @@ -775,7 +775,7 @@ fn show_data_over_time(
}

if stretch.is_none() {
if let Some(x) = time_ranges_ui.x_from_time(time_real) {
if let Some(x) = time_ranges_ui.x_from_time_f32(time_real) {
stretch = Some(Stretch {
start_x: x,
start_time: time,
Expand Down Expand Up @@ -889,7 +889,7 @@ fn initialize_time_ranges_ui(

/// Find a nice view of everything.
fn view_everything(x_range: &RangeInclusive<f32>, timeline_axis: &TimelineAxis) -> TimeView {
let gap_width = time_ranges_ui::gap_width(x_range, &timeline_axis.ranges);
let gap_width = time_ranges_ui::gap_width(x_range, &timeline_axis.ranges) as f32;
let num_gaps = timeline_axis.ranges.len().saturating_sub(1);
let width = *x_range.end() - *x_range.start();
let width_sans_gaps = width - num_gaps as f32 * gap_width;
Expand Down Expand Up @@ -1011,7 +1011,7 @@ fn paint_time_ranges_gaps(
let zig_zag_first_and_last_edges = true;

if let Some(segment) = time_ranges_ui.segments.first() {
let gap_edge = *segment.x.start();
let gap_edge = *segment.x.start() as f32;

if zig_zag_first_and_last_edges {
// Left side of first segment - paint as a very wide gap that we only see the right side of
Expand All @@ -1027,11 +1027,11 @@ fn paint_time_ranges_gaps(
}

for (a, b) in time_ranges_ui.segments.iter().tuple_windows() {
paint_time_gap(*a.x.end(), *b.x.start());
paint_time_gap(*a.x.end() as f32, *b.x.start() as f32);
}

if let Some(segment) = time_ranges_ui.segments.last() {
let gap_edge = *segment.x.end();
let gap_edge = *segment.x.end() as f32;
if zig_zag_first_and_last_edges {
// Right side of last segment - paint as a very wide gap that we only see the left side of
paint_time_gap(gap_edge, gap_edge + 100_000.0);
Expand Down Expand Up @@ -1136,7 +1136,7 @@ fn time_marker_ui(

// show current time as a line:
if let Some(time) = time_ctrl.time() {
if let Some(x) = time_ranges_ui.x_from_time(time) {
if let Some(x) = time_ranges_ui.x_from_time_f32(time) {
let line_rect =
Rect::from_x_y_ranges(x..=x, timeline_rect.top()..=ui.max_rect().bottom())
.expand(interact_radius);
Expand All @@ -1149,7 +1149,7 @@ fn time_marker_ui(

if response.dragged() {
if let Some(pointer_pos) = pointer_pos {
if let Some(time) = time_ranges_ui.time_from_x(pointer_pos.x) {
if let Some(time) = time_ranges_ui.time_from_x_f32(pointer_pos.x) {
let time = time_ranges_ui.clamp_time(time);
time_ctrl.set_time(time);
time_ctrl.pause();
Expand Down Expand Up @@ -1197,7 +1197,7 @@ fn time_marker_ui(
&& !is_anything_being_dragged
&& !is_hovering_the_loop_selection
{
if let Some(time) = time_ranges_ui.time_from_x(pointer_pos.x) {
if let Some(time) = time_ranges_ui.time_from_x_f32(pointer_pos.x) {
let time = time_ranges_ui.clamp_time(time);
time_ctrl.set_time(time);
time_ctrl.pause();
Expand Down
15 changes: 9 additions & 6 deletions crates/re_viewer/src/ui/time_panel/paint_ticks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,34 @@ pub fn paint_time_ranges_and_ticks(
time_type: TimeType,
) {
let clip_rect = ui.clip_rect();
let clip_left = clip_rect.left() as f64;
let clip_right = clip_rect.right() as f64;

for segment in &time_ranges_ui.segments {
let mut x_range = segment.x.clone();
let mut time_range = segment.time;

// Cull:
if *x_range.end() < clip_rect.left() {
if *x_range.end() < clip_left {
continue;
}
if clip_rect.right() < *x_range.start() {
if clip_right < *x_range.start() {
continue;
}

// Clamp segment to the visible portion to save CPU when zoomed in:
let left_t = egui::emath::inverse_lerp(x_range.clone(), clip_rect.left()).unwrap_or(0.5);
let left_t = egui::emath::inverse_lerp(x_range.clone(), clip_left).unwrap_or(0.5);
if 0.0 < left_t && left_t < 1.0 {
x_range = clip_rect.left()..=*x_range.end();
x_range = clip_left..=*x_range.end();
time_range = TimeRangeF::new(time_range.lerp(left_t), time_range.max);
}
let right_t = egui::emath::inverse_lerp(x_range.clone(), clip_rect.right()).unwrap_or(0.5);
let right_t = egui::emath::inverse_lerp(x_range.clone(), clip_right).unwrap_or(0.5);
if 0.0 < right_t && right_t < 1.0 {
x_range = *x_range.start()..=clip_rect.right();
x_range = *x_range.start()..=clip_right;
time_range = TimeRangeF::new(time_range.min, time_range.lerp(right_t));
}

let x_range = (*x_range.start() as f32)..=(*x_range.end() as f32);
let rect = Rect::from_x_y_ranges(x_range, line_y_range.clone());
time_area_painter
.with_clip_rect(rect)
Expand Down
73 changes: 47 additions & 26 deletions crates/re_viewer/src/ui/time_panel/time_ranges_ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
//! egui uses `f32` precision for all screen-space ui coordinates,
//! which makes sense, because that is plenty of precision for things on screen.
//!
//! In this file we need to use `f64` precision because when zoomed in, the screen-space
//! time-ranges of are way outside the screen, leading to precision issues with `f32`.

use std::ops::RangeInclusive;

use egui::{lerp, remap, NumExt};
Expand All @@ -10,23 +16,23 @@ use crate::{misc::time_control::PlayState, TimeView, ViewerContext};
/// The ideal gap between time segments.
///
/// This is later shrunk via [`GAP_EXPANSION_FRACTION`].
const MAX_GAP: f32 = 40.0;
const MAX_GAP: f64 = 40.0;

/// How much of the gap use up to expand segments visually to either side?
///
/// Should be strictly less than half, or we will get overlapping segments.
const GAP_EXPANSION_FRACTION: f32 = 1.0 / 4.0;
const GAP_EXPANSION_FRACTION: f64 = 1.0 / 4.0;

/// Sze of the gap between time segments.
pub fn gap_width(x_range: &RangeInclusive<f32>, segments: &[TimeRange]) -> f32 {
pub fn gap_width(x_range: &RangeInclusive<f32>, segments: &[TimeRange]) -> f64 {
let num_gaps = segments.len().saturating_sub(1);
if num_gaps == 0 {
// gap width doesn't matter when there are no gaps
MAX_GAP
} else {
// shrink gaps if there are a lot of them
let width = *x_range.end() - *x_range.start();
(width / (num_gaps as f32)).at_most(MAX_GAP)
(width as f64 / (num_gaps as f64)).at_most(MAX_GAP)
}
}

Expand All @@ -35,7 +41,10 @@ pub struct Segment {
/// The range on the x-axis in the ui, in screen coordinates.
///
/// Matches [`Self::time`] (linear transform).
pub x: RangeInclusive<f32>,
///
/// Uses `f64` because the ends of this range can be way outside the screen
/// when we are very zoomed in.
pub x: RangeInclusive<f64>,

/// Matches [`Self::x`] (linear transform).
pub time: TimeRangeF,
Expand All @@ -52,7 +61,7 @@ pub struct Segment {
#[derive(Debug)]
pub struct TimeRangesUi {
/// The total UI x-range we are viewing.
x_range: RangeInclusive<f32>,
x_range: RangeInclusive<f64>,

/// The range of time we are viewing.
time_view: TimeView,
Expand All @@ -66,7 +75,7 @@ pub struct TimeRangesUi {
/// x distance per time unit inside the segments,
/// and before/after the last segment.
/// Between segments time moves faster.
points_per_time: f32,
points_per_time: f64,
}

impl Default for TimeRangesUi {
Expand Down Expand Up @@ -99,8 +108,9 @@ impl TimeRangesUi {
// ^ gap

let gap_width_in_ui = gap_width(&x_range, time_ranges);
let x_range = (*x_range.start() as f64)..=(*x_range.end() as f64);
let width_in_ui = *x_range.end() - *x_range.start();
let points_per_time = width_in_ui / time_view.time_spanned as f32;
let points_per_time = width_in_ui / time_view.time_spanned;
let points_per_time = if points_per_time > 0.0 && points_per_time.is_finite() {
points_per_time
} else {
Expand All @@ -115,23 +125,23 @@ impl TimeRangesUi {
time_ranges
.iter()
.tuple_windows()
.fold(f32::INFINITY, |shortest, (a, b)| {
.fold(f64::INFINITY, |shortest, (a, b)| {
debug_assert!(a.max < b.min, "Overlapping time ranges: {a:?}, {b:?}");
let time_gap = b.min - a.max;
time_gap.as_f32().min(shortest)
time_gap.as_f64().min(shortest)
});

let expansion_in_time = TimeReal::from(
(GAP_EXPANSION_FRACTION * gap_width_in_ui / points_per_time)
.at_most(shortest_time_gap * GAP_EXPANSION_FRACTION),
);
let expansion_in_ui = points_per_time * expansion_in_time.as_f32();
let expansion_in_ui = points_per_time * expansion_in_time.as_f64();

let mut left = 0.0; // we will translate things left/right later to align x_range with time_view
let segments = time_ranges
.iter()
.map(|&tight_time_range| {
let range_width = tight_time_range.length().as_f32() * points_per_time;
let range_width = tight_time_range.length().as_f64() * points_per_time;
let right = left + range_width;
let x_range = left..=right;
left = right + gap_width_in_ui;
Expand Down Expand Up @@ -262,14 +272,18 @@ impl TimeRangesUi {
}
}

pub fn x_from_time(&self, needle_time: TimeReal) -> Option<f32> {
pub fn x_from_time_f32(&self, needle_time: TimeReal) -> Option<f32> {
self.x_from_time(needle_time).map(|x| x as f32)
}

pub fn x_from_time(&self, needle_time: TimeReal) -> Option<f64> {
let first_segment = self.segments.first()?;
let mut last_x = *first_segment.x.start();
let mut last_time = first_segment.time.min;

if needle_time < last_time {
// extrapolate:
return Some(last_x - self.points_per_time * (last_time - needle_time).as_f32());
return Some(last_x - self.points_per_time * (last_time - needle_time).as_f64());
}

for segment in &self.segments {
Expand All @@ -286,10 +300,14 @@ impl TimeRangesUi {
}

// extrapolate:
Some(last_x + self.points_per_time * (needle_time - last_time).as_f32())
Some(last_x + self.points_per_time * (needle_time - last_time).as_f64())
}

pub fn time_from_x(&self, needle_x: f32) -> Option<TimeReal> {
pub fn time_from_x_f32(&self, needle_x: f32) -> Option<TimeReal> {
self.time_from_x_f64(needle_x as f64)
}

pub fn time_from_x_f64(&self, needle_x: f64) -> Option<TimeReal> {
let first_segment = self.segments.first()?;
let mut last_x = *first_segment.x.start();
let mut last_time = first_segment.time.min;
Expand Down Expand Up @@ -319,13 +337,16 @@ impl TimeRangesUi {
/// Pan the view, returning the new view.
pub fn pan(&self, delta_x: f32) -> Option<TimeView> {
Some(TimeView {
min: self.time_from_x(*self.x_range.start() + delta_x)?,
min: self.time_from_x_f64(*self.x_range.start() + delta_x as f64)?,
time_spanned: self.time_view.time_spanned,
})
}

/// Zoom the view around the given x, returning the new view.
pub fn zoom_at(&self, x: f32, zoom_factor: f32) -> Option<TimeView> {
let x = x as f64;
let zoom_factor = zoom_factor as f64;

let mut min_x = *self.x_range.start();
let max_x = *self.x_range.end();
let t = remap(x, min_x..=max_x, 0.0..=1.0);
Expand All @@ -338,16 +359,16 @@ impl TimeRangesUi {
min_x -= t * width_delta;

Some(TimeView {
min: self.time_from_x(min_x)?,
time_spanned: self.time_view.time_spanned / zoom_factor as f64,
min: self.time_from_x_f64(min_x)?,
time_spanned: self.time_view.time_spanned / zoom_factor,
})
}

/// How many egui points for each time unit?
pub fn points_per_time(&self) -> Option<f32> {
pub fn points_per_time(&self) -> Option<f64> {
for segment in &self.segments {
let dx = *segment.x.end() - *segment.x.start();
let dt = segment.time.length().as_f32();
let dt = segment.time.length().as_f64();
if dx > 0.0 && dt > 0.0 {
return Some(dx / dt);
}
Expand Down Expand Up @@ -378,11 +399,11 @@ fn test_time_ranges_ui() {
// Sanity check round-tripping:
for segment in &time_range_ui.segments {
assert_eq!(
time_range_ui.time_from_x(*segment.x.start()).unwrap(),
time_range_ui.time_from_x_f64(*segment.x.start()).unwrap(),
segment.time.min
);
assert_eq!(
time_range_ui.time_from_x(*segment.x.end()).unwrap(),
time_range_ui.time_from_x_f64(*segment.x.end()).unwrap(),
segment.time.max
);

Expand Down Expand Up @@ -417,8 +438,8 @@ fn test_time_ranges_ui_2() {
let pixel_precision = 0.5;

for x_in in 0..=500 {
let x_in = x_in as f32;
let time = time_range_ui.time_from_x(x_in).unwrap();
let x_in = x_in as f64;
let time = time_range_ui.time_from_x_f64(x_in).unwrap();
let x_out = time_range_ui.x_from_time(time).unwrap();

assert!(
Expand All @@ -430,7 +451,7 @@ fn test_time_ranges_ui_2() {
for time_in in 0..=50 {
let time_in = TimeReal::from(time_in as f64);
let x = time_range_ui.x_from_time(time_in).unwrap();
let time_out = time_range_ui.time_from_x(x).unwrap();
let time_out = time_range_ui.time_from_x_f64(x).unwrap();

assert!(
(time_in - time_out).abs().as_f64() < 0.1,
Expand Down
Loading