Compositing overlap testing can throw layers into compositing when they should not be.
authorenne@google.com <enne@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 05:13:00 +0000 (05:13 +0000)
committerenne@google.com <enne@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 05:13:00 +0000 (05:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=50192

Reviewed by Simon Fraser.

Source/WebCore:

The previous overlap map behavior was that a non-composited query
layer would become composited due to overlap if and only if the query
layer's absolute bounds overlapped the absolute bounds of some other
layer which:
    - draws before the query layer
    - is or has a compositing ancestor

This behavior, while correct, was too permissive in throwing layers
into compositing, causing many layers to get their own backing when
they could have just gone into their compositing ancestor's backing.

The correct logic is that non-composited query layer needs to be
composited due to overlap if and only if the query layer's absolute
bounds overlap the absolute bounds of some other layer which:
    - draws before the query layer
    - has a different compositing ancestor than the query layer
    - is or has a compositing ancestor that is a descendent of the
      query layer's compositing ancestor

This patch changes the semantics of the overlap map to enable this
behavior.

Rather than having one global overlap map, there is now a stack of
overlap maps. New (empty) overlap maps are pushed onto the stack
whenever a layer becomes a compositing ancestor and popped after all
of the compositing requirements for that layer's children have been
computed.

The compositing ancestor and all of its non-composited children of a
compositing ancestor do not get considered for overlap until their
composited ancestor has been popped off the stack. If a compositing
ancestor has a compositing subtree, then any descendents of that
compositing ancestor that draw after that subtree will consider
everything in the compositing subtree for overlap.

Test: compositing/layer-creation/stacking-context-overlap.html

* platform/graphics/Region.cpp:
(WebCore::Region::intersects):
(WebCore):
* platform/graphics/Region.h:
(Region):
* rendering/RenderLayerCompositor.cpp:
(RenderLayerCompositor::OverlapMap):
(WebCore::RenderLayerCompositor::OverlapMap::OverlapMap):
(WebCore::RenderLayerCompositor::OverlapMap::add):
(WebCore::RenderLayerCompositor::OverlapMap::contains):
(WebCore::RenderLayerCompositor::OverlapMap::overlapsLayers):
(WebCore::RenderLayerCompositor::OverlapMap::isEmpty):
(WebCore::RenderLayerCompositor::OverlapMap::popCompositingContainer):
(WebCore::RenderLayerCompositor::OverlapMap::pushCompositingContainer):
(WebCore::RenderLayerCompositor::addToOverlapMapRecursive):
(WebCore::RenderLayerCompositor::computeCompositingRequirements):
* rendering/RenderLayerCompositor.h:
(RenderLayerCompositor):

LayoutTests:

* compositing/layer-creation/stacking-context-overlap-expected.txt: Added.
* compositing/layer-creation/stacking-context-overlap.html: Added.
* compositing/layer-creation/stacking-context-overlap-nested-expected.txt: Added.
* compositing/layer-creation/stacking-context-overlap-nested.html: Added.
* platform/chromium/test_expectations.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@109851 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/compositing/layer-creation/stacking-context-overlap-expected.txt [new file with mode: 0644]
LayoutTests/compositing/layer-creation/stacking-context-overlap-nested-expected.txt [new file with mode: 0644]
LayoutTests/compositing/layer-creation/stacking-context-overlap-nested.html [new file with mode: 0644]
LayoutTests/compositing/layer-creation/stacking-context-overlap.html [new file with mode: 0644]
LayoutTests/platform/chromium/test_expectations.txt
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Region.cpp
Source/WebCore/platform/graphics/Region.h
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index 7c85d8643c753db3c4d7510205dcf5e5d7890739..3c7597db3644be9583b64f1dead835624632dd4f 100644 (file)
@@ -1,3 +1,16 @@
+2012-03-05  Adrienne Walker  <enne@google.com>
+
+        Compositing overlap testing can throw layers into compositing when they should not be.
+        https://bugs.webkit.org/show_bug.cgi?id=50192
+
+        Reviewed by Simon Fraser.
+
+        * compositing/layer-creation/stacking-context-overlap-expected.txt: Added.
+        * compositing/layer-creation/stacking-context-overlap.html: Added.
+        * compositing/layer-creation/stacking-context-overlap-nested-expected.txt: Added.
+        * compositing/layer-creation/stacking-context-overlap-nested.html: Added.
+        * platform/chromium/test_expectations.txt:
+
 2012-03-05  Yoshifumi Inoue  <yosin@chromium.org>
 
         [Forms] HTMLFieldSetForms.idl doesn't have type attribute.
diff --git a/LayoutTests/compositing/layer-creation/stacking-context-overlap-expected.txt b/LayoutTests/compositing/layer-creation/stacking-context-overlap-expected.txt
new file mode 100644 (file)
index 0000000..15510cb
--- /dev/null
@@ -0,0 +1,21 @@
+(GraphicsLayer
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (children 2
+        (GraphicsLayer
+          (position 8.00 8.00)
+          (bounds 20.00 20.00)
+          (drawsContent 1)
+        )
+        (GraphicsLayer
+          (position 8.00 18.00)
+          (bounds 142.00 142.00)
+          (drawsContent 1)
+        )
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/compositing/layer-creation/stacking-context-overlap-nested-expected.txt b/LayoutTests/compositing/layer-creation/stacking-context-overlap-nested-expected.txt
new file mode 100644 (file)
index 0000000..5c2b30e
--- /dev/null
@@ -0,0 +1,28 @@
+(GraphicsLayer
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (children 2
+        (GraphicsLayer
+          (position 10.00 10.00)
+          (bounds 120.00 120.00)
+          (drawsContent 1)
+          (children 1
+            (GraphicsLayer
+              (position 50.00 50.00)
+              (bounds 220.00 120.00)
+              (drawsContent 1)
+            )
+          )
+        )
+        (GraphicsLayer
+          (position 65.00 65.00)
+          (bounds 76.00 76.00)
+          (drawsContent 1)
+        )
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/compositing/layer-creation/stacking-context-overlap-nested.html b/LayoutTests/compositing/layer-creation/stacking-context-overlap-nested.html
new file mode 100644 (file)
index 0000000..37d0d0b
--- /dev/null
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+  <style>
+    .box {
+      position: absolute;
+      top: 20px;
+      left: 20px;
+      height: 100px;
+      width: 100px;
+      background-color: red;
+    }
+
+    .composited {
+      -webkit-transform: translateZ(0);
+      background-color: green;
+      outline: 10px solid transparent; /* inflate compositing layer bounds */
+    }
+
+    .box > .box {
+      top: 50px;
+      left: 50px;
+      width: 200px;
+      background-color: rgba(255, 0, 0, 0.6);
+    }
+
+    #indicator {
+      position: absolute;
+      top: 75px;
+      left: 75px;
+      height: 56px;
+      width: 56px;
+      background-color: blue;
+    }
+  </style>
+  <script>
+      if (window.layoutTestController) {
+          layoutTestController.dumpAsText();
+          layoutTestController.waitUntilDone();
+      }
+
+      function doTest()
+      {
+          if (window.layoutTestController) {
+              document.getElementById('layertree').innerText = layoutTestController.layerTreeAsText();
+              layoutTestController.notifyDone();
+          }
+      }
+
+      window.addEventListener('load', doTest, false);
+  </script>
+</head>
+<body>
+  <div class="composited box">
+    <div class="composited box"></div>
+  </div>
+
+  <div id="indicator"></div>
+
+  <pre id="layertree"></pre>
+</body>
+</html>
diff --git a/LayoutTests/compositing/layer-creation/stacking-context-overlap.html b/LayoutTests/compositing/layer-creation/stacking-context-overlap.html
new file mode 100644 (file)
index 0000000..8db1677
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style type="text/css" media="screen">
+        .trigger {
+            width: 20px;
+            height: 20px;
+            background-color: blue;
+            -webkit-transform: translateZ(0);
+        }
+
+        .container {
+            position: relative;
+            z-index: 0;
+            top: -10px;
+            height: 120px;
+            width: 120px;
+            padding: 10px;
+            border: 1px solid black;
+        }
+
+        .box {
+            position: relative;
+            height: 100px;
+            width: 100px;
+            margin: 10px;
+            background-color: silver;
+        }
+    </style>
+</head>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+    }
+
+    function doTest()
+    {
+        if (window.layoutTestController) {
+            document.getElementById('layertree').innerText = layoutTestController.layerTreeAsText();
+            layoutTestController.notifyDone();
+        }
+    }
+
+    window.addEventListener('load', doTest, false);
+</script>
+<body>
+    <div class="trigger"></div>
+
+    <div class="container">
+        <div class="child box">
+          <!-- This box doesn't need its own layer. -->
+        </div>
+    </div>
+    <pre id="layertree"></pre>
+</body>
+</html>
index 3c67ce85507d47b0fa50755aee2237bb407e18ea..0b17184e8e42a1236b7674d1a6829f98445365d3 100644 (file)
@@ -3410,8 +3410,6 @@ BUGWK75110 WIN : fast/filesystem/op-restricted-chars.html = TEXT
 BUGWK75161 GPU : media/video-poster-blocked-by-willsendrequest.html = PASS TEXT
 BUGWK75161 CPU : media/video-poster-blocked-by-willsendrequest.html = TEXT
 
-BUGWK75237 SNOWLEOPARD CPU : compositing/reflections/reflection-on-composited.html = PASS IMAGE
-
 BUGWK75367 : svg/css/composite-shadow-example.html = PASS TEXT
 BUGWK75367 : svg/css/composite-shadow-with-opacity.html = PASS TEXT
 
@@ -4312,3 +4310,17 @@ BUGWK78684 : fast/forms/fieldset-legend-padding-unclipped-fieldset-border.html =
 
 // New test; landed at r109779 but results look wrong on most Chrome platforms.
 BUGSENORBLANCO : http/tests/incremental/partial-jpeg.html = FAIL
+
+// Needs rebaselining. Layers are merged, and rotated text drawn better.
+BUGWK50192 : compositing/geometry/limit-layer-bounds-fixed-positioned.html = TEXT
+BUGWK50192 : compositing/geometry/limit-layer-bounds-overflow-root.html = TEXT
+BUGWK50192 : compositing/geometry/limit-layer-bounds-positioned-transition.html = TEXT
+BUGWK50192 : compositing/geometry/limit-layer-bounds-positioned.html = TEXT
+BUGWK50192 WIN LINUX : compositing/geometry/limit-layer-bounds-transformed-overflow.html = TEXT
+BUGWK50192 MAC LEOPARD : compositing/geometry/limit-layer-bounds-transformed-overflow.html = TEXT
+BUGWK50192 : compositing/geometry/limit-layer-bounds-transformed.html = TEXT
+BUGWK50192 : compositing/geometry/fixed-position-transform-composited-page-scale-down.html = IMAGE
+BUGWK50192 LINUX MAC : compositing/geometry/fixed-position-transform-composited-page-scale.html = IMAGE
+BUGWK50192 : compositing/reflections/reflection-on-composited.html = IMAGE
+BUGWK50192 : compositing/shadows/shadow-drawing.html = IMAGE
+BUGWK50192 : fast/repaint/block-selection-gap-in-composited-layer.html = IMAGE
index a36180e70d4a445b1329cc46f6add68ae1c6d8b0..9caee8719e3afd2c0eaab6b0dbb52243a328f28d 100644 (file)
@@ -1,3 +1,66 @@
+2012-03-05  Adrienne Walker  <enne@google.com>
+
+        Compositing overlap testing can throw layers into compositing when they should not be.
+        https://bugs.webkit.org/show_bug.cgi?id=50192
+
+        Reviewed by Simon Fraser.
+
+        The previous overlap map behavior was that a non-composited query
+        layer would become composited due to overlap if and only if the query
+        layer's absolute bounds overlapped the absolute bounds of some other
+        layer which:
+            - draws before the query layer
+            - is or has a compositing ancestor
+
+        This behavior, while correct, was too permissive in throwing layers
+        into compositing, causing many layers to get their own backing when
+        they could have just gone into their compositing ancestor's backing.
+
+        The correct logic is that non-composited query layer needs to be
+        composited due to overlap if and only if the query layer's absolute
+        bounds overlap the absolute bounds of some other layer which:
+            - draws before the query layer
+            - has a different compositing ancestor than the query layer
+            - is or has a compositing ancestor that is a descendent of the
+              query layer's compositing ancestor
+
+        This patch changes the semantics of the overlap map to enable this
+        behavior.
+
+        Rather than having one global overlap map, there is now a stack of
+        overlap maps. New (empty) overlap maps are pushed onto the stack
+        whenever a layer becomes a compositing ancestor and popped after all
+        of the compositing requirements for that layer's children have been
+        computed.
+
+        The compositing ancestor and all of its non-composited children of a
+        compositing ancestor do not get considered for overlap until their
+        composited ancestor has been popped off the stack. If a compositing
+        ancestor has a compositing subtree, then any descendents of that
+        compositing ancestor that draw after that subtree will consider
+        everything in the compositing subtree for overlap.
+
+        Test: compositing/layer-creation/stacking-context-overlap.html
+
+        * platform/graphics/Region.cpp:
+        (WebCore::Region::intersects):
+        (WebCore):
+        * platform/graphics/Region.h:
+        (Region):
+        * rendering/RenderLayerCompositor.cpp:
+        (RenderLayerCompositor::OverlapMap):
+        (WebCore::RenderLayerCompositor::OverlapMap::OverlapMap):
+        (WebCore::RenderLayerCompositor::OverlapMap::add):
+        (WebCore::RenderLayerCompositor::OverlapMap::contains):
+        (WebCore::RenderLayerCompositor::OverlapMap::overlapsLayers):
+        (WebCore::RenderLayerCompositor::OverlapMap::isEmpty):
+        (WebCore::RenderLayerCompositor::OverlapMap::popCompositingContainer):
+        (WebCore::RenderLayerCompositor::OverlapMap::pushCompositingContainer):
+        (WebCore::RenderLayerCompositor::addToOverlapMapRecursive):
+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
+        * rendering/RenderLayerCompositor.h:
+        (RenderLayerCompositor):
+
 2012-03-05  Anders Carlsson  <andersca@apple.com>
 
         Address review comments from https://bugs.webkit.org/show_bug.cgi?id=80368
index ec211d3b49fbb1c3e992779299dc6f001e66a307..3f093ebdcd6d5dfa164f6662b3f67b52eaac556c 100644 (file)
@@ -77,6 +77,17 @@ bool Region::contains(const IntPoint& point) const
     return contains(IntRect(point, IntSize(1, 1)));
 }
 
+bool Region::intersects(const Region& region) const
+{
+    if (!m_bounds.intersects(region.m_bounds))
+        return false;
+
+    // FIXME: this could be optimized.
+    Region tempRegion(*this);
+    tempRegion.intersect(region);
+    return !tempRegion.isEmpty();
+}
+
 unsigned Region::totalArea() const
 {
     Vector<IntRect> rects = this->rects();
index 2fecfe4161aa0f3c3ef8d3f21bb5519139e78c6e..a87269567a5a220005ae94b4dd87abd9131efdc2 100644 (file)
@@ -52,6 +52,9 @@ public:
 
     bool contains(const IntPoint&) const;
 
+    // Returns true if the query region intersects any part of this region.
+    bool intersects(const Region&) const;
+
     unsigned totalArea() const;
 
 #ifndef NDEBUG
index 45197904f4708cee879d54930a8b8df5e0d820e2..0d3cf5630af5e36dc3596a519821b8936280991e 100644 (file)
@@ -75,6 +75,58 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
+class RenderLayerCompositor::OverlapMap {
+    WTF_MAKE_NONCOPYABLE(OverlapMap);
+public:
+    OverlapMap()
+    {
+        // Begin assuming the root layer will be composited so that there is
+        // something on the stack. The root layer should also never get an
+        // popCompositingContainer call.
+        pushCompositingContainer();
+    }
+
+    void add(const RenderLayer* layer, const IntRect& bounds)
+    {
+        // Layers do not contribute to overlap immediately--instead, they will
+        // contribute to overlap as soon as their composited ancestor has been
+        // recursively processed and popped off the stack.
+        ASSERT(m_overlapStack.size() >= 2);
+        m_overlapStack[m_overlapStack.size() - 2].unite(bounds);
+        m_layers.add(layer);
+    }
+
+    bool contains(const RenderLayer* layer)
+    {
+        return m_layers.contains(layer);
+    }
+
+    bool overlapsLayers(const IntRect& bounds) const
+    {
+        return m_overlapStack.last().intersects(bounds);
+    }
+
+    bool isEmpty()
+    {
+        return m_layers.isEmpty();
+    }
+
+    void pushCompositingContainer()
+    {
+        m_overlapStack.append(Region());
+    }
+
+    void popCompositingContainer()
+    {
+        m_overlapStack[m_overlapStack.size() - 2].unite(m_overlapStack.last());
+        m_overlapStack.removeLast();
+    }
+
+private:
+    Vector<Region> m_overlapStack;
+    HashSet<const RenderLayer*> m_layers;
+};
+
 struct CompositingState {
     CompositingState(RenderLayer* compAncestor)
         : m_compositingAncestor(compAncestor)
@@ -637,18 +689,6 @@ void RenderLayerCompositor::addToOverlapMapRecursive(OverlapMap& overlapMap, Ren
     }
 }
 
-bool RenderLayerCompositor::overlapsCompositedLayers(OverlapMap& overlapMap, const IntRect& layerBounds)
-{
-    RenderLayerCompositor::OverlapMap::const_iterator end = overlapMap.end();
-    for (RenderLayerCompositor::OverlapMap::const_iterator it = overlapMap.begin(); it != end; ++it) {
-        const IntRect& bounds = it->second;
-        if (layerBounds.intersects(bounds))
-            return true;
-    }
-    
-    return false;
-}
-
 //  Recurse through the layers in z-index and overflow order (which is equivalent to painting order)
 //  For the z-order children of a compositing layer:
 //      If a child layers has a compositing layer, then all subsequent layers must
@@ -678,7 +718,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* layer, O
         if (absBounds.isEmpty())
             absBounds.setSize(IntSize(1, 1));
         haveComputedBounds = true;
-        mustOverlapCompositedLayers = overlapsCompositedLayers(*overlapMap, absBounds);
+        mustOverlapCompositedLayers = overlapMap->overlapsLayers(absBounds);
     }
     
     layer->setMustOverlapCompositedLayers(mustOverlapCompositedLayers);
@@ -688,7 +728,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* layer, O
     // ancestor with m_subtreeIsCompositing set to false.
     CompositingState childState(compositingState.m_compositingAncestor);
 #ifndef NDEBUG
-    ++childState.m_depth;
+    childState.m_depth = compositingState.m_depth + 1;
 #endif
 
     bool willBeComposited = needsToBeComposited(layer);
@@ -697,10 +737,9 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* layer, O
         compositingState.m_subtreeIsCompositing = true;
         // This layer now acts as the ancestor for kids.
         childState.m_compositingAncestor = layer;
-    }
 
-    if (overlapMap && childState.m_compositingAncestor && !childState.m_compositingAncestor->isRootLayer()) {
-        addToOverlapMap(*overlapMap, layer, absBounds, haveComputedBounds);
+        if (overlapMap)
+            overlapMap->pushCompositingContainer();
     }
 
 #if ENABLE(VIDEO)
@@ -726,7 +765,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* layer, O
                     layer->setMustOverlapCompositedLayers(true);
                     childState.m_compositingAncestor = layer;
                     if (overlapMap)
-                        addToOverlapMap(*overlapMap, layer, absBounds, haveComputedBounds);
+                        overlapMap->pushCompositingContainer();
                     willBeComposited = true;
                 }
             }
