1281593 - Nested elements with mix-blend-mode and background-image have strange artifact when transformed

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Stephen Horlander [:shorlander]]

Attachment: mix-blend-mode + background-image + transform screenshot

I ran into a rendering glitch when I tried to nest an element with mix-blend-mode inside an object with mix-blend-mode and then apply a transform.

Attaching a screenshot.

Top — no transform
Bottom — transform: rotate(45deg);

The bottom image has a grey box that should not be there. 

Test case: http://people.mozilla.org/~shorlander/bugs/transform-blend-mode-glitch.html

Only tested this on OS X, not sure what it does on other platforms.

Comment 1

[Timothy Nikkel (:tnikkel)]

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ffac58123eb089dbc94bed648df72136b1b4dc6d&tochange=77ce3012d481b480f87ff2a0063e1b11d9999f68

Comment 2

[Jet Villegas (inactive)]

Attachment: Force premultiplied source color to black when alpha is 0

Comment 3

[Jet Villegas (inactive)]

Attachment: Force premultiplied source color to transparent when alpha is 0.

Markus: moving r? to you since you reviewed this shader before. Thx!

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Sorry, was waiting to test this locally - the fix looks good, but it needs a D3D11 shader fix as well. A reftest would almost certainly pass on Mac and fail on Windows.

Comment 5

[Jet Villegas (inactive)]

(In reply to David Anderson [:dvander] from comment #4)
> Sorry, was waiting to test this locally - the fix looks good, but it needs a
> D3D11 shader fix as well. A reftest would almost certainly pass on Mac and
> fail on Windows.

No worries. I had to re-work the patch anyway as the comments no longer matched the code. Are you able to port it to D3D? (I'm traveling and don't have a Windows machine.)

Comment 6

[Markus Stange [:mstange]]

Comment on attachment 8773572 [details] [diff] [review]
Force premultiplied source color to transparent when alpha is 0.

Review of attachment 8773572 [details] [diff] [review]:
-----------------------------------------------------------------

A description of the bug would have helped me understand this patch much more quickly :) For future reference, the problem is that the "if (color.a == 0.0) { return backdrop; }" optimization is just wrong - if color.a is 0.0, then what we return must also have .a == 0.0, and that's not necessarily true for the backdrop.

Actually, writing this down cleared things up for me. Couldn't we just change the early return to return vec4(0.0, 0.0, 0.0, 0.0) instead?

By the way, two of the comments have whitespace at the end of the line.

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 1, d3d11 fix

Yeah good point, I think we can just return 0.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: reftest

Comment 9

[David Anderson [:dvander] - inactive, e-mail if emergency]

Jet, I'll just steal this unless you very much want to own a mix-blend bug :) thanks for diagnosing the problem.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: opengl fix

Comment 11

[Markus Stange [:mstange]]

Comment on attachment 8775424 [details] [diff] [review]
opengl fix

Review of attachment 8775424 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/opengl/OGLShaderProgram.cpp
@@ +736,5 @@
>    fs << "  if (backdrop.a == 0.0) {" << endl;
>    fs << "    return color;" << endl;
>    fs << "  }" << endl;
>    fs << "  if (color.a == 0.0) {" << endl;
> +  fs << "    return vec4(0.0, 0.0, 0.0, 0.0);;" << endl;

double semicolon

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d1cec49be7a
Fix D3D11 mix-blending when the source alpha is 0. (bug 1281593 part 1, r=mstange)
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a8ee572051d
Fix OpenGL mix-blending when the source alpha is 0. (bug 1281593 part 2, r=mstange)
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e63d98406fe
Add a reftest for bug 1281593. r=mstange

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7d1cec49be7a
https://hg.mozilla.org/mozilla-central/rev/8a8ee572051d
https://hg.mozilla.org/mozilla-central/rev/0e63d98406fe

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Are you ok with the fix staying in 51, or do you want to try uplift? Unless you think this is a common problem for users or has a big impact I think it's likely ok to stay in 51.

Comment 15

[David Anderson [:dvander] - inactive, e-mail if emergency]

I think this is okay to not uplift for now, it's pretty obscure.