Skip to content

Commit

Permalink
[WebAnimation] Optimize updateCSSTransitions
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261715
rdar://115703255

Reviewed by Mark Lam.

WebAnimation's updateCSSTransitions is inefficient,

1. We are repeatedly copying AnimationProperty while it is held. This is appearing much in the profiler.
   Let's not copy them. We use `const AnimationProperty&`. This is OK since
   1.1. updateCSSTransitionsForStyleableAndProperty's owner is holding this ownership. Reference is always valid.
   1.2. Other change immediately copies or uses switchOn to dispatch. So this is fine.
2. Avoid repeated call of `styleable.hasRunningTransitionForProperty` etc. This is really costly function since
   it involves HashMap lookup. We reorganize updateCSSTransitionsForStyleableAndProperty not to do wasteful
   operations multiple times.

* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::CSSPropertyBlendingContext::CSSPropertyBlendingContext):
(WebCore::CSSPropertyAnimation::blendProperty):
(WebCore::CSSPropertyAnimation::isPropertyAnimatable):
(WebCore::CSSPropertyAnimation::isPropertyAdditiveOrCumulative):
(WebCore::CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration):
(WebCore::CSSPropertyAnimation::animationOfPropertyIsAccelerated):
(WebCore::CSSPropertyAnimation::propertiesEqual):
(WebCore::CSSPropertyAnimation::canPropertyBeInterpolated):
* Source/WebCore/animation/CSSPropertyAnimation.h:
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::hasCompletedTransitionForProperty const):
(WebCore::Element::hasRunningTransitionForProperty const):
* Source/WebCore/dom/Element.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::keyframeEffectForElementAndProperty):
(WebCore::propertyInStyleMatchesValueForTransitionInMap):
(WebCore::transitionMatchesProperty):
(WebCore::updateCSSTransitionsForStyleableAndProperty):
(WebCore::Styleable::updateCSSTransitions const):
* Source/WebCore/style/Styleable.h:
(WebCore::Styleable::hasCompletedTransitionForProperty const):
(WebCore::Styleable::hasRunningTransitionForProperty const):

Canonical link: https://commits.webkit.org/268113@main
  • Loading branch information
