Skip to content

Commit

Permalink
Typename cleanup in SpaceViewClass framework (#2321)
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

`SpaceViewClass` -> `DynSpaceViewClass`
`SpaceViewClassImpl` -> `SpaceViewClass`
As well as rename of associated types on `SpaceViewClass`(Impl) to be
less confusing.

### 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/2321

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/c7708cd/docs
<!-- pr-link-docs:end -->
  • Loading branch information
Wumpf authored Jun 7, 2023
1 parent d3bb4b7 commit 080095b
Show file tree
Hide file tree
Showing 14 changed files with 326 additions and 326 deletions.
20 changes: 10 additions & 10 deletions crates/re_space_view_spatial/src/space_view_class.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use nohash_hasher::IntSet;
use re_log_types::EntityPath;
use re_viewer_context::{SpaceViewClassImpl, SpaceViewId};
use re_viewer_context::{SpaceViewClass, SpaceViewId};

use crate::{
scene::{SpatialSceneContext, SpatialScenePartCollection, SpatialScenePartData},
Expand All @@ -10,10 +10,10 @@ use crate::{
#[derive(Default)]
pub struct SpatialSpaceView;

impl SpaceViewClassImpl for SpatialSpaceView {
type SpaceViewState = SpatialSpaceViewState;
type SceneContext = SpatialSceneContext;
type ScenePartCollection = SpatialScenePartCollection;
impl SpaceViewClass for SpatialSpaceView {
type State = SpatialSpaceViewState;
type Context = SpatialSceneContext;
type SceneParts = SpatialScenePartCollection;
type ScenePartData = SpatialScenePartData;

fn name(&self) -> re_viewer_context::SpaceViewClassName {
Expand All @@ -24,11 +24,11 @@ impl SpaceViewClassImpl for SpatialSpaceView {
&re_ui::icons::SPACE_VIEW_3D
}

fn help_text(&self, re_ui: &re_ui::ReUi, state: &Self::SpaceViewState) -> egui::WidgetText {
fn help_text(&self, re_ui: &re_ui::ReUi, state: &Self::State) -> egui::WidgetText {
state.help_text(re_ui)
}

fn preferred_tile_aspect_ratio(&self, state: &Self::SpaceViewState) -> Option<f32> {
fn preferred_tile_aspect_ratio(&self, state: &Self::State) -> Option<f32> {
match state.nav_mode.get() {
SpatialNavigationMode::TwoD => {
let size = state.scene_bbox_accum.size();
Expand All @@ -41,7 +41,7 @@ impl SpaceViewClassImpl for SpatialSpaceView {
fn prepare_populate(
&self,
ctx: &mut re_viewer_context::ViewerContext<'_>,
state: &Self::SpaceViewState,
state: &Self::State,
entity_paths: &IntSet<EntityPath>,
entity_properties: &mut re_data_store::EntityPropertyMap,
) {
Expand All @@ -52,7 +52,7 @@ impl SpaceViewClassImpl for SpatialSpaceView {
&self,
ctx: &mut re_viewer_context::ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut Self::SpaceViewState,
state: &mut Self::State,
space_origin: &EntityPath,
space_view_id: SpaceViewId,
) {
Expand All @@ -63,7 +63,7 @@ impl SpaceViewClassImpl for SpatialSpaceView {
&self,
ctx: &mut re_viewer_context::ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut Self::SpaceViewState,
state: &mut Self::State,
scene: &mut re_viewer_context::TypedScene<Self>,
space_origin: &EntityPath,
space_view_id: SpaceViewId,
Expand Down
6 changes: 3 additions & 3 deletions crates/re_space_view_text/src/scene_part.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use re_data_store::EntityPath;
use re_log_types::{Component as _, InstanceKey, RowId};
use re_query::{range_entity_with_primary, QueryError};
use re_viewer_context::{
ArchetypeDefinition, ScenePart, ScenePartCollection, SceneQuery, SpaceViewClassImpl,
ArchetypeDefinition, ScenePart, ScenePartCollection, SceneQuery, SpaceViewClass,
SpaceViewHighlights, ViewerContext,
};

Expand Down Expand Up @@ -51,8 +51,8 @@ impl ScenePart<TextSpaceView> for SceneText {
&mut self,
ctx: &mut ViewerContext<'_>,
query: &SceneQuery<'_>,
state: &<TextSpaceView as SpaceViewClassImpl>::SpaceViewState,
_scene_context: &<TextSpaceView as SpaceViewClassImpl>::SceneContext,
state: &<TextSpaceView as SpaceViewClass>::State,
_scene_context: &<TextSpaceView as SpaceViewClass>::Context,
_highlights: &SpaceViewHighlights,
) -> Vec<re_renderer::QueueableDrawData> {
let store = &ctx.store_db.entity_db.data_store;
Expand Down
18 changes: 9 additions & 9 deletions crates/re_space_view_text/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::BTreeMap;
use re_data_ui::item_ui;
use re_log_types::{EntityPath, TimePoint, Timeline};
use re_viewer_context::{
level_to_rich_text, SpaceViewClassImpl, SpaceViewClassName, SpaceViewId, SpaceViewState,
level_to_rich_text, SpaceViewClass, SpaceViewClassName, SpaceViewId, SpaceViewState,
TypedScene, ViewerContext,
};

Expand Down Expand Up @@ -36,10 +36,10 @@ impl SpaceViewState for TextSpaceViewState {
#[derive(Default)]
pub struct TextSpaceView;

impl SpaceViewClassImpl for TextSpaceView {
type SpaceViewState = TextSpaceViewState;
type SceneContext = re_space_view::EmptySceneContext;
type ScenePartCollection = SceneText;
impl SpaceViewClass for TextSpaceView {
type State = TextSpaceViewState;
type Context = re_space_view::EmptySceneContext;
type SceneParts = SceneText;
type ScenePartData = ();

fn name(&self) -> SpaceViewClassName {
Expand All @@ -50,19 +50,19 @@ impl SpaceViewClassImpl for TextSpaceView {
&re_ui::icons::SPACE_VIEW_TEXTBOX
}

fn help_text(&self, _re_ui: &re_ui::ReUi, _state: &Self::SpaceViewState) -> egui::WidgetText {
fn help_text(&self, _re_ui: &re_ui::ReUi, _state: &Self::State) -> egui::WidgetText {
"Shows text entries over time.\nSelect the Space View for filtering options.".into()
}

fn preferred_tile_aspect_ratio(&self, _state: &Self::SpaceViewState) -> Option<f32> {
fn preferred_tile_aspect_ratio(&self, _state: &Self::State) -> Option<f32> {
Some(2.0) // Make text logs wide
}

fn selection_ui(
&self,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut Self::SpaceViewState,
state: &mut Self::State,
_space_origin: &EntityPath,
_space_view_id: SpaceViewId,
) {
Expand Down Expand Up @@ -114,7 +114,7 @@ impl SpaceViewClassImpl for TextSpaceView {
&self,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut Self::SpaceViewState,
state: &mut Self::State,
scene: &mut TypedScene<Self>,
_space_origin: &EntityPath,
_space_view_id: SpaceViewId,
Expand Down
6 changes: 3 additions & 3 deletions crates/re_space_view_text_box/src/scene_part.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use re_arrow_store::LatestAtQuery;
use re_components::Component;
use re_query::{query_entity_with_primary, QueryError};
use re_viewer_context::{
ArchetypeDefinition, ScenePart, ScenePartCollection, SceneQuery, SpaceViewClassImpl,
ArchetypeDefinition, ScenePart, ScenePartCollection, SceneQuery, SpaceViewClass,
SpaceViewHighlights, ViewerContext,
};

Expand Down Expand Up @@ -40,8 +40,8 @@ impl ScenePart<TextBoxSpaceView> for SceneTextBox {
&mut self,
ctx: &mut ViewerContext<'_>,
query: &SceneQuery<'_>,
_space_view_state: &<TextBoxSpaceView as SpaceViewClassImpl>::SpaceViewState,
_scene_context: &<TextBoxSpaceView as SpaceViewClassImpl>::SceneContext,
_space_view_state: &<TextBoxSpaceView as SpaceViewClass>::State,
_scene_context: &<TextBoxSpaceView as SpaceViewClass>::Context,
_highlights: &SpaceViewHighlights,
) -> Vec<re_renderer::QueueableDrawData> {
let store = &ctx.store_db.entity_db.data_store;
Expand Down
16 changes: 8 additions & 8 deletions crates/re_space_view_text_box/src/space_view_class.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use egui::Label;
use re_viewer_context::{
external::re_log_types::EntityPath, SpaceViewClassImpl, SpaceViewClassName, SpaceViewId,
external::re_log_types::EntityPath, SpaceViewClass, SpaceViewClassName, SpaceViewId,
SpaceViewState, TypedScene, ViewerContext,
};

Expand Down Expand Up @@ -35,10 +35,10 @@ impl SpaceViewState for TextBoxSpaceViewState {
#[derive(Default)]
pub struct TextBoxSpaceView;

impl SpaceViewClassImpl for TextBoxSpaceView {
type SpaceViewState = TextBoxSpaceViewState;
type ScenePartCollection = SceneTextBox;
type SceneContext = re_space_view::EmptySceneContext;
impl SpaceViewClass for TextBoxSpaceView {
type State = TextBoxSpaceViewState;
type SceneParts = SceneTextBox;
type Context = re_space_view::EmptySceneContext;
type ScenePartData = ();

fn name(&self) -> SpaceViewClassName {
Expand All @@ -49,15 +49,15 @@ impl SpaceViewClassImpl for TextBoxSpaceView {
&re_ui::icons::SPACE_VIEW_TEXTBOX
}

fn help_text(&self, _re_ui: &re_ui::ReUi, _state: &Self::SpaceViewState) -> egui::WidgetText {
fn help_text(&self, _re_ui: &re_ui::ReUi, _state: &Self::State) -> egui::WidgetText {
"Displays text from a text entry components.".into()
}

fn selection_ui(
&self,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut Self::SpaceViewState,
state: &mut Self::State,
_space_origin: &EntityPath,
_space_view_id: SpaceViewId,
) {
Expand All @@ -76,7 +76,7 @@ impl SpaceViewClassImpl for TextBoxSpaceView {
&self,
_ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut Self::SpaceViewState,
state: &mut Self::State,
scene: &mut TypedScene<Self>,
_space_origin: &EntityPath,
_space_view_id: SpaceViewId,
Expand Down
4 changes: 2 additions & 2 deletions crates/re_viewer/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use re_renderer::WgpuResourcePoolStatistics;
use re_smart_channel::Receiver;
use re_ui::{toasts, Command};
use re_viewer_context::{
AppOptions, ComponentUiRegistry, PlayState, SpaceViewClass, SpaceViewClassRegistry,
AppOptions, ComponentUiRegistry, DynSpaceViewClass, PlayState, SpaceViewClassRegistry,
SpaceViewClassRegistryError,
};

Expand Down Expand Up @@ -231,7 +231,7 @@ impl App {
}

/// Adds a new space view class to the viewer.
pub fn add_space_view_class<T: SpaceViewClass + Default + 'static>(
pub fn add_space_view_class<T: DynSpaceViewClass + Default + 'static>(
&mut self,
) -> Result<(), SpaceViewClassRegistryError> {
self.space_view_class_registry.add::<T>()
Expand Down
4 changes: 2 additions & 2 deletions crates/re_viewer_context/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ pub use selection_state::{
HoverHighlight, HoveredSpace, InteractionHighlight, SelectionHighlight, SelectionState,
};
pub use space_view::{
ArchetypeDefinition, Scene, SceneContext, SceneContextPart, ScenePart, ScenePartCollection,
SceneQuery, SpaceViewClass, SpaceViewClassImpl, SpaceViewClassName, SpaceViewClassRegistry,
ArchetypeDefinition, DynSpaceViewClass, Scene, SceneContext, SceneContextPart, ScenePart,
ScenePartCollection, SceneQuery, SpaceViewClass, SpaceViewClassName, SpaceViewClassRegistry,
SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights,
SpaceViewOutlineMasks, SpaceViewState, TypedScene,
};
Expand Down
107 changes: 107 additions & 0 deletions crates/re_viewer_context/src/space_view/dyn_space_view_class.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
use nohash_hasher::IntSet;
use re_data_store::EntityPropertyMap;
use re_log_types::{ComponentName, EntityPath};

use crate::{Scene, SpaceViewId, ViewerContext};

/// First element is the primary component, all others are optional.
///
/// TODO(andreas/clement): More formal definition of an archetype.
pub type ArchetypeDefinition = vec1::Vec1<ComponentName>;

re_string_interner::declare_new_type!(
/// The unique name of a space view type.
#[derive(serde::Deserialize, serde::Serialize)]
pub struct SpaceViewClassName;
);

/// Defines a class of space view without any concrete types making it suitable for storage and interfacing.
///
/// Implemented by [`crate::SpaceViewClass`].
///
/// Each Space View in the viewer's viewport has a single class assigned immutable at its creation time.
/// The class defines all aspects of its behavior.
/// It determines which entities are queried, how they are rendered, and how the user can interact with them.
pub trait DynSpaceViewClass {
/// Name of this space view class.
///
/// Used for both ui display and identification.
/// Must be unique within a viewer session.
fn name(&self) -> SpaceViewClassName;

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

/// Help text describing how to interact with this space view in the ui.
fn help_text(&self, re_ui: &re_ui::ReUi, state: &dyn SpaceViewState) -> egui::WidgetText;

/// Called once for every new space view instance of this class.
///
/// The state is *not* persisted across viewer sessions, only shared frame-to-frame.
fn new_state(&self) -> Box<dyn SpaceViewState>;

/// Returns a new scene for this space view class.
///
/// Called both to determine the supported archetypes and
/// to populate a scene every frame.
fn new_scene(&self) -> Box<dyn Scene>;

/// Optional archetype of the Space View's blueprint properties.
///
/// Blueprint components that only apply to the space view itself, not to the entities it displays.
fn blueprint_archetype(&self) -> Option<ArchetypeDefinition>;

/// Preferred aspect ratio for the ui tiles of this space view.
fn preferred_tile_aspect_ratio(&self, state: &dyn SpaceViewState) -> Option<f32>;

/// Executed before the scene is populated, can be use for heuristic & state updates before populating the scene.
///
/// Is only allowed to access archetypes defined by [`Self::blueprint_archetype`]
/// Passed entity properties are individual properties without propagated values.
fn prepare_populate(
&self,
ctx: &mut ViewerContext<'_>,
state: &mut dyn SpaceViewState,
entity_paths: &IntSet<EntityPath>,
entity_properties: &mut EntityPropertyMap,
);

/// Ui shown when the user selects a space view of this class.
///
/// TODO(andreas): Should this be instead implemented via a registered `data_ui` of all blueprint relevant types?
fn selection_ui(
&self,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut dyn SpaceViewState,
space_origin: &EntityPath,
space_view_id: SpaceViewId,
);

/// Draws the ui for this space view type and handles ui events.
///
/// The scene passed in was previously created by [`Self::new_scene`] and got populated by the time it is passed.
/// The state passed in was previously created by [`Self::new_state`] and is kept frame-to-frame.
fn ui(
&self,
ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut dyn SpaceViewState,
scene: Box<dyn Scene>,
space_origin: &EntityPath,
space_view_id: SpaceViewId,
);
}

/// Unserialized frame to frame state of a space view.
///
/// For any state that should be persisted, use the Blueprint!
/// This state is used for transient state, such as animation or uncommitted ui state like dragging a camera.
/// (on mouse release, the camera would be committed to the blueprint).
pub trait SpaceViewState: std::any::Any {
/// Converts itself to a reference of [`std::any::Any`], which enables downcasting to concrete types.
fn as_any(&self) -> &dyn std::any::Any;

/// Converts itself to a reference of [`std::any::Any`], which enables downcasting to concrete types.
fn as_any_mut(&mut self) -> &mut dyn std::any::Any;
}
10 changes: 5 additions & 5 deletions crates/re_viewer_context/src/space_view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@
//! Does not implement any concrete space view.
// TODO(andreas): Can we move some of these to the `re_space_view` crate?
mod dyn_space_view_class;
mod highlights;
mod scene;
mod scene_context;
mod scene_part;
mod scene_query;
mod space_view_class;
mod space_view_class_impl;
mod space_view_class_registry;

pub use dyn_space_view_class::{
ArchetypeDefinition, DynSpaceViewClass, SpaceViewClassName, SpaceViewState,
};
pub use highlights::{SpaceViewEntityHighlight, SpaceViewHighlights, SpaceViewOutlineMasks};
pub use scene::{Scene, TypedScene};
pub use scene_context::{SceneContext, SceneContextPart};
pub use scene_part::{ScenePart, ScenePartCollection};
pub use scene_query::SceneQuery;
pub use space_view_class::{
ArchetypeDefinition, SpaceViewClass, SpaceViewClassName, SpaceViewState,
};
pub use space_view_class_impl::SpaceViewClassImpl;
pub use space_view_class::SpaceViewClass;
pub use space_view_class_registry::{SpaceViewClassRegistry, SpaceViewClassRegistryError};
Loading

1 comment on commit 080095b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.25.

Benchmark suite Current: 080095b Previous: d3bb4b7 Ratio
datastore/num_rows=1000/num_instances=1000/packed=false/latest_at/default 408 ns/iter (± 2) 307 ns/iter (± 2) 1.33
datastore/num_rows=1000/num_instances=1000/packed=false/latest_at_missing/primary/default 295 ns/iter (± 1) 222 ns/iter (± 0) 1.33
datastore/num_rows=1000/num_instances=1000/packed=false/latest_at_missing/secondaries/default 451 ns/iter (± 3) 347 ns/iter (± 2) 1.30
datastore/num_rows=1000/num_instances=1000/gc/default 2657670 ns/iter (± 15236) 1728262 ns/iter (± 2576) 1.54
mono_points_arrow/generate_message_bundles 38813013 ns/iter (± 943951) 27974125 ns/iter (± 822782) 1.39
mono_points_arrow/encode_log_msg 177360883 ns/iter (± 666725) 135542141 ns/iter (± 1371956) 1.31
mono_points_arrow/decode_message_bundles 84076248 ns/iter (± 1254043) 58569212 ns/iter (± 472962) 1.44
mono_points_arrow_batched/generate_message_bundles 26540586 ns/iter (± 2446412) 18291675 ns/iter (± 1017403) 1.45
mono_points_arrow_batched/generate_messages 5779553 ns/iter (± 458046) 3751501 ns/iter (± 17055) 1.54
mono_points_arrow_batched/encode_log_msg 659209 ns/iter (± 5077) 386924 ns/iter (± 3613) 1.70
mono_points_arrow_batched/encode_total 36956038 ns/iter (± 2145201) 23290004 ns/iter (± 191170) 1.59
mono_points_arrow_batched/decode_log_msg 526081 ns/iter (± 2556) 300337 ns/iter (± 1584) 1.75
mono_points_arrow_batched/decode_message_bundles 9732627 ns/iter (± 393164) 7492007 ns/iter (± 7758) 1.30
mono_points_arrow_batched/decode_total 10163211 ns/iter (± 244367) 7807194 ns/iter (± 14957) 1.30
batch_points_arrow/encode_log_msg 99494 ns/iter (± 614) 57598 ns/iter (± 136) 1.73
batch_points_arrow/encode_total 376690 ns/iter (± 2330) 289049 ns/iter (± 219) 1.30
batch_points_arrow/decode_log_msg 72049 ns/iter (± 262) 47319 ns/iter (± 84) 1.52
batch_points_arrow/decode_total 78812 ns/iter (± 1907) 51053 ns/iter (± 97) 1.54
arrow_mono_points/insert 2906887967 ns/iter (± 22008719) 1768622911 ns/iter (± 11860905) 1.64
arrow_mono_points/query 1394723 ns/iter (± 26479) 934043 ns/iter (± 8387) 1.49
arrow_batch_points/query 17119 ns/iter (± 122) 12083 ns/iter (± 9) 1.42
arrow_batch_vecs/query 475749 ns/iter (± 575) 315860 ns/iter (± 6354) 1.51

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.