Fixing rendering of backdrop-filter with opacity

Prior to this CL, a node that contained both opacity and backdrop-filter
would sometimes not be properly rendered. The opacity was, in some cases,
applied as if there was an opaque black background behind the filter
region, rather than compositing the filtered image into the background
with the specified opacity. This CL changes the way opacity is applied,
moving it to the paint of the final filtered image, which fixes the
problem.

Bug: 983252
Change-Id: I9fae88c8f7b1c950d5616e1c9eeaaa664bab649a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1707457Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680076}

components/viz/common/skia_helper.cc

@@ -4,6 +4,8 @@
 #include "components/viz/common/skia_helper.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/base/math_util.h"
+#include "third_party/skia/include/effects/SkColorFilterImageFilter.h"
+#include "third_party/skia/include/effects/SkColorMatrix.h"
 #include "third_party/skia/include/effects/SkOverdrawColorFilter.h"
 #include "third_party/skia/include/gpu/GrBackendSurface.h"
 #include "third_party/skia/include/gpu/GrContext.h"
@@ -66,4 +68,11 @@ sk_sp<SkColorFilter> SkiaHelper::MakeOverdrawColorFilter() {
   return SkOverdrawColorFilter::Make(colors);
 }
 
+sk_sp<SkImageFilter> SkiaHelper::BuildOpacityFilter(float opacity) {
+  SkColorMatrix matrix;
+  matrix.setScale(1.f, 1.f, 1.f, opacity);
+  return SkColorFilterImageFilter::Make(SkColorFilters::Matrix(matrix),
+                                        nullptr);
+}
+
 }  // namespace viz

components/viz/common/skia_helper.h

@@ -29,6 +29,8 @@ class VIZ_COMMON_EXPORT SkiaHelper {
                                          bool flush);
 
   static sk_sp<SkColorFilter> MakeOverdrawColorFilter();
+
+  static sk_sp<SkImageFilter> BuildOpacityFilter(float opacity);
 };
 
 }  // namespace viz

components/viz/service/display/gl_renderer.cc

