cc: Implement overdraw feedback debugging feature.

cc/output/gl_renderer.cc

 // Copyright 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/output/gl_renderer.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <limits>
 #include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
+#include "base/barrier_closure.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
@@ skipping 336 matching lines @@
   };
 
   gpu::gles2::GLES2Interface* gl_;
   unsigned query_id_;
   bool is_pending_;
   base::WeakPtrFactory<SyncQuery> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncQuery);
 };
 
+struct GLRenderer::PendingOverdrawQuery {
+  explicit PendingOverdrawQuery(unsigned query) : query(query) {}
+
+  base::CancelableClosure overdraw_callback;

[danakj, 2017/01/12 00:42:58]

this makes a weakptrfactory for each query, is that overkill?

[reveman, 2017/01/13 01:27:43]

This code has been removed from latest patch.

+  const unsigned query;

[danakj, 2017/01/12 00:42:57]

query is a GLuint, can you use that instead of "unsigned" throughout?

[reveman, 2017/01/13 01:27:42]

This code has been removed from latest patch but unsigned instead of GLuint
seems very common in GLRenderer so I used that for consistency.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PendingOverdrawQuery);
+};
+
 GLRenderer::GLRenderer(const RendererSettings* settings,
                        OutputSurface* output_surface,
                        ResourceProvider* resource_provider,
                        TextureMailboxDeleter* texture_mailbox_deleter,
                        int highp_threshold_min)
     : DirectRenderer(settings, output_surface, resource_provider),
       offscreen_framebuffer_id_(0),
       shared_geometry_quad_(QuadVertexRect()),
       gl_(output_surface->context_provider()->ContextGL()),
       context_support_(output_surface->context_provider()->ContextSupport()),
@@ skipping 25 matching lines @@
 }
 
 GLRenderer::~GLRenderer() {
   while (!pending_async_read_pixels_.empty()) {
     PendingAsyncReadPixels* pending_read =
         pending_async_read_pixels_.back().get();
     pending_read->finished_read_pixels_callback.Cancel();
     pending_async_read_pixels_.pop_back();
   }
 
+  while (!pending_overdraw_queries_.empty()) {
+    PendingOverdrawQuery* pending_query =
+        pending_overdraw_queries_.back().get();
+    pending_query->overdraw_callback.Cancel();

[danakj, 2017/01/12 00:42:58]

destroying a WeakPtrFactory (inside the CancelableCallback) invalidates the
weakptr, no need to Cancel. I guess I did not know these things when I wrote
pending_async_read_pixels_ stuff.

[reveman, 2017/01/13 01:27:42]

This code has been removed from latest patch.

+    pending_overdraw_queries_.pop_back();
+  }
+
   CleanupSharedObjects();
 
   if (context_visibility_) {
     auto* context_provider = output_surface_->context_provider();
     auto* cache_controller = context_provider->CacheController();
     cache_controller->ClientBecameNotVisible(std::move(context_visibility_));
   }
 }
 
 bool GLRenderer::CanPartialSwap() {
@@ skipping 60 matching lines @@
 }
 
 void GLRenderer::ClearFramebuffer(DrawingFrame* frame) {
   // On DEBUG builds, opaque render passes are cleared to blue to easily see
   // regions that were not drawn on the screen.
   if (frame->current_render_pass->has_transparent_background)
     gl_->ClearColor(0, 0, 0, 0);
   else
     gl_->ClearColor(0, 0, 1, 1);
 
-  bool always_clear = false;
+  gl_->ClearStencil(0);

[danakj, 2017/01/12 00:42:58]

Does this add cost when not using overdraw feedback?

[reveman, 2017/01/13 01:27:42]

Pretty sure it doesn't and doing this unconditionally is consistent with doing
the ClearColor above even if we're not going to do a clear below.

+
+  bool always_clear = overdraw_feedback_;
 #ifndef NDEBUG
   always_clear = true;
 #endif
   if (always_clear || frame->current_render_pass->has_transparent_background) {
     GLbitfield clear_bits = GL_COLOR_BUFFER_BIT;
     if (always_clear)
       clear_bits |= GL_STENCIL_BUFFER_BIT;
     gl_->Clear(clear_bits);
   }
 }
