- Revision
- 286544
- Author
- [email protected]
- Date
- 2021-12-06 09:20:08 -0800 (Mon, 06 Dec 2021)
Log Message
Clean up virtual methods on AnimationEffect
https://bugs.webkit.org/show_bug.cgi?id=233868
Reviewed by Simon Fraser.
To this day, the only concrete subclass of AnimationEffect has been KeyframeEffect. Soon, we'll introduce
another concrete subclass for callback-based animations leveraging the Web Animations mode: CustomEffect.
To prepare for this new class, we clean up the virtual methods on AnimationEffect such that they make a
bit more sense:
- much of the implementation of timeToNextTick() is brought from KeyframeEffect to AnimationEffect to
account for the cases which are purely based on timing properties shared by any potential AnimationEffect
subclass,
- we add a new ticksContinouslyWhileActive() method which allows subclasses to indicate whether they require
continuous scheduling while active, which is true for KeyframeEffect unless it has no keyframes, no CSS
properties set on its keyframes or is running fully accelerated,
- much of the implementation of setAnimation() is brought from KeyframeEffect to AnimationEffect since
updating the relevance of the animation based on a change of effect should apply to any AnimationEffect,
- the apply() and invalidate() methods are moved from AnimationEffect to KeyframeEffect alone since they
really are specific to KeyframeEffect's interaction with style resolution,
- all the other virtual methods on AnimationEffect have stub implementations such that new subclasses
only need to override them as required: animationDidTick(), animationDidPlay(),
animationDidChangeTimingProperties(), animationWasCanceled(), animationSuspensionStateDidChange(bool)
and animationTimelineDidChange(AnimationTimeline*).
* animation/AnimationEffect.cpp:
(WebCore::AnimationEffect::setAnimation):
(WebCore::AnimationEffect::timeToNextTick const):
* animation/AnimationEffect.h:
(WebCore::AnimationEffect::animationDidTick):
(WebCore::AnimationEffect::animationDidPlay):
(WebCore::AnimationEffect::animationDidChangeTimingProperties):
(WebCore::AnimationEffect::animationWasCanceled):
(WebCore::AnimationEffect::animationSuspensionStateDidChange):
(WebCore::AnimationEffect::animationTimelineDidChange):
(WebCore::AnimationEffect::ticksContinouslyWhileActive const):
(WebCore::AnimationEffect::setAnimation): Deleted.
(WebCore::AnimationEffect::timeToNextTick const): Deleted.
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::setAnimation):
(WebCore::KeyframeEffect::ticksContinouslyWhileActive const):
(WebCore::KeyframeEffect::timeToNextTick const):
* animation/KeyframeEffect.h:
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::invalidateEffect):
(WebCore::WebAnimation::resolve):
(WebCore::WebAnimation::timeToNextTick const):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (286543 => 286544)
--- trunk/Source/WebCore/ChangeLog 2021-12-06 14:35:01 UTC (rev 286543)
+++ trunk/Source/WebCore/ChangeLog 2021-12-06 17:20:08 UTC (rev 286544)
@@ -1,3 +1,57 @@
+2021-12-06 Antoine Quint <[email protected]>
+
+ Clean up virtual methods on AnimationEffect
+ https://bugs.webkit.org/show_bug.cgi?id=233868
+
+ Reviewed by Simon Fraser.
+
+ To this day, the only concrete subclass of AnimationEffect has been KeyframeEffect. Soon, we'll introduce
+ another concrete subclass for callback-based animations leveraging the Web Animations mode: CustomEffect.
+ To prepare for this new class, we clean up the virtual methods on AnimationEffect such that they make a
+ bit more sense:
+
+ - much of the implementation of timeToNextTick() is brought from KeyframeEffect to AnimationEffect to
+ account for the cases which are purely based on timing properties shared by any potential AnimationEffect
+ subclass,
+
+ - we add a new ticksContinouslyWhileActive() method which allows subclasses to indicate whether they require
+ continuous scheduling while active, which is true for KeyframeEffect unless it has no keyframes, no CSS
+ properties set on its keyframes or is running fully accelerated,
+
+ - much of the implementation of setAnimation() is brought from KeyframeEffect to AnimationEffect since
+ updating the relevance of the animation based on a change of effect should apply to any AnimationEffect,
+
+ - the apply() and invalidate() methods are moved from AnimationEffect to KeyframeEffect alone since they
+ really are specific to KeyframeEffect's interaction with style resolution,
+
+ - all the other virtual methods on AnimationEffect have stub implementations such that new subclasses
+ only need to override them as required: animationDidTick(), animationDidPlay(),
+ animationDidChangeTimingProperties(), animationWasCanceled(), animationSuspensionStateDidChange(bool)
+ and animationTimelineDidChange(AnimationTimeline*).
+
+ * animation/AnimationEffect.cpp:
+ (WebCore::AnimationEffect::setAnimation):
+ (WebCore::AnimationEffect::timeToNextTick const):
+ * animation/AnimationEffect.h:
+ (WebCore::AnimationEffect::animationDidTick):
+ (WebCore::AnimationEffect::animationDidPlay):
+ (WebCore::AnimationEffect::animationDidChangeTimingProperties):
+ (WebCore::AnimationEffect::animationWasCanceled):
+ (WebCore::AnimationEffect::animationSuspensionStateDidChange):
+ (WebCore::AnimationEffect::animationTimelineDidChange):
+ (WebCore::AnimationEffect::ticksContinouslyWhileActive const):
+ (WebCore::AnimationEffect::setAnimation): Deleted.
+ (WebCore::AnimationEffect::timeToNextTick const): Deleted.
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::setAnimation):
+ (WebCore::KeyframeEffect::ticksContinouslyWhileActive const):
+ (WebCore::KeyframeEffect::timeToNextTick const):
+ * animation/KeyframeEffect.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::invalidateEffect):
+ (WebCore::WebAnimation::resolve):
+ (WebCore::WebAnimation::timeToNextTick const):
+
2021-12-06 Nikolas Zimmermann <[email protected]>
[LBSE] Toggling the Settings -> Enabled LBSE flag has no immediate effect
Modified: trunk/Source/WebCore/animation/AnimationEffect.cpp (286543 => 286544)
--- trunk/Source/WebCore/animation/AnimationEffect.cpp 2021-12-06 14:35:01 UTC (rev 286543)
+++ trunk/Source/WebCore/animation/AnimationEffect.cpp 2021-12-06 17:20:08 UTC (rev 286544)
@@ -43,6 +43,16 @@
{
}
+void AnimationEffect::setAnimation(WebAnimation* animation)
+{
+ if (m_animation == animation)
+ return;
+
+ m_animation = animation;
+ if (animation)
+ animation->updateRelevance();
+}
+
EffectTiming AnimationEffect::getBindingsTiming() const
{
if (is<DeclarativeAnimation>(animation()))
@@ -551,4 +561,34 @@
return nextStepProgress - iterationProgress;
}
+Seconds AnimationEffect::timeToNextTick(BasicEffectTiming timing) const
+{
+ switch (timing.phase) {
+ case AnimationEffectPhase::Before:
+ // The effect is in its "before" phase, in this case we can wait until it enters its "active" phase.
+ return delay() - *timing.localTime;
+ case AnimationEffectPhase::Active: {
+ if (!ticksContinouslyWhileActive())
+ return endTime() - *timing.localTime;
+ if (auto iterationProgress = getComputedTiming().simpleIterationProgress) {
+ // In case we're in a range that uses a steps() timing function, we can compute the time until the next step starts.
+ if (auto progressUntilNextStep = this->progressUntilNextStep(*iterationProgress))
+ return iterationDuration() * *progressUntilNextStep;
+ }
+ // Other effects that continuously tick in the "active" phase will need to update their animated
+ // progress at the immediate next opportunity.
+ return 0_s;
+ }
+ case AnimationEffectPhase::After:
+ // The effect is in its after phase, which means it will no longer update its progress, so it doens't need a tick.
+ return Seconds::infinity();
+ case AnimationEffectPhase::Idle:
+ ASSERT_NOT_REACHED();
+ return Seconds::infinity();
+ }
+
+ ASSERT_NOT_REACHED();
+ return Seconds::infinity();
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/AnimationEffect.h (286543 => 286544)
--- trunk/Source/WebCore/animation/AnimationEffect.h 2021-12-06 14:35:01 UTC (rev 286543)
+++ trunk/Source/WebCore/animation/AnimationEffect.h 2021-12-06 17:20:08 UTC (rev 286544)
@@ -47,10 +47,6 @@
namespace WebCore {
-namespace Style {
-struct ResolutionContext;
-}
-
class AnimationEffect : public RefCounted<AnimationEffect>, public CanMakeWeakPtr<AnimationEffect> {
public:
virtual ~AnimationEffect();
@@ -65,17 +61,15 @@
ExceptionOr<void> bindingsUpdateTiming(std::optional<OptionalEffectTiming>);
ExceptionOr<void> updateTiming(std::optional<OptionalEffectTiming>);
- virtual void apply(RenderStyle& targetStyle, const Style::ResolutionContext&, std::optional<Seconds> = std::nullopt) = 0;
- virtual void invalidate() = 0;
- virtual void animationDidTick() = 0;
- virtual void animationDidPlay() = 0;
- virtual void animationDidChangeTimingProperties() = 0;
- virtual void animationWasCanceled() = 0;
- virtual void animationSuspensionStateDidChange(bool) = 0;
- virtual void animationTimelineDidChange(AnimationTimeline*) = 0;
+ virtual void animationDidTick() { };
+ virtual void animationDidPlay() { };
+ virtual void animationDidChangeTimingProperties() { };
+ virtual void animationWasCanceled() { };
+ virtual void animationSuspensionStateDidChange(bool) { };
+ virtual void animationTimelineDidChange(AnimationTimeline*) { };
WebAnimation* animation() const { return m_animation.get(); }
- virtual void setAnimation(WebAnimation* animation) { m_animation = animation; }
+ virtual void setAnimation(WebAnimation*);
Seconds delay() const { return m_delay; }
void setDelay(const Seconds&);
@@ -106,11 +100,12 @@
void updateStaticTimingProperties();
- virtual Seconds timeToNextTick() const { return Seconds::infinity(); }
+ virtual Seconds timeToNextTick(BasicEffectTiming) const;
protected:
explicit AnimationEffect();
+ virtual bool ticksContinouslyWhileActive() const { return false; }
virtual std::optional<double> progressUntilNextStep(double) const;
private:
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (286543 => 286544)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2021-12-06 14:35:01 UTC (rev 286543)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2021-12-06 17:20:08 UTC (rev 286544)
@@ -1162,13 +1162,8 @@
{
bool animationChanged = animation != this->animation();
AnimationEffect::setAnimation(animation);
-
- if (!animationChanged)
- return;
-
- if (animation)
- animation->updateRelevance();
- updateEffectStackMembership();
+ if (animationChanged)
+ updateEffectStackMembership();
}
const std::optional<const Styleable> KeyframeEffect::targetStyleable() const
@@ -2120,45 +2115,30 @@
return std::nullopt;
}
-Seconds KeyframeEffect::timeToNextTick() const
+bool KeyframeEffect::ticksContinouslyWhileActive() const
{
- auto timing = getBasicTiming();
- switch (timing.phase) {
- case AnimationEffectPhase::Before:
- // The effect is in its "before" phase, in this case we can wait until it enters its "active" phase.
- return delay() - *timing.localTime;
- case AnimationEffectPhase::Active: {
- auto doesNotAffectStyles = m_blendingKeyframes.isEmpty() || m_blendingKeyframes.properties().isEmpty();
- auto completelyAcceleratedAndRunning = isCompletelyAccelerated() && isRunningAccelerated();
- if (doesNotAffectStyles || completelyAcceleratedAndRunning) {
- // In the case of fully accelerated running effects and effects that don't actually target any CSS property,
- // we do not have a need to invalidate styles.
- if (is<CSSAnimation>(animation())) {
- // However, CSS Animations need to trigger "animationiteration" events, in this case we must wait until the next iteration.
- if (auto iterationProgress = getComputedTiming().simpleIterationProgress)
- return iterationDuration() * (1 - *iterationProgress);
- }
- // Other running effects in the "active" phase can wait until they end.
- return endTime() - *timing.localTime;
+ auto doesNotAffectStyles = m_blendingKeyframes.isEmpty() || m_blendingKeyframes.properties().isEmpty();
+ if (doesNotAffectStyles)
+ return false;
+
+ if (isCompletelyAccelerated() && isRunningAccelerated())
+ return false;
+
+ return true;
+}
+
+Seconds KeyframeEffect::timeToNextTick(BasicEffectTiming timing) const
+{
+ if (timing.phase == AnimationEffectPhase::Active) {
+ // CSS Animations need to trigger "animationiteration" events even if there is no need to
+ // update styles while animating, so if we're dealing with one we must wait until the next iteration.
+ if (!ticksContinouslyWhileActive() && is<CSSAnimation>(animation())) {
+ if (auto iterationProgress = getComputedTiming().simpleIterationProgress)
+ return iterationDuration() * (1 - *iterationProgress);
}
- if (auto iterationProgress = getComputedTiming().simpleIterationProgress) {
- // In case we're in a range that uses a steps() timing function, we can compute the time until the next step starts.
- if (auto progressUntilNextStep = this->progressUntilNextStep(*iterationProgress))
- return iterationDuration() * *progressUntilNextStep;
- }
- // Other effects in the "active" phase will need to update their animated value at the immediate next opportunity.
- return 0_s;
}
- case AnimationEffectPhase::After:
- // The effect is in its after phase, which means it will no longer update its value, so it doens't need a tick.
- return Seconds::infinity();
- case AnimationEffectPhase::Idle:
- ASSERT_NOT_REACHED();
- return Seconds::infinity();
- }
- ASSERT_NOT_REACHED();
- return Seconds::infinity();
+ return AnimationEffect::timeToNextTick(timing);
}
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/KeyframeEffect.h (286543 => 286544)
--- trunk/Source/WebCore/animation/KeyframeEffect.h 2021-12-06 14:35:01 UTC (rev 286543)
+++ trunk/Source/WebCore/animation/KeyframeEffect.h 2021-12-06 17:20:08 UTC (rev 286544)
@@ -46,6 +46,10 @@
class Element;
class FilterOperations;
+namespace Style {
+struct ResolutionContext;
+}
+
class KeyframeEffect : public AnimationEffect
, public CSSPropertyBlendingClient {
public:
@@ -54,8 +58,6 @@
static Ref<KeyframeEffect> create(const Element&, PseudoId);
~KeyframeEffect() { }
- bool isKeyframeEffect() const final { return true; }
-
struct BasePropertyIndexedKeyframe {
std::variant<std::nullptr_t, Vector<std::optional<double>>, double> offset = Vector<std::optional<double>>();
std::variant<Vector<String>, String> easing = Vector<String>();
@@ -123,14 +125,9 @@
void setComposite(CompositeOperation compositeOperation) { m_compositeOperation = compositeOperation; }
void getAnimatedStyle(std::unique_ptr<RenderStyle>& animatedStyle);
- void apply(RenderStyle& targetStyle, const Style::ResolutionContext&, std::optional<Seconds> = std::nullopt) override;
- void invalidate() override;
- void animationDidTick() final;
- void animationDidPlay() final;
- void animationDidChangeTimingProperties() final;
- void animationWasCanceled() final;
- void animationSuspensionStateDidChange(bool) final;
- void animationTimelineDidChange(AnimationTimeline*) final;
+ void apply(RenderStyle& targetStyle, const Style::ResolutionContext&, std::optional<Seconds> = std::nullopt);
+ void invalidate();
+
void animationTimingDidChange();
void transformRelatedPropertyDidChange();
OptionSet<AcceleratedActionApplicationResult> applyPendingAcceleratedActions();
@@ -137,8 +134,6 @@
void willChangeRenderer();
- void setAnimation(WebAnimation*) final;
-
RenderElement* renderer() const override;
const RenderStyle& currentStyle() const override;
bool triggersStackingContext() const { return m_triggersStackingContext; }
@@ -204,8 +199,6 @@
void computeCSSTransitionBlendingKeyframes(const RenderStyle* oldStyle, const RenderStyle& newStyle);
void computeAcceleratedPropertiesState();
void setBlendingKeyframes(KeyframeList&);
- Seconds timeToNextTick() const final;
- std::optional<double> progressUntilNextStep(double) const final;
bool isTargetingTransformRelatedProperty() const;
void checkForMatchingTransformFunctionLists();
void checkForMatchingFilterFunctionLists();
@@ -215,6 +208,19 @@
void checkForMatchingBackdropFilterFunctionLists();
#endif
+ // AnimationEffect
+ bool isKeyframeEffect() const final { return true; }
+ void animationDidTick() final;
+ void animationDidPlay() final;
+ void animationDidChangeTimingProperties() final;
+ void animationWasCanceled() final;
+ void animationSuspensionStateDidChange(bool) final;
+ void animationTimelineDidChange(AnimationTimeline*) final;
+ void setAnimation(WebAnimation*) final;
+ Seconds timeToNextTick(BasicEffectTiming) const final;
+ bool ticksContinouslyWhileActive() const final;
+ std::optional<double> progressUntilNextStep(double) const final;
+
KeyframeList m_blendingKeyframes { emptyString() };
HashSet<CSSPropertyID> m_animatedProperties;
Vector<ParsedKeyframe> m_parsedKeyframes;
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (286543 => 286544)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2021-12-06 14:35:01 UTC (rev 286543)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2021-12-06 17:20:08 UTC (rev 286544)
@@ -797,8 +797,8 @@
void WebAnimation::invalidateEffect()
{
- if (!isEffectInvalidationSuspended() && m_effect)
- m_effect->invalidate();
+ if (!isEffectInvalidationSuspended() && is<KeyframeEffect>(m_effect))
+ downcast<KeyframeEffect>(*m_effect).invalidate();
}
void WebAnimation::updateFinishedState(DidSeek didSeek, SynchronouslyNotify synchronouslyNotify)
@@ -1234,8 +1234,8 @@
updateFinishedState(DidSeek::No, SynchronouslyNotify::Yes);
m_shouldSkipUpdatingFinishedStateWhenResolving = false;
- if (m_effect)
- m_effect->apply(targetStyle, resolutionContext, startTime);
+ if (is<KeyframeEffect>(m_effect))
+ downcast<KeyframeEffect>(*m_effect).apply(targetStyle, resolutionContext, startTime);
}
void WebAnimation::setSuspended(bool isSuspended)
@@ -1453,7 +1453,7 @@
return Seconds::infinity();
ASSERT(effect());
- return effect()->timeToNextTick() / playbackRate;
+ return effect()->timeToNextTick(effect()->getBasicTiming()) / playbackRate;
}
} // namespace WebCore
