Closed Bug 1428558 Opened 2 years ago Closed Last year

Support async animated image updates

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: jrmuizel, Assigned: aosmond)

References

(Blocks 1 open bug)

Details

Attachments

(8 files)

These should work really well with webrender and will avoid us having to do any display list building or scene building.
Blocks: 1449521
I've already been doing the prep work for this.
Assignee: nobody → aosmond
Depends on: 1337111
Hey Andrew & Jeff -- What more is required here once bug 1337111 lands?  How important is this to WR MVP?
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(aosmond)
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 :).
Flags: needinfo?(aosmond)
Update: I got this partly working today. Still some bugs to iron out / understand.
Flags: needinfo?(jmuizelaar)
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.
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
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
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
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
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
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.
See Also: → 1458387
Attachment #9013746 - Attachment description: Bug 1428558 - Part 2. Improve plumbing to sending resource updates to WebRender → Bug 1428558 - Part 2. Improve plumbing to sending resource updates to WebRender.
Blocks: 1458387
Pushed by aosmond@gmail.com:
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
You need to log in before you can comment on or make changes to this bug.