Bug 1770616 - Update color-mix() syntax to match the current spec. r=barret

Test expectation updates for this in the latest patch of the bug.

Differential Revision: https://phabricator.services.mozilla.com/D147002

servo/components/style/values/animated/color.rs

@@ -7,7 +7,7 @@
 use crate::values::animated::{Animate, Procedure, ToAnimatedZero};
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
 use crate::values::generics::color::{Color as GenericColor, ComplexColorRatios};
-use crate::values::specified::color::{ColorSpaceKind, HueAdjuster};
+use crate::values::specified::color::{ColorInterpolationMethod, ColorSpace, HueInterpolationMethod};
 use euclid::default::{Transform3D, Vector3D};
 
 /// An animated RGBA color.
@@ -128,41 +128,40 @@ impl Color {
 
     /// Mix two colors into one.
     pub fn mix(
-        color_space: ColorSpaceKind,
+        interpolation: &ColorInterpolationMethod,
         left_color: &Color,
         left_weight: f32,
         right_color: &Color,
         right_weight: f32,
-        hue_adjuster: HueAdjuster,
     ) -> Self {
-        match color_space {
-            ColorSpaceKind::Srgb => Self::mix_in::<RGBA>(
+        match interpolation.space {
+            ColorSpace::Srgb => Self::mix_in::<RGBA>(
                 left_color,
                 left_weight,
                 right_color,
                 right_weight,
-                hue_adjuster,
+                interpolation.hue,
             ),
-            ColorSpaceKind::Xyz => Self::mix_in::<XYZA>(
+            ColorSpace::Xyz => Self::mix_in::<XYZA>(
                 left_color,
                 left_weight,
                 right_color,
                 right_weight,
-                hue_adjuster,
+                interpolation.hue,
             ),
-            ColorSpaceKind::Lab => Self::mix_in::<LABA>(
+            ColorSpace::Lab => Self::mix_in::<LABA>(
                 left_color,
                 left_weight,
                 right_color,
                 right_weight,
-                hue_adjuster,
+                interpolation.hue,
             ),
-            ColorSpaceKind::Lch => Self::mix_in::<LCHA>(
+            ColorSpace::Lch => Self::mix_in::<LCHA>(
                 left_color,
                 left_weight,
                 right_color,
                 right_weight,
-                hue_adjuster,
+                interpolation.hue,
             ),
         }
     }
@@ -172,7 +171,7 @@ impl Color {
         left_weight: f32,
         right_color: &Color,
         right_weight: f32,
-        hue_adjuster: HueAdjuster,
+        hue_interpolation: HueInterpolationMethod,
     ) -> Self
     where
         S: ModelledColor,
@@ -180,7 +179,7 @@ impl Color {
         let left_bg = S::from(left_color.scaled_rgba());
         let right_bg = S::from(right_color.scaled_rgba());
 
-        let color = S::lerp(left_bg, left_weight, right_bg, right_weight, hue_adjuster);
+        let color = S::lerp(left_bg, left_weight, right_bg, right_weight, hue_interpolation);
         let rgba: RGBA = color.into();
         let rgba = if !rgba.in_gamut() {
             // TODO: Better gamut mapping.
@@ -365,14 +364,14 @@ impl ToAnimatedZero for Color {
 trait ModelledColor: Clone + Copy + From<RGBA> + Into<RGBA> {
     /// Linearly interpolate between the left and right colors.
     ///
-    /// The HueAdjuster parameter is only for color spaces where the hue is
+    /// The HueInterpolationMethod parameter is only for color spaces where the hue is
     /// represented as an angle (e.g., CIE LCH).
     fn lerp(
         left_bg: Self,
         left_weight: f32,
         right_bg: Self,
         right_weight: f32,
-        hue_adjuster: HueAdjuster,
+        hue_interpolation: HueInterpolationMethod,
     ) -> Self;
 }
 
@@ -382,7 +381,7 @@ impl ModelledColor for RGBA {
         left_weight: f32,
         right_bg: Self,
         right_weight: f32,
-        _: HueAdjuster,
+        _: HueInterpolationMethod,
     ) -> Self {
         // Interpolation with alpha, as per
         // https://drafts.csswg.org/css-color/#interpolation-alpha.
@@ -440,7 +439,7 @@ impl ModelledColor for XYZA {
         left_weight: f32,
         right_bg: Self,
         right_weight: f32,
-        _: HueAdjuster,
+        _: HueInterpolationMethod,
     ) -> Self {
         // Interpolation with alpha, as per
         // https://drafts.csswg.org/css-color/#interpolation-alpha.
@@ -503,7 +502,7 @@ impl ModelledColor for LABA {
         left_weight: f32,
         right_bg: Self,
         right_weight: f32,
-        _: HueAdjuster,
+        _: HueInterpolationMethod,
     ) -> Self {
         // Interpolation with alpha, as per
         // https://drafts.csswg.org/css-color/#interpolation-alpha.
@@ -561,7 +560,7 @@ impl LCHA {
 }
 
 impl LCHA {
-    fn adjust(left_bg: Self, right_bg: Self, hue_adjuster: HueAdjuster) -> (Self, Self) {
+    fn adjust(left_bg: Self, right_bg: Self, hue_interpolation: HueInterpolationMethod) -> (Self, Self) {
         use std::f32::consts::{PI, TAU};
 
         let mut left_bg = left_bg;
@@ -583,7 +582,7 @@ impl LCHA {
             }
         }
 
-        if hue_adjuster != HueAdjuster::Specified {
+        if hue_interpolation != HueInterpolationMethod::Specified {
             // Normalize hue into [0, 2 * PI)
             while left_bg.hue < 0. {
                 left_bg.hue += TAU;
@@ -600,8 +599,8 @@ impl LCHA {
             }
         }
 
-        match hue_adjuster {
-            HueAdjuster::Shorter => {
+        match hue_interpolation {
+            HueInterpolationMethod::Shorter => {
                 let delta = right_bg.hue - left_bg.hue;
 
                 if delta > PI {
@@ -611,7 +610,7 @@ impl LCHA {
                 }
             },
 
-            HueAdjuster::Longer => {
+            HueInterpolationMethod::Longer => {
                 let delta = right_bg.hue - left_bg.hue;
                 if 0. < delta && delta < PI {
                     left_bg.hue += TAU;
@@ -620,13 +619,13 @@ impl LCHA {
                 }
             },
 
-            HueAdjuster::Increasing => {
+            HueInterpolationMethod::Increasing => {
                 if right_bg.hue < left_bg.hue {
                     right_bg.hue += TAU;
                 }
             },
 
-            HueAdjuster::Decreasing => {
+            HueInterpolationMethod::Decreasing => {
                 if left_bg.hue < right_bg.hue {
                     left_bg.hue += TAU;
                 }
@@ -634,7 +633,7 @@ impl LCHA {
 
             //Angles are not adjusted. They are interpolated like any other
             //component.
-            HueAdjuster::Specified => {},
+            HueInterpolationMethod::Specified => {},
         }
 
         (left_bg, right_bg)
@@ -647,11 +646,11 @@ impl ModelledColor for LCHA {
         left_weight: f32,
         right_bg: Self,
         right_weight: f32,
-        hue_adjuster: HueAdjuster,
+        hue_interpolation: HueInterpolationMethod,
     ) -> Self {
         // Interpolation with alpha, as per
         // https://drafts.csswg.org/css-color/#interpolation-alpha.
-        let (left_bg, right_bg) = Self::adjust(left_bg, right_bg, hue_adjuster);
+        let (left_bg, right_bg) = Self::adjust(left_bg, right_bg, hue_interpolation);
 
         let mut lightness = 0.;
         let mut chroma = 0.;

servo/components/style/values/specified/color.rs

@@ -21,9 +21,9 @@ use style_traits::{SpecifiedValueInfo, ToCss, ValueParseErrorKind};
 
 /// A color space as defined in [1].
 ///
-/// [1]: https://drafts.csswg.org/css-color-5/#typedef-colorspace
+/// [1]: https://drafts.csswg.org/css-color-4/#typedef-color-space
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss, ToShmem)]
-pub enum ColorSpaceKind {
+pub enum ColorSpace {
     /// The sRGB color space.
     Srgb,
     /// The CIEXYZ color space.
@@ -34,23 +34,81 @@ pub enum ColorSpaceKind {
     Lch,
 }
 
-/// A hue adjuster as defined in [1].
+impl ColorSpace {
+    /// Returns whether this is a `<polar-color-space>`.
+    pub fn is_polar(self) -> bool {
+        match self {
+            Self::Srgb | Self::Xyz | Self::Lab => false,
+            Self::Lch => true,
+        }
+    }
+}
+
+/// A hue-interpolation-method as defined in [1].
 ///
-/// [1]: https://drafts.csswg.org/css-color-5/#typedef-hue-adjuster
+/// [1]: https://drafts.csswg.org/css-color-4/#typedef-hue-interpolation-method
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss, ToShmem)]
-pub enum HueAdjuster {
-    /// The "shorter" angle adjustment.
+pub enum HueInterpolationMethod {
+    /// https://drafts.csswg.org/css-color-4/#shorter
     Shorter,
-    /// The "longer" angle adjustment.
+    /// https://drafts.csswg.org/css-color-4/#longer
     Longer,
-    /// The "increasing" angle adjustment.
+    /// https://drafts.csswg.org/css-color-4/#increasing
     Increasing,
-    /// The "decreasing" angle adjustment.
+    /// https://drafts.csswg.org/css-color-4/#decreasing
     Decreasing,
-    /// The "specified" angle adjustment.
+    /// https://drafts.csswg.org/css-color-4/#specified
     Specified,
 }
 
+/// https://drafts.csswg.org/css-color-4/#color-interpolation-method
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToShmem)]
+pub struct ColorInterpolationMethod {
+    /// The color-space the interpolation should be done in.
+    pub space: ColorSpace,
+    /// The hue interpolation method.
+    pub hue: HueInterpolationMethod,
+}
+
+impl Parse for ColorInterpolationMethod {
+    fn parse<'i, 't>(
+        _: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        input.expect_ident_matching("in")?;
+        let space = ColorSpace::parse(input)?;
+        // https://drafts.csswg.org/css-color-4/#hue-interpolation
+        //     Unless otherwise specified, if no specific hue interpolation
+        //     algorithm is selected by the host syntax, the default is shorter.
+        let hue = if space.is_polar() {
+            input.try_parse(|input| -> Result<_, ParseError<'i>> {
+                let hue = HueInterpolationMethod::parse(input)?;
+                input.expect_ident_matching("hue")?;
+                Ok(hue)
+            }).unwrap_or(HueInterpolationMethod::Shorter)
+        } else {
+            HueInterpolationMethod::Shorter
+        };
+        Ok(Self { space, hue })
+    }
+}
+
+impl ToCss for ColorInterpolationMethod {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        dest.write_str("in ")?;
+        self.space.to_css(dest)?;
+        if self.hue != HueInterpolationMethod::Shorter {
+            dest.write_char(' ')?;
+            self.hue.to_css(dest)?;
+            dest.write_str(" hue")?;
+        }
+        Ok(())
+    }
+}
+
 /// A restricted version of the css `color-mix()` function, which only supports
 /// percentages.
 ///
@@ -58,18 +116,13 @@ pub enum HueAdjuster {
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToShmem)]
 #[allow(missing_docs)]
 pub struct ColorMix {
-    pub color_space: ColorSpaceKind,
+    pub interpolation: ColorInterpolationMethod,
     pub left: Color,
     pub left_percentage: Percentage,
     pub right: Color,
     pub right_percentage: Percentage,
-    pub hue_adjuster: HueAdjuster,
 }
 
-// NOTE(emilio): Syntax is still a bit in-flux, since [1] doesn't seem
-// particularly complete, and disagrees with the examples.
-//
-// [1]: https://github.com/w3c/csswg-drafts/commit/a4316446112f9e814668c2caff7f826f512f8fed
 impl Parse for ColorMix {
     fn parse<'i, 't>(
         context: &ParserContext,
@@ -82,53 +135,51 @@ impl Parse for ColorMix {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
 
-        let color_spaces_enabled = context.chrome_rules_enabled() ||
-            static_prefs::pref!("layout.css.color-mix.color-spaces.enabled");
-
         input.expect_function_matching("color-mix")?;
 
-        // NOTE(emilio): This implements the syntax described here for now,
-        // might need to get updated in the future.
-        //
-        // https://github.com/w3c/csswg-drafts/issues/6066#issuecomment-789836765
         input.parse_nested_block(|input| {
-            input.expect_ident_matching("in")?;
-            let color_space = if color_spaces_enabled {
-                ColorSpaceKind::parse(input)?
-            } else {
-                input.expect_ident_matching("srgb")?;
-                ColorSpaceKind::Srgb
-            };
+            let interpolation = ColorInterpolationMethod::parse(context, input)?;
             input.expect_comma()?;
 
+            let try_parse_percentage = |input: &mut Parser| -> Option<Percentage> {
+                input.try_parse(|input| Percentage::parse_zero_to_a_hundred(context, input)).ok()
+            };
+
+            let mut left_percentage = try_parse_percentage(input);
+
             let left = Color::parse(context, input)?;
-            let left_percentage = input
-                .try_parse(|input| Percentage::parse(context, input))
-                .ok();
+            if left_percentage.is_none() {
+                left_percentage = try_parse_percentage(input);
+            }
 
             input.expect_comma()?;
 
+            let mut right_percentage = try_parse_percentage(input);
+
             let right = Color::parse(context, input)?;
-            let right_percentage = input
-                .try_parse(|input| Percentage::parse(context, input))
-                .unwrap_or_else(|_| {
-                    Percentage::new(1.0 - left_percentage.map_or(0.5, |p| p.get()))
-                });
+
+            if right_percentage.is_none() {
+                right_percentage = try_parse_percentage(input);
+            }
+
+            let right_percentage = right_percentage.unwrap_or_else(|| {
+                Percentage::new(1.0 - left_percentage.map_or(0.5, |p| p.get()))
+            });
 
             let left_percentage =
                 left_percentage.unwrap_or_else(|| Percentage::new(1.0 - right_percentage.get()));
 
-            let hue_adjuster = input
-                .try_parse(|input| HueAdjuster::parse(input))
-                .unwrap_or(HueAdjuster::Shorter);
+            if left_percentage.get() + right_percentage.get() <= 0.0 {
+                // If the percentages sum to zero, the function is invalid.
+                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+            }
 
             Ok(ColorMix {
-                color_space,
+                interpolation,
                 left,
                 left_percentage,
                 right,
                 right_percentage,
-                hue_adjuster,
             })
         })
     }
@@ -152,8 +203,8 @@ impl ToCss for ColorMix {
             (1.0 - percent.get() - other.get()).abs() <= f32::EPSILON
         }
 
-        dest.write_str("color-mix(in ")?;
-        self.color_space.to_css(dest)?;
+        dest.write_str("color-mix(")?;
+        self.interpolation.to_css(dest)?;
         dest.write_str(", ")?;
         self.left.to_css(dest)?;
         if !can_omit(&self.left_percentage, &self.right_percentage, true) {
@@ -166,12 +217,6 @@ impl ToCss for ColorMix {
             dest.write_str(" ")?;
             self.right_percentage.to_css(dest)?;
         }
-
-        if self.hue_adjuster != HueAdjuster::Shorter {
-            dest.write_str(" ")?;
-            self.hue_adjuster.to_css(dest)?;
-        }
-
         dest.write_str(")")
     }
 }
@@ -731,12 +776,11 @@ impl Color {
                 let left = mix.left.to_computed_color(context)?.to_animated_value();
                 let right = mix.right.to_computed_color(context)?.to_animated_value();
                 ToAnimatedValue::from_animated_value(AnimatedColor::mix(
-                    mix.color_space,
+                    &mix.interpolation,
                     &left,
                     mix.left_percentage.get(),
                     &right,
                     mix.right_percentage.get(),
-                    mix.hue_adjuster,
                 ))
             },
             #[cfg(feature = "gecko")]

servo/components/style/values/specified/percentage.rs

@@ -138,6 +138,15 @@ impl Percentage {
         Self::parse_with_clamping_mode(context, input, AllowedNumericType::NonNegative)
     }
 
+    /// Parses a percentage token, but rejects it if it's negative or more than
+    /// 100%.
+    pub fn parse_zero_to_a_hundred<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        Self::parse_with_clamping_mode(context, input, AllowedNumericType::ZeroToOne)
+    }
+
     /// Clamp to 100% if the value is over 100%.
     #[inline]
     pub fn clamp_to_hundred(self) -> Self {

servo/components/style_traits/values.rs

@@ -575,6 +575,8 @@ pub mod specified {
         NonNegative,
         /// Allow only numeric values greater or equal to 1.0.
         AtLeastOne,
+        /// Allow only numeric values from 0 to 1.0.
+        ZeroToOne,
     }
 
     impl Default for AllowedNumericType {
@@ -595,6 +597,7 @@ pub mod specified {
                 AllowedNumericType::All => true,
                 AllowedNumericType::NonNegative => val >= 0.0,
                 AllowedNumericType::AtLeastOne => val >= 1.0,
+                AllowedNumericType::ZeroToOne => val >= 0.0 && val <= 1.0,
             }
         }
 
@@ -602,9 +605,10 @@ pub mod specified {
         #[inline]
         pub fn clamp(&self, val: f32) -> f32 {
             match *self {
-                AllowedNumericType::NonNegative if val < 0. => 0.,
-                AllowedNumericType::AtLeastOne if val < 1. => 1.,
-                _ => val,
+                AllowedNumericType::All => val,
+                AllowedNumericType::NonNegative => val.max(0.),
+                AllowedNumericType::AtLeastOne => val.max(1.),
+                AllowedNumericType::ZeroToOne => val.max(0.).min(1.),
             }
         }
     }