Skip to content

Commit

Permalink
Finalize move of SpatialSpaceView to SpaceViewClass trait framework (#…
Browse files Browse the repository at this point in the history
…2311)

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

This fully decouples all `SpatialSpaceView` code from the rest of the
project. The only thing exported by `re_space_view_spatial` is now the
`SpatialSpaceView` type itself which is registered on space view
startup.

<img width="775" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/b2af26c8-b770-48f8-9d54-e064c4ce66b6">

In order to accomplish this, some of the interfaces of `SpaceViewClass`
needed to be widened a bit. It would be nice to retract some of that in
the future, in particular around object properties and object property
defaults which should be handled by the blueprint.

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

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/f9d42be/docs
<!-- pr-link-docs:end -->
  • Loading branch information
Wumpf authored Jun 7, 2023
1 parent a414f3f commit 11ece53
Show file tree
Hide file tree
Showing 48 changed files with 829 additions and 828 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions crates/re_data_store/src/entity_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,24 @@ pub struct EntityPropertyMap {

#[cfg(feature = "serde")]
impl EntityPropertyMap {
#[inline]
pub fn get(&self, entity_path: &EntityPath) -> EntityProperties {
self.props.get(entity_path).cloned().unwrap_or_default()
}

#[inline]
pub fn set(&mut self, entity_path: EntityPath, prop: EntityProperties) {
if prop == EntityProperties::default() {
self.props.remove(&entity_path); // save space
} else {
self.props.insert(entity_path, prop);
}
}

#[inline]
pub fn iter(&self) -> impl Iterator<Item = (&EntityPath, &EntityProperties)> {
self.props.iter()
}
}

// ----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/examples/depth_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl RenderDepthClouds {
..
} = self;

let world_from_obj = glam::Mat4::from_scale(glam::Vec3::splat(*scale));
let world_from_obj = glam::Affine3A::from_scale(glam::Vec3::splat(*scale));

