- Revision
- 240416
- Author
- [email protected]
- Date
- 2019-01-23 22:42:16 -0800 (Wed, 23 Jan 2019)
Log Message
Cherry-pick r239965. rdar://problem/47295360
Animation and other code is too aggressive about invalidating layer composition
https://bugs.webkit.org/show_bug.cgi?id=193343
Reviewed by Antoine Quint.
Source/WebCore:
We used to have the concept of a "SyntheticStyleChange", which was used to trigger
style updates for animation, and also to get compositing updated.
That morphed into a call to Element::invalidateStyleAndLayerComposition(), which causes
a style update to result in a "RecompositeLayer" diff, which in turn triggers compositing work,
and dirties DOM touch event regions (which can be expensive to update).
However, not all the callers of Element::invalidateStyleAndLayerComposition() need to trigger
compositing, and doing so from animations caused excessive touch event regions on yahoo.com,
which has several visibility:hidden elements with background-position animation.
So fix callers of invalidateStyleAndLayerComposition() which don't care about compositing to instead
call just invalidateStyle().
Also fix KeyframeAnimation::animate to correctly return true when animation state changes—it failed to
do so, because fireAnimationEventsIfNeeded() can run the state machine and change state.
* animation/KeyframeEffect.cpp:
(WebCore::invalidateElement):
* page/animation/AnimationBase.cpp:
(WebCore::AnimationBase::setNeedsStyleRecalc):
* page/animation/CSSAnimationController.cpp:
(WebCore::CSSAnimationControllerPrivate::updateAnimations):
(WebCore::CSSAnimationControllerPrivate::fireEventsAndUpdateStyle):
(WebCore::CSSAnimationControllerPrivate::pauseAnimationAtTime):
(WebCore::CSSAnimationControllerPrivate::pauseTransitionAtTime):
(WebCore::CSSAnimationController::cancelAnimations):
* page/animation/KeyframeAnimation.cpp:
(WebCore::KeyframeAnimation::animate):
* rendering/RenderImage.cpp:
(WebCore::RenderImage::imageChanged):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::calculateClipRects const):
* rendering/svg/SVGResourcesCache.cpp:
(WebCore::SVGResourcesCache::clientStyleChanged):
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::createAnimatedElementUpdate):
* svg/SVGAnimateElementBase.cpp:
(WebCore::applyCSSPropertyToTarget):
(WebCore::removeCSSPropertyFromTarget):
LayoutTests:
This test was clobbering the 'box' class on the animating element and therefore making it disappear.
* legacy-animation-engine/compositing/animation/animation-compositing.html:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239965 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-607-branch/LayoutTests/ChangeLog (240415 => 240416)
--- branches/safari-607-branch/LayoutTests/ChangeLog 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/LayoutTests/ChangeLog 2019-01-24 06:42:16 UTC (rev 240416)
@@ -1,5 +1,76 @@
2019-01-23 Alan Coon <[email protected]>
+ Cherry-pick r239965. rdar://problem/47295360
+
+ Animation and other code is too aggressive about invalidating layer composition
+ https://bugs.webkit.org/show_bug.cgi?id=193343
+
+ Reviewed by Antoine Quint.
+
+ Source/WebCore:
+
+ We used to have the concept of a "SyntheticStyleChange", which was used to trigger
+ style updates for animation, and also to get compositing updated.
+
+ That morphed into a call to Element::invalidateStyleAndLayerComposition(), which causes
+ a style update to result in a "RecompositeLayer" diff, which in turn triggers compositing work,
+ and dirties DOM touch event regions (which can be expensive to update).
+
+ However, not all the callers of Element::invalidateStyleAndLayerComposition() need to trigger
+ compositing, and doing so from animations caused excessive touch event regions on yahoo.com,
+ which has several visibility:hidden elements with background-position animation.
+
+ So fix callers of invalidateStyleAndLayerComposition() which don't care about compositing to instead
+ call just invalidateStyle().
+
+ Also fix KeyframeAnimation::animate to correctly return true when animation state changes—it failed to
+ do so, because fireAnimationEventsIfNeeded() can run the state machine and change state.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::invalidateElement):
+ * page/animation/AnimationBase.cpp:
+ (WebCore::AnimationBase::setNeedsStyleRecalc):
+ * page/animation/CSSAnimationController.cpp:
+ (WebCore::CSSAnimationControllerPrivate::updateAnimations):
+ (WebCore::CSSAnimationControllerPrivate::fireEventsAndUpdateStyle):
+ (WebCore::CSSAnimationControllerPrivate::pauseAnimationAtTime):
+ (WebCore::CSSAnimationControllerPrivate::pauseTransitionAtTime):
+ (WebCore::CSSAnimationController::cancelAnimations):
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::animate):
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::imageChanged):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::calculateClipRects const):
+ * rendering/svg/SVGResourcesCache.cpp:
+ (WebCore::SVGResourcesCache::clientStyleChanged):
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+ * svg/SVGAnimateElementBase.cpp:
+ (WebCore::applyCSSPropertyToTarget):
+ (WebCore::removeCSSPropertyFromTarget):
+
+ LayoutTests:
+
+ This test was clobbering the 'box' class on the animating element and therefore making it disappear.
+
+ * legacy-animation-engine/compositing/animation/animation-compositing.html:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239965 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-01-14 Simon Fraser <[email protected]>
+
+ Animation and other code is too aggressive about invalidating layer composition
+ https://bugs.webkit.org/show_bug.cgi?id=193343
+
+ Reviewed by Antoine Quint.
+
+ This test was clobbering the 'box' class on the animating element and therefore making it disappear.
+
+ * legacy-animation-engine/compositing/animation/animation-compositing.html:
+
+2019-01-23 Alan Coon <[email protected]>
+
Cherry-pick r239905. rdar://problem/47494732
Release assert with <img usemap> in shadow tree
Modified: branches/safari-607-branch/LayoutTests/legacy-animation-engine/compositing/animation/animation-compositing.html (240415 => 240416)
--- branches/safari-607-branch/LayoutTests/legacy-animation-engine/compositing/animation/animation-compositing.html 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/LayoutTests/legacy-animation-engine/compositing/animation/animation-compositing.html 2019-01-24 06:42:16 UTC (rev 240416)
@@ -39,7 +39,7 @@
testRunner.notifyDone();
}
}, false);
- document.getElementById('box').className = 'spinning';
+ document.getElementById('box').classList.add('spinning');
}
window.addEventListener('load', doTest, false);
Modified: branches/safari-607-branch/Source/WebCore/ChangeLog (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/ChangeLog 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/ChangeLog 2019-01-24 06:42:16 UTC (rev 240416)
@@ -1,5 +1,113 @@
2019-01-23 Alan Coon <[email protected]>
+ Cherry-pick r239965. rdar://problem/47295360
+
+ Animation and other code is too aggressive about invalidating layer composition
+ https://bugs.webkit.org/show_bug.cgi?id=193343
+
+ Reviewed by Antoine Quint.
+
+ Source/WebCore:
+
+ We used to have the concept of a "SyntheticStyleChange", which was used to trigger
+ style updates for animation, and also to get compositing updated.
+
+ That morphed into a call to Element::invalidateStyleAndLayerComposition(), which causes
+ a style update to result in a "RecompositeLayer" diff, which in turn triggers compositing work,
+ and dirties DOM touch event regions (which can be expensive to update).
+
+ However, not all the callers of Element::invalidateStyleAndLayerComposition() need to trigger
+ compositing, and doing so from animations caused excessive touch event regions on yahoo.com,
+ which has several visibility:hidden elements with background-position animation.
+
+ So fix callers of invalidateStyleAndLayerComposition() which don't care about compositing to instead
+ call just invalidateStyle().
+
+ Also fix KeyframeAnimation::animate to correctly return true when animation state changes—it failed to
+ do so, because fireAnimationEventsIfNeeded() can run the state machine and change state.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::invalidateElement):
+ * page/animation/AnimationBase.cpp:
+ (WebCore::AnimationBase::setNeedsStyleRecalc):
+ * page/animation/CSSAnimationController.cpp:
+ (WebCore::CSSAnimationControllerPrivate::updateAnimations):
+ (WebCore::CSSAnimationControllerPrivate::fireEventsAndUpdateStyle):
+ (WebCore::CSSAnimationControllerPrivate::pauseAnimationAtTime):
+ (WebCore::CSSAnimationControllerPrivate::pauseTransitionAtTime):
+ (WebCore::CSSAnimationController::cancelAnimations):
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::animate):
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::imageChanged):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::calculateClipRects const):
+ * rendering/svg/SVGResourcesCache.cpp:
+ (WebCore::SVGResourcesCache::clientStyleChanged):
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+ * svg/SVGAnimateElementBase.cpp:
+ (WebCore::applyCSSPropertyToTarget):
+ (WebCore::removeCSSPropertyFromTarget):
+
+ LayoutTests:
+
+ This test was clobbering the 'box' class on the animating element and therefore making it disappear.
+
+ * legacy-animation-engine/compositing/animation/animation-compositing.html:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239965 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-01-14 Simon Fraser <[email protected]>
+
+ Animation and other code is too aggressive about invalidating layer composition
+ https://bugs.webkit.org/show_bug.cgi?id=193343
+
+ Reviewed by Antoine Quint.
+
+ We used to have the concept of a "SyntheticStyleChange", which was used to trigger
+ style updates for animation, and also to get compositing updated.
+
+ That morphed into a call to Element::invalidateStyleAndLayerComposition(), which causes
+ a style update to result in a "RecompositeLayer" diff, which in turn triggers compositing work,
+ and dirties DOM touch event regions (which can be expensive to update).
+
+ However, not all the callers of Element::invalidateStyleAndLayerComposition() need to trigger
+ compositing, and doing so from animations caused excessive touch event regions on yahoo.com,
+ which has several visibility:hidden elements with background-position animation.
+
+ So fix callers of invalidateStyleAndLayerComposition() which don't care about compositing to instead
+ call just invalidateStyle().
+
+ Also fix KeyframeAnimation::animate to correctly return true when animation state changes—it failed to
+ do so, because fireAnimationEventsIfNeeded() can run the state machine and change state.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::invalidateElement):
+ * page/animation/AnimationBase.cpp:
+ (WebCore::AnimationBase::setNeedsStyleRecalc):
+ * page/animation/CSSAnimationController.cpp:
+ (WebCore::CSSAnimationControllerPrivate::updateAnimations):
+ (WebCore::CSSAnimationControllerPrivate::fireEventsAndUpdateStyle):
+ (WebCore::CSSAnimationControllerPrivate::pauseAnimationAtTime):
+ (WebCore::CSSAnimationControllerPrivate::pauseTransitionAtTime):
+ (WebCore::CSSAnimationController::cancelAnimations):
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::animate):
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::imageChanged):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::calculateClipRects const):
+ * rendering/svg/SVGResourcesCache.cpp:
+ (WebCore::SVGResourcesCache::clientStyleChanged):
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+ * svg/SVGAnimateElementBase.cpp:
+ (WebCore::applyCSSPropertyToTarget):
+ (WebCore::removeCSSPropertyFromTarget):
+
+2019-01-23 Alan Coon <[email protected]>
+
Cherry-pick r239947. rdar://problem/47295371
Re-enable ability to build --cloop builds.
Modified: branches/safari-607-branch/Source/WebCore/animation/KeyframeEffect.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/animation/KeyframeEffect.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/animation/KeyframeEffect.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -60,7 +60,7 @@
static inline void invalidateElement(Element* element)
{
if (element)
- element->invalidateStyleAndLayerComposition();
+ element->invalidateStyle();
}
static inline String CSSPropertyIDToIDLAttributeName(CSSPropertyID cssPropertyId)
Modified: branches/safari-607-branch/Source/WebCore/page/animation/AnimationBase.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/page/animation/AnimationBase.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/page/animation/AnimationBase.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -90,7 +90,7 @@
return;
ASSERT(element->document().pageCacheState() == Document::NotInPageCache);
- element->invalidateStyleAndLayerComposition();
+ element->invalidateStyle();
}
double AnimationBase::duration() const
Modified: branches/safari-607-branch/Source/WebCore/page/animation/CSSAnimationController.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/page/animation/CSSAnimationController.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/page/animation/CSSAnimationController.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -143,9 +143,10 @@
if (timeToNextService && timeToNextService.value() == 0_s) {
if (callSetChanged != CallSetChanged)
break;
+
Element& element = *compositeAnimation.key;
ASSERT(element.document().pageCacheState() == Document::NotInPageCache);
- element.invalidateStyleAndLayerComposition();
+ element.invalidateStyle();
calledSetChanged = true;
}
}
@@ -225,7 +226,7 @@
}
for (auto& change : m_elementChangesToDispatch)
- change->invalidateStyleAndLayerComposition();
+ change->invalidateStyle();
m_elementChangesToDispatch.clear();
@@ -412,7 +413,7 @@
{
CompositeAnimation& compositeAnimation = ensureCompositeAnimation(element);
if (compositeAnimation.pauseAnimationAtTime(name, t)) {
- element.invalidateStyleAndLayerComposition();
+ element.invalidateStyle();
startUpdateStyleIfNeededDispatcher();
return true;
}
@@ -424,7 +425,7 @@
{
CompositeAnimation& compositeAnimation = ensureCompositeAnimation(element);
if (compositeAnimation.pauseTransitionAtTime(cssPropertyID(property), t)) {
- element.invalidateStyleAndLayerComposition();
+ element.invalidateStyle();
startUpdateStyleIfNeededDispatcher();
return true;
}
@@ -607,7 +608,7 @@
if (element.document().renderTreeBeingDestroyed())
return;
ASSERT(element.document().pageCacheState() == Document::NotInPageCache);
- element.invalidateStyleAndLayerComposition();
+ element.invalidateStyle();
}
AnimationUpdate CSSAnimationController::updateAnimations(Element& element, const RenderStyle& newStyle, const RenderStyle* oldStyle)
Modified: branches/safari-607-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -158,9 +158,11 @@
bool KeyframeAnimation::animate(CompositeAnimation& compositeAnimation, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
{
- // Fire the start timeout if needed
+ AnimationState oldState = state();
+
+ // Update state and fire the start timeout if needed (FIXME: this function needs a better name).
fireAnimationEventsIfNeeded();
-
+
// If we have not yet started, we will not have a valid start time, so just start the animation if needed.
if (isNew()) {
if (m_animation->playState() == AnimationPlayState::Playing && !compositeAnimation.isSuspended())
@@ -191,9 +193,6 @@
return false;
}
- // FIXME: the code below never changes the state, so this function always returns false.
- AnimationState oldState = state();
-
// Run a cycle of animation.
// We know we will need a new render style, so make one if needed.
if (!animatedStyle)
Modified: branches/safari-607-branch/Source/WebCore/rendering/RenderImage.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/rendering/RenderImage.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/rendering/RenderImage.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -284,7 +284,7 @@
ASSERT(element());
if (element()) {
m_needsToSetSizeForAltText = true;
- element()->invalidateStyleAndLayerComposition();
+ element()->invalidateStyle();
}
return;
}
Modified: branches/safari-607-branch/Source/WebCore/rendering/RenderLayer.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/rendering/RenderLayer.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/rendering/RenderLayer.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -6599,8 +6599,10 @@
void RenderLayer::filterNeedsRepaint()
{
// We use the enclosing element so that we recalculate style for the ancestor of an anonymous object.
- if (Element* element = enclosingElement())
+ if (Element* element = enclosingElement()) {
+ // FIXME: This really shouldn't have to invalidate layer composition, but tests like css3/filters/effect-reference-delete.html fail if that doesn't happen.
element->invalidateStyleAndLayerComposition();
+ }
renderer().repaint();
}
Modified: branches/safari-607-branch/Source/WebCore/rendering/svg/SVGResourcesCache.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/rendering/svg/SVGResourcesCache.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/rendering/svg/SVGResourcesCache.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -116,7 +116,7 @@
RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer, false);
if (renderer.element() && !renderer.element()->isSVGElement())
- renderer.element()->invalidateStyleAndLayerComposition();
+ renderer.element()->invalidateStyle();
}
void SVGResourcesCache::clientWasAddedToTree(RenderObject& renderer)
Modified: branches/safari-607-branch/Source/WebCore/style/StyleTreeResolver.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/style/StyleTreeResolver.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/style/StyleTreeResolver.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -310,7 +310,7 @@
auto& animationController = m_document.frame()->animation();
auto animationUpdate = animationController.updateAnimations(element, *newStyle, oldStyle);
- shouldRecompositeLayer = animationUpdate.stateChanged;
+ shouldRecompositeLayer = animationUpdate.stateChanged; // FIXME: constrain this to just property animations triggering acceleration.
if (animationUpdate.style)
newStyle = WTFMove(animationUpdate.style);
Modified: branches/safari-607-branch/Source/WebCore/svg/SVGAnimateElementBase.cpp (240415 => 240416)
--- branches/safari-607-branch/Source/WebCore/svg/SVGAnimateElementBase.cpp 2019-01-24 06:42:10 UTC (rev 240415)
+++ branches/safari-607-branch/Source/WebCore/svg/SVGAnimateElementBase.cpp 2019-01-24 06:42:16 UTC (rev 240416)
@@ -249,7 +249,7 @@
if (!targetElement.ensureAnimatedSMILStyleProperties().setProperty(id, value, false))
return;
- targetElement.invalidateStyleAndLayerComposition();
+ targetElement.invalidateStyle();
}
static inline void removeCSSPropertyFromTarget(SVGElement& targetElement, CSSPropertyID id)
@@ -256,7 +256,7 @@
{
ASSERT(!targetElement.m_deletionHasBegun);
targetElement.ensureAnimatedSMILStyleProperties().removeProperty(id);
- targetElement.invalidateStyleAndLayerComposition();
+ targetElement.invalidateStyle();
}
static inline void applyCSSPropertyToTargetAndInstances(SVGElement& targetElement, const QualifiedName& attributeName, const String& valueAsString)