@@ -760,13 +799,22 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* layer, O
     
     ASSERT(willBeComposited == needsToBeComposited(layer));
 
+    // All layers (even ones that aren't being composited) need to get added to
+    // the overlap map. Layers that do not composite will draw into their
+    // compositing ancestor's backing, and so are still considered for overlap.
+    if (overlapMap && childState.m_compositingAncestor && !childState.m_compositingAncestor->isRootLayer())
+        addToOverlapMap(*overlapMap, layer, absBounds, haveComputedBounds);
+
     // If we have a software transform, and we have layers under us, we need to also
     // be composited. Also, if we have opacity < 1, then we need to be a layer so that
     // the child layers are opaque, then rendered with opacity on this layer.
     if (!willBeComposited && canBeComposited(layer) && childState.m_subtreeIsCompositing && requiresCompositingWhenDescendantsAreCompositing(layer->renderer())) {
         layer->setMustOverlapCompositedLayers(true);
-        if (overlapMap)
+        childState.m_compositingAncestor = layer;
+        if (overlapMap) {
+            overlapMap->pushCompositingContainer();
             addToOverlapMapRecursive(*overlapMap, layer);
+        }
         willBeComposited = true;
     }
 
@@ -784,11 +832,17 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* layer, O
     // setHasCompositingDescendant() may have changed the answer to needsToBeComposited() when clipping,
     // so test that again.
     if (!willBeComposited && canBeComposited(layer) && clipsCompositingDescendants(layer)) {
-        if (overlapMap)
+        childState.m_compositingAncestor = layer;
+        if (overlapMap) {
+            overlapMap->pushCompositingContainer();
             addToOverlapMapRecursive(*overlapMap, layer);
+        }
         willBeComposited = true;
     }
 
