1670793 - sw-wr: animated GIF has white rectangles that flash or aren't painted

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Chris Peterson [:cpeterson]]

Attachment: Screenshot_of_animated_GIF_with SW-WR.png

Steps to reproduce

1. Enable gfx.webrender.software = true and restart Nightly.
2. Open this animated GIF attached in bug 1670263: https://bug1670263.bmoattachments.org/attachment.cgi?id=9180702
3. Watch the animation.

Expected result

The animation should paint all the pixels.

Actual result

The animation has some white rectangles that flash or don't get painted. See the attached screenshot.

I am running Nightly 83.0a1 (2020-10-12) on Windows 10.

Comment 1

[Chris Peterson [:cpeterson]]

This bug is probably a regression from Composite crash bug 1666119 or gGetGraphicsResetStatus crash bug 1667919. I bisected this bug to this pushlog:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=7e9c740d3bd716cac3a58356ff7e2ca93a3fbf18&tochange=661294ad73578c643850c6122698c2b5d04e097f

Comment 2

[Darkspirit]

After maximizing Nightly (2560x1440 display) and zooming in to 270% I was able to repro this on Linux.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Are you sure you're getting software webrender before that regression range?

I can still repro with those patches backed out, so it's possible that you're just crashing the GPU process before that and not seeing the bug :)

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)

Are you sure you're getting software webrender before that regression range?

I can still repro with those patches backed out, so it's possible that you're just crashing the GPU process before that and not seeing the bug :)

Looks like you're correct. I see in the good builds' about:support "Compositing" is "Basic", not "WebRender (Software)". Since this bug is not necessarily a regression, I will unlink those "Regressed by" bugs.

Comment 5

[Darkspirit]

Gnome X11, Mesa/Nouveau, Debian Testing
Maximize, zoom in.
mozregression --good 2020-04-15 --bad 2020-08-15 --pref gfx.webrender.all:true gfx.webrender.software:true -a https://bug1670263.bmoattachments.org/attachment.cgi?id=9180702

12:23.75 INFO: Last good revision: a0ed52a4de5ece14a3032d2a0af69576acda7cdd
12:23.75 INFO: First bad revision: 46acaaf0f72398fc584ee81ff90a8d39b525bdfa
12:23.75 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a0ed52a4de5ece14a3032d2a0af69576acda7cdd&tochange=46acaaf0f72398fc584ee81ff90a8d39b525bdfa

fdda2e1419dd99d75d961e1a9c5d40f0507adf20 Lee Salzman — Bug 1652894 - add SWGL RenderTextureHosts. r=mattwoodrow
08a8b2422765ec7d46577a76cb873f3f738fb421 Lee Salzman — Bug 1652894 - fix SWGL code formatting. r=mattwoodrow
108cb2f4c72cef66c928b2e31a5e2eede6dd7fee Lee Salzman — Bug 1652894 - add SWGL utility bindings for managing textures. r=jimb
422ffc69f0c90208471fca31137319ec70082162 Lee Salzman — Bug 1652894 - allow SWGL contexts to have multiple strong references. r=jimb

Comment 6

[Matt Woodrow (:mattwoodrow)]

This breaks because WebRenderBridgeParent::UpdateResources assumes that we always copy shared external textures into the texture cache, and notifies the content process that it can recycle the associated memory - https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/gfx/layers/wr/WebRenderBridgeParent.cpp#642

With SWGL however, we render directly from shared surfaces - https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/gfx/layers/wr/WebRenderBridgeParent.cpp#708

Writing to the shared memory for a different animation frame while SWGL is reading from it to composite causes the flashing.

We need to delay notifying that the textures are free until we no longer have a display list referencing them, or go back to copying these surfaces into the texture cache. The former seems like it'd perform better, but maybe it's not a big deal.

Sotaro, do you have any ideas how we could delay freeing these surfaces until we will no longer be drawing from them?

Comment 7

[Sotaro Ikeda [:sotaro]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #6)

Sotaro, do you have any ideas how we could delay freeing these surfaces until we will no longer be drawing from them?

I wonder if ScheduleSharedSurfaceRelease caused the problem. It triggers WebRenderBridgeParent::ObserveSharedSurfaceRelease() at wr::Checkpoint::FrameTexturesUpdated. And it releases SharedSurfaces.
https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/gfx/layers/wr/WebRenderBridgeParent.cpp#814
https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/gfx/layers/wr/WebRenderBridgeParent.cpp#642
https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/gfx/layers/wr/WebRenderBridgeParent.cpp#832

With sw-wr, it seems better to use AsyncImagePipelineManager::HoldExternalImage() as it is called from WebRenderBridgeParent::ReleaseTextureOfImage(). SharedSurface is actually released by ForwardingExternalImage::~ForwardingExternalImage()
Though, name of HoldExternalImage() is confusing now. It hold the external image until a specified epoch and release it when epoch is exceeded.
https://searchfox.org/mozilla-central/source/gfx/layers/wr/WebRenderBridgeParent.cpp#1709
https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/gfx/layers/wr/AsyncImagePipelineManager.cpp#30

Comment 8

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1670793 - Hold animated shared surface until the end of the composite when using software GL. r?sotaro

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6d117305fe6
Hold animated shared surface until the end of the composite when using software GL. r=sotaro

Comment 10

[Andreea Pavel [:apavel]]

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

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Could this have caused bug 1673906?