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

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: shorlander, Assigned: dvander)

Tracking

({regression})

46 Branch
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 wontfix, firefox48 wontfix, firefox49 wontfix, firefox50 wontfix, firefox51 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

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.
Version: unspecified → 46 Branch
Markus: moving r? to you since you reviewed this shader before. Thx!
Attachment #8772360 - Attachment is obsolete: true
Attachment #8772360 - Flags: review?(dvander)
Attachment #8773572 - Flags: review?(mstange)
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.
Flags: needinfo?(dvander)
(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.)
Flags: needinfo?(dvander)
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.
Attachment #8773572 - Flags: review?(mstange)
Yeah good point, I think we can just return 0.
Flags: needinfo?(dvander)
Attachment #8775333 - Flags: review?(mstange)
Attachment #8775333 - Flags: review?(mstange) → review+
Attachment #8775334 - Flags: review?(mstange) → review+
Jet, I'll just steal this unless you very much want to own a mix-blend bug :) thanks for diagnosing the problem.
Assignee: nobody → dvander
Status: NEW → ASSIGNED
Posted patch opengl fixSplinter Review
Attachment #8773572 - Attachment is obsolete: true
Attachment #8775424 - Flags: review?(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
Attachment #8775424 - Flags: review?(mstange) → review+

Comment 12

3 years ago
Pushed by danderson@mozilla.com:
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

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7d1cec49be7a
https://hg.mozilla.org/mozilla-central/rev/8a8ee572051d
https://hg.mozilla.org/mozilla-central/rev/0e63d98406fe
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
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.
I think this is okay to not uplift for now, it's pretty obscure.
Flags: needinfo?(dvander)
You need to log in before you can comment on or make changes to this bug.