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.

Sign up for GitHub

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

Jump to bottom

Enforce root layout container to be always present #5856

Merged
merged 2 commits into from
Oct 29, 2024
Merged

Enforce root layout container to be always present #5856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79bcb70
Enforce root layout container to be always present
Wumpf Apr 8, 2024
468aad1
fix creating a non-empty tile tree when there's no root
Wumpf Oct 23, 2024
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
17 changes: 4 additions & 13 deletions crates/viewer/re_blueprint_tree/src/blueprint_tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,7 @@ impl BlueprintTree {
blueprint: &ViewportBlueprint,
ui: &mut egui::Ui,
) {
let Some(container_id) = blueprint.root_container else {
// nothing to draw if there is no root container
return;
};
let container_id = blueprint.root_container;

let Some(container_blueprint) = blueprint.containers.get(&container_id) else {
re_log::warn_once!("Cannot find root container {container_id}");
Expand Down Expand Up @@ -655,14 +652,12 @@ impl BlueprintTree {
// root container.
let target_container_id =
if let Some(Item::Container(container_id)) = ctx.selection().single_item() {
Some(*container_id)
*container_id
} else {
blueprint.root_container
};

if let Some(target_container_id) = target_container_id {
show_add_space_view_or_container_modal(target_container_id);
}
show_add_space_view_or_container_modal(target_container_id);
}
}

Expand Down Expand Up @@ -821,15 +816,11 @@ impl BlueprintTree {
// TODO(ab): this is a rather primitive behavior. Ideally we should allow dropping in the last container based
// on the horizontal position of the cursor.

let Some(root_container_id) = blueprint.root_container else {
return;
};

if ui.rect_contains_pointer(empty_space) {
let drop_target = re_ui::drag_and_drop::DropTarget::new(
empty_space.x_range(),
empty_space.top(),
Contents::Container(root_container_id),
Contents::Container(blueprint.root_container),
usize::MAX,
);

Expand Down
43 changes: 19 additions & 24 deletions crates/viewer/re_context_menu/src/actions/move_contents_to_new_container.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ impl ContextMenuAction for MoveContentsToNewContainerAction {

ctx.selection.iter().all(|(item, _)| match item {
Item::SpaceView(_) => true,
Item::Container(container_id) => {
ctx.viewport_blueprint.root_container != Some(*container_id)
}
Item::Container(container_id) => ctx.viewport_blueprint.root_container != *container_id,
_ => false,
})
}
Expand All @@ -36,9 +34,7 @@ impl ContextMenuAction for MoveContentsToNewContainerAction {
fn supports_item(&self, ctx: &ContextMenuContext<'_>, item: &Item) -> bool {
match item {
Item::SpaceView(_) => true,
Item::Container(container_id) => {
ctx.viewport_blueprint.root_container != Some(*container_id)
}
Item::Container(container_id) => ctx.viewport_blueprint.root_container != *container_id,
_ => false,
}
}
Expand All @@ -57,26 +53,25 @@ impl ContextMenuAction for MoveContentsToNewContainerAction {
}

fn process_selection(&self, ctx: &ContextMenuContext<'_>) {
if let Some(root_container_id) = ctx.viewport_blueprint.root_container {
let (target_container_id, target_position) = ctx
.clicked_item_enclosing_container_id_and_position()
.unwrap_or((root_container_id, 0));
let root_container_id = ctx.viewport_blueprint.root_container;
let (target_container_id, target_position) = ctx
.clicked_item_enclosing_container_id_and_position()
.unwrap_or((root_container_id, 0));

let contents = ctx
.selection
.iter()
.filter_map(|(item, _)| item.try_into().ok())
.collect();
let contents = ctx
.selection
.iter()
.filter_map(|(item, _)| item.try_into().ok())
.collect();

ctx.viewport_blueprint.move_contents_to_new_container(
contents,
self.0,
target_container_id,
target_position,
);
ctx.viewport_blueprint.move_contents_to_new_container(
contents,
self.0,
target_container_id,
target_position,
);

ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
}
ctx.viewport_blueprint
.mark_user_interaction(ctx.viewer_context);
}
}
4 changes: 1 addition & 3 deletions crates/viewer/re_context_menu/src/actions/remove.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ impl ContextMenuAction for RemoveAction {
fn supports_item(&self, ctx: &ContextMenuContext<'_>, item: &Item) -> bool {
match item {
Item::SpaceView(_) => true,
Item::Container(container_id) => {
ctx.viewport_blueprint.root_container != Some(*container_id)
}
Item::Container(container_id) => ctx.viewport_blueprint.root_container != *container_id,
Item::DataResult(_, instance_path) => instance_path.is_all(),
_ => false,
}
Expand Down
4 changes: 2 additions & 2 deletions crates/viewer/re_context_menu/src/actions/show_hide.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ impl ContextMenuAction for ShowAction {
Item::Container(container_id) => {
!ctx.viewport_blueprint
.is_contents_visible(&Contents::Container(*container_id))
&& ctx.viewport_blueprint.root_container != Some(*container_id)
&& ctx.viewport_blueprint.root_container != *container_id
}
Item::DataResult(space_view_id, instance_path) => {
data_result_visible(ctx, space_view_id, instance_path).is_some_and(|vis| !vis)
Expand Down Expand Up @@ -69,7 +69,7 @@ impl ContextMenuAction for HideAction {
Item::Container(container_id) => {
ctx.viewport_blueprint
.is_contents_visible(&Contents::Container(*container_id))
&& ctx.viewport_blueprint.root_container != Some(*container_id)
&& ctx.viewport_blueprint.root_container != *container_id
}
Item::DataResult(space_view_id, instance_path) => {
data_result_visible(ctx, space_view_id, instance_path).unwrap_or(false)
Expand Down
76 changes: 33 additions & 43 deletions crates/viewer/re_viewport/src/viewport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,8 @@ impl<'a> Viewport<'a> {
);

reset = true;
} else if let Some(parent_id) =
parent_container.or(self.blueprint.root_container)
{
} else {
let parent_id = parent_container.unwrap_or(self.blueprint.root_container);
let tile_id = self.tree.tiles.insert_pane(space_view_id);
let container_tile_id = blueprint_id_to_tile_id(&parent_id);
if let Some(egui_tiles::Tile::Container(container)) =
Expand All @@ -266,31 +265,26 @@ impl<'a> Viewport<'a> {
re_log::trace!("Root was not a container - will re-run auto-layout");
reset = true;
}
} else {
re_log::trace!("No root found - will re-run auto-layout");
}

self.tree_edited = true;
}
TreeAction::AddContainer(container_kind, parent_container) => {
if let Some(parent_id) = parent_container.or(self.blueprint.root_container) {
let tile_id = self
.tree
.tiles
.insert_container(egui_tiles::Container::new(container_kind, vec![]));
if let Some(egui_tiles::Tile::Container(container)) =
self.tree.tiles.get_mut(blueprint_id_to_tile_id(&parent_id))
{
re_log::trace!("Inserting new space view into container {parent_id:?}");
container.add_child(tile_id);
} else {
re_log::trace!(
"Parent or root was not a container - will re-run auto-layout"
);
reset = true;
}
let parent_id = parent_container.unwrap_or(self.blueprint.root_container);
let tile_id = self
.tree
.tiles
.insert_container(egui_tiles::Container::new(container_kind, vec![]));
if let Some(egui_tiles::Tile::Container(container)) =
self.tree.tiles.get_mut(blueprint_id_to_tile_id(&parent_id))
{
re_log::trace!("Inserting new space view into container {parent_id:?}");
container.add_child(tile_id);
} else {
re_log::trace!("No root found - will re-run auto-layout");
re_log::trace!(
"Parent or root was not a container - will re-run auto-layout"
);
reset = true;
}

self.tree_edited = true;
Expand Down Expand Up @@ -461,7 +455,7 @@ struct TabViewer<'a, 'b> {
ctx: &'a ViewerContext<'b>,
viewport_blueprint: &'a ViewportBlueprint,
maximized: &'a mut Option<SpaceViewId>,
root_container_id: Option<ContainerId>,
root_container_id: ContainerId,
tree_action_sender: std::sync::mpsc::Sender<TreeAction>,

/// List of query & system execution results for each space view.
Expand Down Expand Up @@ -773,26 +767,22 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
// drag and drop operation often lead to many spurious empty containers. To work
// around this, we run a simplification pass when a drop occurs.

if let Some(root_container_id) = self.root_container_id {
if self
.tree_action_sender
.send(TreeAction::SimplifyContainer(
root_container_id,
egui_tiles::SimplificationOptions {
prune_empty_tabs: true,
prune_empty_containers: false,
prune_single_child_tabs: true,
prune_single_child_containers: false,
all_panes_must_have_tabs: true,
join_nested_linear_containers: false,
},
))
.is_err()
{
re_log::warn_once!(
"Channel between ViewportBlueprint and Viewport is broken"
);
}
if self
.tree_action_sender
.send(TreeAction::SimplifyContainer(
self.root_container_id,
egui_tiles::SimplificationOptions {
prune_empty_tabs: true,
prune_empty_containers: false,
prune_single_child_tabs: true,
prune_single_child_containers: false,
all_panes_must_have_tabs: true,
join_nested_linear_containers: false,
},
))
.is_err()
{
re_log::warn_once!("Channel between ViewportBlueprint and Viewport is broken");
}

self.edited = true;
Expand Down
16 changes: 16 additions & 0 deletions crates/viewer/re_viewport_blueprint/src/container.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,22 @@ pub struct ContainerBlueprint {
pub grid_columns: Option<u32>,
}

impl Default for ContainerBlueprint {
fn default() -> Self {
Self {
id: ContainerId::random(),
container_kind: egui_tiles::ContainerKind::Grid,
display_name: None,
contents: vec![],
col_shares: vec![],
row_shares: vec![],
active_tab: None,
visible: true,
grid_columns: None,
}
}
}

impl ContainerBlueprint {
/// Attempt to load a [`ContainerBlueprint`] from the blueprint store.
pub fn try_from_db(
Expand Down
43 changes: 19 additions & 24 deletions crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub struct ViewportBlueprint {
pub containers: BTreeMap<ContainerId, ContainerBlueprint>,

/// The root container.
pub root_container: Option<ContainerId>,
pub root_container: ContainerId,

/// The layouts of all the space views.
pub tree: egui_tiles::Tree<SpaceViewId>,
Expand Down Expand Up @@ -129,8 +129,6 @@ impl ViewportBlueprint {
.map(|c| (c.id, c))
.collect();

let root_container = root_container.map(|id| id.0.into());

// Auto layouting and auto space view are only enabled if no blueprint has been provided by the user.
// Only enable auto-space-views if this is the app-default blueprint
let is_app_default_blueprint = blueprint_db
Expand All @@ -144,7 +142,7 @@ impl ViewportBlueprint {
let tree = build_tree_from_space_views_and_containers(
space_views.values(),
containers.values(),
root_container,
root_container.clone(),
);

let past_viewer_recommendations = past_viewer_recommendations
Expand All @@ -153,10 +151,15 @@ impl ViewportBlueprint {
.cloned()
.collect();

let root_container_or_placeholder = root_container.clone().map_or_else(
|| ContainerId::hashed_from_str("placeholder_root_container"),
|id| id.0.into(),
);

Self {
space_views,
containers,
root_container,
root_container: root_container_or_placeholder,
tree,
maximized: maximized.map(|id| id.0.into()),
auto_layout,
Expand Down Expand Up @@ -455,9 +458,7 @@ impl ViewportBlueprint {
///
/// See [`Self::visit_contents_in_container`] for details.
pub fn visit_contents(&self, visitor: &mut impl FnMut(&Contents, &SmallVec<[ContainerId; 4]>)) {
if let Some(root_container) = self.root_container {
self.visit_contents_in_container(&root_container, visitor);
}
self.visit_contents_in_container(&self.root_container, visitor);
}

/// Walk the subtree defined by the provided container id and call `visitor` for each
Expand Down Expand Up @@ -501,11 +502,7 @@ impl ViewportBlueprint {
/// Given a predicate, finds the (first) matching contents by recursively walking from the root
/// container.
pub fn find_contents_by(&self, predicate: &impl Fn(&Contents) -> bool) -> Option<Contents> {
if let Some(root_container) = self.root_container {
self.find_contents_in_container_by(predicate, &root_container)
} else {
None
}
self.find_contents_in_container_by(predicate, &self.root_container)
}

/// Given a predicate, finds the (first) matching contents by recursively walking from the given
Expand Down Expand Up @@ -555,15 +552,11 @@ impl ViewportBlueprint {
&self,
contents: &Contents,
) -> Option<(ContainerId, usize)> {
if let Some(container_id) = self.root_container {
if *contents == Contents::Container(container_id) {
// root doesn't have a parent
return None;
}
self.find_parent_and_position_index_impl(contents, &container_id)
} else {
None
if *contents == Contents::Container(self.root_container) {
// root doesn't have a parent
return None;
}
self.find_parent_and_position_index_impl(contents, &self.root_container)
}

fn find_parent_and_position_index_impl(
Expand Down Expand Up @@ -915,7 +908,7 @@ impl ViewportBlueprint {
fn build_tree_from_space_views_and_containers<'a>(
space_views: impl Iterator<Item = &'a SpaceViewBlueprint>,
containers: impl Iterator<Item = &'a ContainerBlueprint>,
root_container: Option<ContainerId>,
root_container: Option<RootContainer>,
) -> egui_tiles::Tree<SpaceViewId> {
re_tracing::profile_function!();
let mut tree = egui_tiles::Tree::empty("viewport_tree");
Expand All @@ -937,8 +930,10 @@ fn build_tree_from_space_views_and_containers<'a>(
}

// 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);

if let Some(root_container) = root_container {
let root_container = ContainerId::from(root_container.0);
tree.root = Some(blueprint_id_to_tile_id(&root_container));
}

tree
Expand Down
Loading