The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Firefox 51

Status

()

Core
Layout
RESOLVED FIXED
9 months ago
3 months ago

People

(Reporter: shorlander, Assigned: dvander)

Tracking

({regression})

46 Branch
mozilla51
regression
Points:
---

Firefox Tracking Flags

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

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

9 months ago
Created attachment 8764369 [details]
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.
Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ffac58123eb089dbc94bed648df72136b1b4dc6d&tochange=77ce3012d481b480f87ff2a0063e1b11d9999f68
Flags: needinfo?(dvander)
Keywords: regression
Blocks: 1235995
status-firefox47: --- → wontfix
status-firefox48: --- → affected
status-firefox49: --- → affected
status-firefox50: --- → affected
Version: unspecified → 46 Branch

Comment 2

8 months ago
Created attachment 8772360 [details] [diff] [review]
Force premultiplied source color to black when alpha is 0
Attachment #8772360 - Flags: review?(dvander)

Comment 3

8 months ago
Created attachment 8773572 [details] [diff] [review]
Force premultiplied source color to transparent when alpha is 0.

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)
(Assignee)

Comment 4

8 months ago
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)

Comment 5

8 months ago
(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.)

Updated

8 months ago
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)
(Assignee)

Comment 7

8 months ago
Created attachment 8775333 [details] [diff] [review]
part 1, d3d11 fix

Yeah good point, I think we can just return 0.
Flags: needinfo?(dvander)
Attachment #8775333 - Flags: review?(mstange)
(Assignee)

Comment 8

8 months ago
Created attachment 8775334 [details] [diff] [review]
reftest
Attachment #8775334 - Flags: review?(mstange)
Attachment #8775333 - Flags: review?(mstange) → review+
Attachment #8775334 - Flags: review?(mstange) → review+
(Assignee)

Comment 9

8 months ago
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
(Assignee)

Comment 10

8 months ago
Created attachment 8775424 [details] [diff] [review]
opengl fix
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+
status-firefox48: affected → wontfix

Comment 12

8 months 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

8 months 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: 8 months ago
status-firefox51: --- → fixed
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.
status-firefox49: affected → fix-optional
status-firefox50: affected → fix-optional
Flags: needinfo?(dvander)
(Assignee)

Comment 15

8 months ago
I think this is okay to not uplift for now, it's pretty obscure.
Flags: needinfo?(dvander)
status-firefox49: fix-optional → wontfix
status-firefox50: fix-optional → wontfix
You need to log in before you can comment on or make changes to this bug.