Bug 1459065 - Clip filter effects at the stacking context level. r=mstange
☠☠ backed out by 48028d39f5ed ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 27 Aug 2018 19:23:34 +0000
changeset 488651 f4739ab0ece1b6014242b7b0c49e88d46ec2146f
parent 488650 febddb5a4dc21c7b09b02efc39568a03588d893b
child 488652 3bf4771b88cba6f70e0192f15637ce7f9eadc64d
push id9734
push usershindli@mozilla.com
push dateThu, 30 Aug 2018 12:18:07 +0000
treeherdermozilla-beta@71c71ab3afae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1459065
milestone63.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 1459065 - Clip filter effects at the stacking context level. r=mstange Much like mask images. This is the easy fix, for now. We need to override the ASR clips with Nothing() because we don't really want children of this display item to get the parent filter applied. It's not only redundant, but also may be incorrect if the mask image is not opaque for example (maybe WR should prevent that?). This was caught by layout/reftests/w3c-css/submitted/masking/mask-opacity-1a.html Differential Revision: https://phabricator.services.mozilla.com/D4351
layout/painting/nsDisplayList.cpp
layout/reftests/svg/filters/css-filters/reftest.list
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/css/css-filter/blur-clip-stacking-context-001.html
testing/web-platform/tests/css/css-filter/blur-clip-stacking-context-002.html
testing/web-platform/tests/css/css-filter/blur-clip-stacking-context-ref.html
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -9416,18 +9416,18 @@ bool
 nsDisplayMask::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
                                        mozilla::wr::IpcResourceUpdateQueue& aResources,
                                        const StackingContextHelper& aSc,
                                        mozilla::layers::WebRenderLayerManager* aManager,
                                        nsDisplayListBuilder* aDisplayListBuilder)
 {
   bool snap;
   float appUnitsPerDevPixel = mFrame->PresContext()->AppUnitsPerDevPixel();
-  nsRect displayBound = GetBounds(aDisplayListBuilder, &snap);
-  LayoutDeviceRect bounds = LayoutDeviceRect::FromAppUnits(displayBound, appUnitsPerDevPixel);
+  nsRect displayBounds = GetBounds(aDisplayListBuilder, &snap);
+  LayoutDeviceRect bounds = LayoutDeviceRect::FromAppUnits(displayBounds, appUnitsPerDevPixel);
 
   Maybe<wr::WrImageMask> mask = aManager->CommandBuilder().BuildWrMaskImage(this, aBuilder, aResources,
                                                                             aSc, aDisplayListBuilder,
                                                                             bounds);
   Maybe<StackingContextHelper> layer;
   const StackingContextHelper* sc = &aSc;
   if (mask) {
     auto layoutBounds = wr::ToRoundedLayoutRect(bounds);
@@ -9450,17 +9450,19 @@ nsDisplayMask::CreateWebRenderCommands(m
                   /*aTransform: */ nullptr,
                   /*aPerspective: */ nullptr,
                   /*aMixBlendMode: */ gfx::CompositionOp::OP_OVER,
                   /*aBackfaceVisible: */ true,
                   /*aIsPreserve3D: */ false,
                   /*aTransformForScrollData: */ Nothing(),
                   /*aClipNodeId: */ &clipId);
     sc = layer.ptr();
-    aManager->CommandBuilder().PushOverrideForASR(GetActiveScrolledRoot(), Some(clipId));
+    // The whole stacking context will be clipped by us, so no need to have any
+    // parent for the children context's clip.
+    aManager->CommandBuilder().PushOverrideForASR(GetActiveScrolledRoot(), Nothing());
   }
 
   nsDisplaySVGEffects::CreateWebRenderCommands(aBuilder, aResources, *sc, aManager, aDisplayListBuilder);
 
   if (mask) {
     aManager->CommandBuilder().PopOverrideForASR(GetActiveScrolledRoot());
   }
 
@@ -9730,21 +9732,51 @@ nsDisplayFilter::CreateWebRenderCommands
         wrFilters.AppendElement(filterOp);
         break;
       }
       default:
         return false;
     }
   }
 
+  bool snap;
+  float auPerDevPixel = mFrame->PresContext()->AppUnitsPerDevPixel();
+  nsRect displayBounds = GetBounds(aDisplayListBuilder, &snap);
+  auto bounds = LayoutDeviceRect::FromAppUnits(displayBounds, auPerDevPixel);
+  // NOTE(emilio): this clip is going to be intersected with the clip that's
+  // currently on the clip stack for this item.
+  //
+  // FIXME(emilio, bug 1486557): clipping to "bounds" isn't really necessary.
+  wr::WrClipId clipId =
+    aBuilder.DefineClip(Nothing(), wr::ToRoundedLayoutRect(bounds));
+
   float opacity = mFrame->StyleEffects()->mOpacity;
