Title: [98776] trunk
Revision
98776
Author
simon.fra...@apple.com
Date
2011-10-28 16:18:08 -0700 (Fri, 28 Oct 2011)

Log Message

If visibility changes while an accelerated animation is running, element jumps around
https://bugs.webkit.org/show_bug.cgi?id=29984

Source/WebCore:

Reviewed by Chris Marrin.

Compositing now affects whether RenderLayers for visibility:hidden elements
are included in z-order lists. So we have to dirty those lists when we enter
compopsiting mode.

Test: compositing/visibility/animation-visibility.html

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateVisibilityStatus): Removed trailing whitespace.
(WebCore::RenderLayer::dirtyZOrderLists): Call dirtyZOrderListsInternal(), which doesn't have
to ping the compositor.
(WebCore::RenderLayer::dirtyZOrderListsInternal):
(WebCore::RenderLayer::dirtyZOrderListsIncludingDescendants): Recursively dirty z-order
lists.
* rendering/RenderLayer.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::enableCompositingMode): When going into compositing mode,
dirty all z-order lists.

LayoutTests:

Reviewed by Chris Marrin.

Add testcase for visibility changing in the middle of an accelerated animation.

* animations/resources/animation-test-helpers.js: Add some constants for readability.
(checkExpectedValue): Add support for testing 'visibility'.
* compositing/visibility/animation-visibility-expected.png: Added.
* compositing/visibility/animation-visibility-expected.txt: Added.
* compositing/visibility/animation-visibility.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (98775 => 98776)


--- trunk/LayoutTests/ChangeLog	2011-10-28 23:08:51 UTC (rev 98775)
+++ trunk/LayoutTests/ChangeLog	2011-10-28 23:18:08 UTC (rev 98776)
@@ -1,3 +1,18 @@
+2011-10-28  Simon Fraser  <simon.fra...@apple.com>
+
+        If visibility changes while an accelerated animation is running, element jumps around
+        https://bugs.webkit.org/show_bug.cgi?id=29984
+
+        Reviewed by Chris Marrin.
+        
+        Add testcase for visibility changing in the middle of an accelerated animation.
+
+        * animations/resources/animation-test-helpers.js: Add some constants for readability.
+        (checkExpectedValue): Add support for testing 'visibility'.
+        * compositing/visibility/animation-visibility-expected.png: Added.
+        * compositing/visibility/animation-visibility-expected.txt: Added.
+        * compositing/visibility/animation-visibility.html: Added.
+
 2011-10-28  Tim Horton  <timothy_hor...@apple.com>
 
         Implement CSS3 Images cross-fade() image function

Modified: trunk/LayoutTests/animations/resources/animation-test-helpers.js (98775 => 98776)


--- trunk/LayoutTests/animations/resources/animation-test-helpers.js	2011-10-28 23:08:51 UTC (rev 98775)
+++ trunk/LayoutTests/animations/resources/animation-test-helpers.js	2011-10-28 23:18:08 UTC (rev 98776)
@@ -33,6 +33,9 @@
 
 */
 
+const doPixelTest = true;
+const dontDoPixelTest = false;
+
 function isCloseEnough(actual, desired, tolerance)
 {
     var diff = Math.abs(actual - desired);
@@ -140,6 +143,20 @@
         }
         else
             pass = isCloseEnough(computedValue, expectedValue, tolerance);