@@ skipping 2248 matching lines @@
   }
 }
 
 void GLRenderer::FinishDrawingFrame(DrawingFrame* frame) {
   if (use_sync_query_) {
     DCHECK(current_sync_query_);
     current_sync_query_->End();
     pending_sync_queries_.push_back(std::move(current_sync_query_));
   }
 
+  swap_buffer_rect_.Union(frame->root_damage_rect);
+  if (overdraw_feedback_)
+    FlushOverdrawFeedback(frame, swap_buffer_rect_);
+
   current_framebuffer_lock_ = nullptr;
-  swap_buffer_rect_.Union(frame->root_damage_rect);
 
   gl_->Disable(GL_BLEND);
   blend_shadow_ = false;
 
   ScheduleCALayers(frame);
   ScheduleOverlays(frame);
 }
 
 void GLRenderer::FinishDrawingQuadList() {
   FlushTextureQuadCache(SHARED_BINDING);
@@ skipping 217 matching lines @@
 void GLRenderer::GetFramebufferPixelsAsync(
     const DrawingFrame* frame,
     const gfx::Rect& rect,
     std::unique_ptr<CopyOutputRequest> request) {
   DCHECK(!request->IsEmpty());
   if (request->IsEmpty())
     return;
   if (rect.IsEmpty())
     return;
 
+  if (overdraw_feedback_)

[danakj, 2017/01/12 00:42:58]

how come you do this into readbacks?

[reveman, 2017/01/13 01:27:42]

For screenshots. That's very important for debugging purposes.

[danakj, 2017/01/16 16:02:57]

Do you need it in screenshots of non-root render surfaces? Just noting that
non-root stuff seems like a good 50% of the code in here..

[reveman, 2017/01/16 18:02:26]

This is needed for all types of screenshots on ChromeOS as discussed.

+    FlushOverdrawFeedback(frame, rect);
+
   gfx::Rect window_rect = MoveFromDrawToWindowSpace(frame, rect);
   DCHECK_GE(window_rect.x(), 0);
   DCHECK_GE(window_rect.y(), 0);
   DCHECK_LE(window_rect.right(), current_surface_size_.width());
   DCHECK_LE(window_rect.bottom(), current_surface_size_.height());
 
   if (!request->force_bitmap_result()) {
     bool own_mailbox = !request->has_texture_mailbox();
 
     GLuint texture_id = 0;
@@ skipping 180 matching lines @@
   gl_->CopyTexImage2D(GL_TEXTURE_2D, 0, format, window_rect.x(),
                       window_rect.y(), window_rect.width(),
                       window_rect.height(), 0);
   gl_->BindTexture(GL_TEXTURE_2D, 0);
 }
 
 void GLRenderer::BindFramebufferToOutputSurface(DrawingFrame* frame) {
   current_framebuffer_lock_ = nullptr;
   output_surface_->BindFramebuffer();
 
-  if (output_surface_->HasExternalStencilTest()) {
+  if (overdraw_feedback_) {
+    SetupOverdrawFeedback();
+    SetStencilEnabled(true);
+  } else if (output_surface_->HasExternalStencilTest()) {

[danakj, 2017/01/12 00:42:58]

using else here means turning on overdraw feedback does bad things to webview,
which is okay I guess, is this what you intended?

[reveman, 2017/01/13 01:27:42]

I'm expecting an output surface that might have an external stencil test to
never report that stencil operations are available. overdraw_feedback_ should in
that case never be true. Added a DCHECK here to verify that.

     output_surface_->ApplyExternalStencil();
     SetStencilEnabled(true);
   } else {
     SetStencilEnabled(false);
   }
 }
 
 bool GLRenderer::BindFramebufferToTexture(DrawingFrame* frame,
                                           const ScopedResource* texture) {
   DCHECK(texture->id());
 
   // Explicitly release lock, otherwise we can crash when try to lock
   // same texture again.
   current_framebuffer_lock_ = nullptr;
 
-  SetStencilEnabled(false);
   gl_->BindFramebuffer(GL_FRAMEBUFFER, offscreen_framebuffer_id_);
   current_framebuffer_lock_ =
       base::MakeUnique<ResourceProvider::ScopedWriteLockGL>(
           resource_provider_, texture->id(), false);
   current_framebuffer_format_ = texture->format();
   unsigned texture_id = current_framebuffer_lock_->texture_id();
   gl_->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
                             texture_id, 0);
+  if (overdraw_feedback_) {
+    if (!offscreen_stencil_renderbuffer_id_)
+      gl_->GenRenderbuffers(1, &offscreen_stencil_renderbuffer_id_);
+    if (texture->size() != offscreen_stencil_renderbuffer_size_) {
+      gl_->BindRenderbuffer(GL_RENDERBUFFER,
+                            offscreen_stencil_renderbuffer_id_);
+      gl_->RenderbufferStorage(GL_RENDERBUFFER, GL_STENCIL_INDEX8,
+                               texture->size().width(),
+                               texture->size().height());
+      gl_->BindRenderbuffer(GL_RENDERBUFFER, 0);
+      offscreen_stencil_renderbuffer_size_ = texture->size();
+    }
+    gl_->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT,
+                                 GL_RENDERBUFFER,
+                                 offscreen_stencil_renderbuffer_id_);
+  }
 
   DCHECK(gl_->CheckFramebufferStatus(GL_FRAMEBUFFER) ==
              GL_FRAMEBUFFER_COMPLETE ||
          IsContextLost());
+
+  if (overdraw_feedback_) {

[danakj, 2017/01/12 00:42:58]

Hm why do you do stencil and overdraw stuff for child render passes, is this
just for copy requests? What's the use case for that?

[reveman, 2017/01/13 01:27:42]

Yes, this is for copy requests.

+    SetupOverdrawFeedback();
+    SetStencilEnabled(true);
+  } else {
+    SetStencilEnabled(false);
+  }
   return true;
 }
 
 void GLRenderer::SetScissorTestRect(const gfx::Rect& scissor_rect) {
   EnsureScissorTestEnabled();
 
   // Don't unnecessarily ask the context to change the scissor, because it
   // may cause undesired GPU pipeline flushes.
   if (scissor_rect == scissor_rect_)
     return;
@@ skipping 481 matching lines @@
     video_stream_texture_program_[i].Cleanup(gl_);
   }
 
   debug_border_program_.Cleanup(gl_);
   solid_color_program_.Cleanup(gl_);
   solid_color_program_aa_.Cleanup(gl_);
 
   if (offscreen_framebuffer_id_)
     gl_->DeleteFramebuffers(1, &offscreen_framebuffer_id_);
 
+  if (offscreen_stencil_renderbuffer_id_)
+    gl_->DeleteRenderbuffers(1, &offscreen_stencil_renderbuffer_id_);
+
   ReleaseRenderPassTextures();
 }
 
 void GLRenderer::ReinitializeGLState() {
   is_scissor_enabled_ = false;
   scissor_rect_ = gfx::Rect();
   stencil_shadow_ = false;
   blend_shadow_ = true;
   program_shadow_ = 0;
 
@@ skipping 312 matching lines @@
 
   // The alpha has already been applied when copying the RPDQ to an IOSurface.
   GLfloat alpha = 1;
   gl_->ScheduleCALayerSharedStateCHROMIUM(alpha, is_clipped, clip_rect,
                                           sorting_context_id, gl_transform);
   gl_->ScheduleCALayerCHROMIUM(
       texture_id, contents_rect, ca_layer_overlay->background_color,
       ca_layer_overlay->edge_aa_mask, bounds_rect, filter);
 }
 
