Closed Bug 1439485 Opened 4 years ago Closed 4 years ago

Transform animations that don't produce any change hints don't run on the compositor

Categories

(Core :: DOM: Animation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: hiro, Assigned: hiro)

Details

Attachments

(1 file)

While I was investigating the reason why stacking-context-transform-none-animation-before-appending-element.html fails intermittently on WebRender, I noticed that transform animations that don't produce any change hint (e.g. translateX(10px) -> translateX(10px)) don't run on the compositor.  That's because we do update the last transform sync time in KeyframeEffectReadOnly::ComposeStyle() if we have cumulative change hint that affects overflow region, whereas we just checks animating property is transform in CanThrottle().  Thus, if the transform animation does not produce any change hint, we update the transform animation on the main-thread repeatedly.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2cdd2c24a8b0affe025a967943fb8d57dcafada
Attachment #8952309 - Flags: review?(bbirtles)
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
Comment on attachment 8952309 [details]
Bug 1439485 - Don't try to unthrottle transform animations that don't affect overflow region.

https://reviewboard.mozilla.org/r/221558/#review229456

::: dom/animation/KeyframeEffectReadOnly.h:491
(Diff revision 2)
>  
>    static const TimeDuration OverflowRegionRefreshInterval();
>  
>    void UpdateEffectSet(mozilla::EffectSet* aEffectSet = nullptr) const;
>  
> +  bool HasTransformMightAffectOverflowRegion() const

Grammatically there should be a "that" before "might" so:

  HasTransformThatMightAffectOverflowRegion ?

or, slightly simpler:

  HasTransformThatMightAffectOverflow ?

::: dom/animation/KeyframeEffectReadOnly.cpp:773
(Diff revision 2)
>                       prop,
>                       *segment,
>                       computedTiming);
>    }
>  
> -  // If the animation produces any transform change hint, we need to record the
> +  // If the animation produces transform change hint that affects overflow

Nit:
s/produces transform/produces a transform/
s/affects overflow/affects the overflow/
Attachment #8952309 - Flags: review?(bbirtles) → review+
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c4425fcdfb5b
Don't try to unthrottle transform animations that don't affect overflow region. r=birtles
Thanks for the review!
https://hg.mozilla.org/mozilla-central/rev/c4425fcdfb5b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.