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.

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

Improve Visible History to support more general time queries #4123

Merged
merged 15 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 74 additions & 7 deletions crates/re_data_store/src/entity_properties.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#[cfg(feature = "serde")]
use re_log_types::EntityPath;
use re_log_types::TimeInt;

#[cfg(feature = "serde")]
use crate::EditableAutoValue;
Expand Down Expand Up @@ -172,25 +173,91 @@ impl EntityProperties {

// ----------------------------------------------------------------------------

/// One of the boundary of the visible history.
abey79 marked this conversation as resolved.
Show resolved Hide resolved
///
/// The for [`VisibleHistoryBoundary::Relative`] and [`VisibleHistoryBoundary::Absolute`], the value
abey79 marked this conversation as resolved.
Show resolved Hide resolved
/// are either nanos or frames, depending on the type of timeline.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub enum VisibleHistoryBoundary {
/// Boundary is a value relative to the time cursor
Relative(i64),
abey79 marked this conversation as resolved.
Show resolved Hide resolved

/// Boundary is an absolute value
Absolute(i64),

/// The boundary extends to infinity.
Infinite,
}

impl VisibleHistoryBoundary {
pub const AT_CURSOR: Self = Self::Relative(0);
}

impl Default for VisibleHistoryBoundary {
fn default() -> Self {
Self::AT_CURSOR
}
}

/// Visible history bounds.
#[derive(Clone, Copy, Default, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub struct VisibleHistory {
/// Low time boundary.
pub from: VisibleHistoryBoundary,

/// High time boundary.
pub to: VisibleHistoryBoundary,
}

impl VisibleHistory {
/// Value with the visible history feature is disabled.
pub const OFF: Self = Self {
from: VisibleHistoryBoundary::AT_CURSOR,
to: VisibleHistoryBoundary::AT_CURSOR,
};

pub fn from(&self, cursor: TimeInt) -> TimeInt {
match self.from {
VisibleHistoryBoundary::Absolute(value) => TimeInt::from(value),
VisibleHistoryBoundary::Relative(value) => cursor + TimeInt::from(value),
VisibleHistoryBoundary::Infinite => TimeInt::MIN,
}
}

pub fn to(&self, cursor: TimeInt) -> TimeInt {
match self.to {
VisibleHistoryBoundary::Absolute(value) => TimeInt::from(value),
VisibleHistoryBoundary::Relative(value) => cursor + TimeInt::from(value),
VisibleHistoryBoundary::Infinite => TimeInt::MAX,
}
}
}

/// When showing an entity in the history view, add this much history to it.
#[derive(Clone, Copy, Default, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[cfg_attr(feature = "serde", serde(default))]
pub struct ExtraQueryHistory {
/// Zero = off.
pub nanos: i64,
/// Is the feature enabled?
pub enabled: bool,

/// Zero = off.
pub sequences: i64,
/// Visible history settings for time timelines
pub nanos: VisibleHistory,

/// Visible history settings for frame timelines
pub sequences: VisibleHistory,
}

