Closed Bug 1628988 Opened 3 months ago Closed 3 months ago

display: none mask causes continual painting in reftest


(Core :: SVG, defect, P3)




Tracking Status
firefox77 --- fixed


(Reporter: tnikkel, Assigned: tnikkel)




(3 files, 1 obsolete file)

No description provided.
Attached file mask-ref.html

If you run the attach testcase in the reftest harness (either gecko's or the wpt one) it will paint forever and never finish. This is because of the sync decode images flag that is used by both reftest harnesses.

Specifically, we always hit this case

because the maskframe is null (it's display none) and svgReset->mMask.mLayers[i].mImage will never resolve because it is a local ref:

So it seems the function is assuming that it needs to wait for an external resource to load so it resolves, but it already has everything it needs to draw the mask, it just needs to consider that case of a display none mask frame.

Attached patch very hacky patchSplinter Review

This very hacky patch fixes the issue, but I hope there is a better way to fix this.

This came up in landing a test for bug 1624532.

Actually, we should never be returning NOT_READY in this case, even if we are waiting for an external resource to load. ImgDrawResult are only about the result of drawing images. NOT_READY means that we tried to draw an image but there wasn't a decoded version around (even partially decoded), but if we had passed (or pass now or in the future) the sync decode flag to imagelib it could decode the image and give us the decoded image. If we are waiting for an external resource to load that is definitely not true, we can't sync decode anything because we are waiting for it to load. So we shouldn't be returning NOT_READY here.

Returning NOT_READY "works" because of this block

Attachment #9139715 - Attachment is patch: true
Attachment #9139715 - Attachment mime type: application/octet-stream → text/plain

The basic problem here for the page is that we should draw an svg element as if it has no mask specified if the specified mask is display: none. (For html elements in the same situation we should not draw the html element at all.)

The fix is to treat the return values of PaintMaskSurface (which come through nsSVGIntegrationUtils::PaintMask and nsDisplayMasksAndClipPaths::PaintMask) in WebRenderCommandBuilder::BuildWrMaskImage the same way as in CreateAndPaintMaskSurface.

Depends on D70595

Assignee: nobody → tnikkel

PaintMaskSurface shouldn't be applying ImgDrawResult::NOT_READY when we don't have a frame and the mask image hasn't been resolved. ImgDrawResult is only about drawing images, not about waiting for external resources to resolve or frames to get constructed. The only purpose of tracking ImgDrawResult's in painting code is to know which frames we need to invalidate because their rendering might change if we sync decode images during a Draw call. Applying NOT_READY here means we invalidate for every paint with the sync decode images flag (ie reftest paints), and it never changes from NOT_READY. This bites the reftest for this bug 1624532.

To fix it, instead of "overloading" the ImgDrawResult we return a bool to indicate the mask is missing or incomplete.

Comment on attachment 9139918 [details]
Bug 1628988. Handle incomplete masks on svg content properly with webrender. r?mstange!,jrmuizel!

Revision D70596 was moved to bug 1624532. Setting attachment 9139918 [details] to obsolete.

Attachment #9139918 - Attachment is obsolete: true
Priority: -- → P3
Pushed by
Don't apply ImgDrawResult::NOT_READY in PaintMaskSurface. r=mstange
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Blocks: 1624532
You need to log in before you can comment on or make changes to this bug.