Commit df884f79 authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Restrict neutral keyframes from being composited on top of finished animations

This CL prevents animations from running on the compositor when they contain
neutral keyframes and apply on top of existing animaions on the same property.

Bug: 805323
Change-Id: Iba19882454211f127358b78fcbfa32bfdf183048
Reviewed-on: https://chromium-review.googlesource.com/884781Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531814}
parent 5be8e135
<!DOCTYPE html>
<style>
#expected {
position: absolute;
width: 100px;
height: 100px;
background: green;
transform: translateX(100px);
}
</style>
Finished fill forwards animations influence the neutral keyframes of animations on top of them.
<div id="expected"></div>
<!DOCTYPE html>
<style>
div {
position: absolute;
width: 100px;
height: 100px;
}
#expectation {
background: red;
transform: translateX(100px);
}
#target {
background: green;
}
</style>
Finished fill forwards animations influence the neutral keyframes of animations on top of them.
<div id="expectation"></div>
<div id="target"></div>
<script>
'use strict';
if (window.testRunner) {
testRunner.waitUntilDone();
}
target.animate({transform: 'translateX(100px)'}, {fill: 'forwards'});
requestAnimationFrame(() => {
target.animate({transform: 'translateX(100px)'}, 1e10);
if (window.testRunner) {
testRunner.notifyDone();
}
});
</script>
......@@ -65,7 +65,8 @@ namespace blink {
namespace {
bool ConsiderAnimationAsIncompatible(const Animation& animation,
const Animation& animation_to_add) {
const Animation& animation_to_add,
const EffectModel& effect_to_add) {
if (&animation == &animation_to_add)
return false;
......@@ -77,7 +78,10 @@ bool ConsiderAnimationAsIncompatible(const Animation& animation,
return true;
case Animation::kPaused:
case Animation::kFinished:
return Animation::HasLowerPriority(&animation_to_add, &animation);
if (Animation::HasLowerPriority(&animation, &animation_to_add)) {
return effect_to_add.AffectedByUnderlyingAnimations();
}
return true;
default:
NOTREACHED();
return true;
......@@ -103,6 +107,12 @@ bool IsTransformRelatedAnimation(const Element& target_element,
bool HasIncompatibleAnimations(const Element& target_element,
const Animation& animation_to_add,
const EffectModel& effect_to_add) {
if (!target_element.HasAnimations())
return false;
ElementAnimations* element_animations = target_element.GetElementAnimations();
DCHECK(element_animations);
const bool affects_opacity =
effect_to_add.Affects(PropertyHandle(GetCSSPropertyOpacity()));
const bool affects_transform = effect_to_add.IsTransformRelatedEffect();
......@@ -111,16 +121,12 @@ bool HasIncompatibleAnimations(const Element& target_element,
const bool affects_backdrop_filter =
effect_to_add.Affects(PropertyHandle(GetCSSPropertyBackdropFilter()));
if (!target_element.HasAnimations())
return false;
ElementAnimations* element_animations = target_element.GetElementAnimations();
DCHECK(element_animations);
for (const auto& entry : element_animations->Animations()) {
const Animation* attached_animation = entry.key;
if (!ConsiderAnimationAsIncompatible(*attached_animation, animation_to_add))
if (!ConsiderAnimationAsIncompatible(*attached_animation, animation_to_add,
effect_to_add)) {
continue;
}
if ((affects_opacity && attached_animation->Affects(
target_element, GetCSSPropertyOpacity())) ||
......@@ -130,8 +136,9 @@ bool HasIncompatibleAnimations(const Element& target_element,
attached_animation->Affects(target_element, GetCSSPropertyFilter())) ||
(affects_backdrop_filter &&
attached_animation->Affects(target_element,
GetCSSPropertyBackdropFilter())))
GetCSSPropertyBackdropFilter()))) {
return true;
}
}
return false;
......@@ -338,8 +345,10 @@ void CompositorAnimations::CancelIncompatibleAnimationsOnCompositor(
for (const auto& entry : element_animations->Animations()) {
Animation* attached_animation = entry.key;
if (!ConsiderAnimationAsIncompatible(*attached_animation, animation_to_add))
if (!ConsiderAnimationAsIncompatible(*attached_animation, animation_to_add,
effect_to_add)) {
continue;
}
if ((affects_opacity && attached_animation->Affects(
target_element, GetCSSPropertyOpacity())) ||
......@@ -349,8 +358,9 @@ void CompositorAnimations::CancelIncompatibleAnimationsOnCompositor(
attached_animation->Affects(target_element, GetCSSPropertyFilter())) ||
(affects_backdrop_filter &&
attached_animation->Affects(target_element,
GetCSSPropertyBackdropFilter())))
GetCSSPropertyBackdropFilter()))) {
attached_animation->CancelAnimationOnCompositor();
}
}
}
......
......@@ -64,6 +64,7 @@ class CORE_EXPORT EffectModel : public GarbageCollectedFinalized<EffectModel> {
Vector<scoped_refptr<Interpolation>>&) const = 0;
virtual bool Affects(const PropertyHandle&) const { return false; }
virtual bool AffectedByUnderlyingAnimations() const = 0;
virtual bool IsTransformRelatedEffect() const { return false; }
virtual bool IsKeyframeEffectModel() const { return false; }
......
......@@ -291,7 +291,7 @@ void KeyframeEffectModelBase::EnsureInterpolationEffectPopulated() const {
interpolation_effect_.SetPopulated();
}
bool KeyframeEffectModelBase::IsReplaceOnly() {
bool KeyframeEffectModelBase::IsReplaceOnly() const {
EnsureKeyframeGroups();
for (const auto& entry : *keyframe_groups_) {
for (const auto& keyframe : entry.value->Keyframes()) {
......
......@@ -74,7 +74,8 @@ class CORE_EXPORT KeyframeEffectModelBase : public EffectModel {
friend class KeyframeEffectModelBase;
};
bool IsReplaceOnly();
bool AffectedByUnderlyingAnimations() const final { return !IsReplaceOnly(); }
bool IsReplaceOnly() const;
PropertyHandleSet Properties() const;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment