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

Color mix tests added in https://github.com/web-platform-tests/wpt/pull/42940 may ignore alpha premultiplication #43180

Closed
mysteryDate opened this issue Nov 15, 2023 · 8 comments

Comments

@mysteryDate
Copy link
Contributor

mysteryDate commented Nov 15, 2023

Looking at the tests added here: https://github.com/web-platform-tests/wpt/pull/42940/files#diff-17d0bf90434793454df8bbbd34e081eef94d3697534f8ec2c433359700a6b985R368

Using this line as an example: color-mix(in lab, lab(10 20 30 / 25%) 0%, lab(none none none / 0.5))
with the expected value: lab(10 20 30 / 0.5)

And following https://csswg.sesse.net/css-color-4/#interpolation-alpha and https://csswg.sesse.net/css-color-5/#color-mix-with-alpha:

First we must premultiply the values of both colors, so the premultiplied colors are:
lab(2.5 5 7.5 / 0.25) and lab(none none none / 0.5)

The resulting, interpolated value, taking 0% of the left and 100% of the right, and respecting missing components:
lab(2.5 5 7.5 / 0.5)

Which, un-premultiplied with the interpolated alpha value is, dividing all components by 0.5:
lab(5 10 15 / 0.5)

Which is the current result value in chromium. I believe these tests here omit the fact that the alpha used for premultiplication and un-premultiplication are different.

chromium-wpt-export-bot pushed a commit that referenced this issue Nov 15, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2 Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in the appropriate unit for that component: 0, 0%, or 0deg. This includes rendering the color directly, converting it to another color space, performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
#42940
#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 15, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2 Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in the appropriate unit for that component: 0, 0%, or 0deg. This includes rendering the color directly, converting it to another color space, performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
#42940
#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 15, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2 Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in the appropriate unit for that component: 0, 0%, or 0deg. This includes rendering the color directly, converting it to another color space, performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
#42940
#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 16, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
#42940
#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 16, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
#42940
#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 16, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
#42940
#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyix@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225694}
aarongable pushed a commit to chromium/chromium that referenced this issue Nov 16, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyix@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225694}
chromium-wpt-export-bot pushed a commit that referenced this issue Nov 16, 2023
https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
#42940
#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyix@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225694}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 22, 2023
…pace conversion, a=testonly

Automatic update from web-platform-tests
Resolve missing components during colorspace conversion

https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyix@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225694}

--

wpt-commits: 6e624cc9cb5dcba7a7e940aa51ea025ebe24a7e7
wpt-pr: 43065
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 22, 2023
…pace conversion, a=testonly

Automatic update from web-platform-tests
Resolve missing components during colorspace conversion

https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyix@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225694}

--

wpt-commits: 6e624cc9cb5dcba7a7e940aa51ea025ebe24a7e7
wpt-pr: 43065
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this issue Nov 24, 2023
…pace conversion, a=testonly

Automatic update from web-platform-tests
Resolve missing components during colorspace conversion

https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyix@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225694}

--

wpt-commits: 6e624cc9cb5dcba7a7e940aa51ea025ebe24a7e7
wpt-pr: 43065
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this issue Nov 24, 2023
…pace conversion, a=testonly

Automatic update from web-platform-tests
Resolve missing components during colorspace conversion

https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyix@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225694}

--

wpt-commits: 6e624cc9cb5dcba7a7e940aa51ea025ebe24a7e7
wpt-pr: 43065
@foolip
Copy link
Member

foolip commented Nov 29, 2023

@mysteryDate would it be straightforward to update the test expectations here?

@mysteryDate
Copy link
Contributor Author

Yeah, it would be.

Looking into the original change here, I see the logic. Any number interpolated with "none" should result in no change, regardless of the alphas involved. I really don't see anywhere in the spec where this is mentioned though. I would not be surprised at all if the spec does change.

So, should I change the test expectations to match what the spec says, or should I change chromium's behavior to match what the other UAs are doing and what I suspect the spec will eventually say explicitly?

@foolip
Copy link
Member

foolip commented Nov 30, 2023

If you think the behavior makes sense, I'd suggest filing a spec issue requesting that the spec is updated to match, noting which browsers already have this behavior.

Where Firefox and Safari already have the same behavior and pass the test, I wouldn't wait until the spec is changed to align Chromium. That's just creating a risk that the interop issue will turn into a real site compat issue while we wait.

@svgeesus
Copy link
Contributor

Any number interpolated with "none" should result in no change, regardless of the alphas involved. I really don't see anywhere in the spec where this is mentioned though. I would not be surprised at all if the spec does change.

From 12.2. Interpolating with Missing Components:

If a color with a carried forward missing component is interpolated with another color which is not missing that component, the missing component is treated as having the other color’s component value.

and

If the carried forward missing component is alpha, the color must be premultiplied with this carried forward value, not with the zero value that would have resulted from color conversion.

and then from :

otherwise, each component which had been premultiplied is divided by the interpolated alpha value.

