diff --git a/layout/painting/nsCSSRenderingGradients.cpp b/layout/painting/nsCSSRenderingGradients.cpp index e7ca461653b4..49bf8269e33b 100644 --- a/layout/painting/nsCSSRenderingGradients.cpp +++ b/layout/painting/nsCSSRenderingGradients.cpp @@ -408,7 +408,7 @@ static void ResolveMidpoints(nsTArray& stops) { const float alpha = srgb1.alpha + multiplier * (srgb2.alpha - srgb1.alpha); - newStop.mColor = StyleAbsoluteColor::Srgb(red, green, blue, alpha); + newStop.mColor = StyleAbsoluteColor::SrgbLegacy(red, green, blue, alpha); } stops.ReplaceElementsAt(x, 1, newStops, 9); diff --git a/layout/style/StyleColorInlines.h b/layout/style/StyleColorInlines.h index e591c0e14d57..6b7c1e24512a 100644 --- a/layout/style/StyleColorInlines.h +++ b/layout/style/StyleColorInlines.h @@ -16,16 +16,26 @@ namespace mozilla { inline StyleAbsoluteColor StyleAbsoluteColor::FromColor(nscolor aColor) { - return StyleAbsoluteColor::Srgb( + return StyleAbsoluteColor::SrgbLegacy( NS_GET_R(aColor) / 255.0f, NS_GET_G(aColor) / 255.0f, NS_GET_B(aColor) / 255.0f, NS_GET_A(aColor) / 255.0f); } // static -inline StyleAbsoluteColor StyleAbsoluteColor::Srgb(float red, float green, - float blue, float alpha) { - return StyleAbsoluteColor{StyleColorComponents{red, green, blue}, alpha, - StyleColorSpace::Srgb, StyleColorFlags{0}}; +inline StyleAbsoluteColor StyleAbsoluteColor::SrgbLegacy(float red, float green, + float blue, + float alpha) { + const auto ToLegacyComponent = [](float aF) { + if (MOZ_UNLIKELY(!std::isfinite(aF))) { + return 0.0f; + } + return aF; + }; + + return StyleAbsoluteColor{ + StyleColorComponents{ToLegacyComponent(red), ToLegacyComponent(green), + ToLegacyComponent(blue)}, + alpha, StyleColorSpace::Srgb, StyleColorFlags::IS_LEGACY_SRGB}; } template <> diff --git a/servo/components/style/color/mix.rs b/servo/components/style/color/mix.rs index 028dd96a83fb..b2b8cdc24ae8 100644 --- a/servo/components/style/color/mix.rs +++ b/servo/components/style/color/mix.rs @@ -169,8 +169,19 @@ pub fn mix( alpha_multiplier, ); - if flags.contains(ColorMixFlags::RESULT_IN_MODERN_SYNTAX) && result.is_legacy_syntax() { - result.to_color_space(ColorSpace::Srgb).into_modern_syntax() + if flags.contains(ColorMixFlags::RESULT_IN_MODERN_SYNTAX) { + // If the result *MUST* be in modern syntax, then make sure it is in a + // color space that allows the modern syntax. So hsl and hwb will be + // converted to srgb. + if result.is_legacy_syntax() { + result.to_color_space(ColorSpace::Srgb) + } else { + result + } + } else if left_color.is_legacy_syntax() && right_color.is_legacy_syntax() { + // If both sides of the mix is legacy then convert the result back into + // legacy. + result.into_srgb_legacy() } else { result } diff --git a/servo/components/style/color/mod.rs b/servo/components/style/color/mod.rs index ce1bda92e601..bc165b1a0dd6 100644 --- a/servo/components/style/color/mod.rs +++ b/servo/components/style/color/mod.rs @@ -18,6 +18,7 @@ pub struct ColorComponents(pub f32, pub f32, pub f32); impl ColorComponents { /// Apply a function to each of the 3 components of the color. + #[must_use] pub fn map(self, f: impl Fn(f32) -> f32) -> Self { Self(f(self.0), f(self.1), f(self.2)) } @@ -131,9 +132,9 @@ bitflags! { #[derive(Clone, Copy, Default, MallocSizeOf, PartialEq, ToShmem)] #[repr(C)] pub struct ColorFlags : u8 { - /// If set, serializes sRGB colors into `color(srgb ...)` instead of - /// `rgba(...)`. - const AS_COLOR_FUNCTION = 1 << 0; + /// Marks that this color is in the legacy color format. This flag is + /// only valid for the `Srgb` color space. + const IS_LEGACY_SRGB = 1 << 0; /// Whether the 1st color component is `none`. const C1_IS_NONE = 1 << 1; /// Whether the 2nd color component is `none`. @@ -230,7 +231,7 @@ impl AbsoluteColor { components: ColorComponents(0.0, 0.0, 0.0), alpha: 0.0, color_space: ColorSpace::Srgb, - flags: ColorFlags { bits: 0 }, // cbindgen does not like ColorFlags::empty(). + flags: ColorFlags::IS_LEGACY_SRGB, }; /// An opaque black color in the legacy syntax. @@ -238,7 +239,7 @@ impl AbsoluteColor { components: ColorComponents(0.0, 0.0, 0.0), alpha: 1.0, color_space: ColorSpace::Srgb, - flags: ColorFlags { bits: 0 }, // cbindgen does not like ColorFlags::empty(). + flags: ColorFlags::IS_LEGACY_SRGB, }; /// An opaque white color in the legacy syntax. @@ -246,7 +247,7 @@ impl AbsoluteColor { components: ColorComponents(1.0, 1.0, 1.0), alpha: 1.0, color_space: ColorSpace::Srgb, - flags: ColorFlags { bits: 0 }, // cbindgen does not like ColorFlags::empty(). + flags: ColorFlags::IS_LEGACY_SRGB, }; /// Create a new [`AbsoluteColor`] with the given [`ColorSpace`] and @@ -299,16 +300,29 @@ impl AbsoluteColor { } } - /// Convert this color to the modern color syntax. + /// Convert this color into the sRGB color space and set it to the legacy + /// syntax. #[inline] - pub fn into_modern_syntax(mut self) -> Self { - self.flags |= ColorFlags::AS_COLOR_FUNCTION; - self + #[must_use] + pub fn into_srgb_legacy(self) -> Self { + let mut result = if !matches!(self.color_space, ColorSpace::Srgb) { + self.to_color_space(ColorSpace::Srgb) + } else { + self + }; + + // Explicitly set the flags to IS_LEGACY_SRGB only to clear out the + // *_IS_NONE flags, because the legacy syntax doesn't allow "none". + result.flags = ColorFlags::IS_LEGACY_SRGB; + + result } - /// Create a new [`AbsoluteColor`] from rgba values in the sRGB color space. - pub fn srgb(red: f32, green: f32, blue: f32, alpha: f32) -> Self { - Self::new(ColorSpace::Srgb, red, green, blue, alpha) + /// Create a new [`AbsoluteColor`] from rgba legacy syntax values in the sRGB color space. + pub fn srgb_legacy(red: u8, green: u8, blue: u8, alpha: f32) -> Self { + let mut result = Self::new(ColorSpace::Srgb, red, green, blue, alpha); + result.flags = ColorFlags::IS_LEGACY_SRGB; + result } /// Return all the components of the color in an array. (Includes alpha) @@ -322,7 +336,7 @@ impl AbsoluteColor { pub fn is_legacy_syntax(&self) -> bool { // rgb(), rgba(), hsl(), hsla(), hwb(), hwba() match self.color_space { - ColorSpace::Srgb => !self.flags.contains(ColorFlags::AS_COLOR_FUNCTION), + ColorSpace::Srgb => self.flags.contains(ColorFlags::IS_LEGACY_SRGB), ColorSpace::Hsl | ColorSpace::Hwb => true, _ => false, } @@ -453,17 +467,14 @@ impl ToCss for AbsoluteColor { let maybe_alpha = value_or_none!(self.alpha, ALPHA_IS_NONE); match self.color_space { - ColorSpace::Hsl => self.to_color_space(ColorSpace::Srgb).to_css(dest), - - ColorSpace::Hwb => self.to_color_space(ColorSpace::Srgb).to_css(dest), - - ColorSpace::Srgb if !self.flags.contains(ColorFlags::AS_COLOR_FUNCTION) => { + ColorSpace::Srgb if self.flags.contains(ColorFlags::IS_LEGACY_SRGB) => { // The "none" keyword is not supported in the rgb/rgba legacy syntax. cssparser::ToCss::to_css( &cssparser::RgbaLegacy::from_floats(self.components.0, self.components.1, self.components.2, self.alpha), dest, ) }, + ColorSpace::Hsl | ColorSpace::Hwb => self.into_srgb_legacy().to_css(dest), ColorSpace::Lab => cssparser::ToCss::to_css( &cssparser::Lab::new(maybe_c1, maybe_c2, maybe_c3, maybe_alpha), dest, @@ -484,10 +495,9 @@ impl ToCss for AbsoluteColor { let color_space = match self.color_space { ColorSpace::Srgb => { debug_assert!( - self.flags.contains(ColorFlags::AS_COLOR_FUNCTION), - "The case without this flag should be handled in the wrapping match case!!" - ); - + !self.flags.contains(ColorFlags::IS_LEGACY_SRGB), + "legacy srgb is not a color function" + ); cssparser::PredefinedColorSpace::Srgb }, ColorSpace::SrgbLinear => cssparser::PredefinedColorSpace::SrgbLinear, diff --git a/servo/components/style/gecko/values.rs b/servo/components/style/gecko/values.rs index fbdb02c6badd..d04c73c70f3e 100644 --- a/servo/components/style/gecko/values.rs +++ b/servo/components/style/gecko/values.rs @@ -28,12 +28,17 @@ pub fn convert_absolute_color_to_nscolor(color: &AbsoluteColor) -> u32 { /// Convert a given `nscolor` to a Servo AbsoluteColor value. pub fn convert_nscolor_to_absolute_color(color: u32) -> AbsoluteColor { let [r, g, b, a] = color.to_le_bytes(); - AbsoluteColor::srgb( - r as f32 / 255.0, - g as f32 / 255.0, - b as f32 / 255.0, - a as f32 / 255.0, - ) + AbsoluteColor::srgb_legacy(r, g, b, a as f32 / 255.0) +} + +#[test] +fn convert_ns_color_to_absolute_color_should_be_in_legacy_syntax() { + use crate::color::ColorFlags; + + let result = convert_nscolor_to_absolute_color(0x336699CC); + assert!(result.flags.contains(ColorFlags::IS_LEGACY_SRGB)); + + assert!(result.is_legacy_syntax()); } impl CounterStyle { diff --git a/servo/components/style/values/specified/color.rs b/servo/components/style/values/specified/color.rs index 927cd1f0281b..6742d547fa21 100644 --- a/servo/components/style/values/specified/color.rs +++ b/servo/components/style/values/specified/color.rs @@ -149,11 +149,7 @@ impl LightDark { fn compute(&self, cx: &Context) -> ComputedColor { let style_color_scheme = cx.style().get_inherited_ui().clone_color_scheme(); let dark = cx.device().is_dark_color_scheme(&style_color_scheme); - let used = if dark { - &self.dark - } else { - &self.light - }; + let used = if dark { &self.dark } else { &self.light }; used.to_computed_value(cx) } @@ -428,30 +424,13 @@ impl SystemColor { } } -#[inline] -fn new_absolute( - color_space: ColorSpace, - c1: Option, - c2: Option, - c3: Option, - alpha: Option, -) -> Color { - Color::Absolute(Box::new(Absolute { - color: AbsoluteColor::new(color_space, c1, c2, c3, alpha), - authored: None, - })) -} - impl cssparser::FromParsedColor for Color { fn from_current_color() -> Self { Color::CurrentColor } fn from_rgba(r: u8, g: u8, b: u8, a: f32) -> Self { - Self::Absolute(Box::new(Absolute { - color: AbsoluteColor::new(ColorSpace::Srgb, r, g, b, a), - authored: None, - })) + AbsoluteColor::srgb_legacy(r, g, b, a).into() } fn from_hsl( @@ -460,7 +439,7 @@ impl cssparser::FromParsedColor for Color { lightness: Option, alpha: Option, ) -> Self { - new_absolute(ColorSpace::Hsl, hue, saturation, lightness, alpha) + AbsoluteColor::new(ColorSpace::Hsl, hue, saturation, lightness, alpha).into() } fn from_hwb( @@ -469,7 +448,7 @@ impl cssparser::FromParsedColor for Color { blackness: Option, alpha: Option, ) -> Self { - new_absolute(ColorSpace::Hwb, hue, whiteness, blackness, alpha) + AbsoluteColor::new(ColorSpace::Hwb, hue, whiteness, blackness, alpha).into() } fn from_lab( @@ -478,7 +457,7 @@ impl cssparser::FromParsedColor for Color { b: Option, alpha: Option, ) -> Self { - new_absolute(ColorSpace::Lab, lightness, a, b, alpha) + AbsoluteColor::new(ColorSpace::Lab, lightness, a, b, alpha).into() } fn from_lch( @@ -487,7 +466,7 @@ impl cssparser::FromParsedColor for Color { hue: Option, alpha: Option, ) -> Self { - new_absolute(ColorSpace::Lch, lightness, chroma, hue, alpha) + AbsoluteColor::new(ColorSpace::Lch, lightness, chroma, hue, alpha).into() } fn from_oklab( @@ -496,7 +475,7 @@ impl cssparser::FromParsedColor for Color { b: Option, alpha: Option, ) -> Self { - new_absolute(ColorSpace::Oklab, lightness, a, b, alpha) + AbsoluteColor::new(ColorSpace::Oklab, lightness, a, b, alpha).into() } fn from_oklch( @@ -505,7 +484,7 @@ impl cssparser::FromParsedColor for Color { hue: Option, alpha: Option, ) -> Self { - new_absolute(ColorSpace::Oklch, lightness, chroma, hue, alpha) + AbsoluteColor::new(ColorSpace::Oklch, lightness, chroma, hue, alpha).into() } fn from_color_function( @@ -515,13 +494,7 @@ impl cssparser::FromParsedColor for Color { c3: Option, alpha: Option, ) -> Self { - let mut result = new_absolute(color_space.into(), c1, c2, c3, alpha); - if let Color::Absolute(ref mut absolute) = result { - if matches!(absolute.color.color_space, ColorSpace::Srgb) { - absolute.color.flags |= ColorFlags::AS_COLOR_FUNCTION; - } - } - result + AbsoluteColor::new(color_space.into(), c1, c2, c3, alpha).into() } } @@ -636,7 +609,7 @@ impl Color { ColorSpace::Srgb | ColorSpace::Hsl ); let is_color_function = - absolute.color.flags.contains(ColorFlags::AS_COLOR_FUNCTION); + !absolute.color.flags.contains(ColorFlags::IS_LEGACY_SRGB); let pref_enabled = static_prefs::pref!("layout.css.more_color_4.enabled"); (is_legacy_color && !is_color_function) || pref_enabled diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml index 88e74666f377..ac824ee4a861 100644 --- a/servo/ports/geckolib/cbindgen.toml +++ b/servo/ports/geckolib/cbindgen.toml @@ -581,9 +581,9 @@ renaming_overrides_prefixing = true "AbsoluteColor" = """ /** - * Create a new AbsoluteColor in the sRGB color space. + * Create a new AbsoluteColor in the sRGB color space in legacy color syntax. */ - static inline StyleAbsoluteColor Srgb(float red, float green, float blue, float alpha); + static inline StyleAbsoluteColor SrgbLegacy(float red, float green, float blue, float alpha); static inline StyleAbsoluteColor FromColor(nscolor);