let depth_cloud_draw_data = DepthCloudDrawData::new(
re_ctx,
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/renderer/depth_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ mod gpu_data {

pub struct DepthCloud {
/// The extrinsics of the camera used for the projection.
pub world_from_obj: glam::Mat4,
pub world_from_obj: glam::Affine3A,

/// The intrinsics of the camera used for the projection.
///
Expand Down Expand Up @@ -159,7 +159,7 @@ impl DepthCloud {
for corner in corners {
let depth = corner.z;
let pos_in_obj = ((corner.truncate() - offset) * depth / focal_length).extend(depth);
let pos_in_world = self.world_from_obj.project_point3(pos_in_obj);
let pos_in_world = self.world_from_obj.transform_point3(pos_in_obj);
bbox.extend(pos_in_world);
}

Expand Down
2 changes: 2 additions & 0 deletions crates/re_space_view/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ mod data_blueprint;
mod empty_scene_context;
mod empty_space_view_state;
mod screenshot;
mod unreachable_transform_reason;

pub use data_blueprint::{DataBlueprintGroup, DataBlueprintTree};
pub use empty_scene_context::EmptySceneContext;
pub use empty_space_view_state::EmptySpaceViewState;
pub use screenshot::ScreenshotMode;
pub use unreachable_transform_reason::UnreachableTransformReason;
31 changes: 31 additions & 0 deletions crates/re_space_view/src/unreachable_transform_reason.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#[derive(Clone, Copy)]
pub enum UnreachableTransformReason {
/// `SpaceInfoCollection` is outdated and can't find a corresponding space info for the given path.
///
/// If at all, this should only happen for a single frame until space infos are rebuilt.
UnknownSpaceInfo,

/// More than one pinhole camera between this and the reference space.
NestedPinholeCameras,

/// Exiting out of a space with a pinhole camera that doesn't have a resolution is not supported.
InversePinholeCameraWithoutResolution,

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

impl std::fmt::Display for UnreachableTransformReason {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(match self {
Self::UnknownSpaceInfo =>
"Can't determine transform because internal data structures are not in a valid state. Please file an issue on https://github.com/rerun-io/rerun/",
Self::NestedPinholeCameras =>
"Can't display entities under nested pinhole cameras.",
Self::DisconnectedSpace =>
"Can't display entities that are in an explicitly disconnected space.",
Self::InversePinholeCameraWithoutResolution =>
"Can't display entities that would require inverting a pinhole camera without a specified resolution.",
})
}
}
69 changes: 69 additions & 0 deletions crates/re_space_view_spatial/src/axis_lines.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use egui::Color32;
use re_data_store::EntityPath;
use re_log_types::InstanceKey;
use re_renderer::LineStripSeriesBuilder;

const AXIS_COLOR_X: Color32 = Color32::from_rgb(255, 25, 25);
const AXIS_COLOR_Y: Color32 = Color32::from_rgb(0, 240, 0);
const AXIS_COLOR_Z: Color32 = Color32::from_rgb(80, 80, 255);

pub fn add_axis_lines(
line_builder: &mut LineStripSeriesBuilder,
transform: macaw::IsoTransform,
ent_path: Option<&EntityPath>,
axis_length: f32,
) {
use re_renderer::renderer::LineStripFlags;

// TODO(andreas): It would be nice if could display the semantics (left/right/up) as a tooltip on hover.
let line_radius = re_renderer::Size::new_scene(axis_length * 0.05);
let origin = transform.translation();

let mut line_batch =
line_builder
.batch("origin axis")
.picking_object_id(re_renderer::PickingLayerObjectId(
ent_path.map_or(0, |p| p.hash64()),
));
let picking_instance_id = re_renderer::PickingLayerInstanceId(InstanceKey::SPLAT.0);

line_batch
.add_segment(
origin,
origin + transform.transform_vector3(glam::Vec3::X) * axis_length,
)
.radius(line_radius)
.color(AXIS_COLOR_X)
.flags(
LineStripFlags::FLAG_COLOR_GRADIENT
| LineStripFlags::FLAG_CAP_END_TRIANGLE
| LineStripFlags::FLAG_CAP_START_ROUND,
)
.picking_instance_id(picking_instance_id);
line_batch
.add_segment(
origin,
origin + transform.transform_vector3(glam::Vec3::Y) * axis_length,
)
.radius(line_radius)
.color(AXIS_COLOR_Y)
.flags(
LineStripFlags::FLAG_COLOR_GRADIENT
| LineStripFlags::FLAG_CAP_END_TRIANGLE
| LineStripFlags::FLAG_CAP_START_ROUND,
)
.picking_instance_id(picking_instance_id);
line_batch
.add_segment(
origin,
origin + transform.transform_vector3(glam::Vec3::Z) * axis_length,
)
.radius(line_radius)
.color(AXIS_COLOR_Z)
.flags(
LineStripFlags::FLAG_COLOR_GRADIENT
| LineStripFlags::FLAG_CAP_END_TRIANGLE
| LineStripFlags::FLAG_CAP_START_ROUND,
)
.picking_instance_id(picking_instance_id);
}
8 changes: 2 additions & 6 deletions crates/re_space_view_spatial/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//!
//! Space Views that show entities in a 2D or 3D spatial relationship.
mod axis_lines;
mod eye;
mod instance_hash_conversions;
mod mesh_cache;
Expand All @@ -12,10 +13,5 @@ mod space_view_class;
mod ui;
mod ui_2d;
mod ui_3d;
mod ui_renderer_bridge;

// TODO(andreas) should only make the main type public
pub use space_view_class::SpatialSpaceViewClass;

pub use scene::{SceneSpatial, TransformContext, UnreachableTransform};
pub use ui::{SpatialNavigationMode, ViewSpatialState};
pub use space_view_class::SpatialSpaceView;
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::{BTreeMap, BTreeSet};

use nohash_hasher::IntMap;
use re_components::DrawOrder;
use re_log_types::{Component, EntityPathHash};
use re_log_types::{Component, EntityPath, EntityPathHash};
use re_viewer_context::{ArchetypeDefinition, SceneContextPart};

/// Context for creating a mapping from [`DrawOrder`] to [`re_renderer::DepthOffset`].
Expand Down Expand Up @@ -118,3 +118,9 @@ impl SceneContextPart for EntityDepthOffsets {
.collect();
}
}

impl EntityDepthOffsets {
pub fn get(&self, ent_path: &EntityPath) -> Option<re_renderer::DepthOffset> {
self.per_entity.get(&ent_path.hash()).cloned()
}
}
8 changes: 7 additions & 1 deletion crates/re_space_view_spatial/src/scene/contexts/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod annotation_context;
mod depth_offsets;
mod non_interactive_entities;
mod shared_render_builders;
mod transform_context;

Expand All @@ -8,20 +9,23 @@ use std::sync::atomic::AtomicUsize;
pub use annotation_context::AnnotationSceneContext;
pub use depth_offsets::EntityDepthOffsets;
pub use shared_render_builders::SharedRenderBuilders;
pub use transform_context::{TransformContext, UnreachableTransform};
pub use transform_context::TransformContext;

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

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

use self::non_interactive_entities::NonInteractiveEntities;

#[derive(Default)]
pub struct SpatialSceneContext {
pub transforms: TransformContext,
pub depth_offsets: EntityDepthOffsets,
pub annotations: AnnotationSceneContext,
pub shared_render_builders: SharedRenderBuilders,
pub non_interactive_entities: NonInteractiveEntities,

pub num_primitives: AtomicUsize,
pub num_3d_primitives: AtomicUsize,
Expand All @@ -34,6 +38,7 @@ impl SceneContext for SpatialSceneContext {
depth_offsets,
annotations,
shared_render_builders,
non_interactive_entities,
num_3d_primitives: _,
num_primitives: _,
} = self;
Expand All @@ -42,6 +47,7 @@ impl SceneContext for SpatialSceneContext {
depth_offsets,
annotations,
shared_render_builders,
non_interactive_entities,
]
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use nohash_hasher::IntSet;
use re_log_types::EntityPathHash;
use re_viewer_context::SceneContextPart;

/// 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 {
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<'_>,
_space_view_state: &dyn re_viewer_context::SpaceViewState,
) {
re_tracing::profile_function!();

self.0 = query
.entity_props_map
.iter()
.filter_map(|(entity_path, props)| {
if props.interactive {
None
} else {
Some(entity_path.hash())
}
})
.collect();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use parking_lot::{Mutex, MutexGuard};
use re_renderer::{LineStripSeriesBuilder, PointCloudBuilder};
use re_renderer::{LineStripSeriesBuilder, PointCloudBuilder, RenderContext};
use re_viewer_context::{ArchetypeDefinition, SceneContextPart};

use crate::scene::{
Expand All @@ -22,6 +22,32 @@ impl SharedRenderBuilders {
pub fn points(&self) -> MutexGuard<'_, PointCloudBuilder> {
self.points.as_ref().unwrap().lock()
}

pub fn queuable_draw_data(
&mut self,
render_ctx: &mut RenderContext,
) -> Vec<re_renderer::QueueableDrawData> {
let mut result = Vec::new();
result.extend(self.lines.take().and_then(
|l| match l.into_inner().to_draw_data(render_ctx) {
Ok(d) => Some(d.into()),
Err(err) => {
re_log::error_once!("Failed to build line strip draw data: {err}");
None
}
},
));
result.extend(self.points.take().and_then(
|l| match l.into_inner().to_draw_data(render_ctx) {
Ok(d) => Some(d.into()),
Err(err) => {
re_log::error_once!("Failed to build point draw data: {err}");
None
}
},
));
result
}
}

impl SceneContextPart for SharedRenderBuilders {
Expand Down
Loading

1 comment on commit 11ece53

@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: 11ece53 Previous: a414f3f Ratio
mono_points_arrow_batched/encode_log_msg 619210 ns/iter (± 9879) 411995 ns/iter (± 1792) 1.50
batch_points_arrow/encode_log_msg 112881 ns/iter (± 2222) 56372 ns/iter (± 142) 2.00

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

Please sign in to comment.