Skip to content

Commit

Permalink
Fix FixedSizeList deserialization edge-case + trivial optimizations (
Browse files Browse the repository at this point in the history
…#2673)

Deserializing a `FixedSizeList` itself contained within an empty list
would fail; a situation that actually occurs all the time when using the
transform types.

The fix is trivial, and also added a few early out in there to optimize
that path.

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2673) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2673)
- [Docs
preview](https://rerun.io/preview/pr%3Acmc%2Ffixed_size_list_deser_bug/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Ffixed_size_list_deser_bug/examples)
  • Loading branch information
teh-cmc authored Jul 12, 2023
1 parent 05ff805 commit cb39601
Show file tree
Hide file tree
Showing 21 changed files with 2,938 additions and 2,714 deletions.
5 changes: 5 additions & 0 deletions crates/re_types/definitions/rerun/testing/datatypes/fuzzy.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ table SurprisinglyShallowScalar (transparent, order: 001) {
value: FlattenedScalar (order: 100);
}

struct ArrayOfFloats (transparent, order: 001) {
values: [float: 3] (order: 100);
}

table AffixFuzzer1 (
"attr.rust.derive": "PartialEq",
order: 100
Expand Down Expand Up @@ -57,6 +61,7 @@ union AffixFuzzer3 (
degrees: FlattenedScalar (transparent, order: 100),
radians: FlattenedScalar (transparent, nullable, order: 101),
craziness: __AffixFuzzer1Vec (transparent, order: 102),
fixed_size_shenanigans: ArrayOfFloats (transparent, order: 103),
}

table __AffixFuzzer3 (transparent, order: 0) {
Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/source_hash.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# This is a sha256 hash for all direct and indirect dependencies of this crate's build script.
# It can be safely removed at anytime to force the build script to run again.
# Check out build.rs to see how it's computed.
934a605d9f5de40dea31cf0b9c34bac1fa4e0a6f0852251371faeb965511f6b3
478bc67049484da40beb2a0e8fa97aae5ffe1d0a29bd1e30024815ae844956be
1,733 changes: 826 additions & 907 deletions crates/re_types/src/components/fuzzy.rs

Large diffs are not rendered by default.

108 changes: 56 additions & 52 deletions crates/re_types/src/datatypes/angle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,61 +200,65 @@ impl crate::Datatype for Angle {
expected: data.data_type().clone(),
got: data.data_type().clone(),
})?;
let (data_types, data_arrays, data_offsets) =
(data.types(), data.fields(), data.offsets().unwrap());
let radians = {
let data = &*data_arrays[0usize];
if data.is_empty() {
Vec::new()
} else {
let (data_types, data_arrays, data_offsets) =
(data.types(), data.fields(), data.offsets().unwrap());
let radians = {
let data = &*data_arrays[0usize];

data.as_any()
.downcast_ref::<Float32Array>()
.unwrap()
.into_iter()
.map(|v| v.copied())
.collect::<Vec<_>>()
};
let degrees = {
let data = &*data_arrays[1usize];
data.as_any()
.downcast_ref::<Float32Array>()
.unwrap()
.into_iter()
.map(|v| v.copied())
.collect::<Vec<_>>()
};
let degrees = {
let data = &*data_arrays[1usize];

data.as_any()
.downcast_ref::<Float32Array>()
.unwrap()
.into_iter()
.map(|v| v.copied())
.collect::<Vec<_>>()
};
data_types
.iter()
.enumerate()
.map(|(i, typ)| {
let offset = data_offsets[i];
data.as_any()
.downcast_ref::<Float32Array>()
.unwrap()
.into_iter()
.map(|v| v.copied())
.collect::<Vec<_>>()
};
data_types
.iter()
.enumerate()
.map(|(i, typ)| {
let offset = data_offsets[i];

Ok(Some(match typ {
0i8 => Angle::Radians(
radians
.get(offset as usize)
.ok_or_else(|| crate::DeserializationError::OffsetsMismatch {
bounds: (offset as usize, offset as usize),
len: radians.len(),
datatype: data.data_type().clone(),
})?
.clone()
.unwrap(),
),
1i8 => Angle::Degrees(
degrees
.get(offset as usize)
.ok_or_else(|| crate::DeserializationError::OffsetsMismatch {
bounds: (offset as usize, offset as usize),
len: degrees.len(),
datatype: data.data_type().clone(),
})?
.clone()
.unwrap(),
),
_ => unreachable!(),
}))
})
.collect::<crate::DeserializationResult<Vec<_>>>()?
Ok(Some(match typ {
0i8 => Angle::Radians(
radians
.get(offset as usize)
.ok_or_else(|| crate::DeserializationError::OffsetsMismatch {
bounds: (offset as usize, offset as usize),
len: radians.len(),
datatype: data.data_type().clone(),
})?
.clone()
.unwrap(),
),
1i8 => Angle::Degrees(
degrees
.get(offset as usize)
.ok_or_else(|| crate::DeserializationError::OffsetsMismatch {
bounds: (offset as usize, offset as usize),
len: degrees.len(),
datatype: data.data_type().clone(),
})?
.clone()
.unwrap(),
),
_ => unreachable!(),
}))
})
.collect::<crate::DeserializationResult<Vec<_>>>()?
}
})
}
}
Loading

0 comments on commit cb39601

Please sign in to comment.