+    } else if (property == "visibility") {
+        var element;
+        if (iframeId)
+            element = document.getElementById(iframeId).contentDocument.getElementById(elementId);
+        else
+            element = document.getElementById(elementId);
+
+        computedValue = window.getComputedStyle(element).visibility;
+        if (compareElements) {
+            computedValue2 = window.getComputedStyle(document.getElementById(elementId2)).visibility;
+            pass = computedValue == computedValue2;
+        }
+        else
+            pass = computedValue == expectedValue;
     } else {
         var element;
         if (iframeId)

Added: trunk/LayoutTests/compositing/visibility/animation-visibility-expected.png


(Binary files differ)
Property changes on: trunk/LayoutTests/compositing/visibility/animation-visibility-expected.png ___________________________________________________________________

Added: svn:mime-type

Added: trunk/LayoutTests/compositing/visibility/animation-visibility-expected.txt (0 => 98776)


--- trunk/LayoutTests/compositing/visibility/animation-visibility-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/compositing/visibility/animation-visibility-expected.txt	2011-10-28 23:18:08 UTC (rev 98776)
@@ -0,0 +1,3 @@
+PASS - "visibility" property for "box" element at 0.75s saw something close to: visible
+PASS - "webkitTransform.4" property for "box" element at 0.75s saw something close to: 375
+
Property changes on: trunk/LayoutTests/compositing/visibility/animation-visibility-expected.txt
___________________________________________________________________

Added: svn:mime-type

Added: svn:keywords

Added: svn:eol-style

Added: trunk/LayoutTests/compositing/visibility/animation-visibility.html (0 => 98776)


--- trunk/LayoutTests/compositing/visibility/animation-visibility.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/visibility/animation-visibility.html	2011-10-28 23:18:08 UTC (rev 98776)
@@ -0,0 +1,74 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+  <title>Visibility animation</title>
+  <style>
+    .container {
+      position: relative;
+      height: 100px;
+      width: 600px;
+      margin: 20px;
+      border: 1px solid black;
+    }
+    
+    .box {
+      position: absolute;
+      width: 100px;
+      height: 100px;
+    }
+    .indicator {
+      background-color: red;
+      left: 375px;
+    }
+    #box {
+      background-color: green;
+      -webkit-animation: move 1s linear, reveal 1s linear;
+    }
+
+    @-webkit-keyframes move
+    {
+      0%  { -webkit-transform: translateX(0); }
+      100%{ -webkit-transform: translateX(500px); }
+    }
+
+    @-webkit-keyframes reveal
+    {
+      0%  { visibility: hidden; }
+      50% { visibility: hidden; }
+      51% { visibility: visible; }
+      100%{ visibility: visible; }
+    }
+    
+    #result.hidden {
+      opacity: 0; /* Hide in pixel result */
+    }
+  </style>
+  <script src=""
+  <script>
+    if (window.layoutTestController)
+      layoutTestController.dumpAsText(true);
+    
+    const expectedValues = [
+      // [animation-name, time, element-id, property, expected-value, tolerance]
+      ["reveal", 0.75, "box", "visibility", 'visible'],
+      ["move", 0.75, "box", "webkitTransform.4", 375, 10],
+    ];
+  
+    var disablePauseAPI = false;
+    runAnimationTest(expectedValues, null, null, disablePauseAPI, doPixelTest);
+  </script>
+</head>
+<body>
+
+<div class="container">
+  <div class="indicator box"></div>
+  <div id="box" class="box"></div>
+</div>
+<div id="result"></div>
+<script>
+  if (window.layoutTestController)
+    document.getElementById('result').className = 'hidden';
+</script>
+</body>
+</html>
\ No newline at end of file
Property changes on: trunk/LayoutTests/compositing/visibility/animation-visibility.html
___________________________________________________________________

Added: svn:mime-type

Added: svn:keywords

Added: svn:eol-style

Modified: trunk/Source/WebCore/ChangeLog (98775 => 98776)


