diff --git a/Cargo.lock b/Cargo.lock index 9c7b83fba189..dd9d9c2e46c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10050,6 +10050,7 @@ dependencies = [ "js-sys", "parking_lot", "poll-promise", + "rayon", "re_analytics", "re_auth", "re_blueprint_tree", diff --git a/crates/viewer/re_test_viewport/src/lib.rs b/crates/viewer/re_test_viewport/src/lib.rs index 30ae4671983f..8268d82c479c 100644 --- a/crates/viewer/re_test_viewport/src/lib.rs +++ b/crates/viewer/re_test_viewport/src/lib.rs @@ -89,16 +89,16 @@ impl TestContextExt for TestContext { let view_blueprint = viewport_blueprint .view(view_id) .expect("view is known to exist"); + + let class_registry = ctx.view_class_registry(); let class_identifier = view_blueprint.class_identifier(); + let class = class_registry.class(class_identifier).unwrap_or_else(|| panic!("The class '{class_identifier}' must be registered beforehand")); - let visualizable_entities = ctx - .view_class_registry() - .class(class_identifier) - .unwrap_or_else(|| panic!("The class '{class_identifier}' must be registered beforehand")) + let visualizable_entities = class .determine_visualizable_entities( ctx.maybe_visualizable_entities_per_visualizer, ctx.recording(), - &ctx.view_class_registry() + &class_registry .new_visualizer_collection(class_identifier), &view_blueprint.space_origin, ); @@ -109,14 +109,14 @@ impl TestContextExt for TestContext { let mut data_query_result = view_blueprint.contents.execute_query( ctx.store_context, - ctx.view_class_registry(), + class_registry, ctx.blueprint_query, &visualizable_entities, ); let resolver = DataQueryPropertyResolver::new( view_blueprint, - ctx.view_class_registry(), + class_registry, ctx.maybe_visualizable_entities_per_visualizer, &visualizable_entities, &indicated_entities_per_visualizer, @@ -126,9 +126,9 @@ impl TestContextExt for TestContext { ctx.store_context.blueprint, ctx.blueprint_query, ctx.rec_cfg.time_ctrl.read().timeline(), - ctx.view_class_registry(), + class_registry, &mut data_query_result, - &mut self.view_states.lock(), + self.view_states.lock().get_mut_or_create(*view_id, class), ); query_results.insert(*view_id, data_query_result); diff --git a/crates/viewer/re_viewer/Cargo.toml b/crates/viewer/re_viewer/Cargo.toml index 876c43050ad5..ca9f322d5618 100644 --- a/crates/viewer/re_viewer/Cargo.toml +++ b/crates/viewer/re_viewer/Cargo.toml @@ -133,6 +133,7 @@ itertools.workspace = true jiff.workspace = true parking_lot.workspace = true poll-promise = { workspace = true, features = ["web"] } +rayon.workspace = true rfd.workspace = true ron.workspace = true serde = { workspace = true, features = ["derive"] } diff --git a/crates/viewer/re_viewer/src/app_state.rs b/crates/viewer/re_viewer/src/app_state.rs index adebfc1491e0..df6c6bcb8807 100644 --- a/crates/viewer/re_viewer/src/app_state.rs +++ b/crates/viewer/re_viewer/src/app_state.rs @@ -3,7 +3,7 @@ use std::{borrow::Cow, str::FromStr as _}; use ahash::HashMap; use egui::{Ui, text_edit::TextEditState, text_selection::LabelSelectionState}; -use re_chunk::TimelineName; +use re_chunk::{Timeline, TimelineName}; use re_chunk_store::LatestAtQuery; use re_entity_db::EntityDb; use re_log_types::{AbsoluteTimeRangeF, DataSourceMessage, StoreId, TableId}; @@ -14,9 +14,10 @@ use re_types::blueprint::components::PanelState; use re_ui::{ContextExt as _, UiExt as _}; use re_viewer_context::{ AppOptions, ApplicationSelectionState, AsyncRuntimeHandle, BlueprintUndoState, CommandSender, - ComponentUiRegistry, DisplayMode, DragAndDropManager, GlobalContext, Item, PlayState, - RecordingConfig, SelectionChange, StorageContext, StoreContext, StoreHub, SystemCommand, - SystemCommandSender as _, TableStore, ViewClassRegistry, ViewStates, ViewerContext, + ComponentUiRegistry, DataQueryResult, DisplayMode, DragAndDropManager, GlobalContext, + IndicatedEntities, Item, MaybeVisualizableEntities, PerVisualizer, PlayState, RecordingConfig, + SelectionChange, StorageContext, StoreContext, StoreHub, SystemCommand, + SystemCommandSender as _, TableStore, ViewClassRegistry, ViewId, ViewStates, ViewerContext, blueprint_timeline, open_url::{self, ViewerOpenUrl}, }; @@ -278,7 +279,7 @@ impl AppState { view_class_registry.indicated_entities_per_visualizer(recording.store_id()); // Execute the queries for every `View` - let mut query_results = { + let query_results = { re_tracing::profile_scope!("query_results"); viewport_ui .blueprint @@ -359,39 +360,18 @@ impl AppState { // Update the viewport. May spawn new views and handle queued requests (like screenshots). viewport_ui.on_frame_start(&ctx); - for view in viewport_ui.blueprint.views.values() { - if let Some(query_result) = query_results.get_mut(&view.id) { - // TODO(andreas): This needs to be done in a store subscriber that exists per view (instance, not class!). - // Note that right now we determine *all* visualizable entities, not just the queried ones. - // In a store subscriber set this is fine, but on a per-frame basis it's wasteful. - let visualizable_entities = view - .class(view_class_registry) - .determine_visualizable_entities( - &maybe_visualizable_entities_per_visualizer, - recording, - &view_class_registry - .new_visualizer_collection(view.class_identifier()), - &view.space_origin, - ); - - let resolver = re_viewport_blueprint::DataQueryPropertyResolver::new( - view, - view_class_registry, - &maybe_visualizable_entities_per_visualizer, - &visualizable_entities, - &indicated_entities_per_visualizer, - ); - - resolver.update_overrides( - store_context.blueprint, - &blueprint_query, - rec_cfg.time_ctrl.read().timeline(), - view_class_registry, - query_result, - view_states, - ); - } - } + let active_timeline = *rec_cfg.time_ctrl.read().timeline(); + let query_results = update_overrides( + store_context, + query_results, + view_class_registry, + &viewport_ui.blueprint, + &blueprint_query, + &active_timeline, + &maybe_visualizable_entities_per_visualizer, + &indicated_entities_per_visualizer, + view_states, + ); // We need to recreate the context to appease the borrow checker. It is a bit annoying, but // it's just a bunch of refs so not really that big of a deal in practice. @@ -760,6 +740,93 @@ impl AppState { } } +/// Updates the query results for the given viewport UI. +/// +/// Returns query results derived from the previous one. +#[expect(clippy::too_many_arguments)] +fn update_overrides( + store_context: &StoreContext<'_>, + mut query_results: HashMap, + view_class_registry: &ViewClassRegistry, + viewport_blueprint: &ViewportBlueprint, + blueprint_query: &LatestAtQuery, + active_timeline: &Timeline, + maybe_visualizable_entities_per_visualizer: &PerVisualizer, + indicated_entities_per_visualizer: &PerVisualizer, + view_states: &mut ViewStates, +) -> HashMap { + use rayon::iter::{IntoParallelIterator as _, ParallelIterator as _}; + + struct OverridesUpdateTask<'a> { + view: &'a re_viewport_blueprint::ViewBlueprint, + view_state: &'a dyn re_viewer_context::ViewState, + query_result: DataQueryResult, + } + + for view in viewport_blueprint.views.values() { + view_states.ensure_state_exists(view.id, view.class(view_class_registry)); + } + + let work_items = viewport_blueprint + .views + .values() + .filter_map(|view| { + query_results.remove(&view.id).map(|query_result| { + let view_state = view_states + .get(view.id) + .expect("View state should exist, we just called ensure_state_exists on it."); + OverridesUpdateTask { + view, + view_state, + query_result, + } + }) + }) + .collect::>(); + + work_items + .into_par_iter() + .map( + |OverridesUpdateTask { + view, + view_state, + mut query_result, + }| { + // TODO(andreas): This needs to be done in a store subscriber that exists per view (instance, not class!). + // Note that right now we determine *all* visualizable entities, not just the queried ones. + // In a store subscriber set this is fine, but on a per-frame basis it's wasteful. + let visualizable_entities = view + .class(view_class_registry) + .determine_visualizable_entities( + maybe_visualizable_entities_per_visualizer, + store_context.recording, + &view_class_registry.new_visualizer_collection(view.class_identifier()), + &view.space_origin, + ); + + let resolver = re_viewport_blueprint::DataQueryPropertyResolver::new( + view, + view_class_registry, + maybe_visualizable_entities_per_visualizer, + &visualizable_entities, + indicated_entities_per_visualizer, + ); + + resolver.update_overrides( + store_context.blueprint, + blueprint_query, + active_timeline, + view_class_registry, + &mut query_result, + view_state, + ); + + (view.id, query_result) + }, + ) + .collect() +} + fn table_ui( ctx: &ViewerContext<'_>, runtime: &AsyncRuntimeHandle, diff --git a/crates/viewer/re_viewer_context/src/lib.rs b/crates/viewer/re_viewer_context/src/lib.rs index 99e6a375f3ee..b913999816a6 100644 --- a/crates/viewer/re_viewer_context/src/lib.rs +++ b/crates/viewer/re_viewer_context/src/lib.rs @@ -84,11 +84,12 @@ pub use self::{ DataBasedVisualizabilityFilter, DataResult, IdentifiedViewSystem, OptionalViewEntityHighlight, OverridePath, PerSystemDataResults, PerSystemEntities, PropertyOverrides, RecommendedView, SmallVisualizerSet, SystemExecutionOutput, ViewClass, - ViewClassExt, ViewClassLayoutPriority, ViewClassRegistry, ViewClassRegistryError, - ViewContext, ViewContextCollection, ViewContextSystem, ViewEntityHighlight, ViewHighlights, - ViewOutlineMasks, ViewQuery, ViewSpawnHeuristics, ViewState, ViewStateExt, ViewStates, - ViewSystemExecutionError, ViewSystemIdentifier, ViewSystemRegistrator, - VisualizableFilterContext, VisualizerCollection, VisualizerQueryInfo, VisualizerSystem, + ViewClassExt, ViewClassLayoutPriority, ViewClassPlaceholder, ViewClassRegistry, + ViewClassRegistryError, ViewContext, ViewContextCollection, ViewContextSystem, + ViewEntityHighlight, ViewHighlights, ViewOutlineMasks, ViewQuery, ViewSpawnHeuristics, + ViewState, ViewStateExt, ViewStates, ViewSystemExecutionError, ViewSystemIdentifier, + ViewSystemRegistrator, VisualizableFilterContext, VisualizerCollection, + VisualizerQueryInfo, VisualizerSystem, }, viewer_context::{RecordingConfig, ViewerContext}, visitor_flow_control::VisitorControlFlow, diff --git a/crates/viewer/re_viewer_context/src/view/mod.rs b/crates/viewer/re_viewer_context/src/view/mod.rs index 9ec110fcfda9..b43f284e6c40 100644 --- a/crates/viewer/re_viewer_context/src/view/mod.rs +++ b/crates/viewer/re_viewer_context/src/view/mod.rs @@ -28,6 +28,7 @@ pub use view_class::{ ViewClass, ViewClassExt, ViewClassLayoutPriority, ViewState, ViewStateExt, VisualizableFilterContext, }; +pub use view_class_placeholder::ViewClassPlaceholder; pub use view_class_registry::{ViewClassRegistry, ViewClassRegistryError, ViewSystemRegistrator}; pub use view_context::ViewContext; pub use view_context_system::{ViewContextCollection, ViewContextSystem}; diff --git a/crates/viewer/re_viewer_context/src/view/view_class_registry.rs b/crates/viewer/re_viewer_context/src/view/view_class_registry.rs index 210eabec637f..9ed35c39889b 100644 --- a/crates/viewer/re_viewer_context/src/view/view_class_registry.rs +++ b/crates/viewer/re_viewer_context/src/view/view_class_registry.rs @@ -297,7 +297,7 @@ impl ViewClassRegistry { ) } - /// For each visualizer, the set of entities that have at least one matching indicator component. + /// For each visualizer, the set of entities that have at least one component with a matching archetype name. pub fn indicated_entities_per_visualizer( &self, store_id: &re_log_types::StoreId, diff --git a/crates/viewer/re_viewer_context/src/view/visualizer_entity_subscriber.rs b/crates/viewer/re_viewer_context/src/view/visualizer_entity_subscriber.rs index bb6e6431991e..4c266bea4f00 100644 --- a/crates/viewer/re_viewer_context/src/view/visualizer_entity_subscriber.rs +++ b/crates/viewer/re_viewer_context/src/view/visualizer_entity_subscriber.rs @@ -124,12 +124,12 @@ impl VisualizerEntitySubscriber { .map(|mapping| &mapping.maybe_visualizable_entities) } - /// List of entities that at some point in time had any of the indicator components advertised by this visualizer. + /// List of entities that at some point in time had a component of an archetypes matching the visualizer's query. /// /// Useful for quickly evaluating basic "should this visualizer apply by default"-heuristic. /// Does *not* imply that any of the given entities is also in the (maybe-)visualizable-set! /// - /// If the visualizer has no indicator components, this list will contain all entities in the store. + /// If the visualizer has no archetypes, this list will contain all entities in the store. pub fn indicated_entities(&self, store: &StoreId) -> Option<&IndicatedEntities> { self.per_store_mapping .get(store) diff --git a/crates/viewer/re_viewport_blueprint/src/view.rs b/crates/viewer/re_viewport_blueprint/src/view.rs index 4a08e6bd165a..be1136de9356 100644 --- a/crates/viewer/re_viewport_blueprint/src/view.rs +++ b/crates/viewer/re_viewport_blueprint/src/view.rs @@ -463,7 +463,7 @@ mod tests { use re_types::{ComponentDescriptor, blueprint::archetypes::EntityBehavior}; use re_viewer_context::{ IndicatedEntities, MaybeVisualizableEntities, OverridePath, PerVisualizer, - VisualizableEntities, + ViewClassPlaceholder, VisualizableEntities, }; use crate::view_contents::DataQueryPropertyResolver; @@ -516,7 +516,8 @@ mod tests { ); // Basic blueprint - a single view that queries everything. - let view = ViewBlueprint::new_with_root_wildcard("3D".into()); + test_ctx.register_view_class::(); + let view = ViewBlueprint::new_with_root_wildcard(ViewClassPlaceholder::identifier()); let override_root = ViewContents::override_path_for_entity(view.id, &EntityPath::root()); // Things needed to resolve properties: @@ -764,6 +765,12 @@ mod tests { visualizable_entities, ); let mut view_states = ViewStates::default(); + let view_state = view_states.get_mut_or_create( + view.id, + ctx.view_class_registry + .class(view.class_identifier()) + .expect("view class should be registered"), + ); resolver.update_overrides( ctx.blueprint_db(), @@ -771,7 +778,7 @@ mod tests { ctx.rec_cfg.time_ctrl.read().timeline(), ctx.view_class_registry(), &mut query_result, - &mut view_states, + view_state, ); result = Some(query_result.clone()); diff --git a/crates/viewer/re_viewport_blueprint/src/view_contents.rs b/crates/viewer/re_viewport_blueprint/src/view_contents.rs index ea475369cbb9..1a286260f3e9 100644 --- a/crates/viewer/re_viewport_blueprint/src/view_contents.rs +++ b/crates/viewer/re_viewport_blueprint/src/view_contents.rs @@ -22,7 +22,7 @@ use re_types::{ use re_viewer_context::{ DataQueryResult, DataResult, DataResultHandle, DataResultNode, DataResultTree, IndicatedEntities, MaybeVisualizableEntities, OverridePath, PerVisualizer, PropertyOverrides, - QueryRange, ViewClassRegistry, ViewId, ViewStates, ViewerContext, VisualizableEntities, + QueryRange, ViewClassRegistry, ViewId, ViewState, ViewerContext, VisualizableEntities, }; use crate::{ViewBlueprint, ViewProperty}; @@ -462,6 +462,8 @@ impl<'a> DataQueryPropertyResolver<'a> { parent_visible: bool, parent_interactive: bool, ) { + // This is called very frequently, don't put a profile scope here. + let Some(node) = query_result.tree.lookup_node_mut(handle) else { return; }; @@ -594,14 +596,11 @@ impl<'a> DataQueryPropertyResolver<'a> { active_timeline: &Timeline, view_class_registry: &ViewClassRegistry, query_result: &mut DataQueryResult, - view_states: &mut ViewStates, + view_state: &dyn ViewState, ) { - // This is called very frequently, don't put a profile scope here. + re_tracing::profile_function!(); if let Some(root) = query_result.tree.root_handle() { - let class = self.view.class(view_class_registry); - let view_state = view_states.get_mut_or_create(self.view.id, class); - let default_query_range = self.view.query_range( blueprint, blueprint_query,