cc: Use DrawingFrame as state, not function argument

cc/output/direct_renderer.cc

Index: cc/output/direct_renderer.cc
diff --git a/cc/output/direct_renderer.cc b/cc/output/direct_renderer.cc
index cd4bfa5006e9eb30ab850e1dcd83f1d5b9e41ff8..8b7aa22879a680ce938d087d4490e6f52b0359a0 100644
--- a/cc/output/direct_renderer.cc
+++ b/cc/output/direct_renderer.cc
@@ -114,26 +114,21 @@ void DirectRenderer::InitializeViewport(DrawingFrame* frame,
   DCHECK_GE(viewport_rect.y(), 0);
   DCHECK_LE(viewport_rect.right(), surface_size.width());
   DCHECK_LE(viewport_rect.bottom(), surface_size.height());
-  bool flip_y = FlippedFramebuffer(frame);
+  bool flip_y = FlippedFramebuffer();
   if (flip_y) {
-    frame->projection_matrix = OrthoProjectionMatrix(draw_rect.x(),
-                                                     draw_rect.right(),
-                                                     draw_rect.bottom(),
-                                                     draw_rect.y());
+    frame->projection_matrix = OrthoProjectionMatrix(
+        draw_rect.x(), draw_rect.right(), draw_rect.bottom(), draw_rect.y());
   } else {
-    frame->projection_matrix = OrthoProjectionMatrix(draw_rect.x(),
-                                                     draw_rect.right(),
-                                                     draw_rect.y(),
-                                                     draw_rect.bottom());
+    frame->projection_matrix = OrthoProjectionMatrix(
+        draw_rect.x(), draw_rect.right(), draw_rect.y(), draw_rect.bottom());
   }
 
   gfx::Rect window_rect = viewport_rect;
   if (flip_y)
     window_rect.set_y(surface_size.height() - viewport_rect.bottom());
-  frame->window_matrix = window_matrix(window_rect.x(),
-                                       window_rect.y(),
-                                       window_rect.width(),
-                                       window_rect.height());
+  frame->window_matrix =
+      window_matrix(window_rect.x(), window_rect.y(), window_rect.width(),
+                    window_rect.height());
   current_draw_rect_ = draw_rect;
   current_viewport_rect_ = viewport_rect;
   current_surface_size_ = surface_size;
@@ -141,12 +136,11 @@ void DirectRenderer::InitializeViewport(DrawingFrame* frame,
 }
 
 gfx::Rect DirectRenderer::MoveFromDrawToWindowSpace(
-    const DrawingFrame* frame,
     const gfx::Rect& draw_rect) const {
   gfx::Rect window_rect = draw_rect;
   window_rect -= current_draw_rect_.OffsetFromOrigin();
   window_rect += current_viewport_rect_.OffsetFromOrigin();
-  if (FlippedFramebuffer(frame))
+  if (FlippedFramebuffer())
     window_rect.set_y(current_surface_size_.height() - window_rect.bottom());
   return window_rect;
 }
@@ -244,18 +238,21 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
                                                      overdraw_feedback);
 
   DrawingFrame frame;
-  frame.render_passes_in_draw_order = render_passes_in_draw_order;
-  frame.root_render_pass = root_render_pass;
-  frame.root_damage_rect = root_render_pass->damage_rect;
-  frame.root_damage_rect.Union(overlay_processor_->GetAndResetOverlayDamage());
-  frame.root_damage_rect.Intersect(gfx::Rect(device_viewport_size));
-  frame.device_viewport_size = device_viewport_size;
-  frame.device_color_space = device_color_space;
+  current_frame_ = &frame;

[enne (OOO), 2017/02/08 22:32:40]

In general I'm pretty fine with keeping the frame as a member instead of passing
it around everywhere.  That seems like good cleanup.

I'm a little <_< about member pointer to stack variable though.

What about just making current_frame_ a DrawingFrame and not a pointer, and
adding a reset() function or something that you could call here instead?

[ccameron, 2017/02/08 22:35:59]

That was what I originally had in the patch, but then I really wanted it to
explode if you tried to access the current frame outside of DrawFrame(). So then
I added accessor functions and DCHECKs, and it seemed a bit much, so I started
with this.

