Make SkCodec truly default to sRGB
Remove SkEncodedInfo::ICCProfile::MakeSRGB. Instead of creating
this object whenever there is no encoded color profile, just
treat null as sRGB, like skcms does.
This may help with crbug.com/887372. Regardless it simplifies the
code.
Also fix a bug where SkCodec could have passed a null
skcms_ICCProfile to skcms_ApproximatelyEqualProfiles (related
to b/116608007).
Bug: chromium:887372
Change-Id: I2374e8d8a1aed261f1291b7f6fd6c7ea662f26fd
Reviewed-on: https://skia-review.googlesource.com/157561
Auto-Submit: Leon Scroggins <scroggo@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 02d13dd..cb57f5b 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -481,7 +481,7 @@
SkASSERT(!inIco || nullptr != stream->getMemoryBase());
// Set the image info and create a codec.
- auto info = SkEncodedInfo::MakeSRGB(width, height, color, alpha, bitsPerComponent);
+ auto info = SkEncodedInfo::Make(width, height, color, alpha, bitsPerComponent);
codecOut->reset(new SkBmpStandardCodec(std::move(info),
std::unique_ptr<SkStream>(stream),
bitsPerPixel, numColors, bytesPerColor,
@@ -539,7 +539,7 @@
color = SkEncodedInfo::kBGR_Color;
alpha = SkEncodedInfo::kOpaque_Alpha;
}
- auto info = SkEncodedInfo::MakeSRGB(width, height, color, alpha, 8);
+ auto info = SkEncodedInfo::Make(width, height, color, alpha, 8);
codecOut->reset(new SkBmpMaskCodec(std::move(info),
std::unique_ptr<SkStream>(stream), bitsPerPixel,
masks.release(), rowOrder));
@@ -570,7 +570,7 @@
// is uncommon, but we cannot be certain that an RLE bmp will be
// opaque or that we will be able to represent it with a palette.
// For that reason, we always indicate that we are kBGRA.
- auto info = SkEncodedInfo::MakeSRGB(width, height, SkEncodedInfo::kBGRA_Color,
+ auto info = SkEncodedInfo::Make(width, height, SkEncodedInfo::kBGRA_Color,
SkEncodedInfo::kBinary_Alpha, 8);
codecOut->reset(new SkBmpRLECodec(std::move(info),
std::unique_ptr<SkStream>(stream), bitsPerPixel,
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 2c5c3df..ecf253f 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -624,9 +624,16 @@
bool needsColorXform = false;
if (this->usesColorXform() && dstInfo.colorSpace()) {
dstInfo.colorSpace()->toProfile(&fDstProfile);
- if (kRGBA_F16_SkColorType == dstInfo.colorType()
- || !skcms_ApproximatelyEqualProfiles(fEncodedInfo.profile(), &fDstProfile) ) {
+ if (kRGBA_F16_SkColorType == dstInfo.colorType()) {
needsColorXform = true;
+ } else {
+ const auto* srcProfile = fEncodedInfo.profile();
+ if (!srcProfile) {
+ srcProfile = skcms_sRGB_profile();
+ }
+ if (!skcms_ApproximatelyEqualProfiles(srcProfile, &fDstProfile) ) {
+ needsColorXform = true;
+ }
}
}
@@ -653,8 +660,8 @@
}
void SkCodec::applyColorXform(void* dst, const void* src, int count) const {
+ // It is okay for srcProfile to be null. This will use sRGB.
const auto* srcProfile = fEncodedInfo.profile();
- SkASSERT(srcProfile);
SkAssertResult(skcms_Transform(src, fSrcXformFormat, skcms_AlphaFormat_Unpremul, srcProfile,
dst, fDstXformFormat, fDstXformAlphaFormat, &fDstProfile,
count));
diff --git a/src/codec/SkEncodedInfo.cpp b/src/codec/SkEncodedInfo.cpp
index 75c5062..93b960b 100644
--- a/src/codec/SkEncodedInfo.cpp
+++ b/src/codec/SkEncodedInfo.cpp
@@ -17,10 +17,6 @@
return nullptr;
}
-std::unique_ptr<SkEncodedInfo::ICCProfile> SkEncodedInfo::ICCProfile::MakeSRGB() {
- return std::unique_ptr<ICCProfile>(new ICCProfile(*skcms_sRGB_profile()));
-}
-
std::unique_ptr<SkEncodedInfo::ICCProfile> SkEncodedInfo::ICCProfile::Make(
const skcms_ICCProfile& profile) {
return std::unique_ptr<ICCProfile>(new ICCProfile(profile));
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 77160cb..34dfbef 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -91,8 +91,8 @@
// Use kPalette since Gifs are encoded with a color table.
// FIXME: Gifs can actually be encoded with 4-bits per pixel. Using 8 works, but we could skip
// expanding to 8 bits and take advantage of the SkSwizzler to work from 4.
- auto encodedInfo = SkEncodedInfo::MakeSRGB(reader->screenWidth(), reader->screenHeight(),
- SkEncodedInfo::kPalette_Color, alpha, 8);
+ auto encodedInfo = SkEncodedInfo::Make(reader->screenWidth(), reader->screenHeight(),
+ SkEncodedInfo::kPalette_Color, alpha, 8);
return std::unique_ptr<SkCodec>(new SkGifCodec(std::move(encodedInfo), reader.release()));
}
diff --git a/src/codec/SkHeifCodec.cpp b/src/codec/SkHeifCodec.cpp
index f5785d6..c6ce0b9 100644
--- a/src/codec/SkHeifCodec.cpp
+++ b/src/codec/SkHeifCodec.cpp
@@ -139,8 +139,9 @@
auto icc = SkData::MakeWithCopy(frameInfo.mIccData.get(), frameInfo.mIccSize);
profile = SkEncodedInfo::ICCProfile::Make(std::move(icc));
}
- if (!profile || profile->profile()->data_color_space != skcms_Signature_RGB) {
- profile = SkEncodedInfo::ICCProfile::MakeSRGB();
+ if (profile && profile->profile()->data_color_space != skcms_Signature_RGB) {
+ // This will result in sRGB.
+ profile = nullptr;
}
SkEncodedInfo info = SkEncodedInfo::Make(frameInfo.mWidth, frameInfo.mHeight,
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 21570fd..926fedf 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -282,9 +282,7 @@
std::unique_ptr<SkCodec> SkJpegCodec::MakeFromStream(std::unique_ptr<SkStream> stream,
Result* result) {
- return SkJpegCodec::MakeFromStream(std::move(stream), result,
- // FIXME: This may not be used. Can we skip creating it?
- SkEncodedInfo::ICCProfile::MakeSRGB());
+ return SkJpegCodec::MakeFromStream(std::move(stream), result, nullptr);
}
std::unique_ptr<SkCodec> SkJpegCodec::MakeFromStream(std::unique_ptr<SkStream> stream,
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index ece8f7b..288870a 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -371,10 +371,10 @@
if (png_get_valid(png_ptr, info_ptr, PNG_INFO_sRGB)) {
// sRGB chunks also store a rendering intent: Absolute, Relative,
// Perceptual, and Saturation.
- // FIXME (msarett): Extract this information from the sRGB chunk once
+ // FIXME (scroggo): Extract this information from the sRGB chunk once
// we are able to handle this information in
- // SkColorSpace.
- return SkEncodedInfo::ICCProfile::MakeSRGB();
+ // skcms_ICCProfile
+ return nullptr;
}
// Default to SRGB gamut.
@@ -422,7 +422,7 @@
return SkEncodedInfo::ICCProfile::Make(skcmsProfile);
#else // LIBPNG >= 1.6
- return SkEncodedInfo::ICCProfile::MakeSRGB();
+ return nullptr;
#endif // LIBPNG >= 1.6
}
@@ -930,10 +930,6 @@
break;
}
}
- if (!profile) {
- // Treat unsupported/invalid color spaces as sRGB.
- profile = SkEncodedInfo::ICCProfile::MakeSRGB();
- }
if (encodedColorType == PNG_COLOR_TYPE_GRAY_ALPHA) {
png_color_8p sigBits;
diff --git a/src/codec/SkRawCodec.cpp b/src/codec/SkRawCodec.cpp
index 30a3d7b..2b361b5 100644
--- a/src/codec/SkRawCodec.cpp
+++ b/src/codec/SkRawCodec.cpp
@@ -649,20 +649,14 @@
}
std::unique_ptr<SkEncodedInfo::ICCProfile> profile;
- switch (imageData.color_space) {
- case ::piex::PreviewImageData::kSrgb:
- profile = SkEncodedInfo::ICCProfile::MakeSRGB();
- break;
- case ::piex::PreviewImageData::kAdobeRgb: {
- constexpr skcms_TransferFunction twoDotTwo =
- { 2.2f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f };
- skcms_ICCProfile skcmsProfile;
- skcms_Init(&skcmsProfile);
- skcms_SetTransferFunction(&skcmsProfile, &twoDotTwo);
- skcms_SetXYZD50(&skcmsProfile, &gAdobe_RGB_to_XYZD50);
- profile = SkEncodedInfo::ICCProfile::Make(skcmsProfile);
- break;
- }
+ if (imageData.color_space == ::piex::PreviewImageData::kAdobeRgb) {
+ constexpr skcms_TransferFunction twoDotTwo =
+ { 2.2f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f };
+ skcms_ICCProfile skcmsProfile;
+ skcms_Init(&skcmsProfile);
+ skcms_SetTransferFunction(&skcmsProfile, &twoDotTwo);
+ skcms_SetXYZD50(&skcmsProfile, &gAdobe_RGB_to_XYZD50);
+ profile = SkEncodedInfo::ICCProfile::Make(skcmsProfile);
}
// Theoretically PIEX can return JPEG compressed image or uncompressed RGB image. We only
@@ -806,8 +800,8 @@
SkRawCodec::~SkRawCodec() {}
SkRawCodec::SkRawCodec(SkDngImage* dngImage)
- : INHERITED(SkEncodedInfo::MakeSRGB(dngImage->width(), dngImage->height(),
- SkEncodedInfo::kRGB_Color,
- SkEncodedInfo::kOpaque_Alpha, 8),
+ : INHERITED(SkEncodedInfo::Make(dngImage->width(), dngImage->height(),
+ SkEncodedInfo::kRGB_Color,
+ SkEncodedInfo::kOpaque_Alpha, 8),
skcms_PixelFormat_RGBA_8888, nullptr)
, fDngImage(dngImage) {}
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index ac1c9be..2737806 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -97,8 +97,8 @@
auto chunk = SkData::MakeWithCopy(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
profile = SkEncodedInfo::ICCProfile::Make(std::move(chunk));
}
- if (!profile || profile->profile()->data_color_space != skcms_Signature_RGB) {
- profile = SkEncodedInfo::ICCProfile::MakeSRGB();
+ if (profile && profile->profile()->data_color_space != skcms_Signature_RGB) {
+ profile = nullptr;
}
}