src/images/SkImageDecoder_libpng.cpp - Issue 16410009: Add an option to create unpremultiplied bitmaps.

Unified Diff: src/images/SkImageDecoder_libpng.cpp

Issue 16410009: Add an option to create unpremultiplied bitmaps. (Closed) Base URL: https://skia.googlecode.com/svn/trunk

Patch Set: Respond to comments Created 7 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: src/images/SkImageDecoder_libpng.cpp

diff --git a/src/images/SkImageDecoder_libpng.cpp b/src/images/SkImageDecoder_libpng.cpp

index d967a69e84668c502ecf1bbd2c37dead68776447..ac334fc24d55ddd2fd9b3132517c07ea93614fe8 100644

--- a/src/images/SkImageDecoder_libpng.cpp

+++ b/src/images/SkImageDecoder_libpng.cpp

@@ -80,8 +80,9 @@ private:

SkPNGImageIndex* fImageIndex;

bool onDecodeInit(SkStream* stream, png_structp *png_ptrp, png_infop *info_ptrp);

- bool decodePalette(png_structp png_ptr, png_infop info_ptr, bool *hasAlphap,

- bool *reallyHasAlphap, SkColorTable **colorTablep);

+ bool decodePalette(png_structp png_ptr, png_infop info_ptr,

+ bool * SK_RESTRICT hasAlphap, bool *reallyHasAlphap,

+ SkColorTable **colorTablep);

bool getBitmapConfig(png_structp png_ptr, png_infop info_ptr,

SkBitmap::Config *config, bool *hasAlpha,

bool *doDither, SkPMColor *theTranspColor);

@@ -311,7 +312,7 @@ bool SkPNGImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* decodedBitmap,

if (!reuseBitmap) {

decodedBitmap->setConfig(config, sampler.scaledWidth(),

- sampler.scaledHeight(), 0);

+ sampler.scaledHeight());

}

