Skip to content

Commit

Permalink
Fix precision issue when zooming in on the timeline (#1370)
Browse files Browse the repository at this point in the history
Previously the view would become shaky and difficult to navigate
as you zoomed closer and closer.

The fix: switch from f32 to f64.
  • Loading branch information
emilk authored Feb 22, 2023
1 parent 59639c8 commit 93b1ee3
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 60 deletions.
8 changes: 4 additions & 4 deletions crates/re_log_types/src/time_range.rs
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
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
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
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

1 comment on commit 93b1ee3

@github-actions
Copy link

Choose a reason for hiding this comment

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

Rust Benchmark

Benchmark suite Current: 93b1ee3 Previous: 59639c8 Ratio
datastore/insert/batch/rects/insert 571051 ns/iter (± 1974) 549723 ns/iter (± 9307) 1.04
datastore/latest_at/batch/rects/query 1789 ns/iter (± 2) 1831 ns/iter (± 22) 0.98
datastore/latest_at/missing_components/primary 357 ns/iter (± 0) 354 ns/iter (± 3) 1.01
datastore/latest_at/missing_components/secondaries 431 ns/iter (± 0) 416 ns/iter (± 5) 1.04
datastore/range/batch/rects/query 150469 ns/iter (± 162) 148522 ns/iter (± 1878) 1.01
mono_points_arrow/generate_message_bundles 48920832 ns/iter (± 378213) 44067145 ns/iter (± 1478285) 1.11
mono_points_arrow/generate_messages 128886985 ns/iter (± 1047718) 124872076 ns/iter (± 1427855) 1.03
mono_points_arrow/encode_log_msg 162816900 ns/iter (± 1371769) 151616959 ns/iter (± 1283262) 1.07
mono_points_arrow/encode_total 340220798 ns/iter (± 1830224) 322586111 ns/iter (± 2939551) 1.05
mono_points_arrow/decode_log_msg 181012753 ns/iter (± 845781) 173315825 ns/iter (± 1736902) 1.04
mono_points_arrow/decode_message_bundles 66550564 ns/iter (± 1021472) 63511220 ns/iter (± 1013271) 1.05
mono_points_arrow/decode_total 243560774 ns/iter (± 1528176) 235105527 ns/iter (± 2418882) 1.04
batch_points_arrow/generate_message_bundles 337557 ns/iter (± 1186) 331372 ns/iter (± 4436) 1.02
batch_points_arrow/generate_messages 6551 ns/iter (± 10) 6276 ns/iter (± 85) 1.04
batch_points_arrow/encode_log_msg 361841 ns/iter (± 1380) 350428 ns/iter (± 3088) 1.03
batch_points_arrow/encode_total 728488 ns/iter (± 2160) 702377 ns/iter (± 7847) 1.04
batch_points_arrow/decode_log_msg 358487 ns/iter (± 1564) 342795 ns/iter (± 2345) 1.05
batch_points_arrow/decode_message_bundles 2072 ns/iter (± 4) 2053 ns/iter (± 28) 1.01
batch_points_arrow/decode_total 365071 ns/iter (± 2126) 351844 ns/iter (± 2808) 1.04
arrow_mono_points/insert 6425343228 ns/iter (± 26141079) 6202562547 ns/iter (± 125497863) 1.04
arrow_mono_points/query 1838168 ns/iter (± 23501) 1717100 ns/iter (± 18949) 1.07
arrow_batch_points/insert 2967434 ns/iter (± 161622) 2698584 ns/iter (± 28204) 1.10
arrow_batch_points/query 16879 ns/iter (± 53) 16645 ns/iter (± 187) 1.01
tuid/Tuid::random 34 ns/iter (± 0) 34 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.