Avoid using SkClipOp::kReplace_deprecated in software renderer

cc/output/software_renderer.cc

Index: cc/output/software_renderer.cc
diff --git a/cc/output/software_renderer.cc b/cc/output/software_renderer.cc
index bb8010847ef60fe50b26629dd0f52dc6f1d7c20e..6aff7c3b2347121a4a77da2e52ccacb50bd02b91 100644
--- a/cc/output/software_renderer.cc
+++ b/cc/output/software_renderer.cc
@@ -51,6 +51,22 @@ bool IsScaleAndIntegerTranslate(const SkMatrix& matrix) {
          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,
@@ -99,19 +115,10 @@ bool SoftwareRenderer::FlippedFramebuffer() const {
 
 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() {
@@ -140,7 +147,6 @@ bool SoftwareRenderer::BindFramebufferToTexture(
 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) {
@@ -149,9 +155,7 @@ void SoftwareRenderer::SetClipRect(const gfx::Rect& rect) {
   // 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);
 }
 
@@ -208,11 +212,16 @@ 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_);
+  }
+
   gfx::Transform quad_rect_matrix;
   QuadRectTransform(&quad_rect_matrix,
                     quad->shared_quad_state->quad_to_target_transform,
@@ -298,9 +307,6 @@ void SoftwareRenderer::DoDrawQuad(const DrawQuad* quad,
   }
 
   current_canvas_->resetMatrix();
-  if (draw_region) {
-    current_canvas_->restore();
-  }
 }
 
 void SoftwareRenderer::DrawDebugBorderQuad(const DebugBorderDrawQuad* quad) {