@@ -867,6 +867,8 @@ sk_sp<SkImage> GLRenderer::ApplyBackdropFilters(
     const gfx::Transform& backdrop_filter_bounds_transform) {
   DCHECK(ShouldApplyBackdropFilters(params->backdrop_filters));
   DCHECK(params->backdrop_filter_quality);
+  DCHECK(!params->filters)
+      << "Filters should always be in a separate Effect node";
   const RenderPassDrawQuad* quad = params->quad;
   auto use_gr_context = ScopedUseGrContext::Create(this);
 
@@ -874,19 +876,8 @@ sk_sp<SkImage> GLRenderer::ApplyBackdropFilters(
       (params->background_rect.top_right() - unclipped_rect.top_right()) +
       (params->background_rect.bottom_left() - unclipped_rect.bottom_left());
 
-  // Update the backdrop filter to include opacity.
-  cc::FilterOperations backdrop_filters_plus_opacity =
-      *params->backdrop_filters;
-  DCHECK(!params->filters)
-      << "Filters should always be in a separate Effect node";
-  if (quad->shared_quad_state->opacity < 1.0) {
-    backdrop_filters_plus_opacity.Append(
-        cc::FilterOperation::CreateOpacityFilter(
-            quad->shared_quad_state->opacity));
-  }
-
   auto paint_filter = cc::RenderSurfaceFilters::BuildImageFilter(
-      backdrop_filters_plus_opacity, gfx::SizeF(params->background_rect.size()),
+      *params->backdrop_filters, gfx::SizeF(params->background_rect.size()),
       gfx::Vector2dF(clipping_offset));
 
   // TODO(senorblanco): background filters should be moved to the
@@ -971,6 +962,12 @@ sk_sp<SkImage> GLRenderer::ApplyBackdropFilters(
     surface->getCanvas()->resetMatrix();
   }
 
+  SkPaint paint;
+  // Paint the filtered backdrop image with opacity.
+  if (quad->shared_quad_state->opacity < 1.0) {
+    paint.setImageFilter(
+        SkiaHelper::BuildOpacityFilter(quad->shared_quad_state->opacity));
+  }
   // Apply the mask image, if present, to filtered backdrop content. Note that
   // this needs to be performed here, in addition to elsewhere, because of the
   // order of operations:
@@ -979,7 +976,6 @@ sk_sp<SkImage> GLRenderer::ApplyBackdropFilters(
   //   2. Render the parent render pass (containing the "backdrop image" to be
   //      filtered).
   //   3. Run this code, to filter, and possibly mask, the backdrop image.
-  SkPaint paint;
   const SkImage* mask_image = nullptr;
   base::Optional<DisplayResourceProvider::ScopedReadLockSkImage>
       backdrop_image_lock;

components/viz/service/display/software_renderer.cc

@@ -17,6 +17,7 @@
 #include "components/viz/common/quads/solid_color_draw_quad.h"
 #include "components/viz/common/quads/texture_draw_quad.h"
 #include "components/viz/common/quads/tile_draw_quad.h"
+#include "components/viz/common/skia_helper.h"
 #include "components/viz/common/viz_utils.h"
 #include "components/viz/service/display/output_surface.h"
 #include "components/viz/service/display/output_surface_frame.h"
@@ -803,21 +804,13 @@ sk_sp<SkShader> SoftwareRenderer::GetBackdropFilterShader(
       (unclipped_rect.top_right() - backdrop_rect.top_right()) +
       (backdrop_rect.bottom_left() - unclipped_rect.bottom_left());
 
-  // Update the backdrop filter to include opacity.
-  cc::FilterOperations backdrop_filters_plus_opacity = *backdrop_filters;
   DCHECK(!regular_filters)
       << "Filters should always be in a separate Effect node";
-  if (quad->shared_quad_state->opacity < 1.0) {
-    backdrop_filters_plus_opacity.Append(
-        cc::FilterOperation::CreateOpacityFilter(
-            quad->shared_quad_state->opacity));
-  }
-
   gfx::Rect bitmap_rect =
       gfx::Rect(0, 0, backdrop_bitmap.width(), backdrop_bitmap.height());
   sk_sp<SkImageFilter> filter =
       cc::RenderSurfaceFilters::BuildImageFilter(
-          backdrop_filters_plus_opacity,
+          *backdrop_filters,
           gfx::SizeF(bitmap_rect.width(), bitmap_rect.height()),
           clipping_offset)
           ->cached_sk_filter_;
@@ -844,9 +837,16 @@ sk_sp<SkShader> SoftwareRenderer::GetBackdropFilterShader(
     canvas.resetMatrix();
   }
 
+  // Paint the filtered backdrop image with opacity.
+  SkPaint paint;
+  if (quad->shared_quad_state->opacity < 1.0) {
+    paint.setImageFilter(
+        SkiaHelper::BuildOpacityFilter(quad->shared_quad_state->opacity));
+  }
+
   // Now paint the pre-filtered image onto the canvas.
   SkRect bitmap_skrect = RectToSkRect(bitmap_rect);
-  canvas.drawImageRect(filtered_image, bitmap_skrect, bitmap_skrect, nullptr);
+  canvas.drawImageRect(filtered_image, bitmap_skrect, bitmap_skrect, &paint);
 
   return SkImage::MakeFromBitmap(bitmap)->makeShader(
       content_tile_mode, content_tile_mode, &filter_backdrop_transform);

third_party/blink/web_tests/external/wpt/css/filter-effects/backdrop-filter-plus-opacity-ref.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>backdrop-filter: Correctly apply backdrop-filter with opacity</title>
+<link rel="author" title="Mason Freed" href="mailto:masonfreed@chromium.org">
+
+
+
+<p>Expected: A green box.</p>
+
+<div class="greenbox"></div>
+
+
+<style>
+.greenbox {
+  position: absolute;
+  background: green;
+  width: 100px;
+  height: 100px;
+  top: 100px;
+  left: 60px;
+}
+</style>

third_party/blink/web_tests/external/wpt/css/filter-effects/backdrop-filter-plus-opacity.html

+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>backdrop-filter: Correctly apply backdrop-filter with opacity</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-plus-opacity-ref.html">
+
+<p>Expected: A green box.</p>
+
+<div class="greenbox"></div>
+<div class="filter"></div>
+
+<style>
+.greenbox {
+  position: absolute;
+  background: green;
+  width: 100px;
+  height: 100px;
+  top: 100px;
+  left: 60px;
+}
+.filter {
+  position: absolute;
+  width: 200px;
+  height: 200px;
+  top: 50px;
+  left: 10px;
+  backdrop-filter: invert(1);
+  opacity: 0;
+}
+</style>