Bug 1514384 - Properly make CSS clip property clip filters as well. r=mstange
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 10 Jan 2019 20:47:36 +0100
changeset 453612 0026b863c4371ed9856e27b31fb1a61d7ce1e42b
parent 453611 8b61f3ad2ea3ecf97586304fcca6d98eb1508d4b
child 453613 c74c81fe8dcbd39150889d8fe83b93c2ef2344e3
push id35361
push usernbeleuzu@mozilla.com
push dateSat, 12 Jan 2019 09:41:19 +0000
treeherdermozilla-central@a44934afe25e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1514384
milestone66.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 1514384 - Properly make CSS clip property clip filters as well. r=mstange This fixes a bug that my previous patch uncovers, and that's tested by css/css-masking/clip/clip-filter-order.html. We weren't setting up the clips in a way that the clip property clipped filters and such. This test works in Gecko because Gecko won't draw outside of the effect bounds, that account for clip(). Differential Revision: https://phabricator.services.mozilla.com/D16232
layout/generic/nsFrame.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2921,17 +2921,30 @@ void nsIFrame::BuildDisplayListForStacki
     eSeparatorTransforms,
     eOpacity,
     eFilter,
     eBlendContainer
   };
 
   nsDisplayListBuilder::AutoContainerASRTracker contASRTracker(aBuilder);
 
-  DisplayListClipState::AutoSaveRestore clipState(aBuilder);
+  DisplayListClipState::AutoSaveRestore cssClip(aBuilder);
+  {
+    // The clip property clips everything, including filters and what not.
+    if (auto contentClip = GetClipPropClipRect(disp, effects, GetSize())) {
+      nsPoint offset = isTransformed
+                           ? GetOffsetToCrossDoc(
+                                 aBuilder->FindReferenceFrameFor(GetParent()))
+                           : aBuilder->GetCurrentFrameOffsetToReferenceFrame();
+
+      aBuilder->IntersectDirtyRect(*contentClip);
+      aBuilder->IntersectVisibleRect(*contentClip);
+      cssClip.ClipContentDescendants(*contentClip + offset);
+    }
+  }
 
   // If there is a current clip, then depending on the container items we
   // create, different things can happen to it. Some container items simply
   // propagate the clip to their children and aren't clipped themselves.
   // But other container items, especially those that establish a different
   // geometry for their contents (e.g. transforms), capture the clip on
   // themselves and unset the clip for their contents. If we create more than
   // one of those container items, the clip will be captured on the outermost
@@ -2954,48 +2967,42 @@ void nsIFrame::BuildDisplayListForStacki
       clipCapturedBy = ContainerItemType::ePerspective;
     } else {
       clipCapturedBy = ContainerItemType::eTransform;
     }
   } else if (usingFilter) {
     clipCapturedBy = ContainerItemType::eFilter;
   }
 
+  DisplayListClipState::AutoSaveRestore clipState(aBuilder);
   if (clipCapturedBy != ContainerItemType::eNone) {
     clipState.Clear();
   }
 
-  Maybe<nsRect> clipForMask;
-  if (usingMask) {
-    clipForMask = ComputeClipForMaskItem(aBuilder, this);
-  }
-
   mozilla::UniquePtr<HitTestInfo> hitTestInfo;
 
   nsDisplayListCollection set(aBuilder);
+  Maybe<nsRect> clipForMask;
   {
     DisplayListClipState::AutoSaveRestore nestedClipState(aBuilder);
     nsDisplayListBuilder::AutoInTransformSetter inTransformSetter(aBuilder,
                                                                   inTransform);
     nsDisplayListBuilder::AutoEnterFilter filterASRSetter(aBuilder,
                                                           usingFilter);
 
     CheckForApzAwareEventHandlers(aBuilder, this);
 
-    Maybe<nsRect> contentClip = GetClipPropClipRect(disp, effects, GetSize());
-
     if (usingMask) {
-      contentClip = IntersectMaybeRects(contentClip, clipForMask);
-    }
-
-    if (contentClip) {
-      aBuilder->IntersectDirtyRect(*contentClip);
-      aBuilder->IntersectVisibleRect(*contentClip);
-      nestedClipState.ClipContentDescendants(*contentClip +
-                                             aBuilder->ToReferenceFrame(this));
+      clipForMask = ComputeClipForMaskItem(aBuilder, this);
+      if (clipForMask) {
+        aBuilder->IntersectDirtyRect(*clipForMask);
+        aBuilder->IntersectVisibleRect(*clipForMask);
+        nestedClipState.ClipContentDescendants(
+            *clipForMask + aBuilder->GetCurrentFrameOffsetToReferenceFrame());
+      }
     }
 
     // extend3DContext also guarantees that applyAbsPosClipping and
     // usingSVGEffects are false We only modify the preserve-3d rect if we are
     // the top of a preserve-3d heirarchy
     if (extend3DContext) {
       // Mark these first so MarkAbsoluteFramesForDisplayList knows if we are
       // going to be forced to descend into frames.