@mysteryDate You are suggesting that carried forward values should not get un-premultiplied? But did, earlier, get premultiplied?

gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Nov 30, 2023
…pace conversion, a=testonly

Automatic update from web-platform-tests
Resolve missing components during colorspace conversion

https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyixchromium.org>
Commit-Queue: Aaron Krajeski <aaronhkchromium.org>
Cr-Commit-Position: refs/heads/main{#1225694}

--

wpt-commits: 6e624cc9cb5dcba7a7e940aa51ea025ebe24a7e7
wpt-pr: 43065

UltraBlame original commit: 2b12ef1a74b23ed23a261ad7102f8fa5e96afa62
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Nov 30, 2023
…pace conversion, a=testonly

Automatic update from web-platform-tests
Resolve missing components during colorspace conversion

https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyixchromium.org>
Commit-Queue: Aaron Krajeski <aaronhkchromium.org>
Cr-Commit-Position: refs/heads/main{#1225694}

--

wpt-commits: 6e624cc9cb5dcba7a7e940aa51ea025ebe24a7e7
wpt-pr: 43065

UltraBlame original commit: 2b12ef1a74b23ed23a261ad7102f8fa5e96afa62
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Nov 30, 2023
…pace conversion, a=testonly

Automatic update from web-platform-tests
Resolve missing components during colorspace conversion

https://csswg.sesse.net/css-color-4/#missing:

"""
For handling of missing component in color interpolation, see § 12.2
Interpolating with Missing Components.

For all other purposes, a missing component behaves as a zero value, in
the appropriate unit for that component: 0, 0%, or 0deg. This includes
rendering the color directly, converting it to another color space,
performing computations on the color component values, etc.
"""

Remaining color mix test failures may be invalid:
web-platform-tests/wpt#42940
web-platform-tests/wpt#43180

Bug: 1495694, 1445171
Change-Id: I0ecd334b595c9c1ff4d3cb2f655176dd3b3eea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017402
Reviewed-by: Yi Xu <yiyixchromium.org>
Commit-Queue: Aaron Krajeski <aaronhkchromium.org>
Cr-Commit-Position: refs/heads/main{#1225694}

--

wpt-commits: 6e624cc9cb5dcba7a7e940aa51ea025ebe24a7e7
wpt-pr: 43065

UltraBlame original commit: 2b12ef1a74b23ed23a261ad7102f8fa5e96afa62
@mysteryDate
Copy link
Contributor Author

Ah, I misundersood

If a color with a carried forward missing component is interpolated with another color which is not missing that component, the missing component is treated as having the other color’s component value.

If you follow through my logic in #43180 (comment), I was making this subsitution after pre-multiplying (assuming none * x = none), so the result was different. Clearly the substitution should be made before premultiplying.

My misunderstanding:
color-mix(in lab, lab(none 0 0), lab(100 0 0 / 0.6)
Premultiply first: lab(none 0 0), lab(60 0 0 / 0.6)
Interpolating and substituting missing components: lab(60 0 0 / 0.8)
Unpremultiply: lab(62.5 0 0 / 0.8)

Subsituting before premultiplying:
color-mix(in lab, lab(none 0 0), lab(100 0 0 / 0.6)
Substution first: lab(100 0 0), lab(60 0 0 / 0.6)
Interpolating: lab(80 0 0 / 0.8)
Un-premultiplying: lab(100 0 0 / 0.8)

Perhaps it's worth noting in 12.2. Interpolating with Missing Components that pre-multiplication does not affect the result? i.e. if a component is missing in one color, but not missing in the other, the resulting value is equal to the non-missing value, regardless of alpha premultiplication. Or maybe give an example?

After a little bit of algebra it's now clear to me that lerp(x * a1, x * a2, t) / lerp(a1, a2, t) is always equal to x, but I certainly didn't see that at first!

@mysteryDate You are suggesting that carried forward values should not get un-premultiplied? But did, earlier, get premultiplied?

No, I just mis-understood the order. Apologies for my mistake. I'll fix Chromium.

@svgeesus
Copy link
Contributor

If you follow through my logic in #43180 (comment), I was making this subsitution after pre-multiplying (assuming none * x = none), so the result was different. Clearly the substitution should be made before premultiplying.

There is an explicit order of operations, in 12. Color Interpolation. Was it ambiguous or maybe you just missed it?

Perhaps it's worth noting in 12.2. Interpolating with Missing Components that pre-multiplication does not affect the result? i.e. if a component is missing in one color, but not missing in the other, the resulting value is equal to the non-missing value, regardless of alpha premultiplication. Or maybe give an example?

Always happy to point out consequences and add examples!

@svgeesus
Copy link
Contributor

svgeesus commented Jan 5, 2024

@mysteryDate

Clearly the substitution should be made before premultiplying.

Clarified in w3c/csswg-drafts@f4e79df

@mysteryDate
Copy link
Contributor Author

Great! Thank you, that clears up any nits I have.

@svgeesus svgeesus closed this as completed Apr 5, 2024
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