Introduce PlatformBitmap, which is a minimal helper class that wraps an SkBitmap

skia/ext/platform_canvas.h

 // Copyright (c) 2011 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.
 
 #ifndef SKIA_EXT_PLATFORM_CANVAS_H_
 #define SKIA_EXT_PLATFORM_CANVAS_H_
 
 // The platform-specific device will include the necessary platform headers
 // to get the surface type.
 #include "skia/ext/platform_device.h"
@@ skipping 133 matching lines @@
   PlatformSurface GetPlatformSurface() { return platform_surface_; }
  private:
   SkCanvas* canvas_;
   PlatformSurface platform_surface_;
 
   // Disallow copy and assign
   ScopedPlatformPaint(const ScopedPlatformPaint&);
   ScopedPlatformPaint& operator=(const ScopedPlatformPaint&);
 };
 
+class SK_API PlatformBitmap {
+public:

[brettw, 2012/10/09 20:40:12]

nit: indenting for these two classes (2 space indents, public/private should be
indented one space).

[reed1, 2012/10/09 21:06:43]

Done.

+  PlatformBitmap() : surface_(0) {}

[brettw, 2012/10/09 20:40:12]

Can you put this in the .cc file? (Background: Chrome style requires this for
all nontrivial classes since the compiler ends up putting one in every .o file
that uses it, then the linker uniquifies it, and it slows things down. Since
this class has a SkBitmap and a PlatformSurface nested class, this would be
counted as a nontrivial constructor/destructor.)

[reed1, 2012/10/09 21:06:43]

Done.

+  ~PlatformBitmap();
+
+  bool Allocate(int width, int height, bool isOpaque);

[brettw, 2012/10/09 20:40:12]

style: is_opaque.

+
+  PlatformSurface LockSurface();

[brettw, 2012/10/09 20:40:12]

Can these have documentation? Be sure to mention what happens if you don't call
Allocate().

[reed1, 2012/10/09 21:06:43]

Done.

+  void UnlockSurface();
+
+  const SkBitmap& GetBitmap() { return bitmap_; }
+
+private:
+    SkBitmap        bitmap_;

[brettw, 2012/10/09 20:40:12]

Normally we don't try to horizontally align these things, so just one space is
fine.

[reed1, 2012/10/09 21:06:43]

Done.

+    PlatformSurface surface_;
+};

[brettw, 2012/10/09 20:40:12]

Should these classes be DISALLOW_COPY_AND_ASSIGN?

[reed1, 2012/10/09 21:06:43]

That is waht SkNoncopyable does. I have updated PlatformBitmap to also inherit
from it. (SkCanvas also inherits from SkNoncopyable).

[brettw, 2012/10/09 21:08:23]

If you're doing Chrome code I'd prefer the Chrome way of specifying this.

+
+class SK_API ScopedPlatformBitmap : SkNoncopyable {
+public:
+    explicit ScopedPlatformBitmap(PlatformBitmap* bitmap) : bitmap_(bitmap) {

[brettw, 2012/10/09 20:40:12]

Can you also put this constructor & destructor in the .cc?

+        surface_ = bitmap->LockSurface();
+    }
+    ~ScopedPlatformBitmap() { bitmap_->UnlockSurface(); }
+    
+    // Returns the PlatformSurface to use for native platform drawing calls.
+    PlatformSurface GetPlatformSurface() { return surface_; }
+private:
+    PlatformBitmap* bitmap_;
+    PlatformSurface surface_;
+};

[brettw, 2012/10/09 20:40:12]

Probably also DISALLOW_COPY_AND_ASSIGN here.

+    
 }  // namespace skia
 
 #endif  // SKIA_EXT_PLATFORM_CANVAS_H_