Skip to content

Commit

Permalink
Bugfix: still need to grow view bounds when the space view is 2D (#3784)
Browse files Browse the repository at this point in the history
### What
Resolves: #3780

Whether we grow the view bounds based on an image depends on whether or
not it's a 2d or 3d space view. This wasn't available before and
required adding a bit of extra plumbing.

### 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/3784) (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/3784)
- [Docs
preview](https://rerun.io/preview/8367c1f82a01a9f0c5577a6a1b6642dcfff35e74/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8367c1f82a01a9f0c5577a6a1b6642dcfff35e74/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
  • Loading branch information
jleibs authored Oct 10, 2023
1 parent 17d7097 commit e021608
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 19 deletions.
4 changes: 3 additions & 1 deletion crates/re_space_view_spatial/src/contexts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ pub use transform_context::TransformContext;

use re_renderer::DepthOffset;
use re_viewer_context::{
Annotations, NamedViewSystem, SpaceViewClassRegistryError, ViewContextSystem,
Annotations, NamedViewSystem, SpaceViewClassName, SpaceViewClassRegistryError,
ViewContextSystem,
};

/// Context objects for a single entity in a spatial scene.
Expand All @@ -27,6 +28,7 @@ pub struct SpatialSceneEntityContext<'a> {
pub shared_render_builders: &'a SharedRenderBuilders,

pub highlight: &'a re_viewer_context::SpaceViewOutlineMasks, // Not part of the context, but convenient to have here.
pub space_view_class_name: SpaceViewClassName,
}

#[derive(Default)]
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_spatial/src/parts/entity_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ where
annotations: annotations.0.find(ent_path),
shared_render_builders,
highlight: query.highlights.entity_outline_mask(ent_path.hash()),
space_view_class_name: view_ctx.space_view_class_name(),
};