+void GLRenderer::SetupOverdrawFeedback() {
+  gl_->StencilFunc(GL_ALWAYS, 1, 0xffffffff);
+  // First two values are ignored as test always passes.
+  gl_->StencilOp(GL_KEEP, GL_KEEP, GL_INCR);
+  gl_->StencilMask(0xff);
+}
+
+void GLRenderer::FlushOverdrawFeedback(const DrawingFrame* frame,
+                                       const gfx::Rect& output_rect) {
+  // Return early if already flushed.

[danakj, 2017/01/12 00:42:58]

I don't understand how we can flush twice for a single renderpass/frame? Maybe
DCHECK or can you explain to me?

[reveman, 2017/01/13 01:27:42]

Good point. Now a DCHECK.

+  if (!stencil_shadow_)
+    return;
+
+  // Test only, keep everything.
+  gl_->StencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
+  gl_->StencilMask(0);
+
+  EnsureScissorTestDisabled();
+  SetBlendEnabled(true);
+
+  PrepareGeometry(SHARED_BINDING);
+
+  const DebugBorderProgram* program = GetDebugBorderProgram();
+  DCHECK(program && (program->initialized() || IsContextLost()));
+  SetUseProgram(program->program());
+
+  gfx::Transform render_matrix;
+  render_matrix.Translate(0.5 * output_rect.width() + output_rect.x(),
+                          0.5 * output_rect.height() + output_rect.y());
+  render_matrix.Scale(output_rect.width(), output_rect.height());
+  static float gl_matrix[16];
+  GLRenderer::ToGLMatrix(&gl_matrix[0],
+                         frame->projection_matrix * render_matrix);
+  gl_->UniformMatrix4fv(program->vertex_shader().matrix_location(), 1, false,
+                        &gl_matrix[0]);
+
+  // Produce hinting for the amount of overdraw on screen for each pixel by
+  // drawing hint colors to the framebuffer based on the current stencil value.
+  struct {
+    int category;
+    GLenum func;
+    GLint ref;
+    SkColor color;
+  } stencil_tests[] = {
+      {1, GL_EQUAL, 2, 0x2f0000ff},  // Blue: Overdrawn once.
+      {2, GL_EQUAL, 3, 0x2f00ff00},  // Green: Overdrawn twice.
+      {3, GL_EQUAL, 4, 0x3fff0000},  // Pink: Overdrawn three times.
+      {4, GL_LESS, 4, 0x7fff0000},   // Red: Overdrawn four or more times.
+  };
+
+  // Occlusion queries can be expensive, so only collect trace data if we select
+  // cc.debug.overdraw.
+  bool tracing_enabled;
+  TRACE_EVENT_CATEGORY_GROUP_ENABLED(
+      TRACE_DISABLED_BY_DEFAULT("cc.debug.overdraw"), &tracing_enabled);
+
+  // Trace only the root render pass.
+  if (frame->current_render_pass != frame->root_render_pass)
+    tracing_enabled = false;
+
+  base::Closure barrier = base::BarrierClosure(
+      arraysize(stencil_tests),
+      base::Bind(&GLRenderer::UpdateOverdrawCounter, base::Unretained(this)));
+
+  for (const auto& test : stencil_tests) {
+    if (tracing_enabled) {
+      GLuint query = 0;
+      gl_->GenQueriesEXT(1, &query);
+      // TODO(reveman): Use SAMPLES_PASSED_ARB when available for exact amount
+      // of overdraw.
+      gl_->BeginQueryEXT(GL_ANY_SAMPLES_PASSED_EXT, query);

[danakj, 2017/01/12 00:42:57]

would it not work to do one query around the whole loop? what would that do?

[reveman, 2017/01/13 01:27:43]

We want samples for each overdraw category and with SAMPLES_PASSED_ARB even the
exact number of samples for each category. One query outside the loop would just
give us samples for all categories.

+
+      std::unique_ptr<PendingOverdrawQuery> pending_overdraw_query(
+          new PendingOverdrawQuery(query));
+      pending_overdraw_queries_.push_back(std::move(pending_overdraw_query));
+    }
+
+    gl_->StencilFunc(test.func, test.ref, 0xffffffff);
+    // Transparent color unless color-coding of overdraw is enabled.
+    Float4 color =
+        PremultipliedColor(settings_->show_overdraw_feedback ? test.color : 0);
+    gl_->Uniform4fv(program->fragment_shader().color_location(), 1, color.data);
+    gl_->DrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, 0);
+
+    if (tracing_enabled) {
+      base::Closure overdraw_feedback_callback = base::Bind(
+          &GLRenderer::OverdrawFeedback, base::Unretained(this),
+          pending_overdraw_queries_.back()->query, test.category, barrier);
+
+      // Save the overdraw_feedback_callback so it can be cancelled.

[danakj, 2017/01/12 00:42:57]

If you just put a WeakPtrFactory on GLRenderer and post with that as the |this|
instead of Unretained, then you don't need to save the closure at all.

[reveman, 2017/01/13 01:27:42]

Done but updated to be less complicated as a result of removing
PendingOverdrawQuery.

+      pending_overdraw_queries_.back()->overdraw_callback.Reset(
+          overdraw_feedback_callback);
+      base::Closure cancelable_callback =
+          pending_overdraw_queries_.back()->overdraw_callback.callback();
+
+      gl_->EndQueryEXT(GL_ANY_SAMPLES_PASSED_EXT);
+      context_support_->SignalQuery(pending_overdraw_queries_.back()->query,
+                                    cancelable_callback);
+    }
+  }
+
+  gl_->StencilFunc(GL_ALWAYS, 0, 0xffffffff);
+  gl_->Disable(GL_STENCIL_TEST);
+  stencil_shadow_ = false;

