Fix the brightness and invert filters to work with premultiplied alpha. #1722
Conversation
|
Servo/wrench tests pass OK. |
|
@nical Happy to review this one too? :) |
|
@glennw I think i am, but the gecko reftests don't look too happy. Might it be because we are making wrong assumptions in Gecko about pre-multiplication when using the bightness filter? |
glennw
force-pushed
the
filter-brightness
branch
from
5ea8b43
to
b2f8f0c
Compare
|
@nical Ah, I hadn't noticed the try failures. Pushed an updated version, and added several new reftests to cover the cases that Gecko tests for. Try run (successful this time) here: https://hg.mozilla.org/try/rev/bf466f065c0db5379a707e459ca184357f792544 |
webrender/res/ps_blend.glsl
Outdated
| vec4 color = mix(Cs, vec4(Cs.a) - vec4(Cs.rgb, 0.0), amount); | ||
|
|
||
| // Pre-multiply the alpha into the output value. | ||
| return vec4(color.rgb * clamp(color.a, 0.0, 1.0), color.a); |
There was a problem hiding this comment.
color is already premultiplied; here you're multiplying it a second time. Your yaml reftest doesn't catch it because it uses a black result, and multiplying black one more time still stays black.
There was a problem hiding this comment.
Ah, I guess we don't have an (enabled) gecko reftest that catches that either. Will add a specific reftest for this to wrench.
glennw
force-pushed
the
filter-brightness
branch
from
b2f8f0c
to
3a09624
Compare
|
@mstange Switched the invert to do a normal un/pre-multiply and added a reftest with alpha. Does that look right to you? |
|
Yes, looks good now. It took me a second to understand the test; I assume you're putting transparent blue on top of opaque blue in order to avoid the need to fuzz? |
|
@bors-servo r+ |
|
📌 Commit 3a09624 has been approved by |
Fix the brightness and invert filters to work with premultiplied alpha. Fixes #1507. Fixes #1508. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1722) <!-- Reviewable:end -->
|
☀️ Test successful - status-travis |
Fixes #1507.
Fixes #1508.
This change is