Closed Bug 1275478 Opened 4 years ago Closed 3 years ago

SVG mask composition failure on Windows

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox51 --- fixed

People

(Reporter: u459114, Assigned: ethlin)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

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
Assignee: nobody → ethlin
sounds a bit similar to bug 951268
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla49
Attached patch use skia drawtarget (obsolete) — Splinter Review
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.
Attachment #8757835 - Flags: review?(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?
Flags: needinfo?(bas)
Comment on attachment 8757835 [details] [diff] [review]
use skia drawtarget

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

Seems fine as a short-term workaround.
Attachment #8757835 - Flags: review?(mstange) → review+
(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
Flags: needinfo?(bas)
I should use skia when the backend is DIRECT2D1_1 but not DIRECT2D.
Attachment #8757835 - Attachment is obsolete: true
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.
Keywords: leave-open
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
No longer blocks: mask-ship
Blocks: mask-image
I use MaskSurface to fix this issue like the patch in bug 951268. Not sure if this way is correct.
Attachment #8759496 - Flags: feedback?(mstange)
Attachment #8759496 - Flags: feedback?(mstange) → feedback+
Attached patch fix svg maskimage with d2d1 (obsolete) — Splinter Review
I use MaskSurface to fix the problem.
Attachment #8759496 - Attachment is obsolete: true
Attachment #8763066 - Flags: review?(mstange)
Attachment #8763066 - Flags: review?(mstange) → review+
Is there an automated test that detects this failure when you enable the new masking support?  If not, there should be.
Flags: needinfo?(cku)
(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
Flags: needinfo?(cku)
Depends on: 1285857
Summary: SVG mask composition failure on windows → SVG mask composition failure on Windows
Fix try failure.
Attachment #8763066 - Attachment is obsolete: true
Please push attachment 8789299 [details] [diff] [review] to m-c.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/540bd4b5aa3d
Fix svg mask with D2D1 backend. r=mstange
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/540bd4b5aa3d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.