Get rid of sync_blueprint_changes

crates/re_space_view/src/data_query_blueprint.rs

@@ -4,6 +4,7 @@ use re_data_store::{
     EntityProperties, EntityPropertiesComponent, EntityPropertyMap, EntityTree, StoreDb,
 };
 use re_log_types::{DataRow, EntityPath, EntityPathExpr, RowId, TimePoint};
+use re_types_core::archetypes::Clear;
 use re_viewer_context::{
     DataQueryId, DataQueryResult, DataResult, DataResultHandle, DataResultNode, DataResultTree,
     EntitiesPerSystem, EntitiesPerSystemPerClass, SpaceViewClassIdentifier, SpaceViewId,
@@ -28,7 +29,14 @@ use crate::{
 /// The results of recursive expressions are only included if they are found within the [`EntityTree`]
 /// and for which there is a valid `ViewPart` system. This keeps recursive expressions from incorrectly
 /// picking up irrelevant data within the tree.
-#[derive(Clone, PartialEq, Eq)]
+///
+/// Note: [`DataQueryBlueprint`] doesn't implement Clone because it stores an internal
+/// uuid used for identifying the path of its data in the blueprint store. It's ambiguous
+/// whether the intent is for a clone to write to the same place.
+///
+/// If you want a new space view otherwise identical to an existing one, use
+/// [`DataQueryBlueprint::duplicate`].
+#[derive(PartialEq, Eq)]
 pub struct DataQueryBlueprint {
     pub id: DataQueryId,
     pub space_view_class_identifier: SpaceViewClassIdentifier,
@@ -47,6 +55,10 @@ impl DataQueryBlueprint {
     pub const INDIVIDUAL_OVERRIDES_PREFIX: &'static str = "individual_overrides";
     pub const RECURSIVE_OVERRIDES_PREFIX: &'static str = "recursive_overrides";
 
+    /// Creates a new [`DataQueryBlueprint`].
+    ///
+    /// This [`DataQueryBlueprint`] is ephemeral. It must be saved by calling
+    /// `save_to_blueprint_store` on the enclosing `SpaceViewBlueprint`.
     pub fn new(
         space_view_class_identifier: SpaceViewClassIdentifier,
         queries_entities: impl Iterator<Item = EntityPathExpr>,
@@ -58,25 +70,48 @@ impl DataQueryBlueprint {
         }
     }
 
+    /// Attempt to load a [`DataQueryBlueprint`] from the blueprint store.
     pub fn try_from_db(
-        path: &EntityPath,
+        id: DataQueryId,
         blueprint_db: &StoreDb,
         space_view_class_identifier: SpaceViewClassIdentifier,
     ) -> Option<Self> {
         let expressions = blueprint_db
             .store()
-            .query_timeless_component::<QueryExpressions>(path)
+            .query_timeless_component::<QueryExpressions>(&id.as_entity_path())
             .map(|c| c.value)?;
 
-        let id = DataQueryId::from_entity_path(path);
-
         Some(Self {
             id,
             space_view_class_identifier,
             expressions,
         })
     }
 
+    /// Persist the entire [`DataQueryBlueprint`] to the blueprint store.
+    ///
+    /// This only needs to be called if the [`DataQueryBlueprint`] was created with [`Self::new`].
+    ///
+    /// Otherwise, incremental calls to `set_` functions will write just the necessary component
+    /// update directly to the store.
+    pub fn save_to_blueprint_store(&self, ctx: &ViewerContext<'_>) {
+        ctx.save_blueprint_component(&self.id.as_entity_path(), self.expressions.clone());
+    }
+
+    /// Creates a new [`DataQueryBlueprint`] with a the same contents, but a different [`DataQueryId`]
+    pub fn duplicate(&self) -> Self {
+        Self {
+            id: DataQueryId::random(),
+            space_view_class_identifier: self.space_view_class_identifier,
+            expressions: self.expressions.clone(),
+        }
+    }
+
+    pub fn clear(&self, ctx: &ViewerContext<'_>) {
+        let clear = Clear::recursive();
+        ctx.save_blueprint_component(&self.id.as_entity_path(), clear.is_recursive);
+    }
+
     pub fn build_resolver<'a>(
         &self,
         container: SpaceViewId,

crates/re_types/definitions/rerun/blueprint/archetypes/viewport_blueprint.fbs

@@ -19,11 +19,11 @@ table ViewportBlueprint (
     /// All of the space-views that belong to the viewport.
     space_views: rerun.blueprint.components.IncludedSpaceViews ("attr.rerun.component_required", order: 1000);
 
-    /// The layout of the space-views
-    layout: rerun.blueprint.components.ViewportLayout ("attr.rerun.component_required", order: 2000);
-
     // --- Optional ---
 
+    /// The layout of the space-views
+    layout: rerun.blueprint.components.ViewportLayout ("attr.rerun.component_optional", nullable, order: 2000);
+
     /// Show one tab as maximized?
     maximized: rerun.blueprint.components.SpaceViewMaximized ("attr.rerun.component_optional", nullable, order: 3000);
 

crates/re_viewer/src/app.rs

@@ -368,6 +368,7 @@ impl App {
             SystemCommand::ResetBlueprint => {
                 // By clearing the blueprint it will be re-populated with the defaults
                 // at the beginning of the next frame.
+                re_log::debug!("Reset blueprint");
                 store_hub.clear_blueprint();
             }
             SystemCommand::UpdateBlueprint(blueprint_id, updates) => {

crates/re_viewer/src/app_state.rs

@@ -6,10 +6,11 @@ use re_smart_channel::ReceiveSet;
 use re_space_view::DataQuery as _;
 use re_viewer_context::{
     AppOptions, Caches, CommandSender, ComponentUiRegistry, PlayState, RecordingConfig,
-    SelectionState, SpaceViewClassRegistry, StoreContext, ViewerContext,
+    SelectionState, SpaceViewClassRegistry, StoreContext, SystemCommandSender as _, ViewerContext,
 };
 use re_viewport::{
-    identify_entities_per_system_per_class, SpaceInfoCollection, Viewport, ViewportState,
+    identify_entities_per_system_per_class, SpaceInfoCollection, Viewport, ViewportBlueprint,
+    ViewportState,
 };
 
 use crate::ui::recordings_panel_ui;
@@ -104,7 +105,25 @@ impl AppState {
             viewport_state,
         } = self;
 
-        let mut viewport = Viewport::from_db(store_context.blueprint, viewport_state);
+        let viewport_blueprint = ViewportBlueprint::try_from_db(store_context.blueprint);
+        let mut viewport = Viewport::new(
+            &viewport_blueprint,
+            viewport_state,
+            space_view_class_registry,
+        );
+
+        // If the blueprint is invalid, reset it.
+        if viewport.blueprint.is_invalid() {
+            re_log::warn!("Incompatible blueprint detected. Resetting to default.");
+            command_sender.send_system(re_viewer_context::SystemCommand::ResetBlueprint);
+
+            // The blueprint isn't valid so nothing past this is going to work properly.
+            // we might as well return and it will get fixed on the next frame.
+
+            // TODO(jleibs): If we move viewport loading up to a context where the StoreDb is mutable
+            // we can run the clear and re-load.
+            return;
+        }
 
         recording_config_entry(recording_configs, store_db.store_id().clone(), store_db)
             .selection_state
@@ -125,11 +144,11 @@ impl AppState {
             viewport
                 .blueprint
                 .space_views
-                .values_mut()
+                .values()
                 .flat_map(|space_view| {
                     space_view.queries.iter().map(|query| {
-                        let resolver =
-                            query.build_resolver(space_view.id, &space_view.auto_properties);
+                        let props = viewport.state.space_view_props(space_view.id);
+                        let resolver = query.build_resolver(space_view.id, props);
                         (
                             query.id,
                             query.execute_query(
@@ -163,12 +182,6 @@ impl AppState {
         // have the latest information.
         let spaces_info = SpaceInfoCollection::new(ctx.store_db);
 
-        // If the blueprint is invalid, reset it.
-        if viewport.blueprint.is_invalid() {
-            re_log::warn!("Incompatible blueprint detected. Resetting to default.");
-            viewport.blueprint.reset(&ctx, &spaces_info);
-        }
-
         viewport.on_frame_start(&ctx, &spaces_info);
 
         // TODO(jleibs): Running the queries a second time is annoying, but we need
@@ -179,11 +192,11 @@ impl AppState {
             viewport
                 .blueprint
                 .space_views
-                .values_mut()
+                .values()
                 .flat_map(|space_view| {
                     space_view.queries.iter().map(|query| {
-                        let resolver =
-                            query.build_resolver(space_view.id, &space_view.auto_properties);
+                        let props = viewport.state.space_view_props(space_view.id);
+                        let resolver = query.build_resolver(space_view.id, props);
                         (
                             query.id,
                             query.execute_query(
@@ -243,7 +256,7 @@ impl AppState {
                             ui.add_space(4.0);
                         }
 
-                        blueprint_panel_ui(&mut viewport.blueprint, &ctx, ui, &spaces_info);
+                        blueprint_panel_ui(&mut viewport, &ctx, ui, &spaces_info);
                     },
                 );
 
@@ -266,7 +279,8 @@ impl AppState {
                     });
             });
 
-        viewport.sync_blueprint_changes(command_sender);
+        // Process deferred layout operations and apply updates back to blueprint
+        viewport.update_and_sync_tile_tree_to_blueprint(&ctx);
 
         {
             // We move the time at the very end of the frame,

crates/re_viewer/src/ui/blueprint_panel.rs

@@ -1,9 +1,9 @@
 use re_viewer_context::{SystemCommandSender as _, ViewerContext};
-use re_viewport::{SpaceInfoCollection, ViewportBlueprint};
+use re_viewport::{SpaceInfoCollection, Viewport};
 
-/// Show the Blueprint section of the left panel based on the current [`ViewportBlueprint`]
+/// Show the Blueprint section of the left panel based on the current [`Viewport`]
 pub fn blueprint_panel_ui(
-    blueprint: &mut ViewportBlueprint<'_>,
+    viewport: &mut Viewport<'_, '_>,
     ctx: &ViewerContext<'_>,
     ui: &mut egui::Ui,
     spaces_info: &SpaceInfoCollection,
@@ -14,15 +14,15 @@ pub fn blueprint_panel_ui(
             "Blueprint",
             Some("The Blueprint is where you can configure the Rerun Viewer"),
             |ui| {
-                blueprint.add_new_spaceview_button_ui(ctx, ui, spaces_info);
+                viewport.add_new_spaceview_button_ui(ctx, ui, spaces_info);
                 reset_blueprint_button_ui(ctx, ui);
             },
         );
     });
 
     // This call is excluded from `panel_content` because it has a ScrollArea, which should not be
     // inset. Instead, it calls panel_content itself inside the ScrollArea.
-    blueprint.tree_ui(ctx, ui);
+    viewport.tree_ui(ctx, ui);
 }
 
 fn reset_blueprint_button_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) {

crates/re_viewer/src/ui/selection_history_ui.rs

@@ -15,7 +15,7 @@ impl SelectionHistoryUi {
         &mut self,
         re_ui: &re_ui::ReUi,
         ui: &mut egui::Ui,
-        blueprint: &ViewportBlueprint<'_>,
+        blueprint: &ViewportBlueprint,
         history: &mut SelectionHistory,
     ) -> Option<ItemCollection> {
         let next = self.next_button_ui(re_ui, ui, blueprint, history);
@@ -27,7 +27,7 @@ impl SelectionHistoryUi {
         &mut self,
         re_ui: &re_ui::ReUi,
         ui: &mut egui::Ui,
-        blueprint: &ViewportBlueprint<'_>,
+        blueprint: &ViewportBlueprint,
         history: &mut SelectionHistory,
     ) -> Option<ItemCollection> {
         // undo selection
@@ -77,7 +77,7 @@ impl SelectionHistoryUi {
         &mut self,
         re_ui: &re_ui::ReUi,
         ui: &mut egui::Ui,
-        blueprint: &ViewportBlueprint<'_>,
+        blueprint: &ViewportBlueprint,
         history: &mut SelectionHistory,
     ) -> Option<ItemCollection> {
         // redo selection
@@ -126,7 +126,7 @@ impl SelectionHistoryUi {
     #[allow(clippy::unused_self)]
     fn history_item_ui(
         &mut self,
-        blueprint: &ViewportBlueprint<'_>,
+        blueprint: &ViewportBlueprint,
         ui: &mut egui::Ui,
         index: usize,
         history: &mut SelectionHistory,
@@ -157,7 +157,7 @@ fn item_kind_ui(ui: &mut egui::Ui, sel: &Item) {
     ui.weak(RichText::new(format!("({})", sel.kind())));
 }
 
-fn item_collection_to_string(blueprint: &ViewportBlueprint<'_>, items: &ItemCollection) -> String {
+fn item_collection_to_string(blueprint: &ViewportBlueprint, items: &ItemCollection) -> String {
     assert!(!items.is_empty()); // history never contains empty selections.
     if items.len() == 1 {
         item_to_string(blueprint, items.iter().next().unwrap())
@@ -168,7 +168,7 @@ fn item_collection_to_string(blueprint: &ViewportBlueprint<'_>, items: &ItemColl
     }
 }
 
-fn item_to_string(blueprint: &ViewportBlueprint<'_>, item: &Item) -> String {
+fn item_to_string(blueprint: &ViewportBlueprint, item: &Item) -> String {
     match item {
         Item::SpaceView(sid) => {
             if let Some(space_view) = blueprint.space_view(sid) {