Cleanup gfx::Canvas now that 10562027 has landed

ui/gfx/canvas.cc

 // Copyright (c) 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 "ui/gfx/canvas.h"
 
 #include <limits>
 
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/transform.h"
 
 #if defined(OS_WIN)
 #include "ui/gfx/canvas_skia_paint.h"
 #endif
 
 namespace gfx {
 
-Canvas::Canvas(const gfx::Size& size, bool is_opaque)
-    : owned_canvas_(new skia::PlatformCanvas(size.width(), size.height(),
-                                             is_opaque)),
-      canvas_(owned_canvas_.get()) {
-#if defined(OS_WIN) || defined(OS_MACOSX)
-  // skia::PlatformCanvas instances are initialized to 0 by Cairo on Linux, but
-  // uninitialized on Win and Mac.
-  if (!is_opaque)
-    owned_canvas_->clear(SkColorSetARGB(0, 0, 0, 0));
-#endif
-
-  ApplyScaleFactor(ui::SCALE_FACTOR_100P, false);
-}
-
 Canvas::Canvas(const gfx::Size& size,
                ui::ScaleFactor scale_factor,
-               bool is_opaque) {
+               bool is_opaque)
+      : scale_factor_(scale_factor) {
   gfx::Size pixel_size = size.Scale(ui::GetScaleFactorScale(scale_factor));

[sky, 2012/07/16 14:26:51]

Member initialize canvas_ to NULL.

[pkotwicz, 2012/07/17 17:19:52]

canvas_ is initialized in line 30 as part of the constructor.
Do you want me to initialize it in the initializer list too?

[sky, 2012/07/17 21:58:32]

Yes.

   owned_canvas_.reset(new skia::PlatformCanvas(pixel_size.width(),
                                                pixel_size.height(),
                                                is_opaque));
   canvas_ = owned_canvas_.get();
 #if defined(OS_WIN) || defined(OS_MACOSX)
   // skia::PlatformCanvas instances are initialized to 0 by Cairo on Linux, but
   // uninitialized on Win and Mac.
   if (!is_opaque)
     owned_canvas_->clear(SkColorSetARGB(0, 0, 0, 0));
 #endif
 
-  ApplyScaleFactor(scale_factor, true);
+  scale_factor_ = scale_factor;

[sky, 2012/07/16 14:26:51]

Doesn't line 28 set the scale_factor_?

[pkotwicz, 2012/07/17 17:19:52]

Done.

+  SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor));
+  canvas_->scale(scale, scale);
 }
 
 Canvas::Canvas(const gfx::ImageSkiaRep& image_rep, bool is_opaque)
-    : owned_canvas_(new skia::PlatformCanvas(image_rep.pixel_width(),
+    : scale_factor_(image_rep.scale_factor()),
+      owned_canvas_(new skia::PlatformCanvas(image_rep.pixel_width(),
                                              image_rep.pixel_height(),
                                              is_opaque)),
       canvas_(owned_canvas_.get()) {
-  ApplyScaleFactor(image_rep.scale_factor(), true);
+  SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor_));
+  canvas_->scale(scale, scale);
   DrawImageInt(gfx::ImageSkia(image_rep), 0, 0);
 }
 
 Canvas::Canvas()
-    : owned_canvas_(new skia::PlatformCanvas()),
+    : scale_factor_(ui::SCALE_FACTOR_100P),
+      owned_canvas_(new skia::PlatformCanvas()),
       canvas_(owned_canvas_.get()) {
-  ApplyScaleFactor(ui::SCALE_FACTOR_100P, false);
 }
 
-Canvas::Canvas(SkCanvas* canvas,
-               ui::ScaleFactor scale_factor,
-               bool scale_canvas)
-    : owned_canvas_(),
+Canvas::Canvas(SkCanvas* canvas, ui::ScaleFactor scale_factor)

[sky, 2012/07/16 14:26:51]

Move to match position in header.

[pkotwicz, 2012/07/17 17:19:52]

Done.

