Always encode images with sRGB encoded pixels

Bug: skia:
Change-Id: Icb25bc21a30e88f21df5b0e267d5a3a05535e44a
Reviewed-on: https://skia-review.googlesource.com/19544
Commit-Queue: Matt Sarett <msarett@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>

4 files changed, 39 insertions, 46 deletions

diff --git a/src/images/SkImageEncoderFns.h b/src/images/SkImageEncoderFns.h
index 6a917d0db6..6012a45697 100644
--- a/src/images/SkImageEncoderFns.h
+++ b/src/images/SkImageEncoderFns.h
@@ -15,6 +15,7 @@
 #include "SkBitmap.h"
 #include "SkColor.h"
 #include "SkColorPriv.h"
+#include "SkColorSpace_Base.h"
 #include "SkICC.h"
 #include "SkOpts.h"
 #include "SkPreConfig.h"
@@ -256,6 +257,7 @@ static inline void transform_scanline_F16(char* SK_RESTRICT dst, const char* SK_
                                           int width, int, const SkPMColor*) {
     SkRasterPipeline_<256> p;
     p.append(SkRasterPipeline::load_f16, (const void**) &src);
+    p.append(SkRasterPipeline::to_srgb);
     p.append(SkRasterPipeline::store_u16_be, (void**) &dst);
     p.run(0,0, width);
 }
@@ -268,6 +270,7 @@ static inline void transform_scanline_F16_premul(char* SK_RESTRICT dst, const ch
     SkRasterPipeline_<256> p;
     p.append(SkRasterPipeline::load_f16, (const void**) &src);
     p.append(SkRasterPipeline::unpremul);
+    p.append(SkRasterPipeline::to_srgb);
     p.append(SkRasterPipeline::store_u16_be, (void**) &dst);
     p.run(0,0, width);
 }
@@ -312,10 +315,21 @@ static inline void transform_scanline_F16_to_premul_8888(char* SK_RESTRICT dst,
     p.run(0,0, width);
 }
 
