Skip to content

GitLab

T
tangled
Commit 2d136863 authored by alancutter's avatar alancutter Committed by Commit bot
Browse files
Options

Refactor how UseCounter::SyntheticKeyframesInCompositedCSSAnimation is counted 

This patch rewrites the detection logic for composited animations that
have neutral keyframes.

The previous code was relatively expensive and failed a DCHECK when
custom properties were used. This patch moves the use counting down into
where we actually handle compositing neutral keyframes.

BUG=679994

Review-Url: https://codereview.chromium.org/2620303002
Cr-Commit-Position: refs/heads/master@{#443180}
parent c162e0c6
Hide whitespace changes
Inline Side-by-side
Showing with 68 additions and 34 deletions
third_party/WebKit/LayoutTests/animations/use-count-SyntheticKeyframesInCompositedCSSAnimation.html 0 → 100644
View file @ 2d136863
<!DOCTYPE html>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<style>
@keyframes noSyntheticKeyframes {
from { color: black; }
to { color: green; }
}
@keyframes customPropertySyntheticKeyframes {
to { --x: pants; }
}
@keyframes nonCompositedSyntheticKeyframes {
to { color: green; }
}
@keyframes compositedSyntheticKeyframes {
from {
opacity: 0;
}
to {
transform: rotate(45deg);
opacity: 1;
}
}
#target {
animation-duration: 1s;
}
</style>
<div id="target"></div>
<script>
// From UseCounter.h.
var SyntheticKeyframesInCompositedCSSAnimation = 664;
function forceStyleRecalc() {
getComputedStyle(target).left;
}
test(() => {
forceStyleRecalc();
assert_false(internals.isUseCounted(document, SyntheticKeyframesInCompositedCSSAnimation), 'No animations started');
target.style.animationName = 'noSyntheticKeyframes';
forceStyleRecalc();
assert_false(internals.isUseCounted(document, SyntheticKeyframesInCompositedCSSAnimation), 'noSyntheticKeyframes');
target.style.animationName = 'customPropertySyntheticKeyframes';
forceStyleRecalc();
assert_false(internals.isUseCounted(document, SyntheticKeyframesInCompositedCSSAnimation), 'customPropertySyntheticKeyframes');
target.style.animationName = 'nonCompositedSyntheticKeyframes';
forceStyleRecalc();
assert_false(internals.isUseCounted(document, SyntheticKeyframesInCompositedCSSAnimation), 'nonCompositedSyntheticKeyframes');
target.style.animationName = 'compositedSyntheticKeyframes';
forceStyleRecalc();
assert_true(internals.isUseCounted(document, SyntheticKeyframesInCompositedCSSAnimation), 'compositedSyntheticKeyframes');
}, 'The SyntheticKeyframesInCompositedCSSAnimation use counter should only be triggered by animations with composited properties with neutral keyframes');
</script>
third_party/WebKit/Source/core/animation/Keyframe.h
View file @ 2d136863
...@@ -73,10 +73,7 @@ class CORE_EXPORT Keyframe : public RefCounted<Keyframe> { ...@@ -73,10 +73,7 @@ class CORE_EXPORT Keyframe : public RefCounted<Keyframe> {
double underlyingFraction() const { double underlyingFraction() const {
return m_composite == EffectModel::CompositeReplace ? 0 : 1; return m_composite == EffectModel::CompositeReplace ? 0 : 1;
} }
virtual bool isNeutral() const { virtual bool isNeutral() const = 0;
NOTREACHED();
return false;
}
virtual PassRefPtr<PropertySpecificKeyframe> cloneWithOffset( virtual PassRefPtr<PropertySpecificKeyframe> cloneWithOffset(
double offset) const = 0; double offset) const = 0;
......
third_party/WebKit/Source/core/animation/KeyframeEffectModel.cpp
View file @ 2d136863
...@@ -36,6 +36,7 @@ ...@@ -36,6 +36,7 @@
#include "core/css/CSSPropertyEquality.h" #include "core/css/CSSPropertyEquality.h"
#include "core/css/resolver/StyleResolver.h" #include "core/css/resolver/StyleResolver.h"
#include "core/dom/Document.h" #include "core/dom/Document.h"
#include "core/frame/UseCounter.h"
#include "platform/animation/AnimationUtilities.h" #include "platform/animation/AnimationUtilities.h"
#include "platform/geometry/FloatBox.h" #include "platform/geometry/FloatBox.h"
#include "platform/transforms/TransformationMatrix.h" #include "platform/transforms/TransformationMatrix.h"
...@@ -110,15 +111,22 @@ bool KeyframeEffectModelBase::snapshotAllCompositorKeyframes( ...@@ -110,15 +111,22 @@ bool KeyframeEffectModelBase::snapshotAllCompositorKeyframes(
const ComputedStyle* parentStyle) const { const ComputedStyle* parentStyle) const {
m_needsCompositorKeyframesSnapshot = false; m_needsCompositorKeyframesSnapshot = false;
bool updated = false; bool updated = false;
bool hasNeutralCompositableKeyframe = false;
ensureKeyframeGroups(); ensureKeyframeGroups();
for (CSSPropertyID property : CompositorAnimations::compositableProperties) { for (CSSPropertyID property : CompositorAnimations::compositableProperties) {
PropertySpecificKeyframeGroup* keyframeGroup = PropertySpecificKeyframeGroup* keyframeGroup =
m_keyframeGroups->get(PropertyHandle(property)); m_keyframeGroups->get(PropertyHandle(property));
if (!keyframeGroup) if (!keyframeGroup)
continue; continue;
for (auto& keyframe : keyframeGroup->m_keyframes) for (auto& keyframe : keyframeGroup->m_keyframes) {
updated |= keyframe->populateAnimatableValue(property, element, baseStyle, updated |= keyframe->populateAnimatableValue(property, element, baseStyle,
parentStyle); parentStyle);
hasNeutralCompositableKeyframe |= keyframe->isNeutral();
}
}
if (updated && hasNeutralCompositableKeyframe) {
UseCounter::count(element.document(),
UseCounter::SyntheticKeyframesInCompositedCSSAnimation);
} }
return updated; return updated;
} }
......
third_party/WebKit/Source/core/animation/animatable/AnimatableValueKeyframe.h
View file @ 2d136863
...@@ -46,6 +46,7 @@ class CORE_EXPORT AnimatableValueKeyframe : public Keyframe { ...@@ -46,6 +46,7 @@ class CORE_EXPORT AnimatableValueKeyframe : public Keyframe {
return m_value; return m_value;
} }
bool isNeutral() const final { return m_value->isNeutral(); }
PassRefPtr<Keyframe::PropertySpecificKeyframe> neutralKeyframe( PassRefPtr<Keyframe::PropertySpecificKeyframe> neutralKeyframe(
double offset, double offset,
PassRefPtr<TimingFunction> easing) const final; PassRefPtr<TimingFunction> easing) const final;
......
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp
View file @ 2d136863
...@@ -172,35 +172,6 @@ static StringKeyframeEffectModel* createKeyframeEffectModel( ...@@ -172,35 +172,6 @@ static StringKeyframeEffectModel* createKeyframeEffectModel(
DCHECK(!keyframes.front()->offset()); DCHECK(!keyframes.front()->offset());
DCHECK_EQ(keyframes.back()->offset(), 1); DCHECK_EQ(keyframes.back()->offset(), 1);
// This is used for use counting neutral keyframes running on the compositor.
PropertySet allProperties;
for (const auto& keyframe : keyframes) {
for (const auto& property : keyframe->properties())
allProperties.add(property.cssProperty());
}
const PropertyHandleSet& startKeyframeProperties =
startKeyframe->properties();
const PropertyHandleSet& endKeyframeProperties = endKeyframe->properties();
bool missingStartValues =
startKeyframeProperties.size() < allProperties.size();
bool missingEndValues = endKeyframeProperties.size() < allProperties.size();
if (missingStartValues || missingEndValues) {
for (CSSPropertyID property : allProperties) {
bool startNeedsValue =
missingStartValues &&
!startKeyframeProperties.contains(PropertyHandle(property));
bool endNeedsValue =
missingEndValues &&
!endKeyframeProperties.contains(PropertyHandle(property));
if (!startNeedsValue && !endNeedsValue)
continue;
if (CompositorAnimations::isCompositableProperty(property))
UseCounter::count(
elementForScoping->document(),
UseCounter::SyntheticKeyframesInCompositedCSSAnimation);
}
}
StringKeyframeEffectModel* model = StringKeyframeEffectModel* model =
StringKeyframeEffectModel::create(keyframes, &keyframes[0]->easing()); StringKeyframeEffectModel::create(keyframes, &keyframes[0]->easing());
if (animationIndex > 0 && model->hasSyntheticKeyframes()) if (animationIndex > 0 && model->hasSyntheticKeyframes())
......
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