From f4ede104413c3091669fe54f4c829794550300ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 15 Aug 2023 00:40:45 +0200 Subject: [PATCH] style: Update color-mix() syntax to match the current spec Test expectation updates for this in the latest patch of the bug. Differential Revision: https://phabricator.services.mozilla.com/D147002 --- components/style/values/animated/color.rs | 57 ++++--- components/style/values/specified/color.rs | 150 +++++++++++------- .../style/values/specified/percentage.rs | 9 ++ components/style_traits/values.rs | 10 +- 4 files changed, 141 insertions(+), 85 deletions(-) diff --git a/components/style/values/animated/color.rs b/components/style/values/animated/color.rs index f99e344a57a..00bcf63fcfc 100644 --- a/components/style/values/animated/color.rs +++ b/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::( + match interpolation.space { + ColorSpace::Srgb => Self::mix_in::( left_color, left_weight, right_color, right_weight, - hue_adjuster, + interpolation.hue, ), - ColorSpaceKind::Xyz => Self::mix_in::( + ColorSpace::Xyz => Self::mix_in::( left_color, left_weight, right_color, right_weight, - hue_adjuster, + interpolation.hue, ), - ColorSpaceKind::Lab => Self::mix_in::( + ColorSpace::Lab => Self::mix_in::( left_color, left_weight, right_color, right_weight, - hue_adjuster, + interpolation.hue, ), - ColorSpaceKind::Lch => Self::mix_in::( + ColorSpace::Lch => Self::mix_in::( 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 + Into { /// 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.; diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index 4c0683dbce0..6df626c2b43 100644 --- a/components/style/values/specified/color.rs +++ b/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 ``. + 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> { + 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(&self, dest: &mut CssWriter) -> 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,12 +116,11 @@ 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, } #[inline] @@ -82,10 +139,6 @@ fn allow_color_mix_color_spaces() -> bool { return false; } -// 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, @@ -98,53 +151,51 @@ impl Parse for ColorMix { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } - let color_spaces_enabled = context.chrome_rules_enabled() || - allow_color_mix_color_spaces(); - 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 { + 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, }) }) } @@ -168,8 +219,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) { @@ -182,12 +233,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(")") } } @@ -754,12 +799,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")] diff --git a/components/style/values/specified/percentage.rs b/components/style/values/specified/percentage.rs index a152f7d8dc0..7e630720cd4 100644 --- a/components/style/values/specified/percentage.rs +++ b/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::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 { diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index d04485790f8..3bae5d2553b 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -587,6 +587,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 { @@ -607,6 +609,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, } } @@ -614,9 +617,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.), } } }