-  StackingContextHelper sc(aSc, aBuilder, wrFilters, LayoutDeviceRect(), nullptr,
-                           nullptr, opacity != 1.0f && mHandleOpacity ? &opacity : nullptr);
+  StackingContextHelper sc(
+      aSc,
+      aBuilder,
+      wrFilters,
+      LayoutDeviceRect(),
+      nullptr,
+      nullptr,
+      opacity != 1.0f && mHandleOpacity ? &opacity : nullptr,
+      nullptr,
+      nullptr,
+      gfx::CompositionOp::OP_OVER,
+      true,
+      false,
+      Nothing(),
+      &clipId);
+
+  // The whole stacking context will be clipped by us, so no need to have any
+  // parent for the children context's clip.
+  aManager->CommandBuilder().PushOverrideForASR(GetActiveScrolledRoot(), Nothing());
 
   nsDisplaySVGEffects::CreateWebRenderCommands(aBuilder, aResources, sc, aManager, aDisplayListBuilder);
+
+  aManager->CommandBuilder().PopOverrideForASR(GetActiveScrolledRoot());
   return true;
 }
 
 #ifdef MOZ_DUMP_PAINTING
 void
 nsDisplayFilter::PrintEffects(nsACString& aTo)
 {
   nsIFrame* firstFrame =
--- a/layout/reftests/svg/filters/css-filters/reftest.list
+++ b/layout/reftests/svg/filters/css-filters/reftest.list
@@ -2,22 +2,22 @@
 # e.g. filter: blur(3px)
 
 default-preferences pref(layout.css.filters.enabled,true)
 
 fuzzy-if(webrender,9-9,4780-4784) == blur.html blur-ref.html
 == blur.svg blur-ref.svg
 == blur-calc.html blur-calc-ref.html
 == blur-calc-negative.html blur-calc-negative-ref.html
-skip-if(d2d) == blur-cap-large-radius-on-software.html blur-cap-large-radius-on-software-ref.html
+fuzzy-if(webrender,3-3,55913-55913) skip-if(d2d) == blur-cap-large-radius-on-software.html blur-cap-large-radius-on-software-ref.html
 fuzzy-if(webrender,9-9,4780-4784) == blur-em-radius.html blur-em-radius-ref.html
 == blur-invalid-radius.html blur-invalid-radius-ref.html
 fuzzy-if(webrender,9-9,4780-4784) == blur-rem-radius.html blur-rem-radius-ref.html
 == blur-zero-radius.html blur-zero-radius-ref.html
-fuzzy-if(webrender,6-7,20420-21308) == blur-zoomed-page.html blur-zoomed-page-ref.html
+fuzzy-if(webrender,5-7,20420-21308) == blur-zoomed-page.html blur-zoomed-page-ref.html
 == brightness.html brightness-ref.html
 == brightness-darken.html brightness-darken-ref.html
 == brightness-extreme.html brightness-extreme-ref.html
 == brightness-one.html brightness-one-ref.html
 == brightness-percent.html brightness-percent-ref.html
 == brightness-zero.html brightness-zero-ref.html
 == containing-block-1.html containing-block-1-ref.html
 == contrast.html contrast-ref.html
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -112910,16 +112910,40 @@
       [
        "/css/css-fill-stroke/reference/paint-order-001-ref.tentative.html",
        "=="
       ]
      ],
      {}
     ]
    ],
