remove legacy filter-flags, and store FilterLevel directly

src/core/SkPaint.cpp

Index: src/core/SkPaint.cpp
diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp
index 9cb61bd9d128707299306f36f695bd2eb7f1d9df..6ab8e6e1248a8a707ac6b88cabb5d7c30bcc233a 100644
--- a/src/core/SkPaint.cpp
+++ b/src/core/SkPaint.cpp
@@ -251,27 +251,10 @@ void SkPaint::setPaintOptionsAndroid(const SkPaintOptionsAndroid& options) {
 }
 #endif
 
-SkPaint::FilterLevel SkPaint::getFilterLevel() const {
-    int level = 0;
-    if (fFlags & kFilterBitmap_Flag) {
-        level |= 1;
-    }
-    if (fFlags & kHighQualityFilterBitmap_Flag) {
-        level |= 2;
-    }
-    return (FilterLevel)level;
-}
-
 void SkPaint::setFilterLevel(FilterLevel level) {
-    unsigned mask = kFilterBitmap_Flag | kHighQualityFilterBitmap_Flag;
-    unsigned flags = 0;
-    if (level & 1) {
-        flags |= kFilterBitmap_Flag;
-    }
-    if (level & 2) {
-        flags |= kHighQualityFilterBitmap_Flag;
-    }
-    this->setFlags((fFlags & ~mask) | flags);
+    GEN_ID_INC_EVAL((unsigned) level != fFilterLevel);
+    fFilterLevel = level;
+    fDirtyBits |= kBitfields_DirtyBit;
 }
 
 void SkPaint::setHinting(Hinting hintingLevel) {
@@ -2026,12 +2009,76 @@ static uint32_t pack_4(unsigned a, unsigned b, unsigned c, unsigned d) {
     return (a << 24) | (b << 16) | (c << 8) | d;
 }
 
+#ifdef SK_DEBUG
+    static void ASSEERT_FITS_IN(uint32_t value, int bitCount) {

[mtklein, 2014/04/15 16:40:15]

ASSEERT?

[reed1, 2014/04/15 18:57:46]

Done.

+        SkASSERT(bitCount > 0 && bitCount <= 32);
+        uint32_t mask = ~0U;
+        mask >>= (32 - bitCount);
+        SkASSERT(0 == (value & ~mask));

[mtklein, 2014/04/15 16:40:15]

Is there any reason to prefer this to SkASSERT(value == (value & mask)) ?

[reed1, 2014/04/15 18:57:46]

half dozen. I can change it.

+    }
+#else
+    #define ASSEERT_FITS_IN(value, bitcount)
+#endif
+
 enum FlatFlags {
     kHasTypeface_FlatFlag                      = 0x01,
     kHasEffects_FlatFlag                       = 0x02,
     kHasNonDefaultPaintOptionsAndroid_FlatFlag = 0x04,
 };
 
+static uint32_t pack_paint_flags(unsigned flags, unsigned hint, unsigned align,
+                                 unsigned filter, unsigned flatFlags) {
+    ASSEERT_FITS_IN(flags, 16);
+    ASSEERT_FITS_IN(hint, 2);
+    ASSEERT_FITS_IN(align, 2);
+    ASSEERT_FITS_IN(filter, 2);
+    ASSEERT_FITS_IN(flatFlags, 32 - (16 + 2 + 2 + 2));
+
+    // left-align the fields of "known" size, and right-align the last (flatFlags) so it can easly
+    // add more bits in the future.
+    return (flags << 16) | (hint << 14) | (align << 12) | (filter << 10) | flatFlags;
+}
+
+static unsigned unpack_paint_flags(SkPaint* paint, uint32_t packed) {

[mtklein, 2014/04/15 16:40:15]

// Returns FlatFlags
?

[reed1, 2014/04/15 18:57:46]

Done.

+    paint->setFlags(packed >> 16);
+    paint->setHinting((SkPaint::Hinting)((packed >> 14) & 0x3));
+    paint->setTextAlign((SkPaint::Align)((packed >> 12) & 0x3));
+    paint->setFilterLevel((SkPaint::FilterLevel)((packed >> 10) & 0x3));
+    return packed & 0x7;
+}
+
+static unsigned unpack_paint_flags_v22(SkPaint* paint, uint32_t packed) {
+    enum {
+        kFilterBitmap_Flag    = 0x02,
+        kHighQualityFilterBitmap_Flag = 0x4000,
+        kHighQualityDownsampleBitmap_Flag = 0x8000,
+        
+        kAll_Flags = kFilterBitmap_Flag | kHighQualityFilterBitmap_Flag | kHighQualityDownsampleBitmap_Flag
+    };
+
+    // previously flags:16, textAlign:8, flatFlags:8
+    // now flags:16, hinting:4, textAlign:4, flatFlags:8
+    unsigned flags = packed >> 16;
+    SkPaint::FilterLevel filter = SkPaint::kNone_FilterLevel;
+    if (kHighQualityFilterBitmap_Flag & flags) {
+        filter = SkPaint::kHigh_FilterLevel;
+    } else if (kHighQualityDownsampleBitmap_Flag & flags) {
+        filter = SkPaint::kMedium_FilterLevel;
+    } else if (kFilterBitmap_Flag & flags) {
+        filter = SkPaint::kLow_FilterLevel;
+    }
+    paint->setFilterLevel(filter);
+    flags &= ~kAll_Flags;
+
+    paint->setFlags(packed >> 16);
+    
+    // hinting added later. 0 in this nibble means use the default.
+    uint32_t hinting = (packed >> 12) & 0xF;
+    paint->setHinting(0 == hinting ? SkPaint::kNormal_Hinting : static_cast<SkPaint::Hinting>(hinting-1));
+    paint->setTextAlign(static_cast<SkPaint::Align>((packed >> 8) & 0xF));
+    return packed & 0x7;
+}
+
 // The size of a flat paint's POD fields
 static const uint32_t kPODPaintSize =   5 * sizeof(SkScalar) +
                                         1 * sizeof(SkColor) +
@@ -2072,6 +2119,8 @@ void SkPaint::flatten(SkWriteBuffer& buffer) const {
     ptr = write_scalar(ptr, this->getStrokeWidth());
     ptr = write_scalar(ptr, this->getStrokeMiter());
     *ptr++ = this->getColor();
+    
+#if 0
     // previously flags:16, textAlign:8, flatFlags:8

[mtklein, 2014/04/15 16:40:15]

Delete this now?

[reed1, 2014/04/15 18:57:46]

Done.

     // now flags:16, hinting:4, textAlign:4, flatFlags:8
     *ptr++ = (this->getFlags() << 16) |
@@ -2079,6 +2128,10 @@ void SkPaint::flatten(SkWriteBuffer& buffer) const {
         ((this->getHinting()+1) << 12) |
         (this->getTextAlign() << 8) |
         flatFlags;
+#else
+    *ptr++ = pack_paint_flags(this->getFlags(), this->getHinting(), this->getTextAlign(),
+                              this->getFilterLevel(), flatFlags);
+#endif
     *ptr++ = pack_4(this->getStrokeCap(), this->getStrokeJoin(),
                     this->getStyle(), this->getTextEncoding());
 
@@ -2112,7 +2165,6 @@ void SkPaint::flatten(SkWriteBuffer& buffer) const {
 }
 
 void SkPaint::unflatten(SkReadBuffer& buffer) {
-    uint8_t flatFlags = 0;
     SkASSERT(SkAlign4(kPODPaintSize) == kPODPaintSize);
     const void* podData = buffer.skip(kPODPaintSize);
     const uint32_t* pod = reinterpret_cast<const uint32_t*>(podData);
@@ -2125,20 +2177,15 @@ void SkPaint::unflatten(SkReadBuffer& buffer) {
     this->setStrokeMiter(read_scalar(pod));
     this->setColor(*pod++);
 
-    // previously flags:16, textAlign:8, flatFlags:8
-    // now flags:16, hinting:4, textAlign:4, flatFlags:8
-    uint32_t tmp = *pod++;
-    this->setFlags(tmp >> 16);
-
-    // hinting added later. 0 in this nibble means use the default.
-    uint32_t hinting = (tmp >> 12) & 0xF;
-    this->setHinting(0 == hinting ? kNormal_Hinting : static_cast<Hinting>(hinting-1));
-
-    this->setTextAlign(static_cast<Align>((tmp >> 8) & 0xF));
-
-    flatFlags = tmp & 0xFF;
+    const int picVer = buffer.pictureVersion();
+    unsigned flatFlags = 0;
+    if (picVer > 0 && picVer <= 22) {

[mtklein, 2014/04/15 16:40:15]

Don't we need a version bump too?

[reed1, 2014/04/15 18:57:46]

Indeed

+        flatFlags = unpack_paint_flags_v22(this, *pod++);
+    } else {
+        flatFlags = unpack_paint_flags(this, *pod++);
+    }
 
-    tmp = *pod++;
+    uint32_t tmp = *pod++;
     this->setStrokeCap(static_cast<Cap>((tmp >> 24) & 0xFF));
     this->setStrokeJoin(static_cast<Join>((tmp >> 16) & 0xFF));
     this->setStyle(static_cast<Style>((tmp >> 8) & 0xFF));