Skip to content

Commit

Permalink
Fix for corner case with min and max of integers being equal
Browse files Browse the repository at this point in the history
  • Loading branch information
cry-inc committed Mar 18, 2024
1 parent 25f716a commit fe17713
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 15 deletions.
59 changes: 50 additions & 9 deletions src/queue_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,24 @@ impl<'a, T: Read + Seek> QueueReader<'a, T> {
self.byte_streams[i].append(&self.buffer);
}

self.parse_byte_streams()?;
// Find smallest number of expected items in any queue after stream unpacking.
// This is required for the corner case when the bit size of an record
// is zero and we don't know how many items to "unpack" from an empty buffer.
let mut min_queue_size = usize::MAX;
for (i, bs) in self.buffer_sizes.iter().enumerate() {
let bit_size = self.pc.prototype[i].data_type.bit_size();
// We can only check records with a non-zero bit size
if bit_size != 0 {
let bs_items = (bs * 8) / bit_size;
let queue_items = self.queues[i].len();
let items = bs_items + queue_items;
if items < min_queue_size {
min_queue_size = items;
}
}
}

self.parse_byte_streams(min_queue_size)?;
}
};

Expand All @@ -116,7 +133,7 @@ impl<'a, T: Read + Seek> QueueReader<'a, T> {
}

/// Extracts raw values from byte streams into queues.
fn parse_byte_streams(&mut self) -> Result<()> {
fn parse_byte_streams(&mut self, min_queue_size: usize) -> Result<()> {
for (i, r) in self.pc.prototype.iter().enumerate() {
match r.data_type {
RecordDataType::Single { .. } => {
Expand All @@ -125,14 +142,38 @@ impl<'a, T: Read + Seek> QueueReader<'a, T> {
RecordDataType::Double { .. } => {
BitPack::unpack_doubles(&mut self.byte_streams[i], &mut self.queues[i])?
}
RecordDataType::ScaledInteger { min, max, .. } => BitPack::unpack_scaled_ints(
&mut self.byte_streams[i],
min,
max,
&mut self.queues[i],
)?,
RecordDataType::ScaledInteger { min, max, .. } => {
if r.data_type.bit_size() == 0 {
// If the bit size of an record is zero, we don't know how many items to unpack.
// Thats because they are not really unpacked, but instead generated with a zero value.
// We use the supplied minimal size to ensure that we create enough items
// to fill the queue enough to not be the limiting queue between all records.
while self.queues[i].len() < min_queue_size {
self.queues[i].push_back(RecordValue::ScaledInteger(min));
}
} else {
BitPack::unpack_scaled_ints(
&mut self.byte_streams[i],
min,
max,
&mut self.queues[i],
)?
}
}
RecordDataType::Integer { min, max } => {
BitPack::unpack_ints(&mut self.byte_streams[i], min, max, &mut self.queues[i])?
if r.data_type.bit_size() == 0 {
// See comment above for scaled integers!
while self.queues[i].len() < min_queue_size {
self.queues[i].push_back(RecordValue::Integer(min));
}
} else {
BitPack::unpack_ints(
&mut self.byte_streams[i],
min,
max,
&mut self.queues[i],
)?
}
}
};
}
Expand Down
16 changes: 10 additions & 6 deletions src/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub enum RecordName {

/// Represents a raw value of attributes inside a point cloud.
///
/// For scaled integers the record data type with the scale is needed to calulcate the actual f64 value.
/// For scaled integers the record data type with the scale and offset is needed to calculate the actual f64 value.
#[derive(Clone, Debug, PartialEq)]
pub enum RecordValue {
Single(f32),
Expand Down Expand Up @@ -220,9 +220,9 @@ impl RecordDataType {
optional_attribute(node, "minimum", tag_name, type_name)?.unwrap_or(i64::MIN);
let max =
optional_attribute(node, "maximum", tag_name, type_name)?.unwrap_or(i64::MAX);
if max <= min {
if max < min {
Error::invalid(format!(
"Maximum value '{max}' and minimum value '{min}' of type '{type_name}' in XML tag '{tag_name}' are inconsistent"
"Maximum value '{max}' and minimum value '{min}' of type '{type_name}' in XML tag '{tag_name}' are invalid"
))?
}
RecordDataType::Integer { min, max }
Expand All @@ -232,9 +232,9 @@ impl RecordDataType {
optional_attribute(node, "minimum", tag_name, type_name)?.unwrap_or(i64::MIN);
let max =
optional_attribute(node, "maximum", tag_name, type_name)?.unwrap_or(i64::MAX);
if max <= min {
if max < min {
Error::invalid(format!(
"Maximum value '{max}' and minimum value '{min}' of type '{type_name}' in XML tag '{tag_name}' are inconsistent"
"Maximum value '{max}' and minimum value '{min}' of type '{type_name}' in XML tag '{tag_name}' are invalid"
))?
}
let scale = optional_attribute(node, "scale", tag_name, type_name)?.unwrap_or(1.0);
Expand Down Expand Up @@ -426,7 +426,11 @@ fn serialize_integer(value: i64, min: i64, max: i64, buffer: &mut ByteStreamWrit
#[inline]
fn integer_bits(min: i64, max: i64) -> usize {
let range = max as i128 - min as i128;
range.ilog2() as usize + 1
if range > 0 {
range.ilog2() as usize + 1
} else {
0
}
}

fn optional_attribute<T>(
Expand Down

0 comments on commit fe17713

Please sign in to comment.