Incremental decode: only use subset for subsetting

In the initial patch, we modified the subset passed to incremental
decoding to account for sampling in Y. This allowed SkSampledCodec to
use a tighter bounds. But it also means that the implementation cannot
distinguish between lines that are excluded due to subsetting and those
excluded at the beginning and end due to sampling. This becomes
problematic in GIF (crrev.com/2045293002), which may need to fill,
requiring it to reconstruct the actual destination. In truth,
SkGifCodec does not need to support subsets, but cannot distinguish
between the tighter bounds and a true subset.

Fix this by passing the scaled subset to incremental decode, without
using the tighter bounds.

Make SkSampler::rowNeeded take the starting coordinate into account.

In SkPngCodec, compute the number of rows needed in the output, and use
that as a signal to stop.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2420843003

Review-Url: https://codereview.chromium.org/2420843003
This commit is contained in:
scroggo 2016-10-17 14:32:41 -07:00 committed by Commit bot
parent fa1a45ee13
commit 4f2a88cdf0
3 changed files with 23 additions and 33 deletions

View File

@ -458,7 +458,6 @@ public:
SkPngNormalDecoder(const SkEncodedInfo& info, const SkImageInfo& imageInfo, SkStream* stream,
SkPngChunkReader* reader, png_structp png_ptr, png_infop info_ptr, int bitDepth)
: INHERITED(info, imageInfo, stream, reader, png_ptr, info_ptr, bitDepth)
, fLinesDecoded(0)
, fRowsWrittenToOutput(0)
, fDst(nullptr)
, fRowBytes(0)
@ -481,12 +480,6 @@ public:
#endif
private:
// This represents the number of lines reported by libpng, minus any we skipped at the
// beginning. Only used when we are skipping lines (i.e. not in decodeAllRows).
int fLinesDecoded;
// While fLinesDecoded include lines that we skipped, this only includes lines written to the
// output so we can report it to the caller for filling.
// FIXME: Can we remove fLinesDecoded and just rely on fRowsWrittenToOutput?
int fRowsWrittenToOutput;
void* fDst;
size_t fRowBytes;
@ -494,6 +487,7 @@ private:
// Variables for partial decode
int fFirstRow; // FIXME: Move to baseclass?
int fLastRow;
int fRowsNeeded;
typedef SkPngCodec INHERITED;
@ -545,14 +539,18 @@ private:
fLastRow = lastRow;
fDst = dst;
fRowBytes = rowBytes;
fLinesDecoded = 0;
fRowsWrittenToOutput = 0;
fRowsNeeded = fLastRow - fFirstRow + 1;
}
SkCodec::Result decode(int* rowsDecoded) override {
if (this->swizzler()) {
const int sampleY = this->swizzler()->sampleY();
fRowsNeeded = get_scaled_dimension(fLastRow - fFirstRow + 1, sampleY);
}
this->processData();
if (fLinesDecoded == fLastRow - fFirstRow + 1) {
if (fRowsWrittenToOutput == fRowsNeeded) {
return SkCodec::kSuccess;
}
@ -570,17 +568,16 @@ private:
}
SkASSERT(rowNum <= fLastRow);
SkASSERT(fRowsWrittenToOutput < fRowsNeeded);
// If there is no swizzler, all rows are needed.
if (!this->swizzler() || this->swizzler()->rowNeeded(fLinesDecoded)) {
if (!this->swizzler() || this->swizzler()->rowNeeded(rowNum - fFirstRow)) {
this->applyXformRow(fDst, row);
fDst = SkTAddOffset<void>(fDst, fRowBytes);
fRowsWrittenToOutput++;
}
fLinesDecoded++;
if (rowNum == fLastRow) {
if (fRowsWrittenToOutput == fRowsNeeded) {
// Fake error to stop decoding scanlines.
longjmp(PNG_JMPBUF(this->png_ptr()), kStopDecoding);
}
@ -722,17 +719,17 @@ private:
}
return SkCodec::kIncompleteInput;
}
const int lastRow = fLinesDecoded + fFirstRow - 1;
SkASSERT(lastRow <= fLastRow);
const int sampleY = this->swizzler() ? this->swizzler()->sampleY() : 1;
const int rowsNeeded = get_scaled_dimension(fLastRow - fFirstRow + 1, sampleY);
int rowsWrittenToOutput = 0;
// FIXME: For resuming interlace, we may swizzle a row that hasn't changed. But it
// may be too tricky/expensive to handle that correctly.
png_bytep srcRow = fInterlaceBuffer.get();
const int sampleY = this->swizzler() ? this->swizzler()->sampleY() : 1;
void* dst = fDst;
for (int rowNum = fFirstRow; rowNum <= lastRow; rowNum += sampleY) {
for (int rowNum = fFirstRow + get_start_coord(sampleY); rowsWrittenToOutput < rowsNeeded;
rowNum += sampleY) {
this->applyXformRow(dst, srcRow);
dst = SkTAddOffset<void>(dst, fRowBytes);
srcRow = SkTAddOffset<png_byte>(srcRow, fPng_rowbytes * sampleY);

View File

@ -186,7 +186,7 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
// We will need to know about subsetting in the y-dimension in order to use the
// scanline decoder.
// Update the subset to account for scaling done by this->codec().
SkIRect* subsetPtr = options.fSubset;
const SkIRect* subsetPtr = options.fSubset;
// Do the divide ourselves, instead of calling get_scaled_dimension. If
// X and Y are 0, they should remain 0, rather than being upgraded to 1
@ -210,7 +210,7 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
const int samplingOffsetY = get_start_coord(sampleY);
const int startY = samplingOffsetY + subsetY;
int dstHeight = info.height();
const int dstHeight = info.height();
const SkImageInfo nativeInfo = info.makeWH(nativeSize.width(), nativeSize.height());
@ -218,22 +218,15 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
// Although startScanlineDecode expects the bottom and top to match the
// SkImageInfo, startIncrementalDecode uses them to determine which rows to
// decode.
// Note: We *could* use "subsetY" and "subsetHeight" (calculated above) for
// incrementalSubset, but this code gives us a tighter bounds on the subset,
// meaning that we can start with the first row actually needed by the output,
// and stop when we've decoded the last row needed by the output.
SkCodec::Options incrementalOptions = sampledOptions;
SkIRect incrementalSubset;
incrementalSubset.fTop = startY;
incrementalSubset.fBottom = startY + (dstHeight - 1) * sampleY + 1;
if (sampledOptions.fSubset) {
incrementalSubset.fTop = subsetY;
incrementalSubset.fBottom = subsetY + subsetHeight;
incrementalSubset.fLeft = sampledOptions.fSubset->fLeft;
incrementalSubset.fRight = sampledOptions.fSubset->fRight;
} else {
incrementalSubset.fLeft = 0;
incrementalSubset.fRight = nativeSize.width();
incrementalOptions.fSubset = &incrementalSubset;
}
SkCodec::Options incrementalOptions = sampledOptions;
incrementalOptions.fSubset = &incrementalSubset;
const SkCodec::Result startResult = this->codec()->startIncrementalDecode(nativeInfo,
pixels, rowBytes, &incrementalOptions, options.fColorPtr, options.fColorCount);
if (SkCodec::kSuccess == startResult) {

View File

@ -8,6 +8,7 @@
#define SkSampler_DEFINED
#include "SkCodec.h"
#include "SkCodecPriv.h"
#include "SkTypes.h"
class SkSampler : public SkNoncopyable {
@ -37,11 +38,10 @@ public:
/**
* Based on fSampleY, return whether this row belongs in the output.
*
* @param row Row of the image, starting with the first row used in the
* output.
* @param row Row of the image, starting with the first row in the subset.
*/
bool rowNeeded(int row) const {
return row % fSampleY == 0;
return (row + get_start_coord(fSampleY)) % fSampleY == 0;
}
/**