Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parents of queried paths getting visualized, fix 2D objects not showing at all in 3D if their camera parent is not included #5424

Merged
merged 3 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/re_entity_db/src/entity_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl Default for EntityProperties {
visible_history: re_query::ExtraQueryHistory::default(),
interactive: true,
color_mapper: EditableAutoValue::default(),
pinhole_image_plane_distance: EditableAutoValue::default(),
pinhole_image_plane_distance: EditableAutoValue::Auto(1.0),
backproject_depth: EditableAutoValue::Auto(true),
depth_from_world_scale: EditableAutoValue::Auto(1.0),
backproject_radius_scale: EditableAutoValue::Auto(1.0),
Expand Down
2 changes: 1 addition & 1 deletion crates/re_space_view/src/space_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl SpaceViewBlueprint {
DataResult {
entity_path: entity_path.clone(),
visualizers: Default::default(),
direct_included: true,
tree_prefix_only: false,
property_overrides: Some(PropertyOverrides {
accumulated_properties,
individual_properties,
Expand Down
17 changes: 11 additions & 6 deletions crates/re_space_view/src/space_view_contents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,21 @@ impl<'a> QueryExpressionEvaluator<'a> {

let entity_path = &tree.path;

let tree_prefix_only = !self.entity_path_filter.is_included(entity_path);

// TODO(#5067): For now, we always start by setting visualizers to the full list of available visualizers.
// This is currently important for evaluating auto-properties during the space-view `on_frame_start`, which
// is called before the property-overrider has a chance to update this list.
// This list will be updated below during `update_overrides_recursive` by calling `choose_default_visualizers`
// on the space view.
let visualizers: SmallVec<[_; 4]> = self
.visualizable_entities_for_visualizer_systems
.iter()
.filter_map(|(visualizer, ents)| ents.contains(entity_path).then_some(*visualizer))
.collect();
let visualizers: SmallVec<[_; 4]> = if tree_prefix_only {
Default::default()
} else {
self.visualizable_entities_for_visualizer_systems
.iter()
.filter_map(|(visualizer, ents)| ents.contains(entity_path).then_some(*visualizer))
.collect()
};

let children: SmallVec<[_; 4]> = tree
.children
Expand All @@ -316,7 +321,7 @@ impl<'a> QueryExpressionEvaluator<'a> {
data_result: DataResult {
entity_path: entity_path.clone(),
visualizers,
direct_included: self.entity_path_filter.is_included(entity_path),
tree_prefix_only,
property_overrides: None,
},
children,
Expand Down
14 changes: 1 addition & 13 deletions crates/re_viewer_context/src/query_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,7 @@ impl DataQueryResult {
pub fn contains_entity(&self, path: &EntityPath) -> bool {
self.tree
.lookup_result_by_path(path)
.map_or(false, |result| result.direct_included)
}

#[inline]
pub fn contains_group(&self, path: &EntityPath) -> bool {
self.tree
.lookup_result_by_path(path)
.map_or(false, |result| result.direct_included)
}

#[inline]
pub fn contains_any(&self, path: &EntityPath) -> bool {
self.contains_entity(path) || self.contains_group(path)
.map_or(false, |result| !result.tree_prefix_only)
}
}

Expand Down
8 changes: 5 additions & 3 deletions crates/re_viewer_context/src/space_view/view_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ pub struct DataResult {
/// Which `ViewSystems`s to pass the `DataResult` to.
pub visualizers: SmallVisualizerSet,

// This result was actually in the query results, not just a path that
// exists due to a common prefix.
pub direct_included: bool,
/// If true, this path is not actually included in the query results and is just here
/// because of a common prefix.
///
/// If this is true, `visualizers` must be empty.
pub tree_prefix_only: bool,

/// The accumulated property overrides for this `DataResult`.
pub property_overrides: Option<PropertyOverrides>,
Expand Down
2 changes: 1 addition & 1 deletion crates/re_viewport/src/space_view_entity_picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ fn create_entity_add_info(
let can_add: CanAddToSpaceView =
if visualizable_entities.iter().any(|(_, entities)| entities.contains(entity_path)) {
CanAddToSpaceView::Compatible {
already_added: query_result.contains_any(entity_path),
already_added: query_result.contains_entity(entity_path),
}
} else {
// TODO(#4826): This shouldn't necessarily prevent us from adding it.
Expand Down
15 changes: 7 additions & 8 deletions crates/re_viewport/src/viewport_blueprint_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,16 +307,17 @@ impl Viewport<'_, '_> {
// The later is important since `+ image/camera/**` necessarily has `image` and `image/camera` in the data result tree.
let mut projections = Vec::new();
result_tree.visit(&mut |node| {
if !node
if node
.data_result
.entity_path
.starts_with(&space_view.space_origin)
&& node.data_result.direct_included
{
projections.push(node);
false
false // If its under the origin, we're not interested, stop recursing.
} else if node.data_result.tree_prefix_only {
true // Keep recursing until we find a projection.
} else {
true
projections.push(node);
false // We found a projection, stop recursing as everything below is now included in the projections.
}
});
if !projections.is_empty() {
Expand Down Expand Up @@ -430,9 +431,7 @@ impl Viewport<'_, '_> {

let subdued = !space_view_visible
|| !visible
|| data_result_node.map_or(true, |n| {
n.data_result.visualizers.is_empty() && n.children.is_empty()
});
|| data_result_node.map_or(true, |n| n.data_result.visualizers.is_empty());

let list_item = ListItem::new(ctx.re_ui, item_label)
.selected(is_selected)
Expand Down
Loading