From bb55e923bb2a8dd0d639316790ef947f3d804d1e Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Thu, 2 Nov 2023 22:34:38 +0100 Subject: [PATCH] Further changes required by Servo --- Cargo.lock | 12 +++--- Cargo.toml | 2 +- components/canvas/raqote_backend.rs | 38 +++++++++++++++++-- components/layout_2020/display_list/mod.rs | 2 +- .../display_list/stacking_context.rs | 2 +- components/layout_2020/style_ext.rs | 2 +- components/script/canvas_state.rs | 14 +++---- components/script/dom/canvasgradient.rs | 6 +-- components/selectors/Cargo.toml | 2 +- components/style/Cargo.toml | 2 +- components/style/attr.rs | 8 ++-- components/style/servo/media_queries.rs | 4 +- components/style/values/specified/color.rs | 10 ++++- components/style_traits/Cargo.toml | 2 +- tests/unit/style/animated_properties.rs | 8 ++-- 15 files changed, 76 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68184e4a052..0de50631b2a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1170,9 +1170,8 @@ dependencies = [ [[package]] name = "cssparser" -version = "0.29.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f93d03419cb5950ccfd3daf3ff1c7a36ace64609a1a8746d493df1ca0afde0fa" +version = "0.30.0" +source = "git+https://github.com/servo/rust-cssparser?rev=722b30d2f1634714befab967ecae627813fa4cf0#722b30d2f1634714befab967ecae627813fa4cf0" dependencies = [ "cssparser-macros", "dtoa-short", @@ -1188,12 +1187,11 @@ dependencies = [ [[package]] name = "cssparser-macros" -version = "0.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13b588ba4ac1a99f7f2964d24b3d896ddc6bf847ee3855dbd4366f058cfcd331" +version = "0.6.0" +source = "git+https://github.com/servo/rust-cssparser?rev=722b30d2f1634714befab967ecae627813fa4cf0#722b30d2f1634714befab967ecae627813fa4cf0" dependencies = [ "quote", - "syn 2.0.38", + "syn 1.0.103", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index c775a9ffddb..429b60215d6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ compositing_traits = { path = "components/shared/compositing" } content-security-policy = { version = "0.5", features = ["serde"] } cookie = "0.12" crossbeam-channel = "0.5" -cssparser = "0.29" +cssparser = { version = "0.30", git = "https://github.com/servo/rust-cssparser", rev = "722b30d2f1634714befab967ecae627813fa4cf0" } darling = { version = "0.14", default-features = false } data-url = "0.1.0" devtools_traits = { path = "components/shared/devtools" } diff --git a/components/canvas/raqote_backend.rs b/components/canvas/raqote_backend.rs index bb17fb8c344..ab4bab11c8d 100644 --- a/components/canvas/raqote_backend.rs +++ b/components/canvas/raqote_backend.rs @@ -855,10 +855,37 @@ pub trait ToRaqoteGradientStop { fn to_raqote(&self) -> raqote::GradientStop; } +/// Clamp a 0..1 number to a 0..255 range to u8. +/// +/// Whilst scaling by 256 and flooring would provide +/// an equal distribution of integers to percentage inputs, +/// this is not what Gecko does so we instead multiply by 255 +/// and round (adding 0.5 and flooring is equivalent to rounding) +/// +/// Chrome does something similar for the alpha value, but not +/// the rgb values. +/// +/// See +/// +/// Clamping to 256 and rounding after would let 1.0 map to 256, and +/// `256.0_f32 as u8` is undefined behavior: +/// +/// +#[inline] +pub fn clamp_unit_f32(val: f32) -> u8 { + clamp_floor_256_f32(val * 255.) +} + +/// Round and clamp a single number to a u8. +#[inline] +pub fn clamp_floor_256_f32(val: f32) -> u8 { + val.round().clamp(0., 255.) as u8 +} + impl ToRaqoteGradientStop for CanvasGradientStop { fn to_raqote(&self) -> raqote::GradientStop { let color = raqote::Color::new( - self.color.alpha, + clamp_unit_f32(self.color.alpha), self.color.red, self.color.green, self.color.blue, @@ -875,7 +902,7 @@ impl<'a> ToRaqotePattern<'_> for FillOrStrokeStyle { match self { Color(color) => Some(Pattern::Color( - color.alpha, + clamp_unit_f32(color.alpha), color.red, color.green, color.blue, @@ -933,7 +960,12 @@ impl ToRaqoteStyle for RGBA { type Target = raqote::SolidSource; fn to_raqote_style(self) -> Self::Target { - raqote::SolidSource::from_unpremultiplied_argb(self.alpha, self.red, self.green, self.blue) + raqote::SolidSource::from_unpremultiplied_argb( + clamp_unit_f32(self.alpha), + self.red, + self.green, + self.blue, + ) } } diff --git a/components/layout_2020/display_list/mod.rs b/components/layout_2020/display_list/mod.rs index e4b88472554..ec8ad43b270 100644 --- a/components/layout_2020/display_list/mod.rs +++ b/components/layout_2020/display_list/mod.rs @@ -553,7 +553,7 @@ impl<'a> BuilderForBoxFragment<'a> { let style = &self.fragment.style; let b = style.get_background(); let background_color = style.resolve_color(b.background_color.clone()); - if background_color.alpha > 0 { + if background_color.alpha > 0.0 { // https://drafts.csswg.org/css-backgrounds/#background-color // “The background color is clipped according to the background-clip // value associated with the bottom-most background image layer.” diff --git a/components/layout_2020/display_list/stacking_context.rs b/components/layout_2020/display_list/stacking_context.rs index fb479accd36..2ee572a805d 100644 --- a/components/layout_2020/display_list/stacking_context.rs +++ b/components/layout_2020/display_list/stacking_context.rs @@ -505,7 +505,7 @@ impl StackingContext { .to_webrender(); let background_color = style.resolve_color(style.get_background().background_color.clone()); - if background_color.alpha > 0 { + if background_color.alpha > 0.0 { let common = builder.common_properties(painting_area, &style); let color = super::rgba(background_color); builder diff --git a/components/layout_2020/style_ext.rs b/components/layout_2020/style_ext.rs index 4a2ef77f8b4..78f06625e06 100644 --- a/components/layout_2020/style_ext.rs +++ b/components/layout_2020/style_ext.rs @@ -472,7 +472,7 @@ impl ComputedValuesExt for ComputedValues { fn background_is_transparent(&self) -> bool { let background = self.get_background(); let color = self.resolve_color(background.background_color.clone()); - color.alpha == 0 && + color.alpha == 0.0 && background .background_image .0 diff --git a/components/script/canvas_state.rs b/components/script/canvas_state.rs index 12ab63c89f3..353ab15bb18 100644 --- a/components/script/canvas_state.rs +++ b/components/script/canvas_state.rs @@ -12,7 +12,7 @@ use canvas_traits::canvas::{ FillRule, LineCapStyle, LineJoinStyle, LinearGradientStyle, RadialGradientStyle, RepetitionStyle, TextAlign, TextBaseline, }; -use cssparser::{Color as CSSColor, Parser, ParserInput, RGBA}; +use cssparser::{AbsoluteColor, Color as CSSColor, Parser, ParserInput, RGBA}; use euclid::default::{Point2D, Rect, Size2D, Transform2D}; use euclid::vec2; use ipc_channel::ipc::{self, IpcSender, IpcSharedMemory}; @@ -106,7 +106,7 @@ impl CanvasContextState { const DEFAULT_FONT_STYLE: &'static str = "10px sans-serif"; pub(crate) fn new() -> CanvasContextState { - let black = RGBA::new(0, 0, 0, 255); + let black = RGBA::new(0, 0, 0, 1.0); CanvasContextState { global_alpha: 1.0, global_composition: CompositionOrBlending::default(), @@ -302,7 +302,7 @@ impl CanvasState { let color = CSSColor::parse(&mut parser); if parser.is_exhausted() { match color { - Ok(CSSColor::RGBA(rgba)) => Ok(rgba), + Ok(CSSColor::Absolute(AbsoluteColor::Rgba(rgba))) => Ok(rgba), Ok(CSSColor::CurrentColor) => { // TODO: https://github.com/whatwg/html/issues/1099 // Reconsider how to calculate currentColor in a display:none canvas @@ -313,7 +313,7 @@ impl CanvasState { // https://drafts.css-houdini.org/css-paint-api/#2d-rendering-context // Whenever "currentColor" is used as a color in the PaintRenderingContext2D API, // it is treated as opaque black. - None => return Ok(RGBA::new(0, 0, 0, 255)), + None => return Ok(RGBA::new(0, 0, 0, 1.0)), Some(ref canvas) => &**canvas, }; @@ -323,7 +323,7 @@ impl CanvasState { Some(ref s) if canvas_element.has_css_layout_box() => { Ok(s.get_inherited_text().color) }, - _ => Ok(RGBA::new(0, 0, 0, 255)), + _ => Ok(RGBA::new(0, 0, 0, 1.0)), } }, _ => Err(()), @@ -1706,7 +1706,7 @@ pub fn parse_color(string: &str) -> Result { let mut input = ParserInput::new(string); let mut parser = Parser::new(&mut input); match CSSColor::parse(&mut parser) { - Ok(CSSColor::RGBA(rgba)) => { + Ok(CSSColor::Absolute(AbsoluteColor::Rgba(rgba))) => { if parser.is_exhausted() { Ok(rgba) } else { @@ -1732,7 +1732,7 @@ where let green = color.green; let blue = color.blue; - if color.alpha == 255 { + if color.alpha == 1.0 { write!( dest, "#{:x}{:x}{:x}{:x}{:x}{:x}", diff --git a/components/script/dom/canvasgradient.rs b/components/script/dom/canvasgradient.rs index e65f3047a40..10dd7d60cd7 100644 --- a/components/script/dom/canvasgradient.rs +++ b/components/script/dom/canvasgradient.rs @@ -5,7 +5,7 @@ use canvas_traits::canvas::{ CanvasGradientStop, FillOrStrokeStyle, LinearGradientStyle, RadialGradientStyle, }; -use cssparser::{Color as CSSColor, Parser, ParserInput, RGBA}; +use cssparser::{AbsoluteColor, Color as CSSColor, Parser, ParserInput, RGBA}; use dom_struct::dom_struct; use crate::dom::bindings::cell::DomRefCell; @@ -58,8 +58,8 @@ impl CanvasGradientMethods for CanvasGradient { let color = CSSColor::parse(&mut parser); let color = if parser.is_exhausted() { match color { - Ok(CSSColor::RGBA(rgba)) => rgba, - Ok(CSSColor::CurrentColor) => RGBA::new(0, 0, 0, 255), + Ok(CSSColor::Absolute(AbsoluteColor::Rgba(rgba))) => rgba, + Ok(CSSColor::CurrentColor) => RGBA::new(0, 0, 0, 1.0), _ => return Err(Error::Syntax), } } else { diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index b1afce74581..c2935accf7d 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -21,7 +21,7 @@ shmem = ["dep:to_shmem", "dep:to_shmem_derive"] [dependencies] bitflags = "1.0" -cssparser = "0.30" +cssparser = { workspace = true } derive_more = { version = "0.99", default-features = false, features = ["add", "add_assign"] } fxhash = "0.2" log = "0.4" diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index a70c379761d..bab06fcdbd9 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -41,7 +41,7 @@ arrayvec = "0.7" atomic_refcell = "0.1" bitflags = "1.0" byteorder = "1.0" -cssparser = "0.30" +cssparser = { workspace = true } derive_more = { version = "0.99", default-features = false, features = ["add", "add_assign", "deref", "from"] } encoding_rs = { version = "0.8", optional = true } euclid = "0.22" diff --git a/components/style/attr.rs b/components/style/attr.rs index 05833fa08d8..19beb4f436d 100644 --- a/components/style/attr.rs +++ b/components/style/attr.rs @@ -15,7 +15,7 @@ use crate::values::specified::Length; use crate::values::AtomString; use crate::{Atom, LocalName, Namespace, Prefix}; use app_units::Au; -use cssparser::{self, Color, RGBA}; +use cssparser::{self, AbsoluteColor, Color, RGBA}; use euclid::num::Zero; use num_traits::ToPrimitive; use selectors::attr::AttrSelectorOperation; @@ -419,7 +419,7 @@ pub fn parse_legacy_color(mut input: &str) -> Result { } // Step 5. - if let Ok(Color::RGBA(rgba)) = cssparser::parse_color_keyword(input) { + if let Ok(Color::Absolute(AbsoluteColor::Rgba(rgba))) = cssparser::parse_color_keyword(input) { return Ok(rgba); } @@ -431,7 +431,7 @@ pub fn parse_legacy_color(mut input: &str) -> Result { hex(input.as_bytes()[2] as char), hex(input.as_bytes()[3] as char), ) { - return Ok(RGBA::new(r * 17, g * 17, b * 17, 255)); + return Ok(RGBA::new(r * 17, g * 17, b * 17, 1.0)); } } @@ -504,7 +504,7 @@ pub fn parse_legacy_color(mut input: &str) -> Result { hex_string(red).unwrap(), hex_string(green).unwrap(), hex_string(blue).unwrap(), - 255, + 1.0, )); fn hex(ch: char) -> Result { diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index 6440afb45ed..269c8a5c056 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -197,12 +197,12 @@ impl Device { /// Returns the default background color. pub fn default_background_color(&self) -> RGBA { - RGBA::new(255, 255, 255, 255) + RGBA::new(255, 255, 255, 1.0) } /// Returns the default foreground color. pub fn default_color(&self) -> RGBA { - RGBA::new(0, 0, 0, 255) + RGBA::new(0, 0, 0, 1.0) } /// Returns safe area insets diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index c78a502a133..87b063cec86 100644 --- a/components/style/values/specified/color.rs +++ b/components/style/values/specified/color.rs @@ -33,6 +33,14 @@ fn allow_color_mix() -> bool { return false; } +#[inline] +fn allow_more_color_4() -> bool { + #[cfg(feature = "gecko")] + return static_prefs::pref!("layout.css.more_color_4.enabled"); + #[cfg(feature = "servo")] + return false; +} + impl ColorMix { fn parse<'i, 't>( context: &ParserContext, @@ -759,7 +767,7 @@ impl Color { CSSParserColor::CurrentColor => Color::CurrentColor, CSSParserColor::Absolute(absolute) => { let enabled = matches!(absolute, cssparser::AbsoluteColor::Rgba(_)) || - static_prefs::pref!("layout.css.more_color_4.enabled"); + allow_more_color_4(); if !enabled { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } diff --git a/components/style_traits/Cargo.toml b/components/style_traits/Cargo.toml index 6f5d5e6ed2c..f353d203e42 100644 --- a/components/style_traits/Cargo.toml +++ b/components/style_traits/Cargo.toml @@ -17,7 +17,7 @@ gecko = [] [dependencies] app_units = "0.7" bitflags = "1.0" -cssparser = "0.30" +cssparser = { workspace = true } euclid = "0.22" lazy_static = "1" malloc_size_of = { path = "../malloc_size_of" } diff --git a/tests/unit/style/animated_properties.rs b/tests/unit/style/animated_properties.rs index fd03a1904e8..c91a39b1e1a 100644 --- a/tests/unit/style/animated_properties.rs +++ b/tests/unit/style/animated_properties.rs @@ -26,8 +26,8 @@ fn test_rgba_color_interepolation_preserves_transparent() { #[test] fn test_rgba_color_interepolation_alpha() { assert_eq!( - interpolate_rgba(RGBA::new(200, 0, 0, 100), RGBA::new(0, 200, 0, 200), 0.5), - RGBA::new(67, 133, 0, 150) + interpolate_rgba(RGBA::new(200, 0, 0, 0.4), RGBA::new(0, 200, 0, 0.8), 0.5), + RGBA::new(67, 133, 0, 0.6) ); } @@ -41,7 +41,7 @@ fn test_rgba_color_interepolation_out_of_range_1() { RGBA::from_floats(0.0, 1.0, 0.0, 0.6), -0.5 ), - RGBA::new(154, 0, 0, 77) + RGBA::new(154, 0, 0, 0.3) ); } @@ -53,7 +53,7 @@ fn test_rgba_color_interepolation_out_of_range_2() { RGBA::from_floats(0.0, 0.3, 0.0, 0.4), 1.5 ), - RGBA::new(0, 154, 0, 77) + RGBA::new(0, 154, 0, 0.3) ); }