Changeset 170656 in webkit

Timestamp:

Jul 1, 2014 12:15:39 PM (10 years ago)

Author:

Simon Fraser

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/compositing/animation/keyframe-order-expected.html (added)
• LayoutTests/compositing/animation/keyframe-order.html (added)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h (modified) (1 diff)
• Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm (modified) (1 diff)
• Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.h (modified) (1 diff)
• Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm (modified) (2 diffs)
• Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-07-01  Simon Fraser  <simon.fraser@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  <zalan@apple.com>
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2014-07-01  Simon Fraser  <simon.fraser@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  <andersca@apple.com>
 

trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h

@@ -124 +124 @@
         Vector<WebCore::GraphicsLayer::PlatformLayerID> children;
 
-        HashMap<String, PlatformCAAnimationRemote::Properties> addedAnimations;
+        Vector<std::pair<String, PlatformCAAnimationRemote::Properties>> addedAnimations;
         HashSet<String> keyPathsOfAnimationsToRemove;
 

trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm

@@ -1104 +1104 @@
 
         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)

trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.h

@@ -295 +295 @@
     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:

trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm

@@ -776 +776 @@
 }
 
-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;
@@ -783 +783 @@
         [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;

trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp

@@ -302 +302 @@
 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);
 
@@ -312 +322 @@
 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);