1720631 - Setting currentTime on a compositor-accelerated Animation causes a paint + scene build

Status: NEW

(Core :: DOM: Animation, defect)

Description

[Markus Stange [:mstange]]

Attachment: testcase

In the attached testcase, moving the mouse causes main thread paints, which are followed by a WR display list and scene build.

Profile: https://share.firefox.dev/3ifpDLc

I was hoping that using Element.animate with a transform animation could skip main thread painting entirely, and only cause a WR frame build.

This was an experiment for bug 1720517.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

IIUC what the outcome expects, we should skip painting even if compositor-running animation's currentTime is changed, and we should directly manipulate it on the compositor?

Note about transform animations, a transform animation inside a scroll container affects its scrollable area. So it's still worth updating it on the main-thread when the animation's currentTime is modified, I think that's the reason why we paint it at that time. That said, I am not particularly a big fan of the behavior, as far as I know, Chrome doesn't do that for transform animations, so we may be able to align with their behavior.

Comment 2

[Markus Stange [:mstange]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)

IIUC what the outcome expects, we should skip painting even if compositor-running animation's currentTime is changed, and we should directly manipulate it on the compositor?

That would be ideal. Alternatively, if you have other ideas for how to update a WR animated property value from JS without going through display list building + scene building, I'd be very curious to hear them. See bug 1720517.

Note about transform animations, a transform animation inside a scroll container affects its scrollable area. So it's still worth updating it on the main-thread when the animation's currentTime is modified, I think that's the reason why we paint it at that time. That said, I am not particularly a big fan of the behavior, as far as I know, Chrome doesn't do that for transform animations, so we may be able to align with their behavior.

Ah, yeah I'm not a fan of that behavior either, it makes these animations more CPU intensive.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to Markus Stange [:mstange] from comment #2)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)

IIUC what the outcome expects, we should skip painting even if compositor-running animation's currentTime is changed, and we should directly manipulate it on the compositor?

That would be ideal. Alternatively, if you have other ideas for how to update a WR animated property value from JS without going through display list building + scene building, I'd be very curious to hear them. See bug 1720517.

That's a good question, indeed I though it a bit, but don't have clear idea yet. Basic machinery I can think of is;

1. Add a new cheap webrender transaction for updating compositor-running animations (I am not sure transaction is a right term or not)
2. Queue animations into the transaction when currentTime is modified
3. Trigger the transaction if there's any queued animations (I guess it's triggered from the refresh driver's tick or some sort of relevant places)
4. In the transaction, call another new variant of AnimationInfo::AddAnimationsForDisplayItem which is currently called during display list building, but the new variant function should be isolated from nsDisplayListBuilder for this particular case
5. Also in the transaction, we probably have to end up calling this WebRenderParentCommand::TOpAddCompositorAnimations, I am not sure how we reach to the TOpAddCompositorAnimations command TBH.

With these, I believe our OMTASampler will pick the new currentTime from the next vsync tick.