Ensure embedded profile, if any, is always used

Perform sRGB conversion at end of pipe only

withMetadata exports profile, should not convert

Convert one fixture to sRGB to help test

Discovered while investigating #125

README.md

@@ -10,7 +10,9 @@
 
 The typical use case for this high speed Node.js module is to convert large images of many formats to smaller, web-friendly JPEG, PNG and WebP images of varying dimensions.
 
-This module supports reading and writing JPEG, PNG and WebP images to and from Streams, Buffer objects and the filesystem. It also supports reading images of many other types from the filesystem via libmagick++ or libgraphicsmagick++ if present.
+This module supports reading and writing JPEG, PNG and WebP images to and from Streams, Buffer objects and the filesystem.
+It also supports reading images of many other types from the filesystem via libmagick++ or libgraphicsmagick++ if present.
+Colour spaces, embedded ICC profiles and alpha transparency channels are all handled correctly.
 
 Only small regions of uncompressed image data are held in memory and processed at a time, taking full advantage of multiple CPU cores and L1/L2/L3 cache. Resizing an image is typically 4x faster than using the quickest ImageMagick and GraphicsMagick settings.
 
@@ -401,7 +403,9 @@ Use progressive (interlace) scan for JPEG and PNG output. This typically reduces
 
 #### withMetadata()
 
-Include all metadata (ICC, EXIF, XMP) from the input image in the output image. The default behaviour is to strip all metadata.
+Include all metadata (ICC, EXIF, XMP) from the input image in the output image.
+
+The default behaviour is to strip all metadata and convert to the device-independent sRGB colour space.
 
 #### compressionLevel(compressionLevel)
 

src/common.cc

@@ -117,6 +117,13 @@ namespace sharp {
     return image;
   }
 
+  /*
+    Does this image have an embedded profile?
+  */
+  bool HasProfile(VipsImage *image) {
+    return (vips_image_get_typeof(image, VIPS_META_ICC_NAME) > 0) ? TRUE : FALSE;
+  }
+
   /*
     Does this image have an alpha channel?
     Uses colour space interpretation with number of channels to guess this.

src/common.h

@@ -44,6 +44,11 @@ namespace sharp {
   */
   VipsImage* InitImage(ImageType imageType, char const *file, VipsAccess const access);
 
