<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[170656] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/170656">170656</a></dd>
<dt>Author</dt> <dd>simon.fraser@apple.com</dd>
<dt>Date</dt> <dd>2014-07-01 12:15:39 -0700 (Tue, 01 Jul 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2SharedmacRemoteLayerTreeTransactionh">trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h</a></li>
<li><a href="#trunkSourceWebKit2SharedmacRemoteLayerTreeTransactionmm">trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPagemacPlatformCAAnimationRemoteh">trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.h</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPagemacPlatformCAAnimationRemotemm">trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPagemacPlatformCALayerRemotecpp">trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestscompositinganimationkeyframeorderexpectedhtml">trunk/LayoutTests/compositing/animation/keyframe-order-expected.html</a></li>
<li><a href="#trunkLayoutTestscompositinganimationkeyframeorderhtml">trunk/LayoutTests/compositing/animation/keyframe-order.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (170655 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-07-01 19:10:30 UTC (rev 170655)
+++ trunk/LayoutTests/ChangeLog 2014-07-01 19:15:39 UTC (rev 170656)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+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.
+
</ins><span class="cx"> 2014-07-01 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Subpixel rendering: Pixel crack in breadcrumbs at devforums.apple.com.
</span></span></pre></div>
<a id="trunkLayoutTestscompositinganimationkeyframeorderexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/compositing/animation/keyframe-order-expected.html (0 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,48 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestscompositinganimationkeyframeorderhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/compositing/animation/keyframe-order.html (0 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,76 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (170655 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+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).
+
</ins><span class="cx"> 2014-07-01 Anders Carlsson <andersca@apple.com>
</span><span class="cx">
</span><span class="cx"> Add a encodeLegacySessionState function
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedmacRemoteLayerTreeTransactionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.h (170655 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -123,7 +123,7 @@
</span><span class="cx"> std::unique_ptr<WebCore::TransformationMatrix> sublayerTransform;
</span><span class="cx"> Vector<WebCore::GraphicsLayer::PlatformLayerID> children;
</span><span class="cx">
</span><del>- HashMap<String, PlatformCAAnimationRemote::Properties> addedAnimations;
</del><ins>+ Vector<std::pair<String, PlatformCAAnimationRemote::Properties>> addedAnimations;
</ins><span class="cx"> HashSet<String> keyPathsOfAnimationsToRemove;
</span><span class="cx">
</span><span class="cx"> WebCore::FloatPoint3D position;
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedmacRemoteLayerTreeTransactionmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm (170655 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1103,8 +1103,8 @@
</span><span class="cx"> dumpProperty(ts, "filters", layerProperties.filters ? *layerProperties.filters : FilterOperations());
</span><span class="cx">
</span><span class="cx"> if (layerProperties.changedProperties & RemoteLayerTreeTransaction::AnimationsChanged) {
</span><del>- for (const auto& keyValuePair : layerProperties.addedAnimations)
- dumpProperty(ts, "animation " + keyValuePair.key, keyValuePair.value);
</del><ins>+ for (const auto& keyAnimationPair : layerProperties.addedAnimations)
+ dumpProperty(ts, "animation " + keyAnimationPair.first, keyAnimationPair.second);
</ins><span class="cx">
</span><span class="cx"> for (const auto& name : layerProperties.keyPathsOfAnimationsToRemove)
</span><span class="cx"> dumpProperty(ts, "removed animation", name);
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPagemacPlatformCAAnimationRemoteh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.h (170655 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -294,8 +294,8 @@
</span><span class="cx">
</span><span class="cx"> const Properties& properties() const { return m_properties; }
</span><span class="cx">
</span><del>- typedef HashMap<String, Properties> AnimationsMap;
- static void updateLayerAnimations(CALayer *, RemoteLayerTreeHost*, const AnimationsMap& animationsToAdd, const HashSet<String>& animationsToRemove);
</del><ins>+ typedef Vector<std::pair<String, Properties>> AnimationsList;
+ static void updateLayerAnimations(CALayer *, RemoteLayerTreeHost*, const AnimationsList& animationsToAdd, const HashSet<String>& animationsToRemove);
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> PlatformCAAnimationRemote(AnimationType, const String& keyPath);
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPagemacPlatformCAAnimationRemotemm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm (170655 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -775,15 +775,15 @@
</span><span class="cx"> [layer addAnimation:caAnimation.get() forKey:key];
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void PlatformCAAnimationRemote::updateLayerAnimations(CALayer *layer, RemoteLayerTreeHost* layerTreeHost, const AnimationsMap& animationsToAdd, const HashSet<String>& animationsToRemove)
</del><ins>+void PlatformCAAnimationRemote::updateLayerAnimations(CALayer *layer, RemoteLayerTreeHost* layerTreeHost, const AnimationsList& animationsToAdd, const HashSet<String>& animationsToRemove)
</ins><span class="cx"> {
</span><span class="cx"> BEGIN_BLOCK_OBJC_EXCEPTIONS;
</span><span class="cx">
</span><span class="cx"> for (const auto& value : animationsToRemove)
</span><span class="cx"> [layer removeAnimationForKey:value];
</span><span class="cx">
</span><del>- for (const auto& keyValuePair : animationsToAdd)
- addAnimationToLayer(layer, layerTreeHost, keyValuePair.key, keyValuePair.value);
</del><ins>+ for (const auto& keyAnimationPair : animationsToAdd)
+ addAnimationToLayer(layer, layerTreeHost, keyAnimationPair.first, keyAnimationPair.second);
</ins><span class="cx">
</span><span class="cx"> END_BLOCK_OBJC_EXCEPTIONS;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPagemacPlatformCALayerRemotecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp (170655 => 170656)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -301,8 +301,18 @@
</span><span class="cx">
</span><span class="cx"> void PlatformCALayerRemote::addAnimationForKey(const String& key, PlatformCAAnimation* animation)
</span><span class="cx"> {
</span><del>- m_animations.set(key, animation);
- m_properties.addedAnimations.set(key, toPlatformCAAnimationRemote(animation)->properties());
</del><ins>+ 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;
+ }
+ }
+ }
+
</ins><span class="cx"> m_properties.notePropertiesChanged(RemoteLayerTreeTransaction::AnimationsChanged);
</span><span class="cx">
</span><span class="cx"> if (m_context)
</span><span class="lines">@@ -311,8 +321,14 @@
</span><span class="cx">
</span><span class="cx"> void PlatformCALayerRemote::removeAnimationForKey(const String& key)
</span><span class="cx"> {
</span><del>- m_animations.remove(key);
- m_properties.addedAnimations.remove(key);
</del><ins>+ 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;
+ }
+ }
+ }
</ins><span class="cx"> m_properties.keyPathsOfAnimationsToRemove.add(key);
</span><span class="cx"> m_properties.notePropertiesChanged(RemoteLayerTreeTransaction::AnimationsChanged);
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>