Delete unused methods from DataResult and make PropertyOverrides …

…non-optional (#6809)

### What

The latter has been a big annoyance for quite a while. No more!

### 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/6809?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/6809?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/6809)
- [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`.

crates/re_selection_panel/src/visible_time_range_ui.rs

@@ -66,15 +66,8 @@ pub fn visible_time_range_ui_for_data_result(
     ui: &mut Ui,
     data_result: &re_viewer_context::DataResult,
 ) {
-    let Some(override_path) = data_result.recursive_override_path() else {
-        re_log::error_once!("No override computed yet for entity");
-        return;
-    };
-    let Some(overrides) = data_result.property_overrides.as_ref() else {
-        re_log::error_once!("No override computed yet for entity");
-        return;
-    };
-    let query_range = overrides.query_range.clone();
+    let override_path = data_result.recursive_override_path();
+    let query_range = data_result.property_overrides.query_range.clone();
 
     let is_space_view = false;
     visible_time_range_ui(ctx, ui, query_range, override_path, is_space_view);

crates/re_selection_panel/src/visualizer_ui.rs

@@ -81,12 +81,7 @@ pub fn visualizer_ui_impl(
     data_result: &DataResult,
     active_visualizers: &[ViewSystemIdentifier],
 ) {
-    let Some(override_path) = data_result.individual_override_path() else {
-        if cfg!(debug_assertions) {
-            re_log::error!("No override path for entity: {}", data_result.entity_path);
-        }
-        return;
-    };
+    let override_path = data_result.individual_override_path();
 
     let remove_visualizer_button = |ui: &mut egui::Ui, vis_name: ViewSystemIdentifier| {
         let response = ui.small_icon_button(&re_ui::icons::CLOSE);
@@ -220,13 +215,7 @@ fn visualizer_components(
             (None, None, None) => (ValueSource::FallbackOrPlaceholder, raw_fallback.as_ref()),
         };
 
-        let Some(override_path) = data_result.individual_override_path() else {
-            // This shouldn't the `DataResult` is valid.
-            if cfg!(debug_assertions) {
-                re_log::error!("No override path for entity: {}", data_result.entity_path);
-            }
-            return;
-        };
+        let override_path = data_result.individual_override_path();
 
         let value_fn = |ui: &mut egui::Ui, _style| {
             // Edit ui can only handle a single value.
@@ -490,14 +479,7 @@ fn menu_add_new_visualizer(
     active_visualizers: &[ViewSystemIdentifier],
     inactive_visualizers: &[ViewSystemIdentifier],
 ) {
-    // If we don't have an override_path we can't set up an initial override
-    // this shouldn't happen if the `DataResult` is valid.
-    let Some(override_path) = data_result.individual_override_path() else {
-        if cfg!(debug_assertions) {
-            re_log::error!("No override path for entity: {}", data_result.entity_path);
-        }
-        return;
-    };
+    let override_path = data_result.individual_override_path();
 
     ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend);
 

crates/re_space_view/src/query.rs

@@ -130,46 +130,45 @@ fn query_overrides<'a>(
     // First see if any components have overrides.
     let mut overrides = LatestAtResults::default();
 
-    if let Some(prop_overrides) = &data_result.property_overrides {
-        // TODO(jleibs): partitioning overrides by path
-        for component_name in component_names {
-            if let Some(override_value) = prop_overrides
-                .resolved_component_overrides
-                .get(component_name)
-            {
-                let component_override_result = match override_value.store_kind {
-                    re_log_types::StoreKind::Recording => {
-                        // TODO(jleibs): This probably is not right, but this code path is not used
-                        // currently. This may want to use range_query instead depending on how
-                        // component override data-references are resolved.
-                        ctx.store_context.blueprint.query_caches().latest_at(
-                            ctx.store_context.blueprint.store(),
-                            &ctx.current_query(),
-                            &override_value.path,
-                            [*component_name],
-                        )
-                    }
-                    re_log_types::StoreKind::Blueprint => {
-                        ctx.store_context.blueprint.query_caches().latest_at(
-                            ctx.store_context.blueprint.store(),
-                            ctx.blueprint_query,
-                            &override_value.path,
-                            [*component_name],
-                        )
-                    }
-                };
-
-                // If we successfully find a non-empty override, add it to our results.
-
-                // TODO(jleibs): it seems like value could still be null/empty if the override
-                // has been cleared. It seems like something is preventing that from happening
-                // but I don't fully understand what.
-                //
-                // This is extra tricky since the promise hasn't been resolved yet so we can't
-                // actually look at the data.
-                if let Some(value) = component_override_result.components.get(component_name) {
-                    overrides.add(*component_name, value.clone());
+    // TODO(jleibs): partitioning overrides by path
+    for component_name in component_names {
+        if let Some(override_value) = data_result
+            .property_overrides
+            .resolved_component_overrides
+            .get(component_name)
+        {
+            let component_override_result = match override_value.store_kind {
+                re_log_types::StoreKind::Recording => {
+                    // TODO(jleibs): This probably is not right, but this code path is not used
+                    // currently. This may want to use range_query instead depending on how
+                    // component override data-references are resolved.
+                    ctx.store_context.blueprint.query_caches().latest_at(
+                        ctx.store_context.blueprint.store(),
+                        &ctx.current_query(),
+                        &override_value.path,
+                        [*component_name],
+                    )
                 }
+                re_log_types::StoreKind::Blueprint => {
+                    ctx.store_context.blueprint.query_caches().latest_at(
+                        ctx.store_context.blueprint.store(),
+                        ctx.blueprint_query,
+                        &override_value.path,
+                        [*component_name],
+                    )
+                }
+            };
+
+            // If we successfully find a non-empty override, add it to our results.
+
+            // TODO(jleibs): it seems like value could still be null/empty if the override
+            // has been cleared. It seems like something is preventing that from happening
+            // but I don't fully understand what.
+            //
+            // This is extra tricky since the promise hasn't been resolved yet so we can't
+            // actually look at the data.
+            if let Some(value) = component_override_result.components.get(component_name) {
+                overrides.add(*component_name, value.clone());
             }
         }
     }

crates/re_viewer_context/src/space_view/view_query.rs

@@ -46,11 +46,11 @@ pub struct PropertyOverrides {
 
     /// `EntityPath` in the Blueprint store where updated overrides should be written back
     /// for properties that apply recursively.
-    pub recursive_override_path: EntityPath,
+    pub recursive_path: EntityPath,
 
     /// `EntityPath` in the Blueprint store where updated overrides should be written back
     /// for properties that apply to the individual entity only.
-    pub individual_override_path: EntityPath,
+    pub individual_path: EntityPath,
 
     /// What range is queried on the chunk store.
     pub query_range: QueryRange,
@@ -79,25 +79,21 @@ pub struct DataResult {
     pub tree_prefix_only: bool,
 
     /// The accumulated property overrides for this `DataResult`.
-    pub property_overrides: Option<PropertyOverrides>,
+    pub property_overrides: PropertyOverrides,
 }
 
 impl DataResult {
     pub const INDIVIDUAL_OVERRIDES_PREFIX: &'static str = "individual_overrides";
     pub const RECURSIVE_OVERRIDES_PREFIX: &'static str = "recursive_overrides";
 
     #[inline]
-    pub fn recursive_override_path(&self) -> Option<&EntityPath> {
-        self.property_overrides
-            .as_ref()
-            .map(|p| &p.recursive_override_path)
+    pub fn recursive_override_path(&self) -> &EntityPath {
+        &self.property_overrides.recursive_path
     }
 
     #[inline]
-    pub fn individual_override_path(&self) -> Option<&EntityPath> {
-        self.property_overrides
-            .as_ref()
-            .map(|p| &p.individual_override_path)
+    pub fn individual_override_path(&self) -> &EntityPath {
+        &self.property_overrides.individual_path
     }
 
     /// Saves a recursive override OR clears both (!) individual & recursive overrides if the override is due to a parent recursive override or a default value.
@@ -111,20 +107,6 @@ impl DataResult {
     ) {
         re_tracing::profile_function!();
 
-        // TODO(jleibs): Make it impossible for this to happen with different type structure
-        // This should never happen unless we're doing something with a partially processed
-        // query.
-        let (Some(recursive_override_path), Some(individual_override_path)) = (
-            self.recursive_override_path(),
-            self.individual_override_path(),
-        ) else {
-            re_log::warn!(
-                "Tried to save override for {:?} but it has no override path",
-                self.entity_path
-            );
-            return;
-        };
-
         if let Some(current_resolved_override) = self.lookup_override::<C>(ctx) {
             // Do nothing if the resolved override is already the same as the new override.
             if &current_resolved_override == desired_override {
@@ -147,104 +129,27 @@ impl DataResult {
                 || (parent_recursive_override.is_none() && desired_override == &C::default())
             {
                 // TODO(andreas): It might be that only either of these two are necessary, in that case we shouldn't clear both.
-                ctx.save_empty_blueprint_component::<C>(recursive_override_path);
-                ctx.save_empty_blueprint_component::<C>(individual_override_path);
+                ctx.save_empty_blueprint_component::<C>(&self.property_overrides.recursive_path);
+                ctx.save_empty_blueprint_component::<C>(&self.property_overrides.individual_path);
             } else {
-                ctx.save_blueprint_component(recursive_override_path, desired_override);
+                ctx.save_blueprint_component(
+                    &self.property_overrides.recursive_path,
+                    desired_override,
+                );
             }
         } else {
             // No override at all so far, simply set it.
-            ctx.save_blueprint_component(recursive_override_path, desired_override);
+            ctx.save_blueprint_component(&self.property_overrides.recursive_path, desired_override);
         }
     }
 
-    /// Saves a recursive override, does not take into current or default values.
-    ///
-    /// Ignores individual overrides and current value.
-    pub fn save_recursive_override<C: re_types::Component + Eq>(
-        &self,
-        ctx: &ViewerContext<'_>,
-        desired_override: &C,
-    ) {
-        re_tracing::profile_function!();
-
-        // TODO(jleibs): Make it impossible for this to happen with different type structure
-        // This should never happen unless we're doing something with a partially processed
-        // query.
-        let Some(recursive_override_path) = self.recursive_override_path() else {
-            re_log::warn!(
-                "Tried to save override for {:?} but it has no override path",
-                self.entity_path
-            );
-            return;
-        };
-
-        ctx.save_blueprint_component(recursive_override_path, desired_override);
-    }
-
-    /// Saves a recursive override, does not take into current or default values.
-    ///
-    /// Ignores individual overrides and current value.
-    pub fn save_individual_override<C: re_types::Component>(
-        &self,
-        ctx: &ViewerContext<'_>,
-        desired_override: &C,
-    ) {
-        re_tracing::profile_function!();
-
-        // TODO(jleibs): Make it impossible for this to happen with different type structure
-        // This should never happen unless we're doing something with a partially processed
-        // query.
-        let Some(override_path) = self.individual_override_path() else {
-            re_log::warn!(
-                "Tried to save override for {:?} but it has no override path",
-                self.entity_path
-            );
-            return;
-        };
-
-        ctx.save_blueprint_component(override_path, desired_override);
-    }
-
-    /// Clears the recursive override for a given component
-    pub fn clear_recursive_override<C: re_types::Component>(&self, ctx: &ViewerContext<'_>) {
-        // TODO(jleibs): Make it impossible for this to happen with different type structure
-        // This should never happen unless we're doing something with a partially processed
-        // query.
-        let Some(recursive_override_path) = self.recursive_override_path() else {
-            re_log::warn!(
-                "Tried to save override for {:?} but it has no override path",
-                self.entity_path
-            );
-            return;
-        };
-
-        ctx.save_empty_blueprint_component::<C>(recursive_override_path);
-    }
-
-    /// Clears the recursive override for a given component
-    pub fn clear_individual_override<C: re_types::Component>(&self, ctx: &ViewerContext<'_>) {
-        // TODO(jleibs): Make it impossible for this to happen with different type structure
-        // This should never happen unless we're doing something with a partially processed
-        // query.
-        let Some(individual_override_path) = self.individual_override_path() else {
-            re_log::warn!(
-                "Tried to save override for {:?} but it has no override path",
-                self.entity_path
-            );
-            return;
-        };
-
-        ctx.save_empty_blueprint_component::<C>(individual_override_path);
-    }
-
     fn lookup_override<C: 'static + re_types::Component>(
         &self,
         ctx: &ViewerContext<'_>,
     ) -> Option<C> {
         self.property_overrides
-            .as_ref()
-            .and_then(|p| p.resolved_component_overrides.get(&C::name()))
+            .resolved_component_overrides
+            .get(&C::name())
             .and_then(|OverridePath { store_kind, path }| match store_kind {
                 // TODO(#5607): what should happen if the promise is still pending?
                 StoreKind::Blueprint => ctx
@@ -272,8 +177,8 @@ impl DataResult {
         // If we don't have a resolved override, clearly nothing overrode this.
         let active_override = self
             .property_overrides
-            .as_ref()
-            .and_then(|p| p.resolved_component_overrides.get(&component_name))?;
+            .resolved_component_overrides
+            .get(&component_name)?;
 
         // Walk up the tree to find the highest ancestor which has a matching override.
         // This must be the ancestor we inherited the override from. Note that `active_override`
@@ -282,11 +187,11 @@ impl DataResult {
         while let Some(parent_path) = override_source.parent() {
             if result_tree
                 .lookup_result_by_path(&parent_path)
-                .and_then(|data_result| data_result.property_overrides.as_ref())
-                .map_or(true, |property_overrides| {
+                .map_or(true, |data_result| {
                     // TODO(andreas): Assumes all overrides are recursive which is not true,
                     //                This should access `recursive_component_overrides` instead.
-                    property_overrides
+                    data_result
+                        .property_overrides
                         .resolved_component_overrides
                         .get(&component_name)
                         != Some(active_override)
@@ -336,10 +241,7 @@ impl DataResult {
 
     /// Returns the query range for this data result.
     pub fn query_range(&self) -> &QueryRange {
-        const DEFAULT_RANGE: QueryRange = QueryRange::LatestAt;
-        self.property_overrides
-            .as_ref()
-            .map_or(&DEFAULT_RANGE, |p| &p.query_range)
+        &self.property_overrides.query_range
     }
 }
 

crates/re_viewport_blueprint/src/space_view.rs

@@ -760,20 +760,19 @@ mod tests {
                 HashMap::default();
             query_result.tree.visit(&mut |node| {
                 let result = &node.data_result;
-                if let Some(property_overrides) = &result.property_overrides {
-                    if !property_overrides.resolved_component_overrides.is_empty() {
-                        visited.insert(
-                            result.entity_path.clone(),
-                            property_overrides
-                                .resolved_component_overrides
-                                .iter()
-                                .map(|(component_name, OverridePath { store_kind, path })| {
-                                    assert_eq!(store_kind, &StoreKind::Blueprint);
-                                    (*component_name, path.clone())
-                                })
-                                .collect(),
-                        );
-                    }
+                let resolved_component_overrides =
+                    &result.property_overrides.resolved_component_overrides;
+                if !resolved_component_overrides.is_empty() {
+                    visited.insert(
+                        result.entity_path.clone(),
+                        resolved_component_overrides
+                            .iter()
+                            .map(|(component_name, OverridePath { store_kind, path })| {
+                                assert_eq!(*store_kind, StoreKind::Blueprint);
+                                (*component_name, path.clone())
+                            })
+                            .collect(),
+                    );
                 }
                 true
             });