Title: [170656] trunk
Revision
170656
Author
simon.fra...@apple.com
Date
2014-07-01 12:15:39 -0700 (Tue, 01 Jul 2014)

Log Message

[UI-side compositing] Bad spinner on news.google.com: animations need to be ordered
https://bugs.webkit.org/show_bug.cgi?id=134504
<rdar://problem/17507892>

Reviewed by Tim Horton.

Source/WebKit2:
The layer's addedAnimations property needs to maintain order, since the order
in which transforms are applied is important.

* Shared/mac/RemoteLayerTreeTransaction.h: Use a Vector<pair<>> for addedAnimations.
* Shared/mac/RemoteLayerTreeTransaction.mm:
(WebKit::dumpChangedLayers):
* WebProcess/WebPage/mac/PlatformCAAnimationRemote.h:
* WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:
(WebKit::PlatformCAAnimationRemote::updateLayerAnimations):
* WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:
(WebKit::PlatformCALayerRemote::addAnimationForKey): If this is a new entry, we
can just append to addedAnimations, otherwise we have to find the existing one
and update its properties.
(WebKit::PlatformCALayerRemote::removeAnimationForKey): Do linear search to
find the animation to remove (this list will normally be short).

LayoutTests:
Test that the transforms from the animation are applied in the correct order.

* compositing/animation/keyframe-order-expected.html: Added.
* compositing/animation/keyframe-order.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (170655 => 170656)


--- trunk/LayoutTests/ChangeLog	2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/LayoutTests/ChangeLog	2014-07-01 19:15:39 UTC (rev 170656)
@@ -1,3 +1,16 @@
+2014-07-01  Simon Fraser  <simon.fra...@apple.com>
+
+        [UI-side compositing] Bad spinner on news.google.com: animations need to be ordered
+        https://bugs.webkit.org/show_bug.cgi?id=134504
+        <rdar://problem/17507892>
+
+        Reviewed by Tim Horton.
+        
+        Test that the transforms from the animation are applied in the correct order.
+
+        * compositing/animation/keyframe-order-expected.html: Added.
+        * compositing/animation/keyframe-order.html: Added.
+
 2014-07-01  Zalan Bujtas  <za...@apple.com>
 
         Subpixel rendering: Pixel crack in breadcrumbs at devforums.apple.com.

Added: trunk/LayoutTests/compositing/animation/keyframe-order-expected.html (0 => 170656)


--- trunk/LayoutTests/compositing/animation/keyframe-order-expected.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/animation/keyframe-order-expected.html	2014-07-01 19:15:39 UTC (rev 170656)
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    * {
+        box-sizing: border-box;
+    }
+    .container {
+        position: absolute;
+        height: 100px;
+        width: 100px;
+        background-color: green;
+        margin: 100px;
+    }
+
+    .spinner {
+        position: absolute;
+        top: -50px;
+        left: -50px;
+        -webkit-transform: rotateZ(0deg) translate3d(50px, 50px, 0);
+    }
+
+    .inner {
+        width: 50px;
+        height: 50px;
+/*        overflow: hidden*/
+    }
+
+    .bordered {
+        width: 100px;
+        height: 100px;
+        border: 20px solid black;
+    }
+    </style>
+</head>
+<body>
+
+<div class="container">
+    <div class="spinner">
+        <div class="inner">
+            <div class="bordered"></div>
+        </div>
+    </div>
+</div>
+
+</body>
+</html>

Added: trunk/LayoutTests/compositing/animation/keyframe-order.html (0 => 170656)


