- Revision
- 234250
- Author
- [email protected]
- Date
- 2018-07-26 01:33:31 -0700 (Thu, 26 Jul 2018)
Log Message
[Web Animations] REGRESSION: transition added immediately after element creation doesn't work
https://bugs.webkit.org/show_bug.cgi?id=187942
Reviewed by Dean Jackson.
Source/WebCore:
Tests: webanimations/accelerated-transition-by-removing-property.html
webanimations/partly-accelerated-transition-by-removing-property.html
The Style::TreeResolver::createAnimatedElementUpdate() function expected a flag to be set that indicates that
running animations on an element should yield a change in composited state for that element's layer. We did not
have code accounting for this flag in the Web Animations engine. We now have a resolveAnimationsForElement()
method on DocumentTimeline which looks at all animations resolved on the element and see if all of them are
running accelerated and whether at least one of them is pending. In that case, we set the shouldRecompositeLayer
flag in createAnimatedElementUpdate() to true which guarantees the element's layer will have a backing when
we attempt to start the animation in KeyframeEffectReadOnly::applyPendingAcceleratedActions() where we would
have previously failed to have layer-backed renderer to perform an accelerated animation on (under certain
circumstances, see test).
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::resolveAnimationsForElement):
* animation/DocumentTimeline.h:
* animation/KeyframeEffectReadOnly.h:
(WebCore::KeyframeEffectReadOnly::hasPendingAcceleratedAction const):
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::createAnimatedElementUpdate):
LayoutTests:
Creating a new test that runs a transition based on an explicit value being removed in favor
of the implicit value of a property that can be accelerated. To check that we indeed run the
animation, we have a cache that covers the entire range of interpolated values except for the
start and end values and wait 100ms after creating the transition to end the test. Prior to this
patch, the element would be at its start value and a 1px red line would show to the right of the
cache. With this patch, the red line is hidden by the cache as it's animated.
We also add a test that checks that we do not create a composited layer when several transitions,
with only one being potentially accelerated, target the same element.
* webanimations/accelerated-transition-by-removing-property-expected.html: Added.
* webanimations/accelerated-transition-by-removing-property.html: Added.
* webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Added.
* webanimations/partly-accelerated-transition-by-removing-property.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (234249 => 234250)
--- trunk/LayoutTests/ChangeLog 2018-07-26 08:04:03 UTC (rev 234249)
+++ trunk/LayoutTests/ChangeLog 2018-07-26 08:33:31 UTC (rev 234250)
@@ -1,3 +1,25 @@
+2018-07-26 Antoine Quint <[email protected]>
+
+ [Web Animations] REGRESSION: transition added immediately after element creation doesn't work
+ https://bugs.webkit.org/show_bug.cgi?id=187942
+
+ Reviewed by Dean Jackson.
+
+ Creating a new test that runs a transition based on an explicit value being removed in favor
+ of the implicit value of a property that can be accelerated. To check that we indeed run the
+ animation, we have a cache that covers the entire range of interpolated values except for the
+ start and end values and wait 100ms after creating the transition to end the test. Prior to this
+ patch, the element would be at its start value and a 1px red line would show to the right of the
+ cache. With this patch, the red line is hidden by the cache as it's animated.
+
+ We also add a test that checks that we do not create a composited layer when several transitions,
+ with only one being potentially accelerated, target the same element.
+
+ * webanimations/accelerated-transition-by-removing-property-expected.html: Added.
+ * webanimations/accelerated-transition-by-removing-property.html: Added.
+ * webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Added.
+ * webanimations/partly-accelerated-transition-by-removing-property.html: Added.
+
2018-07-26 Basuke Suzuki <[email protected]>
[Curl] Test gardening
Added: trunk/LayoutTests/webanimations/accelerated-transition-by-removing-property-expected.html (0 => 234250)
--- trunk/LayoutTests/webanimations/accelerated-transition-by-removing-property-expected.html (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-transition-by-removing-property-expected.html 2018-07-26 08:33:31 UTC (rev 234250)
@@ -0,0 +1 @@
+<div style="position: absolute; top: 0; left: 1px; width: 798px; height: 100px; background-color: black;"></div>
\ No newline at end of file
Added: trunk/LayoutTests/webanimations/accelerated-transition-by-removing-property.html (0 => 234250)
--- trunk/LayoutTests/webanimations/accelerated-transition-by-removing-property.html (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-transition-by-removing-property.html 2018-07-26 08:33:31 UTC (rev 234250)
@@ -0,0 +1,29 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
+<body>
+<style>
+ div {
+ position: absolute;
+ height: 100px;
+ top: 0;
+ }
+</style>
+<div id="target" style="left: 0; width: 1px; background-color: red;"></div>
+<div style="left: 1px; width: 798px; background-color: black;"></div>
+<script>
+
+if (window.testRunner)
+ testRunner.waitUntilDone();
+
+const target = document.getElementById("target");
+target.style.transform = "translateX(799px)";
+setTimeout(() => {
+ target.style.removeProperty("transform");
+ target.style.transition = "transform 10s linear";
+ setTimeout(() => {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 100);
+});
+
+</script>
+</body>
Added: trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property-expected.txt (0 => 234250)
--- trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property-expected.txt 2018-07-26 08:33:31 UTC (rev 234250)
@@ -0,0 +1 @@
+
Added: trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property.html (0 => 234250)
--- trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property.html (rev 0)
+++ trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property.html 2018-07-26 08:33:31 UTC (rev 234250)
@@ -0,0 +1,29 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
+<body>
+<pre id="results"></pre>
+<div id="target" style="width: 100px; height: 100px; background-color: black;"></div>
+<script>
+
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+}
+
+const target = document.getElementById("target");
+target.style.opacity = "0.5";
+target.style.marginLeft = "100px";
+setTimeout(() => {
+ target.style.removeProperty("opacity");
+ target.style.removeProperty("margin-left");
+ target.style.transitionProperty = "margin-left opacity";
+ target.style.transitionDuration = "1s";
+ requestAnimationFrame(() => {
+ if (window.internals)
+ document.getElementById("results").innerText = internals.layerTreeAsText(document);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+});
+
+</script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (234249 => 234250)
--- trunk/Source/WebCore/ChangeLog 2018-07-26 08:04:03 UTC (rev 234249)
+++ trunk/Source/WebCore/ChangeLog 2018-07-26 08:33:31 UTC (rev 234250)
@@ -1,3 +1,31 @@
+2018-07-26 Antoine Quint <[email protected]>
+
+ [Web Animations] REGRESSION: transition added immediately after element creation doesn't work
+ https://bugs.webkit.org/show_bug.cgi?id=187942
+
+ Reviewed by Dean Jackson.
+
+ Tests: webanimations/accelerated-transition-by-removing-property.html
+ webanimations/partly-accelerated-transition-by-removing-property.html
+
+ The Style::TreeResolver::createAnimatedElementUpdate() function expected a flag to be set that indicates that
+ running animations on an element should yield a change in composited state for that element's layer. We did not
+ have code accounting for this flag in the Web Animations engine. We now have a resolveAnimationsForElement()
+ method on DocumentTimeline which looks at all animations resolved on the element and see if all of them are
+ running accelerated and whether at least one of them is pending. In that case, we set the shouldRecompositeLayer
+ flag in createAnimatedElementUpdate() to true which guarantees the element's layer will have a backing when
+ we attempt to start the animation in KeyframeEffectReadOnly::applyPendingAcceleratedActions() where we would
+ have previously failed to have layer-backed renderer to perform an accelerated animation on (under certain
+ circumstances, see test).
+
+ * animation/DocumentTimeline.cpp:
+ (WebCore::DocumentTimeline::resolveAnimationsForElement):
+ * animation/DocumentTimeline.h:
+ * animation/KeyframeEffectReadOnly.h:
+ (WebCore::KeyframeEffectReadOnly::hasPendingAcceleratedAction const):
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+
2018-07-25 Chris Dumez <[email protected]>
Allow ActiveDOMObject's canSuspend / suspend / resume overrides to destroy ActiveDOMObjects
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (234249 => 234250)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-07-26 08:04:03 UTC (rev 234249)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-07-26 08:33:31 UTC (rev 234250)
@@ -27,6 +27,7 @@
#include "DocumentTimeline.h"
#include "AnimationPlaybackEvent.h"
+#include "CSSPropertyAnimation.h"
#include "DOMWindow.h"
#include "DeclarativeAnimation.h"
#include "Document.h"
@@ -409,6 +410,34 @@
m_acceleratedAnimationsPendingRunningStateChange.clear();
}
+bool DocumentTimeline::resolveAnimationsForElement(Element& element, RenderStyle& targetStyle)
+{
+ bool hasNonAcceleratedAnimations = false;
+ bool hasPendingAcceleratedAnimations = true;
+ for (const auto& animation : animationsForElement(element)) {
+ animation->resolve(targetStyle);
+ if (!hasNonAcceleratedAnimations) {
+ if (auto* effect = animation->effect()) {
+ if (is<KeyframeEffectReadOnly>(effect)) {
+ auto* keyframeEffect = downcast<KeyframeEffectReadOnly>(effect);
+ for (auto cssPropertyId : keyframeEffect->animatedProperties()) {
+ if (!CSSPropertyAnimation::animationOfPropertyIsAccelerated(cssPropertyId)) {
+ hasNonAcceleratedAnimations = true;
+ continue;
+ }
+ if (!hasPendingAcceleratedAnimations)
+ hasPendingAcceleratedAnimations = keyframeEffect->hasPendingAcceleratedAction();
+ }
+ }
+ }
+ }
+ }
+
+ // If there are no non-accelerated animations and we've encountered at least one pending
+ // accelerated animation, we should recomposite this element's layer for animation purposes.
+ return !hasNonAcceleratedAnimations && hasPendingAcceleratedAnimations;
+}
+
bool DocumentTimeline::runningAnimationsForElementAreAllAccelerated(Element& element)
{
// FIXME: This will let animations run using hardware compositing even if later in the active
Modified: trunk/Source/WebCore/animation/DocumentTimeline.h (234249 => 234250)
--- trunk/Source/WebCore/animation/DocumentTimeline.h 2018-07-26 08:04:03 UTC (rev 234249)
+++ trunk/Source/WebCore/animation/DocumentTimeline.h 2018-07-26 08:33:31 UTC (rev 234250)
@@ -60,6 +60,7 @@
void animationAcceleratedRunningStateDidChange(WebAnimation&);
void applyPendingAcceleratedAnimations();
bool runningAnimationsForElementAreAllAccelerated(Element&);
+ bool resolveAnimationsForElement(Element&, RenderStyle&);
void detachFromDocument();
void enqueueAnimationPlaybackEvent(AnimationPlaybackEvent&);
Modified: trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h (234249 => 234250)
--- trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h 2018-07-26 08:04:03 UTC (rev 234249)
+++ trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h 2018-07-26 08:33:31 UTC (rev 234250)
@@ -108,6 +108,7 @@
void animationSuspensionStateDidChange(bool) final;
void applyPendingAcceleratedActions();
bool isRunningAccelerated() const { return m_lastRecordedAcceleratedAction != AcceleratedAction::Stop; }
+ bool hasPendingAcceleratedAction() const { return !m_pendingAcceleratedActions.isEmpty() && isRunningAccelerated(); }
RenderElement* renderer() const override;
const RenderStyle& currentStyle() const override;
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (234249 => 234250)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2018-07-26 08:04:03 UTC (rev 234249)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2018-07-26 08:33:31 UTC (rev 234250)
@@ -286,6 +286,8 @@
{
auto* oldStyle = renderOrDisplayContentsStyle(element);
+ bool shouldRecompositeLayer = false;
+
// New code path for CSS Animations and CSS Transitions.
if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled()) {
// First, we need to make sure that any new CSS animation occuring on this element has a matching WebAnimation
@@ -303,17 +305,11 @@
if (auto timeline = m_document.existingTimeline()) {
// Now we can update all Web animations, which will include CSS Animations as well
// as animations created via the JS API.
- auto webAnimations = timeline->animationsForElement(element);
- if (!webAnimations.isEmpty()) {
- auto animatedStyle = RenderStyle::clonePtr(*newStyle);
- for (const auto& animation : webAnimations)
- animation->resolve(*animatedStyle);
- newStyle = WTFMove(animatedStyle);
- }
+ auto animatedStyle = RenderStyle::clonePtr(*newStyle);
+ shouldRecompositeLayer = timeline->resolveAnimationsForElement(element, *animatedStyle);
+ newStyle = WTFMove(animatedStyle);
}
- bool shouldRecompositeLayer = false;
-
// Old code path for CSS Animations and CSS Transitions.
if (!RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled()) {
auto& animationController = m_document.frame()->animation();
