Pipe factory names independently from the flattenables using them.

Avoids an issue where a flattenable written twice might be written
differently (the first time the flat data may have a name, whereas
the second time it will have an index).

Also add a test which confirms that identical flattenables will have
the same SkFlatData representation.

BUG=https://code.google.com/p/skia/issues/detail?id=721
TEST=FlatDataTest.cpp

Review URL: https://codereview.appspot.com/6431057

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

5 files changed, 63 insertions, 53 deletions

diff --git a/src/core/SkFlattenable.cpp b/src/core/SkFlattenable.cpp
index edfc5aec02..60c04b14c7 100644
--- a/src/core/SkFlattenable.cpp
+++ b/src/core/SkFlattenable.cpp
@@ -46,17 +46,43 @@ SkFlattenableReadBuffer::SkFlattenableReadBuffer() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+SkNamedFactorySet::SkNamedFactorySet() : fNextAddedFactory(0) {}
+
+uint32_t SkNamedFactorySet::find(SkFlattenable::Factory factory) {
+    uint32_t index = fFactorySet.find(factory);
+    if (index > 0) {
+        return index;
+    }
+    const char* name = SkFlattenable::FactoryToName(factory);
+    if (NULL == name) {
+        return 0;
+    }
+    *fNames.append() = name;
+    return fFactorySet.add(factory);
+}
+
+const char* SkNamedFactorySet::getNextAddedFactoryName() {
+    if (fNextAddedFactory < fNames.count()) {
+        return fNames[fNextAddedFactory++];
+    }
+    return NULL;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
 SkFlattenableWriteBuffer::SkFlattenableWriteBuffer() {
     fFlags = (Flags)0;
     fRCSet = NULL;
     fTFSet = NULL;
     fFactorySet = NULL;
+    fNamedFactorySet = NULL;
 }
 
 SkFlattenableWriteBuffer::~SkFlattenableWriteBuffer() {
     SkSafeUnref(fRCSet);
     SkSafeUnref(fTFSet);
     SkSafeUnref(fFactorySet);
+    SkSafeUnref(fNamedFactorySet);
 }
 
 SkRefCntSet* SkFlattenableWriteBuffer::setRefCntRecorder(SkRefCntSet* rec) {
@@ -71,6 +97,20 @@ SkRefCntSet* SkFlattenableWriteBuffer::setTypefaceRecorder(SkRefCntSet* rec) {
 
 SkFactorySet* SkFlattenableWriteBuffer::setFactoryRecorder(SkFactorySet* rec) {
     SkRefCnt_SafeAssign(fFactorySet, rec);
+    if (fNamedFactorySet != NULL) {
+        fNamedFactorySet->unref();
+        fNamedFactorySet = NULL;
+    }
+    return rec;
+}
+
+SkNamedFactorySet* SkFlattenableWriteBuffer::setNamedFactoryRecorder(
+        SkNamedFactorySet* rec) {
+    SkRefCnt_SafeAssign(fNamedFactorySet, rec);
+    if (fFactorySet != NULL) {
+        fFactorySet->unref();
+        fFactorySet = NULL;
+    }
     return rec;
 }
 
diff --git a/src/core/SkOrderedReadBuffer.cpp b/src/core/SkOrderedReadBuffer.cpp
index f3ba4850b3..df949074fb 100644
--- a/src/core/SkOrderedReadBuffer.cpp
+++ b/src/core/SkOrderedReadBuffer.cpp
@@ -45,32 +45,16 @@ SkFlattenable* SkOrderedReadBuffer::readFlattenable() {
         if (0 == index) {
             return NULL; // writer failed to give us the flattenable
         }
-        index = -index; // we stored the negative of the index
         index -= 1;     // we stored the index-base-1
         SkASSERT(index < fFactoryCount);
         factory = fFactoryArray[index];
     } else if (fFactoryTDArray) {
-        const int32_t* peek = (const int32_t*)fReader.peek();
-        if (*peek <= 0) {
-            int32_t index = fReader.readU32();
-            if (0 == index) {
-                return NULL; // writer failed to give us the flattenable
-            }
-            index = -index; // we stored the negative of the index
-            index -= 1;     // we stored the index-base-1
-            factory = (*fFactoryTDArray)[index];
-        } else {
-            const char* name = fReader.readString();
-            factory = SkFlattenable::NameToFactory(name);
-            if (factory) {
-                SkASSERT(fFactoryTDArray->find(factory) < 0);
-                *fFactoryTDArray->append() = factory;
-            } else {
-//                SkDebugf("can't find factory for [%s]\n", name);
-            }
-            // if we didn't find a factory, that's our failure, not the writer's,
-            // so we fall through, so we can skip the sizeRecorded data.
+        int32_t index = fReader.readU32();
+        if (0 == index) {
+            return NULL; // writer failed to give us the flattenable
         }
+        index -= 1;     // we stored the index-base-1
+        factory = (*fFactoryTDArray)[index];
     } else {
         factory = (SkFlattenable::Factory)readFunctionPtr();
         if (NULL == factory) {
diff --git a/src/core/SkOrderedWriteBuffer.cpp b/src/core/SkOrderedWriteBuffer.cpp
index 9c4303347a..fdfb6e3b15 100644
--- a/src/core/SkOrderedWriteBuffer.cpp
+++ b/src/core/SkOrderedWriteBuffer.cpp
@@ -23,8 +23,7 @@ void SkOrderedWriteBuffer::writeFlattenable(SkFlattenable* flattenable) {
     /*
      *  If we have a factoryset, then the first 32bits tell us...
      *       0: failure to write the flattenable
-     *      <0: we store the negative of the (1-based) index
-     *      >0: the length of the name
+     *      >0: (1-based) index into the SkFactorySet or SkNamedFactorySet
      *  If we don't have a factoryset, then the first "ptr" is either the
      *  factory, or null for failure.
      *
@@ -37,7 +36,7 @@ void SkOrderedWriteBuffer::writeFlattenable(SkFlattenable* flattenable) {
         factory = flattenable->getFactory();
     }
     if (NULL == factory) {
-        if (fFactorySet) {
+        if (fFactorySet != NULL || fNamedFactorySet != NULL) {
             this->write32(0);
         } else {
             this->writeFunctionPtr(NULL);
@@ -53,31 +52,17 @@ void SkOrderedWriteBuffer::writeFlattenable(SkFlattenable* flattenable) {
      *      resolve the function-ptrs into strings for its reader. SkPicture
      *      does exactly this, by writing a table of names (matching the indices)
      *      up front in its serialized form.
-     *  3.  names : Reuse fFactorySet to store indices, but only after we've
-     *      written the name the first time. SkGPipe uses this technique, as it
-     *      doesn't require the reader to be told to know the table of names
-     *      up front.
+     *  3.  index into fNamedFactorySet. fNamedFactorySet will also store the
+     *      name. SkGPipe uses this technique so it can write the name to its
+     *      stream before writing the flattenable.
      */
     if (fFactorySet) {
-        if (this->inlineFactoryNames()) {
-            int index = fFactorySet->find(factory);
-            if (index) {
-                // we write the negative of the index, to distinguish it from
-                // the length of a string
-                this->write32(-index);
-            } else {
-                const char* name = SkFlattenable::FactoryToName(factory);
-                if (NULL == name) {
-                    this->write32(0);
-                    return;
-                }
-                this->writeString(name);
-                index = fFactorySet->add(factory);
-            }
-        } else {
-            // we write the negative of the index, to distinguish it from
-            // the length of a string
-            this->write32(-(int)fFactorySet->add(factory));
+        this->write32(fFactorySet->add(factory));
+    } else if (fNamedFactorySet) {
+        int32_t index = fNamedFactorySet->find(factory);
+        this->write32(index);
+        if (0 == index) {
+            return;
         }
     } else {
         this->writeFunctionPtr((void*)factory);
diff --git a/src/core/SkPictureFlat.cpp b/src/core/SkPictureFlat.cpp
index 763ed60c5c..b0c02cc728 100644
--- a/src/core/SkPictureFlat.cpp
+++ b/src/core/SkPictureFlat.cpp
@@ -63,7 +63,7 @@ SkRefCnt* SkRefCntPlayback::set(int index, SkRefCnt* obj) {
 SkFlatData* SkFlatData::Create(SkFlatController* controller, const void* obj,
         int index, void (*flattenProc)(SkOrderedWriteBuffer&, const void*),
         SkRefCntSet* refCntRecorder, SkRefCntSet* faceRecorder,
-        uint32_t writeBufferflags, SkFactorySet* fset) {
+        uint32_t writeBufferflags, SkNamedFactorySet* fset) {
     // a buffer of 256 bytes should be sufficient for most paints, regions,
     // and matrices.
     intptr_t storage[256];
@@ -75,7 +75,7 @@ SkFlatData* SkFlatData::Create(SkFlatController* controller, const void* obj,
         buffer.setTypefaceRecorder(faceRecorder);
     }
     buffer.setFlags(writeBufferflags);
-    buffer.setFactoryRecorder(fset);
+    buffer.setNamedFactoryRecorder(fset);
     
     flattenProc(buffer, obj);
     uint32_t size = buffer.size();
diff --git a/src/core/SkPictureFlat.h b/src/core/SkPictureFlat.h
index bd3c4f89a6..ddcb100d6c 100644
--- a/src/core/SkPictureFlat.h
+++ b/src/core/SkPictureFlat.h
@@ -175,6 +175,7 @@ public:
      * provided.
      */
     virtual void unalloc(void* ptr) = 0;
+
 };
 
 class SkFlatData {
@@ -241,7 +242,7 @@ public:
                               SkRefCntSet* refCntRecorder = NULL,
                               SkRefCntSet* faceRecorder = NULL,
                               uint32_t writeBufferflags = 0,
-                              SkFactorySet* fset = NULL);
+                              SkNamedFactorySet* fset = NULL);
 
     void unflatten(void* result,
                    void (*unflattenProc)(SkOrderedReadBuffer&, void*),
@@ -291,7 +292,7 @@ class SkFlatDictionary {
 public:
     SkFlatDictionary(SkFlatController* controller, SkRefCntSet* refSet = NULL,
                      SkRefCntSet* typeFaceSet = NULL,
-                     SkFactorySet* factorySet = NULL)
+                     SkNamedFactorySet* factorySet = NULL)
     : fController(controller), fRefSet(refSet), fTypefaceSet(typeFaceSet)
     , fFactorySet(factorySet) {
         fFlattenProc = NULL;
@@ -444,7 +445,7 @@ private:
     SkTDArray<const SkFlatData*> fData;
     SkRefCntSet*                 fRefSet;
     SkRefCntSet*                 fTypefaceSet;
-    SkFactorySet*                fFactorySet;
+    SkNamedFactorySet*           fFactorySet;
 
     const SkFlatData* findAndReturnFlat(const T& element,
                                         uint32_t writeBufferflags) {
@@ -540,7 +541,7 @@ class SkBitmapDictionary : public SkFlatDictionary<SkBitmap> {
 public:
     SkBitmapDictionary(SkFlatController* controller, SkRefCntSet* refSet = NULL,
                        SkRefCntSet* typefaceSet = NULL,
-                       SkFactorySet* factorySet = NULL)
+                       SkNamedFactorySet* factorySet = NULL)
     : SkFlatDictionary<SkBitmap>(controller, refSet, typefaceSet, factorySet) {
         fFlattenProc = &SkFlattenObjectProc<SkBitmap>;
         fUnflattenProc = &SkUnflattenObjectProc<SkBitmap>;