Back out bug 1164227, because bug 1236043 fixes the original problem in a better way.
authorMarkus Stange <mstange@themasta.com>
Mon, 28 Mar 2016 18:37:07 -0400
changeset 331526 f5de44ecf07f3cbd369015663b72d452fda6d177
parent 331525 be713202544ac37a9959a9b3881d7a0905b31b4f
child 331527 bdb163d0ba98209e2a1dde73ac9b923d98553ef7
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1164227, 1236043
milestone48.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
Back out bug 1164227, because bug 1236043 fixes the original problem in a better way. MozReview-Commit-ID: F4bD1MvOWDR
layout/base/FrameLayerBuilder.cpp
layout/base/nsDisplayList.cpp
layout/base/nsDisplayList.h
layout/generic/nsGfxScrollFrame.cpp
--- a/layout/base/FrameLayerBuilder.cpp
+++ b/layout/base/FrameLayerBuilder.cpp
@@ -1482,26 +1482,16 @@ public:
   nsIntRegion mRegionToInvalidate;
 
   // The offset between the active scrolled root of this layer
   // and the root of the container for the previous and current
   // paints respectively.
   nsPoint mLastAnimatedGeometryRootOrigin;
   nsPoint mAnimatedGeometryRootOrigin;
 
-  // If mIgnoreInvalidationsOutsideRect is set, this contains the bounds of the
-  // layer's old visible region, in layer pixels.
-  nsIntRect mOldVisibleBounds;
-
-  // If set, invalidations that fall outside of this rect should not result in
-  // calls to layer->InvalidateRegion during DLBI. Instead, the parts outside
-  // this rectangle will be invalidated in InvalidateVisibleBoundsChangesForScrolledLayer.
-  // See the comment in ComputeAndSetIgnoreInvalidationRect for more information.
-  Maybe<nsIntRect> mIgnoreInvalidationsOutsideRect;
-
   RefPtr<ColorLayer> mColorLayer;
   RefPtr<ImageLayer> mImageLayer;
 
   // The region for which display item visibility for this layer has already
   // been calculated. Used to reduce the number of calls to
   // RecomputeVisibilityForItems if it is known in advance that a larger
   // region will be painted during a transaction than in a single call to
   // DrawPaintedLayer, for example when progressive paint is enabled.
@@ -1640,53 +1630,47 @@ AppendToString(nsACString& s, const nsIn
   return s += sfx;
 }
 
 /**
  * Invalidate aRegion in aLayer. aLayer is in the coordinate system
  * *after* aTranslation has been applied, so we need to
  * apply the inverse of that transform before calling InvalidateRegion.
  */
-template<typename RegionOrRect> void
-InvalidatePostTransformRegion(PaintedLayer* aLayer, const RegionOrRect& aRegion,
-                              const nsIntPoint& aTranslation,
-                              PaintedDisplayItemLayerUserData* aData)
+static void
+InvalidatePostTransformRegion(PaintedLayer* aLayer, const nsIntRegion& aRegion,
+                              const nsIntPoint& aTranslation)
 {
   // Convert the region from the coordinates of the container layer
   // (relative to the snapped top-left of the display list reference frame)
   // to the PaintedLayer's own coordinates
-  RegionOrRect rgn = aRegion;
+  nsIntRegion rgn = aRegion;
   rgn.MoveBy(-aTranslation);
-  if (aData->mIgnoreInvalidationsOutsideRect) {
-    rgn = rgn.Intersect(*aData->mIgnoreInvalidationsOutsideRect);
-  }
-  if (!rgn.IsEmpty()) {
-    aLayer->InvalidateRegion(rgn);
+  aLayer->InvalidateRegion(rgn);
 #ifdef MOZ_DUMP_PAINTING
-    if (nsLayoutUtils::InvalidationDebuggingIsEnabled()) {
-      nsAutoCString str;
-      AppendToString(str, rgn);
-      printf_stderr("Invalidating layer %p: %s\n", aLayer, str.get());
-    }
+  if (nsLayoutUtils::InvalidationDebuggingIsEnabled()) {
+    nsAutoCString str;
+    AppendToString(str, rgn);
+    printf_stderr("Invalidating layer %p: %s\n", aLayer, str.get());
+  }
 #endif
-  }
 }
 
 static void
 InvalidatePostTransformRegion(PaintedLayer* aLayer, const nsRect& aRect,
                               const DisplayItemClip& aClip,
                               const nsIntPoint& aTranslation)
 {
   PaintedDisplayItemLayerUserData* data =
       static_cast<PaintedDisplayItemLayerUserData*>(aLayer->GetUserData(&gPaintedDisplayItemLayerUserData));
 
   nsRect rect = aClip.ApplyNonRoundedIntersection(aRect);
 
   nsIntRect pixelRect = rect.ScaleToOutsidePixels(data->mXScale, data->mYScale, data->mAppUnitsPerDevPixel);
-  InvalidatePostTransformRegion(aLayer, pixelRect, aTranslation, data);
+  InvalidatePostTransformRegion(aLayer, pixelRect, aTranslation);
 }
 
 
 static nsIntPoint
 GetTranslationForPaintedLayer(PaintedLayer* aLayer)
 {
   PaintedDisplayItemLayerUserData* data =
     static_cast<PaintedDisplayItemLayerUserData*>
@@ -2235,99 +2219,16 @@ ContainerState::RecyclePaintedLayer(Pain
       printf_stderr("Invalidating layer %p: %s\n", aLayer, str.get());
     }
 #endif
     data->mRegionToInvalidate.SetEmpty();
   }
   return data;
 }
 