+  /*
+    Does this image have an embedded profile?
+  */
+  bool HasProfile(VipsImage *image);
+
   /*
     Does this image have an alpha channel?
     Uses colour space interpretation with number of channels to guess this.

src/metadata.cc

@@ -74,7 +74,7 @@ class MetadataWorker : public NanAsyncWorker {
       baton->height = image->Ysize;
       baton->space = vips_enum_nick(VIPS_TYPE_INTERPRETATION, image->Type);
       baton->channels = image->Bands;
-      baton->hasProfile = (vips_image_get_typeof(image, VIPS_META_ICC_NAME) > 0) ? TRUE : FALSE;
+      baton->hasProfile = HasProfile(image);
       // Derived attributes
       baton->hasAlpha = HasAlpha(image);
       baton->orientation = ExifOrientation(image);

src/resize.cc

@@ -269,34 +269,24 @@ class ResizeWorker : public NanAsyncWorker {
       image = shrunkOnLoad;
     }
 
-    // Handle colour profile, if any, for non sRGB images
+    // Ensure we're using a device-independent colour space
-    if (image->Type != VIPS_INTERPRETATION_sRGB) {
+    if (HasProfile(image)) {
-      // Get the input colour profile
+      // Convert to CIELAB using embedded profile
-      if (vips_image_get_typeof(image, VIPS_META_ICC_NAME)) {
       VipsImage *profile;
-        // Use embedded profile
       if (vips_icc_import(image, &profile, "pcs", VIPS_PCS_XYZ, "embedded", TRUE, NULL)) {
         return Error(baton, hook);
       }
       vips_object_local(hook, profile);
       image = profile;
     } else if (image->Type == VIPS_INTERPRETATION_CMYK) {
+      // Convert to CIELAB using default "USWebCoatedSWOP" CMYK profile
       VipsImage *profile;
-        // CMYK with no embedded profile
       if (vips_icc_import(image, &profile, "pcs", VIPS_PCS_XYZ, "input_profile", (baton->iccProfileCmyk).c_str(), NULL)) {
         return Error(baton, hook);
       }
       vips_object_local(hook, profile);
       image = profile;
     }
-      // Attempt to convert to sRGB colour space
-      VipsImage *colourspaced;
-      if (vips_colourspace(image, &colourspaced, VIPS_INTERPRETATION_sRGB, NULL)) {
-        return Error(baton, hook);
-      }
-      vips_object_local(hook, colourspaced);
-      image = colourspaced;
-    }
 
     // Flatten image to remove alpha channel
     if (baton->flatten && HasAlpha(image)) {
@@ -578,14 +568,22 @@ class ResizeWorker : public NanAsyncWorker {
       image = gammaDecoded;
     }
 
-    // Convert to sRGB colour space, if not already
+    // Convert colour space to either sRGB or RGB-with-profile, if not already
     if (image->Type != VIPS_INTERPRETATION_sRGB) {
-      VipsImage *colourspaced;
+      VipsImage *rgb;
-      if (vips_colourspace(image, &colourspaced, VIPS_INTERPRETATION_sRGB, NULL)) {
+      if (baton->withMetadata && HasProfile(image)) {
+        // Convert to device-dependent RGB using embedded profile of input
+        if (vips_icc_export(image, &rgb, NULL)) {
           return Error(baton, hook);
         }
-      vips_object_local(hook, colourspaced);
+      } else {
-      image = colourspaced;
+        // Convert to device-independent sRGB
+        if (vips_colourspace(image, &rgb, VIPS_INTERPRETATION_sRGB, NULL)) {
+          return Error(baton, hook);
+        }
+      }
+      vips_object_local(hook, rgb);
+      image = rgb;
     }
 
 #if !(VIPS_MAJOR_VERSION >= 7 && VIPS_MINOR_VERSION >= 40 && VIPS_MINOR_VERSION >= 5)

test/unit/metadata.js

@@ -18,7 +18,7 @@ describe('Image metadata', function() {
       assert.strictEqual(2225, metadata.height);
       assert.strictEqual('srgb', metadata.space);
       assert.strictEqual(3, metadata.channels);
-      assert.strictEqual(true, metadata.hasProfile);
+      assert.strictEqual(false, metadata.hasProfile);
       assert.strictEqual(false, metadata.hasAlpha);
       assert.strictEqual('undefined', typeof metadata.orientation);
       done();
@@ -116,7 +116,7 @@ describe('Image metadata', function() {
       assert.strictEqual(2225, metadata.height);
       assert.strictEqual('srgb', metadata.space);
       assert.strictEqual(3, metadata.channels);
-      assert.strictEqual(true, metadata.hasProfile);
+      assert.strictEqual(false, metadata.hasProfile);
       assert.strictEqual(false, metadata.hasAlpha);
       done();
     });
@@ -131,7 +131,7 @@ describe('Image metadata', function() {
       assert.strictEqual(2225, metadata.height);
       assert.strictEqual('srgb', metadata.space);
       assert.strictEqual(3, metadata.channels);
-      assert.strictEqual(true, metadata.hasProfile);
+      assert.strictEqual(false, metadata.hasProfile);
       assert.strictEqual(false, metadata.hasAlpha);
       done();
     }).catch(function(err) {
@@ -149,7 +149,7 @@ describe('Image metadata', function() {
       assert.strictEqual(2225, metadata.height);
       assert.strictEqual('srgb', metadata.space);
       assert.strictEqual(3, metadata.channels);
-      assert.strictEqual(true, metadata.hasProfile);
+      assert.strictEqual(false, metadata.hasProfile);
       assert.strictEqual(false, metadata.hasAlpha);
       done();
     });
@@ -165,7 +165,7 @@ describe('Image metadata', function() {
       assert.strictEqual(2225, metadata.height);
       assert.strictEqual('srgb', metadata.space);
       assert.strictEqual(3, metadata.channels);
-      assert.strictEqual(true, metadata.hasProfile);
+      assert.strictEqual(false, metadata.hasProfile);
       assert.strictEqual(false, metadata.hasAlpha);
       image.resize(metadata.width / 2).toBuffer(function(err, data, info) {
         if (err) throw err;