Bug 1921766 - Keep colors in non-legacy style before using it as an origin color. r=layout-reviewers,dshin,emilio

When the result of a resolved relative color is in the color(srgb ..)
syntax, make sure not to clamp the components during the calculations.

Differential Revision: https://phabricator.services.mozilla.com/D232338
This commit is contained in:
Tiaan Louw
2025-01-02 11:48:49 +00:00
parent 76fb9966d7
commit 5b56101e16
4 changed files with 68 additions and 100 deletions

View File

@@ -102,32 +102,59 @@ impl ColorFunction<AbsoluteColor> {
Ok(match self {
ColorFunction::Rgb(origin_color, r, g, b, alpha) => {
#[inline]
fn resolve(
component: &ColorComponent<NumberOrPercentageComponent>,
origin_color: Option<&AbsoluteColor>,
) -> Result<u8, ()> {
Ok(clamp_floor_256_f32(
component
.resolve(origin_color)?
.map(|value| value.to_number(u8::MAX as f32))
.unwrap_or(0.0),
))
}
// Use `color(srgb ...)` to serialize `rgb(...)` if an origin color is available;
// this is the only reason for now.
let use_color_syntax = origin_color.is_some();
let origin_color = origin_color
.as_ref()
.map(|o| o.to_color_space(ColorSpace::Srgb).into_srgb_legacy());
if use_color_syntax {
let origin_color = origin_color.as_ref().map(|origin| {
let origin = origin.to_color_space(ColorSpace::Srgb);
// Because rgb(..) syntax have components in range [0..255), we have to
// map them.
// NOTE: The IS_LEGACY_SRGB flag is not added back to the color, because
// we're going to return the modern color(srgb ..) syntax.
AbsoluteColor::new(
ColorSpace::Srgb,
origin.c0().map(|v| v * 255.0),
origin.c1().map(|v| v * 255.0),
origin.c2().map(|v| v * 255.0),
origin.alpha(),
)
});
let r = resolve(r, origin_color.as_ref())?;
let g = resolve(g, origin_color.as_ref())?;
let b = resolve(b, origin_color.as_ref())?;
let alpha = alpha!(alpha, origin_color.as_ref()).unwrap_or(0.0);
if origin_color.is_some() {
AbsoluteColor::new(ColorSpace::Srgb, r, g, b, alpha)
// We have to map all the components back to [0..1) range after all the
// calculations.
AbsoluteColor::new(
ColorSpace::Srgb,
r.resolve(origin_color.as_ref())?
.map(|c| c.to_number(255.0) / 255.0),
g.resolve(origin_color.as_ref())?
.map(|c| c.to_number(255.0) / 255.0),
b.resolve(origin_color.as_ref())?
.map(|c| c.to_number(255.0) / 255.0),
alpha!(alpha, origin_color.as_ref()),
)
} else {
AbsoluteColor::srgb_legacy(r, g, b, alpha)
#[inline]
fn resolve(
component: &ColorComponent<NumberOrPercentageComponent>,
origin_color: Option<&AbsoluteColor>,
) -> Result<u8, ()> {
Ok(clamp_floor_256_f32(
component
.resolve(origin_color)?
.map_or(0.0, |value| value.to_number(u8::MAX as f32)),
))
}
let origin_color = origin_color.as_ref().map(|o| o.into_srgb_legacy());
AbsoluteColor::srgb_legacy(
resolve(r, origin_color.as_ref())?,
resolve(g, origin_color.as_ref())?,
resolve(b, origin_color.as_ref())?,
alpha!(alpha, origin_color.as_ref()).unwrap_or(0.0),
)
}
},
ColorFunction::Hsl(origin_color, h, s, l, alpha) => {

View File

@@ -112,24 +112,27 @@ impl<ValueType: ColorComponentType> ColorComponent<ValueType> {
ColorComponent::None => None,
ColorComponent::Value(value) => Some(value.clone()),
ColorComponent::Calc(node) => {
let Ok(resolved_leaf) = node.resolve_map(|leaf| {
Ok(match leaf {
Leaf::ColorComponent(channel_keyword) => {
if let Some(origin_color) = origin_color {
if let Ok(value) =
origin_color.get_component_by_channel_keyword(*channel_keyword)
{
Leaf::Number(value.unwrap_or(0.0))
let Ok(resolved_leaf) = node.resolve_map(
|leaf| {
Ok(match leaf {
Leaf::ColorComponent(channel_keyword) => {
if let Some(origin_color) = origin_color {
if let Ok(value) = origin_color
.get_component_by_channel_keyword(*channel_keyword)
{
Leaf::Number(value.unwrap_or(0.0))
} else {
return Err(());
}
} else {
return Err(());
}
} else {
return Err(());
}
},
l => l.clone(),
})
}, |_| Err(())) else {
},
l => l.clone(),
})
},
|_| Err(()),
) else {
return Err(());
};

View File

@@ -1,33 +0,0 @@
[color-computed-relative-color.html]
[Property color value 'rgb(from rebeccapurple r g b / none)']
expected: FAIL
[Property color value 'rgb(from rgb(20% 40% 60% / 80%) r g b / none)']
expected: FAIL
[Property color value 'color-mix(in srgb, rgb(from rebeccapurple none g b), rebeccapurple)']
expected: FAIL
[Property color value 'color(from rgb(from color(xyz-d50 0.99 0.88 0.77) r g b) xyz-d50 x y z)']
expected: FAIL
[Property color value 'color(from rgb(from color(xyz-d65 0.99 0.88 0.77) r g b) xyz-d65 x y z)']
expected: FAIL
[Property color value 'rgb(from rebeccapurple none none none)']
expected: FAIL
[Property color value 'rgb(from rgb(20% 40% 60% / 80%) r g none / alpha)']
expected: FAIL
[Property color value 'rgb(from rebeccapurple r g none)']
expected: FAIL
[Property color value 'rgb(from rebeccapurple none none none / none)']
expected: FAIL
[Property color value 'rgb(from rebeccapurple r g none / alpha)']
expected: FAIL
[Property color value 'light-dark(color-mix(in srgb, rgb(from rebeccapurple none g b), rebeccapurple), color-mix(in srgb, rgb(from rebeccapurple none g b), rebeccapurple))']
expected: FAIL

View File

@@ -1,29 +0,0 @@
[relative-color-out-of-gamut.html]
bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1921766
[Property color value 'rgb(from color(display-p3 0 1 0) r g b / alpha)']
expected: FAIL
[Property color value 'rgb(from lab(100 104.3 -50.9) r g b)']
expected: FAIL
[Property color value 'rgb(from lab(0 104.3 -50.9) r g b)']
expected: FAIL
[Property color value 'rgb(from lch(100 116 334) r g b)']
expected: FAIL
[Property color value 'rgb(from lch(0 116 334) r g b)']
expected: FAIL
[Property color value 'rgb(from oklab(1 0.365 -0.16) r g b)']
expected: FAIL
[Property color value 'rgb(from oklab(0 0.365 -0.16) r g b)']
expected: FAIL
[Property color value 'rgb(from oklch(1 0.399 336.3) r g b)']
expected: FAIL
[Property color value 'rgb(from oklch(0 0.399 336.3) r g b)']
expected: FAIL