-static void
-ComputeAndSetIgnoreInvalidationRect(PaintedLayer* aLayer,
-                                    PaintedDisplayItemLayerUserData* aData,
-                                    AnimatedGeometryRoot* aAnimatedGeometryRoot,
-                                    nsDisplayListBuilder* aBuilder,
-                                    const nsIntPoint& aLayerTranslation)
-{
-  if (!aLayer->Manager()->IsWidgetLayerManager()) {
-    // This optimization is only useful for layers with retained content.
-    return;
-  }
-
-  const nsIFrame* parentFrame = (*aAnimatedGeometryRoot)->GetParent();
-
-  // GetDirtyRectForScrolledContents will return an empty rect if parentFrame
-  // is not a scrollable frame.
-  nsRect dirtyRect = aBuilder->GetDirtyRectForScrolledContents(parentFrame);
-
-  if (dirtyRect.IsEmpty()) {
-    // parentFrame is not a scrollable frame, or we didn't encounter it during
-    // display list building (though this shouldn't happen), or it's empty.
-    // In all those cases this optimization is not needed.
-    return;
-  }
-
-  // parentFrame is a scrollable frame, and aLayer contains the scrolled
-  // contents of that frame.
-
-  // maxNewVisibleBounds is a conservative approximation of the new visible
-  // region of aLayer.
-  nsIntRect maxNewVisibleBounds =
-    dirtyRect.ScaleToOutsidePixels(aData->mXScale, aData->mYScale,
-                                   aData->mAppUnitsPerDevPixel) - aLayerTranslation;
-  aData->mOldVisibleBounds = aLayer->GetVisibleRegion().ToUnknownRegion().GetBounds();
-
-  // When the visible region of aLayer changes (e.g. due to scrolling),
-  // three distinct types of invalidations need to be triggered:
-  //  (1) Items (or parts of items) that have left the visible region need
-  //      to be invalidated so that the pixels they painted are no longer
-  //      part of the layer's valid region.
-  //  (2) Items (or parts of items) that weren't in the old visible region
-  //      but are in the new visible region need to be invalidated. This
-  //      invalidation isn't required for painting the right layer
-  //      contents, because these items weren't part of the layer's valid
-  //      region, so they'd be painted anyway. It is, however, necessary in
-  //      order to get an accurate invalid region for the layer tree that
-  //      aLayer is in, for example for partial compositing.
-  //  (3) Any changes that happened in the intersection of the old and the
-  //      new visible region need to be invalidated. There shouldn't be any
-  //      of these when scrolling static content.
-  //
-  // We'd like to guarantee that we won't invalidate anything in the
-  // intersection area of the old and the new visible region if all
-  // invalidation are of type (1) and (2). However, if we just call
-  // aLayer->InvalidateRegion for the invalidations of type (1) and (2),
-  // at some point we'll hit the complexity limit of the layer's invalid
-  // region. And the resulting region simplification can cause the region
-  // to intersect with the intersection of the old and the new visible
-  // region.
-  // In order to get around this problem, we're using the following approach:
-  //  - aData->mIgnoreInvalidationsOutsideRect is set to a conservative
-  //    approximation of the intersection of the old and the new visible
-  //    region. At this point we don't know the layer's new visible region.
-  //  - As long as we don't know the layer's new visible region, we ignore all
-  //    invalidations outside that rectangle, so roughly some of the
-  //    invalidations of type (1) and (2).
-  //  - Once we know the layer's new visible region, which happens at some
-  //    point during PostprocessRetainedLayers, we invalidate a conservative
-  //    approximation of (1) and (2). Specifically, we invalidate the region
-  //    union of the old visible bounds and the new visible bounds, minus
-  //    aData->mIgnoreInvalidationsOutsideRect. That region is simple enough
-  //    that it will never be simplified on its own.
-  //    We unset mIgnoreInvalidationsOutsideRect at this point.
-  //  - Any other invalidations that happen on the layer after this point, e.g.
-  //    during WillEndTransaction, will just happen regularly. If they are of
-  //    type (1) or (2), they won't change the layer's invalid region because
-  //    they fall inside the region we invalidated in the previous step.
-  // Consequently, aData->mIgnoreInvalidationsOutsideRect is safe from
-  // invalidations as long as there are no invalidations of type (3).
-  aData->mIgnoreInvalidationsOutsideRect =
-    Some(maxNewVisibleBounds.Intersect(aData->mOldVisibleBounds));
-}
-
 void
 ContainerState::PreparePaintedLayerForUse(PaintedLayer* aLayer,
                                           PaintedDisplayItemLayerUserData* aData,
                                           AnimatedGeometryRoot* aAnimatedGeometryRoot,
                                           const nsIFrame* aReferenceFrame,
                                           const nsPoint& aTopLeft,
                                           bool didResetScrollPositionForLayerPixelAlignment)
 {
@@ -2351,18 +2252,16 @@ ContainerState::PreparePaintedLayerForUs
   // is close to aData->mAnimatedGeometryRootPosition if possible.
   nsIntPoint pixOffset(RoundToMatchResidual(scaledOffset.x, aData->mAnimatedGeometryRootPosition.x),
                        RoundToMatchResidual(scaledOffset.y, aData->mAnimatedGeometryRootPosition.y));
   aData->mTranslation = pixOffset;
   pixOffset += mParameters.mOffset;
   Matrix matrix = Matrix::Translation(pixOffset.x, pixOffset.y);
   aLayer->SetBaseTransform(Matrix4x4::From2D(matrix));
 
-  ComputeAndSetIgnoreInvalidationRect(aLayer, aData, aAnimatedGeometryRoot, mBuilder, pixOffset);
-
   aData->mVisibilityComputedRegion.SetEmpty();
 
   // FIXME: Temporary workaround for bug 681192 and bug 724786.
 #ifndef MOZ_WIDGET_ANDROID
   // Calculate exact position of the top-left of the active scrolled root.
   // This might not be 0,0 due to the snapping in ScaleToNearestPixels.
   gfxPoint animatedGeometryRootTopLeft = scaledOffset - ThebesPoint(matrix.GetTranslation()) + mParameters.mOffset;
   // If it has changed, then we need to invalidate the entire layer since the
@@ -4377,18 +4276,17 @@ FrameLayerBuilder::ComputeGeometryChange
 #endif
   }
   if (!combined.IsEmpty()) {
     if (notifyRenderingChanged) {
       item->NotifyRenderingChanged();
     }
     InvalidatePostTransformRegion(paintedLayer,
         combined.ScaleToOutsidePixels(layerData->mXScale, layerData->mYScale, layerData->mAppUnitsPerDevPixel),
-        layerData->mTranslation,
-        layerData);
+        layerData->mTranslation);
   }
 
   aData->EndUpdate(geometry);
 }
 
 void
 FrameLayerBuilder::AddPaintedDisplayItem(PaintedLayerData* aLayerData,
                                         nsDisplayItem* aItem,
@@ -4507,18 +4405,17 @@ FrameLayerBuilder::AddPaintedDisplayItem
 #endif
         invalid.ScaleRoundOut(paintedData->mXScale, paintedData->mYScale);
 
         if (hasClip) {
           invalid.And(invalid, intClip);
         }
 
         InvalidatePostTransformRegion(layer, invalid,
-                                      GetTranslationForPaintedLayer(layer),
-                                      paintedData);
+                                      GetTranslationForPaintedLayer(layer));
       }
     }
     ClippedDisplayItem* cdi =
       entry->mItems.AppendElement(ClippedDisplayItem(aItem,
                                                      mContainerLayerGeneration));
     cdi->mInactiveLayerManager = tempManager;
   }
 }
