Add support for writing ICC profiles in png encodes
authorMatt Sarett <msarett@google.com>
Wed, 15 Mar 2017 21:50:08 +0000 (17:50 -0400)
committerSkia Commit-Bot <skia-commit-bot@chromium.org>
Tue, 21 Mar 2017 00:05:30 +0000 (00:05 +0000)
BUG=skia:

Change-Id: I99eb2f157f249ed09d724461ec4a1e31db70816a
Reviewed-on: https://skia-review.googlesource.com/9782
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>

gm/encode-srgb.cpp
src/images/SkImageEncoderPriv.h
src/images/SkJPEGImageEncoder.cpp
src/images/SkPNGImageEncoder.cpp
src/images/SkWEBPImageEncoder.cpp
tests/CodecTest.cpp

index 991e6a1..2a89ea3 100644 (file)
@@ -119,7 +119,7 @@ static sk_sp<SkData> encode_data(const SkBitmap& bitmap, SkEncodedImageFormat fo
 
     SkEncodeOptions options;
     if (bitmap.colorSpace()) {
-        options.fPremulBehavior = SkEncodeOptions::PremulBehavior::kGammaCorrect;
+        options.fColorBehavior = SkEncodeOptions::ColorBehavior::kCorrect;
     }
 
     switch (format) {
index 5f7560d..69d45fb 100644 (file)
 #include "SkImageEncoder.h"
 
 struct SkEncodeOptions {
-    enum class PremulBehavior {
+    enum class ColorBehavior {
          // Convert to a linear space before premultiplying or unpremultiplying.
-        kGammaCorrect,
+        kCorrect,
 
         // Ignore the transfer function when premultiplying or unpremultiplying.
         kLegacy,
     };
 
-    PremulBehavior fPremulBehavior = PremulBehavior::kLegacy;
+    ColorBehavior fColorBehavior = ColorBehavior::kLegacy;
 };
 
 #ifdef SK_HAS_JPEG_LIBRARY
index c37aa46..1f8f0d6 100644 (file)
@@ -86,10 +86,10 @@ bool SkEncodeImageAsJPEG(SkWStream* stream, const SkPixmap& pixmap, const SkEnco
             pixmap.colorSpace()->gammaIsLinear());
 
     SkPixmap src = pixmap;
-    if (SkEncodeOptions::PremulBehavior::kLegacy == opts.fPremulBehavior) {
+    if (SkEncodeOptions::ColorBehavior::kLegacy == opts.fColorBehavior) {
         src.setColorSpace(nullptr);
     } else {
-        // kGammaCorrect behavior requires a color space.  It's not actually critical in the
+        // kCorrect behavior requires a color space.  It's not actually critical in the
         // jpeg case (since jpegs are opaque), but Skia color correct behavior generally
         // requires pixels to be tagged with color spaces.
         if (!src.colorSpace()) {
index e4a60f9..69a7b07 100644 (file)
@@ -12,6 +12,7 @@
 #include "SkColor.h"
 #include "SkColorPriv.h"
 #include "SkDither.h"
+#include "SkICC.h"
 #include "SkMath.h"
 #include "SkStream.h"
 #include "SkTemplates.h"
@@ -168,7 +169,7 @@ bool SkEncodeImageAsPNG(SkWStream* stream, const SkPixmap& src, const SkEncodeOp
              src.colorSpace()->gammaIsLinear());
 
     SkPixmap pixmap = src;
-    if (SkEncodeOptions::PremulBehavior::kLegacy == opts.fPremulBehavior) {
+    if (SkEncodeOptions::ColorBehavior::kLegacy == opts.fColorBehavior) {
         pixmap.setColorSpace(nullptr);
     } else {
         if (!pixmap.colorSpace()) {
@@ -334,6 +335,21 @@ static bool do_encode(SkWStream* stream, const SkPixmap& pixmap,
         }
     }
 
+    if (pixmap.colorSpace()) {
+        SkColorSpaceTransferFn fn;
+        SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor);
+        if (pixmap.colorSpace()->isSRGB()) {
+            png_set_sRGB(png_ptr, info_ptr, PNG_sRGB_INTENT_PERCEPTUAL);
+        } else if (pixmap.colorSpace()->isNumericalTransferFn(&fn) &&
+                   pixmap.colorSpace()->toXYZD50(&toXYZD50))
+        {
+            sk_sp<SkData> icc = SkICC::WriteToICC(fn, toXYZD50);
+            png_set_iCCP(png_ptr, info_ptr, "Skia", 0, icc->bytes(), icc->size());
+        }
+
+        // TODO: Should we support writing ICC profiles for additional color spaces?
+    }
+
     png_set_sBIT(png_ptr, info_ptr, &sig_bit);
     png_write_info(png_ptr, info_ptr);
     int pngBytesPerPixel = num_components(pngColorType) * (bitDepth / 8);
index 5928f97..8797ff5 100644 (file)
@@ -126,7 +126,7 @@ static bool do_encode(SkWStream* stream, const SkPixmap& srcPixmap, const SkEnco
             srcPixmap.colorSpace()->gammaIsLinear());
 
     SkPixmap pixmap = srcPixmap;
-    if (SkEncodeOptions::PremulBehavior::kLegacy == opts.fPremulBehavior) {
+    if (SkEncodeOptions::ColorBehavior::kLegacy == opts.fColorBehavior) {
         pixmap.setColorSpace(nullptr);
     } else {
         if (!pixmap.colorSpace()) {
index 265b51e..65e5475 100644 (file)
 #include "SkCodec.h"
 #include "SkCodecImageGenerator.h"
 #include "SkColorSpace_XYZ.h"
+#include "SkColorSpacePriv.h"
 #include "SkData.h"
 #include "SkFrontBufferedStream.h"
 #include "SkImageEncoder.h"
+#include "SkImageEncoderPriv.h"
 #include "SkMD5.h"
 #include "SkOSPath.h"
 #include "SkPngChunkReader.h"
@@ -1527,3 +1529,42 @@ DEF_TEST(Codec_InvalidAnimated, r) {
         codec->incrementalDecode();
     }
 }
+
+DEF_TEST(Codec_EncodeICC, r) {
+    // Test with sRGB color space.
+    SkBitmap srgbBitmap;
+    SkImageInfo srgbInfo = SkImageInfo::MakeS32(1, 1, kOpaque_SkAlphaType);
+    srgbBitmap.allocPixels(srgbInfo);
+    *srgbBitmap.getAddr32(0, 0) = 0;
+    SkPixmap pixmap;
+    srgbBitmap.peekPixels(&pixmap);
+    SkDynamicMemoryWStream srgbBuf;
+    SkEncodeOptions opts;
+    opts.fColorBehavior = SkEncodeOptions::ColorBehavior::kCorrect;
+    SkEncodeImageAsPNG(&srgbBuf, pixmap, opts);
+    sk_sp<SkData> srgbData = srgbBuf.detachAsData();
+    std::unique_ptr<SkCodec> srgbCodec(SkCodec::NewFromData(srgbData));
+    REPORTER_ASSERT(r, srgbCodec->getInfo().colorSpace() == SkColorSpace::MakeSRGB().get());
+
+    // Test with P3 color space.
+    SkDynamicMemoryWStream p3Buf;
+    sk_sp<SkColorSpace> p3 = SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
+                                                   SkColorSpace::kDCIP3_D65_Gamut);
+    pixmap.setColorSpace(p3);
+    SkEncodeImageAsPNG(&p3Buf, pixmap, opts);
+    sk_sp<SkData> p3Data = p3Buf.detachAsData();
+    std::unique_ptr<SkCodec> p3Codec(SkCodec::NewFromData(p3Data));
+    REPORTER_ASSERT(r, p3Codec->getInfo().colorSpace()->gammaCloseToSRGB());
+    SkMatrix44 mat0(SkMatrix44::kUninitialized_Constructor);
+    SkMatrix44 mat1(SkMatrix44::kUninitialized_Constructor);
+    bool success = p3->toXYZD50(&mat0);
+    REPORTER_ASSERT(r, success);
+    success = p3Codec->getInfo().colorSpace()->toXYZD50(&mat1);
+    REPORTER_ASSERT(r, success);
+
+    for (int i = 0; i < 4; i++) {
+        for (int j = 0; j < 4; j++) {
+            REPORTER_ASSERT(r, color_space_almost_equal(mat0.get(0, 0), mat1.get(0, 0)));
+        }
+    }
+}