diff --git a/dm/DM.cpp b/dm/DM.cpp index 41e4c8dc3e..c28f688697 100644 --- a/dm/DM.cpp +++ b/dm/DM.cpp @@ -345,17 +345,35 @@ static void push_codec_srcs(Path path) { bool subset = false; // The following image types are supported by BitmapRegionDecoder, // so we will test full image decodes and subset decodes. - static const char* const exts[] = { + static const char* const subsetExts[] = { "jpg", "jpeg", "png", "webp", "JPG", "JPEG", "PNG", "WEBP", }; - for (const char* ext : exts) { + for (const char* ext : subsetExts) { if (path.endsWith(ext)) { subset = true; break; } } + bool full = false; + // The following image types are only supported by BitmapFactory, + // so we only need to test full image decodes. + static const char* fullExts[] = { + "wbmp", "bmp", "gif", + "WBMP", "BMP", "GIF", + }; + for (const char* ext : fullExts) { + if (path.endsWith(ext)) { + full = true; + break; + } + } + + if (!full && !subset) { + return; + } + const int sampleSizes[] = { 1, 2, 3, 4, 5, 6, 7, 8 }; for (int sampleSize : sampleSizes) { diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index c0625ffb9b..075f97680d 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -400,8 +400,6 @@ public: /** * An enum representing the order in which scanlines will be returned by * the scanline decoder. - * - * This is undefined before startScanlineDecode() is called. */ SkScanlineOrder getScanlineOrder() const { return this->onGetScanlineOrder(); } @@ -620,6 +618,5 @@ private: virtual SkSampler* getSampler(bool /*createIfNecessary*/) { return nullptr; } friend class SkSampledCodec; - friend class SkIcoCodec; }; #endif // SkCodec_DEFINED diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp index c26b3eff9b..cf6e253d97 100644 --- a/src/codec/SkAndroidCodec.cpp +++ b/src/codec/SkAndroidCodec.cpp @@ -34,9 +34,11 @@ SkAndroidCodec* SkAndroidCodec::NewFromStream(SkStream* stream, SkPngChunkReader case kWBMP_SkEncodedFormat: case kBMP_SkEncodedFormat: case kGIF_SkEncodedFormat: - case kICO_SkEncodedFormat: return new SkSampledCodec(codec.detach()); default: + // FIXME: SkSampledCodec is temporarily disabled for other formats + // while focusing on the formats that are supported by + // BitmapRegionDecoder. return nullptr; } } diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp index aff54021a7..191c2ad800 100644 --- a/src/codec/SkBmpCodec.cpp +++ b/src/codec/SkBmpCodec.cpp @@ -481,8 +481,6 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) { // Return the codec switch (inputFormat) { case kStandard_BmpInputFormat: - // We require streams to have a memory base for Bmp-in-Ico decodes. - SkASSERT(!inIco || nullptr != stream->getMemoryBase()); *codecOut = new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors, bytesPerColor, offset - bytesRead, rowOrder, inIco); return true; diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp index 9e90f3c630..fd4d6d18bc 100644 --- a/src/codec/SkBmpStandardCodec.cpp +++ b/src/codec/SkBmpStandardCodec.cpp @@ -27,7 +27,6 @@ SkBmpStandardCodec::SkBmpStandardCodec(const SkImageInfo& info, SkStream* stream , fSrcRowBytes(SkAlign4(compute_row_bytes(this->getInfo().width(), this->bitsPerPixel()))) , fSrcBuffer(new uint8_t [fSrcRowBytes]) , fInIco(inIco) - , fAndMaskRowBytes(fInIco ? SkAlign4(compute_row_bytes(this->getInfo().width(), 1)) : 0) {} /* @@ -61,6 +60,9 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo, *rowsDecoded = rows; return kIncompleteInput; } + if (fInIco) { + return this->decodeIcoMask(dstInfo, dst, dstRowBytes); + } return kSuccess; } @@ -225,8 +227,9 @@ SkCodec::Result SkBmpStandardCodec::prepareToDecode(const SkImageInfo& dstInfo, /* * Performs the bitmap decoding for standard input format */ -int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes, - const Options& opts) { +int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo, + void* dst, size_t dstRowBytes, + const Options& opts) { // Iterate over rows of the image const int height = dstInfo.height(); for (int y = 0; y < height; y++) { @@ -243,75 +246,29 @@ int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo, void* dst, size_t fSwizzler->swizzle(dstRow, fSrcBuffer.get()); } - if (fInIco) { - const int startScanline = this->currScanline(); - if (startScanline < 0) { - // We are not performing a scanline decode. - // Just decode the entire ICO mask and return. - decodeIcoMask(this->stream(), dstInfo, dst, dstRowBytes); - return height; - } - - // In order to perform a scanline ICO decode, we must be able - // to skip ahead in the stream in order to apply the AND mask - // to the requested scanlines. - // We will do this by taking advantage of the fact that - // SkIcoCodec always uses a SkMemoryStream as its underlying - // representation of the stream. - const void* memoryBase = this->stream()->getMemoryBase(); - SkASSERT(nullptr != memoryBase); - SkASSERT(this->stream()->hasLength()); - SkASSERT(this->stream()->hasPosition()); - - const size_t length = this->stream()->getLength(); - const size_t currPosition = this->stream()->getPosition(); - - // Calculate how many bytes we must skip to reach the AND mask. - const int remainingScanlines = this->getInfo().height() - startScanline - height; - const size_t bytesToSkip = remainingScanlines * fSrcRowBytes + - startScanline * fAndMaskRowBytes; - const size_t subStreamStartPosition = currPosition + bytesToSkip; - if (subStreamStartPosition >= length) { - // FIXME: How can we indicate that this decode was actually incomplete? - return height; - } - - // Create a subStream to pass to decodeIcoMask(). It is useful to encapsulate - // the memory base into a stream in order to safely handle incomplete images - // without reading out of bounds memory. - const void* subStreamMemoryBase = SkTAddOffset(memoryBase, - subStreamStartPosition); - const size_t subStreamLength = length - subStreamStartPosition; - // This call does not transfer ownership of the subStreamMemoryBase. - SkMemoryStream subStream(subStreamMemoryBase, subStreamLength, false); - - // FIXME: If decodeIcoMask does not succeed, is there a way that we can - // indicate the decode was incomplete? - decodeIcoMask(&subStream, dstInfo, dst, dstRowBytes); - } - + // Finished decoding the entire image return height; } -void SkBmpStandardCodec::decodeIcoMask(SkStream* stream, const SkImageInfo& dstInfo, +// TODO (msarett): This function will need to be modified in order to perform row by row decodes +// when the Ico scanline decoder is implemented. +SkCodec::Result SkBmpStandardCodec::decodeIcoMask(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes) { // BMP in ICO have transparency, so this cannot be 565, and this mask // prevents us from using kIndex8. The below code depends on the output // being an SkPMColor. SkASSERT(dstInfo.colorType() == kN32_SkColorType); - // If we are sampling, make sure that we only mask the sampled pixels. - // We do not need to worry about sampling in the y-dimension because that - // should be handled by SkSampledCodec. - int sampleX = fSwizzler->sampleX(); - int startX = get_start_coord(sampleX); + // The AND mask is always 1 bit per pixel + const int width = this->getInfo().width(); + const size_t rowBytes = SkAlign4(compute_row_bytes(width, 1)); SkPMColor* dstPtr = (SkPMColor*) dst; for (int y = 0; y < dstInfo.height(); y++) { // The srcBuffer will at least be large enough - if (stream->read(fSrcBuffer.get(), fAndMaskRowBytes) != fAndMaskRowBytes) { + if (stream()->read(fSrcBuffer.get(), rowBytes) != rowBytes) { SkCodecPrintf("Warning: incomplete AND mask for bmp-in-ico.\n"); - return; + return kIncompleteInput; } int row = this->getDstRow(y, dstInfo.height()); @@ -319,15 +276,17 @@ void SkBmpStandardCodec::decodeIcoMask(SkStream* stream, const SkImageInfo& dstI SkPMColor* dstRow = SkTAddOffset(dstPtr, row * dstRowBytes); - for (int x = startX; x < this->getInfo().width(); x += sampleX) { + for (int x = 0; x < width; x++) { int quotient; int modulus; SkTDivMod(x, 8, "ient, &modulus); uint32_t shift = 7 - modulus; - uint32_t alphaBit = (fSrcBuffer.get()[quotient] >> shift) & 0x1; - dstRow[get_dst_coord(x, sampleX)] &= alphaBit - 1; + uint32_t alphaBit = + (fSrcBuffer.get()[quotient] >> shift) & 0x1; + dstRow[x] &= alphaBit - 1; } } + return kSuccess; } uint32_t SkBmpStandardCodec::onGetFillValue(SkColorType colorType, SkAlphaType alphaType) const { diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h index 7f23616a60..d687eaad28 100644 --- a/src/codec/SkBmpStandardCodec.h +++ b/src/codec/SkBmpStandardCodec.h @@ -75,12 +75,7 @@ private: int decodeRows(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes, const Options& opts) override; - /* - * @param stream This may be a pointer to the stream owned by the parent SkCodec - * or a sub-stream of the stream owned by the parent SkCodec. - * Either way, this stream is unowned. - */ - void decodeIcoMask(SkStream* stream, const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes); + Result decodeIcoMask(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes); SkAutoTUnref fColorTable; // owned const uint32_t fNumColors; @@ -90,7 +85,6 @@ private: const size_t fSrcRowBytes; SkAutoTDeleteArray fSrcBuffer; const bool fInIco; - const size_t fAndMaskRowBytes; // only used for fInIco decodes typedef SkBmpCodec INHERITED; }; diff --git a/src/codec/SkCodec_libico.cpp b/src/codec/SkCodec_libico.cpp index e1314d7121..8c5a1b3412 100644 --- a/src/codec/SkCodec_libico.cpp +++ b/src/codec/SkCodec_libico.cpp @@ -15,43 +15,6 @@ #include "SkTDArray.h" #include "SkTSort.h" -static bool ico_conversion_possible(const SkImageInfo& dstInfo) { - // We only support kN32_SkColorType. - // This makes sense for BMP-in-ICO. The presence of an AND - // mask (which changes colors and adds transparency) means that - // we cannot use k565 or kIndex8. - // FIXME: For PNG-in-ICO, we could technically support whichever - // color types that the png supports. - if (kN32_SkColorType != dstInfo.colorType()) { - return false; - } - - // We only support transparent alpha types. This is necessary for - // BMP-in-ICOs since there will be an AND mask. - // FIXME: For opaque PNG-in-ICOs, we should be able to support kOpaque. - return kPremul_SkAlphaType == dstInfo.alphaType() || - kUnpremul_SkAlphaType == dstInfo.alphaType(); -} - -static SkImageInfo fix_embedded_alpha(const SkImageInfo& dstInfo, SkAlphaType embeddedAlpha) { - // FIXME (msarett): ICO is considered non-opaque, even if the embedded BMP - // incorrectly claims it has no alpha. - switch (embeddedAlpha) { - case kPremul_SkAlphaType: - case kUnpremul_SkAlphaType: - // Use the requested alpha type if the embedded codec supports alpha. - embeddedAlpha = dstInfo.alphaType(); - break; - case kOpaque_SkAlphaType: - // If the embedded codec claims it is opaque, decode as if it is opaque. - break; - default: - SkASSERT(false); - break; - } - return dstInfo.makeAlphaType(embeddedAlpha); -} - /* * Checks the start of the stream to see if the image is an Ico or Cur */ @@ -234,7 +197,6 @@ SkIcoCodec::SkIcoCodec(const SkImageInfo& info, SkTArray, true>* codecs) : INHERITED(info, nullptr) , fEmbeddedCodecs(codecs) - , fCurrScanlineCodec(nullptr) {} /* @@ -264,21 +226,15 @@ SkISize SkIcoCodec::onGetScaledDimensions(float desiredScale) const { return fEmbeddedCodecs->operator[](minIndex)->getInfo().dimensions(); } -int SkIcoCodec::chooseCodec(const SkISize& requestedSize, int startIndex) { - SkASSERT(startIndex >= 0); - +bool SkIcoCodec::onDimensionsSupported(const SkISize& dim) { // FIXME: Cache the index from onGetScaledDimensions? - for (int i = startIndex; i < fEmbeddedCodecs->count(); i++) { - if (fEmbeddedCodecs->operator[](i)->getInfo().dimensions() == requestedSize) { - return i; + for (int32_t i = 0; i < fEmbeddedCodecs->count(); i++) { + if (fEmbeddedCodecs->operator[](i)->getInfo().dimensions() == dim) { + return true; } } - return -1; -} - -bool SkIcoCodec::onDimensionsSupported(const SkISize& dim) { - return this->chooseCodec(dim, 0) >= 0; + return false; } /* @@ -293,89 +249,54 @@ SkCodec::Result SkIcoCodec::onGetPixels(const SkImageInfo& dstInfo, return kUnimplemented; } - if (!ico_conversion_possible(dstInfo)) { + if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) { return kInvalidConversion; } - int index = 0; - SkCodec::Result result = kInvalidScale; - while (true) { - index = this->chooseCodec(dstInfo.dimensions(), index); - if (index < 0) { - break; - } + // We return invalid scale if there is no candidate image with matching + // dimensions. + Result result = kInvalidScale; + for (int32_t i = 0; i < fEmbeddedCodecs->count(); i++) { + SkCodec* embeddedCodec = fEmbeddedCodecs->operator[](i); + // If the dimensions match, try to decode + if (dstInfo.dimensions() == embeddedCodec->getInfo().dimensions()) { - SkCodec* embeddedCodec = fEmbeddedCodecs->operator[](index); - SkImageInfo decodeInfo = fix_embedded_alpha(dstInfo, embeddedCodec->getInfo().alphaType()); - SkASSERT(decodeInfo.colorType() == kN32_SkColorType); - result = embeddedCodec->getPixels(decodeInfo, dst, dstRowBytes, &opts, colorTable, - colorCount); + // Perform the decode + // FIXME (msarett): ICO is considered non-opaque, even if the embedded BMP + // incorrectly claims it has no alpha. + SkAlphaType embeddedAlpha = embeddedCodec->getInfo().alphaType(); + switch (embeddedAlpha) { + case kPremul_SkAlphaType: + case kUnpremul_SkAlphaType: + // Use the requested alpha type if the embedded codec supports alpha. + embeddedAlpha = dstInfo.alphaType(); + break; + case kOpaque_SkAlphaType: + // If the embedded codec claims it is opaque, decode as if it is opaque. + break; + default: + SkASSERT(false); + break; + } + SkImageInfo info = dstInfo.makeAlphaType(embeddedAlpha); + result = embeddedCodec->getPixels(info, dst, dstRowBytes, &opts, colorTable, + colorCount); + // The embedded codec will handle filling incomplete images, so we will indicate + // that all of the rows are initialized. + *rowsDecoded = info.height(); - switch (result) { - case kSuccess: - case kIncompleteInput: - // The embedded codec will handle filling incomplete images, so we will indicate - // that all of the rows are initialized. - *rowsDecoded = decodeInfo.height(); - return result; - default: - // Continue trying to find a valid embedded codec on a failed decode. - break; - } + // On a fatal error, keep trying to find an image to decode + if (kInvalidConversion == result || kInvalidInput == result || + kInvalidScale == result) { + SkCodecPrintf("Warning: Attempt to decode candidate ico failed.\n"); + continue; + } - index++; - } - - SkCodecPrintf("Error: No matching candidate image in ico.\n"); - return result; -} - -SkCodec::Result SkIcoCodec::onStartScanlineDecode(const SkImageInfo& dstInfo, - const SkCodec::Options& options, SkPMColor colorTable[], int* colorCount) { - if (!ico_conversion_possible(dstInfo)) { - return kInvalidConversion; - } - - int index = 0; - SkCodec::Result result = kInvalidScale; - while (true) { - index = this->chooseCodec(dstInfo.dimensions(), index); - if (index < 0) { - break; - } - - SkCodec* embeddedCodec = fEmbeddedCodecs->operator[](index); - SkImageInfo decodeInfo = fix_embedded_alpha(dstInfo, embeddedCodec->getInfo().alphaType()); - result = embeddedCodec->startScanlineDecode(decodeInfo, &options, colorTable, colorCount); - if (kSuccess == result) { - fCurrScanlineCodec = embeddedCodec; + // On success or partial success, return the result return result; } - - index++; } SkCodecPrintf("Error: No matching candidate image in ico.\n"); return result; } - -int SkIcoCodec::onGetScanlines(void* dst, int count, size_t rowBytes) { - SkASSERT(fCurrScanlineCodec); - return fCurrScanlineCodec->getScanlines(dst, count, rowBytes); -} - -bool SkIcoCodec::onSkipScanlines(int count) { - SkASSERT(fCurrScanlineCodec); - return fCurrScanlineCodec->skipScanlines(count); -} - -SkCodec::SkScanlineOrder SkIcoCodec::onGetScanlineOrder() const { - // FIXME: This function will possibly return the wrong value if it is called - // before startScanlineDecode(). - return fCurrScanlineCodec ? fCurrScanlineCodec->getScanlineOrder() : - INHERITED::onGetScanlineOrder(); -} - -SkSampler* SkIcoCodec::getSampler(bool createIfNecessary) { - return fCurrScanlineCodec ? fCurrScanlineCodec->getSampler(createIfNecessary) : nullptr; -} diff --git a/src/codec/SkCodec_libico.h b/src/codec/SkCodec_libico.h index a1d99b884d..92675f4d74 100644 --- a/src/codec/SkCodec_libico.h +++ b/src/codec/SkCodec_libico.h @@ -47,45 +47,17 @@ protected: return kICO_SkEncodedFormat; } - SkScanlineOrder onGetScanlineOrder() const override; - private: - Result onStartScanlineDecode(const SkImageInfo& dstInfo, const SkCodec::Options& options, - SkPMColor inputColorPtr[], int* inputColorCount) override; - - int onGetScanlines(void* dst, int count, size_t rowBytes) override; - - bool onSkipScanlines(int count) override; - - SkSampler* getSampler(bool createIfNecessary) override; - - /* - * Searches fEmbeddedCodecs for a codec that matches requestedSize. - * The search starts at startIndex and ends when an appropriate codec - * is found, or we have reached the end of the array. - * - * @return the index of the matching codec or -1 if there is no - * matching codec between startIndex and the end of - * the array. - */ - int chooseCodec(const SkISize& requestedSize, int startIndex); - /* * Constructor called by NewFromStream * @param embeddedCodecs codecs for the embedded images, takes ownership */ - SkIcoCodec(const SkImageInfo& srcInfo, SkTArray, true>* embeddedCodecs); + SkIcoCodec(const SkImageInfo& srcInfo, + SkTArray, true>* embeddedCodecs); - SkAutoTDelete, true>> fEmbeddedCodecs; // owned - - // Only used by the scanline decoder. onStartScanlineDecode() will set - // fCurrScanlineCodec to one of the fEmbeddedCodecs, if it can find a - // codec of the appropriate size. We will use fCurrScanlineCodec for - // subsequent calls to onGetScanlines() or onSkipScanlines(). - // fCurrScanlineCodec is owned by this class, but should not be an - // SkAutoTDelete. It will be deleted by the destructor of fEmbeddedCodecs. - SkCodec* fCurrScanlineCodec; + SkAutoTDelete, true>> + fEmbeddedCodecs; // owned typedef SkCodec INHERITED; }; diff --git a/src/codec/SkSwizzler.h b/src/codec/SkSwizzler.h index bdb10a1416..7f5bbc6209 100644 --- a/src/codec/SkSwizzler.h +++ b/src/codec/SkSwizzler.h @@ -163,16 +163,6 @@ public: SkSampler::Fill(fillInfo, dst, rowBytes, colorOrIndex, zeroInit); } - /** - * If fSampleX > 1, the swizzler is sampling every fSampleX'th pixel and - * discarding the rest. - * - * This getter is currently used by SkBmpStandardCodec for Bmp-in-Ico decodes. - * Ideally, the subclasses of SkCodec would have no knowledge of sampling, but - * this allows us to apply a transparency mask to pixels after swizzling. - */ - int sampleX() const { return fSampleX; } - private: /** diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp index 03dd16f583..697ba749f7 100644 --- a/tests/CodexTest.cpp +++ b/tests/CodexTest.cpp @@ -401,9 +401,9 @@ DEF_TEST(Codec, r) { // FIXME: We are not ready to test incomplete ICOs // These two tests examine interestingly different behavior: // Decodes an embedded BMP image - check(r, "color_wheel.ico", SkISize::Make(128, 128), true, false, false); + check(r, "color_wheel.ico", SkISize::Make(128, 128), false, false, false); // Decodes an embedded PNG image - check(r, "google_chrome.ico", SkISize::Make(256, 256), true, false, false); + check(r, "google_chrome.ico", SkISize::Make(256, 256), false, false, false); // GIF // FIXME: We are not ready to test incomplete GIFs