Avoid using SkClipOp::kReplace_deprecated in software renderer

cc/output/software_renderer.cc

 // Copyright 2012 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/software_renderer.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/base/math_util.h"
 #include "cc/base/render_surface_filters.h"
@@ skipping 33 matching lines @@
 bool IsScaleAndIntegerTranslate(const SkMatrix& matrix) {
   return IsScalarNearlyInteger(matrix[SkMatrix::kMTransX]) &&
          IsScalarNearlyInteger(matrix[SkMatrix::kMTransY]) &&
          SkScalarNearlyZero(matrix[SkMatrix::kMSkewX]) &&
          SkScalarNearlyZero(matrix[SkMatrix::kMSkewY]) &&
          SkScalarNearlyZero(matrix[SkMatrix::kMPersp0]) &&
          SkScalarNearlyZero(matrix[SkMatrix::kMPersp1]) &&
          SkScalarNearlyZero(matrix[SkMatrix::kMPersp2] - 1.0f);
 }
 
+class ScopedSaveCanvas {

[enne (OOO), 2017/05/09 01:02:11]

I think this is SkAutoCanvasRestore.  You can even pass a doSave to it if you
don't want to make it optional.

[ericrk, 2017/05/09 15:53:18]

Yay! I couldn't find this earlier.

+ public:
+  explicit ScopedSaveCanvas(SkCanvas* canvas)
+      : canvas_(canvas), save_count_(canvas->save()) {}
+  ~ScopedSaveCanvas() {
+    canvas_->restore();
+    DCHECK_EQ(save_count_, canvas_->getSaveCount());
+  }
+
+ private:
+  SkCanvas* canvas_;
+  int save_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedSaveCanvas);
+};
+
 }  // anonymous namespace
 
 SoftwareRenderer::SoftwareRenderer(const RendererSettings* settings,
                                    OutputSurface* output_surface,
                                    ResourceProvider* resource_provider)
     : DirectRenderer(settings, output_surface, resource_provider),
       output_device_(output_surface->software_device()) {
 }
 
 SoftwareRenderer::~SoftwareRenderer() {}