+    if (overlapMap && childState.m_compositingAncestor == layer && !layer->isRootLayer())
+        overlapMap->popCompositingContainer();
+
     // If we're back at the root, and no other layers need to be composited, and the root layer itself doesn't need
     // to be composited, then we can drop out of compositing mode altogether. However, don't drop out of compositing mode
     // if there are composited layers that we didn't hit in our traversal (e.g. because of visibility:hidden).
index 86aea07a6371852a364ac7c6423def71a8e1879c..5cf603e8a88e859be06bd91421faae2312903981 100644 (file)
@@ -210,6 +210,8 @@ public:
     void documentBackgroundColorDidChange();
 
 private:
+    class OverlapMap;
+
     // GraphicsLayerClient Implementation
     virtual void notifyAnimationStarted(const GraphicsLayer*, double) { }
     virtual void notifySyncRequired(const GraphicsLayer*) { scheduleLayerFlush(); }
@@ -233,10 +235,8 @@ private:
     // Repaint the given rect (which is layer's coords), and regions of child layers that intersect that rect.
     void recursiveRepaintLayerRect(RenderLayer*, const IntRect&);
 
-    typedef HashMap<RenderLayer*, IntRect> OverlapMap;
     void addToOverlapMap(OverlapMap&, RenderLayer*, IntRect& layerBounds, bool& boundsComputed);
     void addToOverlapMapRecursive(OverlapMap&, RenderLayer*);
-    static bool overlapsCompositedLayers(OverlapMap&, const IntRect& layerBounds);
 
     void updateCompositingLayersTimerFired(Timer<RenderLayerCompositor>*);