force readbuffer clients to use specialized readFoo for flattenables

include/core/SkFlattenableBuffers.h

Index: include/core/SkFlattenableBuffers.h
diff --git a/include/core/SkFlattenableBuffers.h b/include/core/SkFlattenableBuffers.h
index 03c03f3877e6f8e3f508d29e371d5c7ebd91624e..7a5cf811ac6f572aee9ac4e7a20a57947370dd39 100644
--- a/include/core/SkFlattenableBuffers.h
+++ b/include/core/SkFlattenableBuffers.h
@@ -29,6 +29,30 @@ class SkString;
 class SkTypeface;
 class SkWStream;
 
+class SkAnnotation;
+class SkColorTable;
+class SkData;
+class SkDataSet;
+class SkDrawLooper;
+class SkLight;
+class SkPixelRef;
+class SkUnitMapper;
+
+enum SkEffectType {
+    kColorFilter_SkEffectType,
+    kDrawLooper_SkEffectType,
+    kImageFilter_SkEffectType,
+    kMaskFilter_SkEffectType,
+    kPathEffect_SkEffectType,
+    kPixelRef_SkEffectType,
+    kRasterizer_SkEffectType,
+    kShader_SkEffectType,
+    kUnitMapper_SkEffectType,
+    kXfermode_SkEffectType,
+    
+    kLight_SkEffectType,
+};
+
 class SkFlattenableReadBuffer {
 public:
     SkFlattenableReadBuffer();
@@ -63,8 +87,63 @@ public:
     virtual void readString(SkString* string) = 0;
     virtual void* readEncodedString(size_t* length, SkPaint::TextEncoding encoding) = 0;
 
+    SkFlattenable* readEffect(SkEffectType);
+
+    SkColorFilter* readColorFilter() {
+        return (SkColorFilter*)this->readEffect(kColorFilter_SkEffectType);
+    }
+    SkDrawLooper* readDrawLooper() {
+        return (SkDrawLooper*)this->readEffect(kDrawLooper_SkEffectType);
+    }
+    SkImageFilter* readImageFilter() {
+        return (SkImageFilter*)this->readEffect(kImageFilter_SkEffectType);
+    }
+    SkMaskFilter* readMaskFilter() {
+        return (SkMaskFilter*)this->readEffect(kMaskFilter_SkEffectType);
+    }
+    SkPathEffect* readPathEffect() {
+        return (SkPathEffect*)this->readEffect(kPathEffect_SkEffectType);
+    }
+    SkPixelRef* readPixelRef() {
+        return (SkPixelRef*)this->readEffect(kPixelRef_SkEffectType);
+    }
+    SkRasterizer* readRasterizer() {
+        return (SkRasterizer*)this->readEffect(kRasterizer_SkEffectType);
+    }
+    SkShader* readShader() {
+        return (SkShader*)this->readEffect(kShader_SkEffectType);
+    }
+    SkUnitMapper* readUnitMapper() {
+        return (SkUnitMapper*)this->readEffect(kUnitMapper_SkEffectType);
+    }
+    SkXfermode* readXfermode() {
+        return (SkXfermode*)this->readEffect(kXfermode_SkEffectType);
+    }
+    
+    // Does support subclasses, but is internal to our impl, and is not in
+    // core...
+    //
+    SkLight* readLight() {
+        return (SkLight*)this->readEffect(kLight_SkEffectType);
+    }
+    
+    // These flattenables do *not* support subclassing, so perhaps need not
+    // actually inherit from SkFlattenable
+    //
+    SkAnnotation* readAnnotation() {
+        return (SkAnnotation*)this->readAnnotation();
+    }
+    SkColorTable* readColorTable() {
+        return (SkColorTable*)this->readColorTable();
+    }
+    SkData* readData() {
+        return (SkData*)this->readFlattenable();
+    }
+    SkDataSet* readDataSet() {
+        return (SkDataSet*)this->readFlattenable();
+    }
+
     // common data structures
-    virtual SkFlattenable* readFlattenable() = 0;
     virtual void readPoint(SkPoint* point) = 0;
     virtual void readMatrix(SkMatrix* matrix) = 0;
     virtual void readIRect(SkIRect* rect) = 0;
@@ -98,11 +177,15 @@ public:
         return point;
     }
 
+#if 0
     template <typename T> T* readFlattenableT() {

[sugoi, 2013/10/09 20:26:55]

Ok, so I've been thinking about this. Even though the public calls are all
strongly typed, I still think it would be much easier to implement / safer if
the inside of these functions are from a (protected/private) call to a template
function and having something like T::GetType() (or maybe
SkFlattenable::GetTypeFromName(T::GetName())) inside that template function.
We'll need to get the type from the name somehow anyway when reading the stream
to do type checking.

In a more abstract way of saying it, I would really appreciate a design that
would not have us manually link an enum with a class in 2 different spots in the
code (which would be here, and at registration time, if we want type checking to
work -- and we'd need some special registration phase for base classes too, to
link each base class with its enum, I guess, which we don't have now), since I
think that would be error prone. If we could simply link an enum value with each
class ONLY at registration time and use the template here to figure out the type
without explicitly writing it in each function, I would find that much cleaner.

         return static_cast<T*>(this->readFlattenable());
     }
+#endif
 
 private:
+    virtual SkFlattenable* readFlattenable() = 0;
+
     uint32_t fFlags;
 };