Constellation committed Sep 19, 2023
1 parent ba13698 commit 7fd0774
Show file tree
Hide file tree
Showing 12 changed files with 61 additions and 56 deletions.
16 changes: 8 additions & 8 deletions Source/WebCore/animation/CSSPropertyAnimation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ struct CSSPropertyBlendingContext : BlendingContext {
const CSSPropertyBlendingClient& client;
AnimatableProperty property;

CSSPropertyBlendingContext(double progress, bool isDiscrete, CompositeOperation compositeOperation, const CSSPropertyBlendingClient& client, AnimatableProperty property, IterationCompositeOperation iterationCompositeOperation = IterationCompositeOperation::Replace, double currentIteration = 0)
CSSPropertyBlendingContext(double progress, bool isDiscrete, CompositeOperation compositeOperation, const CSSPropertyBlendingClient& client, const AnimatableProperty& property, IterationCompositeOperation iterationCompositeOperation = IterationCompositeOperation::Replace, double currentIteration = 0)
: BlendingContext(progress, isDiscrete, compositeOperation, iterationCompositeOperation, currentIteration)
, client(client)
, property(property)
Expand Down Expand Up @@ -4342,7 +4342,7 @@ static void blendCustomProperty(const CSSPropertyBlendingClient& client, const A
destination.setCustomPropertyValue(blendedCSSCustomPropertyValue(from, to, *fromValue, *toValue, blendingContext), isInherited);
}

void CSSPropertyAnimation::blendProperty(const CSSPropertyBlendingClient& client, AnimatableProperty property, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation compositeOperation, IterationCompositeOperation iterationCompositeOperation, double currentIteration)
void CSSPropertyAnimation::blendProperty(const CSSPropertyBlendingClient& client, const AnimatableProperty& property, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation compositeOperation, IterationCompositeOperation iterationCompositeOperation, double currentIteration)
{
WTF::switchOn(property,
[&] (CSSPropertyID propertyId) {
Expand All @@ -4353,7 +4353,7 @@ void CSSPropertyAnimation::blendProperty(const CSSPropertyBlendingClient& client
);
}

bool CSSPropertyAnimation::isPropertyAnimatable(AnimatableProperty property)
bool CSSPropertyAnimation::isPropertyAnimatable(const AnimatableProperty& property)
{
return WTF::switchOn(property,
[] (CSSPropertyID propertyId) {
Expand All @@ -4366,7 +4366,7 @@ bool CSSPropertyAnimation::isPropertyAnimatable(AnimatableProperty property)
);
}

bool CSSPropertyAnimation::isPropertyAdditiveOrCumulative(AnimatableProperty property)
bool CSSPropertyAnimation::isPropertyAdditiveOrCumulative(const AnimatableProperty& property)
{
return WTF::switchOn(property,
[] (CSSPropertyID propertyId) {
Expand All @@ -4391,7 +4391,7 @@ static bool syntaxValuesRequireBlendingForAccumulativeIteration(const CSSCustomP
});
}

bool CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(const CSSPropertyBlendingClient&, AnimatableProperty property, const RenderStyle& a, const RenderStyle& b)
bool CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(const CSSPropertyBlendingClient&, const AnimatableProperty& property, const RenderStyle& a, const RenderStyle& b)
{
return WTF::switchOn(property,
[&] (CSSPropertyID propertyId) {
Expand Down Expand Up @@ -4426,7 +4426,7 @@ bool CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(cons
);
}

bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(AnimatableProperty property, const Settings& settings)
bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(const AnimatableProperty& property, const Settings& settings)
{
return WTF::switchOn(property,
[&] (CSSPropertyID cssProperty) {
Expand All @@ -4437,7 +4437,7 @@ bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(AnimatableProperty p
);
}

bool CSSPropertyAnimation::propertiesEqual(AnimatableProperty property, const RenderStyle& a, const RenderStyle& b, const Document&)
bool CSSPropertyAnimation::propertiesEqual(const AnimatableProperty& property, const RenderStyle& a, const RenderStyle& b, const Document&)
{
return WTF::switchOn(property,
[&] (CSSPropertyID propertyId) {
Expand Down Expand Up @@ -4483,7 +4483,7 @@ static bool typeOfSyntaxValueCanBeInterpolated(const CSSCustomPropertyValue::Syn
);
}

bool CSSPropertyAnimation::canPropertyBeInterpolated(AnimatableProperty property, const RenderStyle& a, const RenderStyle& b, const Document&)
bool CSSPropertyAnimation::canPropertyBeInterpolated(const AnimatableProperty& property, const RenderStyle& a, const RenderStyle& b, const Document&)
{
return WTF::switchOn(property,
[&] (CSSPropertyID propertyId) {
Expand Down
14 changes: 7 additions & 7 deletions Source/WebCore/animation/CSSPropertyAnimation.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ class Settings;

class CSSPropertyAnimation {
public:
static bool isPropertyAnimatable(AnimatableProperty);
static bool isPropertyAdditiveOrCumulative(AnimatableProperty);
static bool propertyRequiresBlendingForAccumulativeIteration(const CSSPropertyBlendingClient&, AnimatableProperty, const RenderStyle& a, const RenderStyle& b);
static bool animationOfPropertyIsAccelerated(AnimatableProperty, const Settings&);
static bool propertiesEqual(AnimatableProperty, const RenderStyle& a, const RenderStyle& b, const Document&);
static bool canPropertyBeInterpolated(AnimatableProperty, const RenderStyle& a, const RenderStyle& b, const Document&);
static bool isPropertyAnimatable(const AnimatableProperty&);
static bool isPropertyAdditiveOrCumulative(const AnimatableProperty&);
static bool propertyRequiresBlendingForAccumulativeIteration(const CSSPropertyBlendingClient&, const AnimatableProperty&, const RenderStyle& a, const RenderStyle& b);
static bool animationOfPropertyIsAccelerated(const AnimatableProperty&, const Settings&);
static bool propertiesEqual(const AnimatableProperty&, const RenderStyle& a, const RenderStyle& b, const Document&);
static bool canPropertyBeInterpolated(const AnimatableProperty&, const RenderStyle& a, const RenderStyle& b, const Document&);
static CSSPropertyID getPropertyAtIndex(int, std::optional<bool>& isShorthand);
static std::optional<CSSPropertyID> getAcceleratedPropertyAtIndex(int, const Settings&);
static int getNumProperties();

static void blendProperty(const CSSPropertyBlendingClient&, AnimatableProperty, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation, IterationCompositeOperation = IterationCompositeOperation::Replace, double currentIteration = 0);
static void blendProperty(const CSSPropertyBlendingClient&, const AnimatableProperty&, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation, IterationCompositeOperation = IterationCompositeOperation::Replace, double currentIteration = 0);
};

} // namespace WebCore
4 changes: 2 additions & 2 deletions Source/WebCore/animation/CSSTransition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace WebCore {

WTF_MAKE_ISO_ALLOCATED_IMPL(CSSTransition);

Ref<CSSTransition> CSSTransition::create(const Styleable& owningElement, AnimatableProperty property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle& oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
Ref<CSSTransition> CSSTransition::create(const Styleable& owningElement, const AnimatableProperty& property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle& oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
{
auto result = adoptRef(*new CSSTransition(owningElement, property, generationTime, backingAnimation, oldStyle, newStyle, reversingAdjustedStartStyle, reversingShorteningFactor));
result->initialize(&oldStyle, newStyle, { nullptr });
Expand All @@ -49,7 +49,7 @@ Ref<CSSTransition> CSSTransition::create(const Styleable& owningElement, Animata
return result;
}

CSSTransition::CSSTransition(const Styleable& styleable, AnimatableProperty property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle& oldStyle, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
CSSTransition::CSSTransition(const Styleable& styleable, const AnimatableProperty& property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle& oldStyle, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
: DeclarativeAnimation(styleable, backingAnimation)
, m_property(property)
, m_generationTime(generationTime)
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/animation/CSSTransition.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class RenderStyle;
class CSSTransition final : public DeclarativeAnimation {
WTF_MAKE_ISO_ALLOCATED(CSSTransition);
public:
static Ref<CSSTransition> create(const Styleable&, AnimatableProperty, MonotonicTime generationTime, const Animation&, const RenderStyle& oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double);
static Ref<CSSTransition> create(const Styleable&, const AnimatableProperty&, MonotonicTime generationTime, const Animation&, const RenderStyle& oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double);
~CSSTransition() = default;

const AtomString transitionProperty() const;
Expand All @@ -55,7 +55,7 @@ class CSSTransition final : public DeclarativeAnimation {
double reversingShorteningFactor() const { return m_reversingShorteningFactor; }

private:
CSSTransition(const Styleable&, AnimatableProperty, MonotonicTime generationTime, const Animation&, const RenderStyle& oldStyle, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double);
CSSTransition(const Styleable&, const AnimatableProperty&, MonotonicTime generationTime, const Animation&, const RenderStyle& oldStyle, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double);
void setTimingProperties(Seconds delay, Seconds duration);
Ref<DeclarativeAnimationEvent> createEvent(const AtomString& eventType, std::optional<Seconds> scheduledTime, double elapsedTime, PseudoId) final;
void resolve(RenderStyle& targetStyle, const Style::ResolutionContext&, std::optional<Seconds>) final;
Expand Down
19 changes: 10 additions & 9 deletions Source/WebCore/animation/KeyframeEffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -948,29 +948,30 @@ const HashSet<AnimatableProperty>& KeyframeEffect::animatedProperties()
return m_animatedProperties;
}

bool KeyframeEffect::animatesProperty(AnimatableProperty property) const
bool KeyframeEffect::animatesProperty(const AnimatableProperty& property) const
{
if (!m_blendingKeyframes.isEmpty())
return m_blendingKeyframes.containsProperty(property);

return m_parsedKeyframes.findIf([&](const auto& keyframe) {
return WTF::switchOn(property,
[&](CSSPropertyID cssProperty) {
return WTF::switchOn(property,
[&](CSSPropertyID cssProperty) {
return m_parsedKeyframes.findIf([&](const auto& keyframe) {
for (auto keyframeProperty : keyframe.styleStrings.keys()) {
if (keyframeProperty == cssProperty)
return true;
}
return false;
},
[&](const AtomString& customProperty) {
});
},
[&](const AtomString& customProperty) {
return m_parsedKeyframes.findIf([&](const auto& keyframe) {
for (auto keyframeProperty : keyframe.customStyleStrings.keys()) {
if (keyframeProperty == customProperty)
return true;
}
return false;
}
);
}) != notFound;
});
}) != notFound;
}

bool KeyframeEffect::forceLayoutIfNeeded()
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/animation/KeyframeEffect.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class KeyframeEffect final : public AnimationEffect, public CSSPropertyBlendingC
void computeDeclarativeAnimationBlendingKeyframes(const RenderStyle* oldStyle, const RenderStyle& newStyle, const Style::ResolutionContext&);
const KeyframeList& blendingKeyframes() const { return m_blendingKeyframes; }
const HashSet<AnimatableProperty>& animatedProperties();
bool animatesProperty(AnimatableProperty) const;
bool animatesProperty(const AnimatableProperty&) const;

bool computeExtentOfTransformAnimation(LayoutRect&) const;
bool computeTransformedExtentViaTransformList(const FloatRect&, const RenderStyle&, LayoutRect&) const;
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4510,14 +4510,14 @@ const AnimationCollection* Element::animations(PseudoId pseudoId) const
return nullptr;
}

bool Element::hasCompletedTransitionForProperty(PseudoId pseudoId, AnimatableProperty property) const
bool Element::hasCompletedTransitionForProperty(PseudoId pseudoId, const AnimatableProperty& property) const
{
if (auto* animationData = animationRareData(pseudoId))
return animationData->completedTransitionsByProperty().contains(property);
return false;
}

bool Element::hasRunningTransitionForProperty(PseudoId pseudoId, AnimatableProperty property) const
bool Element::hasRunningTransitionForProperty(PseudoId pseudoId, const AnimatableProperty& property) const
{
if (auto* animationData = animationRareData(pseudoId))
return animationData->runningTransitionsByProperty().contains(property);
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/dom/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -563,8 +563,8 @@ class Element : public ContainerNode {
bool hasKeyframeEffects(PseudoId) const;

const AnimationCollection* animations(PseudoId) const;
bool hasCompletedTransitionForProperty(PseudoId, AnimatableProperty) const;
bool hasRunningTransitionForProperty(PseudoId, AnimatableProperty) const;
bool hasCompletedTransitionForProperty(PseudoId, const AnimatableProperty&) const;
bool hasRunningTransitionForProperty(PseudoId, const AnimatableProperty&) const;
bool hasRunningTransitions(PseudoId) const;
AnimationCollection& ensureAnimations(PseudoId);

Expand Down
8 changes: 4 additions & 4 deletions Source/WebCore/rendering/style/KeyframeList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,13 @@ bool KeyframeList::usesContainerUnits() const
return false;
}

void KeyframeList::addProperty(AnimatableProperty property)
void KeyframeList::addProperty(const AnimatableProperty& property)
{
ASSERT(!std::holds_alternative<CSSPropertyID>(property) || std::get<CSSPropertyID>(property) != CSSPropertyCustom);
m_properties.add(property);
}

bool KeyframeList::containsProperty(AnimatableProperty property) const
bool KeyframeList::containsProperty(const AnimatableProperty& property) const
{
return m_properties.contains(property);
}
Expand Down Expand Up @@ -310,13 +310,13 @@ void KeyframeList::updatePropertiesMetadata(const StyleProperties& properties)
}
}

void KeyframeValue::addProperty(AnimatableProperty property)
void KeyframeValue::addProperty(const AnimatableProperty& property)
{
ASSERT(!std::holds_alternative<CSSPropertyID>(property) || std::get<CSSPropertyID>(property) != CSSPropertyCustom);
m_properties.add(property);
}

bool KeyframeValue::containsProperty(AnimatableProperty property) const
bool KeyframeValue::containsProperty(const AnimatableProperty& property) const
{
return m_properties.contains(property);
}
Expand Down
8 changes: 4 additions & 4 deletions Source/WebCore/rendering/style/KeyframeList.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class KeyframeValue {
{
}

void addProperty(AnimatableProperty);
bool containsProperty(AnimatableProperty) const;
void addProperty(const AnimatableProperty&);
bool containsProperty(const AnimatableProperty&) const;
const HashSet<AnimatableProperty>& properties() const { return m_properties; }

double key() const { return m_key; }
Expand Down Expand Up @@ -93,8 +93,8 @@ class KeyframeList {

void insert(KeyframeValue&&);

void addProperty(AnimatableProperty);
bool containsProperty(AnimatableProperty) const;
void addProperty(const AnimatableProperty&);
bool containsProperty(const AnimatableProperty&) const;
const HashSet<AnimatableProperty>& properties() const { return m_properties; }

bool containsAnimatableProperty() const;
Expand Down

0 comments on commit 7fd0774

Please sign in to comment.