Skip to content

Commit

Permalink
Rename ScenePart -> ViewPartSystem + related renamings (#2674)
Browse files Browse the repository at this point in the history
As discussed in todays call and detailed in
https://github.com/rerun-io/rerun/blob/andreas/space_views-rfc/design/space_views.md#viewpartsystem
(WIP rfc)

There's a whole bunch of consecutive renames arising from that, none of
which I brought up since they are much lower stakes: They all revolve
around the idea that "scene" doesn't make sense for the temporary
datastructure we build up every frame. That said, it is still around for
the moment and will be likely removed or renamed as well very soon.
Similarly, there's some follow-up to do in the docs (and the
aforementioned rfc document)

Obsoletes #2522 since it causes too many conflicts there - will
reimplement key ideas from that PR though, seeing a bit clearer by now
where we want to end up.

This 100% pure refactoring, no logic has been touched, only renaming and
moving things around.

Renames:
* ScenePart -> ViewPartSystem
* SceneContextPart -> ViewContextSystem
* ScenePartCollection -> ViewPartSystemCollection
* SceneContext -> ViewContext (todo: weird discrepancy to system
collection)
* SceneQuery -> ViewQuery
* derived types, mostly in SpatialSpaceView crate
* related type fields
* related argument names
* file renamings, flattening out of `scene` folder for
`SpatialSpaceView`

### What

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2674) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2674)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fviewpartsystem-rename/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fviewpartsystem-rename/examples)
  • Loading branch information