+  current_frame_->render_passes_in_draw_order = render_passes_in_draw_order;
+  current_frame_->root_render_pass = root_render_pass;
+  current_frame_->root_damage_rect = root_render_pass->damage_rect;
+  current_frame_->root_damage_rect.Union(
+      overlay_processor_->GetAndResetOverlayDamage());
+  current_frame_->root_damage_rect.Intersect(gfx::Rect(device_viewport_size));
+  current_frame_->device_viewport_size = device_viewport_size;
+  current_frame_->device_color_space = device_color_space;
 
   // Only reshape when we know we are going to draw. Otherwise, the reshape
   // can leave the window at the wrong size if we never draw and the proper
   // viewport size is never set.
-  bool frame_has_alpha = frame.root_render_pass->has_transparent_background;
+  bool frame_has_alpha =
+      current_frame_->root_render_pass->has_transparent_background;
   bool use_stencil = overdraw_feedback_;
   if (device_viewport_size != reshape_surface_size_ ||
       device_scale_factor != reshape_device_scale_factor_ ||
@@ -265,14 +262,15 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
     reshape_surface_size_ = device_viewport_size;
     reshape_device_scale_factor_ = device_scale_factor;
     reshape_device_color_space_ = device_color_space;
-    reshape_has_alpha_ = frame.root_render_pass->has_transparent_background;
+    reshape_has_alpha_ =
+        current_frame_->root_render_pass->has_transparent_background;
     reshape_use_stencil_ = overdraw_feedback_;
     output_surface_->Reshape(
         reshape_surface_size_, reshape_device_scale_factor_,
         reshape_device_color_space_, reshape_has_alpha_, reshape_use_stencil_);
   }
 
-  BeginDrawingFrame(&frame);
+  BeginDrawingFrame();
 
   for (const auto& pass : *render_passes_in_draw_order) {
     if (!pass->filters.IsEmpty())
@@ -290,7 +288,7 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
   for (const auto& pass : *render_passes_in_draw_order) {
     if (pass.get() == root_render_pass)
       break;
-    DrawRenderPassAndExecuteCopyRequests(&frame, pass.get());
+    DrawRenderPassAndExecuteCopyRequests(pass.get());
   }
 
   // Create the overlay candidate for the output surface, and mark it as
@@ -303,78 +301,78 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
         root_render_pass->output_rect;
     output_surface_plane.use_output_surface_for_resource = true;
     output_surface_plane.overlay_handled = true;
-    frame.overlay_list.push_back(output_surface_plane);
+    current_frame_->overlay_list.push_back(output_surface_plane);
   }
 
   // Attempt to replace some or all of the quads of the root render pass with
   // overlays.
   overlay_processor_->ProcessForOverlays(
       resource_provider_, root_render_pass, render_pass_filters_,
-      render_pass_background_filters_, &frame.overlay_list,
-      &frame.ca_layer_overlay_list, &frame.root_damage_rect);
+      render_pass_background_filters_, &current_frame_->overlay_list,
+      &current_frame_->ca_layer_overlay_list,
+      &current_frame_->root_damage_rect);
 
   // We can skip all drawing if the damage rect is now empty.
   bool skip_drawing_root_render_pass =
-      frame.root_damage_rect.IsEmpty() && allow_empty_swap_;
+      current_frame_->root_damage_rect.IsEmpty() && allow_empty_swap_;
 
   // If we have to draw but don't support partial swap, the whole output should
   // be considered damaged.
   if (!skip_drawing_root_render_pass && !use_partial_swap_)
-    frame.root_damage_rect = root_render_pass->output_rect;
+    current_frame_->root_damage_rect = root_render_pass->output_rect;
 
   if (skip_drawing_root_render_pass) {
     // If any of the overlays is the output surface, then ensure that the
     // backbuffer be allocated (allocation of the backbuffer is a side-effect
     // of BindFramebufferToOutputSurface).
-    for (auto& overlay : frame.overlay_list) {
+    for (auto& overlay : current_frame_->overlay_list) {
       if (overlay.use_output_surface_for_resource) {
-        BindFramebufferToOutputSurface(&frame);
+        BindFramebufferToOutputSurface();
         break;
       }
     }
   } else {
-    DrawRenderPassAndExecuteCopyRequests(&frame, root_render_pass);
+    DrawRenderPassAndExecuteCopyRequests(root_render_pass);
   }
 
-  FinishDrawingFrame(&frame);
+  FinishDrawingFrame();
   render_passes_in_draw_order->clear();
   render_pass_filters_.clear();
   render_pass_background_filters_.clear();
+  current_frame_ = nullptr;
 }
 
