Skip to content

Commit

Permalink
Add ui_name() to SpaceViewClass so space views may provide a user…
Browse files Browse the repository at this point in the history
…-facing name (#4393)

### What

With this change, the user-facing string for a space view class can be
changed without breaking blueprint files. This PR also reverts the
change from "TextLog" to "Text Log" to avoid breaking last release's
blueprint (*)

* Fixes #2336 

(*) Unknown space view class name do not trigger a blueprint rebuild,
but instead yield an "unknown space view" in the UI, which is Bad UX™️

### 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 [app.rerun.io](https://app.rerun.io/pr/4393) (if
applicable)
* [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/4393)
- [Docs
preview](https://rerun.io/preview/3b3229f5e6a053574a2e94e1f5f63907da8d703c/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/3b3229f5e6a053574a2e94e1f5f63907da8d703c/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
abey79 committed Nov 30, 2023
1 parent a4bb250 commit ee83418
Show file tree
Hide file tree
Showing 15 changed files with 59 additions and 10 deletions.
1 change: 1 addition & 0 deletions crates/re_space_view_bar_chart/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ impl SpaceViewClass for BarChartSpaceView {
type State = ();

const NAME: &'static str = "Bar Chart";
const DISPLAY_NAME: &'static str = "Bar Chart";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_HISTOGRAM
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_spatial/src/space_view_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ impl SpaceViewClass for SpatialSpaceView2D {
type State = SpatialSpaceViewState;

const NAME: &'static str = "2D";
const DISPLAY_NAME: &'static str = "2D";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_2D
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_spatial/src/space_view_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ impl SpaceViewClass for SpatialSpaceView3D {
type State = SpatialSpaceViewState;

const NAME: &'static str = "3D";
const DISPLAY_NAME: &'static str = "3D";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_3D
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_tensor/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ impl SpaceViewClass for TensorSpaceView {
type State = ViewTensorState;

const NAME: &'static str = "Tensor";
const DISPLAY_NAME: &'static str = "Tensor";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_TENSOR
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_text_document/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ impl SpaceViewClass for TextDocumentSpaceView {
type State = TextDocumentSpaceViewState;

const NAME: &'static str = "Text Document";
const DISPLAY_NAME: &'static str = "Text Document";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_TEXTBOX
Expand Down
3 changes: 2 additions & 1 deletion crates/re_space_view_text_log/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ pub struct TextSpaceView;
impl SpaceViewClass for TextSpaceView {
type State = TextSpaceViewState;

const NAME: &'static str = "Text Log";
const NAME: &'static str = "TextLog";
const DISPLAY_NAME: &'static str = "Text Log";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_TEXTBOX
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_time_series/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl SpaceViewClass for TimeSeriesSpaceView {
type State = TimeSeriesSpaceViewState;

const NAME: &'static str = "Time Series";
const DISPLAY_NAME: &'static str = "Time Series";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_CHART
Expand Down
8 changes: 6 additions & 2 deletions crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ fn what_is_selected_ui(
&format!(
"Space View {:?} of type {}",
space_view.display_name,
space_view_class.name(),
space_view_class.display_name(),
),
);
}
Expand Down Expand Up @@ -369,7 +369,11 @@ fn space_view_top_level_properties(

ui.label("Type")
.on_hover_text("The type of this Space View");
ui.label(&*space_view.class(ctx.space_view_class_registry).name());
ui.label(
space_view
.class(ctx.space_view_class_registry)
.display_name(),
);
ui.end_row();
});
}
Expand Down
10 changes: 6 additions & 4 deletions crates/re_viewer_context/src/space_view/dyn_space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ pub enum SpaceViewClassLayoutPriority {
pub trait DynSpaceViewClass {
/// Name of this space view class.
///
/// Used for both ui display and identification.
/// Must be unique within a viewer session.
///
/// TODO(#2336): Display name and identifier should be separate.
/// Used for identification. Must be unique within a viewer session.
fn name(&self) -> SpaceViewClassName;

/// User-facing name of this space view class.
///
/// Used for UI display.
fn display_name(&self) -> &'static str;

/// Icon used to identify this space view class.
fn icon(&self) -> &'static re_ui::Icon;

Expand Down
20 changes: 18 additions & 2 deletions crates/re_viewer_context/src/space_view/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,27 @@ pub trait SpaceViewClass: std::marker::Sized {
type State: SpaceViewState + Default + 'static;

/// Name for this space view class.
///
/// Used as identifier.
const NAME: &'static str;

/// User-facing name for this space view class
const DISPLAY_NAME: &'static str;

/// Name of this space view class.
///
/// Used for both ui display and identification.
/// Must be unique within a viewer session.
/// Used for identification. Must be unique within a viewer session.
fn name(&self) -> SpaceViewClassName {
Self::NAME.into()
}

/// User-facing name for this space view class.
///
/// Used for UI display.
fn display_name(&self) -> &'static str {
Self::DISPLAY_NAME
}

/// Icon used to identify this space view class.
fn icon(&self) -> &'static re_ui::Icon;

Expand Down Expand Up @@ -128,6 +139,11 @@ impl<T: SpaceViewClass + 'static> DynSpaceViewClass for T {
self.name()
}

#[inline]
fn display_name(&self) -> &'static str {
self.display_name()
}

#[inline]
fn icon(&self) -> &'static re_ui::Icon {
self.icon()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ impl SpaceViewClass for SpaceViewClassPlaceholder {
type State = ();

const NAME: &'static str = "Unknown Space View Class";
const DISPLAY_NAME: &'static str = "Unknown Space View Class";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_UNKNOWN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,17 @@ impl SpaceViewClassRegistry {
self.registry.get(name).map(|boxed| boxed.class.as_ref())
}

/// Returns the user-facing name for the given space view class.
///
/// If the class is unknown, returns a placeholder name.
pub fn display_name(&self, name: &SpaceViewClassName) -> &'static str {
self.registry
.get(name)
.map_or("<unknown space view class>", |boxed| {
boxed.class.display_name()
})
}

/// Queries a Space View type's system registry by class name, returning `None` if the class is not registered.
fn get_system_registry(&self, name: &SpaceViewClassName) -> Option<&SpaceViewSystemRegistry> {
self.registry.get(name).map(|boxed| &boxed.systems)
Expand Down
4 changes: 3 additions & 1 deletion crates/re_viewport/src/space_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl SpaceViewBlueprint {
impl SpaceViewBlueprint {
pub fn new(
space_view_class: SpaceViewClassName,
space_view_class_display_name: &'static str,
space_path: &EntityPath,
query: DataQueryBlueprint,
) -> Self {
Expand All @@ -78,7 +79,7 @@ impl SpaceViewBlueprint {
name.to_string()
} else {
// Include class name in the display for root paths because they look a tad bit too short otherwise.
format!("/ ({space_view_class})")
format!("/ ({space_view_class_display_name})")
};

let id = SpaceViewId::random();
Expand Down Expand Up @@ -428,6 +429,7 @@ mod tests {

let space_view = SpaceViewBlueprint::new(
"3D".into(),
"3D",
&EntityPath::root(),
DataQueryBlueprint::new(
"3D".into(),
Expand Down
5 changes: 5 additions & 0 deletions crates/re_viewport/src/space_view_heuristics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ pub fn all_possible_space_views(
Some((
SpaceViewBlueprint::new(
*class_name,
ctx.space_view_class_registry.display_name(class_name),
candidate_space_path,
candidate_query,
),
Expand Down Expand Up @@ -293,6 +294,8 @@ pub fn default_created_space_views(
);
let mut space_view = SpaceViewBlueprint::new(
*candidate.class_name(),
ctx.space_view_class_registry
.display_name(candidate.class_name()),
&result.entity_path,
query,
);
Expand Down Expand Up @@ -401,6 +404,8 @@ pub fn default_created_space_views(

let mut space_view = SpaceViewBlueprint::new(
*candidate.class_name(),
ctx.space_view_class_registry
.display_name(candidate.class_name()),
&candidate.space_origin,
query,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl SpaceViewClass for ColorCoordinatesSpaceView {
type State = ColorCoordinatesSpaceViewState;

const NAME: &'static str = "Color Coordinates";
const DISPLAY_NAME: &'static str = "Color Coordinates";

fn icon(&self) -> &'static re_ui::Icon {
&re_ui::icons::SPACE_VIEW_SCATTERPLOT
Expand Down

0 comments on commit ee83418

Please sign in to comment.