Wumpf authored Jul 12, 2023
1 parent a225534 commit 05ff805
Show file tree
Hide file tree
Showing 56 changed files with 461 additions and 484 deletions.
4 changes: 2 additions & 2 deletions crates/re_data_ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,13 @@ pub fn annotations(
let mut annotation_map = re_viewer_context::AnnotationMap::default();
let entity_paths: nohash_hasher::IntSet<_> = std::iter::once(entity_path.clone()).collect();
let entity_props_map = re_data_store::EntityPropertyMap::default();
let scene_query = re_viewer_context::SceneQuery::new(
let query = re_viewer_context::ViewQuery::new(
entity_path,
&entity_paths,
query.timeline,
query.at,
&entity_props_map,
);
annotation_map.load(ctx, &scene_query);
annotation_map.load(ctx, &query);
annotation_map.find(entity_path)
}
2 changes: 1 addition & 1 deletion crates/re_space_view_bar_chart/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! A Space View that shows a single bar chart.

mod scene_part;
mod space_view_class;
mod view_part_system;

pub use space_view_class::BarChartSpaceView;
6 changes: 3 additions & 3 deletions crates/re_space_view_bar_chart/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ use re_viewer_context::{
auto_color, SpaceViewClass, SpaceViewClassName, SpaceViewId, TypedScene, ViewerContext,
};

use super::scene_part::SceneBarChart;
use super::view_part_system::BarChartViewPartSystem;

#[derive(Default)]
pub struct BarChartSpaceView;

impl SpaceViewClass for BarChartSpaceView {
type State = ();
type Context = ();
type SceneParts = SceneBarChart;
type ScenePartData = ();
type SystemCollection = BarChartViewPartSystem;
type ViewPartData = ();

fn name(&self) -> SpaceViewClassName {
"Bar Chart".into()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,29 @@ use re_components::Tensor;
use re_data_store::EntityPath;
use re_log_types::Component as _;
use re_viewer_context::{
ArchetypeDefinition, ScenePart, SceneQuery, SpaceViewClass, SpaceViewHighlights, ViewerContext,
ArchetypeDefinition, SpaceViewClass, SpaceViewHighlights, ViewPartSystem, ViewQuery,
ViewerContext,
};

use crate::BarChartSpaceView;

/// A bar chart scene, with everything needed to render it.
/// A bar chart system, with everything needed to render it.
#[derive(Default)]
pub struct SceneBarChart {
pub struct BarChartViewPartSystem {
pub charts: BTreeMap<EntityPath, Tensor>,
}

impl ScenePart<BarChartSpaceView> for SceneBarChart {
impl ViewPartSystem<BarChartSpaceView> for BarChartViewPartSystem {
fn archetype(&self) -> ArchetypeDefinition {
vec1::vec1![Tensor::name()]
}

fn populate(
&mut self,
ctx: &mut ViewerContext<'_>,
query: &SceneQuery<'_>,
query: &ViewQuery<'_>,
_state: &<BarChartSpaceView as SpaceViewClass>::State,
_scene_context: &<BarChartSpaceView as SpaceViewClass>::Context,
_context: &<BarChartSpaceView as SpaceViewClass>::Context,
_highlights: &SpaceViewHighlights,
) -> Vec<re_renderer::QueueableDrawData> {
re_tracing::profile_function!();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
use re_components::AnnotationContext;
use re_log_types::Component;
use re_viewer_context::{AnnotationMap, ArchetypeDefinition, SceneContextPart};
use re_viewer_context::{AnnotationMap, ArchetypeDefinition, ViewContextSystem};

#[derive(Default)]
pub struct AnnotationSceneContext(pub AnnotationMap);

impl SceneContextPart for AnnotationSceneContext {
impl ViewContextSystem for AnnotationSceneContext {
fn archetypes(&self) -> Vec<ArchetypeDefinition> {
vec![vec1::vec1![AnnotationContext::name()]]
}

fn populate(
&mut self,
ctx: &mut re_viewer_context::ViewerContext<'_>,
query: &re_viewer_context::SceneQuery<'_>,
query: &re_viewer_context::ViewQuery<'_>,
_space_view_state: &dyn re_viewer_context::SpaceViewState,
) {
self.0.load(ctx, query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::{BTreeMap, BTreeSet};
use nohash_hasher::IntMap;
use re_components::DrawOrder;
use re_log_types::{Component, EntityPath, EntityPathHash};
use re_viewer_context::{ArchetypeDefinition, SceneContextPart};
use re_viewer_context::{ArchetypeDefinition, ViewContextSystem};

/// Context for creating a mapping from [`DrawOrder`] to [`re_renderer::DepthOffset`].
#[derive(Default)]
Expand All @@ -18,15 +18,15 @@ pub struct EntityDepthOffsets {
pub points: re_renderer::DepthOffset,
}

impl SceneContextPart for EntityDepthOffsets {
impl ViewContextSystem for EntityDepthOffsets {
fn archetypes(&self) -> Vec<ArchetypeDefinition> {
vec![vec1::vec1![DrawOrder::name()]]
}

fn populate(
&mut self,
ctx: &mut re_viewer_context::ViewerContext<'_>,
query: &re_viewer_context::SceneQuery<'_>,
query: &re_viewer_context::ViewQuery<'_>,
_space_view_state: &dyn re_viewer_context::SpaceViewState,
) {
re_tracing::profile_function!();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ pub use transform_context::{pinhole_camera_view_coordinates, TransformContext};

use re_log_types::EntityPath;
use re_renderer::DepthOffset;
use re_viewer_context::{Annotations, SceneContext};
use re_viewer_context::{Annotations, ViewContext};

use self::non_interactive_entities::NonInteractiveEntities;

#[derive(Default)]
pub struct SpatialSceneContext {
pub struct SpatialViewContext {
pub transforms: TransformContext,
pub depth_offsets: EntityDepthOffsets,
pub annotations: AnnotationSceneContext,
Expand All @@ -31,8 +31,8 @@ pub struct SpatialSceneContext {
pub num_3d_primitives: AtomicUsize,
}

impl SceneContext for SpatialSceneContext {
fn vec_mut(&mut self) -> Vec<&mut dyn re_viewer_context::SceneContextPart> {
impl ViewContext for SpatialViewContext {
fn vec_mut(&mut self) -> Vec<&mut dyn re_viewer_context::ViewContextSystem> {
let Self {
transforms,
depth_offsets,
Expand Down Expand Up @@ -60,7 +60,7 @@ impl SceneContext for SpatialSceneContext {
}
}

impl SpatialSceneContext {
impl SpatialViewContext {
pub fn lookup_entity_context<'a>(
&'a self,
ent_path: &EntityPath,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
use nohash_hasher::IntSet;
use re_log_types::EntityPathHash;
use re_viewer_context::SceneContextPart;
use re_viewer_context::ViewContextSystem;

/// List of all non-interactive entities for lookup during picking evaluation.
///
/// TODO(wumpf/jleibs): This is a temporary solution until the picking code can query propagated blueprint properties directly.
#[derive(Default)]
pub struct NonInteractiveEntities(pub IntSet<EntityPathHash>);

impl SceneContextPart for NonInteractiveEntities {
impl ViewContextSystem for NonInteractiveEntities {
fn archetypes(&self) -> Vec<re_viewer_context::ArchetypeDefinition> {
Vec::new()
}

fn populate(
&mut self,
_ctx: &mut re_viewer_context::ViewerContext<'_>,
query: &re_viewer_context::SceneQuery<'_>,
query: &re_viewer_context::ViewQuery<'_>,
_space_view_state: &dyn re_viewer_context::SpaceViewState,
) {
re_tracing::profile_function!();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use parking_lot::{Mutex, MutexGuard};
use re_renderer::{LineStripSeriesBuilder, PointCloudBuilder, RenderContext};
use re_viewer_context::{ArchetypeDefinition, SceneContextPart};
use re_viewer_context::{ArchetypeDefinition, ViewContextSystem};

use crate::scene::{
use crate::parts::{
SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES, SIZE_BOOST_IN_POINTS_FOR_POINT_OUTLINES,
};

Expand Down Expand Up @@ -46,15 +46,15 @@ impl SharedRenderBuilders {
}
}

impl SceneContextPart for SharedRenderBuilders {
impl ViewContextSystem for SharedRenderBuilders {
fn archetypes(&self) -> Vec<ArchetypeDefinition> {
Vec::new()
}

fn populate(
&mut self,
ctx: &mut re_viewer_context::ViewerContext<'_>,
_query: &re_viewer_context::SceneQuery<'_>,
_query: &re_viewer_context::ViewQuery<'_>,
_space_view_state: &dyn re_viewer_context::SpaceViewState,
) {
self.lines = Some(Mutex::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use re_components::{DisconnectedSpace, Pinhole, Transform3D, ViewCoordinates};
use re_data_store::{EntityPath, EntityPropertyMap, EntityTree};
use re_log_types::Component;
use re_space_view::UnreachableTransformReason;
use re_viewer_context::{ArchetypeDefinition, SceneContextPart};
use re_viewer_context::{ArchetypeDefinition, ViewContextSystem};

use crate::scene::image_view_coordinates;
use crate::parts::image_view_coordinates;

#[derive(Clone)]
struct TransformInfo {
Expand Down Expand Up @@ -54,7 +54,7 @@ impl Default for TransformContext {
}
}

impl SceneContextPart for TransformContext {
impl ViewContextSystem for TransformContext {
fn archetypes(&self) -> Vec<ArchetypeDefinition> {
vec![
vec1::vec1![Transform3D::name()],
Expand All @@ -71,32 +71,32 @@ impl SceneContextPart for TransformContext {
fn populate(
&mut self,
ctx: &mut re_viewer_context::ViewerContext<'_>,
scene_query: &re_viewer_context::SceneQuery<'_>,
query: &re_viewer_context::ViewQuery<'_>,
_space_view_state: &dyn re_viewer_context::SpaceViewState,
) {
re_tracing::profile_function!();

let entity_db = &ctx.store_db.entity_db;
let time_ctrl = &ctx.rec_cfg.time_ctrl;
let entity_prop_map = scene_query.entity_props_map;
let entity_prop_map = query.entity_props_map;

self.space_origin = scene_query.space_origin.clone();
self.space_origin = query.space_origin.clone();

// Find the entity path tree for the root.
let Some(mut current_tree) = &entity_db.tree.subtree(scene_query.space_origin) else {
let Some(mut current_tree) = &entity_db.tree.subtree(query.space_origin) else {
// It seems the space path is not part of the object tree!
// This happens frequently when the viewer remembers space views from a previous run that weren't shown yet.
// Naturally, in this case we don't have any transforms yet.
return;
};

let query = time_ctrl.current_query();
let time_query = time_ctrl.current_query();

// Child transforms of this space
self.gather_descendants_transforms(
current_tree,
&entity_db.data_store,
&query,
&time_query,
entity_prop_map,
glam::Affine3A::IDENTITY,
&None, // Ignore potential pinhole camera at the root of the space view, since it regarded as being "above" this root.
Expand All @@ -110,7 +110,7 @@ impl SceneContextPart for TransformContext {
// Unlike not having the space path in the hierarchy, this should be impossible.
re_log::error_once!(
"Path {} is not part of the global Entity tree whereas its child {} is",
parent_path, scene_query.space_origin
parent_path, query.space_origin
);
return;
};
Expand All @@ -120,7 +120,7 @@ impl SceneContextPart for TransformContext {
match transform_at(
&current_tree.path,
&entity_db.data_store,
&query,
&time_query,
// TODO(#1988): See comment in transform_at. This is a workaround for precision issues
// and the fact that there is no meaningful image plane distance for 3D->2D views.
|_| 500.0,
Expand All @@ -141,7 +141,7 @@ impl SceneContextPart for TransformContext {
self.gather_descendants_transforms(
parent_tree,
&entity_db.data_store,
&query,
&time_query,
entity_prop_map,
reference_from_ancestor,
&encountered_pinhole,
Expand Down
6 changes: 4 additions & 2 deletions crates/re_space_view_spatial/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
//! Rerun Spatial Scene Views
//! Rerun Spatial Space Views
//!
//! Space Views that show entities in a 2D or 3D spatial relationship.

mod axis_lines;
mod contexts;
mod eye;
mod instance_hash_conversions;
mod mesh_cache;
mod mesh_loader;
mod scene;
mod parts;
mod picking;
mod space_camera_3d;
mod space_view_class;
mod ui;
Expand Down
Loading

1 comment on commit 05ff805

@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: 05ff805 Previous: a225534 Ratio
batch_points_arrow/encode_log_msg 89844 ns/iter (± 690) 49922 ns/iter (± 490) 1.80

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

Please sign in to comment.