Refactoring in SkImage implementations backed by a SkBitmap

src/image/SkImage_Gpu.cpp

Index: src/image/SkImage_Gpu.cpp
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 5733a89de349074de47e7de85cbcd96635417d2d..97d685528a884414a9775e279e8d6b3be36309cd 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -5,79 +5,47 @@
  * found in the LICENSE file.
  */
 
-#include "SkImage_Base.h"
-#include "SkImagePriv.h"
+#include "SkImage_BitmapBase.h"
+
 #include "SkBitmap.h"
-#include "SkCanvas.h"
-#include "GrContext.h"
+#include "SkImagePriv.h"
 #include "GrTexture.h"
 #include "SkGrPixelRef.h"
 
-class SkImage_Gpu : public SkImage_Base {
+class SkImage_Gpu : public SkImage_BitmapBase {

[bsalomon, 2014/09/18 14:41:51]

Given that we want to remove support for texture-backed SkBitmaps I find it a
bit troubling to bake the bitmap nature of into the inheritance hierarchy.

[Rémi Piotaix, 2014/09/18 18:27:15]

Hum, for now this two classes are backed by a SkBitmap, that's why i did that:
there was a lot of code duplication.
Maybe we can make this change: It will still be possible to modify SkImage_Gpu
when the support for tex-based SkBitmaps will be dropped.

[bsalomon, 2014/09/18 18:46:28]

It wasn't obvious at all from the code, but the direction we want to move is for
them to have radically different implementations so I don't think code-sharing
is a good idea. A common base class is place to accumulate more shared
implementation that has to be undone later.

 public:
     SK_DECLARE_INST_COUNT(SkImage_Gpu)
 
     explicit SkImage_Gpu(const SkBitmap&);
     virtual ~SkImage_Gpu();
 
-    virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*) const SK_OVERRIDE;
-    virtual void onDrawRectToRect(SkCanvas*, const SkRect* src, const SkRect& dst,
-                                  const SkPaint*) const SK_OVERRIDE;
     virtual GrTexture* onGetTexture() const SK_OVERRIDE;
     virtual bool getROPixels(SkBitmap*) const SK_OVERRIDE;
 
-    GrTexture* getTexture() const { return fBitmap.getTexture(); }
-
-    virtual SkShader* onNewShader(SkShader::TileMode,
-                                  SkShader::TileMode,
-                                  const SkMatrix* localMatrix) const SK_OVERRIDE;
-
-    virtual bool isOpaque() const SK_OVERRIDE;
-
 private:
-    SkBitmap    fBitmap;
+//    SkBitmap    fBitmap;

[reed1, 2014/09/18 13:07:16]

remove?

[Rémi Piotaix, 2014/09/18 18:27:15]

Done.

 
-    typedef SkImage_Base INHERITED;
+    typedef SkImage_BitmapBase INHERITED;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 SkImage_Gpu::SkImage_Gpu(const SkBitmap& bitmap)
-    : INHERITED(bitmap.width(), bitmap.height())
-    , fBitmap(bitmap) {
+    : INHERITED(bitmap) {
     SkASSERT(fBitmap.getTexture());
 }
 
 SkImage_Gpu::~SkImage_Gpu() {
 }
 
-SkShader* SkImage_Gpu::onNewShader(SkShader::TileMode tileX,
-                                   SkShader::TileMode tileY,
-                                   const SkMatrix* localMatrix) const
-{
-    return SkShader::CreateBitmapShader(fBitmap, tileX, tileY, localMatrix);
-}
-
-void SkImage_Gpu::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) const {
-    canvas->drawBitmap(fBitmap, x, y, paint);
-}
-
-void SkImage_Gpu::onDrawRectToRect(SkCanvas* canvas, const SkRect* src, const SkRect& dst,
-                                   const SkPaint* paint) const {
-    canvas->drawBitmapRectToRect(fBitmap, src, dst, paint);
-}
-
 GrTexture* SkImage_Gpu::onGetTexture() const {
-    return fBitmap.getTexture();
+    return this->getBitmap().getTexture();
 }
 
 bool SkImage_Gpu::getROPixels(SkBitmap* dst) const {
-    return fBitmap.copyTo(dst, kN32_SkColorType);
+    return this->getBitmap().copyTo(dst, kN32_SkColorType);
 }
 
-bool SkImage_Gpu::isOpaque() const {
-    return fBitmap.isOpaque();
-}
 
 ///////////////////////////////////////////////////////////////////////////////
 
@@ -90,5 +58,5 @@ SkImage* SkImage::NewTexture(const SkBitmap& bitmap) {
 }
 
 GrTexture* SkTextureImageGetTexture(SkImage* image) {

[reed1, 2014/09/18 13:07:16]

Why do we have this helper?

[Rémi Piotaix, 2014/09/18 18:27:15]

Uh... dunno.
It is used in SkSurface_Gpu when making copy of the Texture (copy-on-write).
Maybe it would have been cleaner to make SkSurface_Gpu friend of SkImage_Gpu...

-    return ((SkImage_Gpu*)image)->getTexture();
+    return image->getTexture();
 }