diff options
author | reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2013-10-10 14:44:56 +0000 |
---|---|---|
committer | reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2013-10-10 14:44:56 +0000 |
commit | 0a6151d66cc32d91eca037c91e557158cf8a2be2 (patch) | |
tree | 7fe96285f9fee5c8804bbbc60dd0395b983511a0 | |
parent | b006fe7e2f6178ce076a5e2c76b4540d3d27ca98 (diff) |
Revert "Revert "change SkColorTable to be immutable""
This reverts commit b8162cb840f4cb6002ef68d5ac775c6a122c52a9.
Fixed was call-sites in benches that used the (now gone) setIsOpaque api.
R=scroggo@google.com
Review URL: https://codereview.chromium.org/26572006
git-svn-id: http://skia.googlecode.com/svn/trunk@11695 2bbb7eff-a529-9590-31e7-b0007b416f81
-rw-r--r-- | bench/BitmapBench.cpp | 14 | ||||
-rw-r--r-- | bench/RepeatTileBench.cpp | 12 | ||||
-rw-r--r-- | gm/tinybitmap.cpp | 9 | ||||
-rw-r--r-- | include/core/SkAlpha.h | 3 | ||||
-rw-r--r-- | include/core/SkBitmap.h | 4 | ||||
-rw-r--r-- | include/core/SkColorTable.h | 51 | ||||
-rw-r--r-- | samplecode/SampleBlur.cpp | 10 | ||||
-rw-r--r-- | samplecode/SampleDitherBitmap.cpp | 15 | ||||
-rw-r--r-- | samplecode/SampleTinyBitmap.cpp | 11 | ||||
-rw-r--r-- | src/core/SkBitmap.cpp | 12 | ||||
-rw-r--r-- | src/core/SkBitmapProcState_procs.h | 6 | ||||
-rw-r--r-- | src/core/SkColorTable.cpp | 93 | ||||
-rw-r--r-- | src/core/SkProcSpriteBlitter.cpp | 2 | ||||
-rw-r--r-- | src/core/SkSpriteBlitter_RGB16.cpp | 4 | ||||
-rw-r--r-- | src/gpu/SkGr.cpp | 2 | ||||
-rw-r--r-- | src/images/SkImageDecoder_libgif.cpp | 15 | ||||
-rw-r--r-- | src/images/SkImageDecoder_libpng.cpp | 25 | ||||
-rw-r--r-- | src/opts/SkBitmapProcState_opts_arm.cpp | 2 | ||||
-rw-r--r-- | tests/BitmapCopyTest.cpp | 105 |
19 files changed, 168 insertions, 227 deletions
diff --git a/bench/BitmapBench.cpp b/bench/BitmapBench.cpp index 9b3a6a29dd..982c917c02 100644 --- a/bench/BitmapBench.cpp +++ b/bench/BitmapBench.cpp @@ -33,9 +33,9 @@ static uint8_t compute666Index(SkPMColor c) { return convByteTo6(r) * 36 + convByteTo6(g) * 6 + convByteTo6(b); } -static void convertToIndex666(const SkBitmap& src, SkBitmap* dst) { - SkColorTable* ctable = new SkColorTable(216); - SkPMColor* colors = ctable->lockColors(); +static void convertToIndex666(const SkBitmap& src, SkBitmap* dst, bool isOpaque) { + SkPMColor storage[216]; + SkPMColor* colors = storage; // rrr ggg bbb for (int r = 0; r < 6; r++) { int rr = conv6ToByte(r); @@ -47,7 +47,8 @@ static void convertToIndex666(const SkBitmap& src, SkBitmap* dst) { } } } - ctable->unlockColors(true); + SkColorTable* ctable = new SkColorTable(storage, 216, + isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType); dst->setConfig(SkBitmap::kIndex8_Config, src.width(), src.height()); dst->allocPixels(ctable); ctable->unref(); @@ -120,14 +121,11 @@ protected: onDrawIntoBitmap(bm); if (SkBitmap::kIndex8_Config == fConfig) { - convertToIndex666(bm, &fBitmap); + convertToIndex666(bm, &fBitmap, fIsOpaque); } else { fBitmap = bm; } - if (fBitmap.getColorTable()) { - fBitmap.getColorTable()->setIsOpaque(fIsOpaque); - } fBitmap.setIsOpaque(fIsOpaque); fBitmap.setIsVolatile(fIsVolatile); } diff --git a/bench/RepeatTileBench.cpp b/bench/RepeatTileBench.cpp index c311c4af6c..612a149d6d 100644 --- a/bench/RepeatTileBench.cpp +++ b/bench/RepeatTileBench.cpp @@ -52,9 +52,10 @@ static uint8_t compute_666_index(SkPMColor c) { return conv_byte_to_6(r) * 36 + conv_byte_to_6(g) * 6 + conv_byte_to_6(b); } -static void convert_to_index666(const SkBitmap& src, SkBitmap* dst) { - SkColorTable* ctable = new SkColorTable(216); - SkPMColor* colors = ctable->lockColors(); +static void convert_to_index666(const SkBitmap& src, SkBitmap* dst, + bool isOpaque) { + SkPMColor storage[216]; + SkPMColor* colors = storage; // rrr ggg bbb for (int r = 0; r < 6; r++) { int rr = conv_6_to_byte(r); @@ -66,7 +67,8 @@ static void convert_to_index666(const SkBitmap& src, SkBitmap* dst) { } } } - ctable->unlockColors(true); + SkAlphaType aType = isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType; + SkColorTable* ctable = new SkColorTable(storage, 216, aType); dst->setConfig(SkBitmap::kIndex8_Config, src.width(), src.height()); dst->allocPixels(ctable); ctable->unref(); @@ -119,7 +121,7 @@ protected: if (SkBitmap::kIndex8_Config == fConfig) { SkBitmap tmp; - convert_to_index666(fBitmap, &tmp); + convert_to_index666(fBitmap, &tmp, fIsOpaque); fBitmap = tmp; } diff --git a/gm/tinybitmap.cpp b/gm/tinybitmap.cpp index 26bf25e665..6cb9eded73 100644 --- a/gm/tinybitmap.cpp +++ b/gm/tinybitmap.cpp @@ -14,13 +14,10 @@ namespace skiagm { static SkBitmap make_bitmap() { - SkBitmap bm; - - SkColorTable* ctable = new SkColorTable(1); - SkPMColor* c = ctable->lockColors(); - c[0] = SkPackARGB32(0x80, 0x80, 0, 0); - ctable->unlockColors(true); + const SkPMColor c[] = { SkPackARGB32(0x80, 0x80, 0, 0) }; + SkColorTable* ctable = new SkColorTable(c, SK_ARRAY_COUNT(c)); + SkBitmap bm; bm.setConfig(SkBitmap::kIndex8_Config, 1, 1); bm.allocPixels(ctable); ctable->unref(); diff --git a/include/core/SkAlpha.h b/include/core/SkAlpha.h index 9e0fcab0e2..74ea687f96 100644 --- a/include/core/SkAlpha.h +++ b/include/core/SkAlpha.h @@ -47,4 +47,7 @@ enum SkAlphaType { kLastEnum_SkAlphaType = kUnpremul_SkAlphaType }; +static inline bool SkAlphaTypeIsOpaque(SkAlphaType at) { + return (unsigned)at <= kOpaque_SkAlphaType; +} #endif diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h index 887169ccb5..07d2b95553 100644 --- a/include/core/SkBitmap.h +++ b/include/core/SkBitmap.h @@ -748,7 +748,7 @@ public: } ~SkAutoLockColors() { if (fCTable) { - fCTable->unlockColors(false); + fCTable->unlockColors(); } } @@ -762,7 +762,7 @@ public: */ const SkPMColor* lockColors(SkColorTable* ctable) { if (fCTable) { - fCTable->unlockColors(false); + fCTable->unlockColors(); } fCTable = ctable; fColors = ctable ? ctable->lockColors() : NULL; diff --git a/include/core/SkColorTable.h b/include/core/SkColorTable.h index f8d1ccf07b..c6e31b94d4 100644 --- a/include/core/SkColorTable.h +++ b/include/core/SkColorTable.h @@ -10,6 +10,7 @@ #ifndef SkColorTable_DEFINED #define SkColorTable_DEFINED +#include "SkAlpha.h" #include "SkColor.h" #include "SkFlattenable.h" @@ -25,25 +26,15 @@ public: /** Makes a deep copy of colors. */ SkColorTable(const SkColorTable& src); - /** Preallocates the colortable to have 'count' colors, which - * are initially set to 0. - */ - explicit SkColorTable(int count); - SkColorTable(const SkPMColor colors[], int count); + SkColorTable(const SkPMColor colors[], int count, + SkAlphaType alphaType = kPremul_SkAlphaType); virtual ~SkColorTable(); - enum Flags { - kColorsAreOpaque_Flag = 0x01 //!< if set, all of the colors in the table are opaque (alpha==0xFF) - }; - /** Returns the flag bits for the color table. These can be changed with setFlags(). - */ - unsigned getFlags() const { return fFlags; } - /** Set the flags for the color table. See the Flags enum for possible values. - */ - void setFlags(unsigned flags); + SkAlphaType alphaType() const { return (SkAlphaType)fAlphaType; } - bool isOpaque() const { return (fFlags & kColorsAreOpaque_Flag) != 0; } - void setIsOpaque(bool isOpaque); + bool isOpaque() const { + return SkAlphaTypeIsOpaque(this->alphaType()); + } /** Returns the number of colors in the table. */ @@ -57,25 +48,19 @@ public: return fColors[index]; } - /** Specify the number of colors in the color table. This does not initialize the colors - to any value, just allocates memory for them. To initialize the values, either call - setColors(array, count), or follow setCount(count) with a call to - lockColors()/{set the values}/unlockColors(true). - */ -// void setColors(int count) { this->setColors(NULL, count); } -// void setColors(const SkPMColor[], int count); - - /** Return the array of colors for reading and/or writing. This must be - balanced by a call to unlockColors(changed?), telling the colortable if - the colors were changed during the lock. - */ - SkPMColor* lockColors() { + /** + * Return the array of colors for reading. This must be balanced by a call + * to unlockColors(). + */ + const SkPMColor* lockColors() { SkDEBUGCODE(sk_atomic_inc(&fColorLockCount);) return fColors; } - /** Balancing call to lockColors(). If the colors have been changed, pass true. - */ - void unlockColors(bool changed); + + /** + * Balancing call to lockColors(). + */ + void unlockColors(); /** Similar to lockColors(), lock16BitCache() returns the array of RGB16 colors that mirror the 32bit colors. However, this function @@ -100,7 +85,7 @@ private: SkPMColor* fColors; uint16_t* f16BitCache; uint16_t fCount; - uint8_t fFlags; + uint8_t fAlphaType; SkDEBUGCODE(int fColorLockCount;) SkDEBUGCODE(int f16BitCacheLockCount;) diff --git a/samplecode/SampleBlur.cpp b/samplecode/SampleBlur.cpp index e8e9772287..d951d8508b 100644 --- a/samplecode/SampleBlur.cpp +++ b/samplecode/SampleBlur.cpp @@ -15,14 +15,14 @@ #include "SkView.h" static SkBitmap make_bitmap() { - SkBitmap bm; - SkColorTable* ctable = new SkColorTable(256); - - SkPMColor* c = ctable->lockColors(); + SkPMColor c[256]; for (int i = 0; i < 256; i++) { c[i] = SkPackARGB32(255 - i, 0, 0, 0); } - ctable->unlockColors(true); + + SkBitmap bm; + SkColorTable* ctable = new SkColorTable(c, 256); + bm.setConfig(SkBitmap::kIndex8_Config, 256, 256); bm.allocPixels(ctable); ctable->unref(); diff --git a/samplecode/SampleDitherBitmap.cpp b/samplecode/SampleDitherBitmap.cpp index df727c4203..7f29305a51 100644 --- a/samplecode/SampleDitherBitmap.cpp +++ b/samplecode/SampleDitherBitmap.cpp @@ -54,14 +54,13 @@ static void test_pathregion() { } static SkBitmap make_bitmap() { - SkBitmap bm; - SkColorTable* ctable = new SkColorTable(256); - - SkPMColor* c = ctable->lockColors(); + SkPMColor c[256]; for (int i = 0; i < 256; i++) { c[i] = SkPackARGB32(0xFF, i, 0, 0); } - ctable->unlockColors(true); + SkColorTable* ctable = new SkColorTable(c, 256, kOpaque_SkAlphaType); + + SkBitmap bm; bm.setConfig(SkBitmap::kIndex8_Config, 256, 32); bm.allocPixels(ctable); ctable->unref(); @@ -102,10 +101,14 @@ protected: static void setBitmapOpaque(SkBitmap* bm, bool isOpaque) { SkAutoLockPixels alp(*bm); // needed for ctable bm->setIsOpaque(isOpaque); +#if 0 SkColorTable* ctable = bm->getColorTable(); if (ctable) { - ctable->setIsOpaque(isOpaque); + if (ctable->isOpaque() != isOpaque) { + // how do we change a colortable? don't want to + } } +#endif } static void draw2(SkCanvas* canvas, const SkBitmap& bm) { diff --git a/samplecode/SampleTinyBitmap.cpp b/samplecode/SampleTinyBitmap.cpp index 16cbce4adc..42866d07ad 100644 --- a/samplecode/SampleTinyBitmap.cpp +++ b/samplecode/SampleTinyBitmap.cpp @@ -13,15 +13,15 @@ #include "SkUtils.h" static SkBitmap make_bitmap() { - SkBitmap bm; const int N = 1; - SkColorTable* ctable = new SkColorTable(N); - SkPMColor* c = ctable->lockColors(); + SkPMColor c[N]; for (int i = 0; i < N; i++) { c[i] = SkPackARGB32(0x80, 0x80, 0, 0); } - ctable->unlockColors(true); + SkColorTable* ctable = new SkColorTable(c, N); + + SkBitmap bm; bm.setConfig(SkBitmap::kIndex8_Config, 1, 1); bm.allocPixels(ctable); ctable->unref(); @@ -58,10 +58,13 @@ protected: static void setBitmapOpaque(SkBitmap* bm, bool isOpaque) { SkAutoLockPixels alp(*bm); // needed for ctable bm->setIsOpaque(isOpaque); +#if 0 + // TODO - I think we just want to not allow this anymore SkColorTable* ctable = bm->getColorTable(); if (ctable) { ctable->setIsOpaque(isOpaque); } +#endif } virtual void onDrawContent(SkCanvas* canvas) { diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index 04a58583a1..1d1cc0a45d 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -536,16 +536,12 @@ bool SkBitmap::isOpaque() const { return (fFlags & kImageIsOpaque_Flag) != 0; case kIndex8_Config: { - uint32_t flags = 0; + bool isOpaque; this->lockPixels(); - // if lockPixels failed, we may not have a ctable ptr - if (fColorTable) { - flags = fColorTable->getFlags(); - } + isOpaque = fColorTable && fColorTable->isOpaque(); this->unlockPixels(); - - return (flags & SkColorTable::kColorsAreOpaque_Flag) != 0; + return isOpaque; } case kRGB_565_Config: @@ -1494,7 +1490,7 @@ static bool GetBitmapAlpha(const SkBitmap& src, uint8_t* SK_RESTRICT alpha, s += rb; alpha += alphaRowBytes; } - ct->unlockColors(false); + ct->unlockColors(); } } else { // src is opaque, so just fill alpha[] with 0xFF memset(alpha, 0xFF, h * alphaRowBytes); diff --git a/src/core/SkBitmapProcState_procs.h b/src/core/SkBitmapProcState_procs.h index da9ca89bbf..68c79835a9 100644 --- a/src/core/SkBitmapProcState_procs.h +++ b/src/core/SkBitmapProcState_procs.h @@ -157,7 +157,7 @@ static inline U8CPU Filter_8(unsigned x, unsigned y, #define PREAMBLE(state) const SkPMColor* SK_RESTRICT table = state.fBitmap->getColorTable()->lockColors() #define RETURNDST(src) table[src] #define SRC_TO_FILTER(src) table[src] -#define POSTAMBLE(state) state.fBitmap->getColorTable()->unlockColors(false) +#define POSTAMBLE(state) state.fBitmap->getColorTable()->unlockColors() #include "SkBitmapProcState_sample.h" #undef FILTER_PROC @@ -172,7 +172,7 @@ static inline U8CPU Filter_8(unsigned x, unsigned y, const SkPMColor* SK_RESTRICT table = state.fBitmap->getColorTable()->lockColors() #define RETURNDST(src) SkAlphaMulQ(table[src], alphaScale) #define SRC_TO_FILTER(src) table[src] -#define POSTAMBLE(state) state.fBitmap->getColorTable()->unlockColors(false) +#define POSTAMBLE(state) state.fBitmap->getColorTable()->unlockColors() #include "SkBitmapProcState_sample.h" // SRC == 4444 @@ -337,7 +337,7 @@ static inline U8CPU Filter_8(unsigned x, unsigned y, #define CHECKSTATE(state) SkASSERT(state.fBitmap->config() == SkBitmap::kIndex8_Config) #define PREAMBLE(state) const SkPMColor* SK_RESTRICT table = state.fBitmap->getColorTable()->lockColors() #define SRC_TO_FILTER(src) table[src] -#define POSTAMBLE(state) state.fBitmap->getColorTable()->unlockColors(false) +#define POSTAMBLE(state) state.fBitmap->getColorTable()->unlockColors() #include "SkBitmapProcState_shaderproc.h" #undef NAME_WRAP diff --git a/src/core/SkColorTable.cpp b/src/core/SkColorTable.cpp index e1ebf9202d..c117443954 100644 --- a/src/core/SkColorTable.cpp +++ b/src/core/SkColorTable.cpp @@ -14,27 +14,11 @@ SK_DEFINE_INST_COUNT(SkColorTable) -SkColorTable::SkColorTable(int count) - : f16BitCache(NULL), fFlags(0) -{ - if (count < 0) - count = 0; - else if (count > 256) - count = 256; - - fCount = SkToU16(count); - fColors = (SkPMColor*)sk_malloc_throw(count * sizeof(SkPMColor)); - memset(fColors, 0, count * sizeof(SkPMColor)); - - SkDEBUGCODE(fColorLockCount = 0;) - SkDEBUGCODE(f16BitCacheLockCount = 0;) -} - // As copy constructor is hidden in the class hierarchy, we need to call // default constructor explicitly to suppress a compiler warning. SkColorTable::SkColorTable(const SkColorTable& src) : INHERITED() { f16BitCache = NULL; - fFlags = src.fFlags; + fAlphaType = src.fAlphaType; int count = src.count(); fCount = SkToU16(count); fColors = reinterpret_cast<SkPMColor*>( @@ -45,20 +29,22 @@ SkColorTable::SkColorTable(const SkColorTable& src) : INHERITED() { SkDEBUGCODE(f16BitCacheLockCount = 0;) } -SkColorTable::SkColorTable(const SkPMColor colors[], int count) - : f16BitCache(NULL), fFlags(0) +SkColorTable::SkColorTable(const SkPMColor colors[], int count, SkAlphaType at) + : f16BitCache(NULL), fAlphaType(SkToU8(at)) { - if (count < 0) + SkASSERT(0 == count || NULL != colors); + + if (count < 0) { count = 0; - else if (count > 256) + } else if (count > 256) { count = 256; + } fCount = SkToU16(count); fColors = reinterpret_cast<SkPMColor*>( sk_malloc_throw(count * sizeof(SkPMColor))); - if (colors) - memcpy(fColors, colors, count * sizeof(SkPMColor)); + memcpy(fColors, colors, count * sizeof(SkPMColor)); SkDEBUGCODE(fColorLockCount = 0;) SkDEBUGCODE(f16BitCacheLockCount = 0;) @@ -73,69 +59,30 @@ SkColorTable::~SkColorTable() sk_free(f16BitCache); } -void SkColorTable::setFlags(unsigned flags) -{ - fFlags = SkToU8(flags); -} - -void SkColorTable::unlockColors(bool changed) -{ +void SkColorTable::unlockColors() { SkASSERT(fColorLockCount != 0); SkDEBUGCODE(sk_atomic_dec(&fColorLockCount);) - if (changed) - this->inval16BitCache(); -} - -void SkColorTable::inval16BitCache() -{ - SkASSERT(f16BitCacheLockCount == 0); - if (f16BitCache) - { - sk_free(f16BitCache); - f16BitCache = NULL; - } } #include "SkColorPriv.h" -static inline void build_16bitcache(uint16_t dst[], const SkPMColor src[], int count) -{ - while (--count >= 0) +static inline void build_16bitcache(uint16_t dst[], const SkPMColor src[], + int count) { + while (--count >= 0) { *dst++ = SkPixel32ToPixel16_ToU16(*src++); + } } -const uint16_t* SkColorTable::lock16BitCache() -{ - if (fFlags & kColorsAreOpaque_Flag) - { - if (f16BitCache == NULL) // build the cache - { - f16BitCache = (uint16_t*)sk_malloc_throw(fCount * sizeof(uint16_t)); - build_16bitcache(f16BitCache, fColors, fCount); - } - } - else // our colors have alpha, so no cache - { - this->inval16BitCache(); - if (f16BitCache) - { - sk_free(f16BitCache); - f16BitCache = NULL; - } +const uint16_t* SkColorTable::lock16BitCache() { + if (this->isOpaque() && NULL == f16BitCache) { + f16BitCache = (uint16_t*)sk_malloc_throw(fCount * sizeof(uint16_t)); + build_16bitcache(f16BitCache, fColors, fCount); } SkDEBUGCODE(f16BitCacheLockCount += 1); return f16BitCache; } -void SkColorTable::setIsOpaque(bool isOpaque) { - if (isOpaque) { - fFlags |= kColorsAreOpaque_Flag; - } else { - fFlags &= ~kColorsAreOpaque_Flag; - } -} - /////////////////////////////////////////////////////////////////////////////// SkColorTable::SkColorTable(SkFlattenableReadBuffer& buffer) { @@ -143,7 +90,7 @@ SkColorTable::SkColorTable(SkFlattenableReadBuffer& buffer) { SkDEBUGCODE(fColorLockCount = 0;) SkDEBUGCODE(f16BitCacheLockCount = 0;) - fFlags = buffer.readUInt(); + fAlphaType = SkToU8(buffer.readUInt()); fCount = buffer.getArrayCount(); fColors = (SkPMColor*)sk_malloc_throw(fCount * sizeof(SkPMColor)); SkDEBUGCODE(const uint32_t countRead =) buffer.readColorArray(fColors); @@ -154,6 +101,6 @@ SkColorTable::SkColorTable(SkFlattenableReadBuffer& buffer) { } void SkColorTable::flatten(SkFlattenableWriteBuffer& buffer) const { - buffer.writeUInt(fFlags); + buffer.writeUInt(fAlphaType); buffer.writeColorArray(fColors, fCount); } diff --git a/src/core/SkProcSpriteBlitter.cpp b/src/core/SkProcSpriteBlitter.cpp index a48192056c..2b535d947d 100644 --- a/src/core/SkProcSpriteBlitter.cpp +++ b/src/core/SkProcSpriteBlitter.cpp @@ -36,7 +36,7 @@ public: } if fSource.getColorTable()) - fSource.getColorTable()->unlockColors(false); + fSource.getColorTable()->unlockColors(); } private: diff --git a/src/core/SkSpriteBlitter_RGB16.cpp b/src/core/SkSpriteBlitter_RGB16.cpp index 9936867f36..8cef76719e 100644 --- a/src/core/SkSpriteBlitter_RGB16.cpp +++ b/src/core/SkSpriteBlitter_RGB16.cpp @@ -145,7 +145,7 @@ public: #define SkSPRITE_PREAMBLE(srcBM, x, y) const SkPMColor* ctable = srcBM.getColorTable()->lockColors() #define SkSPRITE_BLIT_PIXEL(dst, src) D16_S32A_Opaque_Pixel(dst, ctable[src]) #define SkSPRITE_NEXT_ROW -#define SkSPRITE_POSTAMBLE(srcBM) srcBM.getColorTable()->unlockColors(false) +#define SkSPRITE_POSTAMBLE(srcBM) srcBM.getColorTable()->unlockColors() #include "SkSpriteBlitterTemplate.h" #define SkSPRITE_CLASSNAME Sprite_D16_SIndex8A_Blend @@ -159,7 +159,7 @@ public: #define SkSPRITE_PREAMBLE(srcBM, x, y) const SkPMColor* ctable = srcBM.getColorTable()->lockColors(); unsigned src_scale = SkAlpha255To256(fSrcAlpha); #define SkSPRITE_BLIT_PIXEL(dst, src) D16_S32A_Blend_Pixel(dst, ctable[src], src_scale) #define SkSPRITE_NEXT_ROW -#define SkSPRITE_POSTAMBLE(srcBM) srcBM.getColorTable()->unlockColors(false); +#define SkSPRITE_POSTAMBLE(srcBM) srcBM.getColorTable()->unlockColors(); #include "SkSpriteBlitterTemplate.h" /////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp index 7505d7527a..0b60ede836 100644 --- a/src/gpu/SkGr.cpp +++ b/src/gpu/SkGr.cpp @@ -31,7 +31,7 @@ static void build_compressed_data(void* buffer, const SkBitmap& bitmap) { char* dst = (char*)buffer; memcpy(dst, ctable->lockColors(), ctable->count() * sizeof(SkPMColor)); - ctable->unlockColors(false); + ctable->unlockColors(); // always skip a full 256 number of entries, even if we memcpy'd fewer dst += kGrColorTableSize; diff --git a/src/images/SkImageDecoder_libgif.cpp b/src/images/SkImageDecoder_libgif.cpp index 08f37efced..ab0fbdaf3f 100644 --- a/src/images/SkImageDecoder_libgif.cpp +++ b/src/images/SkImageDecoder_libgif.cpp @@ -248,10 +248,13 @@ bool SkGIFImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* bm, Mode mode) { if (NULL == cmap) { return error_return(gif, *bm, "null cmap"); } - colorCount = cmap->ColorCount; - SkAutoTMalloc<SkPMColor> colorStorage(colorCount); - SkPMColor* colorPtr = colorStorage.get(); + if (colorCount > 256) { + colorCount = 256; // our kIndex8 can't support more + } + + SkPMColor colorPtr[256]; // storage for worst-case + SkAlphaType alphaType = kOpaque_SkAlphaType; for (int index = 0; index < colorCount; index++) { colorPtr[index] = SkPackARGB32(0xFF, cmap->Colors[index].Red, @@ -263,10 +266,12 @@ bool SkGIFImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* bm, Mode mode) { bool reallyHasAlpha = transpIndex >= 0; if (reallyHasAlpha) { colorPtr[transpIndex] = SK_ColorTRANSPARENT; // ram in a transparent SkPMColor + alphaType = kPremul_SkAlphaType; } - SkAutoTUnref<SkColorTable> ctable(SkNEW_ARGS(SkColorTable, (colorPtr, colorCount))); - ctable->setIsOpaque(!reallyHasAlpha); + SkAutoTUnref<SkColorTable> ctable(SkNEW_ARGS(SkColorTable, + (colorPtr, colorCount, + alphaType))); if (!this->allocPixelRef(bm, ctable)) { return error_return(gif, *bm, "allocPixelRef"); } diff --git a/src/images/SkImageDecoder_libpng.cpp b/src/images/SkImageDecoder_libpng.cpp index 4e4106aede..951083b0f4 100644 --- a/src/images/SkImageDecoder_libpng.cpp +++ b/src/images/SkImageDecoder_libpng.cpp @@ -648,8 +648,6 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, png_colorp palette; png_bytep trans; int numTrans; - bool reallyHasAlpha = false; - SkColorTable* colorTable = NULL; png_get_PLTE(png_ptr, info_ptr, &palette, &numPalette); @@ -660,17 +658,16 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, the colortable by 1 (if its < 256) and duplicate the last color into that slot. */ int colorCount = numPalette + (numPalette < 256); + SkPMColor colorStorage[256]; // worst-case storage + SkPMColor* colorPtr = colorStorage; - colorTable = SkNEW_ARGS(SkColorTable, (colorCount)); - - SkPMColor* colorPtr = colorTable->lockColors(); if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) { png_get_tRNS(png_ptr, info_ptr, &trans, &numTrans, NULL); *hasAlphap = (numTrans > 0); } else { numTrans = 0; - colorTable->setFlags(colorTable->getFlags() | SkColorTable::kColorsAreOpaque_Flag); } + // check for bad images that might make us crash if (numTrans > numPalette) { numTrans = numPalette; @@ -692,7 +689,7 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, *colorPtr++ = proc(*trans++, palette->red, palette->green, palette->blue); palette++; } - reallyHasAlpha |= (transLessThanFF < 0); + bool reallyHasAlpha = (transLessThanFF < 0); for (; index < numPalette; index++) { *colorPtr++ = SkPackARGB32(0xFF, palette->red, palette->green, palette->blue); @@ -703,8 +700,18 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, if (numPalette < 256) { *colorPtr = colorPtr[-1]; } - colorTable->unlockColors(true); - *colorTablep = colorTable; + + SkAlphaType alphaType = kOpaque_SkAlphaType; + if (reallyHasAlpha) { + if (this->getRequireUnpremultipliedColors()) { + alphaType = kUnpremul_SkAlphaType; + } else { + alphaType = kPremul_SkAlphaType; + } + } + + *colorTablep = SkNEW_ARGS(SkColorTable, + (colorStorage, colorCount, alphaType)); *reallyHasAlphap = reallyHasAlpha; return true; } diff --git a/src/opts/SkBitmapProcState_opts_arm.cpp b/src/opts/SkBitmapProcState_opts_arm.cpp index 3a3cb8567c..badb0f4d3b 100644 --- a/src/opts/SkBitmapProcState_opts_arm.cpp +++ b/src/opts/SkBitmapProcState_opts_arm.cpp @@ -184,7 +184,7 @@ void SI8_opaque_D32_nofilter_DX_arm(const SkBitmapProcState& s, ); } - s.fBitmap->getColorTable()->unlockColors(false); + s.fBitmap->getColorTable()->unlockColors(); } #endif // SK_ARM_ARCH >= 6 && !defined(SK_CPU_BENDIAN) diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp index 88ce577f68..38453c89b6 100644 --- a/tests/BitmapCopyTest.cpp +++ b/tests/BitmapCopyTest.cpp @@ -32,47 +32,30 @@ static bool canHaveAlpha(SkBitmap::Config config) { } // copyTo() should preserve isOpaque when it makes sense -static void test_isOpaque(skiatest::Reporter* reporter, const SkBitmap& src, +static void test_isOpaque(skiatest::Reporter* reporter, + const SkBitmap& srcOpaque, const SkBitmap& srcPremul, SkBitmap::Config dstConfig) { - SkBitmap bitmap(src); SkBitmap dst; - // we need the lock so that we get a valid colorTable (when available) - SkAutoLockPixels alp(bitmap); - SkColorTable* ctable = bitmap.getColorTable(); - unsigned ctableFlags = ctable ? ctable->getFlags() : 0; - - if (canHaveAlpha(bitmap.config()) && canHaveAlpha(dstConfig)) { - bitmap.setIsOpaque(false); - if (ctable) { - ctable->setFlags(ctableFlags & ~SkColorTable::kColorsAreOpaque_Flag); - } - REPORTER_ASSERT(reporter, bitmap.copyTo(&dst, dstConfig)); + if (canHaveAlpha(srcPremul.config()) && canHaveAlpha(dstConfig)) { + REPORTER_ASSERT(reporter, srcPremul.copyTo(&dst, dstConfig)); REPORTER_ASSERT(reporter, dst.config() == dstConfig); - if (bitmap.isOpaque() != dst.isOpaque()) { - report_opaqueness(reporter, bitmap, dst); + if (srcPremul.isOpaque() != dst.isOpaque()) { + report_opaqueness(reporter, srcPremul, dst); } } - bitmap.setIsOpaque(true); - if (ctable) { - ctable->setFlags(ctableFlags | SkColorTable::kColorsAreOpaque_Flag); - } - REPORTER_ASSERT(reporter, bitmap.copyTo(&dst, dstConfig)); + REPORTER_ASSERT(reporter, srcOpaque.copyTo(&dst, dstConfig)); REPORTER_ASSERT(reporter, dst.config() == dstConfig); - if (bitmap.isOpaque() != dst.isOpaque()) { - report_opaqueness(reporter, bitmap, dst); - } - - if (ctable) { - ctable->setFlags(ctableFlags); + if (srcOpaque.isOpaque() != dst.isOpaque()) { + report_opaqueness(reporter, srcOpaque, dst); } } -static void init_src(const SkBitmap& bitmap, const SkColorTable* ct) { +static void init_src(const SkBitmap& bitmap) { SkAutoLockPixels lock(bitmap); if (bitmap.getPixels()) { - if (ct) { + if (bitmap.getColorTable()) { sk_bzero(bitmap.getPixels(), bitmap.getSize()); } else { bitmap.eraseColor(SK_ColorWHITE); @@ -80,11 +63,11 @@ static void init_src(const SkBitmap& bitmap, const SkColorTable* ct) { } } -static SkColorTable* init_ctable() { +static SkColorTable* init_ctable(SkAlphaType alphaType) { static const SkColor colors[] = { SK_ColorBLACK, SK_ColorRED, SK_ColorGREEN, SK_ColorBLUE, SK_ColorWHITE }; - return new SkColorTable(colors, SK_ARRAY_COUNT(colors)); + return new SkColorTable(colors, SK_ARRAY_COUNT(colors), alphaType); } struct Pair { @@ -256,18 +239,30 @@ static void TestBitmapCopy(skiatest::Reporter* reporter) { for (size_t i = 0; i < SK_ARRAY_COUNT(gPairs); i++) { for (size_t j = 0; j < SK_ARRAY_COUNT(gPairs); j++) { - SkBitmap src, dst; - SkColorTable* ct = NULL; - - src.setConfig(gPairs[i].fConfig, W, H); - if (SkBitmap::kIndex8_Config == src.config()) { - ct = init_ctable(); + SkBitmap srcOpaque, srcPremul, dst; + + { + SkColorTable* ctOpaque = NULL; + SkColorTable* ctPremul = NULL; + + srcOpaque.setConfig(gPairs[i].fConfig, W, H); + srcPremul.setConfig(gPairs[i].fConfig, W, H); + if (SkBitmap::kIndex8_Config == gPairs[i].fConfig) { + ctOpaque = init_ctable(kOpaque_SkAlphaType); + ctPremul = init_ctable(kPremul_SkAlphaType); + } + srcOpaque.allocPixels(ctOpaque); + srcPremul.allocPixels(ctPremul); + SkSafeUnref(ctOpaque); + SkSafeUnref(ctPremul); + + srcOpaque.setIsOpaque(true); + srcPremul.setIsOpaque(false); } - src.allocPixels(ct); - SkSafeUnref(ct); + init_src(srcOpaque); + init_src(srcPremul); - init_src(src, ct); - bool success = src.copyTo(&dst, gPairs[j].fConfig); + bool success = srcPremul.copyTo(&dst, gPairs[j].fConfig); bool expected = gPairs[i].fValid[j] != '0'; if (success != expected) { SkString str; @@ -277,7 +272,7 @@ static void TestBitmapCopy(skiatest::Reporter* reporter) { reporter->reportFailed(str); } - bool canSucceed = src.canCopyTo(gPairs[j].fConfig); + bool canSucceed = srcPremul.canCopyTo(gPairs[j].fConfig); if (success != canSucceed) { SkString str; str.printf("SkBitmap::copyTo from %s to %s. returned %s canCopyTo %s", @@ -287,31 +282,30 @@ static void TestBitmapCopy(skiatest::Reporter* reporter) { } if (success) { - REPORTER_ASSERT(reporter, src.width() == dst.width()); - REPORTER_ASSERT(reporter, src.height() == dst.height()); + REPORTER_ASSERT(reporter, srcPremul.width() == dst.width()); + REPORTER_ASSERT(reporter, srcPremul.height() == dst.height()); REPORTER_ASSERT(reporter, dst.config() == gPairs[j].fConfig); - test_isOpaque(reporter, src, dst.config()); - if (src.config() == dst.config()) { - SkAutoLockPixels srcLock(src); + test_isOpaque(reporter, srcOpaque, srcPremul, dst.config()); + if (srcPremul.config() == dst.config()) { + SkAutoLockPixels srcLock(srcPremul); SkAutoLockPixels dstLock(dst); - REPORTER_ASSERT(reporter, src.readyToDraw()); + REPORTER_ASSERT(reporter, srcPremul.readyToDraw()); REPORTER_ASSERT(reporter, dst.readyToDraw()); - const char* srcP = (const char*)src.getAddr(0, 0); + const char* srcP = (const char*)srcPremul.getAddr(0, 0); const char* dstP = (const char*)dst.getAddr(0, 0); REPORTER_ASSERT(reporter, srcP != dstP); REPORTER_ASSERT(reporter, !memcmp(srcP, dstP, - src.getSize())); - REPORTER_ASSERT(reporter, src.getGenerationID() == dst.getGenerationID()); + srcPremul.getSize())); + REPORTER_ASSERT(reporter, srcPremul.getGenerationID() == dst.getGenerationID()); } else { - REPORTER_ASSERT(reporter, src.getGenerationID() != dst.getGenerationID()); + REPORTER_ASSERT(reporter, srcPremul.getGenerationID() != dst.getGenerationID()); } // test extractSubset { - SkBitmap bitmap(src); + SkBitmap bitmap(srcOpaque); SkBitmap subset; SkIRect r; r.set(1, 1, 2, 2); - bitmap.setIsOpaque(true); bitmap.setIsVolatile(true); if (bitmap.extractSubset(&subset, r)) { REPORTER_ASSERT(reporter, subset.width() == 1); @@ -335,7 +329,8 @@ static void TestBitmapCopy(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, (copy.getColorTable() != NULL) == hasCT); } - bitmap.setIsOpaque(false); + + bitmap = srcPremul; bitmap.setIsVolatile(false); if (bitmap.extractSubset(&subset, r)) { REPORTER_ASSERT(reporter, @@ -441,7 +436,7 @@ static void TestBitmapCopy(skiatest::Reporter* reporter) { src.setConfig(gPairs[i].fConfig, subW, subH); } if (SkBitmap::kIndex8_Config == src.config()) { - ct = init_ctable(); + ct = init_ctable(kPremul_SkAlphaType); } src.allocPixels(ct); |