Skip to content

Commit

Permalink
Remove stale SpaceViewState struct from re_viewport (#2352)
Browse files Browse the repository at this point in the history
<!--
Open the PR up as a draft until you feel it is ready for a proper
review.

Do not make PR:s from your own `main` branch, as that makes it difficult
for reviewers to add their own fixes.

Add any improvements to the branch as new commits to make it easier for
reviewers to follow the progress. All commits will be squashed to a
single commit once the PR is merged into `main`.

Make sure you mention any issues that this PR closes in the description,
as well as any other related issues.

To get an auto-generated PR description you can put "copilot:summary" or
"copilot:walkthrough" anywhere.
-->

### What

Followup to #2334: Remove old `SpaceViewState` struct from re_viewport
now that all space view states have the same trait type


### 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)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2352

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/b468b7a/docs
Examples preview: https://rerun.io/preview/b468b7a/examples
<!-- pr-link-docs:end -->
  • Loading branch information
Wumpf authored Jun 9, 2023
1 parent a52520f commit c482cac
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 51 deletions.
2 changes: 1 addition & 1 deletion crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ fn blueprint_ui(
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(
let space_view_state = viewport_state.space_view_state_mut(
ctx.space_view_class_registry,
space_view.id,
space_view.class,
Expand Down
9 changes: 3 additions & 6 deletions crates/re_viewport/src/auto_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ use itertools::Itertools as _;
use re_data_store::{EntityPath, EntityPathPart};
use re_viewer_context::{SpaceViewId, ViewerContext};

use super::{
space_view::{SpaceViewBlueprint, SpaceViewState},
view_category::ViewCategory,
};
use super::{space_view::SpaceViewBlueprint, view_category::ViewCategory};

#[derive(Clone, Debug)]
pub struct SpaceMakeInfo {
Expand Down Expand Up @@ -52,7 +49,7 @@ pub(crate) fn tree_from_space_views(
viewport_size: egui::Vec2,
visible: &std::collections::BTreeSet<SpaceViewId>,
space_views: &BTreeMap<SpaceViewId, SpaceViewBlueprint>,
space_view_states: &HashMap<SpaceViewId, SpaceViewState>,
space_view_states: &HashMap<SpaceViewId, Box<dyn re_viewer_context::SpaceViewState>>,
) -> egui_tiles::Tree<SpaceViewId> {
let mut space_make_infos = space_views
.iter()
Expand All @@ -69,7 +66,7 @@ pub(crate) fn tree_from_space_views(
let aspect_ratio = space_view_states.get(space_view_id).and_then(|state| {
ctx.space_view_class_registry
.get_or_log_error(space_view.class)
.and_then(|class| class.preferred_tile_aspect_ratio(state.state.as_ref()))
.and_then(|class| class.preferred_tile_aspect_ratio(state.as_ref()))
});

SpaceMakeInfo {
Expand Down
2 changes: 1 addition & 1 deletion crates/re_viewport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ mod viewport;
pub mod blueprint_components;

pub use space_info::SpaceInfoCollection;
pub use space_view::{SpaceViewBlueprint, SpaceViewState};
pub use space_view::SpaceViewBlueprint;
pub use view_category::ViewCategory;
pub use viewport::{Viewport, ViewportState};

Expand Down
37 changes: 9 additions & 28 deletions crates/re_viewport/src/space_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use re_arrow_store::Timeline;
use re_data_store::{EntityPath, EntityTree, TimeInt};
use re_renderer::ScreenshotProcessor;
use re_space_view::{DataBlueprintTree, ScreenshotMode};
use re_viewer_context::{SpaceViewClassName, SpaceViewHighlights, SpaceViewId, ViewerContext};
use re_viewer_context::{
SpaceViewClassName, SpaceViewHighlights, SpaceViewId, SpaceViewState, ViewerContext,
};

use crate::{
space_info::SpaceInfoCollection,
Expand Down Expand Up @@ -139,25 +141,19 @@ impl SpaceViewBlueprint {

pub fn selection_ui(
&mut self,
view_state: &mut SpaceViewState,
view_state: &mut dyn SpaceViewState,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
) {
re_tracing::profile_function!();
if let Some(space_view_class) = ctx.space_view_class_registry.get_or_log_error(self.class) {
space_view_class.selection_ui(
ctx,
ui,
view_state.state.as_mut(),
&self.space_origin,
self.id,
);
space_view_class.selection_ui(ctx, ui, view_state, &self.space_origin, self.id);
}
}

pub(crate) fn scene_ui(
&mut self,
view_state: &mut SpaceViewState,
view_state: &mut dyn SpaceViewState,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
latest_at: TimeInt,
Expand All @@ -176,7 +172,7 @@ impl SpaceViewBlueprint {

space_view_class.prepare_populate(
ctx,
view_state.state.as_mut(),
view_state,
&self.data_blueprint.entity_paths().clone(), // Clone to workaround borrow checker.
self.data_blueprint.data_blueprints_individual(),
);
Expand All @@ -190,17 +186,10 @@ impl SpaceViewBlueprint {
};

let mut scene = space_view_class.new_scene();
scene.populate(ctx, &query, view_state.state.as_ref(), highlights);
scene.populate(ctx, &query, view_state, highlights);

ui.scope(|ui| {
space_view_class.ui(
ctx,
ui,
view_state.state.as_mut(),
scene,
&self.space_origin,
self.id,
);
space_view_class.ui(ctx, ui, view_state, scene, &self.space_origin, self.id);
});
}

Expand Down Expand Up @@ -249,11 +238,3 @@ impl SpaceViewBlueprint {
}
}
}

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

/// Camera position and similar.
// TODO(wumpf): Remove this field
pub struct SpaceViewState {
pub state: Box<dyn re_viewer_context::SpaceViewState>,
}
32 changes: 17 additions & 15 deletions crates/re_viewport/src/viewport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ use re_data_ui::item_ui;
use re_space_view::DataBlueprintGroup;
use re_viewer_context::{
DataBlueprintGroupHandle, Item, SpaceViewClassName, SpaceViewClassRegistry,
SpaceViewHighlights, SpaceViewId, ViewerContext,
SpaceViewHighlights, SpaceViewId, SpaceViewState, ViewerContext,
};

use crate::{
space_info::SpaceInfoCollection,
space_view::{SpaceViewBlueprint, SpaceViewState},
space_view::SpaceViewBlueprint,
space_view_entity_picker::SpaceViewEntityPicker,
space_view_heuristics::{all_possible_space_views, default_created_space_views},
space_view_highlights::highlights_for_space_view,
Expand Down Expand Up @@ -544,32 +544,30 @@ impl Viewport {
#[derive(Default)]
pub struct ViewportState {
pub(crate) space_view_entity_window: Option<SpaceViewEntityPicker>,
space_view_states: HashMap<SpaceViewId, SpaceViewState>,
space_view_states: HashMap<SpaceViewId, Box<dyn SpaceViewState>>,
}

impl ViewportState {
pub fn show_add_remove_entities_window(&mut self, space_view_id: SpaceViewId) {
self.space_view_entity_window = Some(SpaceViewEntityPicker { space_view_id });
}

pub fn space_view_state(
pub fn space_view_state_mut(
&mut self,
space_view_class_registry: &SpaceViewClassRegistry,
space_view_id: SpaceViewId,
space_view_class: SpaceViewClassName,
) -> &mut SpaceViewState {
) -> &mut dyn SpaceViewState {
self.space_view_states
.entry(space_view_id)
.or_insert_with(|| {
let state = if let Some(state) =
space_view_class_registry.get_or_log_error(space_view_class)
{
if let Some(state) = space_view_class_registry.get_or_log_error(space_view_class) {
state.new_state()
} else {
Box::<()>::default()
};
SpaceViewState { state }
}
})
.as_mut()
}
}

Expand Down Expand Up @@ -696,7 +694,7 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
.space_views
.get_mut(space_view_id)
.expect("Should have been populated beforehand");
let space_view_state = self.viewport_state.space_view_state(
let space_view_state = self.viewport_state.space_view_state_mut(
self.ctx.space_view_class_registry,
space_view_blueprint.id,
space_view_blueprint.class,
Expand Down Expand Up @@ -762,7 +760,11 @@ impl<'a, 'b> egui_tiles::Behavior<SpaceViewId> for TabViewer<'a, 'b> {
let space_view_id = *space_view_id;

let Some(space_view) = self.space_views.get(&space_view_id) else { return; };
let Some(space_view_state) = self.viewport_state.space_view_states.get(&space_view_id) else { return; };
let space_view_state = self.viewport_state.space_view_state_mut(
self.ctx.space_view_class_registry,
space_view_id,
space_view.class,
);

let num_space_views = tiles.tiles.values().filter(|tile| tile.is_pane()).count();

Expand Down Expand Up @@ -827,12 +829,12 @@ fn help_text_ui(
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
space_view_blueprint: &SpaceViewBlueprint,
space_view_state: &SpaceViewState,
space_view_state: &dyn SpaceViewState,
) {
if let Some(help_text) = ctx
.space_view_class_registry
.get_or_log_error(space_view_blueprint.class)
.map(|class| class.help_text(ctx.re_ui, space_view_state.state.as_ref()))
.map(|class| class.help_text(ctx.re_ui, space_view_state))
{
re_ui::help_hover_button(ui).on_hover_text(help_text);
}
Expand All @@ -842,7 +844,7 @@ fn space_view_ui(
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
space_view_blueprint: &mut SpaceViewBlueprint,
space_view_state: &mut SpaceViewState,
space_view_state: &mut dyn SpaceViewState,
space_view_highlights: SpaceViewHighlights,
) {
let Some(latest_at) = ctx.rec_cfg.time_ctrl.time_int() else {
Expand Down

0 comments on commit c482cac

Please sign in to comment.