Automatic fallback for unrecognized Space View Class, start removing …

…old ViewCategory (#2357)

…

<!--
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

Add fallback "unknown" Space View Class, remove usage of old
`ViewCategory` for icons and class names. The later is what I started
out with, but I noticed that handling of missing space view classes got
both tedious and unwanted.
Instead, now when encountering a unknown space view class we always log
an error (log_once) and then return the Unknown space view class which
allows us to show information about what happened.


![image](https://github.com/rerun-io/rerun/assets/1220815/07b6e941-6974-4e33-874b-06657195e2e0)

A (still existing) drawback of the placeholder method is that we don't
get distinct class references and can't show the original name in the
help text or space view content.
For the later there's two workarounds I could come up with
* have a special space view state that contains the name
* let names themselves have an implementation of DynSpaceViewClass
both solutions are fairly hacky for such a small thing. In the future
however, we could do a blueprint query with the space view id to fill
out the text in the space view tile which would solve this problem a bit
better (still somewhat confusing since the class itself will advertise a
different name)

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

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

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/4970988/docs
Examples preview: https://rerun.io/preview/4970988/examples
<!-- pr-link-docs:end -->

crates/re_ui/src/icons.rs

@@ -85,5 +85,9 @@ pub const SPACE_VIEW_HISTOGRAM: Icon = Icon::new(
     "spaceview_histogram",
     include_bytes!("../data/icons/spaceview_histogram.png"),
 );
+pub const SPACE_VIEW_UNKNOWN: Icon = Icon::new(
+    "spaceview_unknown",
+    include_bytes!("../data/icons/spaceview_unknown.png"),
+);
 
 pub const CONTAINER: Icon = Icon::new("container", include_bytes!("../data/icons/container.png"));

crates/re_viewer/src/ui/selection_panel.rs

@@ -180,10 +180,10 @@ fn what_is_selected_ui(
             });
         }
         Item::DataBlueprintGroup(space_view_id, data_blueprint_group_handle) => {
-            if let Some(space_view) = viewport.space_view_mut(space_view_id) {
+            if let Some(space_view) = viewport.space_view(space_view_id) {
                 if let Some(group) = space_view
                     .data_blueprint
-                    .group_mut(*data_blueprint_group_handle)
+                    .group(*data_blueprint_group_handle)
                 {
                     egui::Grid::new("data_blueprint_group")
                         .num_columns(2)
@@ -199,13 +199,7 @@ fn what_is_selected_ui(
                             ui.end_row();
 
                             ui.label("in Space View:");
-                            re_viewport::item_ui::space_view_button_to(
-                                ctx,
-                                ui,
-                                space_view.display_name.clone(),
-                                space_view.id,
-                                space_view.category,
-                            );
+                            re_viewport::item_ui::space_view_button(ctx, ui, space_view);
                             ui.end_row();
                         });
                 }
@@ -258,9 +252,16 @@ fn blueprint_ui(
                 let space_view_state = viewport_state.space_view_state_mut(
                     ctx.space_view_class_registry,
                     space_view.id,
-                    space_view.class,
+                    space_view.class_name(),
+                );
+
+                space_view.class(ctx).selection_ui(
+                    ctx,
+                    ui,
+                    space_view_state,
+                    &space_view.space_origin,
+                    space_view.id,
                 );
-                space_view.selection_ui(space_view_state, ctx, ui);
             }
         }
 

crates/re_viewer_context/src/space_view/mod.rs

@@ -11,6 +11,7 @@ mod scene_context;
 mod scene_part;
 mod scene_query;
 mod space_view_class;
+mod space_view_class_placeholder;
 mod space_view_class_registry;
 
 pub use dyn_space_view_class::{

crates/re_viewer_context/src/space_view/space_view_class_placeholder.rs

@@ -0,0 +1,56 @@
+use crate::{ScenePart, ScenePartCollection, SpaceViewClass, SpaceViewClassName};
+
+/// A placeholder space view class that can be used when the actual class is not registered.
+#[derive(Default)]
+pub struct SpaceViewClassPlaceholder;
+
+impl SpaceViewClass for SpaceViewClassPlaceholder {
+    type State = ();
+    type Context = ();
+    type SceneParts = ();
+    type ScenePartData = ();
+
+    fn name(&self) -> SpaceViewClassName {
+        "Unknown Space View Class".into()
+    }
+
+    fn icon(&self) -> &'static re_ui::Icon {
+        &re_ui::icons::SPACE_VIEW_UNKNOWN
+    }
+
+    fn help_text(&self, _re_ui: &re_ui::ReUi, _state: &()) -> egui::WidgetText {
+        "The Space View Class was not recognized.\nThis happens if either the Blueprint specifies an invalid Space View Class or this version of the Viewer does not know about this type.".into()
+    }
+
+    fn selection_ui(
+        &self,
+        _ctx: &mut crate::ViewerContext<'_>,
+        _ui: &mut egui::Ui,
+        _state: &mut (),
+        _space_origin: &re_log_types::EntityPath,
+        _space_view_id: crate::SpaceViewId,
+    ) {
+    }
+
+    fn ui(
+        &self,
+        ctx: &mut crate::ViewerContext<'_>,
+        ui: &mut egui::Ui,
+        state: &mut (),
+        _scene: &mut crate::TypedScene<Self>,
+        _space_origin: &re_log_types::EntityPath,
+        _space_view_id: crate::SpaceViewId,
+    ) {
+        ui.centered_and_justified(|ui| ui.label(self.help_text(ctx.re_ui, state)));
+    }
+}
+
+impl ScenePartCollection<SpaceViewClassPlaceholder> for () {
+    fn vec_mut(&mut self) -> Vec<&mut dyn ScenePart<SpaceViewClassPlaceholder>> {
+        Vec::new()
+    }
+
+    fn as_any(&self) -> &dyn std::any::Any {
+        self
+    }
+}

crates/re_viewer_context/src/space_view/space_view_class_registry.rs

@@ -2,6 +2,8 @@ use ahash::HashMap;
 
 use crate::{DynSpaceViewClass, SpaceViewClassName};
 
+use super::space_view_class_placeholder::SpaceViewClassPlaceholder;
+
 #[derive(Debug, thiserror::Error)]
 pub enum SpaceViewClassRegistryError {
     #[error("Space view with class name {0:?} was already registered.")]
@@ -12,7 +14,10 @@ pub enum SpaceViewClassRegistryError {
 ///
 /// Expected to be populated on viewer startup.
 #[derive(Default)]
-pub struct SpaceViewClassRegistry(HashMap<SpaceViewClassName, Box<dyn DynSpaceViewClass>>);
+pub struct SpaceViewClassRegistry {
+    registry: HashMap<SpaceViewClassName, Box<dyn DynSpaceViewClass>>,
+    placeholder: SpaceViewClassPlaceholder,
+}
 
 impl SpaceViewClassRegistry {
     /// Adds a new space view type.
@@ -24,7 +29,7 @@ impl SpaceViewClassRegistry {
         let space_view_type = T::default();
         let type_name = space_view_type.name();
         if self
-            .0
+            .registry
             .insert(type_name, Box::new(space_view_type))
             .is_some()
         {
@@ -34,21 +39,22 @@ impl SpaceViewClassRegistry {
         Ok(())
     }
 
-    /// Queries a Space View type by class name.
-    fn get(&self, name: SpaceViewClassName) -> Option<&dyn DynSpaceViewClass> {
-        self.0.get(&name).map(|boxed| boxed.as_ref())
+    /// Queries a Space View type by class name, returning `None` if it is not registered.
+    fn get(&self, name: &SpaceViewClassName) -> Option<&dyn DynSpaceViewClass> {
+        self.registry.get(name).map(|boxed| boxed.as_ref())
     }
 
-    /// Queries a Space View type by class name and logs if it fails.
-    pub fn get_or_log_error(&self, name: SpaceViewClassName) -> Option<&dyn DynSpaceViewClass> {
-        let result = self.get(name);
-        if result.is_none() {
+    /// Queries a Space View type by class name and logs if it fails, returning a placeholder class.
+    pub fn get_or_log_error(&self, name: &SpaceViewClassName) -> &dyn DynSpaceViewClass {
+        if let Some(result) = self.get(name) {
+            result
+        } else {
             re_log::error_once!("Unknown space view class {:?}", name);
+            &self.placeholder
         }
-        result
     }
 
     pub fn iter(&self) -> impl Iterator<Item = &dyn DynSpaceViewClass> {
-        self.0.values().map(|boxed| boxed.as_ref())
+        self.registry.values().map(|boxed| boxed.as_ref())
     }
 }

crates/re_viewport/src/auto_layout.rs

@@ -64,9 +64,9 @@ pub(crate) fn tree_from_space_views(
         })
         .map(|(space_view_id, space_view)| {
             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.as_ref()))
+                space_view
+                    .class(ctx)
+                    .preferred_tile_aspect_ratio(state.as_ref())
             });
 
             SpaceMakeInfo {

crates/re_viewport/src/lib.rs

@@ -27,37 +27,26 @@ pub mod external {
 // TODO(andreas): This should be part of re_data_ui::item_ui.
 pub mod item_ui {
     use re_data_ui::item_ui;
-    use re_viewer_context::{Item, SpaceViewId, ViewerContext};
+    use re_viewer_context::{Item, ViewerContext};
 
-    use crate::{space_view::SpaceViewBlueprint, view_category::ViewCategory};
+    use crate::space_view::SpaceViewBlueprint;
 
     pub fn space_view_button(
         ctx: &mut ViewerContext<'_>,
         ui: &mut egui::Ui,
         space_view: &SpaceViewBlueprint,
     ) -> egui::Response {
-        space_view_button_to(
-            ctx,
-            ui,
-            space_view.display_name.clone(),
-            space_view.id,
-            space_view.category,
-        )
-    }
-
-    pub fn space_view_button_to(
-        ctx: &mut ViewerContext<'_>,
-        ui: &mut egui::Ui,
-        text: impl Into<egui::WidgetText>,
-        space_view_id: SpaceViewId,
-        space_view_category: ViewCategory,
-    ) -> egui::Response {
-        let item = Item::SpaceView(space_view_id);
+        let item = Item::SpaceView(space_view.id);
         let is_selected = ctx.selection().contains(&item);
 
         let response = ctx
             .re_ui
-            .selectable_label_with_icon(ui, space_view_category.icon(), text, is_selected)
+            .selectable_label_with_icon(
+                ui,
+                space_view.class(ctx).icon(),
+                space_view.display_name.clone(),
+                is_selected,
+            )
             .on_hover_text("Space View");
         item_ui::cursor_interact_with_selectable(ctx.selection_state_mut(), response, item)
     }

crates/re_viewport/src/space_view.rs

@@ -3,7 +3,8 @@ use re_data_store::{EntityPath, EntityTree, TimeInt};
 use re_renderer::ScreenshotProcessor;
 use re_space_view::{DataBlueprintTree, ScreenshotMode};
 use re_viewer_context::{
-    SpaceViewClassName, SpaceViewHighlights, SpaceViewId, SpaceViewState, ViewerContext,
+    DynSpaceViewClass, SpaceViewClassName, SpaceViewHighlights, SpaceViewId, SpaceViewState,
+    ViewerContext,
 };
 
 use crate::{
@@ -19,7 +20,7 @@ use crate::{
 pub struct SpaceViewBlueprint {
     pub id: SpaceViewId,
     pub display_name: String,
-    pub class: SpaceViewClassName,
+    class_name: SpaceViewClassName,
 
     /// The "anchor point" of this space view.
     /// The transform at this path forms the reference point for all scene->world transforms in this space view.
@@ -63,7 +64,7 @@ impl SpaceViewBlueprint {
 
         Self {
             display_name,
-            class: space_view_class,
+            class_name: space_view_class,
             id: SpaceViewId::random(),
             space_origin: space_path.clone(),
             data_blueprint: data_blueprint_tree,
@@ -72,6 +73,15 @@ impl SpaceViewBlueprint {
         }
     }
 
+    pub fn class_name(&self) -> &SpaceViewClassName {
+        &self.class_name
+    }
+
+    pub fn class<'a>(&self, ctx: &ViewerContext<'a>) -> &'a dyn DynSpaceViewClass {
+        ctx.space_view_class_registry
+            .get_or_log_error(&self.class_name)
+    }
+
     pub fn on_frame_start(
         &mut self,
         ctx: &mut ViewerContext<'_>,
@@ -139,18 +149,6 @@ impl SpaceViewBlueprint {
         }
     }
 
-    pub fn selection_ui(
-        &mut self,
-        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, &self.space_origin, self.id);
-        }
-    }
-
     pub(crate) fn scene_ui(
         &mut self,
         view_state: &mut dyn SpaceViewState,
@@ -166,11 +164,9 @@ impl SpaceViewBlueprint {
             return;
         }
 
-        let Some(space_view_class) = ctx.space_view_class_registry.get_or_log_error(self.class) else {
-            return;
-        };
+        let class = self.class(ctx);
 
-        space_view_class.prepare_populate(
+        class.prepare_populate(
             ctx,
             view_state,
             &self.data_blueprint.entity_paths().clone(), // Clone to workaround borrow checker.
@@ -185,11 +181,11 @@ impl SpaceViewBlueprint {
             entity_props_map: self.data_blueprint.data_blueprints_projected(),
         };
 
-        let mut scene = space_view_class.new_scene();
+        let mut scene = class.new_scene();
         scene.populate(ctx, &query, view_state, highlights);
 
         ui.scope(|ui| {
-            space_view_class.ui(ctx, ui, view_state, scene, &self.space_origin, self.id);
+            class.ui(ctx, ui, view_state, scene, &self.space_origin, self.id);
         });
     }
 

crates/re_viewport/src/space_view_entity_picker.rs

@@ -324,8 +324,8 @@ fn create_entity_add_info(
         } else {
             CanAddToSpaceView::No {
                 reason: format!(
-                    "Entity can't be displayed by this type of Space View ({})",
-                    space_view.category
+                    "Entity can't be displayed by this class of Space View ({})",
+                    space_view.class_name()
                 ),
             }
         };

crates/re_viewport/src/space_view_heuristics.rs

@@ -253,7 +253,7 @@ fn default_created_space_views_from_candidates(
                         .collect_vec();
 
                     let mut space_view = SpaceViewBlueprint::new(
-                        candidate.class,
+                        *candidate.class_name(),
                         candidate.category,
                         &candidate.space_origin,
                         &entities,

crates/re_viewport/src/view_category.rs

@@ -31,32 +31,6 @@ pub enum ViewCategory {
     Tensor,
 }
 
-impl ViewCategory {
-    pub fn icon(self) -> &'static re_ui::Icon {
-        match self {
-            ViewCategory::Text => &re_ui::icons::SPACE_VIEW_TEXT,
-            ViewCategory::TextBox => &re_ui::icons::SPACE_VIEW_TEXTBOX,
-            ViewCategory::TimeSeries => &re_ui::icons::SPACE_VIEW_SCATTERPLOT,
-            ViewCategory::BarChart => &re_ui::icons::SPACE_VIEW_HISTOGRAM,
-            ViewCategory::Spatial => &re_ui::icons::SPACE_VIEW_3D,
-            ViewCategory::Tensor => &re_ui::icons::SPACE_VIEW_TENSOR,
-        }
-    }
-}
-
-impl std::fmt::Display for ViewCategory {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(match self {
-            ViewCategory::Text => "Text",
-            ViewCategory::TextBox => "Text Box",
-            ViewCategory::TimeSeries => "Time Series",
-            ViewCategory::BarChart => "Bar Chart",
-            ViewCategory::Spatial => "Spatial",
-            ViewCategory::Tensor => "Tensor",
-        })
-    }
-}
-
 pub type ViewCategorySet = enumset::EnumSet<ViewCategory>;
 
 // TODO(cmc): these `categorize_*` functions below are pretty dangerous: make sure you've covered