New issue
Advanced search Search tips

Issue 914957 link

Starred by 1 user
Status: Fixed
Owner:
yigu@chromium.org
Closed: Jan 16
Cc:
majidvp@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 430155


Participants' hotlists:
AnimationWorklet-MVP


Sign in to add a comment

Animation Worklet - main thread should peek only when animation input time has changed

Project Member Reported by majidvp@chromium.org, Dec 13 
Currently, on main thread we don't optimize PEEK to take animation input time into account. In CC we only ask for update if animation input time has changed since the last update. We should implement something similar on main thread as well.

The implication of the current main thread un-optimized behavior is that if we have a scroll-linked worklet animation, the main thread peeks and waked up AnimationWorklet thread every frame even if scroller has not scrolled.

Comment 1 by majidvp@chromium.org, Dec 13 


Per discussion with yigu@: Unlike in cc this is not a simple matter of comparing the active current time with the previous main thread current time where we last peeked. But rather we should compare the active current time with the animation worklet current time that produced the last  peeked value we received.


Here is the code in main: https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc?dr=CSs&g=0&l=529

Here is the code in cc: https://cs.chromium.org/chromium/src/cc/animation/worklet_animation.cc?type=cs&sq=package:chromium&g=0&l=168

Comment 2 by majidvp@chromium.org, Dec 14

Summary: Animation Worklet - main thread should peek only when animation input time has changed (was: Animation Worklet - main thread should peek only if animation input has not changed)

Comment 3 by majidvp@chromium.org, Jan 8

Labels: Pri-3

Comment 4 by majidvp@chromium.org, Jan 8

Labels: -Pri-3 Pri-2

Comment 5 by yigu@chromium.org, Jan 11

Cc: -yigu@chromium.org
Owner: yigu@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 15 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e21c19877108d278decc483c238d5c63f6e45c5f

commit e21c19877108d278decc483c238d5c63f6e45c5f
Author: Yi Gu <yigu@chromium.org>
Date: Tue Jan 15 23:59:14 2019

[Animation Worklet] Main thread only peeks when necessary

Currently on main thread we peek the composited properties every frame.
Rather, we should peek if:
  - the last peek request has not been fulfilled. i.e. worklet has not
  produced local times or
  - input time has changed.

This change ensures we only peek when needed which is particularly
beneficial for scroll-linked animations where the input may not change
for many frames.

Bug:  914957 
Change-Id: I668bdb24b87a3bf669bc0a236427673795578b77
Reviewed-on: https://chromium-review.googlesource.com/c/1406142
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622885}
[modify] https://crrev.com/e21c19877108d278decc483c238d5c63f6e45c5f/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc
[modify] https://crrev.com/e21c19877108d278decc483c238d5c63f6e45c5f/third_party/blink/renderer/modules/animationworklet/worklet_animation.h
[modify] https://crrev.com/e21c19877108d278decc483c238d5c63f6e45c5f/third_party/blink/renderer/modules/animationworklet/worklet_animation_test.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e21c19877108d278decc483c238d5c63f6e45c5f

commit e21c19877108d278decc483c238d5c63f6e45c5f
Author: Yi Gu <yigu@chromium.org>
Date: Tue Jan 15 23:59:14 2019

[Animation Worklet] Main thread only peeks when necessary

Currently on main thread we peek the composited properties every frame.
Rather, we should peek if:
  - the last peek request has not been fulfilled. i.e. worklet has not
  produced local times or
  - input time has changed.

This change ensures we only peek when needed which is particularly
beneficial for scroll-linked animations where the input may not change
for many frames.

Bug:  914957 
Change-Id: I668bdb24b87a3bf669bc0a236427673795578b77
Reviewed-on: https://chromium-review.googlesource.com/c/1406142
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622885}
[modify] https://crrev.com/e21c19877108d278decc483c238d5c63f6e45c5f/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc
[modify] https://crrev.com/e21c19877108d278decc483c238d5c63f6e45c5f/third_party/blink/renderer/modules/animationworklet/worklet_animation.h
[modify] https://crrev.com/e21c19877108d278decc483c238d5c63f6e45c5f/third_party/blink/renderer/modules/animationworklet/worklet_animation_test.cc

Comment 8 by yigu@chromium.org, Jan 16

Status: Fixed (was: Available)
►

Sign in to add a comment