match query_archetype_with_history::<A, N>(
Expand Down
39 changes: 25 additions & 14 deletions crates/re_space_view_spatial/src/parts/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ use re_types::{
Archetype as _, ComponentNameSet,
};
use re_viewer_context::{
default_heuristic_filter, gpu_bridge, DefaultColor, SpaceViewSystemExecutionError,
TensorDecodeCache, TensorStatsCache, ViewPartSystem, ViewQuery, ViewerContext,
default_heuristic_filter, gpu_bridge, DefaultColor, SpaceViewClass,
SpaceViewSystemExecutionError, TensorDecodeCache, TensorStatsCache, ViewPartSystem, ViewQuery,
ViewerContext,
};
use re_viewer_context::{NamedViewSystem, ViewContextCollection};

Expand All @@ -29,6 +30,7 @@ use crate::{
parts::SIZE_BOOST_IN_POINTS_FOR_POINT_OUTLINES,
query_pinhole,
view_kind::SpatialSpaceViewKind,
SpatialSpaceView2D,
};

use super::{entity_iterator::process_archetype_views, SpatialViewPartData};
Expand Down Expand Up @@ -272,11 +274,14 @@ impl ImagesPart {
meaning,
color.into(),
) {
// Only update the bounding box if the image_plane_distance is not auto.
// This is avoids a cyclic relationship where the image plane grows the bounds
// which in turn influence the size of the image plane.
// Only update the bounding box if this is a 2D space view or
// the image_plane_distance is not auto. This is avoids a cyclic
// relationship where the image plane grows the bounds which in
// turn influence the size of the image plane.
// See: https://github.com/rerun-io/rerun/issues/3728
if !ent_props.pinhole_image_plane_distance.is_auto() {
if ent_context.space_view_class_name == SpatialSpaceView2D.name()
|| !ent_props.pinhole_image_plane_distance.is_auto()
{
self.extend_bbox(&textured_rect);
}

Expand Down Expand Up @@ -397,11 +402,14 @@ impl ImagesPart {
meaning,
color.into(),
) {
// Only update the bounding box if the image_plane_distance is not auto.
// This is avoids a cyclic relationship where the image plane grows the bounds
// which in turn influence the size of the image plane.
// Only update the bounding box if this is a 2D space view or
// the image_plane_distance is not auto. This is avoids a cyclic
// relationship where the image plane grows the bounds which in
// turn influence the size of the image plane.
// See: https://github.com/rerun-io/rerun/issues/3728
if !ent_props.pinhole_image_plane_distance.is_auto() {
if ent_context.space_view_class_name == SpatialSpaceView2D.name()
|| !ent_props.pinhole_image_plane_distance.is_auto()
{
self.extend_bbox(&textured_rect);
}

Expand Down Expand Up @@ -490,11 +498,14 @@ impl ImagesPart {
meaning,
color.into(),
) {
// Only update the bounding box if the image_plane_distance is not auto.
// This is avoids a cyclic relationship where the image plane grows the bounds
// which in turn influence the size of the image plane.
// Only update the bounding box if this is a 2D space view or
// the image_plane_distance is not auto. This is avoids a cyclic
// relationship where the image plane grows the bounds which in
// turn influence the size of the image plane.
// See: https://github.com/rerun-io/rerun/issues/3728
if !ent_props.pinhole_image_plane_distance.is_auto() {
if ent_context.space_view_class_name == SpatialSpaceView2D.name()
|| !ent_props.pinhole_image_plane_distance.is_auto()
{
self.extend_bbox(&textured_rect);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl<T: SpaceViewClass + 'static> DynSpaceViewClass for T {
// TODO(andreas): We should be able to parallelize both of these loops
let view_ctx = {
re_tracing::profile_scope!("ViewContextSystem::execute");
let mut view_ctx = systems.new_context_collection();
let mut view_ctx = systems.new_context_collection(self.name());
for (_name, system) in &mut view_ctx.systems {
re_tracing::profile_scope!(_name.as_str());
system.execute(ctx, query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ impl SpaceViewSystemRegistry {
}
}

pub fn new_context_collection(&self) -> ViewContextCollection {
pub fn new_context_collection(
&self,
space_view_class_name: SpaceViewClassName,
) -> ViewContextCollection {
ViewContextCollection {
systems: self
.contexts
Expand All @@ -85,6 +88,7 @@ impl SpaceViewSystemRegistry {
(*name, part)
})
.collect(),
space_view_class_name,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use ahash::HashMap;
use re_types::ComponentNameSet;

use crate::{
NamedViewSystem, SpaceViewSystemExecutionError, ViewQuery, ViewSystemName, ViewerContext,
NamedViewSystem, SpaceViewClassName, SpaceViewSystemExecutionError, ViewQuery, ViewSystemName,
ViewerContext,
};

/// View context that can be used by view parts and ui methods to retrieve information about the scene as a whole.
Expand All @@ -28,8 +29,10 @@ pub trait ViewContextSystem {
fn as_any(&self) -> &dyn std::any::Any;
}

// TODO(jleibs): This probably needs a better name now that it includes class name
pub struct ViewContextCollection {
pub(crate) systems: HashMap<ViewSystemName, Box<dyn ViewContextSystem>>,
pub(crate) space_view_class_name: SpaceViewClassName,
}

impl ViewContextCollection {
Expand All @@ -47,4 +50,8 @@ impl ViewContextCollection {
) -> impl Iterator<Item = (ViewSystemName, &dyn ViewContextSystem)> {
self.systems.iter().map(|s| (*s.0, s.1.as_ref()))
}

pub fn space_view_class_name(&self) -> SpaceViewClassName {
self.space_view_class_name
}
}
5 changes: 4 additions & 1 deletion crates/re_viewport/src/space_view_heuristics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,10 @@ pub fn identify_entities_per_system_per_class(
.map(|(class_name, entry)| {
(
*class_name,
(entry.new_context_collection(), entry.new_part_collection()),
(
entry.new_context_collection(*class_name),
entry.new_part_collection(),
),
)
})
.collect();
Expand Down

0 comments on commit e021608

Please sign in to comment.