From fe177137c1c678bf28079d45e40afc7eff299000 Mon Sep 17 00:00:00 2001 From: cry-inc Date: Mon, 18 Mar 2024 17:10:07 +0100 Subject: [PATCH] Fix for corner case with min and max of integers being equal --- src/queue_reader.rs | 59 ++++++++++++++++++++++++++++++++++++++------- src/record.rs | 16 +++++++----- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/src/queue_reader.rs b/src/queue_reader.rs index 51ee183..dad76ea 100644 --- a/src/queue_reader.rs +++ b/src/queue_reader.rs @@ -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)?; } }; @@ -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 { .. } => { @@ -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], + )? + } } }; } diff --git a/src/record.rs b/src/record.rs index 7bf7b88..3095c01 100644 --- a/src/record.rs +++ b/src/record.rs @@ -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), @@ -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 } @@ -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); @@ -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(