Bug 728983. Part 2: When display items for multiple frames are merged, track the merged frames and mark them all as having an associated container layer. This ensures that invalidations are processed correctly. r=mattwoodrow
authorRobert O'Callahan <robert@ocallahan.org>
Tue, 17 Apr 2012 17:45:04 +1200
changeset 95138 6dbdb799fa6dd7161a05e1712e36c426bd192f5b
parent 95137 9157e2b139132e3325c4089ed9107364999ceacd
child 95139 8951d7e2d564ffdc7f7fb8c3947ab7f688a450a8
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs728983
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 728983. Part 2: When display items for multiple frames are merged, track the merged frames and mark them all as having an associated container layer. This ensures that invalidations are processed correctly. r=mattwoodrow
layout/base/FrameLayerBuilder.cpp
layout/base/FrameLayerBuilder.h
layout/reftests/bugs/728983-1-ref.html
layout/reftests/bugs/728983-1.html
layout/reftests/bugs/reftest.list
--- a/layout/base/FrameLayerBuilder.cpp
+++ b/layout/base/FrameLayerBuilder.cpp
@@ -150,19 +150,19 @@ public:
     // When AllowResidualTranslation is false, display items will be drawn
     // scaled with a translation by integer pixels, so we know how the snapping
     // will work.
     mSnappingEnabled = aManager->IsSnappingEffectiveTransforms() &&
       !mParameters.AllowResidualTranslation();
     CollectOldLayers();
   }
 
-  void SetInvalidThebesContent(const nsIntRegion& aRegion)
+  void AddInvalidThebesContent(const nsIntRegion& aRegion)
   {
-    mInvalidThebesContent = aRegion;
+    mInvalidThebesContent.Or(mInvalidThebesContent, aRegion);
   }
   void SetInvalidateAllThebesContent()
   {
     mInvalidateAllThebesContent = true;
   }
   /**
    * This is the method that actually walks a display list and builds
    * the child layers. We invoke it recursively to process clipped sublists.
@@ -202,16 +202,18 @@ public:
   {
     if (aSnap && mSnappingEnabled) {
       return ScaleToNearestPixels(aRect);
     }
     return aRect.ScaleToInsidePixels(mParameters.mXScale, mParameters.mYScale,
                                      mAppUnitsPerDevPixel);
   }
 
+  const FrameLayerBuilder::ContainerParameters& ScaleParameters() { return mParameters; };
+
 protected:
   /**
    * We keep a stack of these to represent the ThebesLayers that are
    * currently available to have display items added to.
    * We use a stack here because as much as possible we want to
    * assign display items to existing ThebesLayers, and to the lowest
    * ThebesLayer in z-order. This reduces the number of layers and
    * makes it more likely a display item will be rendered to an opaque
@@ -495,16 +497,18 @@ FrameLayerBuilder::Init(nsDisplayListBui
   if (mRootPresContext) {
     mInitialDOMGeneration = mRootPresContext->GetDOMGeneration();
   }
 }
 
 bool
 FrameLayerBuilder::DisplayItemDataEntry::HasNonEmptyContainerLayer()
 {
+  if (mIsSharingContainerLayer)
+    return true;
   for (PRUint32 i = 0; i < mData.Length(); ++i) {
     if (mData[i].mLayer->GetType() == Layer::TYPE_CONTAINER &&
         mData[i].mLayerState != LAYER_ACTIVE_EMPTY)
       return true;
   }
   return false;
 }
 
@@ -1186,17 +1190,17 @@ ContainerState::ThebesLayerData::Accumul
                                             const FrameLayerBuilder::Clip& aClip)
 {
   nscolor uniformColor;
   bool isUniform = aItem->IsUniform(aState->mBuilder, &uniformColor);
   
   /* Mark as available for conversion to image layer if this is a nsDisplayImage and
    * we are the first visible item in the ThebesLayerData object.
    */
-  if (aItem->GetType() == nsDisplayItem::TYPE_IMAGE && mVisibleRegion.IsEmpty()) {
+  if (mVisibleRegion.IsEmpty() && aItem->GetType() == nsDisplayItem::TYPE_IMAGE) {
     mImage = static_cast<nsDisplayImage*>(aItem);
     mImageClip = aClip;
   } else {
     mImage = nsnull;
   }
 
   // Some display items have to exist (so they can set forceTransparentSurface
   // below) but don't draw anything. They'll return true for isUniform but
@@ -1824,26 +1828,59 @@ ChooseScaleAndSetTransform(FrameLayerBui
     }
   }
   if (isRetained && (!canDraw2D || transform2d.HasNonIntegerTranslation())) {
     result.mDisableSubpixelAntialiasingInDescendants = true;
   }
   return result;
 }
 
