Function pointers -> templates in SkPictureFlat.
authorcommit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 2 Jan 2014 22:20:49 +0000 (22:20 +0000)
committercommit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 2 Jan 2014 22:20:49 +0000 (22:20 +0000)
These flatten/unflatten function pointers were driving me nuts when reading the
generated assembly for this code.  We don't need the flexibility of function
pointers here, so let's use templates to make it more manageable.  You'll
notice we get much better typing now on flatten/unflatten.

BUG=
R=reed@google.com

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/123213004

git-svn-id: http://skia.googlecode.com/svn/trunk@12873 2bbb7eff-a529-9590-31e7-b0007b416f81

src/core/SkPictureFlat.cpp
src/core/SkPictureFlat.h
src/core/SkPicturePlayback.cpp
src/pipe/SkGPipeWrite.cpp
tests/BitmapHeapTest.cpp
tests/FlatDataTest.cpp

index 149cf7c..e75023e 100644 (file)
@@ -90,51 +90,3 @@ SkNamedFactorySet* SkFlatController::setNamedFactorySet(SkNamedFactorySet* set)
     return set;
 }
 
-///////////////////////////////////////////////////////////////////////////////
-
-SkFlatData* SkFlatData::Create(SkFlatController* controller,
-                               const void* obj,
-                               int index,
-                               void (*flattenProc)(SkOrderedWriteBuffer&, const void*)) {
-    // a buffer of 256 bytes should be sufficient for most paints, regions,
-    // and matrices.
-    intptr_t storage[256];
-    SkOrderedWriteBuffer buffer(256, storage, sizeof(storage));
-
-    buffer.setBitmapHeap(controller->getBitmapHeap());
-    buffer.setTypefaceRecorder(controller->getTypefaceSet());
-    buffer.setNamedFactoryRecorder(controller->getNamedFactorySet());
-    buffer.setFlags(controller->getWriteBufferFlags());
-
-    flattenProc(buffer, obj);
-    uint32_t size = buffer.size();
-    SkASSERT(SkIsAlign4(size));
-
-    // Allocate enough memory to hold SkFlatData struct and the flat data itself.
-    size_t allocSize = sizeof(SkFlatData) + size;
-    SkFlatData* result = (SkFlatData*) controller->allocThrow(allocSize);
-
-    // Put the serialized contents into the data section of the new allocation.
-    buffer.writeToMemory(result->data());
-    // Stamp the index, size and checksum in the header.
-    result->stampHeader(index, size);
-    return result;
-}
-
-void SkFlatData::unflatten(void* result,
-        void (*unflattenProc)(SkOrderedReadBuffer&, void*),
-        SkBitmapHeap* bitmapHeap,
-        SkTypefacePlayback* facePlayback) const {
-
-    SkOrderedReadBuffer buffer(this->data(), fFlatSize);
-
-    if (bitmapHeap) {
-        buffer.setBitmapStorage(bitmapHeap);
-    }
-    if (facePlayback) {
-        facePlayback->setupBuffer(buffer);
-    }
-
-    unflattenProc(buffer, result);
-    SkASSERT(fFlatSize == (int32_t)buffer.offset());
-}
index 5452501..220ae11 100644 (file)
@@ -267,16 +267,49 @@ private:
 class SkFlatData {
 public:
     // Flatten obj into an SkFlatData with this index.  controller owns the SkFlatData*.
-    static SkFlatData* Create(SkFlatController* controller,
-                              const void* obj,
-                              int index,
-                              void (*flattenProc)(SkOrderedWriteBuffer&, const void*));
-
-    // Unflatten this into result, using bitmapHeap and facePlayback for bitmaps and fonts if given.
-    void unflatten(void* result,
-                   void (*unflattenProc)(SkOrderedReadBuffer&, void*),
+    template <typename Traits, typename T>
+    static SkFlatData* Create(SkFlatController* controller, const T& obj, int index) {
+        // A buffer of 256 bytes should fit most paints, regions, and matrices.
+        uint32_t storage[64];
+        SkOrderedWriteBuffer buffer(256, storage, sizeof(storage));
+
+        buffer.setBitmapHeap(controller->getBitmapHeap());
+        buffer.setTypefaceRecorder(controller->getTypefaceSet());
+        buffer.setNamedFactoryRecorder(controller->getNamedFactorySet());
+        buffer.setFlags(controller->getWriteBufferFlags());
+
+        Traits::flatten(buffer, obj);
+        uint32_t size = buffer.size();
+        SkASSERT(SkIsAlign4(size));
+
+        // Allocate enough memory to hold SkFlatData struct and the flat data itself.
+        size_t allocSize = sizeof(SkFlatData) + size;
+        SkFlatData* result = (SkFlatData*) controller->allocThrow(allocSize);
+
+        // Put the serialized contents into the data section of the new allocation.
+        buffer.writeToMemory(result->data());
+        // Stamp the index, size and checksum in the header.
+        result->stampHeader(index, size);
+        return result;
+    }
+
+    // Unflatten this into result, using bitmapHeap and facePlayback for bitmaps and fonts if given
+    template <typename Traits, typename T>
+    void unflatten(T* result,
                    SkBitmapHeap* bitmapHeap = NULL,
-                   SkTypefacePlayback* facePlayback = NULL) const;
+                   SkTypefacePlayback* facePlayback = NULL) const {
+        SkOrderedReadBuffer buffer(this->data(), fFlatSize);
+
+        if (bitmapHeap) {
+            buffer.setBitmapStorage(bitmapHeap);
+        }
+        if (facePlayback) {
+            facePlayback->setupBuffer(buffer);
+        }
+
+        Traits::unflatten(buffer, result);
+        SkASSERT(fFlatSize == (int32_t)buffer.offset());
+    }
 
     // Do these contain the same data?  Ignores index() and topBot().
     bool operator==(const SkFlatData& that) const {
@@ -333,19 +366,17 @@ private:
     mutable SkScalar fTopBot[2];  // Cache of FontMetrics fTop, fBottom.  Starts as [NaN,?].
     // uint32_t flattenedData[] implicitly hangs off the end.
 
-    template <class T> friend class SkFlatDictionary;
+    template <typename T, typename Traits, int kScratchSizeGuess> friend class SkFlatDictionary;
 };
 
-template <class T>
+template <typename T, typename Traits, int kScratchSizeGuess=0>
 class SkFlatDictionary {
     static const size_t kWriteBufferGrowthBytes = 1024;
 
 public:
-    SkFlatDictionary(SkFlatController* controller, size_t scratchSizeGuess = 0)
-    : fFlattenProc(NULL)
-    , fUnflattenProc(NULL)
-    , fController(SkRef(controller))
-    , fScratchSize(scratchSizeGuess)
+    explicit SkFlatDictionary(SkFlatController* controller)
+    : fController(SkRef(controller))
+    , fScratchSize(kScratchSizeGuess)
     , fScratch(AllocScratch(fScratchSize))
     , fWriteBuffer(kWriteBufferGrowthBytes)
     , fWriteBufferReady(false) {
@@ -471,10 +502,6 @@ public:
         return this->findAndReturnMutableFlat(element);
     }
 
-protected:
-    void (*fFlattenProc)(SkOrderedWriteBuffer&, const void*);
-    void (*fUnflattenProc)(SkOrderedReadBuffer&, void*);
-
 private:
     // Layout: [ SkFlatData header, 20 bytes ] [ data ..., 4-byte aligned ]
     static size_t SizeWithPadding(size_t flatDataSize) {
@@ -523,7 +550,7 @@ private:
 
         // Flatten element into fWriteBuffer (using fScratch as storage).
         fWriteBuffer.reset(fScratch->data(), fScratchSize);
-        fFlattenProc(fWriteBuffer, &element);
+        Traits::flatten(fWriteBuffer, element);
         const size_t bytesWritten = fWriteBuffer.bytesWritten();
 
         // If all the flattened bytes fit into fScratch, we can skip a call to writeToMemory.
@@ -561,10 +588,9 @@ private:
     }
 
     void unflatten(T* dst, const SkFlatData* element) const {
-        element->unflatten(dst,
-                           fUnflattenProc,
-                           fController->getBitmapHeap(),
-                           fController->getTypefacePlayback());
+        element->unflatten<Traits>(dst,
+                                   fController->getBitmapHeap(),
+                                   fController->getTypefacePlayback());
     }
 
     // All SkFlatData* stored in fIndexedData and fHash are owned by the controller.
@@ -589,15 +615,37 @@ private:
 // Some common dictionaries are defined here for both reference and convenience
 ///////////////////////////////////////////////////////////////////////////////
 
-template <class T>
-static void SkFlattenObjectProc(SkOrderedWriteBuffer& buffer, const void* obj) {
-    ((T*)obj)->flatten(buffer);
-}
+struct SkMatrixTraits {
+    static void flatten(SkOrderedWriteBuffer& buffer, const SkMatrix& matrix) {
+        buffer.getWriter32()->writeMatrix(matrix);
+    }
+    static void unflatten(SkOrderedReadBuffer& buffer, SkMatrix* matrix) {
+        buffer.getReader32()->readMatrix(matrix);
+    }
+};
+typedef SkFlatDictionary<SkMatrix, SkMatrixTraits, 36> SkMatrixDictionary;
+
+
+struct SkRegionTraits {
+    static void flatten(SkOrderedWriteBuffer& buffer, const SkRegion& region) {
+        buffer.getWriter32()->writeRegion(region);
+    }
+    static void unflatten(SkOrderedReadBuffer& buffer, SkRegion* region) {
+        buffer.getReader32()->readRegion(region);
+    }
+};
+typedef SkFlatDictionary<SkRegion, SkRegionTraits> SkRegionDictionary;
 
-template <class T>
-static void SkUnflattenObjectProc(SkOrderedReadBuffer& buffer, void* obj) {
-    ((T*)obj)->unflatten(buffer);
-}
+
+struct SkPaintTraits {
+    static void flatten(SkOrderedWriteBuffer& buffer, const SkPaint& paint) {
+        paint.flatten(buffer);
+    }
+    static void unflatten(SkOrderedReadBuffer& buffer, SkPaint* paint) {
+        paint->unflatten(buffer);
+    }
+};
+typedef SkFlatDictionary<SkPaint, SkPaintTraits, 512> SkPaintDictionary;
 
 class SkChunkFlatController : public SkFlatController {
 public:
@@ -635,49 +683,4 @@ private:
     mutable SkTypefacePlayback fTypefacePlayback;
 };
 
-class SkMatrixDictionary : public SkFlatDictionary<SkMatrix> {
- public:
-    // All matrices fit in 36 bytes.
-    SkMatrixDictionary(SkFlatController* controller)
-    : SkFlatDictionary<SkMatrix>(controller, 36) {
-        fFlattenProc = &flattenMatrix;
-        fUnflattenProc = &unflattenMatrix;
-    }
-
-    static void flattenMatrix(SkOrderedWriteBuffer& buffer, const void* obj) {
-        buffer.getWriter32()->writeMatrix(*(SkMatrix*)obj);
-    }
-
-    static void unflattenMatrix(SkOrderedReadBuffer& buffer, void* obj) {
-        buffer.getReader32()->readMatrix((SkMatrix*)obj);
-    }
-};
-
-class SkPaintDictionary : public SkFlatDictionary<SkPaint> {
- public:
-    // The largest paint across ~60 .skps was 500 bytes.
-    SkPaintDictionary(SkFlatController* controller)
-    : SkFlatDictionary<SkPaint>(controller, 512) {
-        fFlattenProc = &SkFlattenObjectProc<SkPaint>;
-        fUnflattenProc = &SkUnflattenObjectProc<SkPaint>;
-    }
-};
-
-class SkRegionDictionary : public SkFlatDictionary<SkRegion> {
- public:
-    SkRegionDictionary(SkFlatController* controller)
-    : SkFlatDictionary<SkRegion>(controller) {
-        fFlattenProc = &flattenRegion;
-        fUnflattenProc = &unflattenRegion;
-    }
-
-    static void flattenRegion(SkOrderedWriteBuffer& buffer, const void* obj) {
-        buffer.getWriter32()->writeRegion(*(SkRegion*)obj);
-    }
-
-    static void unflattenRegion(SkOrderedReadBuffer& buffer, void* obj) {
-        buffer.getReader32()->readRegion((SkRegion*)obj);
-    }
-};
-
 #endif
index 195afc6..97e5667 100644 (file)
@@ -212,9 +212,10 @@ SkPicturePlayback::SkPicturePlayback(const SkPicturePlayback& src, SkPictCopyInf
             SkDEBUGCODE(int heapSize = SafeCount(fBitmapHeap.get());)
             for (int i = 0; i < paintCount; i++) {
                 if (needs_deep_copy(src.fPaints->at(i))) {
-                    deepCopyInfo->paintData[i] = SkFlatData::Create(&deepCopyInfo->controller,
-                                                                    &src.fPaints->at(i), 0,
-                                                                    &SkFlattenObjectProc<SkPaint>);
+                    deepCopyInfo->paintData[i] =
+                        SkFlatData::Create<SkPaintTraits>(&deepCopyInfo->controller,
+                                                          src.fPaints->at(i), 0);
+
                 } else {
                     // this is our sentinel, which we use in the unflatten loop
                     deepCopyInfo->paintData[i] = NULL;
@@ -233,9 +234,8 @@ SkPicturePlayback::SkPicturePlayback(const SkPicturePlayback& src, SkPictCopyInf
         SkTypefacePlayback* tfPlayback = deepCopyInfo->controller.getTypefacePlayback();
         for (int i = 0; i < paintCount; i++) {
             if (deepCopyInfo->paintData[i]) {
-                deepCopyInfo->paintData[i]->unflatten(&fPaints->writableAt(i),
-                                                      &SkUnflattenObjectProc<SkPaint>,
-                                                      bmHeap, tfPlayback);
+                deepCopyInfo->paintData[i]->unflatten<SkPaintTraits>(&fPaints->writableAt(i),
+                                                                     bmHeap, tfPlayback);
             } else {
                 // needs_deep_copy was false, so just need to assign
                 fPaints->writableAt(i) = src.fPaints->at(i);
index 0131ebb..68acc17 100644 (file)
@@ -150,20 +150,13 @@ const SkFlatData* FlattenableHeap::flatToReplace() const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-class FlatDictionary : public SkFlatDictionary<SkFlattenable> {
-public:
-    FlatDictionary(FlattenableHeap* heap)
-            : SkFlatDictionary<SkFlattenable>(heap) {
-        fFlattenProc = &flattenFlattenableProc;
-        // No need to define fUnflattenProc since the writer will never
-        // unflatten the data.
-    }
-    static void flattenFlattenableProc(SkOrderedWriteBuffer& buffer,
-                                       const void* obj) {
-        buffer.writeFlattenable((SkFlattenable*)obj);
+struct SkFlattenableTraits {
+    static void flatten(SkOrderedWriteBuffer& buffer, const SkFlattenable& flattenable) {
+        buffer.writeFlattenable(&flattenable);
     }
-
+    // No need to define unflatten if we never call it.
 };
+typedef SkFlatDictionary<SkFlattenable, SkFlattenableTraits> FlatDictionary;
 
 ///////////////////////////////////////////////////////////////////////////////
 
index 2b5cf83..bcfec22 100644 (file)
 #include "Test.h"
 #include "TestClassDef.h"
 
-class FlatDictionary : public SkFlatDictionary<SkShader> {
-
-public:
-    FlatDictionary(SkFlatController* controller)
-    : SkFlatDictionary<SkShader>(controller) {
-        fFlattenProc = &flattenFlattenableProc;
-        // No need for an unflattenProc
-    }
-    static void flattenFlattenableProc(SkOrderedWriteBuffer& buffer, const void* obj) {
-        buffer.writeFlattenable((SkFlattenable*)obj);
+struct SkShaderTraits {
+    static void flatten(SkOrderedWriteBuffer& buffer, const SkShader& shader) {
+        buffer.writeFlattenable(&shader);
     }
 };
+typedef SkFlatDictionary<SkShader, SkShaderTraits> FlatDictionary;
 
 class SkBitmapHeapTester {
 
index b6133ac..95a2bd0 100644 (file)
 #include "SkShader.h"
 #include "SkXfermode.h"
 
-static void flattenFlattenableProc(SkOrderedWriteBuffer& buffer,
-                                   const void* obj) {
-    buffer.writeFlattenable((SkFlattenable*)obj);
-}
+struct SkFlattenableTraits {
+    static void flatten(SkOrderedWriteBuffer& buffer, const SkFlattenable& flattenable) {
+        buffer.writeFlattenable(&flattenable);
+    }
+};
+struct SkBitmapTraits {
+    static void flatten(SkOrderedWriteBuffer& buffer, const SkBitmap& bitmap) {
+        bitmap.flatten(buffer);
+    }
+};
 
 class Controller : public SkChunkFlatController {
 public:
@@ -38,13 +44,12 @@ private:
  * @param obj Flattenable object to be flattened.
  * @param flattenProc Function that flattens objects with the same type as obj.
  */
-static void testCreate(skiatest::Reporter* reporter, const void* obj,
-                       void (*flattenProc)(SkOrderedWriteBuffer&, const void*)) {
+template <typename Traits, typename T>
+static void testCreate(skiatest::Reporter* reporter, const T& obj) {
     Controller controller;
-    // No need to delete data because that will be taken care of by the
-    // controller.
-    SkFlatData* data1 = SkFlatData::Create(&controller, obj, 0, flattenProc);
-    SkFlatData* data2 = SkFlatData::Create(&controller, obj, 1, flattenProc);
+    // No need to delete data because that will be taken care of by the controller.
+    SkFlatData* data1 = SkFlatData::Create<Traits>(&controller, obj, 0);
+    SkFlatData* data2 = SkFlatData::Create<Traits>(&controller, obj, 1);
     REPORTER_ASSERT(reporter, *data1 == *data2);
 }
 
@@ -56,10 +61,10 @@ DEF_TEST(FlatData, reporter) {
     SkColor colors[2];
     colors[0] = SK_ColorRED;
     colors[1] = SK_ColorBLUE;
-    SkShader* shader = SkGradientShader::CreateLinear(points, colors, NULL,
-                                            2, SkShader::kRepeat_TileMode);
-    SkAutoUnref aur(shader);
-    testCreate(reporter, shader, flattenFlattenableProc);
+
+    SkAutoTUnref<SkShader> shader(SkGradientShader::CreateLinear(points, colors, NULL, 2,
+                                                                 SkShader::kRepeat_TileMode));
+    testCreate<SkFlattenableTraits>(reporter, *shader);
 
     // Test SkBitmap
     {
@@ -70,17 +75,14 @@ DEF_TEST(FlatData, reporter) {
         SkPaint paint;
         paint.setShader(shader);
         canvas.drawPaint(paint);
-        testCreate(reporter, &bm, &SkFlattenObjectProc<SkBitmap>);
+        testCreate<SkBitmapTraits>(reporter, bm);
     }
 
     // Test SkColorFilter
-    SkColorFilter* cf = SkColorFilter::CreateLightingFilter(SK_ColorBLUE,
-                                                            SK_ColorRED);
-    SkAutoUnref aurcf(cf);
-    testCreate(reporter, cf, &flattenFlattenableProc);
+    SkAutoTUnref<SkColorFilter> cf(SkColorFilter::CreateLightingFilter(SK_ColorBLUE, SK_ColorRED));
+    testCreate<SkFlattenableTraits>(reporter, *cf);
 
     // Test SkXfermode
-    SkXfermode* xfer = SkXfermode::Create(SkXfermode::kDstOver_Mode);
-    SkAutoUnref aurxf(xfer);
-    testCreate(reporter, xfer, &flattenFlattenableProc);
+    SkAutoTUnref<SkXfermode> xfer(SkXfermode::Create(SkXfermode::kDstOver_Mode));
+    testCreate<SkFlattenableTraits>(reporter, *xfer);
 }