if (SkImageDecoder::kDecodeBounds_Mode == mode) {

return true;

@@ -435,6 +436,13 @@ bool SkPNGImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* decodedBitmap,

if (0 != theTranspColor) {

reallyHasAlpha |= substituteTranspColor(decodedBitmap, theTranspColor);

}

+ if (reallyHasAlpha && this->getRequireUnpremultipliedColors() &&

+ SkBitmap::kARGB_8888_Config != decodedBitmap->config()) {

+ // If the caller wants an unpremultiplied bitmap, and we let them get

+ // away with a config other than 8888, and it has alpha after all,

+ // return false, since the result will have premultiplied colors.

+ return false;

+ }

decodedBitmap->setIsOpaque(!reallyHasAlpha);

if (reuseBitmap) {

decodedBitmap->notifyPixelsChanged();

@@ -445,7 +453,7 @@ bool SkPNGImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* decodedBitmap,

bool SkPNGImageDecoder::getBitmapConfig(png_structp png_ptr, png_infop info_ptr,

- SkBitmap::Config *configp, bool *hasAlphap,

+ SkBitmap::Config *configp, bool * SK_RESTRICT hasAlphap,

bool *doDitherp, SkPMColor *theTranspColorp) {

png_uint_32 origWidth, origHeight;

int bitDepth, colorType;

@@ -546,9 +554,20 @@ bool SkPNGImageDecoder::getBitmapConfig(png_structp png_ptr, png_infop info_ptr,

}

- return this->chooseFromOneChoice(*configp, origWidth, origHeight);

+ if (!this->chooseFromOneChoice(*configp, origWidth, origHeight)) {

+ return false;

+ }

+ // If the image has alpha and the decoder wants unpremultiplied

+ // colors, the only supported config is 8888.

+ if (this->getRequireUnpremultipliedColors() && *hasAlphap) {

+ *configp = SkBitmap::kARGB_8888_Config;

+ }

+ return true;

}

+typedef uint32_t (*PackColorProc)(U8CPU a, U8CPU r, U8CPU g, U8CPU b);

bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr,

bool *hasAlphap, bool *reallyHasAlphap,

SkColorTable **colorTablep) {

@@ -587,9 +606,17 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr,

int index = 0;

int transLessThanFF = 0;

+ // Choose which function to use to create the color table. If the final destination's

+ // config is unpremultiplied, the color table will store unpremultiplied colors.

+ PackColorProc proc;

+ if (this->getRequireUnpremultipliedColors()) {

+ proc = &SkPackARGB32NoCheck;

+ } else {

+ proc = &SkPreMultiplyARGB;

+ }

for (; index < numTrans; index++) {

transLessThanFF |= (int)*trans - 0xFF;

- *colorPtr++ = SkPreMultiplyARGB(*trans++, palette->red, palette->green, palette->blue);

+ *colorPtr++ = proc(*trans++, palette->red, palette->green, palette->blue);

reed1 2013/06/13 13:51:39 Is there a measurable perf-hit with this change (f

scroggo 2013/06/13 19:04:49 We don't actually use an inlineable function (thou

We don't actually use an inlineable function (though SkPreMultiplyARGB calls an inlineable function). I added a benchmark in the next patch. It did not show a consistently substantial difference among what's checked in, this change, or inlining: old: what's checked in; new: this CL: scroggo@scroggo-linux2:/usr/local/google/skia/trunk$ python bench/bench_compare.py -o master.txt -n unpremul.txt image_decode_hal_163x90.png 8888 c 234.07 257.81 -23.74 -10.1% image_decode_hal_163x90.png GPU c 229.48 229.75 -0.27 -0.1% image_decode_box_19_top-left.png GPU g 0.02 0.02 +0.00 +0.0% image_decode_hal_163x90.png GPU g 0.02 0.02 +0.00 +0.0% image_decode_hal_163x90.png 565 c 230.64 230.11 +0.53 +0.2% image_decode_hal_163x90.png NULLGPU c 231.38 230.36 +1.02 +0.4% image_decode_box_19_top-left.png GPU c 7.74 7.68 +0.06 +0.8% image_decode_box_19_top-left.png NULLGPU c 7.86 7.77 +0.09 +1.1% image_decode_box_19_top-left.png 8888 c 7.85 7.64 +0.21 +2.7% image_decode_box_19_top-left.png 565 c 7.80 7.57 +0.23 +2.9% old: inline; new: this CL: scroggo@scroggo-linux2:/usr/local/google/skia/trunk$ python bench/bench_compare.py -o withInline.txt -n unpremul.txt image_decode_hal_163x90.png 8888 c 240.63 257.81 -17.18 -7.1% image_decode_box_19_top-left.png NULLGPU c 7.73 7.77 -0.04 -0.5% image_decode_box_19_top-left.png 8888 c 7.62 7.64 -0.02 -0.3% image_decode_box_19_top-left.png GPU g 0.02 0.02 +0.00 +0.0% image_decode_hal_163x90.png GPU g 0.02 0.02 +0.00 +0.0% image_decode_hal_163x90.png GPU c 230.31 229.75 +0.56 +0.2% image_decode_box_19_top-left.png 565 c 7.60 7.57 +0.03 +0.4% image_decode_hal_163x90.png 565 c 234.79 230.11 +4.68 +2.0% image_decode_box_19_top-left.png GPU c 7.92 7.68 +0.24 +3.0% image_decode_hal_163x90.png NULLGPU c 244.67 230.36 +14.31 +5.8% I ran the benchmark on the only files I have locally that follow this path, but I could try more. Note that all the configs should be the same since the test does not do any drawing (the following patch sets fIsRendering to false), so the variance in configs is just showing noise.

palette++;

}

reallyHasAlpha |= (transLessThanFF < 0);

@@ -679,7 +706,7 @@ bool SkPNGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {

SkScaledBitmapSampler sampler(origWidth, rect.height(), sampleSize);

SkBitmap decodedBitmap;

- decodedBitmap.setConfig(config, sampler.scaledWidth(), sampler.scaledHeight(), 0);

+ decodedBitmap.setConfig(config, sampler.scaledWidth(), sampler.scaledHeight());

// from here down we are concerned with colortables and pixels

@@ -690,7 +717,7 @@ bool SkPNGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {

SkColorTable* colorTable = NULL;

if (colorType == PNG_COLOR_TYPE_PALETTE) {

- decodePalette(png_ptr, info_ptr, &hasAlpha, &reallyHasAlpha, &colorTable);

+ decodePalette(png_ptr, info_ptr, config, &hasAlpha, &reallyHasAlpha, &colorTable);

}

SkAutoUnref aur(colorTable);

« src/core/SkColor.cpp ('K') | « src/images/SkImageDecoder.cpp ('k') | src/images/SkScaledBitmapSampler.h » ('j') | no next file with comments »