1628988 - display: none mask causes continual painting in reftest

Status: RESOLVED FIXED

(Core :: SVG, defect, P3)

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: mask-ref.html

Comment 2

[Timothy Nikkel (:tnikkel)]

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.

Comment 3

[Timothy Nikkel (:tnikkel)]

Specifically, we always hit this case

https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/layout/svg/nsSVGIntegrationUtils.cpp#509

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

https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/layout/style/nsStyleStruct.cpp#1001

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.

Comment 4

[Timothy Nikkel (:tnikkel)]

Attachment: very hacky patch

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

Comment 5

[Timothy Nikkel (:tnikkel)]

This came up in landing a test for bug 1624532.

Comment 6

[Timothy Nikkel (:tnikkel)]

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

https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/layout/svg/nsSVGIntegrationUtils.cpp#579

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1628988. Handle incomplete masks on svg content properly with webrender. r?mstange!,jrmuizel!

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

Comment 8

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1628988. Don't apply ImgDrawResult::NOT_READY in PaintMaskSurface. r?mstange

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 9

[Phabricator Automation]

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.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37cd04f0b502
Don't apply ImgDrawResult::NOT_READY in PaintMaskSurface. r=mstange

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/37cd04f0b502