Bug 564991. Part 31: Don't cull display items in nsDisplayList::ComputeVisibility. r=mats

layout/base/FrameLayerBuilder.cpp

@@ -837,6 +837,15 @@ ContainerState::ProcessDisplayItems(const nsDisplayList& aList,
 
     // Assign the item to a layer
     if (layerState == LAYER_ACTIVE) {
+      // If the item would have its own layer but is invisible, just hide it.
+      // Note that items without their own layers can't be skipped this
+      // way, since their ThebesLayer may decide it wants to draw them
+      // into its buffer even if they're currently covered.
+      if (itemVisibleRect.IsEmpty()) {
+        InvalidateForLayerChange(item, nsnull);
+        continue;
+      }
+
       // Just use its layer.
       nsRefPtr<Layer> ownLayer = item->BuildLayer(mBuilder, mManager);
       if (!ownLayer) {

layout/base/nsDisplayList.cpp

@@ -289,13 +289,14 @@ nsDisplayList::GetBounds(nsDisplayListBuilder* aBuilder) const {
   return bounds;
 }
 
-void
+PRBool
 nsDisplayList::ComputeVisibility(nsDisplayListBuilder* aBuilder,
                                  nsRegion* aVisibleRegion,
                                  nsRegion* aVisibleRegionBeforeMove) {
   NS_ASSERTION(!aVisibleRegionBeforeMove, "Not supported anymore");
 
   mVisibleRect = aVisibleRegion->GetBounds();
+  PRBool anyVisible = PR_FALSE;
 
   nsAutoTArray<nsDisplayItem*, 512> elements;
   FlattenTo(&elements);
@@ -316,24 +317,22 @@ nsDisplayList::ComputeVisibility(nsDisplayListBuilder* aBuilder,
     itemVisible.And(*aVisibleRegion, bounds);
     item->mVisibleRect = itemVisible.GetBounds();
 
-    if (!item->mVisibleRect.IsEmpty() &&
-        item->ComputeVisibility(aBuilder, aVisibleRegion, aVisibleRegionBeforeMove)) {
-      AppendToBottom(item);
-
+    if (item->ComputeVisibility(aBuilder, aVisibleRegion, aVisibleRegionBeforeMove)) {
+      anyVisible = PR_TRUE;
       nsIFrame* f = item->GetUnderlyingFrame();
       if (item->IsOpaque(aBuilder) && f) {
         // Subtract opaque item from the visible region
         aBuilder->SubtractFromVisibleRegion(aVisibleRegion, nsRegion(bounds));
       }
-    } else {
-      item->~nsDisplayItem();
     }
+    AppendToBottom(item);
   }
 
   mIsOpaque = aVisibleRegion->IsEmpty();
 #ifdef DEBUG
   mDidComputeVisibility = PR_TRUE;
 #endif
+  return anyVisible;
 }
 
 void nsDisplayList::PaintRoot(nsDisplayListBuilder* aBuilder,
@@ -587,8 +586,7 @@ PRBool nsDisplayItem::RecomputeVisibility(nsDisplayListBuilder* aBuilder,
   itemVisible.And(*aVisibleRegion, bounds);
   mVisibleRect = itemVisible.GetBounds();
 
-  if (mVisibleRect.IsEmpty() ||
-      !ComputeVisibility(aBuilder, aVisibleRegion, nsnull))
+  if (!ComputeVisibility(aBuilder, aVisibleRegion, nsnull))
     return PR_FALSE;
 
   if (IsOpaque(aBuilder)) {
@@ -636,6 +634,10 @@ nsDisplayBackground::ComputeVisibility(nsDisplayListBuilder* aBuilder,
                                        nsRegion* aVisibleRegion,
                                        nsRegion* aVisibleRegionBeforeMove)
 {
+  if (!nsDisplayItem::ComputeVisibility(aBuilder, aVisibleRegion,
+                                        aVisibleRegionBeforeMove))
+    return PR_FALSE;
+
   // Return false if the background was propagated away from this
   // frame. We don't want this display item to show up and confuse
   // anything.
@@ -990,9 +992,7 @@ PRBool
 nsDisplayWrapList::ComputeVisibility(nsDisplayListBuilder* aBuilder,
                                      nsRegion* aVisibleRegion,
                                      nsRegion* aVisibleRegionBeforeMove) {
-  mList.ComputeVisibility(aBuilder, aVisibleRegion, aVisibleRegionBeforeMove);
-  // If none of the items are visible, they will all have been deleted
-  return mList.GetTop() != nsnull;
+  return mList.ComputeVisibility(aBuilder, aVisibleRegion, aVisibleRegionBeforeMove);
 }
 
 PRBool

layout/base/nsDisplayList.h

@@ -621,7 +621,7 @@ public:
   virtual PRBool ComputeVisibility(nsDisplayListBuilder* aBuilder,
                                    nsRegion* aVisibleRegion,
                                    nsRegion* aVisibleRegionBeforeMove)
-  { return PR_TRUE; }
+  { return !mVisibleRect.IsEmpty(); }
 
   /**
    * Try to merge with the other item (which is below us in the display
@@ -854,20 +854,24 @@ public:
   void Sort(nsDisplayListBuilder* aBuilder, SortLEQ aCmp, void* aClosure);
 
   /**
-   * Optimize the display list for visibility, removing any elements that
-   * are not visible. We put this logic here so it can be shared by top-level
+   * Compute visiblity for the items in the list.
+   * We put this logic here so it can be shared by top-level
    * painting and also display items that maintain child lists.
    * This is also a good place to put ComputeVisibility-related logic
    * that must be applied to every display item. In particular, this
    * sets mVisibleRect on each display item.
    * This also sets mIsOpaque to whether aVisibleRegion is empty on return.
+   * This does not remove any items from the list, so we can recompute
+   * visiblity with different regions later (see
+   * FrameLayerBuilder::DrawThebesLayer).
    * 
    * @param aVisibleRegion the area that is visible, relative to the
    * reference frame; on return, this contains the area visible under the list
+   * @return true if any item in the list is visible
    */
-  void ComputeVisibility(nsDisplayListBuilder* aBuilder,
-                         nsRegion* aVisibleRegion,
-                         nsRegion* aVisibleRegionBeforeMove);
+  PRBool ComputeVisibility(nsDisplayListBuilder* aBuilder,
+                           nsRegion* aVisibleRegion,
+                           nsRegion* aVisibleRegionBeforeMove);
   /**
    * Returns true if the visible region output from ComputeVisiblity was
    * empty, i.e. everything visible in this list is opaque.
@@ -922,6 +926,10 @@ public:
                nsDisplayItem::HitTestState* aState,
                nsTArray<nsIFrame*> *aOutFrames) const;
 
+#ifdef DEBUG
+  PRBool DidComputeVisibility() const { return mDidComputeVisibility; }
+#endif
+
 private:
   // This class is only used on stack, so we don't have to worry about leaking
   // it.  Don't let us be heap-allocated!

layout/base/nsLayoutDebugger.cpp

@@ -154,6 +154,8 @@ PrintDisplayListTo(nsDisplayListBuilder* aBuilder, const nsDisplayList& aList,
                    PRInt32 aIndent, FILE* aOutput)
 {
   for (nsDisplayItem* i = aList.GetBottom(); i != nsnull; i = i->GetAbove()) {
+    if (aList.DidComputeVisibility() && i->GetVisibleRect().IsEmpty())
+      continue;
     for (PRInt32 j = 0; j < aIndent; ++j) {
       fputc(' ', aOutput);
     }

layout/reftests/scrolling/reftest.list

@@ -2,3 +2,4 @@ HTTP == fixed-1.html fixed-1.html?ref
 HTTP == opacity-mixed-scrolling-1.html opacity-mixed-scrolling-1.html?ref
 HTTP == simple-1.html simple-1.html?ref
 == uncovering-1.html uncovering-1-ref.html
+== uncovering-2.html uncovering-2-ref.html

layout/reftests/scrolling/uncovering-2-ref.html

@@ -0,0 +1,24 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<style>
+body {
+  width: 1500px;
+  overflow: hidden;
+}
+div#bottom {
+  position: fixed;
+  left: 0;
+  top: 0;
+  height: 200px;
+  width: 200px;
+  background: green;
+}
+</style>
+</head>
+<body>
+<div style="margin-left:200px; left:200px; width:100px; height:100px; background:pink;"></div>
+<div style="left:0; width:500px; height:20px; background:blue;"></div>
+<div id="bottom"></div>
+</body>
+</html>

layout/reftests/scrolling/uncovering-2.html

@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<style>
+body {
+  width: 1500px;
+  overflow: hidden;
+}
+div#bottom {
+  position: fixed;
+  left: 0;
+  top: 0;
+  height: 200px;
+  width: 200px;
+  background: green;
+}
+</style>
+</head>
+<body>
+<div style="margin-left:200px; left:200px; width:100px; height:100px; background:pink;"></div>
+<div style="left:0; width:500px; height:20px; background:blue;"></div>
+<div id="bottom"></div>
+<script>
+document.documentElement.scrollLeft = 200;
+function doTest() {
+  document.documentElement.removeAttribute("class");
+  document.documentElement.scrollLeft = 0;
+}
+window.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</body>
+</html>