1673906 - sw-wr: Animated images are not released

Status: VERIFIED FIXED

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

Description

[Jeff Muizelaar [:jrmuizel]]

This shows up very noticeably on https://w3c.github.io/editing/docs/EditContext/explainer.html#example-text-input-methods

5,754,689,664 B (100.0%) -- gfx
└──5,754,689,664 B (100.0%) -- webrender
   ├──5,611,941,888 B (97.52%) -- images/mapped_from_owner
   │  ├──5,606,547,456 B (97.43%) -- pid=892630
892630 is the child process
5,606,547,456 B (97.43%) -- pid=892630
   │  │  ├──2,489,114,624 B (43.25%) ── image(1000x1000, compositor_ref:2, creator_ref:1)/decoded-nonheap [622]
   │  │  ├──1,928,060,928 B (33.50%) ── image(1292x600, compositor_ref:2, creator_ref:1)/decoded-nonheap [621]
   │  │  ├────640,286,720 B (11.13%) ── image(1000x1000, compositor_ref:1, creator_ref:1)/decoded-nonheap [160]
   │  │  ├────502,972,416 B (08.74%) ── image(1292x600, compositor_ref:1, creator_ref:1)/decoded-nonheap [162]
   │  │  ├─────46,104,576 B (00.80%) ── image(800x600, compositor_ref:1, creator_ref:1)/decoded-nonheap [24]
   │  │  └──────────8,192 B (00.00%) ── image(16x16, compositor_ref:1, creator_ref:1)/decoded-nonheap [2]

These are supposed to be released by this code:
https://searchfox.org/mozilla-central/source/gfx/layers/wr/WebRenderBridgeParent.cpp#642

Comment 1

[Jeff Muizelaar [:jrmuizel]]

This could've been regressed by bug 1670793

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Assuming it is bug 1670793, as an alternative, since SWGL probably doesn't have FrameTexturesUpdated, we might be able to use FrameRendered instead with SWGL instead of trying to use HoldExternalImage. We prefer FrameTexturesUpdated to have the event come in as soon as possible and free up the buffer for recycling, but as soon as possible is later in the pipeline with SWGL. Alternatively issue FrameTexturesUpdated at the same time as FrameRendered for SWGL to hide that detail from animated images. The SharedSurfacesParent::Release is insufficient for the animated image case, we need WebRenderBridgeParent::SendWrReleasedImages to be called:

https://searchfox.org/mozilla-central/source/gfx/layers/wr/WebRenderBridgeParent.cpp#839

Comment 3

[Matt Woodrow (:mattwoodrow)]

(In reply to Andrew Osmond [:aosmond] from comment #2)

Assuming it is bug 1670793, as an alternative, since SWGL probably doesn't have FrameTexturesUpdated, we might be able to use FrameRendered instead with SWGL instead of trying to use HoldExternalImage. We prefer FrameTexturesUpdated to have the event come in as soon as possible and free up the buffer for recycling, but as soon as possible is later in the pipeline with SWGL. Alternatively issue FrameTexturesUpdated at the same time as FrameRendered for SWGL to hide that detail from animated images. The SharedSurfacesParent::Release is insufficient for the animated image case, we need WebRenderBridgeParent::SendWrReleasedImages to be called:

https://searchfox.org/mozilla-central/source/gfx/layers/wr/WebRenderBridgeParent.cpp#839

It looks like there are two bugs.

One is that RecvUpdateResources doesn't increment the WR epoch, so we held the image until we got a new epoch for some other reason.

The other is as you mentioned, SendWrReleasedImages not being called. It looks like other cases could result in that happening as well, does that ever matter? I have an alternate patch which ensure we always call it if possible.

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1673906 - Generate a WR epoch for RecvUpdateResources. r?sotaro

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1673906 - Use FrameRendered notifications for releasing animated shared surfaces when using SWGL. r?aosmond

Depends on D95103

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Yes, we need the epoch to be updated as well.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddbb285e1f34
Generate a WR epoch for RecvUpdateResources. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/f132fcd3a499
Use FrameRendered notifications for releasing animated shared surfaces when using SWGL. r=aosmond

Comment 8

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/ddbb285e1f34
https://hg.mozilla.org/mozilla-central/rev/f132fcd3a499

Comment 9

[Rares Doghi, Desktop QA]

Hi Matt, does this issue occur on specific builds ? I tried reproducing this issue on a windows 10 machine with an Nvidia GPU 730 using the attachment from the other Bug 1670793 : https://bug1670263.bmoattachments.org/attachment.cgi?id=9180702 but I cant seem to reproduce this issue at all, even after setting gfx.webrender.software = true, are we missing something ? is there a way to Verify the fix for this issue ?

Comment 10

[Matt Woodrow (:mattwoodrow)]

I used the link from comment 0 and it reproduced reliably.

Note that switching tab will cause the memory to be released, so if you want to check about:memory you need to do so in a separate firefox window. I could also see the growing memory in the windows task manager.

Comment 11

[Rares Doghi, Desktop QA]

Hi Matt, I was a little confused what the issue was but after comparing the two builds, the one from October 27 and the one from today I noticed that the "(97.52%) -- images/mapped_from_owner" and "(97.43%) -- pid=892630" (child process) will not go higher than 55% as for the old build it went up to 97%, like in the Comment 0. Based on this we can mark this issue as Verified ?

Comment 12

[Matt Woodrow (:mattwoodrow)]

Yes we can! That's the expected difference.

Comment 13

[Rares Doghi, Desktop QA]

Based on Comment 12 I will mark this issue as Verified.