Skip to content

Commit

Permalink
Fix silently interpreting zero time range as latest-at query (#6172)
Browse files Browse the repository at this point in the history
### What

* Fixes #6135

plus (and that's the bigger part of this PR) refactor time range
handling to remove old `VisibleHistory` rust types.
Commit by commit review recommended.

Still need to fix the ui to better communicate when latest-at is used.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6172?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6172?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6172)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
Wumpf committed Apr 30, 2024
1 parent aa60aab commit e39a8cf
Show file tree
Hide file tree
Showing 66 changed files with 2,855 additions and 3,480 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions crates/re_entity_db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ pub(crate) use self::entity_tree::{ClearCascade, CompactedStoreEvents};
use re_log_types::DataTableError;
pub use re_log_types::{EntityPath, EntityPathPart, TimeInt, Timeline};

pub use re_query::{ExtraQueryHistory, VisibleHistory, VisibleHistoryBoundary};

#[cfg(feature = "serde")]
pub use blueprint::components::EntityPropertiesComponent;
#[cfg(feature = "serde")]
Expand Down
7 changes: 6 additions & 1 deletion crates/re_log_types/src/time_point/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,12 @@ impl TimeType {
}

#[inline]
pub fn format(&self, time_int: TimeInt, time_zone_for_timestamps: TimeZone) -> String {
pub fn format(
&self,
time_int: impl Into<TimeInt>,
time_zone_for_timestamps: TimeZone,
) -> String {
let time_int = time_int.into();
match time_int {
TimeInt::STATIC => "<static>".into(),
// TODO(#5264): remove time panel hack once we migrate to the new static UI
Expand Down
16 changes: 16 additions & 0 deletions crates/re_log_types/src/time_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,22 @@ impl TimeRange {
max: self.max.max(other.max),
}
}

pub fn from_visible_time_range(
range: &re_types_core::datatypes::VisibleTimeRange,
cursor: impl Into<re_types_core::datatypes::TimeInt>,
) -> Self {
let cursor = cursor.into();

let mut min = range.start.start_boundary_time(cursor);
let mut max = range.end.end_boundary_time(cursor);

if min > max {
std::mem::swap(&mut min, &mut max);
}

Self::new(min, max)
}
}

impl re_types_core::SizeBytes for TimeRange {
Expand Down
2 changes: 0 additions & 2 deletions crates/re_query/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ mod flat_vec_deque;
mod latest_at;
mod promise;
mod range;
mod visible_history;

pub mod clamped_zip;
pub mod range_zip;
Expand All @@ -19,7 +18,6 @@ pub use self::latest_at::{LatestAtComponentResults, LatestAtMonoResult, LatestAt
pub use self::promise::{Promise, PromiseId, PromiseResolver, PromiseResult};
pub use self::range::{RangeComponentResults, RangeData, RangeResults};
pub use self::range_zip::*;
pub use self::visible_history::{ExtraQueryHistory, VisibleHistory, VisibleHistoryBoundary};

pub(crate) use self::latest_at::LatestAtCache;
pub(crate) use self::range::{RangeCache, RangeComponentResultsInner};
Expand Down
126 changes: 0 additions & 126 deletions crates/re_query/src/visible_history.rs

This file was deleted.

6 changes: 0 additions & 6 deletions crates/re_space_view/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ mod screenshot;
mod space_view;
mod space_view_contents;
mod sub_archetypes; // TODO(andreas): better name before `sub_archetype` sticks around?
mod visual_time_range;
mod visualizable;

pub use data_query::{DataQuery, EntityOverrideContext, PropertyResolver};
Expand All @@ -21,11 +20,6 @@ pub use sub_archetypes::{
edit_blueprint_component, entity_path_for_view_property, get_blueprint_component,
query_view_property, query_view_property_or_default, view_property,
};
pub use visual_time_range::{
query_visual_history, time_range_from_visible_time_range,
visible_history_boundary_from_time_range_boundary,
visible_history_boundary_to_time_range_boundary,
};
pub use visualizable::determine_visualizable_entities;

pub mod external {
Expand Down
107 changes: 0 additions & 107 deletions crates/re_space_view/src/visual_time_range.rs

This file was deleted.

0 comments on commit e39a8cf

Please sign in to comment.