1522422 - Opacity not obeyed on SVG USE elements when a Mask is utilized with Direct2D

Status: RESOLVED FIXED

(Core :: Graphics, defect, P2)

Description

[Jon]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce:

1. Create an SVG inline on a HTML page
2. Define a Mask
3. Create a USE element to use the Mask
4. Set opacity to (0.5) via Attribute

Example:
https://jsfiddle.net/bg4o5y63/

Actual results:

On Windows (Firefox 64.0.2) "Opacity" attribute seems to have no affect. "Fill-Opacity" Attribute does work. MacOS does not have the same problem.

Expected results:

Opacity attribute should be obeyed and shape should appear semi transparent.

Comment 1

[Jon]

From my testing (using old versions of Portable Firefox). The last released version that this worked correctly in was 54.0.1. Version 55.0 seems to show it incorrectly.

https://sourceforge.net/projects/portableapps/files/Mozilla%20Firefox%2C%20Portable%20Ed./

Hope that helps.

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6b534147049e9a4afaa3320712d824dc9563ac67&tochange=7fb451a4d22c87e7de74be4c169c85601bdb938e

Regressed by: Bug 1359527

Comment 3

[Jonathan Watt [:jwatt]]

(Which landed over a year and a half ago.)

Comment 4

[Neha Kochar [:neha]]

Sean, could you please find someone to look into this for our 67 soft freeze (Mar 11)?

Comment 5

[Sean Voisen (:svoisen)]

Looks like a long-standing regression based on comment 2 and comment 3.

Will keep an eye on it but believe this will need to be handled by someone on graphics given the regressing bug.

@jrmuizel: Does this seem correct?

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Yeah. It looks like this only happens with Direct2D.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

I saw the cause for this when reading the code a little while ago. It shouldn't be too hard to fix. We're basically assuming the opacity was applied in one case when it actually hasn't been applied

Comment 8

[Tim Spurway [:tspurway]]

Jeff, should we expect a patch for this one for 69?

Comment 9

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

Last poke for this bug as I drop it from weekly regression triage.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

I believe this is caused by DrawTargetD2D1::IntoLuminanceSource not handling the opacity argument.

https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/gfx/2d/DrawTargetD2D1.cpp#141

We can either bail on opacity != 1.0 or handle it properly.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Kris, can you take a look at this when you get a chance.

Comment 12

[Kris Taeleman (:ktaeleman)]

Attachment: Bug 1522422 - Fixing opacity issue on SVG USE elements in D2D.

Comment 13

[Kris Taeleman (:ktaeleman)]

Attachment: Bug 1522422 - Fixing opacity issue on SVG USE elements in D2D.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52cff0e39802
Fixing opacity issue on SVG USE elements in D2D. r=jrmuizel

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/52cff0e39802

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta or can it ride Fx71 to release?

Comment 17

[Kris Taeleman (:ktaeleman)]

I think this is a bit of an edge case (svg + use + mask), so in my opinion it could ride to Fx71.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Given the complaint filed on bug 1601898, we might as well ask to uplift this to esr.

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9089218 [details]
Bug 1522422 - Fixing opacity issue on SVG USE elements in D2D.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Certain web-content is broken without this fix. See bug 1601898.
• User impact if declined: Certain web-content is broken without this fix. See bug 1601898.
• Fix Landed on Version: 71
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is pretty low risk as it has shipped in 71 and 72 without issue. It is, however, a little involved and does have some additional uses of D2D so there is some risk.
• String or UUID changes made by this patch:

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9089218 [details]
Bug 1522422 - Fixing opacity issue on SVG USE elements in D2D.

This will need a rebased patch if we want it on ESR68.

Comment 21

[Jeff Muizelaar [:jrmuizel]]

Kris can you rebase on ESR?

Comment 22

[Kris Taeleman (:ktaeleman)]

Setting back to you Jeff as I won't have time to do this before thursday

Comment 23

[Jeff Muizelaar [:jrmuizel]]

Attachment: Patch for ESR68

This also pulls in the fix from bug 1556470

Comment 24

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9122444 [details] [diff] [review]
Patch for ESR68

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Certain web-content is broken without this fix. See bug 1601898.
• User impact if declined: Certain web-content is broken without this fix. See bug 1601898.
• Fix Landed on Version: 71
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is pretty low risk as it has shipped in 71 and 72 without issue. It is, however, a little involved and does have some additional uses of D2D so there is some risk.
• String or UUID changes made by this patch:

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9122444 [details] [diff] [review]
Patch for ESR68

Fixes a web compat issue being reported in the wild. Baked on release for awhile now without known issues. Approved for 68.5esr.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/2b5ba585e884