Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[css-color-4] Serialization rules for hue component of lch() and oklch() #7782

Closed
weinig opened this issue Sep 23, 2022 · 6 comments
Closed

[css-color-4] Serialization rules for hue component of lch() and oklch() #7782

weinig opened this issue Sep 23, 2022 · 6 comments
Labels
css-color-4 Current Work

Comments

@weinig
Copy link
Contributor

weinig commented Sep 23, 2022 •
edited by svgeesus
Loading

It was brought to my attention in #7750 that in CSS Color 4 the <hue> is is defined with the following:

This number is not constrained to the range [0,360] but is unbounded. Certain operations, such as hue interpolation, may normalize the hue angle during calculations.

(https://www.w3.org/TR/css-color-4/#hue-syntax)

Currently in WebKit, we normalize hues to [0, 360] for serialization, but I can't find anything that requires that in the spec.

It would be good to clarify the intent and perhaps add some examples of hue > 360 and hue < 0 getting serialized.
@tabatkins
Copy link
Member

Yeah, that serialization would cause a behavior change in some cases. Specifically, in "specified" hue interpolation, there's a big difference between "from 10deg to -10deg" and "from 10deg to 350deg". ^_^

@weinig weinig mentioned this issue Sep 23, 2022
Closed
webkit-early-warning-system pushed a commit to weinig/WebKit that referenced this issue Sep 25, 2022
@weinig
Hue interpolation "specified" is not correctly implemented for hues <…
cf0675f 
… 0 or > 360

https://bugs.webkit.org/show_bug.cgi?id=245552
<rdar://100344615>

Reviewed by Darin Adler.

Defers normalization of the hue component of lch(), oklch(), hsl() and hwb()
colors until necessary, such as due to a color conversion, non-"specified" hue
interpolation method use or serialization (though that one is unclear and is
with the spec editors to decide on via w3c/csswg-drafts#7782).

By deferring this normalization, we now implement the "specified" hue interpolation
method correctly, which explicitly allows for a hue to rotate around the spectrum
multiple times if the difference between angles requires it.

To support this for hsl() and hwb(), which usually are stored as 8-bit sRGB, we use
the same technique as was employed for "none" support, and conditionally use their
extended form if an angle < 0 or > 360 is provided. This means that in the common
cases there will be no change in memory usage.

Additionally, I update the "longer" hue interpolation method to match the updated
spec which changed a "< 0" to a "<= 0".

Tests were added to show that gradients that prior to this change would render the
same due to angle normalization, now render differently.

* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-hsl-expected-mismatch.html: Added.
* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-hsl.html: Added.
* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-hwb-expected-mismatch.html: Added.
* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-hwb.html: Added.
* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-lch-expected-mismatch.html: Added.
* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-lch.html: Added.
* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-oklch-expected-mismatch.html: Added.
* LayoutTests/fast/gradients/gradient-using-specified-hue-interpolation-method-oklch.html: Added.
* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::colorByNormalizingHSLComponents):
(WebCore::CSSPropertyParserHelpers::parseHWBParameters):
(WebCore::CSSPropertyParserHelpers::parseLCHParameters):
* Source/WebCore/platform/graphics/ColorConversion.cpp:
(WebCore::HSLA<float>>::convert):
(WebCore::HWBA<float>>::convert):
* Source/WebCore/platform/graphics/ColorInterpolation.cpp:
(WebCore::fixupHueComponentsPriorToInterpolation):
* Source/WebCore/platform/graphics/ColorModels.h:
* Source/WebCore/platform/graphics/ColorNormalization.h:
(WebCore::makeColorTypeByNormalizingComponents<HWBA<float>>):
(WebCore::makeColorTypeByNormalizingComponents<HSLA<float>>): Deleted.
(WebCore::makeColorTypeByNormalizingComponents<LCHA<float>>): Deleted.
(WebCore::makeColorTypeByNormalizingComponents<OKLCHA<float>>): Deleted.
* Source/WebCore/platform/graphics/ColorSerialization.cpp:
(WebCore::serializationOfLabLikeColorsForCSS):
(WebCore::serializationOfLCHLikeColorsForCSS):
(WebCore::serializationForCSS):
(WebCore::serializationOfLabFamilyForCSS): Deleted.

Canonical link: https://commits.webkit.org/254833@main
@svgeesus svgeesus added the css-color-4 Current Work label Oct 3, 2022
@svgeesus
Copy link
Contributor

svgeesus commented Oct 3, 2022

Currently in WebKit, we normalize hues to [0, 360] for serialization, but I can't find anything that requires that in the spec.

The spec is currently silent on resolving and serializing hue in the polar forms oklch() and lch(). The issue doesn't arise for hsl() and hwb() because the original hue is lost on conversion to rgb().

It should not be, and I agree with the point made in #7750 that being required to track the originally-specified hue is onerous and only makes a difference in hue interpolation with specified. So I would be in favor of explicit normalization to [0, 360) (and dropping specified).

@weinig
Copy link
Contributor Author

weinig commented Oct 3, 2022

As a counterpoint, in WebKit it is not onerous to track the original-specified angle, in fact it is less overall code. I don't see a good reason to unconditionally normalize and drop specified. The only reason it didn't work as spec in WebKit was a misunderstanding on my part.

@weinig
Copy link
Contributor Author

weinig commented Oct 3, 2022

Also, my personal opinion is that it makes sense more sense to serialize with the original angle here, as long as we have it.

@svgeesus
Copy link
Contributor

svgeesus commented Oct 4, 2022

The resolution for this issue and for

should be made together so that they are consistent. I see some implementers arguing for normalizing and some not; and that also affects the utility of the specified hue keyword as well. If we normalize, we should drop specified (completely, not just move to a later specification).

@svgeesus svgeesus mentioned this issue Oct 18, 2022
Closed
@svgeesus
Copy link
Contributor

Now that specified is removed in #7750 I have changed the <hue> definition for early canonicalization, thus allowing a freer choice for internal representation of colors and maintaining the 1:1 colorimetric identity between different syntactical forms:

Because this value is so often given in degrees,
the argument can also be given as a number,
which is interpreted as a number of degrees
and is the [=canonical unit=].

This number is normalized
to the range [0,360].

This also enables the edits from #7876 where channel keywords for hues return a number.

jakearchibald pushed a commit to jakearchibald/csswg-drafts that referenced this issue Jan 16, 2023
@svgeesus @jakearchibald
[css-color-4] No longer require as-specified hue to be retained, fix w…
8d99157 
…3c#7782
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
css-color-4 Current Work
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tabatkins @weinig @svgeesus