Merge branch 'refactor-cell' into 'master'

refactor Cell back to 4 byte size #993

Closes #993

See merge request veloren/veloren!1997

common/src/figure/cell.rs

@@ -1,23 +1,28 @@
+use std::num::NonZeroU8;
+
 use crate::vol::Vox;
 use vek::*;
 
-pub(super) const GLOWY: u8 = 1 << 0;
-pub(super) const SHINY: u8 = 1 << 1;
+const GLOWY: u8 = 1 << 1;
+const SHINY: u8 = 1 << 2;
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
-#[repr(packed)]
 pub struct CellData {
     pub col: Rgb<u8>,
-    pub attr: u8, // 0 = glowy, 1 = shiny
+    pub attr: NonZeroU8, // 1 = glowy, 2 = shiny
+}
+
+impl CellData {
+    pub(super) fn new(col: Rgb<u8>, glowy: bool, shiny: bool) -> Self {
+        CellData {
+            col,
+            attr: NonZeroU8::new(1 + glowy as u8 * GLOWY + shiny as u8 * SHINY).unwrap(),
+        }
+    }
 }
 
 impl Default for CellData {
-    fn default() -> Self {
-        Self {
-            col: Rgb::broadcast(255),
-            attr: 0,
-        }
-    }
+    fn default() -> Self { Self::new(Rgb::broadcast(255), false, false) }
 }
 
 /// A type representing a single voxel in a figure.
@@ -29,10 +34,7 @@ pub enum Cell {
 
 impl Cell {
     pub fn new(col: Rgb<u8>, glowy: bool, shiny: bool) -> Self {
-        Cell::Filled(CellData {
-            col,
-            attr: glowy as u8 * GLOWY + shiny as u8 * SHINY,
-        })
+        Cell::Filled(CellData::new(col, glowy, shiny))
     }
 
     pub fn get_color(&self) -> Option<Rgb<u8>> {
@@ -44,14 +46,14 @@ impl Cell {
 
     pub fn is_glowy(&self) -> bool {
         match self {
-            Cell::Filled(data) => data.attr & GLOWY != 0,
+            Cell::Filled(data) => data.attr.get() & GLOWY != 0,
             Cell::Empty => false,
         }
     }
 
     pub fn is_shiny(&self) -> bool {
         match self {
-            Cell::Filled(data) => data.attr & SHINY != 0,
+            Cell::Filled(data) => data.attr.get() & SHINY != 0,
             Cell::Empty => false,
         }
     }
@@ -67,3 +69,14 @@ impl Vox for Cell {
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn cell_size() {
+        assert_eq!(4, std::mem::size_of::<Cell>());
+        assert_eq!(1, std::mem::align_of::<Cell>());
+    }
+}

common/src/figure/mat_cell.rs

@@ -27,3 +27,14 @@ impl Vox for MatCell {
 
     fn is_empty(&self) -> bool { matches!(self, MatCell::None) }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn met_cell_size() {
+        assert_eq!(5, std::mem::size_of::<MatCell>());
+        assert_eq!(1, std::mem::align_of::<MatCell>());
+    }
+}

common/src/figure/mod.rs

@@ -8,7 +8,6 @@ pub use self::{
     mat_cell::MatCell,
 };
 
-use self::cell::{GLOWY, SHINY};
 use crate::{
     vol::{IntoFullPosIterator, IntoFullVolIterator, ReadVol, SizedVol, Vox, WriteVol},
     volumes::dyna::Dyna,
@@ -159,10 +158,7 @@ impl MatSegment {
         for (pos, vox) in self.full_vol_iter() {
             let data = match vox {
                 MatCell::None => continue,
-                MatCell::Mat(mat) => CellData {
-                    col: map(*mat),
-                    attr: 0,
-                },
+                MatCell::Mat(mat) => CellData::new(map(*mat), false, false),
                 MatCell::Normal(data) => *data,
             };
             vol.set(pos, Cell::Filled(data)).unwrap();
@@ -222,12 +218,11 @@ impl MatSegment {
                             .get(index as usize)
                             .copied()
                             .unwrap_or_else(|| Rgb::broadcast(0));
-                        MatCell::Normal(CellData {
-                            col: color,
-                            attr: 0
-                                | ((13..16).contains(&index) as u8 * GLOWY)
-                                | ((8..13).contains(&index) as u8 * SHINY),
-                        })
+                        MatCell::Normal(CellData::new(
+                            color,
+                            (13..16).contains(&index),
+                            (8..13).contains(&index),
+                        ))
                     },
                 };