Put background filters on the makeWithFilter() fast-path

cc/output/gl_renderer.cc

Index: cc/output/gl_renderer.cc
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index 47f1b222c8b2937519ca0539f89a534ffd2bf160..94b416d99d130d94f55c8f32fc31c021d05efbb9 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -849,61 +849,21 @@ sk_sp<SkImage> GLRenderer::ApplyBackgroundFilters(
     DrawingFrame* frame,
     const RenderPassDrawQuad* quad,
     ScopedResource* background_texture,
-    const gfx::RectF& rect) {
+    const gfx::RectF& rect,
+    SkIPoint* offset,
+    SkIRect* subset,
+    bool flip_texture) {
   DCHECK(ShouldApplyBackgroundFilters(quad));
-  auto use_gr_context = ScopedUseGrContext::Create(this, frame);
+
   sk_sp<SkImageFilter> filter = RenderSurfaceFilters::BuildImageFilter(
       quad->background_filters, gfx::SizeF(background_texture->size()));
 
-  // TODO(senorblanco): background filters should be moved to the
-  // makeWithFilter fast-path, and go back to calling ApplyImageFilter().
-  // See http://crbug.com/613233.
-  if (!filter || !use_gr_context)
-    return nullptr;
-
-  ResourceProvider::ScopedReadLockGL lock(resource_provider_,
-                                          background_texture->id());
+  sk_sp<SkImage> background_with_filters = ApplyImageFilter(
+      ScopedUseGrContext::Create(this, frame), resource_provider_, rect, rect,
+      quad->filters_scale, std::move(filter), background_texture, offset,
+      subset, flip_texture);
 
-  bool flip_texture = true;
-  sk_sp<SkImage> src_image =
-      WrapTexture(lock, use_gr_context->context(), flip_texture);
-  if (!src_image) {
-    TRACE_EVENT_INSTANT0(
-        "cc", "ApplyBackgroundFilters wrap background texture failed",
-        TRACE_EVENT_SCOPE_THREAD);
-    return nullptr;
-  }
-
-  // Create surface to draw into.
-  SkImageInfo dst_info =
-      SkImageInfo::MakeN32Premul(rect.width(), rect.height());
-  sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(
-      use_gr_context->context(), SkBudgeted::kYes, dst_info);
-  if (!surface) {
-    TRACE_EVENT_INSTANT0("cc",
-                         "ApplyBackgroundFilters surface allocation failed",
-                         TRACE_EVENT_SCOPE_THREAD);
-    return nullptr;
-  }
-
-  SkMatrix local_matrix;
-  local_matrix.setScale(quad->filters_scale.x(), quad->filters_scale.y());
-
-  SkPaint paint;
-  paint.setImageFilter(filter->makeWithLocalMatrix(local_matrix));
-  surface->getCanvas()->translate(-rect.x(), -rect.y());
-  surface->getCanvas()->drawImage(src_image, rect.x(), rect.y(), &paint);
-  // 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 background_with_filters;
 }
 
 // Map device space quad to local space. Device_transform has no 3d
@@ -1042,6 +1002,8 @@ void GLRenderer::DrawRenderPassQuadInternal(DrawingFrame* frame,
   sk_sp<SkImage> background_image;
   GLuint background_image_id = 0;
   gfx::Rect background_rect;
+  SkIPoint background_offset;
+  SkIRect background_subset;
   if (use_shaders_for_blending) {
     // Compute a bounding box around the pixels that will be visible through
     // the quad.
@@ -1064,7 +1026,8 @@ void GLRenderer::DrawRenderPassQuadInternal(DrawingFrame* frame,
         // Apply the background filters to R, so that it is applied in the
         // pixels' coordinate space.
         background_image = ApplyBackgroundFilters(
-            frame, quad, background_texture.get(), gfx::RectF(background_rect));
+            frame, quad, background_texture.get(), gfx::RectF(background_rect),
+            &background_offset, &background_subset, flip_texture);
         if (background_image)
           background_image_id = skia::GrBackendObjectToGrGLTextureInfo(
                                     background_image->getTextureHandle(true))
@@ -1358,9 +1321,22 @@ void GLRenderer::DrawRenderPassQuadInternal(DrawingFrame* frame,
 
     gl_->Uniform1i(locations.backdrop, ++last_texture_unit);
 
-    gl_->Uniform4f(locations.backdrop_rect, background_rect.x(),
-                   background_rect.y(), background_rect.width(),
-                   background_rect.height());
+    gfx::Size background_texture_size;
+    if (background_image) {
+      background_texture_size.set_width(background_image->width());
+      background_texture_size.set_height(background_image->height());

[Stephen White, 2016/07/12 16:37:52]

It looks like the backdrop_rect is also used when doing shader-based blending,
and setting an empty backdrop_rect may cause problems for that case.

Perhaps we should simply modify background_rect just below the call to
ApplyBackgroundFilters() above to reflect the background filters case, and leave
it unmodified otherwise, so that shader-based blending continues to work.

We should also test the combination of the two. I'm actually not certain what
the correct background_rect is in that case; I suppose it depends on the order
of operations.

[Anton Obzhirov, 2016/07/14 12:32:09]

Yes, you are right, I'll move it up as you suggested. I also couldn't find a
test with blending and background filter, may be new one should be added.

+    }
+
+    gfx::RectF tex_rect(background_rect);
+    tex_rect.set_width(background_texture_size.width());
+    tex_rect.set_height(background_texture_size.height());
+    tex_rect.set_x(background_rect.x() + background_offset.x());
+    tex_rect.set_y(
+        background_rect.y() + background_offset.y() -
+        (background_texture_size.height() - background_subset.height()));
+
+    gl_->Uniform4f(locations.backdrop_rect, tex_rect.x(), tex_rect.y(),
+                   tex_rect.width(), tex_rect.height());
 
     if (background_image_id) {
       gl_->ActiveTexture(GL_TEXTURE0 + last_texture_unit);