tree is now explicitely about editing recursive properties whereas se…

…lection panel always operates on individual

crates/re_space_view/src/data_query_blueprint.rs

@@ -543,8 +543,9 @@ impl DataQueryPropertyResolver<'_> {
                 }
 
                 node.data_result.property_overrides = Some(PropertyOverrides {
-                    individual_properties: individual_properties.cloned(),
                     accumulated_properties,
+                    individual_properties: individual_properties.cloned(),
+                    recursive_properties: recursive_properties.cloned(),
                     component_overrides,
                     recursive_override_path,
                     individual_override_path,

crates/re_space_view/src/space_view.rs

@@ -479,6 +479,7 @@ impl SpaceViewBlueprint {
             property_overrides: Some(PropertyOverrides {
                 accumulated_properties,
                 individual_properties,
+                recursive_properties: Default::default(),
                 component_overrides: Default::default(),
                 recursive_override_path: entity_path.clone(),
                 individual_override_path: entity_path,

crates/re_viewer/src/ui/selection_panel.rs

@@ -881,7 +881,10 @@ fn blueprint_ui_for_space_view(
         // Space View don't inherit properties.
         let root_data_result = space_view.root_data_result(ctx.store_context, ctx.blueprint_query);
 
-        let mut props = root_data_result.accumulated_properties().clone();
+        let mut props = root_data_result
+            .individual_properties()
+            .cloned()
+            .unwrap_or_default();
 
         visible_history_ui(
             ctx,
@@ -904,7 +907,7 @@ fn blueprint_ui_for_space_view(
                 &mut props,
             );
 
-        root_data_result.save_recursive_override(Some(props), ctx);
+        root_data_result.save_individual_override(Some(props), ctx);
     }
 }
 
@@ -928,7 +931,11 @@ fn blueprint_ui_for_instance_path(
                     .lookup_result_by_path(entity_path)
                     .cloned()
                 {
-                    let mut props = data_result.accumulated_properties().clone();
+                    let mut props = data_result
+                        .individual_properties()
+                        .cloned()
+                        .unwrap_or_default();
+
                     entity_props_ui(
                         ctx,
                         ui,
@@ -937,7 +944,7 @@ fn blueprint_ui_for_instance_path(
                         &mut props,
                         data_result.accumulated_properties(),
                     );
-                    data_result.save_recursive_override(Some(props), ctx);
+                    data_result.save_individual_override(Some(props), ctx);
                 }
             }
         }

crates/re_viewer_context/src/space_view/view_query.rs

@@ -24,6 +24,9 @@ pub struct PropertyOverrides {
     /// The individual property set in this `DataResult`, if any.
     pub individual_properties: Option<EntityProperties>,
 
+    /// The recursive property set in this `DataResult`, if any.
+    pub recursive_properties: Option<EntityProperties>,
+
     /// An alternative store and entity path to use for the specified component.
     // NOTE: StoreKind is easier to work with than a `StoreId`` or full `DataStore` but
     // might still be ambiguous when we have multiple stores active at a time.
@@ -86,33 +89,61 @@ impl DataResult {
             .map(|p| &p.individual_override_path)
     }
 
-    /// Write the [`EntityProperties`] for this result back to the Blueprint store.
+    /// Write the [`EntityProperties`] for this result back to the Blueprint store on the recursive override.
     ///
-    /// Writes only if the accumulated (!) properties aren't already the same value.
-    /// Changes will be applied recursively in subsequent frames, meaning this writes to the recursive override path.
-    /// TODO(andreas): This does NOT handle the case when individual properties need clearing to achieve the desired property result.
+    /// Setting `new_recursive_props` to `None` will always clear the override.
+    /// Otherwise, writes only if the recursive properties aren't already the same value.
+    /// (does *not* take into account what the accumulated properties are which are a combination of recursive and individual overwrites)
     pub fn save_recursive_override(
         &self,
-        new_accumulated_props: Option<EntityProperties>,
+        ctx: &ViewerContext<'_>,
+        new_recursive_props: Option<EntityProperties>,
+    ) {
+        self.save_override_internal(
+            ctx,
+            new_recursive_props,
+            self.recursive_override_path(),
+            self.recursive_properties(),
+        );
+    }
+
+    /// Write the [`EntityProperties`] for this result back to the Blueprint store on the individual override.
+    ///
+    /// Setting `new_individual_props` to `None` will always clear the override.
+    /// Otherwise, writes only if the individual properties aren't already the same value.
+    /// (does *not* take into account what the accumulated properties are which are a combination of recursive and individual overwrites)
+    pub fn save_individual_override(
+        &self,
+        new_individual_props: Option<EntityProperties>,
         ctx: &ViewerContext<'_>,
+    ) {
+        self.save_override_internal(
+            ctx,
+            new_individual_props,
+            self.individual_override_path(),
+            self.individual_properties(),
+        );
+    }
+
+    fn save_override_internal(
+        &self,
+        ctx: &ViewerContext<'_>,
+        new_individual_props: Option<EntityProperties>,
+        override_path: Option<&EntityPath>,
+        properties: Option<&EntityProperties>,
     ) {
         // 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.recursive_override_path() else {
+        let Some(override_path) = override_path else {
             re_log::warn!(
                 "Tried to save override for {:?} but it has no override path",
                 self.entity_path
             );
             return;
         };
 
-        // This should never happen if the above didn't return early.
-        let Some(property_overrides) = &self.property_overrides else {
-            return;
-        };
-
-        let cell = match new_accumulated_props {
+        let cell = match new_individual_props {
             None => {
                 re_log::debug!("Clearing {:?}", override_path);
 
@@ -122,7 +153,10 @@ impl DataResult {
                 ))
             }
             Some(props) => {
-                if props.has_edits(&property_overrides.accumulated_properties) {
+                // A value of `None` in the data store means "use the default value", so if
+                // the properties are `None`, we only must save if `props` is different
+                // from the default.
+                if props.has_edits(properties.unwrap_or(&DEFAULT_PROPS)) {
                     re_log::debug!("Overriding {:?} with {:?}", override_path, props);
 
                     let component = EntityPropertiesComponent(props);
@@ -134,21 +168,19 @@ impl DataResult {
             }
         };
 
-        let Some(cell) = cell else {
-            return;
-        };
-
-        let timepoint = blueprint_timepoint_for_writes();
+        if let Some(cell) = cell {
+            let timepoint = blueprint_timepoint_for_writes();
 
-        let row =
-            DataRow::from_cells1_sized(RowId::new(), override_path.clone(), timepoint, 1, cell)
-                .unwrap();
+            let row =
+                DataRow::from_cells1_sized(RowId::new(), override_path.clone(), timepoint, 1, cell)
+                    .unwrap();
 
-        ctx.command_sender
-            .send_system(SystemCommand::UpdateBlueprint(
-                ctx.store_context.blueprint.store_id().clone(),
-                vec![row],
-            ));
+            ctx.command_sender
+                .send_system(SystemCommand::UpdateBlueprint(
+                    ctx.store_context.blueprint.store_id().clone(),
+                    vec![row],
+                ));
+        }
     }
 
     #[inline]
@@ -167,6 +199,13 @@ impl DataResult {
         &property_overrides.accumulated_properties
     }
 
+    #[inline]
+    pub fn recursive_properties(&self) -> Option<&EntityProperties> {
+        self.property_overrides
+            .as_ref()
+            .and_then(|p| p.recursive_properties.as_ref())
+    }
+
     #[inline]
     pub fn individual_properties(&self) -> Option<&EntityProperties> {
         self.property_overrides

crates/re_viewport/src/viewport_blueprint_ui.rs

@@ -285,7 +285,7 @@ impl Viewport<'_, '_> {
         query_result: &DataQueryResult,
         top_node: &DataResultNode,
         space_view: &SpaceViewBlueprint,
-        parent_visible: bool,
+        space_view_visible: bool,
     ) {
         let query = ctx.current_query();
         let store = ctx.entity_db.store();
@@ -314,15 +314,18 @@ impl Viewport<'_, '_> {
             let is_item_hovered =
                 ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered;
 
-            let mut accumulated_properties = data_result.accumulated_properties().clone();
-            let visible = accumulated_properties.visible;
+            let visible = data_result.accumulated_properties().visible;
+            let mut recursive_properties = data_result
+                .recursive_properties()
+                .cloned()
+                .unwrap_or_default();
 
             let name = entity_path
                 .iter()
                 .last()
                 .map_or("unknown".to_owned(), |e| e.ui_string());
 
-            let subdued = !parent_visible
+            let subdued = !space_view_visible
                 || !visible
                 || (data_result.visualizers.is_empty() && child_node.children.is_empty());
 
@@ -331,8 +334,8 @@ impl Viewport<'_, '_> {
                 let vis_response = visibility_button_ui(
                     re_ui,
                     ui,
-                    parent_visible,
-                    &mut accumulated_properties.visible,
+                    space_view_visible,
+                    &mut recursive_properties.visible,
                 );
 
                 let response = remove_button_ui(
@@ -380,7 +383,7 @@ impl Viewport<'_, '_> {
                                 query_result,
                                 child_node,
                                 space_view,
-                                visible,
+                                space_view_visible,
                             );
                         },
                     )
@@ -405,7 +408,7 @@ impl Viewport<'_, '_> {
                 );
             }
 
-            data_result.save_recursive_override(Some(accumulated_properties), ctx);
+            data_result.save_recursive_override(ctx, Some(recursive_properties));
 
             ctx.select_hovered_on_click(&response, item);
         }