1428558 - Support async animated image updates

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P2)

Description

[Jeff Muizelaar [:jrmuizel]]

These should work really well with webrender and will avoid us having to do any display list building or scene building.

Comment 1

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

I've already been doing the prep work for this.

Comment 2

[Maire Reavy [:mreavy]]

Hey Andrew & Jeff -- What more is required here once bug 1337111 lands?  How important is this to WR MVP?

Comment 3

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

I think it is reasonably important. Not only will it avoid scene rebuilds for animated images, it will reduce texture uploads as well for animated images that only animate in parts. Both of these will be commonly hit, although the latter really only matters for larger GIFs like those seen in bug 1447351. I'll see if I can put this together on top of bug 1337111 this week, assuming nothing else preempts me. If it takes me much longer than that, we can consider pushing it to block bug 1386674. It definitely shouldn't go to the back of the queue though :).

Comment 4

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

Update: I got this partly working today. Still some bugs to iron out / understand.

Comment 5

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

Attachment: Bug 1428558 - Part 2. Improve plumbing to sending resource updates to WebRender.

Animated images will require scheduling a composite of the frame in
addition to updating the ImageKey/external image ID bindings. It would
be good if this could be done as part of the same IPDL message.
Additionally a page may have many animated images that we update the
frame for at the same time, so these updates should be batched together.
In the event that we needed to regenerate the display list, or produce
an empty transaction, ideally we would just throw these resource updates
in with the rest of the changes. This patch allows us to do all of that
without unnecessarily burdening the caller with tracking extra state.

Comment 6

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

Attachment: Bug 1428558 - Part 3. Release our reference to an external image ID as soon as possible

Animated images will work by changing the external image ID that an
ImageKey points to. We cannot allow the old external image to be
released and potentially unmapped until we have produced a new frame
with the new external image ID. We currently wait until the epoch has
advanced, but in the future when we don't rebuild the scene to animate
an image, the epoch will remain the same. This could cause us to hold
onto no longer used surfaces for much longer than expected. As such, in
this patch we switch to waiting for a FrameRendered notification from
WebRender, which works even if the scene rebuild was avoided.

Depends on D7499

Comment 7

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

Attachment: Bug 1428558 - Part 4. Move ImageKeyData/SharedUserData definition to SharedSurfacesChild header

This is a non-functional change. It allows objects that build on top of
these helper classes to be exposed outside of SharedSurfacesChild in
future patches in this series.

Depends on D7500

Comment 8

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

Attachment: Bug 1428558 - Part 5. Add SharedSurfacesAnimation to manage single ImageKey for animated images

Depends on D7501

Comment 9

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

Attachment: Bug 1428558 - Part 6. Integrate SharedSurfacesAnimation with ImageContainer

This patch embeds a SharedSurfacesAnimation object inside an
ImageContainer. This allows any consumers of the container to get the
single shared ImageKey for an animation, despite whatever surfaces may
be held inside the container at a given time.

Depends on D7502

Comment 10

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

Attachment: Bug 1428558 - Part 7. Suppress display list regeneration for animated image updates

This patch allows us to intercept invalidation requests for display
items, and avoid regenerating the display list for animated images which
are using SharedSurfacesAnimation.

Depends on D7503

Comment 11

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

Attachment: Bug 1428558 - Part 8. Integrate SharedSurfacesAnimation with the rest of imagelib

This patch makes ImageContainer create a SharedSurfacesAnimation object
when it detects that we are using shared surfaces and are producing full
frames.

Depends on D7504

Comment 12

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

Attachment: Bug 1428558 - Part 1. Streamline mappings between an ImageKey and an ExternalImageId for shared surfaces

Async animated images need a single ImageKey which can point to any
frame represented by its own external image ID. Additionally a frame
could be referenced again directly (e.g. something shows/uses the first
frame of the animated image).

Before this patch, the ownership between an ImageKey and an external
image ID for a shared surface was not clearly expressed. This resulted
in a special command to release the reference to the external image
separately from deleting the image key.

This patch makes the strong reference to an external image ID and an
ImageKey directly related. Not only does this facilitate multiple
ImageKeys owning the same surface, it also simplifies the ownership
semantics.

Comment 13

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

Sanity check before landing...

try (linux, all -- I've touched a lot): https://treeherder.mozilla.org/#/jobs?repo=try&revision=425da9df6049da61110b18008ad63e099f0f211d
try (windows, osx WR): https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9d1db8624cd48e95a3a86f8ed6bcb22a373659b

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48152d6288e3
Part 1. Streamline mappings between an ImageKey and an ExternalImageId for shared surfaces. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a3433fb1c29
Part 2. Improve plumbing to sending resource updates to WebRender. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/92fde8cff068
Part 3. Release our reference to an external image ID as soon as possible. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/0016a111913f
Part 4. Move ImageKeyData/SharedUserData definition to SharedSurfacesChild header. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/f51da704cd5e
Part 5. Add SharedSurfacesAnimation to manage single ImageKey for animated images. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/5329d8f89313
Part 6. Integrate SharedSurfacesAnimation with ImageContainer. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/f474d2aae3cb
Part 7. Suppress display list regeneration for animated image updates. r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b45f9db5e93
Part 8. Integrate SharedSurfacesAnimation with the rest of imagelib. r=nical

Comment 15

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/48152d6288e3
https://hg.mozilla.org/mozilla-central/rev/1a3433fb1c29
https://hg.mozilla.org/mozilla-central/rev/92fde8cff068
https://hg.mozilla.org/mozilla-central/rev/0016a111913f
https://hg.mozilla.org/mozilla-central/rev/f51da704cd5e
https://hg.mozilla.org/mozilla-central/rev/5329d8f89313
https://hg.mozilla.org/mozilla-central/rev/f474d2aae3cb
https://hg.mozilla.org/mozilla-central/rev/7b45f9db5e93