Skip to content

Commit

Permalink
Add a more solid fix for bad ranges
Browse files Browse the repository at this point in the history
  • Loading branch information
emilk committed Jun 30, 2024
1 parent dd733cf commit 7874c43
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 74 deletions.
49 changes: 25 additions & 24 deletions crates/re_space_view_time_series/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use re_viewport_blueprint::ViewProperty;

use crate::line_visualizer_system::SeriesLineSystem;
use crate::point_visualizer_system::SeriesPointSystem;
use crate::util::next_up_f64;
use crate::PlotSeriesKind;

// ---
Expand Down Expand Up @@ -277,13 +276,8 @@ impl SpaceViewClass for TimeSeriesSpaceView {

let scalar_axis =
ViewProperty::from_archetype::<ScalarAxis>(blueprint_db, ctx.blueprint_query, view_id);
let mut y_range = scalar_axis.component_or_fallback::<Range1D>(ctx, self, state)?;
if y_range.start() >= y_range.end() {
// Ensure that the range is valid - egui_plot might debug_assert otherwise.
// `next_up_f64` should be sufficient, but empirically it's not always enough
// (likely we're loosing precision due to some calculations down the line)
*y_range.end_mut() = y_range.start() + 1.0;
}
let y_range = scalar_axis.component_or_fallback::<Range1D>(ctx, self, state)?;
let y_range = make_range_sane(y_range);

let y_zoom_lock =
scalar_axis.component_or_fallback::<LockRangeDuringZoom>(ctx, self, state)?;
Expand Down Expand Up @@ -645,24 +639,31 @@ impl TypedComponentFallbackProvider<Range1D> for TimeSeriesSpaceView {
ctx.view_state
.as_any()
.downcast_ref::<TimeSeriesSpaceViewState>()
.map(|s| {
let mut range = s.scalar_range;
.map(|s| make_range_sane(s.scalar_range))
.unwrap_or_default()
}
}

// egui_plot can't handle a zero or negative range.
// Enforce a minimum range.
if !range.start().is_normal() {
*range.start_mut() = -1.0;
}
if !range.end().is_normal() {
*range.end_mut() = 1.0;
}
if range.start() >= range.end() {
*range.start_mut() = next_up_f64(range.end());
}
/// Make sure the range is finite and positive, or `egui_plot` might be buggy.
fn make_range_sane(y_range: Range1D) -> Range1D {
let (mut start, mut end) = (y_range.start(), y_range.end());

range
})
.unwrap_or_default()
if !start.is_normal() {
start = -1.0;
}
if !end.is_normal() {
end = 1.0;
}

if end < start {
(start, end) = (end, start);
}

if end <= start {
let center = (start + end) / 2.0;
Range1D::new(center - 1.0, center + 1.0)
} else {
Range1D::new(start, end)
}
}

Expand Down
50 changes: 0 additions & 50 deletions crates/re_space_view_time_series/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,53 +269,3 @@ fn add_series_runs(
all_series.push(series);
}
}

/// Returns the least number greater than `self`.
///
/// Unstable feature in Rust. This is a copy of the implementation from the standard library.
///
/// Let `TINY` be the smallest representable positive `f64`. Then,
/// - if `self.is_nan()`, this returns `self`;
/// - if `self` is [`NEG_INFINITY`], this returns [`MIN`];
/// - if `self` is `-TINY`, this returns -0.0;
/// - if `self` is -0.0 or +0.0, this returns `TINY`;
/// - if `self` is [`MAX`] or [`INFINITY`], this returns [`INFINITY`];
/// - otherwise the unique least value greater than `self` is returned.
///
/// The identity `x.next_up() == -(-x).next_down()` holds for all non-NaN `x`. When `x`
/// is finite `x == x.next_up().next_down()` also holds.
///
/// ```ignore
/// #![feature(float_next_up_down)]
/// // f64::EPSILON is the difference between 1.0 and the next number up.
/// assert_eq!(1.0f64.next_up(), 1.0 + f64::EPSILON);
/// // But not for most numbers.
/// assert!(0.1f64.next_up() < 0.1 + f64::EPSILON);
/// assert_eq!(9007199254740992f64.next_up(), 9007199254740994.0);
/// ```
///
/// [`NEG_INFINITY`]: f64::NEG_INFINITY
/// [`INFINITY`]: f64::INFINITY
/// [`MIN`]: f64::MIN
/// [`MAX`]: f64::MAX
pub fn next_up_f64(this: f64) -> f64 {
// We must use strictly integer arithmetic to prevent denormals from
// flushing to zero after an arithmetic operation on some platforms.
const TINY_BITS: u64 = 0x1; // Smallest positive f64.
const CLEAR_SIGN_MASK: u64 = 0x7fff_ffff_ffff_ffff;

let bits = this.to_bits();
if this.is_nan() || bits == f64::INFINITY.to_bits() {
return this;
}

let abs = bits & CLEAR_SIGN_MASK;
let next_bits = if abs == 0 {
TINY_BITS
} else if bits == abs {
bits + 1
} else {
bits - 1
};
f64::from_bits(next_bits)
}

0 comments on commit 7874c43

Please sign in to comment.