@@ skipping 28 matching lines @@
   output_frame.latency_info = std::move(latency_info);
   output_surface_->SwapBuffers(std::move(output_frame));
 }
 
 bool SoftwareRenderer::FlippedFramebuffer() const {
   return false;
 }
 
 void SoftwareRenderer::EnsureScissorTestEnabled() {
   is_scissor_enabled_ = true;
-  SetClipRect(scissor_rect_);
 }
 
 void SoftwareRenderer::EnsureScissorTestDisabled() {
-  // There is no explicit notion of enabling/disabling scissoring in software
-  // rendering, but the underlying effect we want is to clear any existing
-  // clipRect on the current SkCanvas. This is done by setting clipRect to
-  // the viewport's dimensions.
-  if (!current_canvas_)
-    return;
   is_scissor_enabled_ = false;
-  SkISize size = current_canvas_->getBaseLayerSize();
-  SetClipRect(gfx::Rect(size.width(), size.height()));
 }
 
 void SoftwareRenderer::BindFramebufferToOutputSurface() {
   DCHECK(!output_surface_->HasExternalStencilTest());
   current_framebuffer_lock_ = nullptr;
   current_framebuffer_canvas_.reset();
   current_canvas_ = root_canvas_;
 }
 
 bool SoftwareRenderer::BindFramebufferToTexture(
     const ScopedResource* texture) {
   DCHECK(texture->id());
 
   // Explicitly release lock, otherwise we can crash when try to lock
   // same texture again.
   current_framebuffer_lock_ = nullptr;
   current_framebuffer_lock_ =
       base::MakeUnique<ResourceProvider::ScopedWriteLockSoftware>(
           resource_provider_, texture->id());
   current_framebuffer_canvas_ =
       base::MakeUnique<SkCanvas>(current_framebuffer_lock_->sk_bitmap());
   current_canvas_ = current_framebuffer_canvas_.get();
   return true;
 }
 
 void SoftwareRenderer::SetScissorTestRect(const gfx::Rect& scissor_rect) {
   is_scissor_enabled_ = true;
   scissor_rect_ = scissor_rect;
-  SetClipRect(scissor_rect);
 }
 
 void SoftwareRenderer::SetClipRect(const gfx::Rect& rect) {
   if (!current_canvas_)
     return;
   // Skia applies the current matrix to clip rects so we reset it temporary.
   SkMatrix current_matrix = current_canvas_->getTotalMatrix();
   current_canvas_->resetMatrix();

[f(malita), 2017/05/09 13:24:03]

  // SetClipRect is assumed to be applied temporarily, on an otherwise-unclipped
canvas.
  DCHECK_EQ(current_canvas_->getDeviceClipBounds().width(),
current_canvas_->imageInfo().width());
  DCHECK_EQ(current_canvas_->getDeviceClipBounds().height(),
current_canvas_->imageInfo().height());

?

[ericrk, 2017/05/09 15:53:18]

Good suggestion. done.

-  // TODO(fmalita) stop using kReplace (see crbug.com/673851)
-  current_canvas_->clipRect(gfx::RectToSkRect(rect),
-                            SkClipOp::kReplace_deprecated);
+  current_canvas_->clipRect(gfx::RectToSkRect(rect), SkClipOp::kIntersect);

[f(malita), 2017/05/09 13:24:03]

nit: you can omit the clipOp - it defaults to intersect.

[ericrk, 2017/05/09 15:53:18]

Done.

   current_canvas_->setMatrix(current_matrix);
 }
 
 void SoftwareRenderer::ClearCanvas(SkColor color) {
   if (!current_canvas_)
     return;
   current_canvas_->clear(color);
 }
 
 void SoftwareRenderer::ClearFramebuffer() {
@@ skipping 36 matching lines @@
   }
 
   LOG(FATAL) << "Invalid resource type.";
   return false;
 }
 
 void SoftwareRenderer::DoDrawQuad(const DrawQuad* quad,
                                   const gfx::QuadF* draw_region) {
   if (!current_canvas_)
     return;
-  if (draw_region) {
-    current_canvas_->save();
+
+  TRACE_EVENT0("cc", "SoftwareRenderer::DoDrawQuad");
+  base::Optional<ScopedSaveCanvas> scoped_save_canvas_;

[reed1, 2017/05/09 11:50:46]

Can this section just be:

SkAutoCanvasRestore acr(current_canvas_, draw_region);
SetClipRect(scissor_rect_);

?

[ericrk, 2017/05/09 15:53:18]

Refactored - I think we still only want to save/clip if we have a scissor rect
(not always).

+  if (draw_region || is_scissor_enabled_) {
+    scoped_save_canvas_.emplace(current_canvas_);
+  }
+  if (is_scissor_enabled_) {
+    SetClipRect(scissor_rect_);
   }
 
-  TRACE_EVENT0("cc", "SoftwareRenderer::DoDrawQuad");
   gfx::Transform quad_rect_matrix;
   QuadRectTransform(&quad_rect_matrix,
                     quad->shared_quad_state->quad_to_target_transform,
                     gfx::RectF(quad->rect));
   gfx::Transform contents_device_transform =
       current_frame()->window_matrix * current_frame()->projection_matrix *
       quad_rect_matrix;
   contents_device_transform.FlattenTo2d();
   SkMatrix sk_device_matrix;
   gfx::TransformToFlattenedSkMatrix(contents_device_transform,
@@ skipping 65 matching lines @@
       break;
     case DrawQuad::INVALID:
     case DrawQuad::YUV_VIDEO_CONTENT:
     case DrawQuad::STREAM_VIDEO_CONTENT:
       DrawUnsupportedQuad(quad);
       NOTREACHED();
       break;
   }
 
   current_canvas_->resetMatrix();
-  if (draw_region) {
-    current_canvas_->restore();
-  }
 }
 
 void SoftwareRenderer::DrawDebugBorderQuad(const DebugBorderDrawQuad* quad) {
   // We need to apply the matrix manually to have pixel-sized stroke width.
   SkPoint vertices[4];
   gfx::RectFToSkRect(QuadVertexRect()).toQuad(vertices);
   SkPoint transformed_vertices[4];
   current_canvas_->getTotalMatrix().mapPoints(transformed_vertices,
                                               vertices,
                                               4);
@@ skipping 413 matching lines @@
       ApplyImageFilter(filter.get(), quad, backdrop_bitmap, nullptr);
 
   if (!filter_backdrop_image)
     return nullptr;
 
   return filter_backdrop_image->makeShader(content_tile_mode, content_tile_mode,
                                            &filter_backdrop_transform);
 }
 
 }  // namespace cc