-
Notifications
You must be signed in to change notification settings - Fork 49
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-compositing-2] Remove or fix definition of plus-darker #447
Comments
@jakearchibald Would you consider advocating for Here's an example (taken directly from Apple's macOS Sketch Design Template) where (codepen) It also seems silly to only implement |
The first problem here is that nobody's published a formula for what Apple's For what it's worth, if I were to try to make up a Fa = 1; Fb = 1; I have not tested to see if this does anything close to reasonable. (On a related topic, the only difference between |
This test tests for the current spec, but it is also designed so that we can use it to make progress on changing the spec in w3c/fxtf-drafts#446 and w3c/fxtf-drafts#447 . Change-Id: If0f321a0fef7279fabfba9c613556371f42f9875 Bug: 1477259
I wrote a test in web-platform-tests/wpt#41713 that tests the composite modes in canvas. It turns out my test objected to the formula in #447 (comment) because it wasn't producing properly premultiplied colors: there were cases where the color result was greater than the alpha result, which isn't allowed. However, my formula was sort of close; it appears (based on fiddling with my test) that what Safari implements for co = min(1, αs + αb) - min(1, αs x (1 - Cs) + αb x (1 - Cb)); (It's possible there's a simpler way to write this, but that's the expression I figured out that leads to Safari passing the test for |
This test tests for the current spec, but it is also designed so that we can use it to make progress on changing the spec in w3c/fxtf-drafts#446 and w3c/fxtf-drafts#447 . Change-Id: If0f321a0fef7279fabfba9c613556371f42f9875 Bug: 1477259 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4827182 Auto-Submit: David Baron <dbaron@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1190349}
This test tests for the current spec, but it is also designed so that we can use it to make progress on changing the spec in w3c/fxtf-drafts#446 and w3c/fxtf-drafts#447 . Change-Id: If0f321a0fef7279fabfba9c613556371f42f9875 Bug: 1477259 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4827182 Auto-Submit: David Baron <dbaron@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1190349}
This test tests for the current spec, but it is also designed so that we can use it to make progress on changing the spec in w3c/fxtf-drafts#446 and w3c/fxtf-drafts#447 . Change-Id: If0f321a0fef7279fabfba9c613556371f42f9875 Bug: 1477259 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4827182 Auto-Submit: David Baron <dbaron@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1190349}
…s., a=testonly Automatic update from web-platform-tests Add canvas-based test for composite modes. This test tests for the current spec, but it is also designed so that we can use it to make progress on changing the spec in w3c/fxtf-drafts#446 and w3c/fxtf-drafts#447 . Change-Id: If0f321a0fef7279fabfba9c613556371f42f9875 Bug: 1477259 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4827182 Auto-Submit: David Baron <dbaron@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1190349} -- wpt-commits: 808ba394dd5f6a6904521f4a5f810fac8e70a8e3 wpt-pr: 41713
…s., a=testonly Automatic update from web-platform-tests Add canvas-based test for composite modes. This test tests for the current spec, but it is also designed so that we can use it to make progress on changing the spec in w3c/fxtf-drafts#446 and w3c/fxtf-drafts#447 . Change-Id: If0f321a0fef7279fabfba9c613556371f42f9875 Bug: 1477259 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4827182 Auto-Submit: David Baron <dbaron@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1190349} -- wpt-commits: 808ba394dd5f6a6904521f4a5f810fac8e70a8e3 wpt-pr: 41713
Comments in Apple source code say:
Where R=result, D=destination, S=source as premultiplied values. |
The CSS Working Group just discussed
The full IRC log of that discussion<bramus> astearns: there was a q here on what was actually implemented, and we got some feedback from impl. is that enough to know what to do?<bramus> vmpstr: generally we want to align with webkit here in the spec <bramus> vmpstr: and then implement from thereon <bramus> dbaron: yes, it seems like the results I got from testing with safari and what apple (simon) says agrees, so we should put that in the spec <bramus> … one side comment is that i am interest in feedback from color experts on how this is supposed to work outside of srgb <chris> q+ <bramus> … some formulas have max fns in them in interesting ways that assume the color space runs from 0 to 1 (I think) <bramus> … would be good to get that written down by experts <astearns> ack chris <bramus> … so this is a call for interested folks to look at it <bramus> chris: I can give a brief explanation <bramus> astearns: or we could resolve to define this for srgb and open issue for the rest (if that is needed) <bramus> chris: sgtm <bramus> … and that was a good question dbaron <bramus> dbaron: (agrees) <bramus> astearns: would the possible improvmenets need to be added to ??? <bramus> dbaron: not all, maybe 3 or 4 <bramus> astearns: PROPOSED RESOLUTION: define plus-darker based on research results in the issue and raise issue to deal with max/min in various cases <bramus> RESOLVED: define plus-darker based on research results in the issue and raise issue to deal with max/min in various cases |
This test tests for the current spec, but it is also designed so that we can use it to make progress on changing the spec in w3c/fxtf-drafts#446 and w3c/fxtf-drafts#447 . Change-Id: If0f321a0fef7279fabfba9c613556371f42f9875 Bug: 1477259 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4827182 Auto-Submit: David Baron <dbaron@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1190349}
The logic in here is hard to follow.
That's correct, except I think this is a typo when it was integrated into AppKit. The original blend mode can be found in CoreGraphics with an important twist:
I don't have an easy system to test CoreCG with, so I'm going to assume this is the actual formula. There are some other formulas that you could come up with, like
This is clearly incorrect, as should be evidenced by the resulting formula being effectively the same as plus-lighter (additive blending). You can't swap out the 1 for both "Da+Sa" and "Da"/"Sa" without a clamp. |
https://drafts.fxtf.org/compositing/#porterduffcompositingoperators_plus_darker
The definition here seems plain wrong. For instance:
max(0, x)
is used, which suggestsx
will be a maximum of 1, but the definition produces values 0-2 (assuming pixel values are 0-1).Safari supports the
plus-darker
value formix-blend-mode
.The definition in Apple's docs matches the spec, so it's incorrect for the same reasons. The implementation is closed source, so I can't check how it actually works.
It seems like, on WebKit Linux (via cairo),
plus-darker
is an alias ofdarken
, which doesn't match what I see in Safari. I might not be interpreting the source correctly.We have not heard any use-cases for plus-darker, although maybe that will become clearer with correct documentation.
The text was updated successfully, but these errors were encountered: