Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(101)

Unified Diff: src/codec/SkJpegCodec.cpp

Issue 2319293003: Checking for valid colorType, alphaType, colorSpace in SkCodec (Closed)
Patch Set: Created 4 years, 3 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/codec/SkJpegCodec.cpp
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index fcf89d0c1d23b7c3ca6d55ec2674754602778cbb..da608ab5d133bff3f050c763b7e2696f40fceb5e 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -357,7 +357,7 @@ bool SkJpegCodec::onRewind() {
* image has been implemented
* Sets the output color space
*/
-bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColorXform) {
+bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo) {
if (kUnknown_SkAlphaType == dstInfo.alphaType()) {
return false;
}
@@ -384,7 +384,7 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo
case kBGRA_8888_SkColorType:
if (isCMYK) {
fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
- } else if (needsColorXform) {
+ } else if (fColorXform) {
// Our color transformation code requires RGBA order inputs, but it'll swizzle
// to BGRA for us.
fDecoderMgr->dinfo()->out_color_space = JCS_EXT_RGBA;
@@ -393,7 +393,7 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo
}
return true;
case kRGB_565_SkColorType:
- if (needsColorXform) {
+ if (fColorXform) {
return false;
}
@@ -405,14 +405,17 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo
}
return true;
case kGray_8_SkColorType:
- if (needsColorXform || JCS_GRAYSCALE != encodedColorType) {
+ if (fColorXform || JCS_GRAYSCALE != encodedColorType) {
return false;
}
fDecoderMgr->dinfo()->out_color_space = JCS_GRAYSCALE;
return true;
case kRGBA_F16_SkColorType:
- SkASSERT(needsColorXform);
+ SkASSERT(fColorXform);
+ if (!dstInfo.colorSpace()->gammaIsLinear()) {
+ return false;
+ }
if (isCMYK) {
fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
@@ -545,14 +548,16 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
return fDecoderMgr->returnFailure("setjmp", kInvalidInput);
}
- // Check if we can decode to the requested destination and set the output color space
bool needsColorXform = needs_color_xform(dstInfo, this->getInfo());
- if (!this->setOutputColorSpace(dstInfo, needsColorXform)) {
- return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion);
+ if (needsColorXform) {
+ if (!this->initializeColorXform(dstInfo)) {
+ return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters);
+ }
}
- if (!this->initializeColorXform(dstInfo, needsColorXform)) {
- return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters);
+ // Check if we can decode to the requested destination and set the output color space
+ if (!this->setOutputColorSpace(dstInfo)) {
+ return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion);
}
if (!jpeg_start_decompress(dinfo)) {
@@ -630,13 +635,12 @@ void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options&
SkASSERT(fSwizzler);
}
-bool SkJpegCodec::initializeColorXform(const SkImageInfo& dstInfo, bool needsColorXform) {
- if (needsColorXform) {
- fColorXform = SkColorSpaceXform::New(sk_ref_sp(this->getInfo().colorSpace()),
- sk_ref_sp(dstInfo.colorSpace()));
- if (!fColorXform && kRGBA_F16_SkColorType == dstInfo.colorType()) {
- return false;
- }
+bool SkJpegCodec::initializeColorXform(const SkImageInfo& dstInfo) {
+ fColorXform = SkColorSpaceXform::New(sk_ref_sp(this->getInfo().colorSpace()),
+ sk_ref_sp(dstInfo.colorSpace()));
+ // TODO: Can we assert this instead?
msarett 2016/09/08 16:01:58 This can be an assert when Brian's CL lands: https
+ if (!fColorXform) {
+ return false;
}
return true;
@@ -661,14 +665,16 @@ SkCodec::Result SkJpegCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
return kInvalidInput;
}
- // Check if we can decode to the requested destination and set the output color space
bool needsColorXform = needs_color_xform(dstInfo, this->getInfo());
scroggo 2016/09/08 16:33:11 Both callers check this first and otherwise don't
msarett 2016/09/08 16:49:45 Yes, sgtm.
- if (!this->setOutputColorSpace(dstInfo, needsColorXform)) {
- return kInvalidConversion;
+ if (needsColorXform) {
+ if (!this->initializeColorXform(dstInfo)) {
+ return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters);
+ }
}
- if (!this->initializeColorXform(dstInfo, needsColorXform)) {
- return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters);
+ // Check if we can decode to the requested destination and set the output color space
+ if (!this->setOutputColorSpace(dstInfo)) {
+ return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion);
}
if (!jpeg_start_decompress(fDecoderMgr->dinfo())) {
« src/codec/SkCodecPriv.h ('K') | « src/codec/SkJpegCodec.h ('k') | src/codec/SkPngCodec.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698