Skip to content

Commit

Permalink
review
Browse files Browse the repository at this point in the history
  • Loading branch information
teh-cmc committed Mar 18, 2024
1 parent f612014 commit f338850
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 22 deletions.
10 changes: 2 additions & 8 deletions crates/re_log_types/src/data_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,7 @@ impl DataTable {
col_timelines
.iter()
.filter_map(|(timeline, times)| {
times[i]
// Soundness: it should never happen that we have timestamps with an `i64::MIN`
// value in the data since users cannot create those.
.and_then(NonMinI64::new)
.map(|time| (*timeline, time.into()))
times[i].map(|time| (*timeline, crate::TimeInt::new_temporal(time)))
})
.collect::<BTreeMap<_, _>>(),
),
Expand All @@ -530,9 +526,7 @@ impl DataTable {
.flatten()
.max()
.copied()
// Soundness: it should never happen that we have timestamps with an `i64::MIN`
// value in the data since users cannot create those.
.and_then(NonMinI64::new);
.map(crate::TimeInt::new_temporal);

if let Some(time) = time {
timepoint.insert(*timeline, time);
Expand Down
6 changes: 2 additions & 4 deletions crates/re_log_types/src/time_point/time_int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,13 @@ impl TimeInt {
/// For time timelines.
#[inline]
pub fn from_milliseconds(millis: NonMinI64) -> Self {
// Soundness: we cannot hit the min value with a saturing positive multiplication.
Self(NonMinI64::new(millis.get().saturating_mul(1_000_000)))
Self::new_temporal(millis.get().saturating_mul(1_000_000))
}

/// For time timelines.
#[inline]
pub fn from_seconds(seconds: NonMinI64) -> Self {
// Soundness: we cannot hit the min value with a saturing positive multiplication.
Self(NonMinI64::new(seconds.get().saturating_mul(1_000_000_000)))
Self::new_temporal(seconds.get().saturating_mul(1_000_000_000))
}

/// For sequence timelines.
Expand Down
3 changes: 1 addition & 2 deletions crates/re_log_types/src/time_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ impl TimeRange {

#[inline]
pub fn center(&self) -> TimeInt {
let center =
NonMinI64::new((self.abs_length() / 2) as i64).unwrap_or(NonMinI64::new(0).unwrap());
let center = NonMinI64::new((self.abs_length() / 2) as i64).unwrap_or(NonMinI64::MIN);
self.min + TimeInt::from(center)
}

Expand Down
6 changes: 3 additions & 3 deletions crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1947,7 +1947,7 @@ impl RecordingStream {
time
} else {
re_log::error!(
illegal_value = time.nanos_since_epoch(),
illegal_value = seconds,
new_value = TimeInt::MIN.as_i64(),
"set_time_seconds() called with illegal value - clamped to minimum legal value"
);
Expand Down Expand Up @@ -1990,9 +1990,9 @@ impl RecordingStream {
time
} else {
re_log::error!(
illegal_value = time.nanos_since_epoch(),
illegal_value = ns,
new_value = TimeInt::MIN.as_i64(),
"set_time_seconds() called with illegal value - clamped to minimum legal value"
"set_time_nanos() called with illegal value - clamped to minimum legal value"
);
TimeInt::MIN
};
Expand Down
2 changes: 1 addition & 1 deletion crates/re_time_panel/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ fn help_button(ui: &mut egui::Ui) {

/// A user can drag the time slider to between the timeless data and the first real data.
///
/// The time interpolated there is really weird, as it goes from [`TimeInt::MIN`]
/// The time interpolated there is really weird, as it goes from [`TimeInt::MIN_TIME_PANEL`]
/// (which is extremely long time ago) to whatever tim the user logged.
/// So we do not want to display these times to the user.
///
Expand Down
5 changes: 3 additions & 2 deletions crates/re_time_panel/src/time_ranges_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,14 @@ impl TimeRangesUi {
TimeReal::from(last.tight_time.max()),
);

// Special: don't allow users dragging time between STATIC (-∞ = timeless data) and some real time.
// Special: don't allow users dragging time between MIN_TIME_PANEL (-∞ = timeless data)
// and some real time.
//
// Otherwise we get weird times (e.g. dates in 1923).
// Selecting times between other segments is not as problematic, as all other segments are
// real times, so interpolating between them always produces valid times
// (we want users to have a smooth experience dragging the time handle anywhere else).
// By disallowing times between STATIC and the first real segment,
// By disallowing times between MIN_TIME_PANEL and the first real segment,
// we also disallow users dragging the time to be between -∞ and the
// real beginning of their data. That further highlights the specialness of -∞.
//
Expand Down
5 changes: 3 additions & 2 deletions crates/re_viewer_context/src/blueprint_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ impl StoreContext<'_> {
.times_per_timeline()
.get(&timeline)
.and_then(|times| times.last_key_value())
.map_or(0, |(time, _)| time.as_i64());
.map_or(0, |(time, _)| time.as_i64())
.saturating_add(1);

TimePoint::from([(timeline, TimeInt::new_temporal(max_time + 1))])
TimePoint::from([(timeline, TimeInt::new_temporal(max_time))])
}
}

Expand Down

0 comments on commit f338850

Please sign in to comment.