Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the new container blueprints the default behavior #4642

Merged
merged 5 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,4 @@ debug = true
# egui-wgpu = { path = "../../egui/crates/egui-wgpu" }
# emath = { path = "../../egui/crates/emath" }

egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "dcdcde10b493683f3e97db47d128683bec0e65b9" } # https://github.com/rerun-io/egui_tiles/pull/40
egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "b6e4fd457b2eee2c671747ead12f4a20feb380e8" } # Merge of: https://github.com/rerun-io/egui_tiles/pull/41
16 changes: 8 additions & 8 deletions crates/re_viewer/src/ui/rerun_menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,14 @@ impl App {
ui.close_menu();
}

self.re_ui
.checkbox(
ui,
&mut self.state.app_options.legacy_container_blueprint,
"Use the legacy container blueprint storage for the viewport",
)
.on_hover_text("The legacy container blueprint storage is deprecated, but may be helpful if unexpected regressions are found in the new container blueprints.");

#[cfg(debug_assertions)]
{
ui.separator();
Expand Down Expand Up @@ -363,14 +371,6 @@ impl App {
)
.on_hover_text("Show the Blueprint data in the Time Panel tree view. This is useful for debugging the internal blueprint state.");

self.re_ui
.checkbox(
ui,
&mut self.state.app_options.experimental_container_blueprints,
"Use experimental container blueprints",
)
.on_hover_text("Load and save the container state using new container archetypes");

ui.menu_button("Crash", |ui| {
#[allow(clippy::manual_assert)]
if ui.button("panic!").clicked() {
Expand Down
6 changes: 3 additions & 3 deletions crates/re_viewer_context/src/app_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ pub struct AppOptions {
/// Enable experimental dataframe space views.
pub experimental_dataframe_space_view: bool,

/// Enable experimental support for new container blueprints
pub experimental_container_blueprints: bool,
/// Use the legacy container blueprint storage for the space view.
pub legacy_container_blueprint: bool,

/// Displays an overlay for debugging picking.
pub show_picking_debug_overlay: bool,
Expand All @@ -43,7 +43,7 @@ impl Default for AppOptions {

experimental_dataframe_space_view: false,

experimental_container_blueprints: cfg!(debug_assertions),
legacy_container_blueprint: false,

show_picking_debug_overlay: false,

Expand Down
42 changes: 20 additions & 22 deletions crates/re_viewport/src/viewport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ pub enum TreeAction {
/// Remove a tile and all its children.
Remove(egui_tiles::TileId),

/// Simplify the specified subtree with the provided options
SimplifyTree(egui_tiles::TileId, egui_tiles::SimplificationOptions),
/// Simplify the container with the provided options
SimplifyContainer(egui_tiles::TileId, egui_tiles::SimplificationOptions),
}

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -380,7 +380,7 @@ impl<'a, 'b> Viewport<'a, 'b> {
self.edited = true;
}
TreeAction::Remove(tile_id) => {
for tile in self.tree.tiles.remove_recursively(tile_id) {
for tile in self.tree.remove_recursively(tile_id) {
re_log::trace!("Removing tile {tile_id:?}");
if let egui_tiles::Tile::Pane(space_view_id) = tile {
re_log::trace!("Removing space view {space_view_id}");
Expand All @@ -394,9 +394,9 @@ impl<'a, 'b> Viewport<'a, 'b> {
}
self.edited = true;
}
TreeAction::SimplifyTree(tile_id, options) => {
TreeAction::SimplifyContainer(tile_id, options) => {
re_log::trace!("Simplifying tree with options: {options:?}");
self.tree.simplify_tile(tile_id, &options);
self.tree.simplify_children_of_tile(tile_id, &options);
self.edited = true;
}
}
Expand All @@ -412,24 +412,22 @@ impl<'a, 'b> Viewport<'a, 'b> {

// Finally, save any edits to the blueprint tree
// This is a no-op if the tree hasn't changed.
if ctx.app_options.experimental_container_blueprints {
if self.edited {
// TODO(abey79): Decide what simplification to do here. Some of this
// might need to get rolled into the save logic instead.

// Simplify before we save the tree. Normally additional simplification will
// happen on the next render loop, but that's too late -- unsimplified
// changes will be baked into the tree.
let options = egui_tiles::SimplificationOptions {
all_panes_must_have_tabs: true,
..Default::default()
};
self.tree.simplify(&options);

self.blueprint.save_tree_as_containers(&self.tree, ctx);
}
} else {
if ctx.app_options.legacy_container_blueprint {
self.blueprint.set_tree(&self.tree, ctx);
} else if self.edited {
// TODO(abey79): Decide what simplification to do here. Some of this
// might need to get rolled into the save logic instead.

// Simplify before we save the tree. Normally additional simplification will
// happen on the next render loop, but that's too late -- unsimplified
// changes will be baked into the tree.
let options = egui_tiles::SimplificationOptions {
all_panes_must_have_tabs: true,
..Default::default()
};
self.tree.simplify(&options);

self.blueprint.save_tree_as_containers(&self.tree, ctx);
}
}

Expand Down
99 changes: 47 additions & 52 deletions crates/re_viewport/src/viewport_blueprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,22 @@ impl ViewportBlueprint {

let maximized = arch.maximized.and_then(|id| id.0.map(|id| id.into()));

let tree = blueprint_db
.store()
.query_timeless_component_quiet::<ViewportLayout>(&VIEWPORT_PATH.into())
.map(|space_view| space_view.value)
.unwrap_or_default()
.0;

let mut blueprint = ViewportBlueprint {
let tree = if app_options.legacy_container_blueprint {
blueprint_db
.store()
.query_timeless_component_quiet::<ViewportLayout>(&VIEWPORT_PATH.into())
.map(|space_view| space_view.value)
.unwrap_or_default()
.0
} else {
build_tree_from_space_views_and_containers(
space_views.keys(),
containers.values(),
root_container,
)
};

ViewportBlueprint {
space_views,
containers,
root_container,
Expand All @@ -148,28 +156,8 @@ impl ViewportBlueprint {
auto_layout,
auto_space_views,
tree_action_sender,
};

if app_options.experimental_container_blueprints {
let mut tree = blueprint.build_tree_from_containers();

// TODO(abey79): Figure out if we want to simplify here or not.
if false {
let options = egui_tiles::SimplificationOptions {
all_panes_must_have_tabs: true,
..Default::default()
};

tree.simplify(&options);
}

re_log::trace!("Loaded tree from blueprint: {tree:#?}");

blueprint.tree = tree;
}

blueprint

// TODO(jleibs): Need to figure out if we have to re-enable support for
// auto-discovery of SpaceViews logged via the experimental blueprint APIs.
/*
Expand Down Expand Up @@ -402,13 +390,16 @@ impl ViewportBlueprint {
self.send_tree_action(TreeAction::SetContainerKind(container_id, kind));
}

/// Simplify the container subtree with the provided options.
pub fn simplify_tree(
/// Simplify the container tree with the provided options.
pub fn simplify_container(
&self,
tile_id: egui_tiles::TileId,
simplification_options: SimplificationOptions,
) {
self.send_tree_action(TreeAction::SimplifyTree(tile_id, simplification_options));
self.send_tree_action(TreeAction::SimplifyContainer(
tile_id,
simplification_options,
));
}

#[allow(clippy::unused_self)]
Expand Down Expand Up @@ -565,30 +556,34 @@ impl ViewportBlueprint {
ctx.save_empty_blueprint_component::<RootContainer>(&VIEWPORT_PATH.into());
}
}
}

pub fn build_tree_from_containers(&self) -> egui_tiles::Tree<SpaceViewId> {
re_tracing::profile_function!();
let mut tree = egui_tiles::Tree::empty("viewport_tree");

// First add all the space_views
for space_view in self.space_views.keys() {
let tile_id = blueprint_id_to_tile_id(space_view);
let pane = egui_tiles::Tile::Pane(*space_view);
tree.tiles.insert(tile_id, pane);
}

// Now add all the containers
for container in self.containers.values() {
let tile_id = blueprint_id_to_tile_id(&container.id);
fn build_tree_from_space_views_and_containers<'a>(
space_views: impl Iterator<Item = &'a SpaceViewId>,
containers: impl Iterator<Item = &'a ContainerBlueprint>,
root_container: Option<ContainerId>,
) -> egui_tiles::Tree<SpaceViewId> {
re_tracing::profile_function!();
let mut tree = egui_tiles::Tree::empty("viewport_tree");

// First add all the space_views
for space_view in space_views {
let tile_id = blueprint_id_to_tile_id(space_view);
let pane = egui_tiles::Tile::Pane(*space_view);
tree.tiles.insert(tile_id, pane);
}

tree.tiles.insert(tile_id, container.to_tile());
}
// Now add all the containers
for container in containers {
let tile_id = blueprint_id_to_tile_id(&container.id);

// And finally, set the root
if let Some(root_container) = self.root_container.map(|id| blueprint_id_to_tile_id(&id)) {
tree.root = Some(root_container);
}
tree.tiles.insert(tile_id, container.to_tile());
}

tree
// And finally, set the root
if let Some(root_container) = root_container.map(|id| blueprint_id_to_tile_id(&id)) {
tree.root = Some(root_container);
}

tree
}
Loading