From 5c7dd215d46b97a68fa72cddb20a335ab7f85f14 Mon Sep 17 00:00:00 2001 From: Tiaan Louw Date: Sun, 23 Jul 2023 23:42:05 +0200 Subject: [PATCH] Rgba does not store Option as component Legacy `rgb`/`rgba` syntax does not allow the "none" keyword and although the modern `rgb`/`rgba` syntax does support it, it is immediately converted to 0, because we never have to serialize back to "none" keywords for `rgb`/`rgba`. Also `RGBA` is now renamed to `Rgba` as per common rust naming conventions now that it is not used directly in Gecko any more. --- src/color.rs | 141 ++++++++++++++++++++++++--------------------------- src/lib.rs | 2 +- src/tests.rs | 27 ++++------ 3 files changed, 79 insertions(+), 91 deletions(-) diff --git a/src/color.rs b/src/color.rs index d114c3fc..30a195e1 100644 --- a/src/color.rs +++ b/src/color.rs @@ -82,44 +82,34 @@ fn normalize_hue(hue: f32) -> f32 { /// A color with red, green, blue, and alpha components, in a byte each. #[derive(Clone, Copy, PartialEq, Debug)] -pub struct RGBA { +pub struct RgbaLegacy { /// The red component. - pub red: Option, + pub red: u8, /// The green component. - pub green: Option, + pub green: u8, /// The blue component. - pub blue: Option, + pub blue: u8, /// The alpha component. - pub alpha: Option, + pub alpha: f32, } -impl RGBA { +impl RgbaLegacy { /// Constructs a new RGBA value from float components. It expects the red, /// green, blue and alpha channels in that order, and all values will be /// clamped to the 0.0 ... 1.0 range. #[inline] - pub fn from_floats( - red: Option, - green: Option, - blue: Option, - alpha: Option, - ) -> Self { + pub fn from_floats(red: f32, green: f32, blue: f32, alpha: f32) -> Self { Self::new( - red.map(clamp_unit_f32), - green.map(clamp_unit_f32), - blue.map(clamp_unit_f32), - alpha.map(|a| a.clamp(0.0, OPAQUE)), + clamp_unit_f32(red), + clamp_unit_f32(green), + clamp_unit_f32(blue), + alpha.clamp(0.0, OPAQUE), ) } /// Same thing, but with `u8` values instead of floats in the 0 to 1 range. #[inline] - pub const fn new( - red: Option, - green: Option, - blue: Option, - alpha: Option, - ) -> Self { + pub const fn new(red: u8, green: u8, blue: u8, alpha: f32) -> Self { Self { red, green, @@ -130,7 +120,7 @@ impl RGBA { } #[cfg(feature = "serde")] -impl Serialize for RGBA { +impl Serialize for RgbaLegacy { fn serialize(&self, serializer: S) -> Result where S: Serializer, @@ -140,32 +130,32 @@ impl Serialize for RGBA { } #[cfg(feature = "serde")] -impl<'de> Deserialize<'de> for RGBA { +impl<'de> Deserialize<'de> for RgbaLegacy { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { let (r, g, b, a) = Deserialize::deserialize(deserializer)?; - Ok(RGBA::new(r, g, b, a)) + Ok(RgbaLegacy::new(r, g, b, a)) } } -impl ToCss for RGBA { +impl ToCss for RgbaLegacy { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write, { - let has_alpha = self.alpha.unwrap_or(0.0) != OPAQUE; + let has_alpha = self.alpha != OPAQUE; dest.write_str(if has_alpha { "rgba(" } else { "rgb(" })?; - self.red.unwrap_or(0).to_css(dest)?; + self.red.to_css(dest)?; dest.write_str(", ")?; - self.green.unwrap_or(0).to_css(dest)?; + self.green.to_css(dest)?; dest.write_str(", ")?; - self.blue.unwrap_or(0).to_css(dest)?; + self.blue.to_css(dest)?; // Legacy syntax does not allow none components. - serialize_color_alpha(dest, Some(self.alpha.unwrap_or(0.0)), true)?; + serialize_color_alpha(dest, Some(self.alpha), true)?; dest.write_char(')') } @@ -213,7 +203,7 @@ impl ToCss for Hsl { self.lightness.unwrap_or(0.0), ); - RGBA::from_floats(Some(red), Some(green), Some(blue), self.alpha).to_css(dest) + RgbaLegacy::from_floats(red, green, blue, self.alpha.unwrap_or(OPAQUE)).to_css(dest) } } @@ -280,7 +270,7 @@ impl ToCss for Hwb { self.blackness.unwrap_or(0.0), ); - RGBA::from_floats(Some(red), Some(green), Some(blue), self.alpha).to_css(dest) + RgbaLegacy::from_floats(red, green, blue, self.alpha.unwrap_or(OPAQUE)).to_css(dest) } } @@ -621,7 +611,7 @@ pub enum Color { /// The 'currentcolor' keyword. CurrentColor, /// Specify sRGB colors directly by their red/green/blue/alpha chanels. - Rgba(RGBA), + Rgba(RgbaLegacy), /// Specifies a color in sRGB using hue, saturation and lightness components. Hsl(Hsl), /// Specifies a color in sRGB using hue, whiteness and blackness components. @@ -812,7 +802,7 @@ pub trait FromParsedColor { fn from_current_color() -> Self; /// Construct a new color from red, green, blue and alpha components. - fn from_rgba(red: Option, green: Option, blue: Option, alpha: Option) -> Self; + fn from_rgba(red: u8, green: u8, blue: u8, alpha: f32) -> Self; /// Construct a new color from hue, saturation, lightness and alpha components. fn from_hsl( @@ -876,28 +866,28 @@ where { Ok(match value.len() { 8 => O::from_rgba( - Some(from_hex(value[0])? * 16 + from_hex(value[1])?), - Some(from_hex(value[2])? * 16 + from_hex(value[3])?), - Some(from_hex(value[4])? * 16 + from_hex(value[5])?), - Some((from_hex(value[6])? * 16 + from_hex(value[7])?) as f32 / 255.0), + from_hex(value[0])? * 16 + from_hex(value[1])?, + from_hex(value[2])? * 16 + from_hex(value[3])?, + from_hex(value[4])? * 16 + from_hex(value[5])?, + (from_hex(value[6])? * 16 + from_hex(value[7])?) as f32 / 255.0, ), 6 => O::from_rgba( - Some(from_hex(value[0])? * 16 + from_hex(value[1])?), - Some(from_hex(value[2])? * 16 + from_hex(value[3])?), - Some(from_hex(value[4])? * 16 + from_hex(value[5])?), - Some(OPAQUE), + from_hex(value[0])? * 16 + from_hex(value[1])?, + from_hex(value[2])? * 16 + from_hex(value[3])?, + from_hex(value[4])? * 16 + from_hex(value[5])?, + OPAQUE, ), 4 => O::from_rgba( - Some(from_hex(value[0])? * 17), - Some(from_hex(value[1])? * 17), - Some(from_hex(value[2])? * 17), - Some((from_hex(value[3])? * 17) as f32 / 255.0), + from_hex(value[0])? * 17, + from_hex(value[1])? * 17, + from_hex(value[2])? * 17, + (from_hex(value[3])? * 17) as f32 / 255.0, ), 3 => O::from_rgba( - Some(from_hex(value[0])? * 17), - Some(from_hex(value[1])? * 17), - Some(from_hex(value[2])? * 17), - Some(OPAQUE), + from_hex(value[0])? * 17, + from_hex(value[1])? * 17, + from_hex(value[2])? * 17, + OPAQUE, ), _ => return Err(()), }) @@ -935,8 +925,8 @@ impl FromParsedColor for Color { } #[inline] - fn from_rgba(red: Option, green: Option, blue: Option, alpha: Option) -> Self { - Color::Rgba(RGBA::new(red, green, blue, alpha)) + fn from_rgba(red: u8, green: u8, blue: u8, alpha: f32) -> Self { + Color::Rgba(RgbaLegacy::new(red, green, blue, alpha)) } fn from_hsl( @@ -1174,10 +1164,10 @@ where } match_ignore_ascii_case! { ident , - "transparent" => Ok(Output::from_rgba(Some(0), Some(0), Some(0), Some(0.0))), + "transparent" => Ok(Output::from_rgba(0, 0, 0, 0.0)), "currentcolor" => Ok(Output::from_current_color()), _ => keyword(ident) - .map(|(r, g, b)| Output::from_rgba(Some(*r), Some(*g), Some(*b), Some(1.0))) + .map(|&(r, g, b)| Output::from_rgba(r, g, b, 1.0)) .ok_or(()), } } @@ -1327,47 +1317,50 @@ where // are parsing the legacy syntax. let is_legacy_syntax = maybe_red.is_some() && arguments.try_parse(|p| p.expect_comma()).is_ok(); - let red: Option; - let green: Option; - let blue: Option; - - let alpha = if is_legacy_syntax { - match maybe_red.unwrap() { + let (red, green, blue, alpha) = if is_legacy_syntax { + let (red, green, blue) = match maybe_red.unwrap() { NumberOrPercentage::Number { value } => { - red = Some(clamp_floor_256_f32(value)); - green = Some(clamp_floor_256_f32(color_parser.parse_number(arguments)?)); + let red = clamp_floor_256_f32(value); + let green = clamp_floor_256_f32(color_parser.parse_number(arguments)?); arguments.expect_comma()?; - blue = Some(clamp_floor_256_f32(color_parser.parse_number(arguments)?)); + let blue = clamp_floor_256_f32(color_parser.parse_number(arguments)?); + (red, green, blue) } NumberOrPercentage::Percentage { unit_value } => { - red = Some(clamp_unit_f32(unit_value)); - green = Some(clamp_unit_f32(color_parser.parse_percentage(arguments)?)); + let red = clamp_unit_f32(unit_value); + let green = clamp_unit_f32(color_parser.parse_percentage(arguments)?); arguments.expect_comma()?; - blue = Some(clamp_unit_f32(color_parser.parse_percentage(arguments)?)); + let blue = clamp_unit_f32(color_parser.parse_percentage(arguments)?); + (red, green, blue) } - } + }; - Some(parse_legacy_alpha(color_parser, arguments)?) + let alpha = parse_legacy_alpha(color_parser, arguments)?; + + (red, green, blue, alpha) } else { #[inline] - fn get_component_value(c: Option) -> Option { + fn get_component_value(c: Option) -> u8 { c.map(|c| match c { NumberOrPercentage::Number { value } => clamp_floor_256_f32(value), NumberOrPercentage::Percentage { unit_value } => clamp_unit_f32(unit_value), }) + .unwrap_or(0) } - red = get_component_value(maybe_red); + let red = get_component_value(maybe_red); - green = get_component_value(parse_none_or(arguments, |p| { + let green = get_component_value(parse_none_or(arguments, |p| { color_parser.parse_number_or_percentage(p) })?); - blue = get_component_value(parse_none_or(arguments, |p| { + let blue = get_component_value(parse_none_or(arguments, |p| { color_parser.parse_number_or_percentage(p) })?); - parse_modern_alpha(color_parser, arguments)? + let alpha = parse_modern_alpha(color_parser, arguments)?.unwrap_or(0.0); + + (red, green, blue, alpha) }; Ok(P::Output::from_rgba(red, green, blue, alpha)) diff --git a/src/lib.rs b/src/lib.rs index 7ab88e8f..74173cf6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,7 +70,7 @@ fn parse_border_spacing(_context: &ParserContext, input: &mut Parser) pub use crate::color::{ hsl_to_rgb, hwb_to_rgb, parse_color_keyword, parse_color_with, parse_hash_color, serialize_color_alpha, AngleOrNumber, Color, ColorFunction, ColorParser, FromParsedColor, Hsl, - Hwb, Lab, Lch, NumberOrPercentage, Oklab, Oklch, PredefinedColorSpace, RGBA, + Hwb, Lab, Lch, NumberOrPercentage, Oklab, Oklch, PredefinedColorSpace, RgbaLegacy, }; pub use crate::cow_rc_str::CowRcStr; pub use crate::from_bytes::{stylesheet_encoding, EncodingSupport}; diff --git a/src/tests.rs b/src/tests.rs index e8f94a40..64e995e5 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -18,8 +18,8 @@ use super::{ parse_important, parse_nth, parse_one_declaration, parse_one_rule, stylesheet_encoding, AtRuleParser, BasicParseError, BasicParseErrorKind, Color, CowRcStr, DeclarationParser, Delimiter, EncodingSupport, ParseError, ParseErrorKind, Parser, ParserInput, ParserState, - QualifiedRuleParser, RuleBodyItemParser, RuleBodyParser, SourceLocation, StyleSheetParser, - ToCss, Token, TokenSerializationType, UnicodeRange, RGBA, + QualifiedRuleParser, RgbaLegacy, RuleBodyItemParser, RuleBodyParser, SourceLocation, + StyleSheetParser, ToCss, Token, TokenSerializationType, UnicodeRange, }; macro_rules! JArray { @@ -592,19 +592,19 @@ fn serialize_current_color() { #[test] fn serialize_rgb_full_alpha() { - let c = Color::Rgba(RGBA::new(Some(255), Some(230), Some(204), Some(1.0))); + let c = Color::Rgba(RgbaLegacy::new(255, 230, 204, 1.0)); assert_eq!(c.to_css_string(), "rgb(255, 230, 204)"); } #[test] fn serialize_rgba() { - let c = Color::Rgba(RGBA::new(Some(26), Some(51), Some(77), Some(0.125))); + let c = Color::Rgba(RgbaLegacy::new(26, 51, 77, 0.125)); assert_eq!(c.to_css_string(), "rgba(26, 51, 77, 0.125)"); } #[test] fn serialize_rgba_two_digit_float_if_roundtrips() { - let c = Color::Rgba(RGBA::from_floats(Some(0.), Some(0.), Some(0.), Some(0.5))); + let c = Color::Rgba(RgbaLegacy::from_floats(0., 0., 0., 0.5)); assert_eq!(c.to_css_string(), "rgba(0, 0, 0, 0.5)"); } @@ -1526,7 +1526,7 @@ fn generic_parser() { #[derive(Debug, PartialEq)] enum OutputType { CurrentColor, - Rgba(Option, Option, Option, Option), + Rgba(u8, u8, u8, f32), Hsl(Option, Option, Option, Option), Hwb(Option, Option, Option, Option), Lab(Option, Option, Option, Option), @@ -1547,12 +1547,7 @@ fn generic_parser() { OutputType::CurrentColor } - fn from_rgba( - red: Option, - green: Option, - blue: Option, - alpha: Option, - ) -> Self { + fn from_rgba(red: u8, green: u8, blue: u8, alpha: f32) -> Self { OutputType::Rgba(red, green, blue, alpha) } @@ -1630,10 +1625,10 @@ fn generic_parser() { #[rustfmt::skip] const TESTS: &[(&str, OutputType)] = &[ ("currentColor", OutputType::CurrentColor), - ("rgb(1, 2, 3)", OutputType::Rgba(Some(1), Some(2), Some(3), Some(1.0))), - ("rgba(1, 2, 3, 0.4)", OutputType::Rgba(Some(1), Some(2), Some(3), Some(0.4))), - ("rgb(none none none / none)", OutputType::Rgba(None, None, None, None)), - ("rgb(1 none 3 / none)", OutputType::Rgba(Some(1), None, Some(3), None)), + ("rgb(1, 2, 3)", OutputType::Rgba(1, 2, 3, 1.0)), + ("rgba(1, 2, 3, 0.4)", OutputType::Rgba(1, 2, 3, 0.4)), + ("rgb(none none none / none)", OutputType::Rgba(0, 0, 0, 0.0)), + ("rgb(1 none 3 / none)", OutputType::Rgba(1, 0, 3, 0.0)), ("hsla(45deg, 20%, 30%, 0.4)", OutputType::Hsl(Some(45.0), Some(0.2), Some(0.3), Some(0.4))), ("hsl(45deg none none)", OutputType::Hsl(Some(45.0), None, None, Some(1.0))),