Skip to content

Commit 12cde30

Browse files
grtlrWumpf
andauthored
Use ComponentDescriptor in HybridLatestAtResult (#9938)
### Related * Part of #6889. * Builds on #9929. * Requires #9933. ### What Title. --------- Co-authored-by: Andreas Reich <[email protected]> Co-authored-by: Andreas Reich <[email protected]>
1 parent 8d3ad7f commit 12cde30

File tree

26 files changed

+185
-174
lines changed

26 files changed

+185
-174
lines changed

crates/store/re_chunk/src/chunk.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,8 @@ impl ChunkComponents {
103103
}
104104

105105
/// Whether any of the components in this chunk has the given name.
106-
pub fn contains_component_name(&self, component_name: ComponentName) -> bool {
107-
self.0
108-
.keys()
109-
.any(|desc| desc.component_name == component_name)
106+
pub fn contains_component(&self, component_descr: &ComponentDescriptor) -> bool {
107+
self.0.contains_key(component_descr)
110108
}
111109

112110
/// Whether any of the components in this chunk is tagged with the given archetype name.

crates/store/re_chunk_store/src/query.rs

Lines changed: 6 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ use re_chunk::{Chunk, LatestAtQuery, RangeQuery, TimelineName};
1010
use re_log_types::ResolvedTimeRange;
1111
use re_log_types::{EntityPath, TimeInt, Timeline};
1212
use re_types_core::{
13-
ComponentDescriptor, ComponentDescriptorSet, ComponentName, ComponentNameSet,
14-
UnorderedComponentDescriptorSet,
13+
ComponentDescriptor, ComponentDescriptorSet, ComponentName, UnorderedComponentDescriptorSet,
1514
};
1615

1716
use crate::{ChunkStore, store::ChunkIdSetPerTime};
@@ -146,56 +145,6 @@ impl ChunkStore {
146145
}
147146
}
148147

149-
/// Retrieve all the [`ComponentName`]s that have been written to for a given [`EntityPath`] on
150-
/// the specified [`Timeline`].
151-
///
152-
/// Static components are always included in the results.
153-
///
154-
/// Returns `None` if the entity doesn't exist at all on this `timeline`.
155-
// TODO(#6889): Remove in favor of `all_components_on_timeline_sorted`.
156-
pub fn all_components_on_timeline_sorted_by_name(
157-
&self,
158-
timeline: &TimelineName,
159-
entity_path: &EntityPath,
160-
) -> Option<ComponentNameSet> {
161-
re_tracing::profile_function!();
162-
163-
let static_components: Option<ComponentNameSet> = self
164-
.static_chunk_ids_per_entity
165-
.get(entity_path)
166-
.map(|static_chunks_per_component| {
167-
static_chunks_per_component
168-
.keys()
169-
.map(|descr| descr.component_name)
170-
.collect::<ComponentNameSet>()
171-
})
172-
.filter(|names| !names.is_empty());
173-
174-
let temporal_components: Option<ComponentNameSet> = self
175-
.temporal_chunk_ids_per_entity_per_component
176-
.get(entity_path)
177-
.map(|temporal_chunk_ids_per_timeline| {
178-
temporal_chunk_ids_per_timeline
179-
.get(timeline)
180-
.map(|temporal_chunk_ids_per_component| {
181-
temporal_chunk_ids_per_component
182-
.keys()
183-
.map(|descr| descr.component_name)
184-
.collect::<ComponentNameSet>()
185-
})
186-
.unwrap_or_default()
187-
})
188-
.filter(|names| !names.is_empty());
189-
190-
match (static_components, temporal_components) {
191-
(None, None) => None,
192-
(None, Some(comps)) | (Some(comps), None) => Some(comps),
193-
(Some(static_comps), Some(temporal_comps)) => {
194-
Some(static_comps.into_iter().chain(temporal_comps).collect())
195-
}
196-
}
197-
}
198-
199148
/// Retrieve all the [`ComponentName`]s that have been written to for a given [`EntityPath`] on
200149
/// the specified [`Timeline`].
201150
///
@@ -292,29 +241,24 @@ impl ChunkStore {
292241
pub fn all_components_for_entity_sorted(
293242
&self,
294243
entity_path: &EntityPath,
295-
) -> Option<ComponentNameSet> {
244+
) -> Option<ComponentDescriptorSet> {
296245
re_tracing::profile_function!();
297246

298-
let static_components: Option<ComponentNameSet> = self
247+
let static_components: Option<ComponentDescriptorSet> = self
299248
.static_chunk_ids_per_entity
300249
.get(entity_path)
301250
.map(|static_chunks_per_component| {
302-
static_chunks_per_component
303-
.keys()
304-
.map(|descr| descr.component_name)
305-
.collect()
251+
static_chunks_per_component.keys().cloned().collect()
306252
});
307253

308-
let temporal_components: Option<ComponentNameSet> = self
254+
let temporal_components: Option<ComponentDescriptorSet> = self
309255
.temporal_chunk_ids_per_entity_per_component
310256
.get(entity_path)
311257
.map(|temporal_chunk_ids_per_timeline| {
312258
temporal_chunk_ids_per_timeline
313259
.iter()
314260
.flat_map(|(_, temporal_chunk_ids_per_component)| {
315-
temporal_chunk_ids_per_component
316-
.keys()
317-
.map(|descr| descr.component_name)
261+
temporal_chunk_ids_per_component.keys().cloned()
318262
})
319263
.collect()
320264
});

crates/store/re_chunk_store/tests/reads.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use re_log_types::{
1212
example_components::{MyColor, MyIndex, MyPoint},
1313
};
1414
use re_types::{
15-
ComponentDescriptor, ComponentNameSet,
15+
ComponentDescriptor, ComponentDescriptorSet,
1616
testing::{LargeStruct, build_some_large_structs},
1717
};
1818
use re_types_core::Component as _;
@@ -57,12 +57,10 @@ fn all_components() -> anyhow::Result<()> {
5757
|store: &ChunkStore, entity_path: &EntityPath, expected: Option<&[ComponentDescriptor]>| {
5858
let timeline = TimelineName::new("frame_nr");
5959

60-
let component_names =
61-
store.all_components_on_timeline_sorted_by_name(&timeline, entity_path);
60+
let component_names = store.all_components_on_timeline_sorted(&timeline, entity_path);
6261

6362
let expected_component_names = expected.map(|expected| {
64-
let expected: ComponentNameSet =
65-
expected.iter().map(|desc| desc.component_name).collect();
63+
let expected: ComponentDescriptorSet = expected.iter().cloned().collect();
6664
expected
6765
});
6866

crates/store/re_query/src/latest_at.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,9 +436,8 @@ impl LatestAtResults {
436436
&self,
437437
log_level: re_log::Level,
438438
instance_index: usize,
439+
component_descr: &ComponentDescriptor,
439440
) -> Option<C> {
440-
let component_descr = self.find_component_descriptor(C::name())?;
441-
442441
self.components.get(component_descr).and_then(|unit| {
443442
self.ok_or_log_err(
444443
log_level,
@@ -452,8 +451,17 @@ impl LatestAtResults {
452451
///
453452
/// Logs an error if the data cannot be deserialized, or if the instance index is out of bounds.
454453
#[inline]
455-
pub fn component_instance<C: Component>(&self, instance_index: usize) -> Option<C> {
456-
self.component_instance_with_log_level(re_log::Level::Error, instance_index)
454+
pub fn component_instance<C: Component>(
455+
&self,
456+
instance_index: usize,
457+
component_descr: &ComponentDescriptor,
458+
) -> Option<C> {
459+
debug_assert_eq!(component_descr.component_name, C::name());
460+
self.component_instance_with_log_level(
461+
re_log::Level::Error,
462+
instance_index,
463+
component_descr,
464+
)
457465
}
458466

459467
/// Returns the deserialized data for the specified component at the given instance index.

crates/store/re_types_core/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ pub use self::{
4444
component_descriptor::ComponentDescriptor,
4545
id::{ChunkId, RowId},
4646
loggable::{
47-
Component, ComponentDescriptorSet, ComponentName, ComponentNameSet, DatatypeName, Loggable,
47+
Component, ComponentDescriptorSet, ComponentName, DatatypeName, Loggable,
4848
UnorderedComponentDescriptorSet,
4949
},
5050
loggable_batch::{

crates/store/re_types_core/src/loggable.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,6 @@ pub trait Component: Loggable {
123123

124124
pub type UnorderedComponentDescriptorSet = IntSet<ComponentDescriptor>;
125125

126-
pub type ComponentNameSet = std::collections::BTreeSet<ComponentName>;
127-
128126
pub type ComponentDescriptorSet = std::collections::BTreeSet<ComponentDescriptor>;
129127

130128
re_string_interner::declare_new_type!(

crates/viewer/re_view/src/annotation_scene_context.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use re_types::{Archetype as _, ComponentNameSet, archetypes::AnnotationContext};
1+
use re_types::{Archetype as _, ComponentDescriptorSet, archetypes::AnnotationContext};
22
use re_viewer_context::{
33
AnnotationMap, IdentifiedViewSystem, ViewContextSystem, ViewSystemIdentifier,
44
};
@@ -13,12 +13,12 @@ impl IdentifiedViewSystem for AnnotationSceneContext {
1313
}
1414

1515
impl ViewContextSystem for AnnotationSceneContext {
16-
fn compatible_component_sets(&self) -> Vec<ComponentNameSet> {
16+
fn compatible_component_sets(&self) -> Vec<ComponentDescriptorSet> {
1717
vec![
1818
AnnotationContext::required_components()
1919
.iter()
20-
.map(|descr| descr.component_name)
21-
.collect(), //
20+
.cloned()
21+
.collect(),
2222
]
2323
}
2424

crates/viewer/re_view/src/results_ext.rs

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::{borrow::Cow, sync::Arc};
22

33
use itertools::Itertools as _;
44

5-
use re_chunk_store::{Chunk, LatestAtQuery, RangeQuery, UnitChunkShared};
5+
use re_chunk_store::{Chunk, LatestAtQuery, RangeQuery};
66
use re_log_types::hash::Hash64;
77
use re_query::{LatestAtResults, RangeResults};
88
use re_types::ComponentDescriptor;
@@ -39,16 +39,7 @@ pub struct HybridRangeResults<'a> {
3939
}
4040

4141
impl HybridLatestAtResults<'_> {
42-
/// Returns the [`UnitChunkShared`] for the specified [`re_types_core::Component`].
43-
#[inline]
44-
pub fn get(&self, component_name: impl Into<ComponentName>) -> Option<&UnitChunkShared> {
45-
let component_name = component_name.into();
46-
self.overrides
47-
.get_by_name(&component_name)
48-
.or_else(|| self.results.get_by_name(&component_name))
49-
.or_else(|| self.defaults.get_by_name(&component_name))
50-
}
51-
42+
// TODO(#6889): Right now, fallbacks are on a per-component basis, so it's fine to pass the component name here.
5243
pub fn fallback_raw(&self, component_name: ComponentName) -> arrow::array::ArrayRef {
5344
let query_context = QueryContext {
5445
viewer_ctx: self.ctx.viewer_ctx,
@@ -68,41 +59,66 @@ impl HybridLatestAtResults<'_> {
6859

6960
/// Utility for retrieving the first instance of a component, ignoring defaults.
7061
#[inline]
71-
pub fn get_required_mono<C: re_types_core::Component>(&self) -> Option<C> {
72-
self.get_required_instance(0)
62+
pub fn get_required_mono<C: re_types_core::Component>(
63+
&self,
64+
component_descr: &ComponentDescriptor,
65+
) -> Option<C> {
66+
self.get_required_instance(0, component_descr)
7367
}
7468

7569
/// Utility for retrieving the first instance of a component.
7670
#[inline]
77-
pub fn get_mono<C: re_types_core::Component>(&self) -> Option<C> {
78-
self.get_instance(0)
71+
pub fn get_mono<C: re_types_core::Component>(
72+
&self,
73+
component_descr: &ComponentDescriptor,
74+
) -> Option<C> {
75+
self.get_instance(0, component_descr)
7976
}
8077

8178
/// Utility for retrieving the first instance of a component.
8279
#[inline]
83-
pub fn get_mono_with_fallback<C: re_types_core::Component + Default>(&self) -> C {
84-
self.get_instance_with_fallback(0)
80+
pub fn get_mono_with_fallback<C: re_types_core::Component + Default>(
81+
&self,
82+
component_descr: &ComponentDescriptor,
83+
) -> C {
84+
debug_assert_eq!(component_descr.component_name, C::name());
85+
86+
self.get_instance_with_fallback(0, component_descr)
8587
}
8688

8789
/// Utility for retrieving a single instance of a component, not checking for defaults.
8890
///
8991
/// If overrides or defaults are present, they will only be used respectively if they have a component at the specified index.
9092
#[inline]
91-
pub fn get_required_instance<C: re_types_core::Component>(&self, index: usize) -> Option<C> {
92-
self.overrides.component_instance::<C>(index).or_else(||
93+
pub fn get_required_instance<C: re_types_core::Component>(
94+
&self,
95+
index: usize,
96+
component_descr: &ComponentDescriptor,
97+
) -> Option<C> {
98+
self.overrides
99+
.component_instance::<C>(index, component_descr)
100+
.or_else(||
93101
// No override -> try recording store instead
94-
self.results.component_instance::<C>(index))
102+
self.results.component_instance::<C>(index, component_descr))
95103
}
96104

97105
/// Utility for retrieving a single instance of a component.
98106
///
99107
/// If overrides or defaults are present, they will only be used respectively if they have a component at the specified index.
100108
#[inline]
101-
pub fn get_instance<C: re_types_core::Component>(&self, index: usize) -> Option<C> {
102-
self.get_required_instance(index).or_else(|| {
103-
// No override & no store -> try default instead
104-
self.defaults.component_instance::<C>(index)
105-
})
109+
pub fn get_instance<C: re_types_core::Component>(
110+
&self,
111+
index: usize,
112+
component_descr: &ComponentDescriptor,
113+
) -> Option<C> {
114+
debug_assert_eq!(component_descr.component_name, C::name());
115+
116+
self.get_required_instance(index, component_descr)
117+
.or_else(|| {
118+
// No override & no store -> try default instead
119+
self.defaults
120+
.component_instance::<C>(index, component_descr)
121+
})
106122
}
107123

108124
/// Utility for retrieving a single instance of a component.
@@ -112,11 +128,15 @@ impl HybridLatestAtResults<'_> {
112128
pub fn get_instance_with_fallback<C: re_types_core::Component + Default>(
113129
&self,
114130
index: usize,
131+
component_descr: &ComponentDescriptor,
115132
) -> C {
116-
self.get_instance(index)
133+
debug_assert_eq!(component_descr.component_name, C::name());
134+
135+
let component_name = component_descr.component_name;
136+
self.get_instance(index, component_descr)
117137
.or_else(|| {
118138
// No override, no store, no default -> try fallback instead
119-
let raw_fallback = self.fallback_raw(C::name());
139+
let raw_fallback = self.fallback_raw(component_name);
120140
C::from_arrow(raw_fallback.as_ref())
121141
.ok()
122142
.and_then(|r| r.first().cloned())

crates/viewer/re_view_bar_chart/src/visualizer_system.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ impl VisualizerSystem for BarChartVisualizerSystem {
5959
let results = data_result
6060
.latest_at_with_blueprint_resolved_data::<BarChart>(ctx, &timeline_query);
6161

62-
let Some(tensor) = results.get_required_mono::<components::TensorData>() else {
62+
let Some(tensor) =
63+
results.get_required_mono::<components::TensorData>(&BarChart::descriptor_values())
64+
else {
6365
continue;
6466
};
6567

6668
if tensor.is_vector() {
67-
let color = results.get_mono_with_fallback();
69+
let color = results.get_mono_with_fallback(&BarChart::descriptor_color());
6870
self.charts
6971
.insert(data_result.entity_path.clone(), (tensor.0.clone(), color));
7072
}

0 commit comments

Comments
 (0)