Merge pull request #117 from linebender/limit-color-string-precision

Limit Color::to_rgba_string precision

src/glyph/serialize.rs

@@ -264,9 +264,29 @@ impl PointType {
 }
 
 impl Color {
+    /// Serializes the color into a string as defined by the [UFO specification][0].
+    /// Precision is limited to three decimal places, which is enough to losslessly
+    /// roundtrip to colors represented by `u8` tuples.
+    ///
+    /// [0]: https://unifiedfontobject.org/versions/ufo3/conventions/#colors
     pub fn to_rgba_string(&self) -> String {
+        use std::fmt::Write;
+
         // TODO: Check that all channels are 0.0..=1.0
-        format!("{},{},{},{}", self.red, self.green, self.blue, self.alpha)
+        let mut result = String::new();
+        let mut scratch = String::new();
+        let Color { red, green, blue, alpha } = self;
+        for channel in &[red, green, blue, alpha] {
+            if !result.is_empty() {
+                result.push(',');
+            }
+
+            scratch.clear();
+            // This can only fail on an allocation error, in which case we have other problems.
+            let _ = write!(&mut scratch, "{:.3}", channel);
+            result.push_str(scratch.trim_end_matches('0').trim_end_matches('.'));
+        }
+        result
     }
 }
 

src/layer.rs

@@ -256,7 +256,7 @@ impl Layer {
         // for us to get this far, this mut have a file name
         let path = path.file_name().unwrap().into();
 
-        Ok(Layer { contents, name, glyphs, color, path, lib })
+        Ok(Layer { glyphs, name, path, contents, color, lib })
     }
 
     // Problem: layerinfo.plist contains a nested plist dictionary and the plist crate

src/shared_types.rs

@@ -52,7 +52,7 @@ impl Serialize for Color {
     where
         S: Serializer,
     {
-        let color_string = format!("{},{},{},{}", self.red, self.green, self.blue, self.alpha);
+        let color_string = self.to_rgba_string();
         serializer.serialize_str(&color_string)
     }
 }
@@ -243,7 +243,7 @@ impl<'de> Deserialize<'de> for NonNegativeIntegerOrFloat {
 mod tests {
     use std::convert::TryFrom;
 
-    use serde_test::{assert_tokens, Token};
+    use serde_test::{assert_de_tokens, assert_ser_tokens, assert_tokens, Token};
 
     use crate::{Color, Guideline, Identifier, IntegerOrFloat, Line, NonNegativeIntegerOrFloat};
 
@@ -254,6 +254,18 @@ mod tests {
 
         let c2 = Color { red: 0.0, green: 0.5, blue: 0.0, alpha: 0.5 };
         assert_tokens(&c2, &[Token::Str("0,0.5,0,0.5")]);
+
+        let c3 = Color { red: 0.0, green: 0.0, blue: 0.0, alpha: 0.0 };
+        assert_tokens(&c3, &[Token::Str("0,0,0,0")]);
+
+        let c4 = Color { red: 0.123, green: 0.456, blue: 0.789, alpha: 0.159 };
+        assert_tokens(&c4, &[Token::Str("0.123,0.456,0.789,0.159")]);
+
+        let c5 = Color { red: 0.123456789, green: 0.456789123, blue: 0.789123456, alpha: 0.1 };
+        assert_ser_tokens(&c5, &[Token::Str("0.123,0.457,0.789,0.1")]);
+
+        let c6 = Color { red: 0.123456789, green: 0.456789123, blue: 0.789123456, alpha: 0.1 };
+        assert_de_tokens(&c6, &[Token::Str("0.123456789,0.456789123,0.789123456,0.1")]);
     }
 
     #[test]