Skip to content

Commit

Permalink
Refactor: Swap the naming of Viewport and ViewportBlueprint (#2595)
Browse files Browse the repository at this point in the history
### What
In the process of plumbing new State through for things like
auto-properties I found myself getting very mixed up with the
organization and naming related to Viewport and Blueprint.

Previously ViewportBlueprint contained an internal "Viewport". This
flips the naming to make Viewport the the top-level container, with
separate internals members for `blueprint` and `state`:
```
pub struct Viewport<'a, 'b> {
    pub blueprint: ViewportBlueprint<'a>,
    pub state: &'b mut ViewportState,
}

...

let mut viewport = Viewport::from_db(store_context.blueprint, viewport_state);
```

The idea is the *Blueprint* represents the entity stored in the databse,
while the *Viewport* represents the thing shown in the viewer, which
users the blueprint as part of its display logic.

I also split the ui related functions for the blueprint into a separate
`viewport_blueprint_ui.rs`.

Sadly, this diff ended up quite ugly since I both swapped the names of
the structs and the corresponding files.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] ~~I've included a screenshot or gif (if applicable)~~
Refactor-only
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2595) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2595)
- [Docs
preview](https://rerun.io/preview/pr%3Ajleibs%2Fmore_viewport_refactor/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Ajleibs%2Fmore_viewport_refactor/examples)
  • Loading branch information
jleibs authored Jul 4, 2023
1 parent f7be901 commit 0669703
Show file tree
Hide file tree
Showing 8 changed files with 834 additions and 793 deletions.
23 changes: 11 additions & 12 deletions crates/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use re_viewer_context::{
AppOptions, Caches, CommandSender, ComponentUiRegistry, PlayState, RecordingConfig,
SpaceViewClassRegistry, StoreContext, ViewerContext,
};
use re_viewport::{SpaceInfoCollection, ViewportBlueprint, ViewportState};
use re_viewport::{SpaceInfoCollection, Viewport, ViewportState};

use crate::{app_blueprint::AppBlueprint, store_hub::StoreHub, ui::blueprint_panel_ui};

Expand Down Expand Up @@ -83,8 +83,6 @@ impl AppState {
) {
re_tracing::profile_function!();

let mut blueprint = ViewportBlueprint::from_db(store_context.blueprint);

let Self {
app_options,
cache,
Expand All @@ -94,14 +92,16 @@ impl AppState {
viewport_state,
} = self;

let mut viewport = Viewport::from_db(store_context.blueprint, viewport_state);

recording_config_entry(
recording_configs,
store_db.store_id().clone(),
rx.source(),
store_db,
)
.selection_state
.on_frame_start(|item| blueprint.is_item_valid(item));
.on_frame_start(|item| viewport.is_item_valid(item));

let rec_cfg = recording_config_entry(
recording_configs,
Expand All @@ -125,10 +125,9 @@ impl AppState {

time_panel.show_panel(&mut ctx, ui, app_blueprint.time_panel_expanded);
selection_panel.show_panel(
viewport_state,
&mut ctx,
ui,
&mut blueprint,
&mut viewport,
app_blueprint.selection_panel_expanded,
);

Expand All @@ -143,10 +142,10 @@ impl AppState {
.show_inside(ui, |ui| {
let spaces_info = SpaceInfoCollection::new(&ctx.store_db.entity_db);

blueprint.viewport.on_frame_start(&mut ctx, &spaces_info);
viewport.on_frame_start(&mut ctx, &spaces_info);

blueprint_panel_ui(
&mut blueprint,
&mut viewport.blueprint,
&mut ctx,
ui,
&spaces_info,
Expand All @@ -161,16 +160,16 @@ impl AppState {
egui::CentralPanel::default()
.frame(viewport_frame)
.show_inside(ui, |ui| {
blueprint.viewport.viewport_ui(viewport_state, ui, &mut ctx);
viewport.viewport_ui(ui, &mut ctx);
});

// If the viewport was user-edited, then disable auto space views
if blueprint.viewport.has_been_user_edited {
blueprint.viewport.auto_space_views = false;
if viewport.blueprint.has_been_user_edited {
viewport.blueprint.auto_space_views = false;
}
});

blueprint.sync_changes(command_sender);
viewport.sync_blueprint_changes(command_sender);

{
// We move the time at the very end of the frame,
Expand Down
10 changes: 4 additions & 6 deletions crates/re_viewer/src/ui/blueprint_panel.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use re_viewer_context::ViewerContext;
use re_viewport::{SpaceInfoCollection, Viewport, ViewportBlueprint};
use re_viewport::{SpaceInfoCollection, ViewportBlueprint};

/// Show the left-handle panel based on the current [`ViewportBlueprint`]
pub fn blueprint_panel_ui(
Expand Down Expand Up @@ -28,7 +28,7 @@ pub fn blueprint_panel_ui(
..Default::default()
}
.show(ui, |ui| {
blueprint.viewport.tree_ui(ctx, ui);
blueprint.tree_ui(ctx, ui);
});
});
}
Expand All @@ -54,9 +54,7 @@ fn title_bar_ui(
ui.available_size_before_wrap(),
egui::Layout::right_to_left(egui::Align::Center),
|ui| {
blueprint
.viewport
.add_new_spaceview_button_ui(ctx, ui, spaces_info);
blueprint.add_new_spaceview_button_ui(ctx, ui, spaces_info);
reset_button_ui(blueprint, ctx, ui, spaces_info);
},
);
Expand All @@ -76,6 +74,6 @@ fn reset_button_ui(
.on_hover_text("Re-populate Viewport with automatically chosen Space Views")
.clicked()
{
blueprint.viewport = Viewport::new(ctx, spaces_info);
blueprint.reset(ctx, spaces_info);
}
}
4 changes: 2 additions & 2 deletions crates/re_viewer/src/ui/selection_history_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,15 @@ fn item_collection_to_string(blueprint: &ViewportBlueprint<'_>, items: &ItemColl
fn item_to_string(blueprint: &ViewportBlueprint<'_>, item: &Item) -> String {
match item {
Item::SpaceView(sid) => {
if let Some(space_view) = blueprint.viewport.space_view(sid) {
if let Some(space_view) = blueprint.space_view(sid) {
space_view.display_name.clone()
} else {
"<removed space view>".to_owned()
}
}
Item::InstancePath(_, entity_path) => entity_path.to_string(),
Item::DataBlueprintGroup(sid, handle) => {
if let Some(space_view) = blueprint.viewport.space_view(sid) {
if let Some(space_view) = blueprint.space_view(sid) {
if let Some(group) = space_view.data_blueprint.group(*handle) {
group.display_name.clone()
} else {
Expand Down
57 changes: 31 additions & 26 deletions crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use re_data_store::{ColorMapper, Colormap, EditableAutoValue, EntityPath, Entity
use re_data_ui::{item_ui, DataUi};
use re_log_types::TimeType;
use re_viewer_context::{Item, SpaceViewId, UiVerbosity, ViewerContext};
use re_viewport::{Viewport, ViewportBlueprint, ViewportState};
use re_viewport::{Viewport, ViewportBlueprint};

use super::selection_history_ui::SelectionHistoryUi;

Expand All @@ -21,10 +21,9 @@ pub(crate) struct SelectionPanel {
impl SelectionPanel {
pub fn show_panel(
&mut self,
viewport_state: &mut ViewportState,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
blueprint: &mut ViewportBlueprint<'_>,
viewport: &mut Viewport<'_, '_>,
expanded: bool,
) {
let screen_width = ui.ctx().screen_rect().width();
Expand All @@ -50,7 +49,7 @@ impl SelectionPanel {
if let Some(selection) = self.selection_state_ui.selection_ui(
ctx.re_ui,
ui,
blueprint,
&viewport.blueprint,
&mut ctx.selection_state_mut().history,
) {
ctx.selection_state_mut()
Expand All @@ -66,7 +65,7 @@ impl SelectionPanel {
..Default::default()
}
.show(ui, |ui| {
self.contents(viewport_state, ctx, ui, blueprint);
self.contents(ctx, ui, viewport);
});
});
});
Expand All @@ -75,10 +74,9 @@ impl SelectionPanel {
#[allow(clippy::unused_self)]
fn contents(
&mut self,
viewport_state: &mut ViewportState,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
blueprint: &mut ViewportBlueprint<'_>,
viewport: &mut Viewport<'_, '_>,
) {
re_tracing::profile_function!();

Expand All @@ -92,7 +90,7 @@ impl SelectionPanel {
let selection = ctx.selection().to_vec();
for (i, item) in selection.iter().enumerate() {
ui.push_id(i, |ui| {
what_is_selected_ui(ui, ctx, &mut blueprint.viewport, item);
what_is_selected_ui(ui, ctx, &mut viewport.blueprint, item);

if has_data_section(item) {
ctx.re_ui.large_collapsing_header(ui, "Data", true, |ui| {
Expand All @@ -102,7 +100,7 @@ impl SelectionPanel {

ctx.re_ui
.large_collapsing_header(ui, "Blueprint", true, |ui| {
blueprint_ui(viewport_state, ui, ctx, blueprint, item);
blueprint_ui(ui, ctx, viewport, item);
});

if i + 1 < num_selections {
Expand All @@ -126,7 +124,7 @@ fn has_data_section(item: &Item) -> bool {
fn what_is_selected_ui(
ui: &mut egui::Ui,
ctx: &mut ViewerContext<'_>,
viewport: &mut Viewport,
viewport: &mut ViewportBlueprint<'_>,
item: &Item,
) {
match item {
Expand Down Expand Up @@ -205,15 +203,19 @@ fn what_is_selected_ui(

/// What is the blueprint stuff for this item?
fn blueprint_ui(
viewport_state: &mut ViewportState,
ui: &mut egui::Ui,
ctx: &mut ViewerContext<'_>,
blueprint: &mut ViewportBlueprint<'_>,
viewport: &mut Viewport<'_, '_>,
item: &Item,
) {
match item {
Item::ComponentPath(component_path) => {
list_existing_data_blueprints(ui, ctx, component_path.entity_path(), blueprint);
list_existing_data_blueprints(
ui,
ctx,
component_path.entity_path(),
&viewport.blueprint,
);
}

Item::SpaceView(space_view_id) => {
Expand All @@ -223,7 +225,7 @@ fn blueprint_ui(
.on_hover_text("Manually add or remove entities from the Space View.")
.clicked()
{
viewport_state
viewport
.show_add_remove_entities_window(*space_view_id);
}

Expand All @@ -232,19 +234,19 @@ fn blueprint_ui(
.on_hover_text("Create an exact duplicate of this Space View including all blueprint settings")
.clicked()
{
if let Some(space_view) = blueprint.viewport.space_view(space_view_id) {
if let Some(space_view) = viewport.blueprint.space_view(space_view_id) {
let mut new_space_view = space_view.clone();
new_space_view.id = SpaceViewId::random();
blueprint.viewport.add_space_view(new_space_view);
blueprint.viewport.mark_user_interaction();
viewport.blueprint.add_space_view(new_space_view);
viewport.blueprint.mark_user_interaction();
}
}
});

ui.add_space(ui.spacing().item_spacing.y);

if let Some(space_view) = blueprint.viewport.space_view_mut(space_view_id) {
let space_view_state = viewport_state.space_view_state_mut(
if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) {
let space_view_state = viewport.state.space_view_state_mut(
ctx.space_view_class_registry,
space_view.id,
space_view.class_name(),
Expand All @@ -264,7 +266,7 @@ fn blueprint_ui(

Item::InstancePath(space_view_id, instance_path) => {
if let Some(space_view_id) = space_view_id {
if let Some(space_view) = blueprint.viewport.space_view_mut(space_view_id) {
if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) {
if instance_path.instance_key.is_specific() {
ui.horizontal(|ui| {
ui.label("Part of");
Expand All @@ -285,12 +287,17 @@ fn blueprint_ui(
}
}
} else {
list_existing_data_blueprints(ui, ctx, &instance_path.entity_path, blueprint);
list_existing_data_blueprints(
ui,
ctx,
&instance_path.entity_path,
&viewport.blueprint,
);
}
}

Item::DataBlueprintGroup(space_view_id, data_blueprint_group_handle) => {
if let Some(space_view) = blueprint.viewport.space_view_mut(space_view_id) {
if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) {
if let Some(group) = space_view
.data_blueprint
.group_mut(*data_blueprint_group_handle)
Expand All @@ -310,9 +317,7 @@ fn list_existing_data_blueprints(
entity_path: &EntityPath,
blueprint: &ViewportBlueprint<'_>,
) {
let space_views_with_path = blueprint
.viewport
.space_views_containing_entity_path(entity_path);
let space_views_with_path = blueprint.space_views_containing_entity_path(entity_path);

if space_views_with_path.is_empty() {
ui.weak("(Not shown in any Space View)");
Expand All @@ -322,7 +327,7 @@ fn list_existing_data_blueprints(

ui.indent("list of data blueprints indent", |ui| {
for space_view_id in &space_views_with_path {
if let Some(space_view) = blueprint.viewport.space_view(space_view_id) {
if let Some(space_view) = blueprint.space_view(space_view_id) {
item_ui::entity_path_button_to(
ctx,
ui,
Expand Down
1 change: 1 addition & 0 deletions crates/re_viewport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ mod space_view_highlights;
mod view_category;
mod viewport;
mod viewport_blueprint;
mod viewport_blueprint_ui;

pub mod blueprint_components;

Expand Down
Loading

0 comments on commit 0669703

Please sign in to comment.