-gfx::Rect DirectRenderer::ComputeScissorRectForRenderPass(
-    const DrawingFrame* frame) {
-  if (frame->current_render_pass == frame->root_render_pass)
-    return frame->root_damage_rect;
+gfx::Rect DirectRenderer::DrawingFrame::ComputeScissorRectForRenderPass()
+    const {
+  if (current_render_pass == root_render_pass)
+    return root_damage_rect;
 
   // If the root damage rect has been expanded due to overlays, all the other
   // damage rect calculations are incorrect.
-  if (!frame->root_render_pass->damage_rect.Contains(frame->root_damage_rect))
-    return frame->current_render_pass->output_rect;
+  if (!root_render_pass->damage_rect.Contains(root_damage_rect))
+    return current_render_pass->output_rect;
 
-  DCHECK(frame->current_render_pass->copy_requests.empty() ||
-         (frame->current_render_pass->damage_rect ==
-          frame->current_render_pass->output_rect));
-  return frame->current_render_pass->damage_rect;
+  DCHECK(
+      current_render_pass->copy_requests.empty() ||
+      (current_render_pass->damage_rect == current_render_pass->output_rect));
+  return current_render_pass->damage_rect;
 }
 
-gfx::Rect DirectRenderer::DeviceViewportRectInDrawSpace(
-    const DrawingFrame* frame) const {
-  gfx::Rect device_viewport_rect(frame->device_viewport_size);
+gfx::Rect DirectRenderer::DeviceViewportRectInDrawSpace() const {
+  gfx::Rect device_viewport_rect(current_frame_->device_viewport_size);
   device_viewport_rect -= current_viewport_rect_.OffsetFromOrigin();
   device_viewport_rect += current_draw_rect_.OffsetFromOrigin();
   return device_viewport_rect;
 }
 
-gfx::Rect DirectRenderer::OutputSurfaceRectInDrawSpace(
-    const DrawingFrame* frame) const {
-  if (frame->current_render_pass == frame->root_render_pass) {
-    gfx::Rect output_surface_rect(frame->device_viewport_size);
+gfx::Rect DirectRenderer::OutputSurfaceRectInDrawSpace() const {
+  if (current_frame_->current_render_pass == current_frame_->root_render_pass) {
+    gfx::Rect output_surface_rect(current_frame_->device_viewport_size);
     output_surface_rect -= current_viewport_rect_.OffsetFromOrigin();
     output_surface_rect += current_draw_rect_.OffsetFromOrigin();
     return output_surface_rect;
   } else {
-    return frame->current_render_pass->output_rect;
+    return current_frame_->current_render_pass->output_rect;
   }
 }
 
@@ -393,7 +391,6 @@ bool DirectRenderer::ShouldSkipQuad(const DrawQuad& quad,
 }
 
 void DirectRenderer::SetScissorStateForQuad(
-    const DrawingFrame* frame,
     const DrawQuad& quad,
     const gfx::Rect& render_pass_scissor,
     bool use_render_pass_scissor) {
@@ -401,10 +398,10 @@ void DirectRenderer::SetScissorStateForQuad(
     gfx::Rect quad_scissor_rect = render_pass_scissor;
     if (quad.shared_quad_state->is_clipped)
       quad_scissor_rect.Intersect(quad.shared_quad_state->clip_rect);
-    SetScissorTestRectInDrawSpace(frame, quad_scissor_rect);
+    SetScissorTestRectInDrawSpace(quad_scissor_rect);
     return;
   } else if (quad.shared_quad_state->is_clipped) {
-    SetScissorTestRectInDrawSpace(frame, quad.shared_quad_state->clip_rect);
+    SetScissorTestRectInDrawSpace(quad.shared_quad_state->clip_rect);
     return;
   }
 
@@ -412,31 +409,28 @@ void DirectRenderer::SetScissorStateForQuad(
 }
 
 void DirectRenderer::SetScissorTestRectInDrawSpace(
-    const DrawingFrame* frame,
     const gfx::Rect& draw_space_rect) {
-  gfx::Rect window_space_rect =
-      MoveFromDrawToWindowSpace(frame, draw_space_rect);
+  gfx::Rect window_space_rect = MoveFromDrawToWindowSpace(draw_space_rect);
   SetScissorTestRect(window_space_rect);
 }
 
 void DirectRenderer::DoDrawPolygon(const DrawPolygon& poly,
-                                   DrawingFrame* frame,
                                    const gfx::Rect& render_pass_scissor,
                                    bool use_render_pass_scissor) {
-  SetScissorStateForQuad(frame, *poly.original_ref(), render_pass_scissor,
+  SetScissorStateForQuad(*poly.original_ref(), render_pass_scissor,
                          use_render_pass_scissor);
 
   // If the poly has not been split, then it is just a normal DrawQuad,
   // and we should save any extra processing that would have to be done.
   if (!poly.is_split()) {
-    DoDrawQuad(frame, poly.original_ref(), NULL);
+    DoDrawQuad(poly.original_ref(), NULL);
     return;
   }
 
   std::vector<gfx::QuadF> quads;
   poly.ToQuads2D(&quads);
   for (size_t i = 0; i < quads.size(); ++i) {
-    DoDrawQuad(frame, poly.original_ref(), &quads[i]);
+    DoDrawQuad(poly.original_ref(), &quads[i]);
   }
 }
 
@@ -464,7 +458,6 @@ const FilterOperations* DirectRenderer::BackgroundFiltersForPass(
 
 void DirectRenderer::FlushPolygons(
     std::deque<std::unique_ptr<DrawPolygon>>* poly_list,
-    DrawingFrame* frame,
     const gfx::Rect& render_pass_scissor,
     bool use_render_pass_scissor) {
   if (poly_list->empty()) {
@@ -472,57 +465,54 @@ void DirectRenderer::FlushPolygons(
   }
 
   BspTree bsp_tree(poly_list);
-  BspWalkActionDrawPolygon action_handler(this, frame, render_pass_scissor,
+  BspWalkActionDrawPolygon action_handler(this, render_pass_scissor,
                                           use_render_pass_scissor);
   bsp_tree.TraverseWithActionHandler(&action_handler);
   DCHECK(poly_list->empty());
 }
 
 void DirectRenderer::DrawRenderPassAndExecuteCopyRequests(
-    DrawingFrame* frame,
     RenderPass* render_pass) {
   if (render_pass_bypass_quads_.find(render_pass->id) !=
       render_pass_bypass_quads_.end()) {
     return;
   }
 
-  DrawRenderPass(frame, render_pass);
+  DrawRenderPass(render_pass);
 
   bool first_request = true;
   for (auto& copy_request : render_pass->copy_requests) {
     // Doing a readback is destructive of our state on Mac, so make sure
     // we restore the state between readbacks. http://crbug.com/99393.
     if (!first_request)
-      UseRenderPass(frame, render_pass);
-    CopyCurrentRenderPassToBitmap(frame, std::move(copy_request));
+      UseRenderPass(render_pass);
+    CopyCurrentRenderPassToBitmap(std::move(copy_request));
     first_request = false;
   }
 }
 
-void DirectRenderer::DrawRenderPass(DrawingFrame* frame,
-                                    const RenderPass* render_pass) {
+void DirectRenderer::DrawRenderPass(const RenderPass* render_pass) {
   TRACE_EVENT0("cc", "DirectRenderer::DrawRenderPass");
-  if (!UseRenderPass(frame, render_pass))
+  if (!UseRenderPass(render_pass))
     return;
 
-  const gfx::Rect surface_rect_in_draw_space =
-      OutputSurfaceRectInDrawSpace(frame);
+  const gfx::Rect surface_rect_in_draw_space = OutputSurfaceRectInDrawSpace();
   gfx::Rect render_pass_scissor_in_draw_space = surface_rect_in_draw_space;
 
-  if (frame->current_render_pass == frame->root_render_pass) {
+  if (current_frame_->current_render_pass == current_frame_->root_render_pass) {
     render_pass_scissor_in_draw_space.Intersect(
-        DeviceViewportRectInDrawSpace(frame));
+        DeviceViewportRectInDrawSpace());
   }
 
   if (use_partial_swap_) {
     render_pass_scissor_in_draw_space.Intersect(
-        ComputeScissorRectForRenderPass(frame));
+        current_frame_->ComputeScissorRectForRenderPass());
   }
 
   bool render_pass_is_clipped =
       !render_pass_scissor_in_draw_space.Contains(surface_rect_in_draw_space);
   bool is_root_render_pass =
-      frame->current_render_pass == frame->root_render_pass;
+      current_frame_->current_render_pass == current_frame_->root_render_pass;
   bool has_external_stencil_test =
       is_root_render_pass && output_surface_->HasExternalStencilTest();
   bool should_clear_surface =
@@ -532,7 +522,7 @@ void DirectRenderer::DrawRenderPass(DrawingFrame* frame,
   // If |has_external_stencil_test| we can't discard or clear. Make sure we
   // don't need to.
   DCHECK(!has_external_stencil_test ||
-         !frame->current_render_pass->has_transparent_background);
+         !current_frame_->current_render_pass->has_transparent_background);
 
   SurfaceInitializationMode mode;
   if (should_clear_surface && render_pass_is_clipped) {
@@ -544,8 +534,7 @@ void DirectRenderer::DrawRenderPass(DrawingFrame* frame,
   }
 
   PrepareSurfaceForPass(
-      frame, mode,
-      MoveFromDrawToWindowSpace(frame, render_pass_scissor_in_draw_space));
+      mode, MoveFromDrawToWindowSpace(render_pass_scissor_in_draw_space));
 
   const QuadList& quad_list = render_pass->quad_list;
   std::deque<std::unique_ptr<DrawPolygon>> poly_list;
@@ -563,7 +552,7 @@ void DirectRenderer::DrawRenderPass(DrawingFrame* frame,
 
     if (last_sorting_context_id != quad.shared_quad_state->sorting_context_id) {
       last_sorting_context_id = quad.shared_quad_state->sorting_context_id;
-      FlushPolygons(&poly_list, frame, render_pass_scissor_in_draw_space,
+      FlushPolygons(&poly_list, render_pass_scissor_in_draw_space,
                     render_pass_is_clipped);
     }
 
@@ -580,25 +569,24 @@ void DirectRenderer::DrawRenderPass(DrawingFrame* frame,
     }
 
     // We are not in a 3d sorting context, so we should draw the quad normally.
-    SetScissorStateForQuad(frame, quad, render_pass_scissor_in_draw_space,
+    SetScissorStateForQuad(quad, render_pass_scissor_in_draw_space,
                            render_pass_is_clipped);
 
-    DoDrawQuad(frame, &quad, nullptr);
+    DoDrawQuad(&quad, nullptr);
   }
-  FlushPolygons(&poly_list, frame, render_pass_scissor_in_draw_space,
+  FlushPolygons(&poly_list, render_pass_scissor_in_draw_space,
                 render_pass_is_clipped);
   FinishDrawingQuadList();
 }
 
-bool DirectRenderer::UseRenderPass(DrawingFrame* frame,
-                                   const RenderPass* render_pass) {
-  frame->current_render_pass = render_pass;
-  frame->current_texture = NULL;
-  if (render_pass == frame->root_render_pass) {
-    BindFramebufferToOutputSurface(frame);
-    InitializeViewport(frame, render_pass->output_rect,
-                       gfx::Rect(frame->device_viewport_size),
-                       frame->device_viewport_size);
+bool DirectRenderer::UseRenderPass(const RenderPass* render_pass) {
+  current_frame_->current_render_pass = render_pass;
+  current_frame_->current_texture = NULL;
+  if (render_pass == current_frame_->root_render_pass) {
+    BindFramebufferToOutputSurface();
+    InitializeViewport(current_frame_, render_pass->output_rect,
+                       gfx::Rect(current_frame_->device_viewport_size),
+                       current_frame_->device_viewport_size);
     return true;
   }
 
@@ -611,12 +599,12 @@ bool DirectRenderer::UseRenderPass(DrawingFrame* frame,
   if (!texture->id()) {
     texture->Allocate(size,
                       ResourceProvider::TEXTURE_HINT_IMMUTABLE_FRAMEBUFFER,
-                      BackbufferFormat(), frame->device_color_space);
+                      BackbufferFormat(), current_frame_->device_color_space);
   }
   DCHECK(texture->id());
 
-  if (BindFramebufferToTexture(frame, texture)) {
-    InitializeViewport(frame, render_pass->output_rect,
+  if (BindFramebufferToTexture(texture)) {
+    InitializeViewport(current_frame_, render_pass->output_rect,
                        gfx::Rect(render_pass->output_rect.size()),
                        texture->size());
     return true;