cc: Use the onscreen context to perform composited ganesh filters.

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 87195747d0bcbeafddd72bfbe42d3db936fc5dd4..2222a104905a569ae48bac5a3bae885cf58f43c9 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -177,6 +177,39 @@ const float kAntiAliasingEpsilon = 1.0f / 1024.0f;
 
 }  // anonymous namespace
 
+class GLRenderer::UseGrContext {

[enne (OOO), 2014/04/23 19:53:41]

ScopedUseGrContext?

[Stephen White, 2014/04/23 19:57:20]

I find this guard class to be too much magic. I like guard classes which do
cleanup on destruction, but I'm not fond of meaty code that's done in a guard's
destructor. Are there actual early-outs that are made cleaner/less verbose by
the use of this guard, or could we simply make the explicit calls when
necessary?

[danakj, 2014/04/23 20:11:10]

 2014/04/23 19:57:20, Stephen White wrote:
This ensures that we clean up before returning to the GLRenderer. There's no
reason we can't add explicit calls and have that work. But then when someone
adds an early out some day in the future, we'll have to fix bugs.

This reminds me a lot of the skia manual-refcounting vs smart pointer
discussions. As with there, I sit on the side of do-it-with-scoped-objects when
you want to do something that is scoped in nature.

+ public:
+  static scoped_ptr<UseGrContext> Create(GLRenderer* renderer,
+                                         DrawingFrame* frame) {
+    if (!renderer->output_surface_->context_provider()->GrContext())
+      return scoped_ptr<UseGrContext>();
+    return make_scoped_ptr(new UseGrContext(renderer, frame));
+  }
+
+  ~UseGrContext() { PassControlToGLRenderer(); }
+
+  GrContext* context() const {
+    return renderer_->output_surface_->context_provider()->GrContext();
+  }
+
+ private:
+  UseGrContext(GLRenderer* renderer, DrawingFrame* frame)
+      : renderer_(renderer), frame_(frame) {
+    PassControlToSkia();
+  }
+
+  void PassControlToSkia() { context()->resetContext(); }
+
+  void PassControlToGLRenderer() {
+    context()->flush();
+    renderer_->RestoreGLState(frame_);
+  }
+
+  GLRenderer* renderer_;
+  DrawingFrame* frame_;
+  DISALLOW_COPY_AND_ASSIGN(UseGrContext);

[enne (OOO), 2014/04/23 19:53:41]

Stick a blank line before DISALLOW for readability?

+};
+
 struct GLRenderer::PendingAsyncReadPixels {
   PendingAsyncReadPixels() : buffer(0) {}
 
@@ -300,7 +333,7 @@ GLRenderer::GLRenderer(RendererClient* client,
   // so we only need to avoid POT textures if we have an NPOT fast-path.
   capabilities_.avoid_pow2_textures = context_caps.gpu.fast_npot_mo8_textures;
 
-  capabilities_.using_offscreen_context3d = true;
+  capabilities_.using_offscreen_context3d = false;
 
   capabilities_.using_map_image =
       settings_->use_map_image && context_caps.gpu.map_image;
@@ -569,25 +602,21 @@ void GLRenderer::DrawDebugBorderQuad(const DrawingFrame* frame,
   GLC(gl_, gl_->DrawElements(GL_LINE_LOOP, 4, GL_UNSIGNED_SHORT, 0));
 }
 
-static SkBitmap ApplyImageFilter(GLRenderer* renderer,
-                                 ContextProvider* offscreen_contexts,
-                                 const gfx::Point& origin,
-                                 SkImageFilter* filter,
-                                 ScopedResource* source_texture_resource) {
+static SkBitmap ApplyImageFilter(
+    scoped_ptr<GLRenderer::UseGrContext> use_gr_context,
+    ResourceProvider* resource_provider,
+    const gfx::Point& origin,
+    SkImageFilter* filter,
+    ScopedResource* source_texture_resource) {
   if (!filter)
     return SkBitmap();
 
-  if (!offscreen_contexts || !offscreen_contexts->GrContext())
+  if (!use_gr_context)
     return SkBitmap();
 
-  ResourceProvider::ScopedReadLockGL lock(renderer->resource_provider(),
+  ResourceProvider::ScopedReadLockGL lock(resource_provider,
                                           source_texture_resource->id());
 
-  // Flush the compositor context to ensure that textures there are available
-  // in the shared context.  Do this after locking/creating the compositor
-  // texture.
-  renderer->resource_provider()->Flush();
-
   // Wrap the source texture in a Ganesh platform texture.
   GrBackendTextureDesc backend_texture_description;
   backend_texture_description.fWidth = source_texture_resource->size().width();
@@ -597,7 +626,7 @@ static SkBitmap ApplyImageFilter(GLRenderer* renderer,
   backend_texture_description.fTextureHandle = lock.texture_id();
   backend_texture_description.fOrigin = kBottomLeft_GrSurfaceOrigin;
   skia::RefPtr<GrTexture> texture =
-      skia::AdoptRef(offscreen_contexts->GrContext()->wrapBackendTexture(
+      skia::AdoptRef(use_gr_context->context()->wrapBackendTexture(
           backend_texture_description));
 
   SkImageInfo info = {
@@ -622,12 +651,12 @@ static SkBitmap ApplyImageFilter(GLRenderer* renderer,
   desc.fConfig = kSkia8888_GrPixelConfig;
   desc.fOrigin = kBottomLeft_GrSurfaceOrigin;
   GrAutoScratchTexture scratch_texture(
-      offscreen_contexts->GrContext(), desc, GrContext::kExact_ScratchTexMatch);
+      use_gr_context->context(), desc, GrContext::kExact_ScratchTexMatch);
   skia::RefPtr<GrTexture> backing_store =
       skia::AdoptRef(scratch_texture.detach());
 
   // Create a device and canvas using that backing store.
-  SkGpuDevice device(offscreen_contexts->GrContext(), backing_store.get());
+  SkGpuDevice device(use_gr_context->context(), backing_store.get());
   SkCanvas canvas(&device);
 
   // Draw the source bitmap through the filter to the canvas.
@@ -641,25 +670,17 @@ static SkBitmap ApplyImageFilter(GLRenderer* renderer,
   canvas.translate(SkIntToScalar(-origin.x()), SkIntToScalar(-origin.y()));
   canvas.drawSprite(source, 0, 0, &paint);
 
-  // Flush skia context so that all the rendered stuff appears on the
-  // texture.
-  offscreen_contexts->GrContext()->flush();
-
-  // Flush the GL context so rendering results from this context are
-  // visible in the compositor's context.
-  offscreen_contexts->ContextGL()->Flush();
-
   return device.accessBitmap(false);
 }
 
 static SkBitmap ApplyBlendModeWithBackdrop(
-    GLRenderer* renderer,
-    ContextProvider* offscreen_contexts,
+    scoped_ptr<GLRenderer::UseGrContext> use_gr_context,
+    ResourceProvider* resource_provider,
     SkBitmap source_bitmap_with_filters,
     ScopedResource* source_texture_resource,
     ScopedResource* background_texture_resource,
     SkXfermode::Mode blend_mode) {
-  if (!offscreen_contexts || !offscreen_contexts->GrContext())
+  if (!use_gr_context)
     return source_bitmap_with_filters;
 
   DCHECK(background_texture_resource);
@@ -681,17 +702,12 @@ static SkBitmap ApplyBlendModeWithBackdrop(
     source_texture_with_filters_id = texture->getTextureHandle();
   } else {
     lock.reset(new ResourceProvider::ScopedReadLockGL(
-        renderer->resource_provider(), source_texture_resource->id()));
+        resource_provider, source_texture_resource->id()));
     source_texture_with_filters_id = lock->texture_id();
   }
 
   ResourceProvider::ScopedReadLockGL lock_background(
-      renderer->resource_provider(), background_texture_resource->id());
-
-  // Flush the compositor context to ensure that textures there are available
-  // in the shared context.  Do this after locking/creating the compositor
-  // texture.
-  renderer->resource_provider()->Flush();
+      resource_provider, background_texture_resource->id());
 
   // Wrap the source texture in a Ganesh platform texture.
   GrBackendTextureDesc backend_texture_description;
@@ -702,14 +718,14 @@ static SkBitmap ApplyBlendModeWithBackdrop(
   backend_texture_description.fHeight = source_size.height();
   backend_texture_description.fTextureHandle = source_texture_with_filters_id;
   skia::RefPtr<GrTexture> source_texture =
-      skia::AdoptRef(offscreen_contexts->GrContext()->wrapBackendTexture(
+      skia::AdoptRef(use_gr_context->context()->wrapBackendTexture(
           backend_texture_description));
 
   backend_texture_description.fWidth = background_size.width();
   backend_texture_description.fHeight = background_size.height();
   backend_texture_description.fTextureHandle = lock_background.texture_id();
   skia::RefPtr<GrTexture> background_texture =
-      skia::AdoptRef(offscreen_contexts->GrContext()->wrapBackendTexture(
+      skia::AdoptRef(use_gr_context->context()->wrapBackendTexture(
           backend_texture_description));
 
   SkImageInfo source_info = {
@@ -748,12 +764,12 @@ static SkBitmap ApplyBlendModeWithBackdrop(
   desc.fConfig = kSkia8888_GrPixelConfig;
   desc.fOrigin = kBottomLeft_GrSurfaceOrigin;
   GrAutoScratchTexture scratch_texture(
-      offscreen_contexts->GrContext(), desc, GrContext::kExact_ScratchTexMatch);
+      use_gr_context->context(), desc, GrContext::kExact_ScratchTexMatch);
   skia::RefPtr<GrTexture> backing_store =
       skia::AdoptRef(scratch_texture.detach());
 
   // Create a device and canvas using that backing store.
-  SkGpuDevice device(offscreen_contexts->GrContext(), backing_store.get());
+  SkGpuDevice device(use_gr_context->context(), backing_store.get());
   SkCanvas canvas(&device);
 
   // Draw the source bitmap through the filter to the canvas.
@@ -763,14 +779,6 @@ static SkBitmap ApplyBlendModeWithBackdrop(
   paint.setXfermodeMode(blend_mode);
   canvas.drawSprite(source, 0, 0, &paint);
 
-  // Flush skia context so that all the rendered stuff appears on the
-  // texture.
-  offscreen_contexts->GrContext()->flush();
-
-  // Flush the GL context so rendering results from this context are
-  // visible in the compositor's context.
-  offscreen_contexts->ContextGL()->Flush();
-
   return device.accessBitmap(false);
 }
 
@@ -847,8 +855,8 @@ scoped_ptr<ScopedResource> GLRenderer::GetBackgroundWithFilters(
   SkBitmap filtered_device_background;
   if (apply_background_filters) {
     filtered_device_background =
-        ApplyImageFilter(this,
-                         frame->offscreen_context_provider,
+        ApplyImageFilter(UseGrContext::Create(this, frame),
+                         resource_provider_,
                          quad->rect.origin(),
                          filter.get(),
                          device_background_texture.get());
@@ -959,8 +967,6 @@ void GLRenderer::DrawRenderPassQuad(DrawingFrame* frame,
   SkBitmap filter_bitmap;
   SkScalar color_matrix[20];
   bool use_color_matrix = false;
-  // TODO(ajuma): Always use RenderSurfaceFilters::BuildImageFilter, not just
-  // when we have a reference filter.
   if (!quad->filters.IsEmpty()) {
     skia::RefPtr<SkImageFilter> filter = RenderSurfaceFilters::BuildImageFilter(
         quad->filters, contents_texture->size());
@@ -978,8 +984,8 @@ void GLRenderer::DrawRenderPassQuad(DrawingFrame* frame,
         // in the compositor.
         use_color_matrix = true;
       } else {
-        filter_bitmap = ApplyImageFilter(this,
-                                         frame->offscreen_context_provider,
+        filter_bitmap = ApplyImageFilter(UseGrContext::Create(this, frame),
+                                         resource_provider_,
                                          quad->rect.origin(),
                                          filter.get(),
                                          contents_texture);
@@ -990,8 +996,8 @@ void GLRenderer::DrawRenderPassQuad(DrawingFrame* frame,
   if (quad->shared_quad_state->blend_mode != SkXfermode::kSrcOver_Mode &&
       background_texture) {
     filter_bitmap =
-        ApplyBlendModeWithBackdrop(this,
-                                   frame->offscreen_context_provider,
+        ApplyBlendModeWithBackdrop(UseGrContext::Create(this, frame),
+                                   resource_provider_,
                                    filter_bitmap,
                                    contents_texture,
                                    background_texture.get(),
@@ -3131,6 +3137,41 @@ void GLRenderer::ReinitializeGLState() {
   scissor_rect_needs_reset_ = true;
 }
 
+void GLRenderer::RestoreGLState(DrawingFrame* frame) {

[enne (OOO), 2014/04/23 19:53:41]

How does this function compare to ReinitializeGLState?

[danakj, 2014/04/23 19:56:05]

It's similar but

1. It doesn't change any state on the GLRenderer class itself, except the
program.

(Whereas the other resets variables to default states and forces things off/on
which is not valid in the middle of drawing a set of quads.)

2. It does UseRenderPass() to re-setup the viewport.

[enne (OOO), 2014/04/23 20:01:10]

I think you should combine these if possible.  I think you could make
ReinitializeGLState do what you do here, to set stencil and blend to the
shadowed state rather than clobbering.  I also think this doesn't do the right
thing wrt scissor.  You should have to set scissor_rect_needs_reset_ to true and
do something with is_scissor_enabled_ as well, right?
That is different, but maybe that can be moved into the UseGrContext class?

[danakj, 2014/04/23 20:11:10]

I can give merging them a try again.
I don't think so. The scissor_rect_needs_reset_ = true causes gl_->Scissor() to
be called. But we're calling it here. I specifically want to not change any
member variables on GLRenderer, so it can come back in the same state it was in.

Because I've set the Scissor, I could change scissor_rect_needs_reset_ to false
here, but it will always be false during the drawing of the frame if the scissor
was ever enabled.
I'll try making it a separate method on the GLRenderer.

+  shared_geometry_->PrepareForDraw();
+
+  GLC(gl_, gl_->Disable(GL_DEPTH_TEST));
+  GLC(gl_, gl_->Disable(GL_CULL_FACE));
+  GLC(gl_, gl_->ColorMask(true, true, true, true));
+  GLC(gl_, gl_->BlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA));
+  GLC(gl_, gl_->ActiveTexture(GL_TEXTURE0));
+
+  if (stencil_shadow_)

[Stephen White, 2014/04/23 19:57:20]

I'm a little nervous that this state-setting code is only used by filters, and
not by GPU rasterization, for example, which might leave it fragile.

Is there a way we could refactor this with ReinitializeGLState()? I.e.,
ReinitializeGLState() could set the member vars (stencil_shadow_, blend_shadow_,
is_scissor_enabled_, etc), and then just call this function to actually set GL
state? Then it would get exercised more broadly.

+    GLC(gl_, gl_->Enable(GL_STENCIL_TEST));
+  else
+    GLC(gl_, gl_->Disable(GL_STENCIL_TEST));
+
+  if (blend_shadow_)
+    GLC(gl_, gl_->Enable(GL_BLEND));
+  else
+    GLC(gl_, gl_->Disable(GL_BLEND));
+
+  if (is_scissor_enabled_) {
+    GLC(gl_, gl_->Enable(GL_SCISSOR_TEST));
+    GLC(gl_,
+        gl_->Scissor(scissor_rect_.x(),
+                     scissor_rect_.y(),
+                     scissor_rect_.width(),
+                     scissor_rect_.height()));
+  } else {
+    GLC(gl_, gl_->Disable(GL_SCISSOR_TEST));
+  }
+
+  SetUseProgram(0);
+
+  UseRenderPass(frame, frame->current_render_pass);
+}
+
 bool GLRenderer::IsContextLost() {
   return output_surface_->context_provider()->IsContextLost();
 }