Revert of Make SkAndroidCodec support ico (patchset #7 id:130002 of https://codereview.chromium.org/1472933002/ )

Reason for revert:
Crashing: https://uberchromegw.corp.google.com/i/client.skia.android/builders/Test-Android-GCC-NexusPlayer-CPU-SSE4-x86-Release/builds/1499/steps/dm/logs/stdio

Also, related ASAN failures:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-ASAN/builds/3676/steps/dm/logs/stdio

Original issue's description:
> Make SkAndroidCodec support ico
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/1603e9310f62cf0dd543cdb09dea06aa78373f13

TBR=djsollen@google.com,msarett@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1498903004
This commit is contained in:
scroggo 2015-12-04 07:09:57 -08:00 committed by Commit bot
parent 41cbf3f807
commit 97ff7f5662
10 changed files with 93 additions and 242 deletions

View File

@ -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) {

View File

@ -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

View File

@ -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;
}
}

View File

@ -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;

View File

@ -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<const void>(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<SkPMColor>(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, &quotient, &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 {

View File

@ -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<SkColorTable> fColorTable; // owned
const uint32_t fNumColors;
@ -90,7 +85,6 @@ private:
const size_t fSrcRowBytes;
SkAutoTDeleteArray<uint8_t> fSrcBuffer;
const bool fInIco;
const size_t fAndMaskRowBytes; // only used for fInIco decodes
typedef SkBmpCodec INHERITED;
};

View File

@ -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<SkAutoTDelete<SkCodec>, 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;
}

View File

@ -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<SkAutoTDelete<SkCodec>, true>* embeddedCodecs);
SkIcoCodec(const SkImageInfo& srcInfo,
SkTArray<SkAutoTDelete<SkCodec>, true>* embeddedCodecs);
SkAutoTDelete<SkTArray<SkAutoTDelete<SkCodec>, 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<SkTArray<SkAutoTDelete<SkCodec>, true>>
fEmbeddedCodecs; // owned
typedef SkCodec INHERITED;
};

View File

@ -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:
/**

View File

@ -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