From: msarett Date: Thu, 9 Apr 2015 19:43:10 +0000 (-0700) Subject: ***Disables swizzles to 565. X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~2822 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=438b2adefb9e9213e0ddaf0609405d3087a1cf0a;p=platform%2Fupstream%2FlibSkiaSharp.git ***Disables swizzles to 565. We may want to enable swizzles to 565 for images that are encoded in a format similar to 565, however, we do not want to take images that decode naturally to kN32 and then convert them to 565. ***Enable swizzles to kIndex_8. For images encoded in a color table format, we suggest that they be decoded to kIndex_8. When we decode, we only allow conversion to kIndex_8 if it matches the suggested color type (except wbmp which seems good as is). ***Modify dm to test images that decode to kIndex_8. BUG=skia:3257 BUG=skia:3440 Review URL: https://codereview.chromium.org/1055743003 --- diff --git a/dm/DM.cpp b/dm/DM.cpp index 296b7c8..2a71500 100644 --- a/dm/DM.cpp +++ b/dm/DM.cpp @@ -178,6 +178,46 @@ static void push_src(const char* tag, const char* options, Src* s) { } } +static void push_codec_srcs(Path path) { + SkAutoTUnref encoded(SkData::NewFromFileName(path.c_str())); + if (!encoded) { + SkDebugf("Couldn't read %s.", path.c_str()); + return; + } + SkAutoTDelete codec(SkCodec::NewFromData(encoded)); + if (NULL == codec.get()) { + SkDebugf("Couldn't create codec for %s.", path.c_str()); + return; + } + + // Build additional test cases for images that decode natively to non-canvas types + switch(codec->getInfo().colorType()) { + case kGray_8_SkColorType: + push_src("image", "codec kGray8", new CodecSrc(path, CodecSrc::kNormal_Mode, + CodecSrc::kGrayscale_Always_DstColorType)); + push_src("image", "scanline kGray8", new CodecSrc(path, CodecSrc::kScanline_Mode, + CodecSrc::kGrayscale_Always_DstColorType)); + // Intentional fall through + // FIXME: Is this a long term solution for testing wbmps decodes to kIndex8? + // Further discussion on this topic is at skbug.com/3683 + case kIndex_8_SkColorType: + push_src("image", "codec kIndex8", new CodecSrc(path, CodecSrc::kNormal_Mode, + CodecSrc::kIndex8_Always_DstColorType)); + push_src("image", "scanline kIndex8", new CodecSrc(path, CodecSrc::kScanline_Mode, + CodecSrc::kIndex8_Always_DstColorType)); + break; + default: + // Do nothing + break; + } + + // Decode all images to the canvas color type + push_src("image", "codec", new CodecSrc(path, CodecSrc::kNormal_Mode, + CodecSrc::kGetFromCanvas_DstColorType)); + push_src("image", "scanline", new CodecSrc(path, CodecSrc::kScanline_Mode, + CodecSrc::kGetFromCanvas_DstColorType)); +} + static bool codec_supported(const char* ext) { // FIXME: Once other versions of SkCodec are available, we can add them to this // list (and eventually we can remove this check once they are all supported). @@ -223,8 +263,7 @@ static void gather_srcs() { push_src("image", "decode", new ImageSrc(path)); // Decode entire image push_src("image", "subset", new ImageSrc(path, 2)); // Decode into 2x2 subsets if (codec_supported(exts[j])) { - push_src("image", "codec", new CodecSrc(path, CodecSrc::kNormal_Mode)); - push_src("image", "scanline", new CodecSrc(path, CodecSrc::kScanline_Mode)); + push_codec_srcs(path); } } } @@ -232,8 +271,7 @@ static void gather_srcs() { // assume that FLAGS_images[i] is a valid image if it is a file. push_src("image", "decode", new ImageSrc(flag)); // Decode entire image. push_src("image", "subset", new ImageSrc(flag, 2)); // Decode into 2 x 2 subsets - push_src("image", "codec", new CodecSrc(flag, CodecSrc::kNormal_Mode)); - push_src("image", "scanline", new CodecSrc(flag, CodecSrc::kScanline_Mode)); + push_codec_srcs(flag); } } } diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index de9ee76..9e9a77c 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -52,7 +52,11 @@ Name GMSrc::name() const { /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ -CodecSrc::CodecSrc(Path path, Mode mode) : fPath(path), fMode(mode) {} +CodecSrc::CodecSrc(Path path, Mode mode, DstColorType dstColorType) + : fPath(path) + , fMode(mode) + , fDstColorType(dstColorType) +{} Error CodecSrc::draw(SkCanvas* canvas) const { SkImageInfo canvasInfo; @@ -66,27 +70,53 @@ Error CodecSrc::draw(SkCanvas* canvas) const { if (!encoded) { return SkStringPrintf("Couldn't read %s.", fPath.c_str()); } - SkAutoTDelete codec(SkCodec::NewFromData(encoded)); - if (!codec) { - return SkStringPrintf("Couldn't decode %s.", fPath.c_str()); + if (NULL == codec.get()) { + return SkStringPrintf("Couldn't create codec for %s.", fPath.c_str()); + } + + // Choose the color type to decode to + SkImageInfo decodeInfo = codec->getInfo(); + SkColorType canvasColorType = canvasInfo.colorType(); + switch (fDstColorType) { + case kIndex8_Always_DstColorType: + case kGrayscale_Always_DstColorType: + if (kRGB_565_SkColorType == canvasColorType) { + return Error::Nonfatal("Testing non-565 to 565 is uninteresting."); + } + break; + default: + decodeInfo = decodeInfo.makeColorType(canvasColorType); + break; + } + + // Construct a color table for the decode if necessary + SkAutoTUnref colorTable(NULL); + SkPMColor* colorPtr = NULL; + int* colorCountPtr = NULL; + int maxColors = 256; + if (kIndex_8_SkColorType == decodeInfo.colorType()) { + SkPMColor colors[256]; + colorTable.reset(SkNEW_ARGS(SkColorTable, (colors, maxColors))); + colorPtr = const_cast(colorTable->readColors()); + colorCountPtr = &maxColors; } - SkImageInfo decodeInfo = codec->getInfo().makeColorType(canvasInfo.colorType()); + // FIXME: Currently we cannot draw unpremultiplied sources. if (decodeInfo.alphaType() == kUnpremul_SkAlphaType) { - // FIXME: Currently we cannot draw unpremultiplied sources. decodeInfo = decodeInfo.makeAlphaType(kPremul_SkAlphaType); } SkBitmap bitmap; - if (!bitmap.tryAllocPixels(decodeInfo)) { + if (!bitmap.tryAllocPixels(decodeInfo, NULL, colorTable.get())) { return SkStringPrintf("Image(%s) is too large (%d x %d)\n", fPath.c_str(), decodeInfo.width(), decodeInfo.height()); } switch (fMode) { case kNormal_Mode: - switch (codec->getPixels(decodeInfo, bitmap.getPixels(), bitmap.rowBytes())) { + switch (codec->getPixels(decodeInfo, bitmap.getPixels(), bitmap.rowBytes(), NULL, + colorPtr, colorCountPtr)) { case SkImageGenerator::kSuccess: // We consider incomplete to be valid, since we should still decode what is // available. diff --git a/dm/DMSrcSink.h b/dm/DMSrcSink.h index 57af911..0807cb6 100644 --- a/dm/DMSrcSink.h +++ b/dm/DMSrcSink.h @@ -13,6 +13,7 @@ #include "SkBBoxHierarchy.h" #include "SkBitmap.h" #include "SkCanvas.h" +#include "SkCodec.h" #include "SkData.h" #include "SkGPipe.h" #include "SkPicture.h" @@ -94,14 +95,20 @@ public: kNormal_Mode, kScanline_Mode, }; - CodecSrc(Path, Mode); + enum DstColorType { + kGetFromCanvas_DstColorType, + kIndex8_Always_DstColorType, + kGrayscale_Always_DstColorType, + }; + CodecSrc(Path, Mode, DstColorType); Error draw(SkCanvas*) const override; SkISize size() const override; Name name() const override; private: - Path fPath; - Mode fMode; + Path fPath; + Mode fMode; + DstColorType fDstColorType; }; diff --git a/gyp/tests.gypi b/gyp/tests.gypi index 606aed2..9a2404b 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -5,6 +5,7 @@ # Common gypi for unit tests. { 'include_dirs': [ + '../src/codec', '../src/core', '../src/effects', '../src/image', @@ -214,6 +215,7 @@ '../tests/StrokerTest.cpp', '../tests/SurfaceTest.cpp', '../tests/SVGDeviceTest.cpp', + '../tests/SwizzlerTest.cpp', '../tests/TessellatingPathRendererTests.cpp', '../tests/TArrayTest.cpp', '../tests/TemplatesTest.cpp', diff --git a/src/codec/SkCodec_libbmp.cpp b/src/codec/SkCodec_libbmp.cpp index 2b20fae..b553dea 100644 --- a/src/codec/SkCodec_libbmp.cpp +++ b/src/codec/SkCodec_libbmp.cpp @@ -9,6 +9,7 @@ #include "SkCodecPriv.h" #include "SkColorPriv.h" #include "SkStream.h" +#include "SkUtils.h" /* * @@ -23,12 +24,33 @@ static bool conversion_possible(const SkImageInfo& dst, return false; } - // Check for supported color and alpha types + // Check for supported alpha types + if (src.alphaType() != dst.alphaType()) { + if (kOpaque_SkAlphaType == src.alphaType()) { + // If the source is opaque, we must decode to opaque + return false; + } + + // The source is not opaque + switch (dst.alphaType()) { + case kPremul_SkAlphaType: + case kUnpremul_SkAlphaType: + // The source is not opaque, so either of these is okay + break; + default: + // We cannot decode a non-opaque image to opaque (or unknown) + return false; + } + } + + // Check for supported color types switch (dst.colorType()) { + // Allow output to kN32 from any type of input case kN32_SkColorType: - return src.alphaType() == dst.alphaType() || - (kPremul_SkAlphaType == dst.alphaType() && - kUnpremul_SkAlphaType == src.alphaType()); + return true; + // Allow output to kIndex_8 from compatible inputs + case kIndex_8_SkColorType: + return kIndex_8_SkColorType == src.colorType(); default: return false; } @@ -420,12 +442,17 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool isIco, SkCodec** codecOut) { } iBuffer.free(); - // Additionally, 32 bit bmp-in-icos use the alpha channel - if (isIco && 32 == bitsPerPixel) { + // Additionally, 32 bit bmp-in-icos use the alpha channel. + // And, RLE inputs may skip pixels, leaving them as transparent. This + // is uncommon, but we cannot be certain that an RLE bmp will be opaque. + if ((isIco && 32 == bitsPerPixel) || (kRLE_BitmapInputFormat == inputFormat)) { alphaType = kUnpremul_SkAlphaType; } - // Check for valid bits per pixel input + // Check for valid bits per pixel. + // At the same time, use this information to choose a suggested color type + // and to set default masks. + SkColorType colorType = kN32_SkColorType; switch (bitsPerPixel) { // In addition to more standard pixel compression formats, bmp supports // the use of bit masks to determine pixel components. The standard @@ -440,10 +467,18 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool isIco, SkCodec** codecOut) { inputFormat = kBitMask_BitmapInputFormat; } break; + // We want to decode to kIndex_8 for input formats that are already + // designed in index format. case 1: case 2: case 4: case 8: + // However, we cannot in RLE format since we may need to leave some + // pixels as transparent. Similarly, we also cannot for ICO images + // since we may need to apply a transparent mask. + if (kRLE_BitmapInputFormat != inputFormat && !isIco) { + colorType = kIndex_8_SkColorType; + } case 24: case 32: break; @@ -476,11 +511,10 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool isIco, SkCodec** codecOut) { if (codecOut) { // Return the codec - // We will use ImageInfo to store width, height, and alpha type. We - // will set color type to kN32_SkColorType because that should be the - // default output. + // We will use ImageInfo to store width, height, suggested color type, and + // suggested alpha type. const SkImageInfo& imageInfo = SkImageInfo::Make(width, height, - kN32_SkColorType, alphaType); + colorType, alphaType); *codecOut = SkNEW_ARGS(SkBmpCodec, (imageInfo, stream, bitsPerPixel, inputFormat, masks.detach(), numColors, bytesPerColor, @@ -541,8 +575,9 @@ SkBmpCodec::SkBmpCodec(const SkImageInfo& info, SkStream* stream, */ SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes, - const Options&, - SkPMColor*, int*) { + const Options& opts, + SkPMColor* inputColorPtr, + int* inputColorCount) { // Check for proper input and output formats SkCodec::RewindState rewindState = this->rewindIfNeeded(); if (rewindState == kCouldNotRewind_RewindState) { @@ -562,17 +597,26 @@ SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo, } // Create the color table if necessary and prepare the stream for decode - if (!createColorTable(dstInfo.alphaType())) { + // Note that if it is non-NULL, inputColorCount will be modified + if (!createColorTable(dstInfo.alphaType(), inputColorCount)) { SkCodecPrintf("Error: could not create color table.\n"); return kInvalidInput; } + // Copy the color table to the client if necessary + if (kIndex_8_SkColorType == dstInfo.colorType()) { + SkASSERT(NULL != inputColorPtr); + SkASSERT(NULL != inputColorCount); + SkASSERT(NULL != fColorTable.get()); + sk_memcpy32(inputColorPtr, fColorTable->readColors(), *inputColorCount); + } + // Perform the decode switch (fInputFormat) { case kBitMask_BitmapInputFormat: return decodeMask(dstInfo, dst, dstRowBytes); case kRLE_BitmapInputFormat: - return decodeRLE(dstInfo, dst, dstRowBytes); + return decodeRLE(dstInfo, dst, dstRowBytes, opts); case kStandard_BitmapInputFormat: return decode(dstInfo, dst, dstRowBytes); default: @@ -586,7 +630,7 @@ SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo, * Process the color table for the bmp input * */ - bool SkBmpCodec::createColorTable(SkAlphaType alphaType) { + bool SkBmpCodec::createColorTable(SkAlphaType alphaType, int* numColors) { // Allocate memory for color table uint32_t colorBytes = 0; uint32_t maxColors = 0; @@ -599,6 +643,15 @@ SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo, fNumColors = maxColors; } + // Inform the caller of the number of colors + if (NULL != numColors) { + SkASSERT(256 == *numColors); + // We set the number of colors to maxColors in order to ensure + // safe memory accesses. Otherwise, an invalid pixel could + // access memory outside of our color table array. + *numColors = maxColors; + } + // Read the color table from the stream colorBytes = fNumColors * fBytesPerColor; SkAutoTDeleteArray cBuffer(SkNEW_ARRAY(uint8_t, colorBytes)); @@ -632,9 +685,13 @@ SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo, uint8_t blue = get_byte(cBuffer.get(), i*fBytesPerColor); uint8_t green = get_byte(cBuffer.get(), i*fBytesPerColor + 1); uint8_t red = get_byte(cBuffer.get(), i*fBytesPerColor + 2); - uint8_t alpha = kOpaque_SkAlphaType == alphaType ? 0xFF : - (fMasks->getAlphaMask() >> 24) & - get_byte(cBuffer.get(), i*fBytesPerColor + 3); + uint8_t alpha; + if (kOpaque_SkAlphaType == alphaType || kRLE_BitmapInputFormat == fInputFormat) { + alpha = 0xFF; + } else { + alpha = (fMasks->getAlphaMask() >> 24) & + get_byte(cBuffer.get(), i*fBytesPerColor + 3); + } colorTable[i] = packARGB(alpha, red, green, blue); } @@ -740,7 +797,7 @@ SkCodec::Result SkBmpCodec::decodeMask(const SkImageInfo& dstInfo, * Set an RLE pixel using the color table * */ -void SkBmpCodec::setRLEPixel(SkPMColor* dst, size_t dstRowBytes, +void SkBmpCodec::setRLEPixel(void* dst, size_t dstRowBytes, const SkImageInfo& dstInfo, uint32_t x, uint32_t y, uint8_t index) { // Set the row @@ -755,17 +812,11 @@ void SkBmpCodec::setRLEPixel(SkPMColor* dst, size_t dstRowBytes, // Set the pixel based on destination color type switch (dstInfo.colorType()) { case kN32_SkColorType: { - SkPMColor* dstRow = SkTAddOffset(dst, + SkPMColor* dstRow = SkTAddOffset((SkPMColor*) dst, row * (int) dstRowBytes); dstRow[x] = fColorTable->operator[](index); break; } - case kRGB_565_SkColorType: { - uint16_t* dstRow = SkTAddOffset(dst, - row * (int) dstRowBytes); - dstRow[x] = SkPixel32ToPixel16(fColorTable->operator[](index)); - break; - } default: // This case should not be reached. We should catch an invalid // color type when we check that the conversion is possible. @@ -779,7 +830,7 @@ void SkBmpCodec::setRLEPixel(SkPMColor* dst, size_t dstRowBytes, * Set an RLE pixel from R, G, B values * */ -void SkBmpCodec::setRLE24Pixel(SkPMColor* dst, size_t dstRowBytes, +void SkBmpCodec::setRLE24Pixel(void* dst, size_t dstRowBytes, const SkImageInfo& dstInfo, uint32_t x, uint32_t y, uint8_t red, uint8_t green, uint8_t blue) { @@ -795,17 +846,11 @@ void SkBmpCodec::setRLE24Pixel(SkPMColor* dst, size_t dstRowBytes, // Set the pixel based on destination color type switch (dstInfo.colorType()) { case kN32_SkColorType: { - SkPMColor* dstRow = SkTAddOffset(dst, + SkPMColor* dstRow = SkTAddOffset((SkPMColor*) dst, row * (int) dstRowBytes); dstRow[x] = SkPackARGB32NoCheck(0xFF, red, green, blue); break; } - case kRGB_565_SkColorType: { - uint16_t* dstRow = SkTAddOffset(dst, - row * (int) dstRowBytes); - dstRow[x] = SkPack888ToRGB16(red, green, blue); - break; - } default: // This case should not be reached. We should catch an invalid // color type when we check that the conversion is possible. @@ -821,7 +866,8 @@ void SkBmpCodec::setRLE24Pixel(SkPMColor* dst, size_t dstRowBytes, * */ SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo, - void* dst, size_t dstRowBytes) { + void* dst, size_t dstRowBytes, + const Options& opts) { // Set RLE flags static const uint8_t RLE_ESCAPE = 0; static const uint8_t RLE_EOL = 0; @@ -846,10 +892,15 @@ SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo, // Destination parameters int x = 0; int y = 0; - // If the code skips pixels, remaining pixels are transparent or black - // TODO: Skip this if memory was already zeroed. - memset(dst, 0, dstRowBytes * height); - SkPMColor* dstPtr = (SkPMColor*) dst; + + // Set the background as transparent. Then, if the RLE code skips pixels, + // the skipped pixels will be transparent. + // Because of the need for transparent pixels, kN32 is the only color + // type that makes sense for the destination format. + SkASSERT(kN32_SkColorType == dstInfo.colorType()); + if (kNo_ZeroInitialized == opts.fZeroInitialized) { + SkSwizzler::Fill(dst, dstInfo, dstRowBytes, 0, SK_ColorTRANSPARENT, NULL); + } while (true) { // Every entry takes at least two bytes @@ -920,11 +971,11 @@ SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo, case 4: { SkASSERT(currByte < totalBytes); uint8_t val = buffer.get()[currByte++]; - setRLEPixel(dstPtr, dstRowBytes, dstInfo, x++, + setRLEPixel(dst, dstRowBytes, dstInfo, x++, y, val >> 4); numPixels--; if (numPixels != 0) { - setRLEPixel(dstPtr, dstRowBytes, dstInfo, + setRLEPixel(dst, dstRowBytes, dstInfo, x++, y, val & 0xF); numPixels--; } @@ -932,7 +983,7 @@ SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo, } case 8: SkASSERT(currByte < totalBytes); - setRLEPixel(dstPtr, dstRowBytes, dstInfo, x++, + setRLEPixel(dst, dstRowBytes, dstInfo, x++, y, buffer.get()[currByte++]); numPixels--; break; @@ -941,7 +992,7 @@ SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo, uint8_t blue = buffer.get()[currByte++]; uint8_t green = buffer.get()[currByte++]; uint8_t red = buffer.get()[currByte++]; - setRLE24Pixel(dstPtr, dstRowBytes, dstInfo, + setRLE24Pixel(dst, dstRowBytes, dstInfo, x++, y, red, green, blue); numPixels--; } @@ -977,7 +1028,7 @@ SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo, uint8_t green = buffer.get()[currByte++]; uint8_t red = buffer.get()[currByte++]; while (x < endX) { - setRLE24Pixel(dstPtr, dstRowBytes, dstInfo, x++, y, red, + setRLE24Pixel(dst, dstRowBytes, dstInfo, x++, y, red, green, blue); } } else { @@ -994,7 +1045,7 @@ SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo, // Set the indicated number of pixels for (int which = 0; x < endX; x++) { - setRLEPixel(dstPtr, dstRowBytes, dstInfo, x, y, + setRLEPixel(dst, dstRowBytes, dstInfo, x, y, indices[which]); which = !which; } @@ -1084,7 +1135,8 @@ SkCodec::Result SkBmpCodec::decode(const SkImageInfo& dstInfo, // Now we adjust the output image with some additional behavior that // SkSwizzler does not support. Firstly, all bmp images that contain // alpha are masked by the alpha mask. Secondly, many fully transparent - // bmp images are intended to be opaque. Here, we make those corrections. + // bmp images are intended to be opaque. Here, we make those corrections + // in the kN32 case. /* SkPMColor* dstRow = (SkPMColor*) dst; if (SkSwizzler::kBGRA == config) { diff --git a/src/codec/SkCodec_libbmp.h b/src/codec/SkCodec_libbmp.h index 81b514e..8ad613e 100644 --- a/src/codec/SkCodec_libbmp.h +++ b/src/codec/SkCodec_libbmp.h @@ -86,9 +86,9 @@ private: /* * * Creates the color table - * + * Sets colorCount to the new color count if it is non-NULL */ - bool createColorTable(SkAlphaType alphaType); + bool createColorTable(SkAlphaType alphaType, int* colorCount); /* * @@ -121,7 +121,7 @@ private: * Set an RLE pixel using the color table * */ - void setRLEPixel(SkPMColor* dst, size_t dstRowBytes, + void setRLEPixel(void* dst, size_t dstRowBytes, const SkImageInfo& dstInfo, uint32_t x, uint32_t y, uint8_t index); /* @@ -129,7 +129,7 @@ private: * Set an RLE24 pixel from R, G, B values * */ - void setRLE24Pixel(SkPMColor* dst, size_t dstRowBytes, + void setRLE24Pixel(void* dst, size_t dstRowBytes, const SkImageInfo& dstInfo, uint32_t x, uint32_t y, uint8_t red, uint8_t green, uint8_t blue); @@ -139,7 +139,7 @@ private: * */ Result decodeRLE(const SkImageInfo& dstInfo, void* dst, - size_t dstRowBytes); + size_t dstRowBytes, const Options& opts); /* * @@ -181,7 +181,7 @@ private: const uint16_t fBitsPerPixel; const BitmapInputFormat fInputFormat; SkAutoTDelete fMasks; // owned - SkAutoTDelete fColorTable; // owned + SkAutoTUnref fColorTable; // owned uint32_t fNumColors; const uint32_t fBytesPerColor; const uint32_t fOffset; diff --git a/src/codec/SkCodec_libgif.cpp b/src/codec/SkCodec_libgif.cpp index 9356a69..efb3a90 100644 --- a/src/codec/SkCodec_libgif.cpp +++ b/src/codec/SkCodec_libgif.cpp @@ -70,11 +70,11 @@ static GifFileType* open_gif(SkStream* stream) { * This function cleans up the gif object after the decode completes * It is used in a SkAutoTCallIProc template */ -int32_t SkGifCodec::CloseGif(GifFileType* gif) { +void SkGifCodec::CloseGif(GifFileType* gif) { #if GIFLIB_MAJOR < 5 || (GIFLIB_MAJOR == 5 && GIFLIB_MINOR == 0) - return DGifCloseFile(gif); + DGifCloseFile(gif); #else - return DGifCloseFile(gif, NULL); + DGifCloseFile(gif, NULL); #endif } @@ -131,42 +131,77 @@ static uint32_t find_trans_index(const SavedImage& image) { } /* - * Assumes IsGif was called and returned true - * Creates a gif decoder - * Reads enough of the stream to determine the image format + * Read enough of the stream to initialize the SkGifCodec. + * Returns a bool representing success or failure. + * + * @param codecOut + * If it returned true, and codecOut was not NULL, + * codecOut will be set to a new SkGifCodec. + * + * @param gifOut + * If it returned true, and codecOut was NULL, + * gifOut must be non-NULL and gifOut will be set to a new + * GifFileType pointer. + * + * @param stream + * Deleted on failure. + * codecOut will take ownership of it in the case where we created a codec. + * Ownership is unchanged when we returned a gifOut. + * */ -SkCodec* SkGifCodec::NewFromStream(SkStream* stream) { +bool SkGifCodec::ReadHeader(SkStream* stream, SkCodec** codecOut, GifFileType** gifOut) { SkAutoTDelete streamDeleter(stream); + // Read gif header, logical screen descriptor, and global color table - SkAutoTCallIProc gif(open_gif(stream)); + SkAutoTCallVProc gif(open_gif(stream)); if (NULL == gif) { gif_error("DGifOpen failed.\n"); - return NULL; + return false; } - // Get fields from header - const int32_t width = gif->SWidth; - const int32_t height = gif->SHeight; - if (width <= 0 || height <= 0) { - gif_error("Invalid dimensions.\n"); - return NULL; + if (NULL != codecOut) { + // Get fields from header + const int32_t width = gif->SWidth; + const int32_t height = gif->SHeight; + if (width <= 0 || height <= 0) { + gif_error("Invalid dimensions.\n"); + return false; + } + + // Return the codec + // kIndex is the most natural color type for gifs, so we set this as + // the default. + // Many gifs specify a color table index for transparent pixels. Every + // other pixel is guaranteed to be opaque. Despite this, because of the + // possiblity of transparent pixels, we cannot assume that the image is + // opaque. We have the option to set the alpha type as kPremul or + // kUnpremul. Both are valid since the alpha component will always be + // 0xFF or the entire 32-bit pixel will be set to zero. We prefer + // kPremul because we support kPremul, and it is more efficient to + // use kPremul directly even when kUnpremul is supported. + const SkImageInfo& imageInfo = SkImageInfo::Make(width, height, + kIndex_8_SkColorType, kPremul_SkAlphaType); + *codecOut = SkNEW_ARGS(SkGifCodec, (imageInfo, streamDeleter.detach(), gif.detach())); + } else { + SkASSERT(NULL != gifOut); + streamDeleter.detach(); + *gifOut = gif.detach(); } + return true; +} - // Return the codec - // kIndex is the most natural color type for gifs, so we set this as - // the default. - // Many gifs specify a color table index for transparent pixels. Every - // other pixel is guaranteed to be opaque. Despite this, because of the - // possiblity of transparent pixels, we cannot assume that the image is - // opaque. We have the option to set the alpha type as kPremul or - // kUnpremul. Both are valid since the alpha component will always be - // 0xFF or the entire 32-bit pixel will be set to zero. We prefer - // kPremul because we support kPremul, and it is more efficient to - // use kPremul directly even when kUnpremul is supported. - const SkImageInfo& imageInfo = SkImageInfo::Make(width, height, - kIndex_8_SkColorType, kPremul_SkAlphaType); - return SkNEW_ARGS(SkGifCodec, (imageInfo, streamDeleter.detach(), gif.detach())); +/* + * Assumes IsGif was called and returned true + * Creates a gif decoder + * Reads enough of the stream to determine the image format + */ +SkCodec* SkGifCodec::NewFromStream(SkStream* stream) { + SkCodec* codec = NULL; + if (ReadHeader(stream, &codec, NULL)) { + return codec; + } + return NULL; } SkGifCodec::SkGifCodec(const SkImageInfo& srcInfo, SkStream* stream, @@ -191,6 +226,9 @@ static bool conversion_possible(const SkImageInfo& dst, case kN32_SkColorType: return kPremul_SkAlphaType == dst.alphaType() || kUnpremul_SkAlphaType == dst.alphaType(); + case kIndex_8_SkColorType: + return kPremul_SkAlphaType == dst.alphaType() || + kUnpremul_SkAlphaType == dst.alphaType(); default: return false; } @@ -201,11 +239,24 @@ static bool conversion_possible(const SkImageInfo& dst, */ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes, - const Options& opts, SkPMColor*, int*) { - // Check for valid input parameters - if (!this->rewindIfNeeded()) { + const Options& opts, + SkPMColor* inputColorPtr, + int* inputColorCount) { + // Rewind if necessary + SkCodec::RewindState rewindState = this->rewindIfNeeded(); + if (rewindState == kCouldNotRewind_RewindState) { return kCouldNotRewind; + } else if (rewindState == kRewound_RewindState) { + GifFileType* gifOut = NULL; + if (!ReadHeader(this->stream(), NULL, &gifOut)) { + return kCouldNotRewind; + } else { + SkASSERT(NULL != gifOut); + fGif.reset(gifOut); + } } + + // Check for valid input parameters if (dstInfo.dimensions() != this->getInfo().dimensions()) { return gif_error("Scaling not supported.\n", kInvalidScale); } @@ -285,11 +336,23 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, imageTop = 0; } + // Create a color table to store colors the giflib colorMap + SkPMColor alternateColorPtr[256]; + SkPMColor* colorTable; + SkColorType dstColorType = dstInfo.colorType(); + if (kIndex_8_SkColorType == dstColorType) { + SkASSERT(NULL != inputColorPtr); + SkASSERT(NULL != inputColorCount); + SkASSERT(256 == *inputColorCount); + colorTable = inputColorPtr; + } else { + colorTable = alternateColorPtr; + } + // Set up the color table uint32_t colorCount = 0; // Allocate maximum storage to deal with invalid indices safely const uint32_t maxColors = 256; - SkPMColor colorTable[maxColors]; ColorMapObject* colorMap = fGif->Image.ColorMap; // If there is no local color table, use the global color table if (NULL == colorMap) { @@ -310,7 +373,6 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, // This is used to fill unspecified pixels in the image data. uint32_t fillIndex = fGif->SBackGroundColor; - bool fillBackground = true; ZeroInitialized zeroInit = opts.fZeroInitialized; // Gifs have the option to specify the color at a single @@ -324,14 +386,11 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, // is out of range. uint32_t transIndex = find_trans_index(saveExt); - // If the background is already zeroed and we have a valid - // transparent index, we do not need to fill the background. if (transIndex < colorCount) { colorTable[transIndex] = SK_ColorTRANSPARENT; // If there is a transparent index, we also use this as // the fill index. fillIndex = transIndex; - fillBackground = (kYes_ZeroInitialized != zeroInit); } else if (fillIndex >= colorCount) { // If the fill index is invalid, we default to 0. This // behavior is unspecified but matches SkImageDecoder. @@ -339,6 +398,14 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, } } + // Check if we can skip filling the background of the image. We + // may be able to if the memory is zero initialized. + bool skipBackground = + ((kN32_SkColorType == dstColorType && colorTable[fillIndex] == 0) || + (kIndex_8_SkColorType == dstColorType && fillIndex == 0)) && + kYes_ZeroInitialized == zeroInit; + + // Fill in the color table for indices greater than color count. // This allows for predictable, safe behavior. for (uint32_t i = colorCount; i < maxColors; i++) { @@ -357,19 +424,8 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, // FIXME: This may not be the behavior that we want for // animated gifs where we draw on top of the // previous frame. - SkColorType dstColorType = dstInfo.colorType(); - if (fillBackground) { - switch (dstColorType) { - case kN32_SkColorType: - sk_memset32((SkPMColor*) dst, - colorTable[fillIndex], - ((int) dstRowBytes) * height - / sizeof(SkPMColor)); - break; - default: - SkASSERT(false); - break; - } + if (!skipBackground) { + SkSwizzler::Fill(dst, dstInfo, dstRowBytes, 0, fillIndex, colorTable); } // Modify the dst pointer @@ -404,7 +460,7 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, if (GIF_ERROR == DGifGetLine(fGif, buffer.get(), innerWidth)) { // Recover from error by filling remainder of image - if (fillBackground) { + if (!skipBackground) { memset(buffer.get(), fillIndex, innerWidth); for (; y < innerHeight; y++) { swizzler->next(buffer.get(), iter.nextY()); @@ -421,12 +477,9 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo, for (int32_t y = 0; y < innerHeight; y++) { if (GIF_ERROR == DGifGetLine(fGif, buffer.get(), innerWidth)) { - if (fillBackground) { - SkPMColor* dstPtr = (SkPMColor*) SkTAddOffset - (dst, y * dstRowBytes); - sk_memset32(dstPtr, colorTable[fillIndex], - (height - y) * ((int) dstRowBytes) - / sizeof(SkPMColor)); + if (!skipBackground) { + SkSwizzler::Fill(dst, dstInfo, dstRowBytes, y, fillIndex, + colorTable); } return gif_error(SkStringPrintf( "Could not decode line %d of %d.\n", diff --git a/src/codec/SkCodec_libgif.h b/src/codec/SkCodec_libgif.h index de59c6c..d805acc 100644 --- a/src/codec/SkCodec_libgif.h +++ b/src/codec/SkCodec_libgif.h @@ -34,6 +34,27 @@ public: protected: /* + * Read enough of the stream to initialize the SkGifCodec. + * Returns a bool representing success or failure. + * + * @param codecOut + * If it returned true, and codecOut was not NULL, + * codecOut will be set to a new SkGifCodec. + * + * @param gifOut + * If it returned true, and codecOut was NULL, + * gifOut must be non-NULL and gifOut will be set to a new + * GifFileType pointer. + * + * @param stream + * Deleted on failure. + * codecOut will take ownership of it in the case where we created a codec. + * Ownership is unchanged when we returned a gifOut. + * + */ + static bool ReadHeader(SkStream* stream, SkCodec** codecOut, GifFileType** gifOut); + + /* * Initiates the gif decode */ Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, @@ -49,7 +70,7 @@ private: * This function cleans up the gif object after the decode completes * It is used in a SkAutoTCallIProc template */ - static int32_t CloseGif(GifFileType* gif); + static void CloseGif(GifFileType* gif); /* * Frees any extension data used in the decode @@ -68,7 +89,7 @@ private: */ SkGifCodec(const SkImageInfo& srcInfo, SkStream* stream, GifFileType* gif); - SkAutoTCallIProc fGif; // owned + SkAutoTCallVProc fGif; // owned typedef SkCodec INHERITED; }; diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp index 121b74f..250d815 100644 --- a/src/codec/SkCodec_libpng.cpp +++ b/src/codec/SkCodec_libpng.cpp @@ -15,6 +15,7 @@ #include "SkSize.h" #include "SkStream.h" #include "SkSwizzler.h" +#include "SkUtils.h" /////////////////////////////////////////////////////////////////////////////// // Helper macros @@ -119,7 +120,7 @@ typedef uint32_t (*PackColorProc)(U8CPU a, U8CPU r, U8CPU g, U8CPU b); // Note: SkColorTable claims to store SkPMColors, which is not necessarily // the case here. -bool SkPngCodec::decodePalette(bool premultiply) { +bool SkPngCodec::decodePalette(bool premultiply, int bitDepth, int* ctableCount) { int numPalette; png_colorp palette; png_bytep trans; @@ -128,14 +129,7 @@ bool SkPngCodec::decodePalette(bool premultiply) { return false; } - /* BUGGY IMAGE WORKAROUND - - We hit some images (e.g. fruit_.png) who contain bytes that are == colortable_count - which is a problem since we use the byte as an index. To work around this we grow - the colortable by 1 (if its < 256) and duplicate the last color into that slot. - */ - const int colorCount = numPalette + (numPalette < 256); - // Note: These are not necessarily SkPMColors. + // Note: These are not necessarily SkPMColors SkPMColor colorStorage[256]; // worst-case storage SkPMColor* colorPtr = colorStorage; @@ -175,9 +169,24 @@ bool SkPngCodec::decodePalette(bool premultiply) { palette++; } - // see BUGGY IMAGE WORKAROUND comment above - if (numPalette < 256) { - *colorPtr = colorPtr[-1]; + /* BUGGY IMAGE WORKAROUND + Invalid images could contain pixel values that are greater than the number of palette + entries. Since we use pixel values as indices into the palette this could result in reading + beyond the end of the palette which could leak the contents of uninitialized memory. To + ensure this doesn't happen, we grow the colortable to the maximum size that can be + addressed by the bitdepth of the image and fill it with the last palette color or black if + the palette is empty (really broken image). + */ + int colorCount = SkTMax(numPalette, 1 << SkTMin(bitDepth, 8)); + SkPMColor lastColor = index > 0 ? colorPtr[-1] : SkPackARGB32(0xFF, 0, 0, 0); + for (; index < colorCount; index++) { + *colorPtr++ = lastColor; + } + + // Set the new color count + if (ctableCount != NULL) { + SkASSERT(256 == *ctableCount); + *ctableCount = colorCount; } fColorTable.reset(SkNEW_ARGS(SkColorTable, (colorStorage, colorCount))); @@ -276,10 +285,7 @@ static bool read_header(SkStream* stream, png_structp* png_ptrp, SkAlphaType skAlphaType; switch (colorType) { case PNG_COLOR_TYPE_PALETTE: - // Technically, this is true of the data, but I don't think we want - // to support it. - // skColorType = kIndex8_SkColorType; - skColorType = kN32_SkColorType; + skColorType = kIndex_8_SkColorType; skAlphaType = has_transparency_in_palette(png_ptr, info_ptr) ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType; break; @@ -387,22 +393,43 @@ void SkPngCodec::destroyReadStruct() { static bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) { // TODO: Support other conversions - if (dst.colorType() != src.colorType()) { - return false; - } if (dst.profileType() != src.profileType()) { return false; } - if (dst.alphaType() == src.alphaType()) { - return true; + + // Check for supported alpha types + if (src.alphaType() != dst.alphaType()) { + if (kOpaque_SkAlphaType == src.alphaType()) { + // If the source is opaque, we must decode to opaque + return false; + } + + // The source is not opaque + switch (dst.alphaType()) { + case kPremul_SkAlphaType: + case kUnpremul_SkAlphaType: + // The source is not opaque, so either of these is okay + break; + default: + // We cannot decode a non-opaque image to opaque (or unknown) + return false; + } + } + + // Check for supported color types + switch (dst.colorType()) { + // Allow output to kN32 from any type of input + case kN32_SkColorType: + return true; + default: + return dst.colorType() == src.colorType(); } - return kPremul_SkAlphaType == dst.alphaType() && - kUnpremul_SkAlphaType == src.alphaType(); } SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo, void* dst, size_t rowBytes, - const Options& options) { + const Options& options, + int* ctableCount) { // FIXME: Could we use the return value of setjmp to specify the type of // error? if (setjmp(png_jmpbuf(fPng_ptr))) { @@ -423,7 +450,8 @@ SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo, fReallyHasAlpha = false; if (PNG_COLOR_TYPE_PALETTE == pngColorType) { fSrcConfig = SkSwizzler::kIndex; - if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType())) { + if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(), bitDepth, + ctableCount)) { return kInvalidInput; } } else if (kAlpha_8_SkColorType == requestedInfo.colorType()) { @@ -492,8 +520,18 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* return kInvalidConversion; } + // Note that ctableCount will be modified if there is a color table const Result result = this->initializeSwizzler(requestedInfo, dst, rowBytes, - options); + options, ctableCount); + + // Copy the color table to the client if necessary + if (kIndex_8_SkColorType == requestedInfo.colorType()) { + SkASSERT(NULL != ctable); + SkASSERT(NULL != ctableCount); + SkASSERT(NULL != fColorTable.get()); + sk_memcpy32(ctable, fColorTable->readColors(), *ctableCount); + } + if (result != kSuccess) { return result; } @@ -631,7 +669,9 @@ SkScanlineDecoder* SkPngCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo) Options opts; // FIXME: Pass this in to getScanlineDecoder? opts.fZeroInitialized = kNo_ZeroInitialized; - if (this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), opts) != kSuccess) { + // FIXME: onGetScanlineDecoder does not currently have a way to get color table information + // for a kIndex8 decoder. + if (this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), opts, NULL) != kSuccess) { SkCodecPrintf("failed to initialize the swizzler.\n"); return NULL; } diff --git a/src/codec/SkCodec_libpng.h b/src/codec/SkCodec_libpng.h index 103839b..e9bcd04 100644 --- a/src/codec/SkCodec_libpng.h +++ b/src/codec/SkCodec_libpng.h @@ -49,10 +49,10 @@ private: // Helper to set up swizzler and color table. Also calls png_read_update_info. Result initializeSwizzler(const SkImageInfo& requestedInfo, void* dst, - size_t rowBytes, const Options&); + size_t rowBytes, const Options&, int* ctableCount); // Calls rewindIfNeeded, and returns true if the decoder can continue. bool handleRewind(); - bool decodePalette(bool premultiply); + bool decodePalette(bool premultiply, int bitDepth, int* ctableCount); void finish(); void destroyReadStruct(); diff --git a/src/codec/SkMaskSwizzler.cpp b/src/codec/SkMaskSwizzler.cpp index 944042d..58ae11d 100644 --- a/src/codec/SkMaskSwizzler.cpp +++ b/src/codec/SkMaskSwizzler.cpp @@ -63,22 +63,6 @@ static SkSwizzler::ResultAlpha swizzle_mask16_to_n32_premul( return COMPUTE_RESULT_ALPHA; } -static SkSwizzler::ResultAlpha swizzle_mask16_to_565( - void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) { - - // Use the masks to decode to the destination - uint16_t* srcPtr = (uint16_t*) srcRow; - uint16_t* dstPtr = (uint16_t*) dstRow; - for (int i = 0; i < width; i++) { - uint16_t p = srcPtr[i]; - uint8_t red = masks->getRed(p); - uint8_t green = masks->getGreen(p); - uint8_t blue = masks->getBlue(p); - dstPtr[i] = SkPack888ToRGB16(red, green, blue); - } - return SkSwizzler::kOpaque_ResultAlpha; -} - static SkSwizzler::ResultAlpha swizzle_mask24_to_n32_opaque( void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) { @@ -130,21 +114,6 @@ static SkSwizzler::ResultAlpha swizzle_mask24_to_n32_premul( return COMPUTE_RESULT_ALPHA; } -static SkSwizzler::ResultAlpha swizzle_mask24_to_565( - void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) { - - // Use the masks to decode to the destination - uint16_t* dstPtr = (uint16_t*) dstRow; - for (int i = 0; i < 3*width; i += 3) { - uint32_t p = srcRow[i] | (srcRow[i + 1] << 8) | srcRow[i + 2] << 16; - uint8_t red = masks->getRed(p); - uint8_t green = masks->getGreen(p); - uint8_t blue = masks->getBlue(p); - dstPtr[i/3] = SkPack888ToRGB16(red, green, blue); - } - return SkSwizzler::kOpaque_ResultAlpha; -} - static SkSwizzler::ResultAlpha swizzle_mask32_to_n32_opaque( void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) { @@ -199,22 +168,6 @@ static SkSwizzler::ResultAlpha swizzle_mask32_to_n32_premul( return COMPUTE_RESULT_ALPHA; } -static SkSwizzler::ResultAlpha swizzle_mask32_to_565( - void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) { - - // Use the masks to decode to the destination - uint32_t* srcPtr = (uint32_t*) srcRow; - uint16_t* dstPtr = (uint16_t*) dstRow; - for (int i = 0; i < width; i++) { - uint32_t p = srcPtr[i]; - uint8_t red = masks->getRed(p); - uint8_t green = masks->getGreen(p); - uint8_t blue = masks->getBlue(p); - dstPtr[i] = SkPack888ToRGB16(red, green, blue); - } - return SkSwizzler::kOpaque_ResultAlpha; -} - /* * * Create a new mask swizzler @@ -244,15 +197,6 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler( break; } break; - case kRGB_565_SkColorType: - switch (info.alphaType()) { - case kOpaque_SkAlphaType: - proc = &swizzle_mask16_to_565; - break; - default: - break; - } - break; default: break; } @@ -274,15 +218,6 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler( break; } break; - case kRGB_565_SkColorType: - switch (info.alphaType()) { - case kOpaque_SkAlphaType: - proc = &swizzle_mask24_to_565; - break; - default: - break; - } - break; default: break; } @@ -304,15 +239,6 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler( break; } break; - case kRGB_565_SkColorType: - switch (info.alphaType()) { - case kOpaque_SkAlphaType: - proc = &swizzle_mask32_to_565; - break; - default: - break; - } - break; default: break; } diff --git a/src/codec/SkSwizzler.cpp b/src/codec/SkSwizzler.cpp index 0486cf2..754b088 100644 --- a/src/codec/SkSwizzler.cpp +++ b/src/codec/SkSwizzler.cpp @@ -9,6 +9,7 @@ #include "SkColorPriv.h" #include "SkSwizzler.h" #include "SkTemplates.h" +#include "SkUtils.h" SkSwizzler::ResultAlpha SkSwizzler::GetResult(uint8_t zeroAlpha, uint8_t maxAlpha) { @@ -20,11 +21,11 @@ SkSwizzler::ResultAlpha SkSwizzler::GetResult(uint8_t zeroAlpha, // kIndex1, kIndex2, kIndex4 -static SkSwizzler::ResultAlpha swizzle_small_index_to_n32( +static SkSwizzler::ResultAlpha swizzle_small_index_to_index( void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width, int bitsPerPixel, int y, const SkPMColor ctable[]) { - SkPMColor* SK_RESTRICT dst = (SkPMColor*) dstRow; + uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow; INIT_RESULT_ALPHA; const uint32_t pixelsPerByte = 8 / bitsPerPixel; const size_t rowBytes = compute_row_bytes_ppb(width, pixelsPerByte); @@ -34,9 +35,8 @@ static SkSwizzler::ResultAlpha swizzle_small_index_to_n32( uint8_t pixelData = src[byte]; for (uint32_t p = 0; p < pixelsPerByte && x < width; p++) { uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask; - SkPMColor c = ctable[index]; - UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT); - dst[x] = c; + UPDATE_RESULT_ALPHA(ctable[index] >> SK_A32_SHIFT); + dst[x] = index; pixelData <<= bitsPerPixel; x++; } @@ -44,11 +44,12 @@ static SkSwizzler::ResultAlpha swizzle_small_index_to_n32( return COMPUTE_RESULT_ALPHA; } -static SkSwizzler::ResultAlpha swizzle_small_index_to_565( +static SkSwizzler::ResultAlpha swizzle_small_index_to_n32( void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width, int bitsPerPixel, int y, const SkPMColor ctable[]) { - uint16_t* SK_RESTRICT dst = (uint16_t*) dstRow; + SkPMColor* SK_RESTRICT dst = (SkPMColor*) dstRow; + INIT_RESULT_ALPHA; const uint32_t pixelsPerByte = 8 / bitsPerPixel; const size_t rowBytes = compute_row_bytes_ppb(width, pixelsPerByte); const uint8_t mask = (1 << bitsPerPixel) - 1; @@ -57,13 +58,30 @@ static SkSwizzler::ResultAlpha swizzle_small_index_to_565( uint8_t pixelData = src[byte]; for (uint32_t p = 0; p < pixelsPerByte && x < width; p++) { uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask; - uint16_t c = SkPixel32ToPixel16(ctable[index]); + SkPMColor c = ctable[index]; + UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT); dst[x] = c; pixelData <<= bitsPerPixel; x++; } } - return SkSwizzler::kOpaque_ResultAlpha; + return COMPUTE_RESULT_ALPHA; +} + +static SkSwizzler::ResultAlpha swizzle_index_to_index( + void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width, + int bytesPerPixel, int y, const SkPMColor ctable[]) { + + uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow; + memcpy(dst, src, width); + // TODO (msarett): Should we skip the loop here and guess that the row is opaque/not opaque? + // SkScaledBitmap sampler just guesses that it is opaque. This is dangerous + // and probably wrong since gif and bmp (rarely) may have alpha. + INIT_RESULT_ALPHA; + for (int x = 0; x < width; x++) { + UPDATE_RESULT_ALPHA(ctable[src[x]] >> SK_A32_SHIFT); + } + return COMPUTE_RESULT_ALPHA; } // kIndex @@ -75,10 +93,9 @@ static SkSwizzler::ResultAlpha swizzle_index_to_n32( SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow; INIT_RESULT_ALPHA; for (int x = 0; x < width; x++) { - SkPMColor c = ctable[*src]; + SkPMColor c = ctable[src[x]]; UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT); dst[x] = c; - src++; } return COMPUTE_RESULT_ALPHA; } @@ -90,29 +107,15 @@ static SkSwizzler::ResultAlpha swizzle_index_to_n32_skipZ( SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow; INIT_RESULT_ALPHA; for (int x = 0; x < width; x++) { - SkPMColor c = ctable[*src]; + SkPMColor c = ctable[src[x]]; UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT); if (c != 0) { dst[x] = c; } - src++; } return COMPUTE_RESULT_ALPHA; } -static SkSwizzler::ResultAlpha swizzle_index_to_565( - void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width, - int bytesPerPixel, int y, const SkPMColor ctable[]) { - - uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow; - for (int x = 0; x < width; x++) { - uint16_t c = SkPixel32ToPixel16(ctable[*src]); - dst[x] = c; - src++; - } - return SkSwizzler::kOpaque_ResultAlpha; -} - #undef A32_MASK_IN_PLACE static SkSwizzler::ResultAlpha swizzle_bgrx_to_n32( @@ -127,18 +130,6 @@ static SkSwizzler::ResultAlpha swizzle_bgrx_to_n32( return SkSwizzler::kOpaque_ResultAlpha; } -static SkSwizzler::ResultAlpha swizzle_bgrx_to_565( - void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width, - int bytesPerPixel, int y, const SkPMColor ctable[]) { - - uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow; - for (int x = 0; x < width; x++) { - dst[x] = SkPack888ToRGB16(src[2], src[1], src[0]); - src += bytesPerPixel; - } - return SkSwizzler::kOpaque_ResultAlpha; -} - // kBGRA static SkSwizzler::ResultAlpha swizzle_bgra_to_n32_unpremul( @@ -282,8 +273,8 @@ SkSwizzler* SkSwizzler::CreateSwizzler(SkSwizzler::SrcConfig sc, case kN32_SkColorType: proc = &swizzle_small_index_to_n32; break; - case kRGB_565_SkColorType: - proc = &swizzle_small_index_to_565; + case kIndex_8_SkColorType: + proc = &swizzle_small_index_to_index; break; default: break; @@ -301,8 +292,8 @@ SkSwizzler* SkSwizzler::CreateSwizzler(SkSwizzler::SrcConfig sc, break; } break; - case kRGB_565_SkColorType: - proc = &swizzle_index_to_565; + case kIndex_8_SkColorType: + proc = &swizzle_index_to_index; break; default: break; @@ -314,9 +305,6 @@ SkSwizzler* SkSwizzler::CreateSwizzler(SkSwizzler::SrcConfig sc, case kN32_SkColorType: proc = &swizzle_bgrx_to_n32; break; - case kRGB_565_SkColorType: - proc = &swizzle_bgrx_to_565; - break; default: break; } @@ -433,3 +421,56 @@ SkSwizzler::ResultAlpha SkSwizzler::next(const uint8_t* SK_RESTRICT src, return fRowProc(row, src, fDstInfo.width(), fDeltaSrc, fCurrY, fColorTable); } + +void SkSwizzler::Fill(void* dst, const SkImageInfo& dstInfo, size_t dstRowBytes, uint32_t y, + uint32_t colorOrIndex, SkPMColor* colorTable) { + SkASSERT(dst != NULL); + SkASSERT(y < (uint32_t) dstInfo.height()); + + // Get dst start row + void* dstRow = SkTAddOffset(dst, y * dstRowBytes); + + // Calculate remaining bytes. This is tricky since the final row may not be padded. + const size_t totalBytes = dstInfo.getSafeSize(dstRowBytes); + const size_t remainingBytes = totalBytes - y * dstRowBytes; + + // Use the proper memset routine to fill the remaining bytes + switch(dstInfo.colorType()) { + case kN32_SkColorType: + // Assume input is an index if we have a color table + uint32_t color; + if (NULL != colorTable) { + SkASSERT(colorOrIndex == (uint8_t) colorOrIndex); + color = colorTable[colorOrIndex]; + // Otherwise, assume the input is a color + } else { + color = colorOrIndex; + } + + // We must fill row by row in the case of unaligned row bytes + if (SkIsAlign4((size_t) dstRow) && SkIsAlign4(dstRowBytes)) { + sk_memset32((uint32_t*) dstRow, color, + (uint32_t) remainingBytes / sizeof(SkPMColor)); + } else { + // This is an unlikely, slow case + SkCodecPrintf("Warning: Strange number of row bytes, fill will be slow.\n"); + for (int32_t row = y; row < dstInfo.height(); row++) { + uint32_t* dstPtr = (uint32_t*) dstRow; + for (int32_t col = 0; col < dstInfo.width(); col++) { + dstPtr[col] = color; + } + dstRow = SkTAddOffset(dstRow, dstRowBytes); + } + } + break; + // On an index destination color type, always assume the input is an index + case kIndex_8_SkColorType: + SkASSERT(colorOrIndex == (uint8_t) colorOrIndex); + memset(dstRow, colorOrIndex, remainingBytes); + break; + default: + SkCodecPrintf("Error: Unsupported dst color type for fill(). Doing nothing.\n"); + SkASSERT(false); + break; + } +} diff --git a/src/codec/SkSwizzler.h b/src/codec/SkSwizzler.h index 63afbf7..6044c86 100644 --- a/src/codec/SkSwizzler.h +++ b/src/codec/SkSwizzler.h @@ -61,7 +61,7 @@ public: static bool IsOpaque(ResultAlpha r) { return kOpaque_ResultAlpha == r; } - + /* * * Constructs the proper result code based on accumulated alpha masks @@ -128,6 +128,36 @@ public: const SkImageInfo&, void* dst, size_t dstRowBytes, SkImageGenerator::ZeroInitialized); + + /** + * Fill the remainder of the destination with a single color + * + * @param y + * The starting row for the fill. + * + * @param colorOrIndex + * @param colorTable + * If dstInfo.colorType() is kIndex8, colorOrIndex is assumed to be a uint8_t + * index, and colorTable is ignored. Each 8-bit pixel will be set to (uint8_t) + * index. + * + * If dstInfo.colorType() is kN32, colorOrIndex is treated differently depending on + * whether colorTable is NULL: + * + * A NULL colorTable means colorOrIndex is treated as an SkPMColor (premul or + * unpremul, depending on dstInfo.alphaType()). Each 4-byte pixel will be set to + * colorOrIndex. + + * A non-NULL colorTable means colorOrIndex is treated as a uint8_t index into + * the colorTable. i.e. each 4-byte pixel will be set to + * colorTable[(uint8_t) colorOrIndex]. + * + * Other SkColorTypes are not supported. + * + */ + static void Fill(void* dst, const SkImageInfo& dstInfo, size_t dstRowBytes, uint32_t y, + uint32_t colorOrIndex, SkPMColor* colorTable); + /** * Swizzle the next line. Call height times, once for each row of source. * @param src The next row of the source data. @@ -156,6 +186,7 @@ public: * destination? */ void setDstRow(void* dst) { fDstRow = dst; } + private: #ifdef SK_DEBUG diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp index 26846a4..a771716 100644 --- a/tests/CodexTest.cpp +++ b/tests/CodexTest.cpp @@ -41,7 +41,12 @@ static void check(skiatest::Reporter* r, ERRORF(r, "Unable to decode '%s'", path); return; } - SkImageInfo info = codec->getInfo(); + + // This test is used primarily to verify rewinding works properly. Using kN32 allows + // us to test this without the added overhead of creating different bitmaps depending + // on the color type (ex: building a color table for kIndex8). DM is where we test + // decodes to all possible destination color types. + SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType); REPORTER_ASSERT(r, info.dimensions() == size); SkBitmap bm; bm.allocPixels(info); @@ -94,6 +99,11 @@ DEF_TEST(Codec, r) { // Decodes an embedded PNG image check(r, "google_chrome.ico", SkISize::Make(256, 256), false); + // GIF + check(r, "box.gif", SkISize::Make(200, 55), false); + check(r, "color_wheel.gif", SkISize::Make(128, 128), false); + check(r, "randPixels.gif", SkISize::Make(8, 8), false); + // PNG check(r, "arrow.png", SkISize::Make(187, 312), true); check(r, "baby_tux.png", SkISize::Make(240, 246), true); diff --git a/tests/SwizzlerTest.cpp b/tests/SwizzlerTest.cpp new file mode 100644 index 0000000..1355f3b --- /dev/null +++ b/tests/SwizzlerTest.cpp @@ -0,0 +1,112 @@ +/* + * Copyright 2015 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkSwizzler.h" +#include "Test.h" + +// These are the values that we will look for to indicate that the fill was successful +static const uint8_t kFillIndex = 0x1; +static const uint32_t kFillColor = 0x22334455; + +static void check_fill(skiatest::Reporter* r, + const SkImageInfo& imageInfo, + uint32_t startRow, + size_t rowBytes, + uint32_t offset, + uint32_t colorOrIndex, + SkPMColor* colorTable) { + + // Calculate the total size of the image in bytes. Use the smallest possible size. + // The offset value tells us to adjust the pointer from the memory we allocate in order + // to test on different memory alignments. If offset is nonzero, we need to increase the + // size of the memory we allocate in order to make sure that we have enough. We are + // still allocating the smallest possible size. + const size_t totalBytes = imageInfo.getSafeSize(rowBytes) + offset; + + // Create fake image data where every byte has a value of 0 + SkAutoTDelete storage((uint8_t*) sk_malloc_throw(totalBytes)); + memset(storage.get(), 0, totalBytes); + // Adjust the pointer in order to test on different memory alignments + uint8_t* imageData = storage.get() + offset; + + // Fill image with the fill value starting at the indicated row + SkSwizzler::Fill(imageData, imageInfo, rowBytes, startRow, colorOrIndex, colorTable); + + // Ensure that the pixels are filled properly + // The bots should catch any memory corruption + uint8_t* indexPtr = imageData + startRow * rowBytes; + uint32_t* colorPtr = (uint32_t*) indexPtr; + for (int32_t y = startRow; y < imageInfo.height(); y++) { + for (int32_t x = 0; x < imageInfo.width(); x++) { + if (kIndex_8_SkColorType == imageInfo.colorType()) { + REPORTER_ASSERT(r, kFillIndex == indexPtr[x]); + } else { + REPORTER_ASSERT(r, kFillColor == colorPtr[x]); + } + } + indexPtr += rowBytes; + colorPtr = (uint32_t*) indexPtr; + } +} + +// Test Fill() with different combinations of dimensions, alignment, and padding +DEF_TEST(SwizzlerFill, r) { + // Set up a color table + SkPMColor colorTable[kFillIndex + 1]; + colorTable[kFillIndex] = kFillColor; + // Apart from the fill index, we will leave the other colors in the color table uninitialized. + // If we incorrectly try to fill with this uninitialized memory, the bots will catch it. + + // Test on an invalid width and representative widths + const uint32_t widths[] = { 0, 10, 50 }; + + // In order to call Fill(), there must be at least one row to fill + // Test on the smallest possible height and representative heights + const uint32_t heights[] = { 1, 5, 10 }; + + // Test on interesting possibilities for row padding + const uint32_t paddings[] = { 0, 1, 2, 3, 4 }; + + // Iterate over test dimensions + for (uint32_t width : widths) { + for (uint32_t height : heights) { + + // Create image info objects + const SkImageInfo colorInfo = SkImageInfo::MakeN32(width, height, + kUnknown_SkAlphaType); + const SkImageInfo indexInfo = colorInfo.makeColorType(kIndex_8_SkColorType); + + for (uint32_t padding : paddings) { + + // Calculate row bytes + size_t colorRowBytes = SkColorTypeBytesPerPixel(kN32_SkColorType) * width + + padding; + size_t indexRowBytes = width + padding; + + // If there is padding, we can invent an offset to change the memory alignment + for (uint32_t offset = 0; offset <= padding; offset++) { + + // Test all possible start rows + for (uint32_t startRow = 0; startRow < height; startRow++) { + + // Fill with an index that we use to look up a color + check_fill(r, colorInfo, startRow, colorRowBytes, offset, kFillIndex, + colorTable); + + // Fill with a color + check_fill(r, colorInfo, startRow, colorRowBytes, offset, kFillColor, + NULL); + + // Fill with an index + check_fill(r, indexInfo, startRow, indexRowBytes, offset, kFillIndex, + NULL); + } + } + } + } + } +}