--- trunk/LayoutTests/compositing/animation/keyframe-order.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/animation/keyframe-order.html	2014-07-01 19:15:39 UTC (rev 170656)
@@ -0,0 +1,76 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    * {
+        box-sizing: border-box;
+    }
+    .container {
+        position: absolute;
+        height: 100px;
+        width: 100px;
+        background-color: green;
+        margin: 100px;
+    }
+
+    .spinner {
+        position: absolute;
+        top: -50px;
+        left: -50px;
+    }
+    
+    .spinner.animating {
+        -webkit-animation: imageSpin 999999999s linear 0 infinite
+    }
+
+    .inner {
+        width: 50px;
+        height: 50px;
+    }
+
+    .bordered {
+        width: 100px;
+        height: 100px;
+        border: 20px solid black;
+    }
+
+    @-webkit-keyframes imageSpin {
+        from {
+            -webkit-transform: rotateZ(0deg) translate3d(50px, 50px, 0)
+        }
+
+        to {
+            -webkit-transform: rotateZ(360deg) translate3d(50px, 50px 0)
+        }
+    }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+        
+        function doTest()
+        {
+            var box = document.getElementById('spinner');
+            box.addEventListener('webkitAnimationEnd', function() {
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, false);
+            box.classList.add('animating');
+        }
+
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+
+<div class="container">
+    <div id="spinner" class="spinner">
+        <div class="inner">
+            <div class="bordered"></div>
+        </div>
+    </div>
+</div>
+
+</body>
+</html>

Modified: trunk/Source/WebKit2/ChangeLog (170655 => 170656)


--- trunk/Source/WebKit2/ChangeLog	2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/Source/WebKit2/ChangeLog	2014-07-01 19:15:39 UTC (rev 170656)
@@ -1,3 +1,27 @@
+2014-07-01  Simon Fraser  <simon.fra...@apple.com>
+
+        [UI-side compositing] Bad spinner on news.google.com: animations need to be ordered
+        https://bugs.webkit.org/show_bug.cgi?id=134504
+        <rdar://problem/17507892>
+
+        Reviewed by Tim Horton.
+        
+        The layer's addedAnimations property needs to maintain order, since the order
+        in which transforms are applied is important.
+
+        * Shared/mac/RemoteLayerTreeTransaction.h: Use a Vector<pair<>> for addedAnimations.
+        * Shared/mac/RemoteLayerTreeTransaction.mm:
+        (WebKit::dumpChangedLayers):
+        * WebProcess/WebPage/mac/PlatformCAAnimationRemote.h:
+        * WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:
+        (WebKit::PlatformCAAnimationRemote::updateLayerAnimations):
+        * WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:
+        (WebKit::PlatformCALayerRemote::addAnimationForKey): If this is a new entry, we
+        can just append to addedAnimations, otherwise we have to find the existing one
+        and update its properties.
+        (WebKit::PlatformCALayerRemote::removeAnimationForKey): Do linear search to
+        find the animation to remove (this list will normally be short).
+
 2014-07-01  Anders Carlsson  <ander...@apple.com>
 
         Add a encodeLegacySessionState function

Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h (170655 => 170656)


--- trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h	2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h	2014-07-01 19:15:39 UTC (rev 170656)
@@ -123,7 +123,7 @@
         std::unique_ptr<WebCore::TransformationMatrix> sublayerTransform;
         Vector<WebCore::GraphicsLayer::PlatformLayerID> children;
 
-        HashMap<String, PlatformCAAnimationRemote::Properties> addedAnimations;
+        Vector<std::pair<String, PlatformCAAnimationRemote::Properties>> addedAnimations;
         HashSet<String> keyPathsOfAnimationsToRemove;
 
         WebCore::FloatPoint3D position;

Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm (170655 => 170656)


--- trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm	2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm	2014-07-01 19:15:39 UTC (rev 170656)
@@ -1103,8 +1103,8 @@
             dumpProperty(ts, "filters", layerProperties.filters ? *layerProperties.filters : FilterOperations());
 
         if (layerProperties.changedProperties & RemoteLayerTreeTransaction::AnimationsChanged) {
-            for (const auto& keyValuePair : layerProperties.addedAnimations)
-                dumpProperty(ts, "animation " +  keyValuePair.key, keyValuePair.value);
+            for (const auto& keyAnimationPair : layerProperties.addedAnimations)
+                dumpProperty(ts, "animation " +  keyAnimationPair.first, keyAnimationPair.second);
 
             for (const auto& name : layerProperties.keyPathsOfAnimationsToRemove)
                 dumpProperty(ts, "removed animation", name);

Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.h (170655 => 170656)


--- trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.h	2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.h	2014-07-01 19:15:39 UTC (rev 170656)
@@ -294,8 +294,8 @@
 
     const Properties& properties() const { return m_properties; }
 
-    typedef HashMap<String, Properties> AnimationsMap;
-    static void updateLayerAnimations(CALayer *, RemoteLayerTreeHost*, const AnimationsMap& animationsToAdd, const HashSet<String>& animationsToRemove);
+    typedef Vector<std::pair<String, Properties>> AnimationsList;
+    static void updateLayerAnimations(CALayer *, RemoteLayerTreeHost*, const AnimationsList& animationsToAdd, const HashSet<String>& animationsToRemove);
 
 private:
     PlatformCAAnimationRemote(AnimationType, const String& keyPath);

Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm (170655 => 170656)


--- trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm	2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm	2014-07-01 19:15:39 UTC (rev 170656)
@@ -775,15 +775,15 @@
     [layer addAnimation:caAnimation.get() forKey:key];
 }
 
-void PlatformCAAnimationRemote::updateLayerAnimations(CALayer *layer, RemoteLayerTreeHost* layerTreeHost, const AnimationsMap& animationsToAdd, const HashSet<String>& animationsToRemove)
+void PlatformCAAnimationRemote::updateLayerAnimations(CALayer *layer, RemoteLayerTreeHost* layerTreeHost, const AnimationsList& animationsToAdd, const HashSet<String>& animationsToRemove)
 {
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
 
     for (const auto& value : animationsToRemove)
         [layer removeAnimationForKey:value];
 
-    for (const auto& keyValuePair : animationsToAdd)
-        addAnimationToLayer(layer, layerTreeHost, keyValuePair.key, keyValuePair.value);
+    for (const auto& keyAnimationPair : animationsToAdd)
+        addAnimationToLayer(layer, layerTreeHost, keyAnimationPair.first, keyAnimationPair.second);
 
     END_BLOCK_OBJC_EXCEPTIONS;
 }

Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp (170655 => 170656)


--- trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp	2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp	2014-07-01 19:15:39 UTC (rev 170656)
@@ -301,8 +301,18 @@
 
 void PlatformCALayerRemote::addAnimationForKey(const String& key, PlatformCAAnimation* animation)
 {
-    m_animations.set(key, animation);
-    m_properties.addedAnimations.set(key, toPlatformCAAnimationRemote(animation)->properties());
+    auto addResult = m_animations.set(key, animation);
+    if (addResult.isNewEntry)
+        m_properties.addedAnimations.append(std::pair<String, PlatformCAAnimationRemote::Properties>(key, toPlatformCAAnimationRemote(animation)->properties()));
+    else {
+        for (auto& keyAnimationPair : m_properties.addedAnimations) {
+            if (keyAnimationPair.first == key) {
+                keyAnimationPair.second = toPlatformCAAnimationRemote(animation)->properties();
+                break;
+            }
+        }
+    }
+    
     m_properties.notePropertiesChanged(RemoteLayerTreeTransaction::AnimationsChanged);
 
     if (m_context)
@@ -311,8 +321,14 @@
 
 void PlatformCALayerRemote::removeAnimationForKey(const String& key)
 {
-    m_animations.remove(key);
-    m_properties.addedAnimations.remove(key);
+    if (m_animations.remove(key)) {
+        for (size_t i = 0; i < m_properties.addedAnimations.size(); ++i) {
+            if (m_properties.addedAnimations[i].first == key) {
+                m_properties.addedAnimations.remove(i);
+                break;
+            }
+        }
+    }
     m_properties.keyPathsOfAnimationsToRemove.add(key);
     m_properties.notePropertiesChanged(RemoteLayerTreeTransaction::AnimationsChanged);
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to