@@ -4770,49 +4667,16 @@ ContainerState::SetupScrollingMetadata(N
     metricsArray.AppendElement(*metadata);
   }
 
   // Watch out for FrameMetrics copies in profiles
   aEntry->mLayer->SetScrollMetadata(metricsArray);
   aEntry->mLayer->SetAncestorMaskLayers(maskLayers);
 }
 
-static void
-InvalidateVisibleBoundsChangesForScrolledLayer(PaintedLayer* aLayer)
-{
-  PaintedDisplayItemLayerUserData* data =
-    static_cast<PaintedDisplayItemLayerUserData*>(aLayer->GetUserData(&gPaintedDisplayItemLayerUserData));
-
-  if (data->mIgnoreInvalidationsOutsideRect) {
-    // We haven't invalidated anything outside *data->mIgnoreInvalidationsOutsideRect
-    // during DLBI. Now is the right time to do that, because at this point aLayer
-    // knows its new visible region.
-    // We use the visible regions' bounds here (as opposed to the true region)
-    // in order to limit rgn's complexity. The only possible disadvantage of
-    // this is that it might cause us to unnecessarily recomposite parts of the
-    // window that are in the visible region's bounds but not in the visible
-    // region itself, but that is acceptable for scrolled layers.
-    nsIntRegion rgn;
-    rgn.Or(data->mOldVisibleBounds, aLayer->GetVisibleRegion().ToUnknownRegion().GetBounds());
-    rgn.Sub(rgn, *data->mIgnoreInvalidationsOutsideRect);
-    if (!rgn.IsEmpty()) {
-      aLayer->InvalidateRegion(rgn);
-#ifdef MOZ_DUMP_PAINTING
-      if (nsLayoutUtils::InvalidationDebuggingIsEnabled()) {
-        printf_stderr("Invalidating changes of the visible region bounds of the scrolled contents\n");
-        nsAutoCString str;
-        AppendToString(str, rgn);
-        printf_stderr("Invalidating layer %p: %s\n", aLayer, str.get());
-      }
-#endif
-    }
-    data->mIgnoreInvalidationsOutsideRect = Nothing();
-  }
-}
-
 static inline const Maybe<ParentLayerIntRect>&
 GetStationaryClipInContainer(Layer* aLayer)
 {
   if (size_t metricsCount = aLayer->GetScrollMetadataCount()) {
     return aLayer->GetScrollMetadata(metricsCount - 1).GetClipRect();
   }
   return aLayer->GetClipRect();
 }
