From 21af958c8769e2648784fea76a113305452802ac Mon Sep 17 00:00:00 2001 From: Narcis Beleuzu Date: Wed, 1 Nov 2023 03:22:46 +0200 Subject: [PATCH] Backed out changeset e5e1bc9736a5 (bug 1847503) for reftest failures on anim-css-floodcolor-overflow-1-from-by.svg . CLOSED TREE --- layout/reftests/svg/smil/style/reftest.list | 16 +- layout/style/StyleColor.cpp | 38 +--- modules/libpref/init/StaticPrefList.yaml | 6 - servo/components/style/color/gamut.rs | 196 ------------------ servo/components/style/color/mod.rs | 10 - servo/ports/geckolib/glue.rs | 7 - .../css-color/color-mix-non-srgb-001-ref.html | 18 +- .../css/css-color/color-mix-non-srgb-001.html | 18 +- 8 files changed, 38 insertions(+), 271 deletions(-) delete mode 100644 servo/components/style/color/gamut.rs diff --git a/layout/reftests/svg/smil/style/reftest.list b/layout/reftests/svg/smil/style/reftest.list index 0d062509d5c3..a7888a3abeb6 100644 --- a/layout/reftests/svg/smil/style/reftest.list +++ b/layout/reftests/svg/smil/style/reftest.list @@ -46,14 +46,16 @@ fuzzy(0-1,0-580) == anim-css-fill-3-from-by-ident-ident.svg anim-css-fill-3-ref. fuzzy(0-1,0-580) == anim-css-fill-3-from-by-rgb-ident.svg anim-css-fill-3-ref.svg # check handling of overflowing color values -# Gamut mapping is now available and presentation colors are not clipped -# any more. Because the result is dependent on hardware, we use a fairly high -# fuzzy for now. +# NOTE: Some of the tests below fail in Gecko because we compute +# "from + by" as the animation end-point, and we clamp that final color value +# (due to bug 515919) and use the clamped value for interpolation. +# That's earlier than the SVG spec wants us to clamp -- we're only supposed to +# clamp *final presentation values*. # (Reference: SVG 1.1 Appendix F.4) -fuzzy(0-19,0-900) == anim-css-fill-overflow-1-by.svg anim-css-fill-overflow-1-ref.svg -fuzzy(0-19,0-900) == anim-css-fill-overflow-1-from-by.svg anim-css-fill-overflow-1-ref.svg -fuzzy(0-19,0-900) == anim-css-stopcolor-overflow-1-from-by.svg anim-css-stopcolor-overflow-1-ref.svg -fuzzy(0-19,0-900) == anim-css-floodcolor-overflow-1-from-by.svg anim-css-floodcolor-overflow-1-ref.svg +== anim-css-fill-overflow-1-by.svg anim-css-fill-overflow-1-ref.svg +== anim-css-fill-overflow-1-from-by.svg anim-css-fill-overflow-1-ref.svg # bug 515919 +== anim-css-stopcolor-overflow-1-from-by.svg anim-css-stopcolor-overflow-1-ref.svg # bug 515919 +== anim-css-floodcolor-overflow-1-from-by.svg anim-css-floodcolor-overflow-1-ref.svg # bug 515919 # 'fill-opacity' property fuzzy(0-1,0-885) == anim-css-fillopacity-1-by.svg anim-css-fillopacity-1-ref.svg diff --git a/layout/style/StyleColor.cpp b/layout/style/StyleColor.cpp index 03e516990a11..df58cf093d65 100644 --- a/layout/style/StyleColor.cpp +++ b/layout/style/StyleColor.cpp @@ -7,7 +7,8 @@ #include "mozilla/StyleColorInlines.h" #include "mozilla/ComputedStyle.h" -#include "mozilla/StaticPrefs_layout.h" +#include "mozilla/ComputedStyleInlines.h" +#include "mozilla/dom/BindingDeclarations.h" #include "nsIFrame.h" #include "nsStyleStruct.h" @@ -64,33 +65,16 @@ StyleAbsoluteColor StyleAbsoluteColor::ToColorSpace( nscolor StyleAbsoluteColor::ToColor() const { auto srgb = ToColorSpace(StyleColorSpace::Srgb); - constexpr float MIN = 0.0f; - constexpr float MAX = 1.0f; + // TODO(tlouw): Needs gamut mapping here. Right now we just hard clip the + // components to [0..1], which will yield invalid colors. + // https://bugzilla.mozilla.org/show_bug.cgi?id=1626624 + auto red = std::clamp(srgb.components._0, 0.0f, 1.0f); + auto green = std::clamp(srgb.components._1, 0.0f, 1.0f); + auto blue = std::clamp(srgb.components._2, 0.0f, 1.0f); - // We KNOW the values are in srgb so we can do a quick gamut limit check - // here and avoid calling into Servo_MapColorIntoGamutLimits and let it - // return early anyway. - auto isColorInGamut = - (srgb.components._0 >= MIN && srgb.components._0 <= MAX && - srgb.components._1 >= MIN && srgb.components._1 <= MAX && - srgb.components._2 >= MIN && srgb.components._2 <= MAX); - - if (!isColorInGamut) { - if (StaticPrefs::layout_css_gamut_map_for_rendering_enabled()) { - srgb = Servo_MapColorIntoGamutLimits(&srgb); - } else { - // If gamut mapping is not enabled, we just naively clip the colors at - // sRGB gamut limits. This will go away completely when gamut mapping is - // enabled. - srgb.components._0 = std::clamp(srgb.components._0, 0.0f, 1.0f); - srgb.components._1 = std::clamp(srgb.components._1, 0.0f, 1.0f); - srgb.components._2 = std::clamp(srgb.components._2, 0.0f, 1.0f); - } - } - - return NS_RGBA(nsStyleUtil::FloatToColorComponent(srgb.components._0), - nsStyleUtil::FloatToColorComponent(srgb.components._1), - nsStyleUtil::FloatToColorComponent(srgb.components._2), + return NS_RGBA(nsStyleUtil::FloatToColorComponent(red), + nsStyleUtil::FloatToColorComponent(green), + nsStyleUtil::FloatToColorComponent(blue), nsStyleUtil::FloatToColorComponent(srgb.alpha)); } diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 7fdc240bde11..04604e961c80 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -8407,12 +8407,6 @@ value: 3 mirror: always -# Should colors that are out of gamut for sRGB mapped to within limits? -- name: layout.css.gamut-map-for-rendering.enabled - type: bool - value: @IS_NIGHTLY_BUILD@ - mirror: always - # Is support for GeometryUtils.getBoxQuads enabled? - name: layout.css.getBoxQuads.enabled type: bool diff --git a/servo/components/style/color/gamut.rs b/servo/components/style/color/gamut.rs deleted file mode 100644 index 9929d6c398af..000000000000 --- a/servo/components/style/color/gamut.rs +++ /dev/null @@ -1,196 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -//! Gamut mapping. -//! - -use super::{AbsoluteColor, ColorSpace}; - -impl AbsoluteColor { - /// Map the components of this color into it's gamut limits if needed. - /// - pub fn map_into_gamut_limits(&self) -> Self { - // 1. if destination has no gamut limits (XYZ-D65, XYZ-D50, Lab, LCH, - // Oklab, Oklch) return origin. - if matches!( - self.color_space, - ColorSpace::Lab | - ColorSpace::Lch | - ColorSpace::Oklab | - ColorSpace::Oklch | - ColorSpace::XyzD50 | - ColorSpace::XyzD65 - ) { - return self.clone(); - } - - // If the color does have gamut limits, then we can check it here and - // possibly skip the binary search. - if self.in_gamut() { - return self.clone(); - } - - // 2. let origin_Oklch be origin converted from origin color space to - // the Oklch color space. - let origin_oklch = self.to_color_space(ColorSpace::Oklch); - - // 3. if the Lightness of origin_Oklch is greater than or equal to - // 100%, return { 1 1 1 origin.alpha } in destination. - if origin_oklch.components.1 >= 1.0 { - return AbsoluteColor::new(self.color_space, 1.0, 1.0, 1.0, self.alpha); - } - - // 4. if the Lightness of origin_Oklch is less than than or equal to - // 0%, return { 0 0 0 origin.alpha } in destination. - if origin_oklch.components.1 <= 0.0 { - return AbsoluteColor::new(self.color_space, 0.0, 0.0, 0.0, self.alpha); - } - - // 5. let inGamut(color) be a function which returns true if, when - // passed a color, that color is inside the gamut of destination. - // For HSL and HWB, it returns true if the color is inside the gamut - // of sRGB. - // See [`Color::in_gamut`]. - - // 6. if inGamut(origin_Oklch) is true, convert origin_Oklch to - // destination and return it as the gamut mapped color. - // We already checked if the color is in gamut limits above. - - // 7. otherwise, let delta(one, two) be a function which returns the - // deltaEOK of color one compared to color two. - // See the [`delta_eok`] function. - - // 8. let JND be 0.02 - const JND: f32 = 0.02; - - // 9. let epsilon be 0.0001 - const EPSILON: f32 = 0.0001; - - // 10. let clip(color) be a function which converts color to - // destination, converts all negative components to zero, converts - // all components greater that one to one, and returns the result. - // See [`Color::clip`]. - - // 11. set min to zero - let mut min = 0.0; - - // 12. set max to the Oklch chroma of origin_Oklch. - let mut max = origin_oklch.components.1; - - // 13. let min_inGamut be a boolean that represents when min is still - // in gamut, and set it to true - let mut min_in_gamut = true; - - let mut current = origin_oklch.clone(); - let mut current_in_space = self.clone(); - - // If we are already within the JND threshold, return the clipped color - // and skip the binary search. - let clipped = current_in_space.clip(); - if delta_eok(&clipped, ¤t) < JND { - return clipped; - } - - // 14. while (max - min is greater than epsilon) repeat the following - // steps. - while max - min > EPSILON { - // 14.1. set chroma to (min + max) / 2 - let chroma = (min + max) / 2.0; - - // 14.2. set current to origin_Oklch and then set the chroma - // component to chroma - current.components.1 = chroma; - - current_in_space = current.to_color_space(self.color_space); - - // 14.3. if min_inGamut is true and also if inGamut(current) is - // true, set min to chroma and continue to repeat these steps. - if min_in_gamut && current_in_space.in_gamut() { - min = chroma; - continue; - } - - // 14.4. otherwise, if inGamut(current) is false carry out these - // steps: - - // 14.4.1. set clipped to clip(current) - let clipped = current_in_space.clip(); - - // 14.4.2. set E to delta(clipped, current) - let e = delta_eok(&clipped, ¤t); - - // 14.4.3. if E < JND - if e < JND { - // 14.4.3.1. if (JND - E < epsilon) return clipped as the gamut - // mapped color - if JND - e < EPSILON { - return clipped; - } - - // 14.4.3.2. otherwise - - // 14.4.3.2.1. set min_inGamut to false - min_in_gamut = false; - - // 14.4.3.2.2. set min to chroma - min = chroma; - } else { - // 14.4.4. otherwise, set max to chroma and continue to repeat - // these steps - max = chroma; - } - } - - // 15. return current as the gamut mapped color current - current_in_space - } - - /// Clamp this color to within the [0..1] range. - fn clip(&self) -> Self { - let mut result = self.clone(); - result.components = result.components.map(|c| c.clamp(0.0, 1.0)); - result - } - - /// Returns true if this color is within its gamut limits. - fn in_gamut(&self) -> bool { - macro_rules! in_range { - ($c:expr) => {{ - $c >= 0.0 && $c <= 1.0 - }}; - } - - match self.color_space { - ColorSpace::Hsl | ColorSpace::Hwb => self.to_color_space(ColorSpace::Srgb).in_gamut(), - ColorSpace::Srgb | - ColorSpace::SrgbLinear | - ColorSpace::DisplayP3 | - ColorSpace::A98Rgb | - ColorSpace::ProphotoRgb | - ColorSpace::Rec2020 => { - in_range!(self.components.0) && - in_range!(self.components.1) && - in_range!(self.components.2) - }, - ColorSpace::Lab | - ColorSpace::Lch | - ColorSpace::Oklab | - ColorSpace::Oklch | - ColorSpace::XyzD50 | - ColorSpace::XyzD65 => true, - } - } -} - -/// Calculate deltaE OK (simple root sum of squares). -/// -fn delta_eok(reference: &AbsoluteColor, sample: &AbsoluteColor) -> f32 { - // Delta is calculated in the oklab color space. - let reference = reference.to_color_space(ColorSpace::Oklab); - let sample = sample.to_color_space(ColorSpace::Oklab); - - let diff = reference.components - sample.components; - let diff = diff * diff; - (diff.0 + diff.1 + diff.2).sqrt() -} diff --git a/servo/components/style/color/mod.rs b/servo/components/style/color/mod.rs index 444a2476c293..797a1cb00fd2 100644 --- a/servo/components/style/color/mod.rs +++ b/servo/components/style/color/mod.rs @@ -9,8 +9,6 @@ pub mod convert; pub mod mix; pub mod parsing; -mod gamut; - use cssparser::color::PredefinedColorSpace; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -28,14 +26,6 @@ impl ColorComponents { } } -impl std::ops::Sub for ColorComponents { - type Output = Self; - - fn sub(self, rhs: Self) -> Self::Output { - Self(self.0 - rhs.0, self.1 - rhs.1, self.2 - rhs.2) - } -} - impl std::ops::Mul for ColorComponents { type Output = Self; diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index e14f728b7613..3d49c54480cf 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -7855,13 +7855,6 @@ pub extern "C" fn Servo_ConvertColorSpace( color.to_color_space(color_space) } -#[no_mangle] -pub extern "C" fn Servo_MapColorIntoGamutLimits( - color: &AbsoluteColor -) -> AbsoluteColor { - color.map_into_gamut_limits() -} - #[no_mangle] pub unsafe extern "C" fn Servo_IntersectionObserverRootMargin_Parse( value: &nsACString, diff --git a/testing/web-platform/tests/css/css-color/color-mix-non-srgb-001-ref.html b/testing/web-platform/tests/css/css-color/color-mix-non-srgb-001-ref.html index fcb52326f67d..15556737c7f8 100644 --- a/testing/web-platform/tests/css/css-color/color-mix-non-srgb-001-ref.html +++ b/testing/web-platform/tests/css/css-color/color-mix-non-srgb-001-ref.html @@ -9,15 +9,15 @@ div { color: black; }