AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Set fOptions in SkCodec::getPixels

Browse Source 
Subclasses sometimes try to read fOptions, but it used to not get set
for full decodes. As a result, they might be reading the Options from a
previous scanline/incremental decode. In addition to being wrong, this
is bad in the case of an fSubset pointing to a rectangle that no longer
exists.

So set fOptions in getPixels, prior to any attempts to read it by sub-
classes. Use a different workaround for the webp/incomplete bug. Set
fSubset to null prior to calling fillIncompleteImage. It can only be
non-null for webp, and in that case we do not want the fill call to
be using the subset width.

Modify the Codec_jpeg_rewind test to use an incomplete image, so that
it will also test fillIncompleteImage.

DM tests of inc0.webp and inc1.webp will verify that the incomplete
bug has not resurfaced.

BUG=skia:5772

Change-Id: If5e1e3c9a7f337183783299c0a9e58dcbbc84119
Reviewed-on: https://skia-review.googlesource.com/7682
Commit-Queue: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
This commit is contained in:
Leon Scroggins III 2017-01-27 13:16:28 -05:00 committed by Skia Commit-Bot
parent c121a8849c
commit 428865794b
2 changed files with 20 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/codec/SkCodec.cpp
View File

@ -215,9 +215,7 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
}
fDstInfo = info;
// FIXME: fOptions should be updated to options here, since fillIncompleteImage (called below
// in this method) accesses it. Without updating, it uses the old value.
//fOptions = *options;
fOptions = *options;
// On an incomplete decode, the subclass will specify the number of scanlines that it decoded
// successfully.
@ -235,6 +233,12 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
// their own. They indicate that all of the memory has been filled by
// setting rowsDecoded equal to the height.
if (kIncompleteInput == result && rowsDecoded != info.height()) {
// FIXME: (skbug.com/5772) fillIncompleteImage will fill using the swizzler's width, unless
// there is a subset. In that case, it will use the width of the subset. From here, the
// subset will only be non-null in the case of SkWebpCodec, but it treats the subset
// differenty from the other codecs, and it needs to use the width specified by the info.
// Set the subset to null so SkWebpCodec uses the correct width.
fOptions.fSubset = nullptr;
this->fillIncompleteImage(info, pixels, rowBytes, options->fZeroInitialized, info.height(),
rowsDecoded);
}

21
tests/CodecTest.cpp
View File

@ -1000,11 +1000,13 @@ DEF_TEST(Codec_wbmp_max_size, r) {
DEF_TEST(Codec_jpeg_rewind, r) {
const char* path = "mandrill_512_q075.jpg";
std::unique_ptr<SkStream> stream(GetResourceAsStream(path));
if (!stream) {
sk_sp<SkData> data(GetResourceAsData(path));
if (!data) {
return;
}
std::unique_ptr<SkAndroidCodec> codec(SkAndroidCodec::NewFromStream(stream.release()));
data = SkData::MakeSubset(data.get(), 0, data->size() / 2);
std::unique_ptr<SkAndroidCodec> codec(SkAndroidCodec::NewFromData(data));
if (!codec) {
ERRORF(r, "Unable to create codec '%s'.", path);
return;
@ -1018,13 +1020,13 @@ DEF_TEST(Codec_jpeg_rewind, r) {
// Perform a sampled decode.
SkAndroidCodec::AndroidOptions opts;
opts.fSampleSize = 12;
SkCodec::Result result = codec->getAndroidPixels(
codec->getInfo().makeWH(width / 12, height / 12), pixelStorage.get(), rowBytes, &opts);
REPORTER_ASSERT(r, SkCodec::kSuccess == result);
auto sampledInfo = codec->getInfo().makeWH(width / 12, height / 12);
auto result = codec->getAndroidPixels(sampledInfo, pixelStorage.get(), rowBytes, &opts);
REPORTER_ASSERT(r, SkCodec::kIncompleteInput == result);
// Rewind the codec and perform a full image decode.
result = codec->getPixels(codec->getInfo(), pixelStorage.get(), rowBytes);
REPORTER_ASSERT(r, SkCodec::kSuccess == result);
REPORTER_ASSERT(r, SkCodec::kIncompleteInput == result);
// Now perform a subset decode.
{
@ -1033,14 +1035,17 @@ DEF_TEST(Codec_jpeg_rewind, r) {
opts.fSubset = &subset;
result = codec->getAndroidPixels(codec->getInfo().makeWH(100, 100), pixelStorage.get(),
rowBytes, &opts);
// Though we only have half the data, it is enough to decode this subset.
REPORTER_ASSERT(r, SkCodec::kSuccess == result);
}
// Perform another full image decode. ASAN will detect if we look at the subset when it is
// out of scope. This would happen if we depend on the old state in the codec.
// This tests two layers of bugs: both SkJpegCodec::readRows and SkCodec::fillIncompleteImage
// used to look at the old subset.
opts.fSubset = nullptr;
result = codec->getAndroidPixels(codec->getInfo(), pixelStorage.get(), rowBytes, &opts);
REPORTER_ASSERT(r, SkCodec::kSuccess == result);
REPORTER_ASSERT(r, SkCodec::kIncompleteInput == result);
}
static void check_color_xform(skiatest::Reporter* r, const char* path) {