Refactor viewport blueprint logic

[emilk]

The viewport blueprint code is a mess, and modifying it is extremely error prone

• This is blocking: Undo (and redo) in viewer #3135
• Related: Marking of edited for the blueprint is error prone #4687
• Related: Cannot find root container warning on startup #8051

No one is to blame - this has evolved over time - but we need to untangle this.

How and why is it messy?

I don't fully understand all of it.

Triplicate state

• As components in the blueprint store (persisted)
• As egui_tiles::Tree (created at the start of each frame)
• As struct ViewportBlueprint (created at the start of each frame)

Existence of blueprint entities is significant

We use SystemCommand::DropEntity to permanently remove space-view entities from the blueprint store. This means we cannot undo the removal of a space-view.

Our loader should instead start with the RootContainer and recuse down, and ignore all entities that aren't reachable.
When I tried to implement this however, I created more trouble for myself.

Views

We view the ViewportBlueprint it in two places:

• The BlueprintTree panel (left)
• The viewport panel (center)

If we only mutated the state at the end/beginning of the frame, this wouldn't be a problem,
but I suspect we actually mutate state between calling these two views, which of course causes problems.

Auto-layout

We want to automatically create space-views if there is no explicit blueprint, leading to complicated state like auto_layout and auto_space_views:

rerun/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs

Lines 44 to 54 in 31a166f

	/// Whether the viewport layout is determined automatically.
	///
	/// Set to `false` the first time the user messes around with the viewport blueprint.
	/// Note: we use a mutex here because writes needs to be effective immediately during the frame.
	auto_layout: AtomicBool,
	/// Whether space views should be created automatically.
	///
	/// Note: we use a mutex here because writes needs to be effective immediately during the frame.
	auto_space_views: AtomicBool,

Delayed action

Almost all edits of the state is delayed, often in multiple steps.

This is because of (at least) two reasons (as far as I can tell)

• we cannot modify a egui_tiles::Tree while we're iterating over it
• when modifying the state, we want to modify all THREE copies of it… and the best way to do that is to wait until the end of the frame so we only edit the persisted store state

Delays happen in

• TreeAction
• A TreeAction can however lead to another frame-delay:

  rerun/crates/viewer/re_viewport/src/viewport.rs

  Lines 415 to 421 in 31a166f

  	// written to the store yet.
  	re_log::trace!("Clearing the blueprint tree to force reset on the next frame");
  	self.tree = egui_tiles::Tree::empty("viewport_tree");
  	self.tree_edited = true;
  	}
  	// Finally, save any edits to the blueprint tree

[abey79]

GUI automated testing would require TreeActions to be somehow processed by our TestContext mock. The mess described in this issue makes this basically impossible. A refactor of the above would ideally lead to some execute_tree_actions function in re_viewport_blueprint that could be called by TestContext as appropriate. (Currently this function lives in re_viewport and its current implementation is deeply entangled with egui_tile stuff.)

cc @lucasmerlin

[emilk]

Plan

The ViewportBlueprint and its egui_tiles::Tree is the source of truth. If a space view is not in the tree, IT DOES NOT EXIST.

Replace ViewportBlueprint::tree_action_sender with deferred_actions: Mutex<Vec<TreeAction>> and add ViewportBlueprint.apply_pending_actions.

Remove Viewport::tree. Instead use Viewport::blueprint.tree. When you need to mutate the tree (in Viewport::viewport_ui), create a clone of the tree, call tree.ui. If it changed (user dragged a tile), then create a TreeAction::SetTree(tree), which will be applied at the end of the frame.

Chronology over a frame

• Load ViewportBlueprint from the blueprint store at the start of the frame
  • For the Tree, recurse from root container (like we do in the undo-PR: https://github.com/rerun-io/rerun/pull/7546/files#diff-c03bce493e319f634c710bbb00833f404f2bb9249a7197d91d51bfcd847547df)
  • If there is no root, or if it is pointing to an empty container, this means the viewport is and should be empty
  • Also load auto_layout and auto_spawn_space_views bools from blueprint. Default values are true.
• TreeAction for all mutation
  • If auto_spawn_space_views is set, that may results in TreeAction::AddSpaceView
  • Right-clicking and "Add SpaceView" results in TreeAction
  • Tree.ui mutate a clone of the tree. If it changed, produce a TreeAction::set_tree
• We run all TreeActions at the end of the frame. They are all applied directly on the ViewportBlueprint and its Tree (i.e. mutate it)
• If auto_layout is set:
  • Get all space-views _currently in the Tree and run auto-layout on that
• Write the tree to the store (if modified)

Three levels of auto

• Action: Reset everything (delete tree and space views, set both autos below to true)
• State (default: true): Auto-layout (take existing space-views, and re-range their tree-structure)
• State (default: true): Auto-spawn-space-view (if new recording entities are received, create space views for them)

Sources of confusion

The existence of space-views is separate from their existence in the tree

[emilk]

Checklist for ongoing work

• Remove duplicated tree
• Only mutate the tree at the end of the frame
• Rename TreeAction to ViewportCommand
• Load blueprint recursively from the root
• Rename Viewport to ViewportUi
• Delete IncludedSpaceView
• Put ViewportBlueprint into ViewerContext?
• Only write to the blueprint store inside update_and_sync_tile_tree_to_blueprint
• Do not remove entities from blueprint store
• Figure out what happens when you reset the blueprint, and why that results in a missing container