viz: In SkiaRenderer remove bool tracking whether is drawing render pass

Remove SkiaRenderer's bool that tracks whether we are drawing root
render pass or child render pass. Also leads to deleting function on
SkiaOutputSurface since it knows the current render pass id and both
functions share majority of code.

Bug: 644851
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I332012ed89f5edc825acaee4230f28bdfc271044
Reviewed-on: https://chromium-review.googlesource.com/1241734
Commit-Queue: weiliangc <weiliangc@chromium.org>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594429}

components/viz/service/display/skia_output_surface.h

@@ -34,15 +34,6 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurface : public OutputSurface {
   // called.
   virtual SkCanvas* BeginPaintCurrentFrame() = 0;
 
-  // Finish painting the current frame. It should be paired with
-  // BeginPaintCurrentFrame. This method will schedule a GPU task to play the
-  // DDL back on GPU thread on the SkSurface for the framebuffer. This method
-  // returns a sync token which can be waited on in a command buffer to ensure
-  // the paint operation is completed. This token is released when the GPU ops
-  // from painting the current frame have been seen and processed by the GPU
-  // main.
-  virtual gpu::SyncToken FinishPaintCurrentFrame() = 0;
-
   // Make a promise SkImage from the given |metadata|. The SkiaRenderer can use
   // the image with SkCanvas returned by |GetSkCanvasForCurrentFrame|, but Skia
   // will not read the content of the resource until the sync token in the
@@ -75,13 +66,13 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurface : public OutputSurface {
                                          ResourceFormat format,
                                          bool mipmap) = 0;
 
-  // Finish painting a render pass. It should be paired with
-  // BeginPaintRenderPass. This method will schedule a GPU task to play the DDL
-  // back on GPU thread on a cached SkSurface. This method returns a sync token
-  // which can be waited on in a command buffer to ensure the paint operation is
-  // completed. This token is released when the GPU ops from painting the render
-  // pass have been seen and processed by the GPU main.
-  virtual gpu::SyncToken FinishPaintRenderPass() = 0;
+  // Finish painting the current frame or current render pass, depends on which
+  // BeginPaint function is called last. This method will schedule a GPU task to
+  // play the DDL back on GPU thread on a cached SkSurface. This method returns
+  // a sync token which can be waited on in a command buffer to ensure the paint
+  // operation is completed. This token is released when the GPU ops from
+  // painting the render pass have been seen and processed by the GPU main.
+  virtual gpu::SyncToken SubmitPaint() = 0;
 
   // Make a promise SkImage from a render pass id. The render pass has been
   // painted with BeginPaintRenderPass and FinishPaintRenderPass. The format

components/viz/service/display/skia_renderer.cc

@@ -324,7 +324,6 @@ void SkiaRenderer::BindFramebufferToOutputSurface() {
   // How to setup is in ResourceProvider. (http://crbug.com/644851)
   if (is_using_ddl()) {
     root_canvas_ = skia_output_surface_->BeginPaintCurrentFrame();
-    is_drawing_render_pass_ = false;
     DCHECK(root_canvas_);
   } else {
     auto* gr_context = GetGrContext();
@@ -405,7 +404,6 @@ void SkiaRenderer::BindFramebufferToTexture(const RenderPassId render_pass_id) {
     current_surface_ = non_root_surface_.get();
     current_canvas_ = non_root_surface_->getCanvas();
   }
-  is_drawing_render_pass_ = true;
 }
 
 void SkiaRenderer::SetScissorTestRect(const gfx::Rect& scissor_rect) {
@@ -934,8 +932,14 @@ void SkiaRenderer::CopyDrawnRenderPass(
   }
 
   if (is_using_ddl()) {
-    auto render_pass_id =
-        is_drawing_render_pass_ ? current_frame()->current_render_pass->id : 0;
+    // Root framebuffer uses id 0 in SkiaOutputSurface.
+    RenderPassId render_pass_id = 0;
+    // If we are in child render pass and we don't have overdraw, copy the
+    // current render pass.
+    if (root_canvas_ != current_canvas_ &&
+        current_canvas_ != nway_canvas_.get()) {
+      render_pass_id = current_frame()->current_render_pass->id;
+    }
     skia_output_surface_->CopyOutput(render_pass_id, window_copy_rect,
                                      std::move(request));
     return;
@@ -965,10 +969,7 @@ void SkiaRenderer::DidChangeVisibility() {
 
 void SkiaRenderer::FinishDrawingQuadList() {
   if (is_using_ddl()) {
-    gpu::SyncToken sync_token =
-        is_drawing_render_pass_
-            ? skia_output_surface_->FinishPaintRenderPass()
-            : skia_output_surface_->FinishPaintCurrentFrame();
+    gpu::SyncToken sync_token = skia_output_surface_->SubmitPaint();
     promise_images_.clear();
     yuv_promise_images_.clear();
     lock_set_for_external_use_.UnlockResources(sync_token);

components/viz/service/display/skia_renderer.h

@@ -125,7 +125,6 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer {
 
   gfx::Rect scissor_rect_;
 
-  bool is_drawing_render_pass_ = false;
   sk_sp<SkSurface> root_surface_;
   sk_sp<SkSurface> non_root_surface_;
   sk_sp<SkSurface> overdraw_surface_;

components/viz/service/display_embedder/skia_output_surface_impl.cc

@@ -287,30 +287,6 @@ SkCanvas* SkiaOutputSurfaceImpl::BeginPaintCurrentFrame() {
   return recorder_->getCanvas();
 }
 
-gpu::SyncToken SkiaOutputSurfaceImpl::FinishPaintCurrentFrame() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK(recorder_);
-
-  gpu::SyncToken sync_token(gpu::CommandBufferNamespace::VIZ_OUTPUT_SURFACE,
-                            impl_on_gpu_->command_buffer_id(),
-                            ++sync_fence_release_);
-  sync_token.SetVerifyFlush();
-
-  auto ddl = recorder_->detach();
-  DCHECK(ddl);
-  recorder_.reset();
-  auto sequence_id = gpu_service_->skia_output_surface_sequence_id();
-  // impl_on_gpu_ is released on the GPU thread by a posted task from
-  // SkiaOutputSurfaceImpl::dtor. So it is safe to use base::Unretained.
-  auto callback =
-      base::BindOnce(&SkiaOutputSurfaceImplOnGpu::FinishPaintCurrentFrame,
-                     base::Unretained(impl_on_gpu_.get()), std::move(ddl),
-                     std::move(yuv_resource_metadatas_), sync_fence_release_);
-  gpu_service_->scheduler()->ScheduleTask(gpu::Scheduler::Task(
-      sequence_id, std::move(callback), std::move(resource_sync_tokens_)));
-  return sync_token;
-}
-
 sk_sp<SkImage> SkiaOutputSurfaceImpl::MakePromiseSkImage(
     ResourceMetadata metadata) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
@@ -436,27 +412,39 @@ SkCanvas* SkiaOutputSurfaceImpl::BeginPaintRenderPass(
   return offscreen_surface_recorder_->getCanvas();
 }
 
-gpu::SyncToken SkiaOutputSurfaceImpl::FinishPaintRenderPass() {
+gpu::SyncToken SkiaOutputSurfaceImpl::SubmitPaint() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK(current_render_pass_id_);
-  DCHECK(offscreen_surface_recorder_);
+  // If current_render_pass_id_ is not 0, we are painting a render pass.
+  // Otherwise we are painting a frame.
+  bool painting_render_pass = current_render_pass_id_ != 0;
+  auto& current_recorder =
+      painting_render_pass ? offscreen_surface_recorder_ : recorder_;
 
+  DCHECK(current_recorder);
   gpu::SyncToken sync_token(gpu::CommandBufferNamespace::VIZ_OUTPUT_SURFACE,
                             impl_on_gpu_->command_buffer_id(),
                             ++sync_fence_release_);
   sync_token.SetVerifyFlush();
 
-  auto ddl = offscreen_surface_recorder_->detach();
-  offscreen_surface_recorder_.reset();
+  auto ddl = current_recorder->detach();
   DCHECK(ddl);
-
+  current_recorder.reset();
   auto sequence_id = gpu_service_->skia_output_surface_sequence_id();
   // impl_on_gpu_ is released on the GPU thread by a posted task from
   // SkiaOutputSurfaceImpl::dtor. So it is safe to use base::Unretained.
-  auto callback = base::BindOnce(
-      &SkiaOutputSurfaceImplOnGpu::FinishPaintRenderPass,
-      base::Unretained(impl_on_gpu_.get()), current_render_pass_id_,
-      std::move(ddl), std::move(yuv_resource_metadatas_), sync_fence_release_);
+  base::OnceCallback<void()> callback;
+  if (painting_render_pass) {
+    callback =
+        base::BindOnce(&SkiaOutputSurfaceImplOnGpu::FinishPaintRenderPass,
+                       base::Unretained(impl_on_gpu_.get()),
+                       current_render_pass_id_, std::move(ddl),
+                       std::move(yuv_resource_metadatas_), sync_fence_release_);
+  } else {
+    callback =
+        base::BindOnce(&SkiaOutputSurfaceImplOnGpu::FinishPaintCurrentFrame,
+                       base::Unretained(impl_on_gpu_.get()), std::move(ddl),
+                       std::move(yuv_resource_metadatas_), sync_fence_release_);
+  }
   gpu_service_->scheduler()->ScheduleTask(gpu::Scheduler::Task(
       sequence_id, std::move(callback), std::move(resource_sync_tokens_)));
   current_render_pass_id_ = 0;

components/viz/service/display_embedder/skia_output_surface_impl.h

@@ -74,7 +74,6 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceImpl : public SkiaOutputSurface {
 
   // SkiaOutputSurface implementation:
   SkCanvas* BeginPaintCurrentFrame() override;
-  gpu::SyncToken FinishPaintCurrentFrame() override;
   sk_sp<SkImage> MakePromiseSkImage(ResourceMetadata metadata) override;
   sk_sp<SkImage> MakePromiseSkImageFromYUV(
       std::vector<ResourceMetadata> metadatas,
@@ -84,7 +83,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceImpl : public SkiaOutputSurface {
                                  const gfx::Size& surface_size,
                                  ResourceFormat format,
                                  bool mipmap) override;
-  gpu::SyncToken FinishPaintRenderPass() override;
+  gpu::SyncToken SubmitPaint() override;
   sk_sp<SkImage> MakePromiseSkImageFromRenderPass(const RenderPassId& id,
                                                   const gfx::Size& size,
                                                   ResourceFormat format,