diff --git a/include/core/SkPackBits.h b/include/core/SkPackBits.h index f0614a0843..1e32ee0875 100644 --- a/include/core/SkPackBits.h +++ b/include/core/SkPackBits.h @@ -14,66 +14,33 @@ class SkPackBits { public: - /** Given the number of 16bit values that will be passed to Pack16, - returns the worst-case size needed for the dst[] buffer. - */ - static size_t ComputeMaxSize16(int count); - /** Given the number of 8bit values that will be passed to Pack8, returns the worst-case size needed for the dst[] buffer. */ static size_t ComputeMaxSize8(int count); - /** Write the src array into a packed format. The packing process may end - up writing more bytes than it read, so dst[] must be large enough. - @param src Input array of 16bit values - @param count Number of entries in src[] - @param dst Buffer (allocated by caller) to write the packed data - into - @return the number of bytes written to dst[] - */ - static size_t Pack16(const uint16_t src[], int count, uint8_t dst[]); - /** Write the src array into a packed format. The packing process may end up writing more bytes than it read, so dst[] must be large enough. @param src Input array of 8bit values - @param count Number of entries in src[] + @param srcSize Number of entries in src[] @param dst Buffer (allocated by caller) to write the packed data into + @param dstSize Number of bytes in the output buffer. @return the number of bytes written to dst[] */ - static size_t Pack8(const uint8_t src[], int count, uint8_t dst[]); - - /** Unpack the data in src[], and expand it into dst[]. The src[] data was - written by a previous call to Pack16. - @param src Input data to unpack, previously created by Pack16. - @param srcSize Number of bytes of src to unpack - @param dst Buffer (allocated by caller) to expand the src[] into. - @return the number of dst elements (not bytes) written into dst. - */ - static int Unpack16(const uint8_t src[], size_t srcSize, uint16_t dst[]); + static size_t Pack8(const uint8_t src[], size_t srcSize, uint8_t dst[], + size_t dstSize); /** Unpack the data in src[], and expand it into dst[]. The src[] data was written by a previous call to Pack8. @param src Input data to unpack, previously created by Pack8. @param srcSize Number of bytes of src to unpack @param dst Buffer (allocated by caller) to expand the src[] into. + @param dstSize Number of bytes in the output buffer. @return the number of bytes written into dst. */ - static int Unpack8(const uint8_t src[], size_t srcSize, uint8_t dst[]); - - /** Unpack the data from src[], skip the first dstSkip bytes, then write - dstWrite bytes into dst[]. The src[] data was written by a previous - call to Pack8. Return the number of bytes actually writtten into dst[] - @param src Input data to unpack, previously created by Pack8. - @param dst Buffer (allocated by caller) to expand the src[] into. - @param dstSkip Number of bytes of unpacked src to skip before writing - into dst - @param dstWrite Number of bytes of unpacked src to write into dst (after - skipping dstSkip bytes) - */ - static void Unpack8(uint8_t dst[], size_t dstSkip, size_t dstWrite, - const uint8_t src[]); + static int Unpack8(const uint8_t src[], size_t srcSize, uint8_t dst[], + size_t dstSize); }; #endif diff --git a/src/core/SkPackBits.cpp b/src/core/SkPackBits.cpp index 3c6197b6f2..a3424e2bdc 100644 --- a/src/core/SkPackBits.cpp +++ b/src/core/SkPackBits.cpp @@ -1,4 +1,3 @@ - /* * Copyright 2011 Google Inc. * @@ -7,150 +6,14 @@ */ #include "SkPackBits.h" -#define GATHER_STATSx - -static inline void small_memcpy(void* SK_RESTRICT dst, - const void* SK_RESTRICT src, size_t n) { - SkASSERT(n > 0 && n <= 15); - uint8_t* d = (uint8_t*)dst; - const uint8_t* s = (const uint8_t*)src; - switch (n) { - case 15: *d++ = *s++; - case 14: *d++ = *s++; - case 13: *d++ = *s++; - case 12: *d++ = *s++; - case 11: *d++ = *s++; - case 10: *d++ = *s++; - case 9: *d++ = *s++; - case 8: *d++ = *s++; - case 7: *d++ = *s++; - case 6: *d++ = *s++; - case 5: *d++ = *s++; - case 4: *d++ = *s++; - case 3: *d++ = *s++; - case 2: *d++ = *s++; - case 1: *d++ = *s++; - case 0: break; - } -} - -static inline void small_memset(void* dst, uint8_t value, size_t n) { - SkASSERT(n > 0 && n <= 15); - uint8_t* d = (uint8_t*)dst; - switch (n) { - case 15: *d++ = value; - case 14: *d++ = value; - case 13: *d++ = value; - case 12: *d++ = value; - case 11: *d++ = value; - case 10: *d++ = value; - case 9: *d++ = value; - case 8: *d++ = value; - case 7: *d++ = value; - case 6: *d++ = value; - case 5: *d++ = value; - case 4: *d++ = value; - case 3: *d++ = value; - case 2: *d++ = value; - case 1: *d++ = value; - case 0: break; - } -} - -// can we do better for small counts with our own inlined memcpy/memset? - -#define PB_MEMSET(addr, value, count) \ -do { \ -if ((count) > 15) { \ -memset(addr, value, count); \ -} else { \ -small_memset(addr, value, count); \ -} \ -} while (0) - -#define PB_MEMCPY(dst, src, count) \ -do { \ - if ((count) > 15) { \ - memcpy(dst, src, count); \ - } else { \ - small_memcpy(dst, src, count); \ - } \ -} while (0) - -/////////////////////////////////////////////////////////////////////////////// - -#ifdef GATHER_STATS - static int gMemSetBuckets[129]; - static int gMemCpyBuckets[129]; - static int gCounter; - -static void register_memset_count(int n) { - SkASSERT((unsigned)n <= 128); - gMemSetBuckets[n] += 1; - gCounter += 1; - - if ((gCounter & 0xFF) == 0) { - SkDebugf("----- packbits memset stats: "); - for (size_t i = 0; i < SK_ARRAY_COUNT(gMemSetBuckets); i++) { - if (gMemSetBuckets[i]) { - SkDebugf(" %d:%d", i, gMemSetBuckets[i]); - } - } - } -} -static void register_memcpy_count(int n) { - SkASSERT((unsigned)n <= 128); - gMemCpyBuckets[n] += 1; - gCounter += 1; - - if ((gCounter & 0x1FF) == 0) { - SkDebugf("----- packbits memcpy stats: "); - for (size_t i = 0; i < SK_ARRAY_COUNT(gMemCpyBuckets); i++) { - if (gMemCpyBuckets[i]) { - SkDebugf(" %d:%d", i, gMemCpyBuckets[i]); - } - } - } -} -#else -#define register_memset_count(n) -#define register_memcpy_count(n) -#endif - - -/////////////////////////////////////////////////////////////////////////////// - -size_t SkPackBits::ComputeMaxSize16(int count) { - // worst case is the number of 16bit values (times 2) + - // 1 byte per (up to) 128 entries. - return ((count + 127) >> 7) + (count << 1); -} - size_t SkPackBits::ComputeMaxSize8(int count) { // worst case is the number of 8bit values + 1 byte per (up to) 128 entries. return ((count + 127) >> 7) + count; } -static uint8_t* flush_same16(uint8_t dst[], uint16_t value, int count) { +static uint8_t* flush_same8(uint8_t dst[], uint8_t value, size_t count) { while (count > 0) { - int n = count; - if (n > 128) { - n = 128; - } - *dst++ = (uint8_t)(n - 1); - *dst++ = (uint8_t)(value >> 8); - *dst++ = (uint8_t)value; - count -= n; - } - return dst; -} - -static uint8_t* flush_same8(uint8_t dst[], uint8_t value, int count) { - while (count > 0) { - int n = count; - if (n > 128) { - n = 128; - } + int n = count > 128 ? 128 : count; *dst++ = (uint8_t)(n - 1); *dst++ = (uint8_t)value; count -= n; @@ -158,31 +21,12 @@ static uint8_t* flush_same8(uint8_t dst[], uint8_t value, int count) { return dst; } -static uint8_t* flush_diff16(uint8_t* SK_RESTRICT dst, - const uint16_t* SK_RESTRICT src, int count) { - while (count > 0) { - int n = count; - if (n > 128) { - n = 128; - } - *dst++ = (uint8_t)(n + 127); - PB_MEMCPY(dst, src, n * sizeof(uint16_t)); - src += n; - dst += n * sizeof(uint16_t); - count -= n; - } - return dst; -} - static uint8_t* flush_diff8(uint8_t* SK_RESTRICT dst, - const uint8_t* SK_RESTRICT src, int count) { + const uint8_t* SK_RESTRICT src, size_t count) { while (count > 0) { - int n = count; - if (n > 128) { - n = 128; - } + int n = count > 128 ? 128 : count; *dst++ = (uint8_t)(n + 127); - PB_MEMCPY(dst, src, n); + memcpy(dst, src, n); src += n; dst += n; count -= n; @@ -190,64 +34,20 @@ static uint8_t* flush_diff8(uint8_t* SK_RESTRICT dst, return dst; } -size_t SkPackBits::Pack16(const uint16_t* SK_RESTRICT src, int count, - uint8_t* SK_RESTRICT dst) { - uint8_t* origDst = dst; - const uint16_t* stop = src + count; - - for (;;) { - count = SkToInt(stop - src); - SkASSERT(count >= 0); - if (count == 0) { - return dst - origDst; - } - if (1 == count) { - *dst++ = 0; - *dst++ = (uint8_t)(*src >> 8); - *dst++ = (uint8_t)*src; - return dst - origDst; - } - - unsigned value = *src; - const uint16_t* s = src + 1; - - if (*s == value) { // accumulate same values... - do { - s++; - if (s == stop) { - break; - } - } while (*s == value); - dst = flush_same16(dst, value, SkToInt(s - src)); - } else { // accumulate diff values... - do { - if (++s == stop) { - goto FLUSH_DIFF; - } - } while (*s != s[-1]); - s -= 1; // back up so we don't grab one of the "same" values that follow - FLUSH_DIFF: - dst = flush_diff16(dst, src, SkToInt(s - src)); - } - src = s; +size_t SkPackBits::Pack8(const uint8_t* SK_RESTRICT src, size_t srcSize, + uint8_t* SK_RESTRICT dst, size_t dstSize) { + if (dstSize < ComputeMaxSize8(srcSize)) { + return 0; } -} -size_t SkPackBits::Pack8(const uint8_t* SK_RESTRICT src, int count, - uint8_t* SK_RESTRICT dst) { - uint8_t* origDst = dst; - const uint8_t* stop = src + count; + uint8_t* const origDst = dst; + const uint8_t* stop = src + srcSize; - for (;;) { - count = SkToInt(stop - src); - SkASSERT(count >= 0); - if (count == 0) { - return dst - origDst; - } + for (intptr_t count = stop - src; count > 0; count = stop - src) { if (1 == count) { *dst++ = 0; *dst++ = *src; - return dst - origDst; + break; } unsigned value = *src; @@ -275,137 +75,35 @@ size_t SkPackBits::Pack8(const uint8_t* SK_RESTRICT src, int count, } src = s; } + return dst - origDst; } #include "SkUtils.h" -int SkPackBits::Unpack16(const uint8_t* SK_RESTRICT src, size_t srcSize, - uint16_t* SK_RESTRICT dst) { - uint16_t* origDst = dst; - const uint8_t* stop = src + srcSize; - - while (src < stop) { - unsigned n = *src++; - if (n <= 127) { // repeat count (n + 1) - n += 1; - sk_memset16(dst, (src[0] << 8) | src[1], n); - src += 2; - } else { // same count (n - 127) - n -= 127; - PB_MEMCPY(dst, src, n * sizeof(uint16_t)); - src += n * sizeof(uint16_t); - } - dst += n; - } - SkASSERT(src == stop); - return SkToInt(dst - origDst); -} - int SkPackBits::Unpack8(const uint8_t* SK_RESTRICT src, size_t srcSize, - uint8_t* SK_RESTRICT dst) { - uint8_t* origDst = dst; + uint8_t* SK_RESTRICT dst, size_t dstSize) { + uint8_t* const origDst = dst; + uint8_t* const endDst = dst + dstSize; const uint8_t* stop = src + srcSize; while (src < stop) { unsigned n = *src++; if (n <= 127) { // repeat count (n + 1) n += 1; - PB_MEMSET(dst, *src++, n); + if (dst >(endDst - n)) { + return 0; + } + memset(dst, *src++, n); } else { // same count (n - 127) n -= 127; - PB_MEMCPY(dst, src, n); + if (dst > (endDst - n)) { + return 0; + } + memcpy(dst, src, n); src += n; } dst += n; } - SkASSERT(src == stop); + SkASSERT(src <= stop); return SkToInt(dst - origDst); } - -enum UnpackState { - CLEAN_STATE, - REPEAT_BYTE_STATE, - COPY_SRC_STATE -}; - -void SkPackBits::Unpack8(uint8_t* SK_RESTRICT dst, size_t dstSkip, - size_t dstWrite, const uint8_t* SK_RESTRICT src) { - if (dstWrite == 0) { - return; - } - - UnpackState state = CLEAN_STATE; - size_t stateCount = 0; - - // state 1: do the skip-loop - while (dstSkip > 0) { - size_t n = *src++; - if (n <= 127) { // repeat count (n + 1) - n += 1; - if (n > dstSkip) { - state = REPEAT_BYTE_STATE; - stateCount = n - dstSkip; - n = dstSkip; - // we don't increment src here, since its needed in stage 2 - } else { - src++; // skip the src byte - } - } else { // same count (n - 127) - n -= 127; - if (n > dstSkip) { - state = COPY_SRC_STATE; - stateCount = n - dstSkip; - n = dstSkip; - } - src += n; - } - dstSkip -= n; - } - - // stage 2: perform any catchup from the skip-stage - if (stateCount > dstWrite) { - stateCount = dstWrite; - } - switch (state) { - case REPEAT_BYTE_STATE: - SkASSERT(stateCount > 0); - register_memset_count(stateCount); - PB_MEMSET(dst, *src++, stateCount); - break; - case COPY_SRC_STATE: - SkASSERT(stateCount > 0); - register_memcpy_count(stateCount); - PB_MEMCPY(dst, src, stateCount); - src += stateCount; - break; - default: - SkASSERT(stateCount == 0); - break; - } - dst += stateCount; - dstWrite -= stateCount; - - // copy at most dstWrite bytes into dst[] - while (dstWrite > 0) { - size_t n = *src++; - if (n <= 127) { // repeat count (n + 1) - n += 1; - if (n > dstWrite) { - n = dstWrite; - } - register_memset_count(n); - PB_MEMSET(dst, *src++, n); - } else { // same count (n - 127) - n -= 127; - if (n > dstWrite) { - n = dstWrite; - } - register_memcpy_count(n); - PB_MEMCPY(dst, src, n); - src += n; - } - dst += n; - dstWrite -= n; - } - SkASSERT(0 == dstWrite); -} diff --git a/src/effects/SkTableColorFilter.cpp b/src/effects/SkTableColorFilter.cpp index 787126807e..27a5def5e5 100644 --- a/src/effects/SkTableColorFilter.cpp +++ b/src/effects/SkTableColorFilter.cpp @@ -201,8 +201,8 @@ static const uint8_t gCountNibBits[] = { void SkTable_ColorFilter::flatten(SkWriteBuffer& buffer) const { uint8_t storage[5*256]; int count = gCountNibBits[fFlags & 0xF]; - size_t size = SkPackBits::Pack8(fStorage, count * 256, storage); - SkASSERT(size <= sizeof(storage)); + size_t size = SkPackBits::Pack8(fStorage, count * 256, storage, + sizeof(storage)); buffer.write32(fFlags); buffer.writeByteArray(storage, size); @@ -223,7 +223,8 @@ SkFlattenable* SkTable_ColorFilter::CreateProc(SkReadBuffer& buffer) { } uint8_t unpackedStorage[4*256]; - size_t unpackedSize = SkPackBits::Unpack8(packedStorage, packedSize, unpackedStorage); + size_t unpackedSize = SkPackBits::Unpack8(packedStorage, packedSize, + unpackedStorage, sizeof(unpackedStorage)); // now check that we got the size we expected if (!buffer.validate(unpackedSize == count*256)) { return NULL; diff --git a/tests/PackBitsTest.cpp b/tests/PackBitsTest.cpp index ce4e8be467..ac9a0aedfb 100644 --- a/tests/PackBitsTest.cpp +++ b/tests/PackBitsTest.cpp @@ -8,57 +8,8 @@ #include "SkPackBits.h" #include "Test.h" -static const uint16_t gTest0[] = { 0, 0, 1, 1 }; -static const uint16_t gTest1[] = { 1, 2, 3, 4, 5, 6 }; -static const uint16_t gTest2[] = { 0, 0, 0, 1, 2, 3, 3, 3 }; -static const uint16_t gTest3[] = { 0, 0, 0, 0, 0, 0, 1, 2, 3, 3, 3, 0, 0, 1 }; - #include "SkRandom.h" static SkRandom gRand; -static void rand_fill(uint16_t buffer[], int count) { - for (int i = 0; i < count; i++) - buffer[i] = (uint16_t)gRand.nextU(); -} - -static void test_pack16(skiatest::Reporter* reporter) { - static const struct { - const uint16_t* fSrc; - int fCount; - } gTests[] = { - { gTest0, SK_ARRAY_COUNT(gTest0) }, - { gTest1, SK_ARRAY_COUNT(gTest1) }, - { gTest2, SK_ARRAY_COUNT(gTest2) }, - { gTest3, SK_ARRAY_COUNT(gTest3) } - }; - - for (size_t i = 0; i < SK_ARRAY_COUNT(gTests); i++) { - uint8_t dst[100]; - size_t dstSize = SkPackBits::Pack16(gTests[i].fSrc, - gTests[i].fCount, dst); - uint16_t src[100]; - int srcCount = SkPackBits::Unpack16(dst, dstSize, src); - bool match = gTests[i].fCount == srcCount && memcmp(gTests[i].fSrc, src, - gTests[i].fCount * sizeof(uint16_t)) == 0; - REPORTER_ASSERT(reporter, match); - } - - for (int n = 1000; n; n--) { - int size = 50; - uint16_t src[100], src2[100]; - uint8_t dst[200]; - rand_fill(src, size); - - size_t dstSize = SkPackBits::Pack16(src, size, dst); - size_t maxSize = SkPackBits::ComputeMaxSize16(size); - REPORTER_ASSERT(reporter, maxSize >= dstSize); - - int srcCount = SkPackBits::Unpack16(dst, dstSize, src2); - REPORTER_ASSERT(reporter, size == srcCount); - bool match = memcmp(src, src2, size * sizeof(uint16_t)) == 0; - REPORTER_ASSERT(reporter, match); - } -} - static const uint8_t gTest80[] = { 0, 0, 1, 1 }; static const uint8_t gTest81[] = { 1, 2, 3, 4, 5, 6 }; static const uint8_t gTest82[] = { 0, 0, 0, 1, 2, 3, 3, 3 }; @@ -86,10 +37,15 @@ static void test_pack8(skiatest::Reporter* reporter) { uint8_t dst[100]; size_t maxSize = SkPackBits::ComputeMaxSize8(gTests[i].fCount); size_t dstSize = SkPackBits::Pack8(gTests[i].fSrc, - gTests[i].fCount, dst); + gTests[i].fCount, dst, maxSize - 1); + REPORTER_ASSERT(reporter, dstSize == 0); + dstSize = SkPackBits::Pack8(gTests[i].fSrc, + gTests[i].fCount, dst, sizeof(dst)); REPORTER_ASSERT(reporter, dstSize <= maxSize); uint8_t src[100]; - int srcCount = SkPackBits::Unpack8(dst, dstSize, src); + int srcCount = SkPackBits::Unpack8(dst, dstSize, src, gTests[i].fCount - 1); + REPORTER_ASSERT(reporter, srcCount == 0); + srcCount = SkPackBits::Unpack8(dst, dstSize, src, sizeof(src)); bool match = gTests[i].fCount == srcCount && memcmp(gTests[i].fSrc, src, gTests[i].fCount * sizeof(uint8_t)) == 0; @@ -102,30 +58,18 @@ static void test_pack8(skiatest::Reporter* reporter) { uint8_t dst[600]; rand_fill(src, size); - size_t dstSize = SkPackBits::Pack8(src, size, dst); + size_t dstSize = SkPackBits::Pack8(src, size, dst, sizeof(dst)); size_t maxSize = SkPackBits::ComputeMaxSize8(size); REPORTER_ASSERT(reporter, maxSize >= dstSize); - size_t srcCount = SkPackBits::Unpack8(dst, dstSize, src2); + size_t srcCount = SkPackBits::Unpack8(dst, dstSize, src2, size); REPORTER_ASSERT(reporter, size == srcCount); bool match = memcmp(src, src2, size * sizeof(uint8_t)) == 0; REPORTER_ASSERT(reporter, match); - - for (int j = 0; j < 100; j++) { - uint32_t skip = gRand.nextU() % size; - uint32_t write = gRand.nextU() % size; - if (skip + write > size) { - write = size - skip; - } - SkPackBits::Unpack8(src, skip, write, dst); - bool match = memcmp(src, src2 + skip, write) == 0; - REPORTER_ASSERT(reporter, match); - } } } } DEF_TEST(PackBits, reporter) { test_pack8(reporter); - test_pack16(reporter); }