--- trunk/Source/WebCore/ChangeLog	2011-10-28 23:08:51 UTC (rev 98775)
+++ trunk/Source/WebCore/ChangeLog	2011-10-28 23:18:08 UTC (rev 98776)
@@ -1,3 +1,28 @@
+2011-10-28  Simon Fraser  <simon.fra...@apple.com>
+
+        If visibility changes while an accelerated animation is running, element jumps around
+        https://bugs.webkit.org/show_bug.cgi?id=29984
+
+        Reviewed by Chris Marrin.
+        
+        Compositing now affects whether RenderLayers for visibility:hidden elements
+        are included in z-order lists. So we have to dirty those lists when we enter
+        compopsiting mode.
+
+        Test: compositing/visibility/animation-visibility.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateVisibilityStatus): Removed trailing whitespace.
+        (WebCore::RenderLayer::dirtyZOrderLists): Call dirtyZOrderListsInternal(), which doesn't have
+        to ping the compositor.
+        (WebCore::RenderLayer::dirtyZOrderListsInternal):
+        (WebCore::RenderLayer::dirtyZOrderListsIncludingDescendants): Recursively dirty z-order
+        lists.
+        * rendering/RenderLayer.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::enableCompositingMode): When going into compositing mode,
+        dirty all z-order lists.
+
 2011-10-28  Tim Horton  <timothy_hor...@apple.com>
 
         Implement CSS3 Images cross-fade() image function

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (98775 => 98776)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2011-10-28 23:08:51 UTC (rev 98775)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2011-10-28 23:18:08 UTC (rev 98776)
@@ -617,7 +617,7 @@
     if (m_visibleDescendantStatusDirty) {
         m_hasVisibleDescendant = false;
         for (RenderLayer* child = firstChild(); child; child = child->nextSibling()) {
-            child->updateVisibilityStatus();        
+            child->updateVisibilityStatus();
             if (child->m_hasVisibleContent || child->m_hasVisibleDescendant) {
                 m_hasVisibleDescendant = true;
                 break;
@@ -3970,16 +3970,29 @@
 
 void RenderLayer::dirtyZOrderLists()
 {
+    dirtyZOrderListsInternal();
+    
+#if USE(ACCELERATED_COMPOSITING)
+    if (!renderer()->documentBeingDestroyed())
+        compositor()->setCompositingLayersNeedRebuild();
+#endif
+}
+
+void RenderLayer::dirtyZOrderListsInternal()
+{
     if (m_posZOrderList)
         m_posZOrderList->clear();
     if (m_negZOrderList)
         m_negZOrderList->clear();
     m_zOrderListsDirty = true;
+}
 
-#if USE(ACCELERATED_COMPOSITING)
-    if (!renderer()->documentBeingDestroyed())
-        compositor()->setCompositingLayersNeedRebuild();
-#endif
+void RenderLayer::dirtyZOrderListsIncludingDescendants()
+{
+    dirtyZOrderListsInternal();
+
+    for (RenderLayer* child = firstChild(); child; child = child->nextSibling())
+        child->dirtyZOrderListsIncludingDescendants();
 }
 
 void RenderLayer::dirtyStackingContextZOrderLists()

Modified: trunk/Source/WebCore/rendering/RenderLayer.h (98775 => 98776)


--- trunk/Source/WebCore/rendering/RenderLayer.h	2011-10-28 23:08:51 UTC (rev 98775)
+++ trunk/Source/WebCore/rendering/RenderLayer.h	2011-10-28 23:18:08 UTC (rev 98776)
@@ -376,6 +376,7 @@
     bool isStackingContext() const { return !hasAutoZIndex() || renderer()->isRenderView(); }
 
     void dirtyZOrderLists();
+    void dirtyZOrderListsIncludingDescendants();
     void dirtyStackingContextZOrderLists();
     void updateZOrderLists();
     Vector<RenderLayer*>* posZOrderList() const { return m_posZOrderList; }
@@ -540,6 +541,8 @@
     void setParent(RenderLayer* parent);
     void setFirstChild(RenderLayer* first) { m_first = first; }
     void setLastChild(RenderLayer* last) { m_last = last; }
+    
+    void dirtyZOrderListsInternal();
 
     LayoutPoint renderBoxLocation() const { return renderer()->isBox() ? toRenderBox(renderer())->location() : LayoutPoint(); }
     LayoutUnit renderBoxX() const { return renderBoxLocation().x(); }

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (98775 => 98776)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2011-10-28 23:08:51 UTC (rev 98775)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2011-10-28 23:18:08 UTC (rev 98776)
@@ -124,6 +124,8 @@
         
         if (m_compositing) {
             ensureRootLayer();
+            // Compositing affects whether visibility:hidden layers are included in z-order lists, so we have to dirty the lists here.
+            rootRenderLayer()->dirtyZOrderListsIncludingDescendants();
             notifyIFramesOfCompositingChange();
         } else
             destroyRootLayer();
@@ -1124,7 +1126,6 @@
     }
 }
 
-
 void RenderLayerCompositor::repaintCompositedLayersAbsoluteRect(const LayoutRect& absRect)
 {
     recursiveRepaintLayerRect(rootRenderLayer(), absRect);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to