@@ -4851,21 +4715,16 @@ ContainerState::PostprocessRetainedLayer
       }
     }
 
     SetOuterVisibleRegionForLayer(e->mLayer,
                                   e->mVisibleRegion,
                                   e->mLayerContentsVisibleRect.width >= 0 ? &e->mLayerContentsVisibleRect : nullptr,
                                   e->mUntransformedVisibleRegion);
 
-    PaintedLayer* p = e->mLayer->AsPaintedLayer();
-    if (p) {
-      InvalidateVisibleBoundsChangesForScrolledLayer(p);
-    }
-
     if (!e->mOpaqueRegion.IsEmpty()) {
       AnimatedGeometryRoot* animatedGeometryRootToCover = animatedGeometryRootForOpaqueness;
       if (e->mOpaqueForAnimatedGeometryRootParent &&
           e->mAnimatedGeometryRoot->mParentAGR == mContainerAnimatedGeometryRoot) {
         animatedGeometryRootToCover = mContainerAnimatedGeometryRoot;
         data = FindOpaqueRegionEntry(opaqueRegions, animatedGeometryRootToCover);
       }
 
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -1428,34 +1428,16 @@ nsDisplayListBuilder::ExitSVGEffectsCont
 void
 nsDisplayListBuilder::AppendNewScrollInfoItemForHoisting(nsDisplayScrollInfoLayer* aScrollInfoItem)
 {
   MOZ_ASSERT(ShouldBuildScrollInfoItemsForHoisting());
   MOZ_ASSERT(mScrollInfoItemsForHoisting);
   mScrollInfoItemsForHoisting->AppendNewToTop(aScrollInfoItem);
 }
 
-void
-nsDisplayListBuilder::StoreDirtyRectForScrolledContents(const nsIFrame* aScrollableFrame,
-                                                        const nsRect& aDirty)
-{
-  mDirtyRectForScrolledContents.Put(const_cast<nsIFrame*>(aScrollableFrame),
-                                    aDirty + ToReferenceFrame(aScrollableFrame));
-}
-
-nsRect
-nsDisplayListBuilder::GetDirtyRectForScrolledContents(const nsIFrame* aScrollableFrame) const
-{
-  nsRect result;
-  if (!mDirtyRectForScrolledContents.Get(const_cast<nsIFrame*>(aScrollableFrame), &result)) {
-    return nsRect();
-  }
-  return result;
-}
-
 bool
 nsDisplayListBuilder::IsBuildingLayerEventRegions()
 {
   if (IsPaintingToWindow()) {
     // Note: this function and LayerEventRegionsEnabled are the only places
     // that get to query LayoutEventRegionsEnabled 'directly' - other code
     // should call this function.
     return gfxPrefs::LayoutEventRegionsEnabledDoNotUseDirectly() ||
--- a/layout/base/nsDisplayList.h
+++ b/layout/base/nsDisplayList.h
@@ -1065,32 +1065,16 @@ public:
   void ExitSVGEffectsContents();
 
   bool ShouldBuildScrollInfoItemsForHoisting() const
   { return mSVGEffectsBuildingDepth > 0; }
 
   void AppendNewScrollInfoItemForHoisting(nsDisplayScrollInfoLayer* aScrollInfoItem);
 
   /**
-   * Store the dirty rect of the scrolled contents of aScrollableFrame. This
-   * is a bound for the extents of the new visible region of the scrolled
-   * layer.
-   * @param aScrollableFrame the scrollable frame
-   * @param aDirty           the dirty rect, relative to aScrollableFrame
-   */
-  void StoreDirtyRectForScrolledContents(const nsIFrame* aScrollableFrame, const nsRect& aDirty);
-
-  /**
-   * Retrieve the stored dirty rect for the scrolled contents of aScrollableFrame.
-   * @param  aScrollableFrame the scroll frame
-   * @return                  the dirty rect, relative to aScrollableFrame's *reference frame*
-   */
-  nsRect GetDirtyRectForScrolledContents(const nsIFrame* aScrollableFrame) const;
-
-  /**
    * A helper class to install/restore nsDisplayListBuilder::mPreserves3DCtx.
    *
    * mPreserves3DCtx is used by class AutoAccumulateTransform &
    * AutoAccumulateRect to passing data between frames in the 3D
    * context.  If a frame create a new 3D context, it should restore
    * the value of mPreserves3DCtx before returning back to the parent.
    * This class do it for the users.
    */
@@ -1216,19 +1200,16 @@ private:
   // and thus is in-budget.
   nsTHashtable<nsPtrHashKey<nsIFrame> > mWillChangeBudgetSet;
 
   // Area of animated geometry root budget already allocated
   uint32_t mUsedAGRBudget;
   // Set of frames already counted in budget
   nsTHashtable<nsPtrHashKey<nsIFrame> > mAGRBudgetSet;
 
-  // rects are relative to the frame's reference frame
-  nsDataHashtable<nsPtrHashKey<nsIFrame>, nsRect> mDirtyRectForScrolledContents;
-
   // Relative to mCurrentFrame.
   nsRect                         mDirtyRect;
   nsRegion                       mWindowExcludeGlassRegion;
   nsRegion                       mWindowOpaqueRegion;
   LayoutDeviceIntRegion          mWindowDraggingRegion;
   LayoutDeviceIntRegion          mWindowNoDraggingRegion;
   // The display item for the Windows window glass background, if any
   nsDisplayItem*                 mGlassDisplayItem;
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -3331,17 +3331,16 @@ ScrollFrameHelper::BuildDisplayList(nsDi
         if (mClipAllDescendants) {
           inactiveScrollClip = clipStateForScrollClip.InsertInactiveScrollClipForContentDescendants(aBuilder, sf);
         } else {
           inactiveScrollClip = clipStateForScrollClip.InsertInactiveScrollClipForContainingBlockDescendants(aBuilder, sf);
         }
         MOZ_ASSERT(!inactiveScrollClip->mIsAsyncScrollable);
       }
 
-      aBuilder->StoreDirtyRectForScrolledContents(mOuter, dirtyRect);
       mOuter->BuildDisplayListForChild(aBuilder, mScrolledFrame, dirtyRect, scrolledContent);
     }
 
     if (contentBoxClipForNonCaretContent) {
       DisplayListClipState::AutoSaveRestore contentBoxClipState(aBuilder);
       if (mClipAllDescendants) {
         contentBoxClipState.ClipContentDescendants(*contentBoxClipForNonCaretContent);
       } else {