-static inline sk_sp<SkData> icc_from_color_space(const SkColorSpace& cs) {
+static inline sk_sp<SkData> icc_from_color_space(const SkImageInfo& info) {
+    SkColorSpace* cs = info.colorSpace();
+    if (!cs) {
+        return nullptr;
+    }
+
+    sk_sp<SkColorSpace> owned;
+    if (kRGBA_F16_SkColorType == info.colorType()) {
+        owned = as_CSB(cs)->makeSRGBGamma();
+        cs = owned.get();
+    }
+
     SkColorSpaceTransferFn fn;
     SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor);
-    if (cs.isNumericalTransferFn(&fn) && cs.toXYZD50(&toXYZD50)) {
+    if (cs->isNumericalTransferFn(&fn) && cs->toXYZD50(&toXYZD50)) {
         return SkICC::WriteToICC(fn, toXYZD50);
     }
 
diff --git a/src/images/SkJpegEncoder.cpp b/src/images/SkJpegEncoder.cpp
index eaea77e0e0..a0ee398615 100644
--- a/src/images/SkJpegEncoder.cpp
+++ b/src/images/SkJpegEncoder.cpp
@@ -207,29 +207,19 @@ std::unique_ptr<SkEncoder> SkJpegEncoder::Make(SkWStream* dst, const SkPixmap& s
     jpeg_set_quality(encoderMgr->cinfo(), options.fQuality, TRUE);
     jpeg_start_compress(encoderMgr->cinfo(), TRUE);
 
-    if (SkColorSpace* cs = src.colorSpace()) {
-        sk_sp<SkColorSpace> owned;
-        if (src.colorType() == kRGBA_F16_SkColorType) {
-            // We'll be converting to 8-bit sRGB, so we'd better tag it that way.
-            owned = as_CSB(src.colorSpace())->makeSRGBGamma();
-            cs = owned.get();
-        }
-
-        sk_sp<SkData> icc = icc_from_color_space(*cs);
-        if (icc) {
-            // Create a contiguous block of memory with the icc signature followed by the profile.
-            sk_sp<SkData> markerData =
-                    SkData::MakeUninitialized(kICCMarkerHeaderSize + icc->size());
-            uint8_t* ptr = (uint8_t*) markerData->writable_data();
-            memcpy(ptr, kICCSig, sizeof(kICCSig));
-            ptr += sizeof(kICCSig);
-            *ptr++ = 1; // This is the first marker.
-            *ptr++ = 1; // Out of one total markers.
-            memcpy(ptr, icc->data(), icc->size());
-
-            jpeg_write_marker(encoderMgr->cinfo(), kICCMarker, markerData->bytes(),
-                              markerData->size());
-        }
+    sk_sp<SkData> icc = icc_from_color_space(src.info());
+    if (icc) {
+        // Create a contiguous block of memory with the icc signature followed by the profile.
+        sk_sp<SkData> markerData =
+                SkData::MakeUninitialized(kICCMarkerHeaderSize + icc->size());
+        uint8_t* ptr = (uint8_t*) markerData->writable_data();
+        memcpy(ptr, kICCSig, sizeof(kICCSig));
+        ptr += sizeof(kICCSig);
+        *ptr++ = 1; // This is the first marker.
+        *ptr++ = 1; // Out of one total markers.
+        memcpy(ptr, icc->data(), icc->size());
+
+        jpeg_write_marker(encoderMgr->cinfo(), kICCMarker, markerData->bytes(), markerData->size());
     }
 
     return std::unique_ptr<SkJpegEncoder>(new SkJpegEncoder(std::move(encoderMgr), src));
diff --git a/src/images/SkPngEncoder.cpp b/src/images/SkPngEncoder.cpp
index cc925207b1..b0c2b23dfb 100644
--- a/src/images/SkPngEncoder.cpp
+++ b/src/images/SkPngEncoder.cpp
@@ -53,7 +53,7 @@ public:
     bool setHeader(const SkImageInfo& srcInfo, const SkPngEncoder::Options& options);
     bool setPalette(const SkImageInfo& srcInfo, SkColorTable* colorTable,
                     SkTransferFunctionBehavior);
-    bool setColorSpace(SkColorSpace* colorSpace);
+    bool setColorSpace(const SkImageInfo& info);
     bool writeInfo(const SkImageInfo& srcInfo);
     void chooseProc(const SkImageInfo& srcInfo, SkTransferFunctionBehavior unpremulBehavior);
 
@@ -355,8 +355,8 @@ bool SkPngEncoderMgr::setPalette(const SkImageInfo& srcInfo, SkColorTable* color
     return true;
 }
 
-static void set_icc(png_structp png_ptr, png_infop info_ptr, const SkColorSpace& colorSpace) {
-    sk_sp<SkData> icc = icc_from_color_space(colorSpace);
+static void set_icc(png_structp png_ptr, png_infop info_ptr, const SkImageInfo& info) {
+    sk_sp<SkData> icc = icc_from_color_space(info);
     if (!icc) {
         return;
     }
@@ -372,17 +372,15 @@ static void set_icc(png_structp png_ptr, png_infop info_ptr, const SkColorSpace&
     png_set_iCCP(png_ptr, info_ptr, name, 0, iccPtr, icc->size());
 }
 
-bool SkPngEncoderMgr::setColorSpace(SkColorSpace* colorSpace) {
+bool SkPngEncoderMgr::setColorSpace(const SkImageInfo& info) {
     if (setjmp(png_jmpbuf(fPngPtr))) {
         return false;
     }
 
-    if (colorSpace) {
-        if (colorSpace->isSRGB()) {
-            png_set_sRGB(fPngPtr, fInfoPtr, PNG_sRGB_INTENT_PERCEPTUAL);
-        } else {
-            set_icc(fPngPtr, fInfoPtr, *colorSpace);
-        }
+    if (info.colorSpace() && info.colorSpace()->isSRGB()) {
+        png_set_sRGB(fPngPtr, fInfoPtr, PNG_sRGB_INTENT_PERCEPTUAL);
+    } else {
+        set_icc(fPngPtr, fInfoPtr, info);
     }
 
     return true;
@@ -429,7 +427,7 @@ std::unique_ptr<SkEncoder> SkPngEncoder::Make(SkWStream* dst, const SkPixmap& sr
         return nullptr;
     }
 
-    if (!encoderMgr->setColorSpace(src.colorSpace())) {
+    if (!encoderMgr->setColorSpace(src.info())) {
         return nullptr;
     }
 
diff --git a/src/images/SkWebpEncoder.cpp b/src/images/SkWebpEncoder.cpp
index 81ed7e1845..296d4f4cf0 100644
--- a/src/images/SkWebpEncoder.cpp
+++ b/src/images/SkWebpEncoder.cpp
@@ -194,16 +194,7 @@ bool SkWebpEncoder::Encode(SkWStream* stream, const SkPixmap& pixmap, const Opti
     // If there is no need to embed an ICC profile, we write directly to the input stream.
     // Otherwise, we will first encode to |tmp| and use a mux to add the ICC chunk.  libwebp
     // forces us to have an encoded image before we can add a profile.
-    sk_sp<SkData> icc;
-    if (SkColorSpace* cs = pixmap.colorSpace()) {
-        sk_sp<SkColorSpace> owned;
-        if (pixmap.colorType() == kRGBA_F16_SkColorType) {
-            // We'll be converting to 8-bit sRGB, so we'd better tag it that way.
-            owned = as_CSB(pixmap.colorSpace())->makeSRGBGamma();
-            cs = owned.get();
-        }
-        icc = icc_from_color_space(*cs);
-    }
+    sk_sp<SkData> icc = icc_from_color_space(pixmap.info());
     SkDynamicMemoryWStream tmp;
     pic.custom_ptr = icc ? (void*)&tmp : (void*)stream;