Plumbing for animation <--> displayItem

[Xida Chen]

Hey,

Recently I did some profiling for CSS animations on top 1k websites, and the result shows that there is some potential performance gain if we could move more CSS animations to compositor thread. I am currently looking at background-color and I need some help with the plumbing.

Animating background-color on compositor thread requires two things on cc side:

1) which displayItem is in charge of background-color?

2) mutate the color for that particular displayItem. (enne@'s custom display list work should enable this)

So I am stuck at #1. Basically when blink pass the display item list to cc, it needs to tell cc which display item is for background-color, and my hope is that the plumbing would fit into spv2.

My current idea is to keep a map from animation id <--> index of the display item in the list. The map can be kept in PaintArtifact so that whenever there is a change to the list, we update the map. Does this make sense at all?

If it makes sense, there are some more things to sort out.

1. In PaintArtifact::AppendToWebDisplayItemList, we would need to update the map. Now in that function, it goes through all display items in the list, and we probably don't want to update the map for each display item. For a specific display item, if we know that it is not animation related, we can just ignore it and don't update the map. So my idea is that maybe we can add a new type in DisplayItem::Type.

2. Say that the above point is reasonable and we do have a new type called kBackgroundColorAnimation, when we construct a new DisplayItem with that type, how do we associate an animation id with that? My guess is that somewhere in the CompositorAnimations.h, maybe we could do that?

Please let me know your ideas, suggestion / comments are highly appreciated.

[Tien-Ren Chen]

This proposal makes sense to me. Though I feel
BACKGROUND_COLOR_ANIMATION may be too specific. How about just
DYNAMIC_DRAWING, which works similar to DrawingDisplayItem except the
actual drawing is driven by some impl-side driver?

Then for your second question, I think we need to make a few decision
first before a conclusion can be reached:
* Should background color animation be a compositing trigger? (i.e.
cause a CompositedLayerMapping to be created for that element?)
* Should there ever be main-thread-driven background color animation?
(as opposed to always driven by cc impl-side)
In my opinion, answer to both questions should be no. And the
implication is that:
- One layer may contain multiple dynamic drawing items
- Dynamic drawing items don't need to be at the beginning of a layer's item list
Now back to your question. We have existing mechanism to lookup
ElementId from AnimationPlayer, where ElementId is an unique handle
for the DOM element that owns an animation object.
Currently in SPv1 it is guaranteed that each ElementId will map to one
or zero Layer/LayerImpl. This is likely going to change in SPv2, so
that ElementId will map to property nodes.

Either way, animation_player->element_id() won't be powerful enough
for your use case, because the element that's animating its background
color may not necessarily create a layer.
Conceptually the animation subsystem should not know about layer
stuffs, however, for invalidating tiles as background color mutates,
someone needs to know which LayerImpl owns the animated display item.
I think each DynamicDrawingDisplayItem should have an animation_id
that maps to a cc::Animation. At commit time, LayerTreeHostImpl should
maintain a mapping from animation_id to a (layer_id,
display_item_index) pair for each DynamicDrawingDisplayItem it saw.
----
There are other subtleties need to be worried about.

1. How do we throttle BG animation? Because animating BG induces tile
invalidation, it needs to be done on the pending tree, how do we avoid
starving (pending tree never gets ready due to chasing a moving
target)?
2. How does it work when a BG animation shares a timeline with
non-raster-inducing animations? (i.e. one animation applies on pending
tree, the other applies on active tree, but both need to apply
synchronously)
3. Opaque layers that have partial pixels due to contents scale are
overfilled with "safe opaque background color". Should we animate that
as well?

> --
> You received this message because you are subscribed to the Google Groups
> "paint-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to paint-dev+...@chromium.org.
> To post to this group, send email to pain...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/paint-dev/CALRxhAuGuU%3DC6AYOURJKEWjAOu29qZnjThd%3DSw3FH04BCd1YKw%40mail.gmail.com.

[wko...@chromium.org]

I am hoping we can get input here from Xianzhu or Chris who have spent more time considering when/whether to create a new display item type vs. routing data in some other manner.

I'm also not sure where the best place is to create and maintain the link between an animation and its relevant display item(s). I think a single animated element could have multiple display items for different phases, though in the end there's probably only one that you'll care about for animation purposes. In case it is helpful, a couple of notes to guide investigation:

- for SPv1, DocumentAnimations::UpdateAnimations (which I believe is main entry point into, eventually, through other classes, the rest of CompositorAnimations logic) is called before Paint, and so before display items are known. Here is the SPv1 call that leads into UpdateAnimations:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/FrameView.cpp?q=frameview.cpp&sq=package:chromium&dr&l=3131

- for SPv2, we have intentionally moved DocumentAnimations::update to be called after Paint so that we can make use of data that's an output of PaintArtifactCompositor, which uses the paint property trees. Here is where we do it for SPv2:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/FrameView.cpp?q=frameview.cpp&sq=package:chromium&dr&l=3188

I mention this because you were suggesting adding logic to CompositorAnimations, but at that point in SPv1 at least we won't actually have paint info.

Perhaps what we need is to add some information to the existing display item(s) that you care about, when they are created during painting, to then allow us to see this when we look at the display item cc-side, and we could build/maintain maps cc-side if needed. So for example when we paint a box's background here:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/BoxPainter.cpp?q=boxpainter.cpp&sq=package:chromium&dr&l=147

perhaps we should also be checking whether there's an active background color animation somehow and if so add some add'l info to that drawing recorder, or add another display item.

Note that there is almost zero animation specific logic in core/paint currently, try:

.../third_party/WebKit/Source/core/paint$ find . -type f | xargs grep -i animation | grep -v -i test

so some hesitation on adding it, but then this is the first time we'll be tying an animation to its display item, after all.

This is all just rather free form brainstorming, since that is where we are currently.

[Victor Miura]

+graphics-dev 

khushalsagar@ and vmpstr@ are looking into raster inducing invalidations via the pending tree.  I think the idea would be that we don't animate on the pending tree, rather we commit a snapshot of animation state to a new pending tree.  This may mean merging with an incoming commit from the main-thread.  If the pending tree is busy then this throttles commits (same as with main-thread commits).

2. How does it work when a BG animation shares a timeline with
non-raster-inducing animations? (i.e. one animation applies on pending
tree, the other applies on active tree, but both need to apply
synchronously)

How does that work currently?  I'm not sure we could be synchronizing such animations today without doing both animations on the main thread.

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/paint-dev/CAKxkHO_z9RRkY1uob%2BXzZdb1if6OwFR0DmTO7QMVZbXQuvLrRQ%40mail.gmail.com.

[Xida Chen]

Had an offline discussion with Tien-Ren. I don't think currently we are synchronizing such animations. For example, if BG-color and opacity animations share the same timeline, they are probably not synced perfectly because BG-color is on main and opacity is on compositor thread.

 

3. Opaque layers that have partial pixels due to contents scale are
overfilled with "safe opaque background color". Should we animate that
as well?

I'd say that we probably should not, at least for the initial stage. Animating those layers will add a lot complexity.