Model Extra Bytes column width

CHANGELOG.md

@@ -5,6 +5,9 @@
 - add LAS/COPC-native column types in `copc-core`, materialized column reads in
   `copc-reader`, and a `copc-writer` `ColumnBatchSource` adapter for writing
   neutral `LasColumnBatch` values directly
+- model LAS Extra Bytes columns as fixed-width byte data with explicit
+  per-point width metadata, so `ColumnSelection::all()` and point-format
+  layouts can include Extra Bytes without rejecting valid multi-byte records
 - keep row iteration supported while documenting that column reads are owned,
   materialized buffers decoded from compressed LAZ chunks, not zero-copy views
   into COPC files

copc-core/src/columns.rs

@@ -170,24 +170,46 @@ pub enum ScalarType {
 pub struct ColumnSpec {
     pub dimension: LasDimension,
     pub scalar: ScalarType,
+    /// For `LasDimension::ExtraBytes`, the fixed byte count stored for each point.
+    pub byte_width: Option<usize>,
 }
 
 impl ColumnSpec {
     pub const fn new(dimension: LasDimension, scalar: ScalarType) -> Self {
-        Self { dimension, scalar }
+        Self {
+            dimension,
+            scalar,
+            byte_width: None,
+        }
+    }
+
+    pub const fn extra_bytes(byte_width: usize) -> Self {
+        Self {
+            dimension: LasDimension::ExtraBytes,
+            scalar: ScalarType::U8,
+            byte_width: Some(byte_width),
+        }
     }
 
     /// Returns the default fixed LAS/COPC scalar for `dimension`, when it has one.
     pub const fn default_for(dimension: LasDimension) -> Option<Self> {
         match dimension.default_scalar() {
-            Some(scalar) => Some(Self { dimension, scalar }),
+            Some(scalar) => Some(Self {
+                dimension,
+                scalar,
+                byte_width: None,
+            }),
             None => None,
         }
     }
 
     /// Returns whether this specification has the canonical scalar for its dimension.
     pub const fn has_default_scalar(self) -> bool {
-        self.dimension.accepts_scalar(self.scalar)
+        if matches!(self.dimension, LasDimension::ExtraBytes) {
+            matches!(self.scalar, ScalarType::U8) && self.byte_width.is_some()
+        } else {
+            self.byte_width.is_none() && self.dimension.accepts_scalar(self.scalar)
+        }
     }
 
     /// Returns whether `data` has the scalar type declared by this spec.
@@ -197,16 +219,26 @@ impl ColumnSpec {
 
     /// Validate the declared scalar against the supplied data.
     pub fn validate_data(self, data: &ColumnData) -> Result<()> {
-        if self.matches_data(data) {
-            Ok(())
-        } else {
-            Err(Error::InvalidInput(format!(
+        if !self.matches_data(data) {
+            return Err(Error::InvalidInput(format!(
                 "column {:?} declares {:?} data but contains {:?}",
                 self.dimension,
                 self.scalar,
                 data.scalar()
-            )))
+            )));
         }
+        if self.dimension == LasDimension::ExtraBytes && self.extra_byte_width().is_none() {
+            return Err(Error::InvalidInput(
+                "ExtraBytes column requires a non-zero byte width".into(),
+            ));
+        }
+        if self.dimension != LasDimension::ExtraBytes && self.byte_width.is_some() {
+            return Err(Error::InvalidInput(format!(
+                "column {:?} cannot declare byte width {:?}",
+                self.dimension, self.byte_width
+            )));
+        }
+        Ok(())
     }
 
     /// Validate that this spec uses the fixed LAS/COPC scalar for its dimension.
@@ -222,6 +254,31 @@ impl ColumnSpec {
             )))
         }
     }
+
+    pub fn extra_byte_width(self) -> Option<usize> {
+        match (self.dimension, self.scalar, self.byte_width) {
+            (LasDimension::ExtraBytes, ScalarType::U8, Some(width)) if width > 0 => Some(width),
+            _ => None,
+        }
+    }
+
+    pub fn point_count_for_data(self, data: &ColumnData) -> Result<usize> {
+        self.validate_data(data)?;
+        if self.dimension == LasDimension::ExtraBytes {
+            let width = self.extra_byte_width().ok_or_else(|| {
+                Error::InvalidInput("ExtraBytes column requires a non-zero byte width".into())
+            })?;
+            if data.len() % width != 0 {
+                return Err(Error::InvalidInput(format!(
+                    "ExtraBytes column has {} bytes, which is not divisible by byte width {width}",
+                    data.len()
+                )));
+            }
+            Ok(data.len() / width)
+        } else {
+            Ok(data.len())
+        }
+    }
 }
 
 /// Owned column values.
@@ -411,9 +468,8 @@ pub fn layout_for_las_format(format: LasPointFormat) -> Vec<ColumnSpec> {
         );
     }
     if format.extra_bytes > 0 {
-        columns.push(ColumnSpec::new(LasDimension::ExtraBytes, ScalarType::U8));
+        columns.push(ColumnSpec::extra_bytes(usize::from(format.extra_bytes)));
     }
-
     columns
 }
 
