Skip to content

Commit

Permalink
review
Browse files Browse the repository at this point in the history
  • Loading branch information
teh-cmc committed Apr 8, 2024
1 parent 422e5bf commit 96d427a
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 51 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.

1 change: 1 addition & 0 deletions crates/re_query2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ itertools = { workspace = true }
nohash-hasher.workspace = true
serde = { workspace = true, features = ["derive", "rc"], optional = true }
smallvec.workspace = true
static_assertions.workspace = true
thiserror.workspace = true

# Optional:
Expand Down
4 changes: 2 additions & 2 deletions crates/re_query2/benches/latest_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ fn query_and_visit_points(store: &DataStore, paths: &[EntityPath]) -> Vec<SavePo
);

let points = results.get_required::<Position2D>().unwrap();
let colors = results.get_optional::<Color>();
let colors = results.get_or_empty::<Color>();

let points = points
.iter_dense::<Position2D>(&resolver)
Expand Down Expand Up @@ -325,7 +325,7 @@ fn query_and_visit_strings(store: &DataStore, paths: &[EntityPath]) -> Vec<SaveS
);

let points = results.get_required::<Position2D>().unwrap();
let colors = results.get_optional::<Text>();
let colors = results.get_or_empty::<Text>();

let points = points
.iter_dense::<Position2D>(&resolver)
Expand Down
6 changes: 3 additions & 3 deletions crates/re_query2/examples/latest_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ fn main() -> anyhow::Result<()> {
// _component batch_ itself (that says nothing about its _instances_!).
//
// * `get_required` returns an error if the component batch is missing
// * `get_optional` returns an empty set of results if the component if missing
// * `get_or_empty` returns an empty set of results if the component if missing
// * `get` returns an option
let points: &LatestAtComponentResults = results.get_required::<MyPoint>()?;
let colors: &LatestAtComponentResults = results.get_optional::<MyColor>();
let labels: &LatestAtComponentResults = results.get_optional::<MyLabel>();
let colors: &LatestAtComponentResults = results.get_or_empty::<MyColor>();
let labels: &LatestAtComponentResults = results.get_or_empty::<MyLabel>();

// Then comes the time to resolve/convert and deserialize the data.
// These steps have to be done together for efficiency reasons.
Expand Down
24 changes: 8 additions & 16 deletions crates/re_query2/src/latest_at/results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{Promise, PromiseResolver, PromiseResult};
/// The data is neither deserialized, nor resolved/converted.
/// It it the raw [`DataCell`]s, straight from our datastore.
///
/// Use [`LatestAtResults::get`], [`LatestAtResults::get_required`] and [`LatestAtResults::get_optional`]
/// Use [`LatestAtResults::get`], [`LatestAtResults::get_required`] and [`LatestAtResults::get_or_empty`]
/// in order to access the raw results for each individual component.
#[derive(Debug, Clone)]
pub struct LatestAtResults {
Expand Down Expand Up @@ -69,11 +69,11 @@ impl LatestAtResults {
///
/// Returns empty results if the component is not present.
#[inline]
pub fn get_optional<C: Component>(&self) -> &LatestAtComponentResults {
pub fn get_or_empty<C: Component>(&self) -> &LatestAtComponentResults {
if let Some(component) = self.components.get(&C::name()) {
component
} else {
static DEFAULT: LatestAtComponentResults = LatestAtComponentResults::empty();
static DEFAULT: LatestAtComponentResults = LatestAtComponentResults::EMPTY;
&DEFAULT
}
}
Expand Down Expand Up @@ -120,18 +120,15 @@ pub struct LatestAtComponentResults {
impl Default for LatestAtComponentResults {
#[inline]
fn default() -> Self {
Self::empty()
Self::EMPTY
}
}

impl LatestAtComponentResults {
#[inline]
const fn empty() -> Self {
Self {
index: (TimeInt::STATIC, RowId::ZERO),
promise: None,
}
}
const EMPTY: Self = Self {
index: (TimeInt::STATIC, RowId::ZERO),
promise: None,
};
}

impl LatestAtComponentResults {
Expand Down Expand Up @@ -188,11 +185,6 @@ impl LatestAtComponentResults {
&self,
resolver: &PromiseResolver,
) -> PromiseResult<DeserializationResult<Vec<Option<C>>>> {
// Manufactured empty result.
if self.promise.is_none() {
return PromiseResult::Ready(Ok(vec![]));
}

if let Some(cell) = self.promise.as_ref() {
resolver.resolve(cell).map(|cell| {
cell.try_to_native_opt()
Expand Down
2 changes: 2 additions & 0 deletions crates/re_query2/src/promise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub struct Promise {
source: DataCell,
}

static_assertions::assert_eq_size!(Promise, Option<Promise>);

impl Promise {
#[inline]
pub fn new(source: DataCell) -> Self {
Expand Down
42 changes: 12 additions & 30 deletions crates/re_query2/tests/latest_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,10 @@ fn simple_query() -> anyhow::Result<()> {
];

let points = results.get_required::<MyPoint>()?;
let point_data = points
.iter_dense::<MyPoint>(&resolver)
.flatten()
.unwrap();

let colors = results.get_optional::<MyColor>();
let color_data = colors
.iter_sparse::<MyColor>(&resolver)
.flatten()
.unwrap();
let point_data = points.iter_dense::<MyPoint>(&resolver).flatten().unwrap();

let colors = results.get_or_empty::<MyColor>();
let color_data = colors.iter_sparse::<MyColor>(&resolver).flatten().unwrap();
let color_default_fn = || Some(MyColor::from(0xFF00FFFF));

let (got_points, got_colors): (Vec<_>, Vec<_>) =
Expand Down Expand Up @@ -137,16 +131,10 @@ fn static_query() -> anyhow::Result<()> {
];

let points = results.get_required::<MyPoint>()?;
let point_data = points
.iter_dense::<MyPoint>(&resolver)
.flatten()
.unwrap();

let colors = results.get_optional::<MyColor>();
let color_data = colors
.iter_sparse::<MyColor>(&resolver)
.flatten()
.unwrap();
let point_data = points.iter_dense::<MyPoint>(&resolver).flatten().unwrap();

let colors = results.get_or_empty::<MyColor>();
let color_data = colors.iter_sparse::<MyColor>(&resolver).flatten().unwrap();
let color_default_fn = || Some(MyColor::from(0xFF00FFFF));

let (got_points, got_colors): (Vec<_>, Vec<_>) =
Expand Down Expand Up @@ -212,16 +200,10 @@ fn no_instance_join_query() -> anyhow::Result<()> {
];

let points = results.get_required::<MyPoint>()?;
let point_data = points
.iter_dense::<MyPoint>(&resolver)
.flatten()
.unwrap();

let colors = results.get_optional::<MyColor>();
let color_data = colors
.iter_sparse::<MyColor>(&resolver)
.flatten()
.unwrap();
let point_data = points.iter_dense::<MyPoint>(&resolver).flatten().unwrap();

let colors = results.get_or_empty::<MyColor>();
let color_data = colors.iter_sparse::<MyColor>(&resolver).flatten().unwrap();
let color_default_fn = || Some(MyColor::from(0xFF00FFFF));

let (got_points, got_colors): (Vec<_>, Vec<_>) =
Expand Down

0 comments on commit 96d427a

Please sign in to comment.