+   "css/css-filter/blur-clip-stacking-context-001.html": [
+    [
+     "/css/css-filter/blur-clip-stacking-context-001.html",
+     [
+      [
+       "/css/css-filter/blur-clip-stacking-context-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
+   "css/css-filter/blur-clip-stacking-context-002.html": [
+    [
+     "/css/css-filter/blur-clip-stacking-context-002.html",
+     [
+      [
+       "/css/css-filter/blur-clip-stacking-context-ref.html",
+       "!="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/css-filter/filtered-block-is-container.html": [
     [
      "/css/css-filter/filtered-block-is-container.html",
      [
       [
        "/css/css-filter/filtered-block-is-container-ref.html",
        "=="
       ]
@@ -244068,16 +244092,21 @@
      {}
     ]
    ],
    "css/css-filter/META.yml": [
     [
      {}
     ]
    ],
+   "css/css-filter/blur-clip-stacking-context-ref.html": [
+    [
+     {}
+    ]
+   ],
    "css/css-filter/filtered-block-is-container-ref.html": [
     [
      {}
     ]
    ],
    "css/css-filter/filtered-html-is-not-container-ref.html": [
     [
      {}
@@ -521832,17 +521861,17 @@
    "e7a9d200d710d3bb84991eab13e449d03d4bb064",
    "reftest"
   ],
   "css/css-display/display-contents-dynamic-table-002-none.html": [
    "964b6d72f0a237765fc677e4c3b0f9cb0cf027b6",
    "reftest"
   ],
   "css/css-display/display-contents-fieldset-nested-legend-ref.html": [
-   "6f547b3201c3b3c2521102ecbcf144df28447f7a",
+   "c4afe0e53ba02d47bc0704eb71801aec602759be",
    "support"
   ],
   "css/css-display/display-contents-fieldset-nested-legend.html": [
    "a3bd2fc9983aabd8f046b4d4e51511bd65295a5b",
    "reftest"
   ],
   "css/css-display/display-contents-fieldset.html": [
    "f38a3763cfb954966c4eb8527d45a213635ea525",
@@ -523139,16 +523168,28 @@
   "css/css-fill-stroke/reference/paint-order-001-ref.tentative.html": [
    "af56147f52291a8b4515c67f843505703875c594",
    "support"
   ],
   "css/css-filter/META.yml": [
    "8d0683319b0fbbd1262cbdd12cdbcb727b2aa9a0",
    "support"
   ],
+  "css/css-filter/blur-clip-stacking-context-001.html": [
+   "48c1bae3042b6912ba5edd7ae74c190e5bd8f2d2",
+   "reftest"
+  ],
+  "css/css-filter/blur-clip-stacking-context-002.html": [
+   "f4df453b17b86a6da67c4cf5f71261e9315eab95",
+   "reftest"
+  ],
+  "css/css-filter/blur-clip-stacking-context-ref.html": [
+   "82b907334fd71791cd2f6530e2e85169e29afa41",
+   "support"
+  ],
   "css/css-filter/filtered-block-is-container-ref.html": [
    "fc9467f8717dfc722dcb6dbbbb31bfd8c2baee3b",
    "support"
   ],
   "css/css-filter/filtered-block-is-container.html": [
    "6f99f364e638028098ae10319290e8bf2a9a9457",
    "reftest"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-filter/blur-clip-stacking-context-001.html
@@ -0,0 +1,35 @@
+<!doctype html>
+<title>CSS Test: Blurs are correctly clipped by an overflow: hidden ancestor</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://drafts.fxtf.org/filter-effects/#funcdef-filter-blur">
+<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#valdef-overflow-hidden">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1459065">
+<link rel="match" href="blur-clip-stacking-context-ref.html">
+<style>
+  #cover, #clip, #blur {
+    width: 100px;
+    height: 100px;
+  }
+
+  #cover, #clip {
+    position: absolute;
+    top: 10px;
+    left: 10px;
+  }
+
+  #cover {
+    background: green;
+  }
+
+  #clip {
+    overflow: hidden;
+    background: red;
+  }
+
+  #blur {
+    background: blue;
+    filter: blur(20px);
+  }
+</style>
+<div id="clip"><div id="blur"></div></div>
+<div id="cover"></div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-filter/blur-clip-stacking-context-002.html
@@ -0,0 +1,28 @@
+<!doctype html>
+<title>CSS Test: Blurs on an overflow: hidden element are correctly not-clipped to the padding box</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://drafts.fxtf.org/filter-effects/#funcdef-filter-blur">
+<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#valdef-overflow-hidden">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1459065">
+<link rel="mismatch" href="blur-clip-stacking-context-ref.html">
+<style>
+  #cover, #clip {
+    position: absolute;
+    top: 10px;
+    left: 10px;
+    width: 100px;
+    height: 100px;
+  }
+
+  #cover {
+    background: green;
+  }
+
+  #clip {
+    overflow: hidden;
+    background: blue;
+    filter: blur(20px);
+  }
+</style>
+<div id="clip"></div>
+<div id="cover"></div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-filter/blur-clip-stacking-context-ref.html
@@ -0,0 +1,14 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<style>
+  div {
+    position: absolute;
+    top: 10px;
+    left: 10px;
+    background: green;
+    width: 100px;
+    height: 100px;
+  }
+</style>
+<div></div>