@@ -443,7 +499,10 @@ pub struct LasColumnBatch {
 
 impl LasColumnBatch {
     pub fn new(columns: Vec<(ColumnSpec, ColumnData)>) -> Result<Self> {
-        let len = columns.first().map(|(_, data)| data.len()).unwrap_or(0);
+        let len = match columns.first() {
+            Some((spec, data)) => spec.point_count_for_data(data)?,
+            None => 0,
+        };
         let batch = Self { len, columns };
         batch.validate()?;
         Ok(batch)
@@ -480,13 +539,11 @@ impl LasColumnBatch {
     /// Validate scalar declarations and column lengths for this batch.
     pub fn validate(&self) -> Result<()> {
         for (spec, data) in &self.columns {
-            spec.validate_data(data)?;
-            if data.len() != self.len {
+            let point_count = spec.point_count_for_data(data)?;
+            if point_count != self.len {
                 return Err(Error::InvalidInput(format!(
-                    "column {:?} has {} values but batch len is {}",
-                    spec.dimension,
-                    data.len(),
-                    self.len
+                    "column {:?} has {} points but batch len is {}",
+                    spec.dimension, point_count, self.len
                 )));
             }
         }
@@ -605,13 +662,43 @@ mod tests {
         assert!(batch.validate().is_err());
     }
 
+    #[test]
+    fn batch_validates_fixed_width_extra_bytes() {
+        let batch = LasColumnBatch::new(vec![(
+            ColumnSpec::extra_bytes(3),
+            ColumnData::U8(vec![1, 2, 3, 4, 5, 6]),
+        )])
+        .unwrap();
+
+        assert_eq!(2, batch.len());
+        assert_eq!(
+            Some(&ColumnData::U8(vec![1, 2, 3, 4, 5, 6])),
+            batch.column(LasDimension::ExtraBytes)
+        );
+
+        let invalid = LasColumnBatch::new(vec![(
+            ColumnSpec::extra_bytes(3),
+            ColumnData::U8(vec![1, 2, 3, 4]),
+        )]);
+        assert!(invalid.is_err());
+
+        let missing_width = LasColumnBatch::new(vec![(
+            ColumnSpec::new(LasDimension::ExtraBytes, ScalarType::U8),
+            ColumnData::U8(vec![1, 2, 3]),
+        )]);
+        assert!(missing_width.is_err());
+    }
+
     #[test]
     fn default_scalar_validation_allows_extra_bytes() {
         assert_eq!(
             ColumnSpec::new(LasDimension::GpsTime, ScalarType::F64),
             ColumnSpec::default_for(LasDimension::GpsTime).unwrap()
         );
-        assert!(ColumnSpec::new(LasDimension::ExtraBytes, ScalarType::I32).has_default_scalar());
+        assert!(ColumnSpec::extra_bytes(4).has_default_scalar());
+        assert!(ColumnSpec::new(LasDimension::ExtraBytes, ScalarType::U8)
+            .validate_default_scalar()
+            .is_err());
         assert!(
             ColumnSpec::new(LasDimension::ScanAngleRank, ScalarType::F32)
                 .validate_default_scalar()
@@ -727,15 +814,12 @@ mod tests {
     }
 
     #[test]
-    fn layout_includes_extra_bytes_when_format_declares_them() {
+    fn layout_includes_extra_bytes_with_byte_width_when_format_declares_them() {
         let mut format = LasPointFormat::new(0).unwrap();
         format.extra_bytes = 4;
 
         let layout = layout_for_las_format(format);
 
-        assert_eq!(
-            Some(&ColumnSpec::new(LasDimension::ExtraBytes, ScalarType::U8)),
-            layout.last()
-        );
+        assert_eq!(Some(&ColumnSpec::extra_bytes(4)), layout.last());
     }
 }

copc-reader/src/points.rs

@@ -226,15 +226,11 @@ fn selected_column_builders(
     layout_for_las_format(point_format)
         .into_iter()
         .filter(|spec| selection.contains(spec.dimension))
-        .map(|spec| empty_column(spec, capacity, point_format.extra_bytes))
+        .map(|spec| empty_column(spec, capacity))
         .collect()
 }
 
-fn empty_column(
-    spec: ColumnSpec,
-    capacity: usize,
-    extra_bytes: u16,
-) -> Result<(ColumnSpec, ColumnData)> {
+fn empty_column(spec: ColumnSpec, capacity: usize) -> Result<(ColumnSpec, ColumnData)> {
     let data = match spec.scalar {
         copc_core::ScalarType::F64 => ColumnData::F64(Vec::with_capacity(capacity)),
         copc_core::ScalarType::F32 => ColumnData::F32(Vec::with_capacity(capacity)),
@@ -246,11 +242,16 @@ fn empty_column(
         copc_core::ScalarType::U32 => ColumnData::U32(Vec::with_capacity(capacity)),
         copc_core::ScalarType::U16 => ColumnData::U16(Vec::with_capacity(capacity)),
         copc_core::ScalarType::U8 => {
-            if spec.dimension == LasDimension::ExtraBytes && extra_bytes > 1 {
-                return Err(Error::Unsupported(format!(
-                    "materialized ExtraBytes columns require one byte per point, got {extra_bytes}"
-                )));
-            }
+            let capacity = if spec.dimension == LasDimension::ExtraBytes {
+                let width = spec.extra_byte_width().ok_or_else(|| {
+                    Error::InvalidInput("ExtraBytes column requires a non-zero byte width".into())
+                })?;
+                capacity.checked_mul(width).ok_or_else(|| {
+                    Error::InvalidInput("ExtraBytes column capacity exceeds usize range".into())
+                })?
+            } else {
+                capacity
+            };
             ColumnData::U8(Vec::with_capacity(capacity))
         }
         copc_core::ScalarType::Bool => ColumnData::Bool(Vec::with_capacity(capacity)),
@@ -287,7 +288,7 @@ fn append_columns(
     };
 
     for (spec, data) in columns {
-        append_column(spec.dimension, data, &context)?;
+        append_column(*spec, data, &context)?;
     }
     Ok(())
 }
@@ -303,10 +304,11 @@ struct ColumnAppendContext<'a> {
 }
 
 fn append_column(
-    dimension: LasDimension,
+    spec: ColumnSpec,
     data: &mut ColumnData,
     context: &ColumnAppendContext<'_>,
 ) -> Result<()> {
+    let dimension = spec.dimension;
     let scalar = data.scalar();
     match (dimension, data) {
         (LasDimension::X, ColumnData::F64(values)) => values.push(context.xyz.0),
@@ -404,7 +406,16 @@ fn append_column(
             );
         }
         (LasDimension::ExtraBytes, ColumnData::U8(values)) => {
-            values.push(context.raw_point.extra_bytes.first().copied().unwrap_or(0));
+            let width = spec.extra_byte_width().ok_or_else(|| {
+                Error::InvalidData("ExtraBytes column requires a non-zero byte width".into())
+            })?;
+            if context.raw_point.extra_bytes.len() != width {
+                return Err(Error::InvalidData(format!(
+                    "ExtraBytes point has {} bytes, expected {width}",
+                    context.raw_point.extra_bytes.len()
+                )));
+            }
+            values.extend_from_slice(&context.raw_point.extra_bytes);
         }
         _ => {
             return Err(Error::InvalidData(format!(
@@ -731,3 +742,75 @@ fn voxel_bounds(key: copc_core::VoxelKey, info: &CopcInfo) -> Result<Bounds> {
     );
     Ok(Bounds::new(min, (min.0 + side, min.1 + side, min.2 + side)))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn selected_column_builders_include_extra_bytes_width() {
+        let mut format = LasPointFormat::new(6).unwrap();
+        format.extra_bytes = 3;
+
+        let columns = selected_column_builders(format, ColumnSelection::all(), 2).unwrap();
+
+        let extra_spec = columns
+            .iter()
+            .map(|(spec, _)| *spec)
+            .find(|spec| spec.dimension == LasDimension::ExtraBytes)
+            .expect("ExtraBytes column spec");
+        assert_eq!(Some(3), extra_spec.extra_byte_width());
+        assert_eq!(copc_core::ScalarType::U8, extra_spec.scalar);
+    }
+
+    #[test]
+    fn append_columns_preserves_fixed_width_extra_bytes() {
+        let mut format = LasPointFormat::new(6).unwrap();
+        format.extra_bytes = 3;
+        let mut columns = selected_column_builders(
+            format,
+            ColumnSelection::from_dimensions([LasDimension::X, LasDimension::ExtraBytes]),
+            1,
+        )
+        .unwrap();
+        let raw_point = las::raw::Point {
+            x: 10,
+            y: 20,
+            z: 30,
+            flags: las::raw::point::Flags::ThreeByte(1 | (1 << 4), 0, 2),
+            scan_angle: las::raw::point::ScanAngle::from(0.0),
+            extra_bytes: vec![9, 8, 7],
+            ..Default::default()
+        };
+
+        append_columns(&mut columns, &raw_point, (1.0, 2.0, 3.0)).unwrap();
+        let batch = LasColumnBatch::new(columns).unwrap();
+
+        assert_eq!(1, batch.len());
+        assert_eq!(
+            Some(&ColumnData::U8(vec![9, 8, 7])),
+            batch.column(LasDimension::ExtraBytes)
+        );
+    }
+
+    #[test]
+    fn append_columns_rejects_wrong_extra_bytes_width() {
+        let mut format = LasPointFormat::new(6).unwrap();
+        format.extra_bytes = 3;
+        let mut columns = selected_column_builders(
+            format,
+            ColumnSelection::from_dimensions([LasDimension::ExtraBytes]),
+            1,
+        )
+        .unwrap();
+        let raw_point = las::raw::Point {
+            flags: las::raw::point::Flags::ThreeByte(1 | (1 << 4), 0, 2),
+            extra_bytes: vec![9, 8],
+            ..Default::default()
+        };
+
+        let err = append_columns(&mut columns, &raw_point, (1.0, 2.0, 3.0)).unwrap_err();
+
+        assert!(err.to_string().contains("expected 3"));
+    }
+}