Skip to content

Commit

Permalink
Allow systems to specify custom filter for the applicable set beyond …
Browse files Browse the repository at this point in the history
…required_components (#4604)

### What

Part of general query effort, mostly
* [#4388](#4388)

Allows visualizers to specify `VisualizerAdditionalApplicabilityFilter`
which can be used to answer global visualizer applicability questions
like "is this an image tensor". Therefore moving these kind of checks
out of the `heuristic_filter`.

Right now this has a negligible (presumed positive) impact on
performance. However, there's two strong motivating factors here:
* `heuristic_filter` is right now incorrectly applied on a per space
view _class_ bases. It currently is used to extract the "visualizable
set" which actually **has** to be determined per space view _instance_
since it is dependent on things like the space's origin. Once
`heuristic_filter` runs per instance (or much much worse, per instance
candidate during heuristic!) the now moved checks become a huge burden.
This is about to happen in a follow-up PR!
* previously, we did a latest-at-query for things that should be
resolved with a store diff. Therefore, this change makes the
entity->visualizer mapping more correct & deterministic since missing an
update because of rendering pacing is no longer possible!

### 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 the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4604/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4604/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4604/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [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/4604)
- [Docs
preview](https://rerun.io/preview/a5b72a18a7c05938ec66e510aea455fc42b3608b/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/a5b72a18a7c05938ec66e510aea455fc42b3608b/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
Wumpf authored Dec 21, 2023
1 parent 88f7bbb commit a85a268
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 62 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

19 changes: 19 additions & 0 deletions crates/re_space_view/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,22 @@ pub use data_query::{DataQuery, EntityOverrideContext, PropertyResolver};
pub use data_query_blueprint::DataQueryBlueprint;
pub use screenshot::ScreenshotMode;
pub use unreachable_transform_reason::UnreachableTransformReason;

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

use re_data_store::external::re_arrow_store;

/// Utility for implementing [`re_viewer_context::VisualizerAdditionalApplicabilityFilter`] using on the properties of a concrete component.
#[inline]
pub fn diff_component_filter<T: re_types_core::Component>(
event: &re_arrow_store::StoreEvent,
filter: impl Fn(&T) -> bool,
) -> bool {
let filter = &filter;
event.diff.cells.iter().any(|(component_name, cell)| {
component_name == &T::name()
&& T::from_arrow(cell.as_arrow_ref())
.map(|components| components.iter().any(filter))
.unwrap_or(false)
})
}
39 changes: 15 additions & 24 deletions crates/re_space_view_bar_chart/src/view_part_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ use std::collections::BTreeMap;

use re_arrow_store::LatestAtQuery;
use re_data_store::EntityPath;
use re_space_view::diff_component_filter;
use re_types::{
archetypes::{BarChart, Tensor},
components::Color,
datatypes::TensorData,
Archetype, ComponentNameSet,
};
use re_viewer_context::{
default_heuristic_filter, HeuristicFilterContext, IdentifiedViewSystem,
SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem, ViewQuery, ViewerContext,
IdentifiedViewSystem, SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem,
ViewQuery, ViewerContext, VisualizerAdditionalApplicabilityFilter,
};

/// A bar chart system, with everything needed to render it.
Expand All @@ -25,6 +26,16 @@ impl IdentifiedViewSystem for BarChartViewPartSystem {
}
}

struct BarChartVisualizerEntityFilter;

impl VisualizerAdditionalApplicabilityFilter for BarChartVisualizerEntityFilter {
fn update_applicability(&mut self, event: &re_arrow_store::StoreEvent) -> bool {
diff_component_filter(event, |tensor: &re_types::components::TensorData| {
tensor.is_vector()
})
}
}

impl ViewPartSystem for BarChartViewPartSystem {
fn required_components(&self) -> ComponentNameSet {
BarChart::required_components()
Expand All @@ -42,28 +53,8 @@ impl ViewPartSystem for BarChartViewPartSystem {
.collect()
}

fn heuristic_filter(
&self,
store: &re_arrow_store::DataStore,
ent_path: &EntityPath,
_ctx: HeuristicFilterContext,
query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
if !default_heuristic_filter(entity_components, &self.indicator_components()) {
return false;
}

// NOTE: We want to make sure we query at the right time, otherwise we might take into
// account a `Clear()` that actually only applies into the future, and then
// `is_vector` will righfully fail because of the empty tensor.
if let Some(tensor) =
store.query_latest_component::<re_types::components::TensorData>(ent_path, query)
{
tensor.is_vector()
} else {
false
}
fn applicability_filter(&self) -> Option<Box<dyn VisualizerAdditionalApplicabilityFilter>> {
Some(Box::new(BarChartVisualizerEntityFilter))
}

fn execute(
Expand Down
32 changes: 20 additions & 12 deletions crates/re_space_view_spatial/src/parts/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use re_renderer::{
renderer::{DepthCloud, DepthClouds, RectangleOptions, TexturedRect},
Colormap,
};
use re_space_view::diff_component_filter;
use re_types::{
archetypes::{DepthImage, Image, SegmentationImage},
components::{Color, DrawOrder, TensorData, ViewCoordinates},
Expand All @@ -21,7 +22,7 @@ use re_types::{
use re_viewer_context::{
default_heuristic_filter, gpu_bridge, DefaultColor, HeuristicFilterContext, SpaceViewClass,
SpaceViewSystemExecutionError, TensorDecodeCache, TensorStatsCache, ViewPartSystem, ViewQuery,
ViewerContext,
ViewerContext, VisualizerAdditionalApplicabilityFilter,
};
use re_viewer_context::{IdentifiedViewSystem, ViewContextCollection};

Expand Down Expand Up @@ -643,6 +644,16 @@ impl IdentifiedViewSystem for ImagesPart {
}
}

struct ImageVisualizerEntityFilter;

impl VisualizerAdditionalApplicabilityFilter for ImageVisualizerEntityFilter {
fn update_applicability(&mut self, event: &re_arrow_store::StoreEvent) -> bool {
diff_component_filter(event, |tensor: &re_types::components::TensorData| {
tensor.is_shaped_like_an_image()
})
}
}

impl ViewPartSystem for ImagesPart {
fn required_components(&self) -> ComponentNameSet {
let image: ComponentNameSet = Image::required_components()
Expand Down Expand Up @@ -677,12 +688,16 @@ impl ViewPartSystem for ImagesPart {
.collect()
}

fn applicability_filter(&self) -> Option<Box<dyn VisualizerAdditionalApplicabilityFilter>> {
Some(Box::new(ImageVisualizerEntityFilter))
}

fn heuristic_filter(
&self,
store: &re_arrow_store::DataStore,
ent_path: &EntityPath,
_store: &re_arrow_store::DataStore,
_ent_path: &EntityPath,
ctx: HeuristicFilterContext,
query: &LatestAtQuery,
_query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
if !default_heuristic_filter(entity_components, &self.indicator_components()) {
Expand All @@ -697,14 +712,7 @@ impl ViewPartSystem for ImagesPart {
return false;
}

// NOTE: We want to make sure we query at the right time, otherwise we might take into
// account a `Clear()` that actually only applies into the future, and then
// `is_shaped_like_an_image` will righfully fail because of the empty tensor.
if let Some(tensor) = store.query_latest_component::<TensorData>(ent_path, query) {
tensor.is_shaped_like_an_image()
} else {
false
}
true
}

fn execute(
Expand Down
1 change: 1 addition & 0 deletions crates/re_space_view_tensor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ re_data_ui.workspace = true
re_log_types.workspace = true
re_log.workspace = true
re_renderer.workspace = true
re_space_view.workspace = true
re_tracing.workspace = true
re_types.workspace = true
re_ui.workspace = true
Expand Down
38 changes: 15 additions & 23 deletions crates/re_space_view_tensor/src/view_part_system.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use re_arrow_store::{LatestAtQuery, VersionedComponent};
use re_data_store::EntityPath;
use re_log_types::RowId;
use re_space_view::diff_component_filter;
use re_types::{
archetypes::Tensor, components::TensorData, tensor_data::DecodedTensor, Archetype,
ComponentNameSet,
};
use re_viewer_context::{
default_heuristic_filter, HeuristicFilterContext, IdentifiedViewSystem,
SpaceViewSystemExecutionError, TensorDecodeCache, ViewContextCollection, ViewPartSystem,
ViewQuery, ViewerContext,
IdentifiedViewSystem, SpaceViewSystemExecutionError, TensorDecodeCache, ViewContextCollection,
ViewPartSystem, ViewQuery, ViewerContext, VisualizerAdditionalApplicabilityFilter,
};

#[derive(Default)]
Expand All @@ -22,6 +22,16 @@ impl IdentifiedViewSystem for TensorSystem {
}
}

struct TensorVisualizerEntityFilter;

impl VisualizerAdditionalApplicabilityFilter for TensorVisualizerEntityFilter {
fn update_applicability(&mut self, event: &re_arrow_store::StoreEvent) -> bool {
diff_component_filter(event, |tensor: &re_types::components::TensorData| {
!tensor.is_vector()
})
}
}

impl ViewPartSystem for TensorSystem {
fn required_components(&self) -> ComponentNameSet {
Tensor::required_components()
Expand All @@ -34,26 +44,8 @@ impl ViewPartSystem for TensorSystem {
std::iter::once(Tensor::indicator().name()).collect()
}

fn heuristic_filter(
&self,
store: &re_arrow_store::DataStore,
ent_path: &EntityPath,
_ctx: HeuristicFilterContext,
query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
if !default_heuristic_filter(entity_components, &self.indicator_components()) {
return false;
}

// The tensor view can't display anything with less than two dimensions.
if let Some(tensor) =
store.query_latest_component::<re_types::components::TensorData>(ent_path, query)
{
!tensor.is_vector()
} else {
false
}
fn applicability_filter(&self) -> Option<Box<dyn VisualizerAdditionalApplicabilityFilter>> {
Some(Box::new(TensorVisualizerEntityFilter))
}

fn execute(
Expand Down
1 change: 1 addition & 0 deletions crates/re_viewer_context/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub use space_view::{
SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewState, SpaceViewSystemExecutionError,
SpaceViewSystemRegistrator, SystemExecutionOutput, ViewContextCollection, ViewContextSystem,
ViewPartCollection, ViewPartSystem, ViewQuery, ViewSystemIdentifier,
VisualizerAdditionalApplicabilityFilter,
};
pub use store_context::StoreContext;
pub use tensor::{TensorDecodeCache, TensorStats, TensorStatsCache};
Expand Down
1 change: 1 addition & 0 deletions crates/re_viewer_context/src/space_view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub use view_part_system::{
default_heuristic_filter, HeuristicFilterContext, ViewPartCollection, ViewPartSystem,
};
pub use view_query::{DataResult, PerSystemDataResults, PropertyOverrides, ViewQuery};
pub use visualizer_entity_subscriber::VisualizerAdditionalApplicabilityFilter;

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

Expand Down
8 changes: 8 additions & 0 deletions crates/re_viewer_context/src/space_view/view_part_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use re_types::ComponentNameSet;
use crate::{
IdentifiedViewSystem, SpaceViewClassIdentifier, SpaceViewSystemExecutionError,
ViewContextCollection, ViewQuery, ViewSystemIdentifier, ViewerContext,
VisualizerAdditionalApplicabilityFilter,
};

/// This is additional context made available to the `heuristic_filter`.
Expand Down Expand Up @@ -81,6 +82,13 @@ pub trait ViewPartSystem: Send + Sync + 'static {
default_heuristic_filter(entity_components, &self.indicator_components())
}

/// Additional filter for applicability.
///
/// If none is specified, applicability is solely determined by required components.
fn applicability_filter(&self) -> Option<Box<dyn VisualizerAdditionalApplicabilityFilter>> {
None
}

/// Queries the data store and performs data conversions to make it ready for display.
///
/// Mustn't query any data outside of the archetype.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,45 @@ pub struct VisualizerEntitySubscriber {
required_components_indices: IntMap<ComponentName, usize>,

per_store_mapping: HashMap<StoreId, VisualizerEntityMapping>,

/// Additional filter for applicability.
applicability_filter: Box<dyn VisualizerAdditionalApplicabilityFilter>,
}

/// Additional filter for applicability on top of the default check for required components.
pub trait VisualizerAdditionalApplicabilityFilter: Send + Sync {
/// Updates the internal applicability filter state based on the given events.
///
/// Called for every update no matter whether the entity is already has all required components or not.
///
/// Returns true if the entity changed in the event is now applicable to the visualizer, false otherwise.
/// Once a entity passes this filter, it can never go back to being filtered out.
/// **This implies that the filter does not _need_ to be stateful.**
/// It is perfectly fine to return `true` only if something in the diff is regarded as applicable and false otherwise.
/// (However, if necessary, the applicability filter *can* keep track of state.)
fn update_applicability(&mut self, _event: &re_arrow_store::StoreEvent) -> bool;
}

struct DefaultVisualizerApplicabilityFilter;

impl VisualizerAdditionalApplicabilityFilter for DefaultVisualizerApplicabilityFilter {
#[inline]
fn update_applicability(&mut self, _event: &re_arrow_store::StoreEvent) -> bool {
true
}
}

#[derive(Default)]
struct VisualizerEntityMapping {
/// For each entity, which of the required components are present.
///
/// Last bit is used for the applicability filter.
///
/// In order of `required_components`.
/// If all bits are set, the entity is applicable to the visualizer.
// TODO(andreas): We could just limit the number of required components to 32 or 64 and
// then use a single u32/u64 as a bitmap.
required_component_bitmap_per_entity: IntMap<EntityPathHash, BitVec>,
required_component_and_filter_bitmap_per_entity: IntMap<EntityPathHash, BitVec>,

/// Which entities the visualizer can be applied to.
///
Expand All @@ -59,6 +87,9 @@ impl VisualizerEntitySubscriber {
.map(|(i, name)| (name, i))
.collect(),
per_store_mapping: Default::default(),
applicability_filter: visualizer
.applicability_filter()
.unwrap_or_else(|| Box::new(DefaultVisualizerApplicabilityFilter)),
}
}

Expand Down Expand Up @@ -102,10 +133,10 @@ impl StoreSubscriber for VisualizerEntitySubscriber {
.or_default();

let required_components_bitmap = store_mapping
.required_component_bitmap_per_entity
.required_component_and_filter_bitmap_per_entity
.entry(event.diff.entity_path.hash())
.or_insert_with(|| {
BitVec::from_elem(self.required_components_indices.len(), false)
BitVec::from_elem(self.required_components_indices.len() + 1, false)
});

if required_components_bitmap.all() {
Expand All @@ -119,6 +150,15 @@ impl StoreSubscriber for VisualizerEntitySubscriber {
}
}

let bit_index_for_filter = self.required_components_indices.len();
let custom_filter = required_components_bitmap[bit_index_for_filter];
if !custom_filter {
required_components_bitmap.set(
bit_index_for_filter,
self.applicability_filter.update_applicability(event),
);
}

if required_components_bitmap.all() {
re_log::debug!(
"Entity {:?} in store {:?} is now applicable to visualizer {:?}",
Expand Down

0 comments on commit a85a268

Please sign in to comment.