+static void
+ApplyThebesLayerInvalidation(nsDisplayListBuilder* aBuilder,
+                             nsIFrame* aContainerFrame,
+                             nsDisplayItem* aContainerItem,
+                             ContainerState& aState,
+                             nsPoint* aCurrentOffset)
+{
+  FrameProperties props = aContainerFrame->Properties();
+  nsPoint* offsetAtLastPaint = static_cast<nsPoint*>
+    (props.Get(ThebesLayerLastPaintOffsetProperty()));
+  *aCurrentOffset = aContainerItem ? aContainerItem->ToReferenceFrame()
+    : aBuilder->ToReferenceFrame(aContainerFrame);
+
+  nsRegion* invalidThebesContent = static_cast<nsRegion*>
+    (props.Get(ThebesLayerInvalidRegionProperty()));
+  if (invalidThebesContent) {
+    nsPoint offset = offsetAtLastPaint ? *offsetAtLastPaint : *aCurrentOffset;
+    invalidThebesContent->MoveBy(offset);
+    const FrameLayerBuilder::ContainerParameters& scaleParameters = aState.ScaleParameters();
+    aState.AddInvalidThebesContent(invalidThebesContent->
+      ScaleToOutsidePixels(scaleParameters.mXScale, scaleParameters.mYScale,
+                           aState.GetAppUnitsPerDevPixel()));
+    // We have to preserve the current contents of invalidThebesContent
+    // because there might be multiple container layers for the same
+    // frame and we need to invalidate the ThebesLayer children of all
+    // of them.
+    invalidThebesContent->MoveBy(-offset);
+  } else {
+    // The region was deleted to indicate that everything should be
+    // invalidated.
+    aState.SetInvalidateAllThebesContent();
+  }
+}
+
 already_AddRefed<ContainerLayer>
 FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder* aBuilder,
                                           LayerManager* aManager,
                                           nsIFrame* aContainerFrame,
                                           nsDisplayItem* aContainerItem,
                                           const nsDisplayList& aChildren,
                                           const ContainerParameters& aParameters,
                                           const gfx3DMatrix* aTransform)
 {
-  FrameProperties props = aContainerFrame->Properties();
   PRUint32 containerDisplayItemKey =
     aContainerItem ? aContainerItem->GetPerFrameKey() : 0;
   NS_ASSERTION(aContainerFrame, "Container display items here should have a frame");
   NS_ASSERTION(!aContainerItem ||
                aContainerItem->GetUnderlyingFrame() == aContainerFrame,
                "Container display item must match given frame");
 
   nsRefPtr<ContainerLayer> containerLayer;
@@ -1888,40 +1925,37 @@ FrameLayerBuilder::BuildContainerLayerFo
                        scaleParameters);
 
   if (aManager == mRetainingManager) {
     DisplayItemDataEntry* entry = mNewDisplayItemData.PutEntry(aContainerFrame);
     if (entry) {
       entry->mData.AppendElement(
           DisplayItemData(containerLayer, containerDisplayItemKey, LAYER_ACTIVE));
     }
-
-    nsPoint* offsetAtLastPaint = static_cast<nsPoint*>
-      (props.Get(ThebesLayerLastPaintOffsetProperty()));
-    nsPoint currentOffset = aBuilder->ToReferenceFrame(aContainerFrame);
+    nsPoint currentOffset;
+    ApplyThebesLayerInvalidation(aBuilder, aContainerFrame, aContainerItem, state,
+                                 &currentOffset);
+    SetHasContainerLayer(aContainerFrame, currentOffset);
 
-    nsRegion* invalidThebesContent(static_cast<nsRegion*>
-      (props.Get(ThebesLayerInvalidRegionProperty())));
-    if (invalidThebesContent) {
-      nsPoint offset = offsetAtLastPaint ? *offsetAtLastPaint : currentOffset;
-      invalidThebesContent->MoveBy(offset);
-      state.SetInvalidThebesContent(invalidThebesContent->
-        ScaleToOutsidePixels(scaleParameters.mXScale, scaleParameters.mYScale,
-                             state.GetAppUnitsPerDevPixel()));
-      // We have to preserve the current contents of invalidThebesContent
-      // because there might be multiple container layers for the same
-      // frame and we need to invalidate the ThebesLayer children of all
-      // of them.
-      invalidThebesContent->MoveBy(-offset);
-    } else {
-      // The region was deleted to indicate that everything should be
-      // invalidated.
-      state.SetInvalidateAllThebesContent();
+    nsAutoTArray<nsIFrame*,4> mergedFrames;
+    if (aContainerItem) {
+      aContainerItem->GetMergedFrames(&mergedFrames);
     }
-    SetHasContainerLayer(aContainerFrame, currentOffset);
+    for (PRUint32 i = 0; i < mergedFrames.Length(); ++i) {
+      nsIFrame* mergedFrame = mergedFrames[i];
+      DisplayItemDataEntry* entry = mNewDisplayItemData.PutEntry(mergedFrame);
+      if (entry) {
+        // Ensure that UpdateDisplayItemDataForFrame recognizes that we
+        // still have a container layer associated with this frame.
+        entry->mIsSharingContainerLayer = true;
+      }
+      ApplyThebesLayerInvalidation(aBuilder, mergedFrame, nsnull, state,
+                                   &currentOffset);
+      SetHasContainerLayer(mergedFrame, currentOffset);
+    }
   }
 
   Clip clip;
   state.ProcessDisplayItems(aChildren, clip);
 
   // Set CONTENT_COMPONENT_ALPHA if any of our children have it.
   // This is suboptimal ... a child could have text that's over transparent
   // pixels in its own layer, but over opaque parts of previous siblings.
--- a/layout/base/FrameLayerBuilder.h
+++ b/layout/base/FrameLayerBuilder.h
@@ -435,28 +435,29 @@ protected:
 
   /**
    * We accumulate DisplayItemData elements in a hashtable during
    * the paint process, and store them in the frame property only when
    * paint is complete. This is the hashentry for that hashtable.
    */
   class DisplayItemDataEntry : public nsPtrHashKey<nsIFrame> {
   public:
-    DisplayItemDataEntry(const nsIFrame *key) : nsPtrHashKey<nsIFrame>(key) {}
+    DisplayItemDataEntry(const nsIFrame *key) : nsPtrHashKey<nsIFrame>(key), mIsSharingContainerLayer(false) {}
     DisplayItemDataEntry(DisplayItemDataEntry &toCopy) :
-      nsPtrHashKey<nsIFrame>(toCopy.mKey)
+      nsPtrHashKey<nsIFrame>(toCopy.mKey), mIsSharingContainerLayer(toCopy.mIsSharingContainerLayer)
     {
       // This isn't actually a copy-constructor; notice that it steals toCopy's
       // array.  Be careful.
       mData.SwapElements(toCopy.mData);
     }
 
     bool HasNonEmptyContainerLayer();
 
     nsAutoTArray<DisplayItemData, 1> mData;
+    bool mIsSharingContainerLayer;
 
     enum { ALLOW_MEMMOVE = false };
   };
 
   // LayerManagerData needs to see DisplayItemDataEntry.
   friend class LayerManagerData;
 
   // Flash the area within the context clip if paint flashing is enabled.
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/728983-1-ref.html
@@ -0,0 +1,21 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<body>
+<div style="-moz-column-count:2; column-count:2; width:300px; height:100px;">
+  <div id="o" style="opacity:0.5; width:100px; height:200px; background:lime;">
+	<div id="d" style="height:50px; width:80px; background:green; padding:2px">Text</div>
+  </div>
+</div>
+<script>
+var o = document.getElementById("o");
+var d = document.getElementById("d");
+document.body.getBoundingClientRect();
+o.style.opacity = 0.8;
+function doTest() {
+  o.style.opacity = 0.9;
+  document.documentElement.removeAttribute("class");
+}
+window.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/728983-1.html
@@ -0,0 +1,23 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<body>
+<div style="-moz-column-count:2; column-count:2; width:300px; height:100px;">
+  <div id="o" style="opacity:0.5; width:100px; height:200px; background:lime;">
+	<div id="d" style="height:50px; width:80px; background:red; padding:2px">Text</div>
+  </div>
+</div>
+<script>
+var o = document.getElementById("o");
+var d = document.getElementById("d");
+document.body.getBoundingClientRect();
+o.style.opacity = 0.8;
+function doTest() {
+  o.style.opacity = 0.9;
+  document.body.getBoundingClientRect();
+  d.style.background = "green";
+  document.documentElement.removeAttribute("class");
+}
+window.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1689,12 +1689,13 @@ needs-focus != 703186-1.html 703186-2.ht
 == 714519-1-as.html 714519-1-ref.html
 == 714519-1-q.html 714519-1-ref.html
 == 714519-2-as.html 714519-2-ref.html
 == 714519-2-q.html 714519-2-ref.html
 fuzzy-if(d2d,1,19) fuzzy-if(cocoaWidget,1,170) == 718521.html 718521-ref.html
 == 720987.html 720987-ref.html
 == 722923-1.html 722923-1-ref.html
 == 723484-1.html 723484-1-ref.html
+== 728983-1.html 728983-1-ref.html
 == 729143-1.html 729143-1-ref.html
 == 731521-1.html 731521-1-ref.html
 needs-focus == 731726-1.html 731726-1-ref.html
 == 735481-1.html 735481-1-ref.html