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

Fix infinite recursion when putting a container inside a viewer tab #3534

Merged
merged 4 commits into from
Sep 29, 2023
Merged

Fix infinite recursion when putting a container inside a viewer tab #3534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3af725e
Add "callstack" feature to egui. Hold all modifier keys to see callstack
emilk Sep 29, 2023
dfd8d0b
Fix infinite recursion when putting a container inside a viewer tab
emilk Sep 29, 2023
1db3157
Consistent error text
emilk Sep 29, 2023
918148c
Clicking a group tab deselects
emilk Sep 29, 2023
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

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

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ eframe = { version = "0.23.0", default-features = false, features = [
"x11",
] }
egui = { version = "0.23.0", features = [
"callstack",
"extra_debug_asserts",
"log",
"puffin",
Expand Down
101 changes: 61 additions & 40 deletions crates/re_viewport/src/viewport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use std::collections::BTreeMap;

use ahash::HashMap;
use egui_tiles::Behavior;
use egui_tiles::Behavior as _;
use nohash_hasher::IntMap;

use re_ui::{Icon, ReUi};
Expand Down Expand Up @@ -242,6 +242,10 @@ impl<'a, 'b> Viewport<'a, 'b> {

// ----------------------------------------------------------------------------

/// `egui_tiles` has _tiles_ which are either _containers_ or _panes_.
///
/// In our case, each pane is a space view,
/// while containers are just groups of things.
struct TabViewer<'a, 'b> {
viewport_state: &'a mut ViewportState,
ctx: &'a mut ViewerContext<'b>,
Expand Down Expand Up @@ -285,13 +289,33 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
}

fn tab_title_for_pane(&mut self, space_view_id: &SpaceViewId) -> egui::WidgetText {
let Some(space_view) = self.space_views.get_mut(space_view_id) else {
// this shouldn't happen unless we have a bug
re_log::debug_once!("SpaceViewId missing during egui_tiles");
return "internal_error".into();
};
if let Some(space_view) = self.space_views.get_mut(space_view_id) {
space_view.display_name.clone().into()
} else {
// All panes are space views, so this shouldn't happen unless we have a bug
re_log::warn_once!("SpaceViewId missing during egui_tiles");
self.ctx.re_ui.error_text("Internal error").into()
}
}

egui::WidgetText::RichText(egui::RichText::new(space_view.display_name.clone()))
fn tab_title_for_tile(
&mut self,
tiles: &egui_tiles::Tiles<SpaceViewId>,
tile_id: egui_tiles::TileId,
) -> egui::WidgetText {
if let Some(tile) = tiles.get(tile_id) {
match tile {
egui_tiles::Tile::Pane(pane) => self.tab_title_for_pane(pane),

// E.g. a tab with a grid of other tiles
egui_tiles::Tile::Container(container) => {
format!("{:?} Container", container.kind()).into()
}
}
} else {
re_log::warn_once!("SpaceViewId missing during tab_title_for_tile");
self.ctx.re_ui.error_text("Internal error").into()
}
}

#[allow(clippy::fn_params_excessive_bools)]
Expand All @@ -304,23 +328,13 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
active: bool,
is_being_dragged: bool,
) -> egui::Response {
let Some(tab_widget) = TabWidget::new(self, ui, tiles, tile_id, active, 1.0) else {
return egui_tiles::Behavior::<SpaceViewId>::tab_ui(
self,
tiles,
ui,
id,
tile_id,
active,
is_being_dragged,
);
};
let tab_widget = TabWidget::new(self, ui, tiles, tile_id, active, 1.0);

let response = ui.interact(tab_widget.rect, id, egui::Sense::click_and_drag());

// Show a gap when dragged
if ui.is_rect_visible(tab_widget.rect) && !is_being_dragged {
tab_widget.ui(ui);
tab_widget.paint(ui);
}

self.on_tab_button(tiles, tile_id, &response);
Expand All @@ -334,9 +348,7 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
ui: &mut egui::Ui,
tile_id: egui_tiles::TileId,
) {
let Some(tab_widget) = TabWidget::new(self, ui, tiles, tile_id, true, 0.5) else {
return egui_tiles::Behavior::<SpaceViewId>::drag_ui(self, tiles, ui, tile_id);
};
let tab_widget = TabWidget::new(self, ui, tiles, tile_id, true, 0.5);

let frame = egui::Frame {
inner_margin: egui::Margin::same(0.),
Expand All @@ -348,7 +360,7 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
};

frame.show(ui, |ui| {
tab_widget.ui(ui);
tab_widget.paint(ui);
});
}

Expand All @@ -362,6 +374,10 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
if let Some(egui_tiles::Tile::Pane(space_view_id)) = tiles.get(tile_id) {
self.ctx
.set_single_selection(&Item::SpaceView(*space_view_id));
} else {
// Clicked a group tab - we don't support selecting that yet,
// so deselect whatever was selected to make it less confusing:
self.ctx.rec_cfg.selection_state.clear_current();
}
}
}
Expand Down Expand Up @@ -480,6 +496,11 @@ fn space_view_ui(
space_view_blueprint.scene_ui(space_view_state, ctx, ui, latest_at, space_view_highlights);
}

/// A tab button for a tab in the viewport.
///
/// The tab can contain any `egui_tiles::Tile`,
/// which is either a Pane with a Space View, or a Container,
/// e.g. a grid of tiles.
struct TabWidget {
galley: egui::widget_text::WidgetTextGalley,
rect: egui::Rect,
Expand All @@ -499,24 +520,28 @@ impl TabWidget {
tile_id: egui_tiles::TileId,
active: bool,
gamma: f32,
) -> Option<Self> {
// make sure we have a space view to work with
) -> Self {
// Not all tabs are for tiles (space views) - some are for containers (e.g. a grid of space views).
let space_view = if let Some(egui_tiles::Tile::Pane(space_view_id)) = tiles.get(tile_id) {
tab_viewer.space_views.get(space_view_id)
} else {
return None;
};
let Some(space_view) = space_view else {
return None;
None
};
let space_view_id = space_view.id;
let selected = space_view.map_or(false, |space_view| {
tab_viewer
.ctx
.selection()
.contains(&Item::SpaceView(space_view.id))
});

// tab icon
let icon_size = ReUi::small_icon_size();
let icon_width_plus_padding = icon_size.x + ReUi::text_to_icon_padding();
let icon = space_view
.class(tab_viewer.ctx.space_view_class_registry)
.icon();
let icon = space_view.map_or(&re_ui::icons::CONTAINER, |space_view| {
space_view
.class(tab_viewer.ctx.space_view_class_registry)
.icon()
});

// tab title
let text = tab_viewer.tab_title_for_tile(tiles, tile_id);
Expand All @@ -537,10 +562,6 @@ impl TabWidget {
icon_size,
);

let selected = tab_viewer
.ctx
.selection()
.contains(&Item::SpaceView(space_view_id));
let bg_color = if selected {
ui.visuals().selection.bg_fill
} else {
Expand All @@ -551,7 +572,7 @@ impl TabWidget {
.tab_text_color(ui.visuals(), tiles, tile_id, active)
.gamma_multiply(gamma);

Some(Self {
Self {
galley,
rect,
galley_rect,
Expand All @@ -560,10 +581,10 @@ impl TabWidget {
icon_rect,
bg_color,
text_color,
})
}
}

fn ui(self, ui: &mut egui::Ui) {
fn paint(self, ui: &mut egui::Ui) {
ui.painter()
.rect(self.rect, 0.0, self.bg_color, egui::Stroke::NONE);

Expand Down
Loading