Skip to content

Commit

Permalink
Remove legacy SpaceInfo (#4937)
Browse files Browse the repository at this point in the history
### What

* Fixes #4388
   * this is the last piece 🥳  
* Related to
   * #4935
   * #4826

`SpaceInfo` used to be used to determine space connectivity, this is by
now done by `SpatialTopology`

Not a "pure" refactor: Slight changes in behavior
* Adding a space view via the 'plus' top left uses the default-created
list, not the now removed "all" list (which was poorly defined)
* Similarly, suggestions for root now use the default-created list
* adding entities via the add-ui no longer uses any kind of transform
analysis (it relied on SpaceInfo)
   * Does not fix  #4826
* .. since it still won't allow adding things that aren't visualizable
at all



### 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)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4937/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4937/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4937/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4937)
- [Docs
preview](https://rerun.io/preview/d27d81f094e026a2cecfd42ac25dc133d9ed26f6/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/d27d81f094e026a2cecfd42ac25dc133d9ed26f6/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
Wumpf authored Jan 30, 2024
1 parent 74d3a0f commit fb8d5d9
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 544 deletions.
2 changes: 0 additions & 2 deletions crates/re_space_view/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ mod data_query;
mod data_query_blueprint;
mod heuristics;
mod screenshot;
mod unreachable_transform_reason;

pub use data_query::{DataQuery, EntityOverrideContext, PropertyResolver};
pub use data_query_blueprint::DataQueryBlueprint;
pub use heuristics::suggest_space_view_for_each_entity;
pub use screenshot::ScreenshotMode;
pub use unreachable_transform_reason::UnreachableTransformReason;

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

Expand Down
36 changes: 0 additions & 36 deletions crates/re_space_view/src/unreachable_transform_reason.rs

This file was deleted.

18 changes: 9 additions & 9 deletions crates/re_space_view_spatial/src/contexts/transform_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use nohash_hasher::IntMap;

use re_data_store::LatestAtQuery;
use re_entity_db::{EntityPath, EntityPropertyMap, EntityTree};
use re_space_view::UnreachableTransformReason;
use re_types::{
components::{DisconnectedSpace, PinholeProjection, Transform3D, ViewCoordinates},
ComponentNameSet, Loggable as _,
Expand All @@ -26,6 +25,15 @@ struct TransformInfo {
pub parent_pinhole: Option<EntityPath>,
}

#[derive(Clone, Copy)]
enum UnreachableTransformReason {
/// More than one pinhole camera between this and the reference space.
NestedPinholeCameras,

/// Unknown transform between this and the reference space.
DisconnectedSpace,
}

/// Provides transforms from an entity to a chosen reference space for all elements in the scene
/// for the currently selected time & timeline.
///
Expand Down Expand Up @@ -285,14 +293,6 @@ impl TransformContext {
.get(ent_path)
.and_then(|i| i.parent_pinhole.as_ref())
}

// This method isn't currently implemented, but we might need it in the future.
// All the necessary data on why a subtree isn't reachable is already stored.
//
// Returns why (if actually) a path isn't reachable.
// pub fn unreachable_reason(&self, _entity_path: &EntityPath) -> Option<UnreachableTransformReason> {
// None
// }
}

fn transform_at(
Expand Down
10 changes: 2 additions & 8 deletions crates/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ use re_viewer_context::{
ComponentUiRegistry, PlayState, RecordingConfig, SpaceViewClassRegistry, StoreContext,
SystemCommandSender as _, ViewerContext,
};
use re_viewport::{
determine_visualizable_entities, SpaceInfoCollection, Viewport, ViewportBlueprint,
ViewportState,
};
use re_viewport::{determine_visualizable_entities, Viewport, ViewportBlueprint, ViewportState};

use crate::ui::recordings_panel_ui;
use crate::{app_blueprint::AppBlueprint, store_hub::StoreHub, ui::blueprint_panel_ui};
Expand Down Expand Up @@ -237,8 +234,6 @@ impl AppState {
// First update the viewport and thus all active space views.
// This may update their heuristics, so that all panels that are shown in this frame,
// have the latest information.
let spaces_info = SpaceInfoCollection::new(ctx.entity_db);

viewport.on_frame_start(&ctx);

{
Expand Down Expand Up @@ -290,7 +285,6 @@ impl AppState {
&ctx,
ui,
&mut viewport,
&spaces_info,
app_blueprint.selection_panel_expanded,
);

Expand Down Expand Up @@ -331,7 +325,7 @@ impl AppState {
ui.add_space(4.0);
}

blueprint_panel_ui(&mut viewport, &ctx, ui, &spaces_info);
blueprint_panel_ui(&mut viewport, &ctx, ui);
},
);

Expand Down
5 changes: 2 additions & 3 deletions crates/re_viewer/src/ui/blueprint_panel.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
use re_viewer_context::{SystemCommandSender as _, ViewerContext};
use re_viewport::{SpaceInfoCollection, Viewport};
use re_viewport::Viewport;

/// Show the Blueprint section of the left panel based on the current [`Viewport`]
pub fn blueprint_panel_ui(
viewport: &mut Viewport<'_, '_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
spaces_info: &SpaceInfoCollection,
) {
ctx.re_ui.panel_content(ui, |_, ui| {
ctx.re_ui.panel_title_bar_with_buttons(
ui,
"Blueprint",
Some("The Blueprint is where you can configure the Rerun Viewer"),
|ui| {
viewport.add_new_spaceview_button_ui(ctx, ui, spaces_info);
viewport.add_new_spaceview_button_ui(ctx, ui);
reset_blueprint_button_ui(ctx, ui);
},
);
Expand Down
20 changes: 4 additions & 16 deletions crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use re_viewer_context::{
};
use re_viewport::{
external::re_space_view::blueprint::components::QueryExpressions, icon_for_container_kind,
Contents, SpaceInfoCollection, Viewport, ViewportBlueprint,
Contents, Viewport, ViewportBlueprint,
};

use crate::ui::visible_history::visible_history_ui;
Expand Down Expand Up @@ -74,7 +74,6 @@ impl SelectionPanel {
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
viewport: &mut Viewport<'_, '_>,
spaces_info: &SpaceInfoCollection,
expanded: bool,
) {
let screen_width = ui.ctx().screen_rect().width();
Expand Down Expand Up @@ -124,7 +123,7 @@ impl SelectionPanel {
.show(ui, |ui| {
ui.add_space(ui.spacing().item_spacing.y);
ctx.re_ui.panel_content(ui, |_, ui| {
self.contents(ctx, ui, viewport, spaces_info);
self.contents(ctx, ui, viewport);
});
});
});
Expand All @@ -136,7 +135,6 @@ impl SelectionPanel {
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
viewport: &mut Viewport<'_, '_>,
spaces_info: &SpaceInfoCollection,
) {
re_tracing::profile_function!();

Expand Down Expand Up @@ -170,13 +168,7 @@ impl SelectionPanel {
}

Item::SpaceView(space_view_id) => {
space_view_top_level_properties(
ui,
ctx,
viewport.blueprint,
spaces_info,
space_view_id,
);
space_view_top_level_properties(ui, ctx, viewport.blueprint, space_view_id);
}

_ => {}
Expand Down Expand Up @@ -550,7 +542,6 @@ fn space_view_top_level_properties(
ui: &mut egui::Ui,
ctx: &ViewerContext<'_>,
viewport: &ViewportBlueprint,
spaces_info: &SpaceInfoCollection,
space_view_id: &SpaceViewId,
) {
if let Some(space_view) = viewport.space_view(space_view_id) {
Expand All @@ -574,10 +565,7 @@ fn space_view_top_level_properties(
);

super::space_view_space_origin_ui::space_view_space_origin_widget_ui(
ui,
ctx,
spaces_info,
space_view,
ui, ctx, space_view,
);

ui.end_row();
Expand Down
8 changes: 2 additions & 6 deletions crates/re_viewer/src/ui/space_view_space_origin_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use egui::{Key, Ui};

use re_ui::{ReUi, SyntaxHighlighting};
use re_viewer_context::ViewerContext;
use re_viewport::{SpaceInfoCollection, SpaceViewBlueprint};
use re_viewport::SpaceViewBlueprint;

/// State of the space origin widget.
#[derive(Default, Clone)]
Expand All @@ -27,7 +27,6 @@ enum SpaceOriginEditState {
pub(crate) fn space_view_space_origin_widget_ui(
ui: &mut Ui,
ctx: &ViewerContext<'_>,
spaces_info: &SpaceInfoCollection,
space_view: &SpaceViewBlueprint,
) {
let is_editing_id = ui.make_persistent_id(space_view.id.hash());
Expand Down Expand Up @@ -55,7 +54,6 @@ pub(crate) fn space_view_space_origin_widget_ui(
let keep_editing = space_view_space_origin_widget_editing_ui(
ui,
ctx,
spaces_info,
origin_string,
*entered_editing,
space_view,
Expand All @@ -77,7 +75,6 @@ pub(crate) fn space_view_space_origin_widget_ui(
fn space_view_space_origin_widget_editing_ui(
ui: &mut Ui,
ctx: &ViewerContext<'_>,
spaces_info: &SpaceInfoCollection,
space_origin_string: &mut String,
entered_editing: bool,
space_view: &SpaceViewBlueprint,
Expand All @@ -92,9 +89,8 @@ fn space_view_space_origin_widget_editing_ui(
// All suggestions for this class of space views.
// TODO(#4895): we should have/use a much simpler heuristic API to get a list of compatible entity sub-tree
let space_view_suggestions =
re_viewport::space_view_heuristics::all_possible_space_views(ctx, spaces_info)
re_viewport::space_view_heuristics::default_created_space_views(ctx)
.into_iter()
.map(|(space_view, _)| space_view)
.filter(|this_space_view| {
this_space_view.class_identifier() == space_view.class_identifier()
})
Expand Down
24 changes: 0 additions & 24 deletions crates/re_viewport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ pub const VIEWPORT_PATH: &str = "viewport";

mod auto_layout;
mod container;
mod space_info;
mod space_view;
mod space_view_entity_picker;
pub mod space_view_heuristics;
Expand All @@ -24,7 +23,6 @@ mod viewport_blueprint_ui;
pub mod blueprint;

pub use container::{ContainerBlueprint, Contents};
pub use space_info::SpaceInfoCollection;
pub use space_view::{SpaceViewBlueprint, SpaceViewName};
pub use viewport::{Viewport, ViewportState};
pub use viewport_blueprint::ViewportBlueprint;
Expand All @@ -41,28 +39,6 @@ use re_viewer_context::{
ApplicableEntities, DynSpaceViewClass, PerVisualizer, VisualizableEntities,
};

/// Utility for querying a pinhole archetype instance.
///
/// TODO(andreas): It should be possible to convert `re_query::ArchetypeView` to its corresponding Archetype for situations like this.
/// TODO(andreas): This is duplicated into `re_space_view_spatial`
fn query_pinhole(
store: &re_data_store::DataStore,
query: &re_data_store::LatestAtQuery,
entity_path: &re_log_types::EntityPath,
) -> Option<re_types::archetypes::Pinhole> {
store
.query_latest_component(entity_path, query)
.map(|image_from_camera| re_types::archetypes::Pinhole {
image_from_camera: image_from_camera.value,
resolution: store
.query_latest_component(entity_path, query)
.map(|c| c.value),
camera_xyz: store
.query_latest_component(entity_path, query)
.map(|c| c.value),
})
}

/// Determines the set of visible entities for a given space view.
// TODO(andreas): This should be part of the SpaceView's (non-blueprint) state.
// Updated whenever `applicable_entities_per_visualizer` or the space view blueprint changes.
Expand Down
Loading

0 comments on commit fb8d5d9

Please sign in to comment.