[danakj, 2017/01/12 00:42:58]

It feels weird that this method is changing this, call SetStencilEnabled
instead?

But also can you maybe do that at the caller instead of hiding it inside this
method, whose name does not sound like it will change stencil states?

And why do we do this at all, we did not disable stencil at the end of the frame
(or render pass) when it was enabled for webview?

[reveman, 2017/01/13 01:27:42]

Good point. Removed it.

+}
+
+void GLRenderer::OverdrawFeedback(unsigned query,
+                                  int category,
+                                  base::Closure barrier) {

[danakj, 2017/01/12 00:42:58]

const&

[reveman, 2017/01/13 01:27:42]

Done.

+  DCHECK(!pending_overdraw_queries_.empty());
+  DCHECK_EQ(pending_overdraw_queries_.front()->query, query);
+  pending_overdraw_queries_.pop_front();
+
+  unsigned result = 0;
+  if (query) {
+    gl_->GetQueryObjectuivEXT(query, GL_QUERY_RESULT_EXT, &result);
+    gl_->DeleteQueriesEXT(1, &query);
+  }
+
+  if (result)
+    current_overdraw_.insert(category);
+  else
+    current_overdraw_.erase(category);
+
+  barrier.Run();
+}
+
+void GLRenderer::UpdateOverdrawCounter() const {
+  // Report overdraw as multiple of the screen size. ie. 1x for the whole
+  // screen is 1.0. Note: this will always report the worst overdraw on screen
+  // as the overdraw for the whole screen until SAMPLES_PASSED_ARB is supported.
+  TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("cc.debug.overdraw"), "GPU Overdraw",
+                 current_overdraw_.empty() ? 0 : *current_overdraw_.rbegin());
+}
+
 }  // namespace cc