Skip to content

Commit

Permalink
Visible Range as a component override / fix non-inheriting visual ran…
Browse files Browse the repository at this point in the history 
…ge regression (#5538)

### What

* next step on #5463

Fixes the "Visual Range does not inherit" regression we got when
removing groups. Does this by moving the visual time range out of
`EntityProperties`. I kept all the previous data structures and convert
from and to avoid even deeper changes than already necessary.

This PR explicitly aims at getting 0.14 time range behavior back, the ui
is expected to behave exactly as it did before.



![image](https://github.com/rerun-io/rerun/assets/1220815/1ac1223c-3bc4-44d9-bf08-1ab2cd394fbe)
(much same)


### 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 newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5538/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5538/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5538/index.html?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/5538)
- [Docs
preview](https://rerun.io/preview/477bfed3dd24a6cf61a5867e309ed41d9ea7eab8/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/477bfed3dd24a6cf61a5867e309ed41d9ea7eab8/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
@Wumpf
Wumpf authored Mar 18, 2024
1 parent 300d82a commit 1a79510
Show file tree
Hide file tree
Showing 68 changed files with 2,616 additions and 590 deletions.
8 changes: 1 addition & 7 deletions crates/re_entity_db/src/entity_properties.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ impl FromIterator<(EntityPath, EntityProperties)> for EntityPropertyMap {
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[cfg_attr(feature = "serde", serde(default))]
pub struct EntityProperties {
pub visible_history: re_query::ExtraQueryHistory,
pub interactive: bool,

/// What kind of color mapping should be applied (none, map, texture, transfer..)?
Expand Down Expand Up @@ -143,7 +142,6 @@ pub struct EntityProperties {
impl Default for EntityProperties {
fn default() -> Self {
Self {
visible_history: re_query::ExtraQueryHistory::default(),
interactive: true,
color_mapper: EditableAutoValue::default(),
pinhole_image_plane_distance: EditableAutoValue::Auto(1.0),
Expand All @@ -164,7 +162,6 @@ impl EntityProperties {
/// Multiply/and these together.
pub fn with_child(&self, child: &Self) -> Self {
Self {
visible_history: self.visible_history.with_child(&child.visible_history),
interactive: self.interactive && child.interactive,

color_mapper: self.color_mapper.or(&child.color_mapper).clone(),
Expand Down Expand Up @@ -208,7 +205,6 @@ impl EntityProperties {
/// loaded from the Blueprint store where the Auto values are not up-to-date.
pub fn merge_with(&self, other: &Self) -> Self {
Self {
visible_history: self.visible_history.with_child(&other.visible_history),
interactive: other.interactive,

color_mapper: other.color_mapper.or(&self.color_mapper).clone(),
Expand Down Expand Up @@ -246,7 +242,6 @@ impl EntityProperties {
/// Determine whether this `EntityProperty` has user-edits relative to another `EntityProperty`
pub fn has_edits(&self, other: &Self) -> bool {
let Self {
visible_history,
interactive,
color_mapper,
pinhole_image_plane_distance,
Expand All @@ -260,8 +255,7 @@ impl EntityProperties {
time_series_aggregator,
} = self;

visible_history != &other.visible_history
|| interactive != &other.interactive
interactive != &other.interactive
|| color_mapper.has_edits(&other.color_mapper)
|| pinhole_image_plane_distance.has_edits(&other.pinhole_image_plane_distance)
|| backproject_depth.has_edits(&other.backproject_depth)
Expand Down
7 changes: 7 additions & 0 deletions crates/re_log_types/src/time_point/time_int.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ impl From<TimeInt> for Duration {
}
}

impl From<TimeInt> for re_types_core::datatypes::TimeInt {
#[inline]
fn from(int: TimeInt) -> Self {
Self(int.as_i64())
}
}

impl std::ops::Neg for TimeInt {
type Output = Self;

Expand Down
17 changes: 0 additions & 17 deletions crates/re_query/src/util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use crate::{query_archetype, range::range_archetype, ArchetypeView};
/// For [`VisibleHistoryBoundary::RelativeToTimeCursor`] and [`VisibleHistoryBoundary::Absolute`],
/// the value 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
RelativeToTimeCursor(i64),
Expand All @@ -36,7 +35,6 @@ impl Default for VisibleHistoryBoundary {

/// 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,
Expand Down Expand Up @@ -99,8 +97,6 @@ impl VisibleHistory {

/// 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 {
/// Is the feature enabled?
pub enabled: bool,
Expand All @@ -112,19 +108,6 @@ pub struct ExtraQueryHistory {
pub sequences: VisibleHistory,
}

impl ExtraQueryHistory {
/// Multiply/and these together.
pub fn with_child(&self, child: &Self) -> Self {
if child.enabled {
*child
} else if self.enabled {
*self
} else {
Self::default()
}
}
}

// ---

pub fn query_archetype_with_history<'a, A: Archetype + 'a, const N: usize>(
Expand Down
5 changes: 5 additions & 0 deletions crates/re_space_view/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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 @@ -20,6 +21,10 @@ pub use sub_archetypes::{
entity_path_for_space_view_sub_archetype, query_space_view_sub_archetype,
query_space_view_sub_archetype_or_default,
};
pub use visual_time_range::{
default_time_range, query_visual_history, time_range_boundary_to_visible_history_boundary,
visible_history_boundary_to_time_range_boundary, visible_time_range_to_time_range,
};
pub use visualizable::determine_visualizable_entities;

// -----------
Expand Down
178 changes: 42 additions & 136 deletions crates/re_space_view/src/space_view.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use itertools::{FoldWhile, Itertools};
use re_data_store::LatestAtQuery;
use re_entity_db::{EntityDb, EntityPath, EntityProperties, VisibleHistory};
use re_entity_db::{EntityPropertiesComponent, EntityPropertyMap};
use nohash_hasher::IntMap;

use crate::SpaceViewContents;
use re_data_store::LatestAtQuery;
use re_entity_db::{EntityDb, EntityPath, EntityPropertiesComponent, EntityPropertyMap};
use re_log_types::{DataRow, EntityPathSubs, RowId};
use re_query::query_archetype;
use re_types::blueprint::archetypes as blueprint_archetypes;
Expand All @@ -14,9 +14,9 @@ use re_types::{
use re_types_core::archetypes::Clear;
use re_types_core::Archetype as _;
use re_viewer_context::{
DataResult, PerSystemEntities, PropertyOverrides, RecommendedSpaceView, SpaceViewClass,
SpaceViewClassIdentifier, SpaceViewId, SpaceViewState, StoreContext, SystemCommand,
SystemCommandSender as _, SystemExecutionOutput, ViewQuery, ViewerContext,
DataResult, OverridePath, PerSystemEntities, PropertyOverrides, RecommendedSpaceView,
SpaceViewClass, SpaceViewClassIdentifier, SpaceViewId, SpaceViewState, StoreContext,
SystemCommand, SystemCommandSender as _, SystemExecutionOutput, ViewQuery, ViewerContext,
};

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -416,46 +416,56 @@ impl SpaceViewBlueprint {
}

pub fn root_data_result(&self, ctx: &StoreContext<'_>, query: &LatestAtQuery) -> DataResult {
let entity_path = self.entity_path();

let is_time_series = self.class_identifier == "Time Series";
let base_override_root = self.entity_path();
let individual_override_path =
base_override_root.join(&DataResult::INDIVIDUAL_OVERRIDES_PREFIX.into());
let recursive_override_path =
base_override_root.join(&DataResult::RECURSIVE_OVERRIDES_PREFIX.into());

let mut individual_properties = ctx
let individual_properties = ctx
.blueprint
.store()
.query_latest_component_quiet::<EntityPropertiesComponent>(&self.entity_path(), query)
.query_latest_component_quiet::<EntityPropertiesComponent>(
&individual_override_path,
query,
)
.map(|result| result.value.0);
let accumulated_properties = individual_properties.clone().unwrap_or_default();

// TODO(#4194): this should come from delegation to the space-view-class
if individual_properties.is_none() && is_time_series {
let mut time_series_defaults = EntityProperties::default();
time_series_defaults.visible_history.nanos = VisibleHistory::ALL;
time_series_defaults.visible_history.sequences = VisibleHistory::ALL;
individual_properties = Some(time_series_defaults);
}

let mut accumulated_properties = individual_properties.clone().unwrap_or_default();

if is_time_series {
// TODO(#4194): enabled == false means use defaults
if !accumulated_properties.visible_history.enabled {
accumulated_properties.visible_history.enabled = true;
accumulated_properties.visible_history.nanos = VisibleHistory::ALL;
accumulated_properties.visible_history.sequences = VisibleHistory::ALL;
// Gather recursive component overrides.
// Ignore individual overrides on SpaceView root.
let mut recursive_component_overrides = IntMap::default();
if let Some(recursive_override_subtree) =
ctx.blueprint.tree().subtree(&recursive_override_path)
{
for component in recursive_override_subtree.entity.components.keys() {
if let Some(component_data) = ctx
.blueprint
.store()
.latest_at(query, &recursive_override_path, *component, &[*component])
.and_then(|(_, _, cells)| cells[0].clone())
{
if !component_data.is_empty() {
recursive_component_overrides.insert(
*component,
OverridePath::blueprint_path(recursive_override_path.clone()),
);
}
}
}
}

DataResult {
entity_path: entity_path.clone(),
entity_path: base_override_root,
visualizers: Default::default(),
tree_prefix_only: false,
property_overrides: Some(PropertyOverrides {
accumulated_properties,
individual_properties,
recursive_properties: Default::default(),
resolved_component_overrides: Default::default(),
recursive_override_path: entity_path.clone(),
individual_override_path: entity_path,
resolved_component_overrides: recursive_component_overrides,
recursive_override_path,
individual_override_path,
}),
}
}
Expand All @@ -464,7 +474,7 @@ impl SpaceViewBlueprint {
#[cfg(test)]
mod tests {
use crate::data_query::{DataQuery, PropertyResolver};
use re_entity_db::EntityDb;
use re_entity_db::{EntityDb, EntityProperties, EntityPropertiesComponent};
use re_log_types::{
example_components::{MyColor, MyLabel, MyPoint},
DataCell, DataRow, RowId, StoreId, StoreKind, TimePoint,
Expand Down Expand Up @@ -673,110 +683,6 @@ mod tests {
assert!(!result.accumulated_properties().interactive);
}
}

// Override interactive range on root
{
let root = space_view.root_data_result(
&StoreContext {
app_id: re_log_types::ApplicationId::unknown(),
blueprint: &blueprint,
recording: Some(&recording),
all_recordings: vec![],
},
&blueprint_query,
);
let mut overrides = root.individual_properties().cloned().unwrap_or_default();
overrides.visible_history.enabled = true;
overrides.visible_history.nanos = VisibleHistory::ALL;

save_override(
overrides,
root.recursive_override_path().unwrap(),
&mut blueprint,
);
}

// Everyone has interactive history
{
let ctx = StoreContext {
app_id: re_log_types::ApplicationId::unknown(),
blueprint: &blueprint,
recording: Some(&recording),
all_recordings: vec![],
};
let mut query_result = contents.execute_query(&ctx, &visualizable_entities);
resolver.update_overrides(&ctx, &blueprint_query, &mut query_result);

let parent = query_result
.tree
.lookup_result_by_path(&EntityPath::from("parent"))
.unwrap();
let child1 = query_result
.tree
.lookup_result_by_path(&EntityPath::from("parent/skip/child1"))
.unwrap();
let child2 = query_result
.tree
.lookup_result_by_path(&EntityPath::from("parent/skip/child2"))
.unwrap();

for result in [parent, child1, child2] {
assert!(result.accumulated_properties().visible_history.enabled);
assert_eq!(
result.accumulated_properties().visible_history.nanos,
VisibleHistory::ALL
);
}

let mut overrides = child2.individual_properties().cloned().unwrap_or_default();
overrides.visible_history.enabled = true;

save_override(
overrides,
child2.individual_override_path().unwrap(),
&mut blueprint,
);
}

// Child2 has its own interactive history
{
let ctx = StoreContext {
app_id: re_log_types::ApplicationId::unknown(),
blueprint: &blueprint,
recording: Some(&recording),
all_recordings: vec![],
};

let mut query_result = contents.execute_query(&ctx, &visualizable_entities);
resolver.update_overrides(&ctx, &blueprint_query, &mut query_result);

let parent = query_result
.tree
.lookup_result_by_path(&EntityPath::from("parent"))
.unwrap();
let child1 = query_result
.tree
.lookup_result_by_path(&EntityPath::from("parent/skip/child1"))
.unwrap();
let child2 = query_result
.tree
.lookup_result_by_path(&EntityPath::from("parent/skip/child2"))
.unwrap();

for result in [parent, child1] {
assert!(result.accumulated_properties().visible_history.enabled);
assert_eq!(
result.accumulated_properties().visible_history.nanos,
VisibleHistory::ALL
);
}

assert!(child2.accumulated_properties().visible_history.enabled);
assert_eq!(
child2.accumulated_properties().visible_history.nanos,
VisibleHistory::OFF
);
}
}

#[test]
Expand Down
Loading

0 comments on commit 1a79510

Please sign in to comment.