Merge #313

313: Improve serializing r=Ogeon a=Ogeon

This fixes a long-standing bug in how `Alpha` and `PreAlpha` are serialized. They used to be flattened to a map, which is fine in JSON but not in more expressive formats, such as RON. The replacement implementation will only handle a subset of possible values (structs, tuples and sequences), but they should be the most common ones. It should also be possible to extend it later.

I have also taken the opportunity to add some helpful utility functions for serializing and de-serializing.

## Closed Issues

* Fixes #130, using a custom implementation.

## Breaking Change

Technically breaking, since it changes the serialization format a bit, but the old behavior was buggy and not always usable. The serialized type and name for `Alpha` and `PreAlpha` is now inherited from the color's type. So, for example, `Srgba` will be serialized and de-serialized as an `Rgb` struct with an additional `alpha` field. Unit types get extended to newtypes, and newtypes get extended to tuples.

Co-authored-by: Erik Hedvall <erikwhedvall@gmail.com>

palette/Cargo.toml

@@ -79,6 +79,7 @@ lazy_static = "1"
 serde = "1"
 serde_derive = "1"
 serde_json = "1"
+ron = "0.8.0"
 enterpolation = "0.2.0"
 scad = "1.2.2" # For regression testing #283
 

palette/src/alpha/alpha.rs

@@ -30,11 +30,9 @@ use crate::{
 
 /// An alpha component wrapper for colors.
 #[derive(Clone, Copy, Debug)]
-#[cfg_attr(feature = "serializing", derive(Serialize, Deserialize))]
 #[repr(C)]
 pub struct Alpha<C, T> {
     /// The color.
-    #[cfg_attr(feature = "serializing", serde(flatten))]
     pub color: C,
 
     /// The transparency component. 0.0 is fully transparent and 1.0 is fully
@@ -658,6 +656,48 @@ where
     }
 }
 
+#[cfg(feature = "serializing")]
+impl<C, T> serde::Serialize for Alpha<C, T>
+where
+    C: serde::Serialize,
+    T: serde::Serialize,
+{
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        self.color.serialize(crate::serde::AlphaSerializer {
+            inner: serializer,
+            alpha: &self.alpha,
+        })
+    }
+}
+
+#[cfg(feature = "serializing")]
+impl<'de, C, T> serde::Deserialize<'de> for Alpha<C, T>
+where
+    C: serde::Deserialize<'de>,
+    T: serde::Deserialize<'de>,
+{
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let mut alpha: Option<T> = None;
+
+        let color = C::deserialize(crate::serde::AlphaDeserializer {
+            inner: deserializer,
+            alpha: &mut alpha,
+        })?;
+
+        if let Some(alpha) = alpha {
+            Ok(Self { color, alpha })
+        } else {
+            Err(serde::de::Error::missing_field("alpha"))
+        }
+    }
+}
+
 #[cfg(feature = "random")]
 impl<C, T> Distribution<Alpha<C, T>> for Standard
 where
