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.

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

[css-color-4] Incorrect description on preprocessing hue angles for interpolation #8838

Closed
yarusome opened this issue May 13, 2023 · 7 comments

Comments

@yarusome
Copy link

yarusome commented May 13, 2023

The Hue Interpolation section says:

Both angles and their difference need to be constrained to [0, 360] prior to interpolation. To do this, the minimum number of turns that fit in the lesser angle is added or subtracted from both angles, bringing the lesser angle into the range [0,360); and if the difference between them is greater than 360° then the minimum number of turns to bring the difference into the range [0,360] is further subtracted from the greater angle.

However, the angle pair 1, θ2) = (40deg, 400deg) passes the preprocessing steps without being modified, leaving with θ2 > 360deg.

Based on the four algorithms described in this section, it seems that only the lesser of the two angles needs to be in [0deg, 360deg) (and the difference be in [0deg, 360deg]). However, this implies that the greater angle can be outside [0deg, 360deg], which contradicts Section 4.3:

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

@svgeesus
Copy link
Contributor

svgeesus commented Jul 3, 2023

@LeaVerou could you look into this please?

@svgeesus
Copy link
Contributor

@svgeesus
Copy link
Contributor

@LeaVerou I suspect that should be (my emphasis)

constrained to [0, 360 )

and

is greater than or equal to 360°

which, in the example above, would give 400 - 360 = 40

@yarusome
Copy link
Author

yarusome commented Aug 24, 2023

@svgeesus Two small issues regarding 2641c47:

  1. After this commit, it's no longer possible to specify a full circumference with increasing hue and decreasing hue. Is this behavioral change intended? (I'd like to make sure before updating MDN.)

  2. The explanatory texts for pseudo-codes:

    <h4 id="hue-longer">
    <dfn export>longer</dfn></h4>
    Angles are adjusted so that |θ₂ - θ₁| ∈ {[-360, -180], [180, 360]}. In pseudo-Javascript:
    <pre>
    if (0 < θ₂ - θ₁ < 180) {
    θ₁ += 360;
    }
    else if (-180 < θ₂ - θ₁ <= 0) {
    θ₂ += 360;
    }
    </pre>
    <h4 id="hue-increasing">
    <dfn export>increasing</dfn></h4>
    Angles are adjusted so that |θ₂ - θ₁| ∈ [0, 360]. In pseudo-Javascript:
    <pre>
    if (θ₂ < θ₁) {
    θ₂ += 360;
    }
    </pre>
    <h4 id="hue-decreasing">
    <dfn export>decreasing</dfn></h4>
    Angles are adjusted so that |θ₂ - θ₁| ∈ [-360, 0]. In pseudo-Javascript:
    <pre>
    if (θ₁ < θ₂) {
    θ₁ += 360;
    }
    </pre>

    may also be updated:

    - 	Angles are adjusted so that |θ₂ - θ₁| ∈ {[-360, -180], [180, 360]}. In pseudo-Javascript:
    + 	Angles are adjusted so that |θ₂ - θ₁| ∈ {(-360, -180], [180, 360]}. In pseudo-Javascript:
    - 	Angles are adjusted so that |θ₂ - θ₁| ∈ [0, 360]. In pseudo-Javascript:
    + 	Angles are adjusted so that |θ₂ - θ₁| ∈ [0, 360). In pseudo-Javascript:
    - 	Angles are adjusted so that |θ₂ - θ₁| ∈ [-360, 0]. In pseudo-Javascript:
    + 	Angles are adjusted so that |θ₂ - θ₁| ∈ (-360, 0]. In pseudo-Javascript:

@svgeesus
Copy link
Contributor

it's no longer possible to specify a full circumference

Yes, intentional. We actually decided that a while ago (when we changed HSL to allow round-tripping of out of gamut colors) and I noticed that I had not updated the spec there.

Thanks! Will update the pseudo-codes asap.

@yarusome
Copy link
Author

yarusome commented Sep 6, 2023

@svgeesus Sorry to bother again 🙏, but is <hue> also supposed to be normalized into [0, 360) now? If that's the case, then two more places need edits:

<pre class='prod'>
<dfn>&lt;hue></dfn> = <<number>> | <<angle>>
</pre>
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].

<!-- to 14 Jan 2023 -->
<li>Define specified value for Lab, LCH, Oklab, Oklch</li>
<li>Define specified value for other sRGB colors</li>
<li>Define specified values for named and system colors</li>
<li>Clamp alpha, Lightness, Chroma and Hue at parsed-value time</li>
<li>Remove passing mention of specular white and CIE Lightness</li>
<li>No longer require as-specified Hue to be retained; clamp to [0, 360]</li>

@svgeesus
Copy link
Contributor

svgeesus commented Sep 6, 2023

Well spotted, agreed and fixed for the first one. Te second is in a change log so it stays (but there will be another entry in the changes, when I add recent changes, about this).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@LeaVerou @svgeesus @yarusome and others