impl ExtraQueryHistory {
/// Multiply/and these together.
#[allow(dead_code)]
fn with_child(&self, child: &Self) -> Self {
Self {
nanos: self.nanos.max(child.nanos),
sequences: self.sequences.max(child.sequences),
if child.enabled {
*child
} else {
*self
}
}
} // ----------------------------------------------------------------------------
Expand Down
4 changes: 4 additions & 0 deletions crates/re_data_store/src/store_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,10 @@ impl StoreDb {
&self.entity_db.times_per_timeline
}

pub fn time_histogram(&self, timeline: &Timeline) -> Option<&crate::TimeHistogram> {
self.entity_db().tree.prefix_times.get(timeline)
}

pub fn num_timeless_messages(&self) -> usize {
self.entity_db.tree.num_timeless_messages()
}
Expand Down
9 changes: 5 additions & 4 deletions crates/re_query/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use re_arrow_store::{DataStore, LatestAtQuery, RangeQuery, TimeInt, TimeRange, Timeline};
use re_data_store::ExtraQueryHistory;
use re_data_store::{ExtraQueryHistory, VisibleHistory};
use re_log_types::EntityPath;
use re_types_core::Archetype;

Expand All @@ -17,14 +17,15 @@ pub fn query_archetype_with_history<'a, A: Archetype + 'a, const N: usize>(
re_log_types::TimeType::Sequence => history.sequences,
};

if visible_history == 0 {
if !history.enabled || visible_history == VisibleHistory::OFF {
Copy link
Member

Choose a reason for hiding this comment

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

to clarify, when can it happen that one is off but the other is enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How the visible history is stored is not fully normalised: there is a slight duplication:

  • the enabled/disabled bool
  • the fact that {from: Relative(0), to: Relative(0)} (that's what VisibleHistory::OFF is) disable the feature for all practical purposes.

I've introduced that "enabled" bool to allow the user to disable the feature without losing their configured boundaries (ie. that checkbox next to the Visible History label). Overall, better UX I believe, but the cost is this check.

BTW, the test against VisibleHistory::OFF could actually be dropped and the result would be the same. So it can be seen as an optimisation to avoid a range query when we know none is needed :)

let latest_query = LatestAtQuery::new(*timeline, *time);
let latest = query_archetype::<A>(store, &latest_query, ent_path)?;

Ok(itertools::Either::Left(std::iter::once(latest)))
} else {
let min_time = *time - TimeInt::from(visible_history);
let range_query = RangeQuery::new(*timeline, TimeRange::new(min_time, *time));
let min_time = visible_history.from(*time);
let max_time = visible_history.to(*time);
let range_query = RangeQuery::new(*timeline, TimeRange::new(min_time, max_time));

let range = range_archetype::<A, N>(store, &range_query, ent_path);

Expand Down
5 changes: 1 addition & 4 deletions crates/re_time_panel/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,10 +822,7 @@ fn initialize_time_ranges_ui(

if let Some(times) = ctx
.store_db
.entity_db()
.tree
.prefix_times
.get(ctx.rec_cfg.time_ctrl.timeline())
.time_histogram(ctx.rec_cfg.time_ctrl.timeline())
{
// NOTE: `times` can be empty if a GC wiped everything.
if !times.is_empty() {
Expand Down
36 changes: 36 additions & 0 deletions crates/re_ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub use command_palette::CommandPalette;
pub use design_tokens::DesignTokens;
pub use icons::Icon;
pub use layout_job_builder::LayoutJobBuilder;
use std::ops::RangeInclusive;
pub use toggle_switch::toggle_switch;

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -445,6 +446,41 @@ impl ReUi {
.inner
}

/// Shows a `egui::DragValue` for a time in nanoseconds.
///
/// The value and unit displayed adapts to the provided allowable time range.
#[allow(clippy::unused_self)]
pub fn time_drag_value(
&self,
ui: &mut egui::Ui,
value: &mut i64,
time_range: &RangeInclusive<i64>,
) {
let span = time_range.end() - time_range.start();

let (unit, factor) = if span / 1_000_000_000 > 0 {
("s", 1_000_000_000.)
} else if span / 1_000_000 > 0 {
("ms", 1_000_000.)
} else if span / 1_000 > 0 {
("μs", 1_000.)
} else {
("ns", 1.)
};
Wumpf marked this conversation as resolved.
Show resolved Hide resolved

let mut time_unit = *value as f32 / factor;
let time_range = *time_range.start() as f32 / factor..=*time_range.end() as f32 / factor;
let speed = (time_range.end() - time_range.start()) * 0.005;

ui.add(
egui::DragValue::new(&mut time_unit)
.clamp_range(time_range)
.speed(speed)
.suffix(unit),
);
*value = (time_unit * factor).round() as _;
}

pub fn large_button(&self, ui: &mut egui::Ui, icon: &Icon) -> egui::Response {
self.large_button_impl(ui, icon, None, None)
}
Expand Down