@@ -865,21 +905,109 @@ mod test {
     #[cfg(feature = "serializing")]
     #[test]
     fn serialize() {
-        let serialized = ::serde_json::to_string(&Rgba::<Srgb>::new(0.3, 0.8, 0.1, 0.5)).unwrap();
+        let color = Rgba::<Srgb>::new(0.3, 0.8, 0.1, 0.5);
 
         assert_eq!(
-            serialized,
+            serde_json::to_string(&color).unwrap(),
             r#"{"red":0.3,"green":0.8,"blue":0.1,"alpha":0.5}"#
         );
+
+        assert_eq!(
+            ron::to_string(&color).unwrap(),
+            r#"(red:0.3,green:0.8,blue:0.1,alpha:0.5)"#
+        );
     }
 
     #[cfg(feature = "serializing")]
     #[test]
     fn deserialize() {
-        let deserialized: Rgba<Srgb> =
-            ::serde_json::from_str(r#"{"red":0.3,"green":0.8,"blue":0.1,"alpha":0.5}"#).unwrap();
+        let color = Rgba::<Srgb>::new(0.3, 0.8, 0.1, 0.5);
+
+        assert_eq!(
+            serde_json::from_str::<Rgba<Srgb>>(r#"{"alpha":0.5,"red":0.3,"green":0.8,"blue":0.1}"#)
+                .unwrap(),
+            color
+        );
+
+        assert_eq!(
+            ron::from_str::<Rgba<Srgb>>(r#"(alpha:0.5,red:0.3,green:0.8,blue:0.1)"#).unwrap(),
+            color
+        );
+
+        assert_eq!(
+            ron::from_str::<Rgba<Srgb>>(r#"Rgb(alpha:0.5,red:0.3,green:0.8,blue:0.1)"#).unwrap(),
+            color
+        );
+    }
+
+    #[cfg(feature = "serializing")]
+    #[test]
+    fn serde_round_trips() {
+        let color = Rgba::<Srgb>::new(0.3, 0.8, 0.1, 0.5);
+
+        assert_eq!(
+            serde_json::from_str::<Rgba<Srgb>>(&serde_json::to_string(&color).unwrap()).unwrap(),
+            color
+        );
+
+        assert_eq!(
+            ron::from_str::<Rgba<Srgb>>(&ron::to_string(&color).unwrap()).unwrap(),
+            color
+        );
+    }
+
+    #[cfg(feature = "serializing")]
+    #[test]
+    fn serde_various_types() {
+        macro_rules! test_roundtrip {
+            ($value:expr $(, $ron_name:expr)?) => {
+                let value = super::Alpha {
+                    color: $value,
+                    alpha: 0.5,
+                };
+                assert_eq!(
+                    serde_json::from_str::<super::Alpha<_, f32>>(
+                        &serde_json::to_string(&value).expect("json serialization")
+                    )
+                    .expect("json deserialization"),
+                    value
+                );
+
+                let ron_string = ron::to_string(&value).expect("ron serialization");
+                assert_eq!(
+                    ron::from_str::<super::Alpha<_, f32>>(&ron_string)
+                        .expect("ron deserialization"),
+                    value
+                );
+                $(
+                    assert_eq!(
+                        ron::from_str::<super::Alpha<_, f32>>(&format!("{}{ron_string}", $ron_name))
+                            .expect("ron deserialization"),
+                        value
+                    );
+                )?
+            };
+        }
+
+        #[derive(Serialize, Deserialize, PartialEq, Debug)]
+        struct Empty;
+        #[derive(Serialize, Deserialize, PartialEq, Debug)]
+        struct UnitTuple();
+        #[derive(Serialize, Deserialize, PartialEq, Debug)]
+        struct Newtype(f32);
+        #[derive(Serialize, Deserialize, PartialEq, Debug)]
+        struct Tuple(f32, f32);
+        #[derive(Serialize, Deserialize, PartialEq, Debug)]
+        struct Struct {
+            value: f32,
+        }
 
-        assert_eq!(deserialized, Rgba::<Srgb>::new(0.3, 0.8, 0.1, 0.5));
+        test_roundtrip!(());
+        test_roundtrip!(Empty, "Empty");
+        test_roundtrip!(UnitTuple(), "UnitTuple");
+        test_roundtrip!(Newtype(0.1), "Newtype");
+        test_roundtrip!(Tuple(0.1, 0.2), "Tuple");
+        test_roundtrip!(Struct { value: 0.1 }, "Struct");
     }
 
     #[cfg(feature = "random")]

palette/src/blend/pre_alpha.rs

@@ -36,11 +36,9 @@ use super::Premultiply;
 /// component to be clamped to [0.0, 1.0], and fully transparent colors will
 /// become black.
 #[derive(Clone, Copy, Debug)]
-#[cfg_attr(feature = "serializing", derive(Serialize, Deserialize))]
 #[repr(C)]
 pub struct PreAlpha<C: Premultiply> {
     /// The premultiplied color components (`original.color * original.alpha`).
-    #[cfg_attr(feature = "serializing", serde(flatten))]
     pub color: C,
 
     /// The transparency component. 0.0 is fully transparent and 1.0 is fully
@@ -347,6 +345,48 @@ impl<C: Premultiply> DerefMut for PreAlpha<C> {
     }
 }
 
+#[cfg(feature = "serializing")]
+impl<C> serde::Serialize for PreAlpha<C>
+where
+    C: Premultiply + serde::Serialize,
+    C::Scalar: serde::Serialize,
+{
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        self.color.serialize(crate::serde::AlphaSerializer {
+            inner: serializer,
+            alpha: &self.alpha,
+        })
+    }
+}
+
+#[cfg(feature = "serializing")]
+impl<'de, C> serde::Deserialize<'de> for PreAlpha<C>
+where
+    C: Premultiply + serde::Deserialize<'de>,
+    C::Scalar: serde::Deserialize<'de>,
+{
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let mut alpha: Option<C::Scalar> = None;
+
+        let color = C::deserialize(crate::serde::AlphaDeserializer {
+            inner: deserializer,
+            alpha: &mut alpha,
+        })?;
+
+        if let Some(alpha) = alpha {
+            Ok(Self { color, alpha })
+        } else {
+            Err(serde::de::Error::missing_field("alpha"))
+        }
+    }
+}
+
 #[cfg(feature = "bytemuck")]
 unsafe impl<C> bytemuck::Zeroable for PreAlpha<C>
 where
@@ -380,25 +420,43 @@ mod test {
             alpha: 0.5,
         };
 
-        let serialized = ::serde_json::to_string(&color).unwrap();
-
         assert_eq!(
-            serialized,
+            serde_json::to_string(&color).unwrap(),
             r#"{"red":0.3,"green":0.8,"blue":0.1,"alpha":0.5}"#
         );
+
+        assert_eq!(
+            ron::to_string(&color).unwrap(),
+            r#"(red:0.3,green:0.8,blue:0.1,alpha:0.5)"#
+        );
     }
 
     #[cfg(feature = "serializing")]
     #[test]
     fn deserialize() {
-        let expected = PreAlpha {
+        let color = PreAlpha {
             color: LinSrgb::new(0.3, 0.8, 0.1),
             alpha: 0.5,
         };
 
-        let deserialized: PreAlpha<_> =
-            ::serde_json::from_str(r#"{"red":0.3,"green":0.8,"blue":0.1,"alpha":0.5}"#).unwrap();
+        assert_eq!(
+            serde_json::from_str::<PreAlpha<LinSrgb>>(
+                r#"{"alpha":0.5,"red":0.3,"green":0.8,"blue":0.1}"#
+            )
+            .unwrap(),
+            color
+        );
 
-        assert_eq!(deserialized, expected);
+        assert_eq!(
+            ron::from_str::<PreAlpha<LinSrgb>>(r#"(alpha:0.5,red:0.3,green:0.8,blue:0.1)"#)
+                .unwrap(),
+            color
+        );
+
+        assert_eq!(
+            ron::from_str::<PreAlpha<LinSrgb>>(r#"Rgb(alpha:0.5,red:0.3,green:0.8,blue:0.1)"#)
+                .unwrap(),
+            color
+        );
     }
 }

palette/src/lib.rs

@@ -247,7 +247,7 @@ extern crate phf;
 
 #[cfg(feature = "serializing")]
 #[macro_use]
-extern crate serde;
+extern crate serde as _;
 #[cfg(all(test, feature = "serializing"))]
 extern crate serde_json;
 
@@ -447,6 +447,9 @@ pub mod named;
 #[cfg(feature = "random")]
 mod random_sampling;
 
+#[cfg(feature = "serializing")]
+pub mod serde;
+
 mod alpha;
 pub mod angle;
 pub mod blend;