1275478 - SVG mask composition failure on Windows

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u459114]

After patches in Bug 1228280 landed, now we support multiple SVG masks.
Thing works fine on all platforms, except window

At [1], 
  maskDT->DrawSurface(svgMask, drawRect, drawRect);

svgMask is a Cairo A8 surface created in nsSVGMaskFrame::GetMaskForMaskedFrame. 
maskDT is an instance of DrawTargetD2D1 with A8 surface format.
Drawing a Cairo A8 surface onto a DrawTargetD2D1 takes no effect. 

PS:
Replacing maskDT by DrawTargetCario or DrawTargetSkia, everything works fine. That's why the composition result on Mac/ Linux is correct.

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/27914ad6e245f9d4485b1e23842039274de2290d#l1.119

Comment 1

[Robert Longson [:longsonr]]

sounds a bit similar to bug 951268

Comment 2

[Ethan Lin[:ethlin]]

Attachment: use skia drawtarget

Markus, this bug makes SVG mask-image almost cannot work on windows. I am still investigating. We want to have a trial run on FF49, so the patch is a workaround to use skia drawtarget to avoid the d2d problem. We can keep this bug opened to find the real problem later.

Comment 3

[Markus Stange [:mstange]]

(In reply to Robert Longson from comment #1)
> sounds a bit similar to bug 951268

Indeed it does.
There's one difference: In that bug, the destination DrawTarget usually had an RGBA format. Here, the destination is also A8.

Bas, is DrawSurface of an A8 surface onto an A8 DT a supported operation?

Comment 4

[Markus Stange [:mstange]]

Comment on attachment 8757835 [details] [diff] [review]
use skia drawtarget

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

Seems fine as a short-term workaround.

Comment 5

[Bas Schouten (:bas.schouten)]

(In reply to Markus Stange [:mstange] from comment #3)
> (In reply to Robert Longson from comment #1)
> > sounds a bit similar to bug 951268
> 
> Indeed it does.
> There's one difference: In that bug, the destination DrawTarget usually had
> an RGBA format. Here, the destination is also A8.
> 
> Bas, is DrawSurface of an A8 surface onto an A8 DT a supported operation?

Intuitively I'd say no. CopySurface should work but I not sure how DrawSurface would be defined. I can think of two ways I guess. We could make it work in a certain way if we spec it for Moz2D

Comment 6

[Ethan Lin[:ethlin]]

Attachment: use skia drawtarget (carry r+: mstange)

I should use skia when the backend is DIRECT2D1_1 but not DIRECT2D.

Comment 7

[u459114]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d813f7d5f90cba36d4401e970f4b559cb3153f72
Bug 1275478 - Workaround for svg mask on windows. r=mstange

Comment 8

[Jeff Muizelaar [:jrmuizel]]

It probably wouldn't hurt to add assertions that trigger in this situations to the backends so others don't have to discover the behaviour difference themselves.

Comment 9

[u459114]

Leave-open since we still need figure out why DIRECT2D1_1 failed. And since we already phase-in short term solution, remove this bug from the block-list of Bug 1251161

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d813f7d5f90c

Comment 11

[Ethan Lin[:ethlin]]

Attachment: use masksurface to replace drawsurface

I use MaskSurface to fix this issue like the patch in bug 951268. Not sure if this way is correct.

Comment 12

[Ethan Lin[:ethlin]]

Attachment: fix svg maskimage with d2d1

I use MaskSurface to fix the problem.

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Is there an automated test that detects this failure when you enable the new masking support?  If not, there should be.

Comment 14

[u459114]

(In reply to David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) from comment #13)
> Is there an automated test that detects this failure when you enable the new
> masking support?  If not, there should be.
After enable MASK_AS_SHORTHAND, mask-image-3x reftests failed on win7 platform, so I filed this issue. The answer is yes

Comment 15

[Ethan Lin[:ethlin]]

try server:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5647c9a4cb7&selectedJob=25710080

Comment 16

[Ethan Lin[:ethlin]]

Attachment: fix svg maskimage with d2d1 (carry r+: mstange)

Fix try failure.

Comment 17

[Ethan Lin[:ethlin]]

try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce2a6256b9bb

Comment 18

[Ethan Lin[:ethlin]]

Please push attachment 8789299 [details] [diff] [review] to m-c.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/540bd4b5aa3d
Fix svg mask with D2D1 backend. r=mstange

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/540bd4b5aa3d