Fixed PACK implementation for read/write

src/dot_vox_data.rs

@@ -26,7 +26,7 @@ impl DotVoxData {
 
         // Write out all of the children of MAIN first to get the number of bytes.
         let mut children_buffer = Vec::new();
-        self.write_models(&mut children_buffer)?;
+        self.write_models(&mut children_buffer, false)?;
         self.write_palette_chunk(&mut children_buffer)?;
         let num_main_children_bytes = children_buffer.len() as u32;
 
@@ -48,8 +48,8 @@ impl DotVoxData {
         Self::write_chunk(writer, "MAIN", &[], num_children_bytes)
     }
 
-    fn write_models<W: Write>(&self, writer: &mut W) -> Result<(), io::Error> {
-        if self.models.len() > 1 {
+    fn write_models<W: Write>(&self, writer: &mut W, should_pack: bool) -> Result<(), io::Error> {
+        if self.models.len() > 1 && should_pack {
             self.write_pack_chunk(writer)?;
         }
         for model in self.models.iter() {

src/model.rs

@@ -17,7 +17,11 @@ pub struct Model {
 impl Model {
     /// Number of bytes when encoded in `.vox` format.
     pub fn num_vox_bytes(&self) -> u32 {
-        12 + 4 * self.voxels.len() as u32
+        // The number 40 comes from:
+        // - 24 bytes for the chunk header format (SIZE/XYZI labels, chunk and child sizes, etc.)
+        // - 12 bytes for the SIZE contents (x, y, z)
+        // - 4 bytes for the voxel length u32
+        40 + 4 * self.voxels.len() as u32
     }
 }
 

src/parser.rs

@@ -25,7 +25,7 @@ pub enum Chunk {
     Main(Vec<Chunk>),
     Size(Size),
     Voxels(Vec<Voxel>),
-    Pack(Model),
+    Pack(Vec<Chunk>),
     Palette(Vec<Color>),
     Material(Material),
     TransformNode(SceneTransform),
@@ -214,7 +214,19 @@ fn map_chunk_to_data(version: u32, main: Chunk) -> DotVoxData {
                             models.push(Model { size, voxels })
                         }
                     }
-                    Chunk::Pack(model) => models.push(model),
+                    Chunk::Pack(packed_chunks) => {
+                        for packed_chunk in packed_chunks {
+                            match packed_chunk {
+                                Chunk::Size(size) => size_holder = Some(size),
+                                Chunk::Voxels(voxels) => {
+                                    if let Some(size) = size_holder {
+                                        models.push(Model { size, voxels })
+                                    }
+                                }
+                                _ => debug!("Unmapped chunk {:?}", packed_chunk),
+                            }
+                        }
+                    }
                     Chunk::Palette(palette) => palette_holder = palette,
                     Chunk::Material(material) => materials.push(material),
                     Chunk::TransformNode(scene_transform) => {
@@ -280,12 +292,16 @@ fn parse_chunk(i: &[u8]) -> IResult<&[u8], Chunk> {
     Ok((i, chunk))
 }
 
+fn parse_num_models(i: &[u8]) -> IResult<&[u8], u32> {
+    let (i, num_models) = le_u32(i)?;
+    Ok((i, num_models))
+}
+
 fn build_chunk(id: &str, chunk_content: &[u8], children_size: u32, child_content: &[u8]) -> Chunk {
     if children_size == 0 {
         match id {
             "SIZE" => build_size_chunk(chunk_content),
             "XYZI" => build_voxel_chunk(chunk_content),
-            "PACK" => build_pack_chunk(chunk_content),
             "RGBA" => build_palette_chunk(chunk_content),
             "MATL" => build_material_chunk(chunk_content),
             "nTRN" => build_scene_transform_chunk(chunk_content),
@@ -308,7 +324,7 @@ fn build_chunk(id: &str, chunk_content: &[u8], children_size: u32, child_content
         };
         match id {
             "MAIN" => Chunk::Main(child_chunks),
-            "PACK" => build_pack_chunk(chunk_content),
+            "PACK" => build_pack_chunk(chunk_content, child_chunks),
             _ => {
                 debug!("Unknown chunk with children {:?}", id);
                 Chunk::Unknown(id.to_owned())
@@ -331,19 +347,20 @@ fn build_palette_chunk(chunk_content: &[u8]) -> Chunk {
     Chunk::Invalid(chunk_content.to_vec())
 }
 
-// NOTE: this does not seem consistent with the PACK documentation.  However,
-// files with PACK chunks seem rare. It's likely this hasn't been tested.  Is
-// this correct?
-fn build_pack_chunk(chunk_content: &[u8]) -> Chunk {
-    if let Ok((chunk_content, Chunk::Size(size))) = parse_chunk(chunk_content) {
-        if let Ok((_, Chunk::Voxels(voxels))) = parse_chunk(chunk_content) {
-            return Chunk::Pack(Model {
-                size,
-                voxels: voxels.to_vec(),
-            });
-        }
+fn build_pack_chunk(chunk_content: &[u8], child_chunks: Vec<Chunk>) -> Chunk {
+    let num_models = match parse_num_models(chunk_content) {
+        Ok((_, num_models)) => num_models,
+        _ => 0,
+    };
+
+    // This check is to make sure the PACK chunk is valid according
+    // to the vox documentation. It's divided by 2 because each model
+    // has a SIZE and XYZI chunk.
+    if num_models >= 1 && child_chunks.len() as u32 / 2 == num_models {
+        Chunk::Pack(child_chunks)
+    } else {
+        Chunk::Invalid(chunk_content.to_vec())
     }
-    Chunk::Invalid(chunk_content.to_vec())
 }
 
 fn build_size_chunk(chunk_content: &[u8]) -> Chunk {