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 7c85d86..3c7597d 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 3c67ce8..0b17184 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 a36180e..9caee87 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 ec211d3..3f093eb 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 2fecfe4..a872695 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 4519790..0d3cf56 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 86aea07..5cf603e 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>*);