Remove expansion of backdrop-filter input for pixel-moving filters
See [1] for the PR for the spec change. Essentially, with this CL,
the input to the filter will be the bounding box of the element
with backdrop-filter. The bounds will not be expanded for pixel-
moving filters, and will not include extra border for children.
Note that the rebaselined layout tests are all for very small
single-pixel errors, except for backdrop_filter_blur_outsets.png
and backdrop-filter-boundary-expected.png, which change because
of this new behavior.
This needs a (small) spec change, but it will be beneficial in
these four ways:
- It agrees more closely with existing Safari behavior.
- It improves performance, by filtering fewer pixels in the
case of pixel-moving filters.
- Developers have been disappointed with the existing behavior
of bringing in pixels from outside the bounds of the
element, and have requested a way to turn this behavior off.
See the discussion starting at [2].
- It eliminates the problem of crbug.com/972173, where content
outside the renderer viewport can get dragged into the filter.
[1] https://github.com/w3c/fxtf-drafts/pull/342
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=497522#c196
TBR=masonfreed@chromium.org
(cherry picked from commit 16f5c5ac41dcf6415c1e1a4ab916f693ba850a25)
Bug: 972173,497522
Change-Id: I18cc6d08cf9fde263b9e166b5e96ea57a1c41973
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1650418
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Auto-Submit: Mason Freed <masonfreed@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#668894}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660122
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#316}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/cc/trees/layer_tree_host_pixeltest_filters.cc b/cc/trees/layer_tree_host_pixeltest_filters.cc
index f65f0b7f..6c371749f 100644
--- a/cc/trees/layer_tree_host_pixeltest_filters.cc
+++ b/cc/trees/layer_tree_host_pixeltest_filters.cc
@@ -186,14 +186,17 @@
}
TEST_P(LayerTreeHostFiltersPixelTestGPU, BackdropFilterBlurOutsets) {
+ if (renderer_type() == RENDERER_SKIA_GL ||
+ renderer_type() == RENDERER_SKIA_VK) {
+ // TODO(973696): Implement bounds clipping in skia_renderer.
+ return;
+ }
scoped_refptr<SolidColorLayer> background = CreateSolidColorLayer(
gfx::Rect(200, 200), SK_ColorWHITE);
- // The green border is outside the layer with backdrop blur, but the
- // backdrop blur should use pixels from outside its layer borders, up to the
- // radius of the blur effect. So the border should be blurred underneath the
- // top layer causing the green to bleed under the transparent layer, but not
- // in the 1px region between the transparent layer and the green border.
+ // The green border is outside the layer with backdrop blur, so the backdrop
+ // blur should not include pixels from outside its layer borders. So the
+ // interior region of the transparent layer should be perfectly white.
scoped_refptr<SolidColorLayer> green_border = CreateSolidColorLayerWithBorder(
gfx::Rect(1, 1, 198, 198), SK_ColorWHITE, 10, kCSSGreen);
scoped_refptr<SolidColorLayer> blur = CreateSolidColorLayer(
@@ -1043,11 +1046,12 @@
float device_scale_factor_ = 1;
};
+// TODO(973699): This test is broken in software_renderer. Re-enable this test
+// when fixed.
INSTANTIATE_TEST_SUITE_P(,
BackdropFilterWithDeviceScaleFactorTest,
::testing::Values(LayerTreeTest::RENDERER_GL,
- LayerTreeTest::RENDERER_SKIA_GL,
- LayerTreeTest::RENDERER_SOFTWARE));
+ LayerTreeTest::RENDERER_SKIA_GL));
TEST_P(BackdropFilterWithDeviceScaleFactorTest, StandardDpi) {
RunPixelTestType(
diff --git a/components/viz/service/display/gl_renderer.cc b/components/viz/service/display/gl_renderer.cc
index b7a65f6..64f2992 100644
--- a/components/viz/service/display/gl_renderer.cc
+++ b/components/viz/service/display/gl_renderer.cc
@@ -734,37 +734,10 @@
gfx::Rect backdrop_rect = gfx::ToEnclosingRect(cc::MathUtil::MapClippedRect(
params->contents_device_transform, scaled_region.BoundingBox()));
- if (ShouldApplyBackdropFilters(params->backdrop_filters)) {
- SkMatrix matrix;
- // |filters_scale| is the ratio of render pass physical pixels to root layer
- // layer space, including content-to-target-space scale and device pixel
- // ratio.
- matrix.setScale(quad->filters_scale.x(), quad->filters_scale.y());
- if (FlippedFramebuffer()) {
- // TODO(jbroman): This probably isn't the right way to account for this.
- // Probably some combination of current_frame()->projection_matrix,
- // current_frame()->window_matrix and contents_device_transform?
- // Likely this should be window_matrix*projection_matrix.
- matrix.postScale(1, -1);
- }
- // |backdrop_rect| is now expanded for pixel moving backdrop_filters, offset
- // by any backdrop-filter drop-shadow offset (flipped if
- // FlippedFramebuffer()). Note that scale is not applied to the
- // backdrop_rect itself, only the sigma or x/y offset of filters.
- backdrop_rect =
- params->backdrop_filters->MapRectReverse(backdrop_rect, matrix);
- }
-
- if (!backdrop_rect.IsEmpty() && params->use_aa) {
- const int kOutsetForAntialiasing = 1;
- backdrop_rect.Inset(-kOutsetForAntialiasing, -kOutsetForAntialiasing);
- }
-
- if (params->filters) {
- DCHECK(!params->filters->IsEmpty());
- // If we have regular filters, grab an extra one-pixel border around the
- // background, so texture edge clamping gives us a transparent border
- // in case the filter expands the result.
+ if (!backdrop_rect.IsEmpty() && (params->filters || params->use_aa)) {
+ // If we have regular filters or antialiasing, grab an extra one-pixel
+ // border around the background, so texture edge clamping gives us a
+ // transparent border.
backdrop_rect.Inset(-1, -1, -1, -1);
}
@@ -843,6 +816,19 @@
return texture_id;
}
+static sk_sp<SkImage> FinalizeImage(sk_sp<SkSurface> surface) {
+ // Flush the drawing before source texture read lock goes out of scope.
+ // Skia API does not guarantee that when the SkImage goes out of scope,
+ // its externally referenced resources would force the rendering to be
+ // flushed.
+ surface->getCanvas()->flush();
+ sk_sp<SkImage> image = surface->makeImageSnapshot();
+ if (!image || !image->isTextureBacked()) {
+ return nullptr;
+ }
+ return image;
+}
+
sk_sp<SkImage> GLRenderer::ApplyBackdropFilters(
DrawRenderPassDrawQuadParams* params,
const gfx::Rect& unclipped_rect,
@@ -920,11 +906,19 @@
surface->getCanvas()->drawImageRect(src_image, RectFToSkRect(src_image_rect),
dest_rect, nullptr);
- // Can't crop here, because the crop rect is applied prior to filtering, and
- // some filters move pixels and need to process the full image.
- // TODO(916314): this could probably be put back to just using
- // drawImageRect on the unfiltered image, with &paint last argument to handle
- // filters and opacity. Would be cleaner.
+ if (backdrop_filter_bounds.has_value()) {
+ // Crop the source image to the backdrop_filter_bounds.
+ gfx::Rect filter_clip = gfx::ToEnclosingRect(cc::MathUtil::MapClippedRect(
+ backdrop_filter_bounds_transform, backdrop_filter_bounds->rect()));
+ filter_clip.Intersect(gfx::Rect(src_image->width(), src_image->height()));
+ if (filter_clip.IsEmpty())
+ return FinalizeImage(surface);
+ src_image = src_image->makeSubset(RectToSkIRect(filter_clip));
+ src_image_rect = gfx::RectF(filter_clip.width(), filter_clip.height());
+ dest_rect = RectToSkRect(
+ ScaleToEnclosingRect(filter_clip, params->backdrop_filter_quality));
+ }
+
SkIPoint offset;
SkIRect subset;
sk_sp<SkImage> filtered_image = SkiaHelper::ApplyImageFilter(
@@ -951,17 +945,7 @@
surface->getCanvas()->restore();
}
- // Flush the drawing before source texture read lock goes out of scope.
- // Skia API does not guarantee that when the SkImage goes out of scope,
- // its externally referenced resources would force the rendering to be
- // flushed.
- surface->getCanvas()->flush();
- sk_sp<SkImage> image = surface->makeImageSnapshot();
- if (!image || !image->isTextureBacked()) {
- return nullptr;
- }
-
- return image;
+ return FinalizeImage(surface);
}
const TileDrawQuad* GLRenderer::CanPassBeDrawnDirectly(const RenderPass* pass) {
diff --git a/components/viz/service/display/software_renderer.cc b/components/viz/service/display/software_renderer.cc
index e9f4f44..e783b198 100644
--- a/components/viz/service/display/software_renderer.cc
+++ b/components/viz/service/display/software_renderer.cc
@@ -738,18 +738,6 @@
gfx::Rect backdrop_rect = gfx::ToEnclosingRect(cc::MathUtil::MapClippedRect(
contents_device_transform, QuadVertexRect()));
- if (ShouldApplyBackdropFilters(backdrop_filters)) {
- SkMatrix matrix;
- // |filters_scale| is the ratio of render pass physical pixels to root layer
- // layer space, including content-to-target-space scale and device pixel
- // ratio.
- matrix.setScale(quad->filters_scale.x(), quad->filters_scale.y());
- // |backdrop_rect| is now expanded for pixel moving backdrop_filters, offset
- // by any backdrop-filter drop-shadow offset. Note that scale is not applied
- // to the backdrop_rect itself, only the sigma or x/y offset of filters.
- backdrop_rect = backdrop_filters->MapRectReverse(backdrop_rect, matrix);
- }
-
if (regular_filters) {
DCHECK(!regular_filters->IsEmpty());
// If we have regular filters, grab an extra one-pixel border around the
diff --git a/components/viz/test/data/backdrop_filter_blur_off_axis.png b/components/viz/test/data/backdrop_filter_blur_off_axis.png
index 89355ae4..c347075f 100644
--- a/components/viz/test/data/backdrop_filter_blur_off_axis.png
+++ b/components/viz/test/data/backdrop_filter_blur_off_axis.png
Binary files differ
diff --git a/components/viz/test/data/backdrop_filter_blur_outsets.png b/components/viz/test/data/backdrop_filter_blur_outsets.png
index e5b81d65..a74b270 100644
--- a/components/viz/test/data/backdrop_filter_blur_outsets.png
+++ b/components/viz/test/data/backdrop_filter_blur_outsets.png
Binary files differ
diff --git a/third_party/blink/web_tests/css3/filters/backdrop-filter-basic-blur-expected.png b/third_party/blink/web_tests/css3/filters/backdrop-filter-basic-blur-expected.png
index d3be83b..cb19a6e9 100644
--- a/third_party/blink/web_tests/css3/filters/backdrop-filter-basic-blur-expected.png
+++ b/third_party/blink/web_tests/css3/filters/backdrop-filter-basic-blur-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/css3/filters/backdrop-filter-boundary-expected.png b/third_party/blink/web_tests/css3/filters/backdrop-filter-boundary-expected.png
deleted file mode 100644
index f5794ba..0000000
--- a/third_party/blink/web_tests/css3/filters/backdrop-filter-boundary-expected.png
+++ /dev/null
Binary files differ
diff --git a/third_party/blink/web_tests/css3/filters/backdrop-filter-boundary.html b/third_party/blink/web_tests/css3/filters/backdrop-filter-boundary.html
index 325f1976..2e5b8ac 100644
--- a/third_party/blink/web_tests/css3/filters/backdrop-filter-boundary.html
+++ b/third_party/blink/web_tests/css3/filters/backdrop-filter-boundary.html
@@ -27,5 +27,5 @@
<div class="bg"><div class="fg" style="backdrop-filter: blur(96px);"></div></div>
</div>
-<!-- This should show an increasing series of blurred blocks, with the higher
- blur values bringing in lime green from the edges. -->
+<!-- This should show an increasing series of blurred blocks. No lime green should
+ be brought in to the blurred regions. -->
diff --git a/third_party/blink/web_tests/css3/filters/backdrop-filter-edge-pixels-expected.png b/third_party/blink/web_tests/css3/filters/backdrop-filter-edge-pixels-expected.png
index 7e810e0..f8bda03 100644
--- a/third_party/blink/web_tests/css3/filters/backdrop-filter-edge-pixels-expected.png
+++ b/third_party/blink/web_tests/css3/filters/backdrop-filter-edge-pixels-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/css3/filters/backdrop-filter-plus-filter-expected.png b/third_party/blink/web_tests/css3/filters/backdrop-filter-plus-filter-expected.png
index 50c0a55f..89c49dc 100644
--- a/third_party/blink/web_tests/css3/filters/backdrop-filter-plus-filter-expected.png
+++ b/third_party/blink/web_tests/css3/filters/backdrop-filter-plus-filter-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/external/wpt/css/filter-effects/backdrop-filter-edge-clipping.html b/third_party/blink/web_tests/external/wpt/css/filter-effects/backdrop-filter-edge-clipping.html
new file mode 100644
index 0000000..5d09c414
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/filter-effects/backdrop-filter-edge-clipping.html
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>backdrop-filter: Filter input is at element bounds</title>
+<link rel="author" title="Mason Freed" href="mailto:masonfreed@chromium.org">
+<link rel="help" href="https://drafts.fxtf.org/filter-effects-2/#BackdropFilterProperty">
+<link rel="match" href="backdrop-filter-paint-order-ref.html">
+
+<div>
+ <p>Expected: A pure white box with a blue border, surrounded by green.<br>
+ No green should be observed within the white box.<br>
+ No dark/black should be observed within the white box either.</p>
+</div>
+
+<div class="greenbox top"></div>
+<div class="greenbox right"></div>
+<div class="greenbox bottom"></div>
+<div class="filterbox">
+ <div class="children" style="top:-31px;left:35px;"></div>
+ <div class="children" style="top:101px;left:35px;"></div>
+ <div class="children" style="top:35px;left:101px;"></div>
+ <div class="children" style="top:35px;left:-31px;"></div>
+</div>
+<style>
+.filterbox {
+ position: absolute;
+ width: 100px;
+ height: 100px;
+ top: 150px;
+ left: 0px;
+ border: 1px solid blue;
+ backdrop-filter: blur(20px);
+}
+.greenbox {
+ position:absolute;
+ width: 150px;
+ height: 50px;
+ background: green;
+}
+.top {
+ top:100px;
+ left: 10px;
+}
+.right {
+ top:130px;
+ left: 102px;
+ width: 58px;
+ height: 150px;
+}
+.bottom {
+ top:252px;
+ left: 10px;
+}
+.children {
+ position: absolute;
+ width: 30px;
+ height: 30px;
+ top: 0px;
+ left: 0px;
+ background: green;
+}
+</style>
+
diff --git a/third_party/blink/web_tests/platform/linux/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png b/third_party/blink/web_tests/platform/linux/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png
new file mode 100644
index 0000000..a42e0c5
--- /dev/null
+++ b/third_party/blink/web_tests/platform/linux/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/platform/mac/css3/filters/backdrop-filter-boundary-expected.png b/third_party/blink/web_tests/platform/mac/css3/filters/backdrop-filter-boundary-expected.png
new file mode 100644
index 0000000..d07c43a
--- /dev/null
+++ b/third_party/blink/web_tests/platform/mac/css3/filters/backdrop-filter-boundary-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/platform/mac/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png b/third_party/blink/web_tests/platform/mac/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png
new file mode 100644
index 0000000..a42e0c5
--- /dev/null
+++ b/third_party/blink/web_tests/platform/mac/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/platform/mac/virtual/scalefactor200/css3/filters/backdrop-filter-edge-pixels-expected.png b/third_party/blink/web_tests/platform/mac/virtual/scalefactor200/css3/filters/backdrop-filter-edge-pixels-expected.png
new file mode 100644
index 0000000..46558de
--- /dev/null
+++ b/third_party/blink/web_tests/platform/mac/virtual/scalefactor200/css3/filters/backdrop-filter-edge-pixels-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/platform/win/css3/filters/backdrop-filter-boundary-expected.png b/third_party/blink/web_tests/platform/win/css3/filters/backdrop-filter-boundary-expected.png
new file mode 100644
index 0000000..7b4acc9
--- /dev/null
+++ b/third_party/blink/web_tests/platform/win/css3/filters/backdrop-filter-boundary-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/platform/win/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png b/third_party/blink/web_tests/platform/win/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png
new file mode 100644
index 0000000..dc263a1
--- /dev/null
+++ b/third_party/blink/web_tests/platform/win/virtual/scalefactor200/css3/filters/backdrop-filter-boundary-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/platform/win/virtual/scalefactor200/css3/filters/backdrop-filter-edge-pixels-expected.png b/third_party/blink/web_tests/platform/win/virtual/scalefactor200/css3/filters/backdrop-filter-edge-pixels-expected.png
new file mode 100644
index 0000000..d6b716e8
--- /dev/null
+++ b/third_party/blink/web_tests/platform/win/virtual/scalefactor200/css3/filters/backdrop-filter-edge-pixels-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/virtual/scalefactor200/css3/filters/backdrop-filter-basic-blur-expected.png b/third_party/blink/web_tests/virtual/scalefactor200/css3/filters/backdrop-filter-basic-blur-expected.png
new file mode 100644
index 0000000..24a2623
--- /dev/null
+++ b/third_party/blink/web_tests/virtual/scalefactor200/css3/filters/backdrop-filter-basic-blur-expected.png
Binary files differ
diff --git a/third_party/blink/web_tests/virtual/scalefactor200/css3/filters/backdrop-filter-plus-filter-expected.png b/third_party/blink/web_tests/virtual/scalefactor200/css3/filters/backdrop-filter-plus-filter-expected.png
new file mode 100644
index 0000000..398cdce
--- /dev/null
+++ b/third_party/blink/web_tests/virtual/scalefactor200/css3/filters/backdrop-filter-plus-filter-expected.png
Binary files differ
diff --git a/ui/compositor/layer_unittest.cc b/ui/compositor/layer_unittest.cc
index 4b4f48e..252f92a 100644
--- a/ui/compositor/layer_unittest.cc
+++ b/ui/compositor/layer_unittest.cc
@@ -1833,20 +1833,23 @@
base::FilePath ref_img2 = test_data_dir().AppendASCII("BackgroundBlur2.png");
SkBitmap bitmap;
+ // 25% of image can have up to a difference of 3.
+ cc::FuzzyPixelComparator fuzzy_comparator(true, 25.f, 0.0f, 3.f, 3, 0);
+
l0->Add(l1.get());
l0->Add(l2.get());
DrawTree(l0.get());
ReadPixels(&bitmap);
ASSERT_FALSE(bitmap.empty());
// WritePNGFile(bitmap, ref_img1, false);
- EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img1, cc::ExactPixelComparator(true)));
+ EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img1, fuzzy_comparator));
l0->StackAtTop(l1.get());
DrawTree(l0.get());
ReadPixels(&bitmap);
ASSERT_FALSE(bitmap.empty());
// WritePNGFile(bitmap, ref_img2, false);
- EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img2, cc::ExactPixelComparator(true)));
+ EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img2, fuzzy_comparator));
}
// Checks that background blur bounds rect gets properly updated when device
@@ -1875,6 +1878,9 @@
test_data_dir().AppendASCII("BackgroundBlur1_zoom.png");
SkBitmap bitmap;
+ // 25% of image can have up to a difference of 3.
+ cc::FuzzyPixelComparator fuzzy_comparator(true, 25.f, 0.0f, 3.f, 3, 0);
+
l0->Add(l1.get());
l0->Add(l2.get());
DrawTree(l0.get());
@@ -1882,7 +1888,7 @@
ASSERT_FALSE(bitmap.empty());
// See LayerWithRealCompositorTest.BackgroundBlur test to rewrite this
// baseline.
- EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img1, cc::ExactPixelComparator(true)));
+ EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img1, fuzzy_comparator));
allocator.GenerateId();
// Now change the scale, and make sure the bounds are still correct.
@@ -1893,7 +1899,7 @@
ReadPixels(&bitmap);
ASSERT_FALSE(bitmap.empty());
// WritePNGFile(bitmap, ref_img2, false);
- EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img2, cc::ExactPixelComparator(true)));
+ EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img2, fuzzy_comparator));
}
// It is really hard to write pixel test on text rendering,