+    : scale_factor_(scale_factor),
+      owned_canvas_(),
       canvas_(canvas) {
   DCHECK(canvas);
-  ApplyScaleFactor(scale_factor, scale_canvas);
 }
 
 Canvas::~Canvas() {
-  if (scale_factor_scales_canvas_) {
-    SkScalar scale = 1.0f / ui::GetScaleFactorScale(scale_factor_);
-    canvas_->scale(scale, scale);
-  }
+}
+
+// static
+Canvas* Canvas::CreateCanvasWithoutScaling(SkCanvas* canvas,
+                                          ui::ScaleFactor scale_factor) {

[sky, 2012/07/16 14:26:51]

nit: spacing

[pkotwicz, 2012/07/17 17:19:52]

Done.

+  return new Canvas(canvas, scale_factor);
 }
 
 void Canvas::RecreateBackingCanvas(const gfx::Size& size,
                                    ui::ScaleFactor scale_factor,
                                    bool is_opaque) {
+  scale_factor_ = scale_factor;
   gfx::Size pixel_size = size.Scale(ui::GetScaleFactorScale(scale_factor));
   owned_canvas_.reset(new skia::PlatformCanvas(pixel_size.width(),
                                                pixel_size.height(),
                                                is_opaque));
   canvas_ = owned_canvas_.get();
-
-  ApplyScaleFactor(scale_factor, true);
+  SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor_));
+  canvas_->scale(scale, scale);
 }
 
 // static
 int Canvas::GetStringWidth(const string16& text, const gfx::Font& font) {
   int width = 0, height = 0;
   Canvas::SizeStringInt(text, font, &width, &height, NO_ELLIPSIS);
   return width;
 }
 
 // static
 int Canvas::DefaultCanvasTextAlignment() {
   return base::i18n::IsRTL() ? TEXT_ALIGN_RIGHT : TEXT_ALIGN_LEFT;
 }
 
-SkBitmap Canvas::ExtractBitmap() const {
+gfx::ImageSkiaRep Canvas::ExtractImageRep() const {
   const SkBitmap& device_bitmap = canvas_->getDevice()->accessBitmap(false);
 
   // Make a bitmap to return, and a canvas to draw into it. We don't just want
   // to call extractSubset or the copy constructor, since we want an actual copy
   // of the bitmap.
   SkBitmap result;
   device_bitmap.copyTo(&result, SkBitmap::kARGB_8888_Config);
-  return result;
-}
 
-gfx::ImageSkiaRep Canvas::ExtractImageSkiaRep() const {
-  return gfx::ImageSkiaRep(ExtractBitmap(), scale_factor_);
+  return gfx::ImageSkiaRep(result, scale_factor_);
 }
 
 void Canvas::DrawDashedRect(const gfx::Rect& rect, SkColor color) {
   // Create a 2D bitmap containing alternating on/off pixels - we do this
   // so that you never get two pixels of the same color around the edges
   // of the focus rect (this may mean that opposing edges of the rect may
   // have a dot pattern out of phase to each other).
   static SkColor last_color;
   static SkBitmap* dots = NULL;
   if (!dots || last_color != color) {
@@ skipping 371 matching lines @@
   return canvas_->getClipBounds(&clip) &&
       clip.intersect(SkIntToScalar(x), SkIntToScalar(y), SkIntToScalar(x + w),
                      SkIntToScalar(y + h));
 }
 
 bool Canvas::IntersectsClipRect(const gfx::Rect& rect) {
   return IntersectsClipRectInt(rect.x(), rect.y(),
                                rect.width(), rect.height());
 }
 
-void Canvas::ApplyScaleFactor(ui::ScaleFactor scale_factor,
-                              bool scale_canvas) {
-  scale_factor_scales_canvas_ = scale_canvas;
-  scale_factor_ = scale_factor;
-  if (scale_canvas) {
-    SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor));
-    canvas_->scale(scale, scale);
-  }
-}
-
 const gfx::ImageSkiaRep& Canvas::GetImageRepToPaint(
     const gfx::ImageSkia& image) const {
   return GetImageRepToPaint(image, 1.0f, 1.0f);
 }
 
 const gfx::ImageSkiaRep& Canvas::GetImageRepToPaint(
     const gfx::ImageSkia& image,
     float user_additional_scale_x,
     float user_additional_scale_y) const {
   const gfx::ImageSkiaRep& image_rep = image.GetRepresentation(scale_factor_);
 
   if (!image_rep.is_null()) {
     SkMatrix m = canvas_->getTotalMatrix();
     float scale_x = SkScalarToFloat(SkScalarAbs(m.getScaleX())) *
         user_additional_scale_x;
     float scale_y = SkScalarToFloat(SkScalarAbs(m.getScaleY())) *
         user_additional_scale_y;
 
     float bitmap_scale = image_rep.GetScale();
     if (scale_x < bitmap_scale || scale_y < bitmap_scale)
       const_cast<SkBitmap&>(image_rep.sk_bitmap()).buildMipMap();
   }
 
   return image_rep;
 }
 
 }  // namespace gfx