Ensure premultiply op occurs before box shrink #605

docs/changelog.md

@@ -24,6 +24,11 @@ Requires libvips v8.4.2.
   [#601](https://github.com/lovell/sharp/issues/601)
   [@dynamite-ready](https://github.com/dynamite-ready)
 
+* Ensure premultiply operation occurs before box filter shrink.
+  [#605](https://github.com/lovell/sharp/issues/605)
+  [@CmdrShepardsPie](https://github.com/CmdrShepardsPie)
+  [@teroparvinen](https://github.com/teroparvinen)
+
 * Add support for PNG and WebP tile-based output formats (in addition to JPEG).
   [#622](https://github.com/lovell/sharp/pull/622)
   [@ppaskaris](https://github.com/ppaskaris)

src/operations.cc

@@ -300,6 +300,12 @@ namespace sharp {
     Calculate the intensity of edges, skin tone and saturation
   */
   double AttentionStrategy::operator()(VImage image) {
+    // Flatten RGBA onto a mid-grey background
+    if (image.bands() == 4 && HasAlpha(image)) {
+      double const midgrey = sharp::Is16Bit(image.interpretation()) ? 32768.0 : 128.0;
+      std::vector<double> background { midgrey, midgrey, midgrey };
+      image = image.flatten(VImage::option()->set("background", background));
+    }
     // Convert to LAB colourspace
     VImage lab = image.colourspace(VIPS_INTERPRETATION_LAB);
     VImage l = lab[0];

src/pipeline.cc

@@ -290,9 +290,6 @@ class PipelineWorker : public Nan::AsyncWorker {
         );
       }
 
-      // Calculate maximum alpha value based on input image pixel depth
-      double const maxAlpha = sharp::MaximumImageAlpha(image.interpretation());
-
       // Flatten image to remove alpha channel
       if (baton->flatten && HasAlpha(image)) {
         // Scale up 8-bit values to match 16-bit input image
@@ -305,7 +302,6 @@ class PipelineWorker : public Nan::AsyncWorker {
         };
         image = image.flatten(VImage::option()
           ->set("background", background)
-          ->set("max_alpha", maxAlpha)
         );
       }
 
@@ -324,7 +320,33 @@ class PipelineWorker : public Nan::AsyncWorker {
         image = image.colourspace(VIPS_INTERPRETATION_B_W);
       }
 
-      if (xshrink > 1 || yshrink > 1) {
+      // Ensure image has an alpha channel when there is an overlay
+      bool hasOverlay = baton->overlay != nullptr;
+      if (hasOverlay && !HasAlpha(image)) {
+        double const multiplier = sharp::Is16Bit(image.interpretation()) ? 256.0 : 1.0;
+        image = image.bandjoin(
+          VImage::new_matrix(image.width(), image.height()).new_from_image(255 * multiplier)
+        );
+      }
+
+      bool const shouldShrink = xshrink > 1 || yshrink > 1;
+      bool const shouldReduce = xresidual != 1.0 || yresidual != 1.0;
+      bool const shouldBlur = baton->blurSigma != 0.0;
+      bool const shouldConv = baton->convKernelWidth * baton->convKernelHeight > 0;
+      bool const shouldSharpen = baton->sharpenSigma != 0.0;
+      bool const shouldCutout = baton->overlayCutout;
+      bool const shouldPremultiplyAlpha = HasAlpha(image) &&
+        (shouldShrink || shouldReduce || shouldBlur || shouldConv || shouldSharpen || (hasOverlay && !shouldCutout));
+
+      // Premultiply image alpha channel before all transformations to avoid
+      // dark fringing around bright pixels
+      // See: http://entropymine.com/imageworsener/resizealpha/
+      if (shouldPremultiplyAlpha) {
+        image = image.premultiply();
+      }
+
+      // Fast, integral box-shrink
+      if (shouldShrink) {
         if (yshrink > 1) {
           image = image.shrinkv(yshrink);
         }
@@ -349,32 +371,8 @@ class PipelineWorker : public Nan::AsyncWorker {
         }
       }
 
-      // Ensure image has an alpha channel when there is an overlay
-      bool hasOverlay = baton->overlay != nullptr;
-      if (hasOverlay && !HasAlpha(image)) {
-        double const multiplier = sharp::Is16Bit(image.interpretation()) ? 256.0 : 1.0;
-        image = image.bandjoin(
-          VImage::new_matrix(image.width(), image.height()).new_from_image(255 * multiplier)
-        );
-      }
-
-      bool shouldAffineTransform = xresidual != 1.0 || yresidual != 1.0;
-      bool shouldBlur = baton->blurSigma != 0.0;
-      bool shouldConv = baton->convKernelWidth * baton->convKernelHeight > 0;
-      bool shouldSharpen = baton->sharpenSigma != 0.0;
-      bool shouldCutout = baton->overlayCutout;
-      bool shouldPremultiplyAlpha = HasAlpha(image) &&
-        (shouldAffineTransform || shouldBlur || shouldConv || shouldSharpen || (hasOverlay && !shouldCutout));
-
-      // Premultiply image alpha channel before all transformations to avoid
-      // dark fringing around bright pixels
-      // See: http://entropymine.com/imageworsener/resizealpha/
-      if (shouldPremultiplyAlpha) {
-        image = image.premultiply(VImage::option()->set("max_alpha", maxAlpha));
-      }
-
       // Use affine increase or kernel reduce with the remaining float part
-      if (shouldAffineTransform) {
+      if (xresidual != 1.0 || yresidual != 1.0) {
         // Insert tile cache to prevent over-computation of previous operations
         if (baton->accessMethod == VIPS_ACCESS_SEQUENTIAL) {
           image = sharp::TileCache(image, yresidual);
@@ -651,7 +649,7 @@ class PipelineWorker : public Nan::AsyncWorker {
 
       // Reverse premultiplication after all transformations:
       if (shouldPremultiplyAlpha) {
-        image = image.unpremultiply(VImage::option()->set("max_alpha", maxAlpha));
+        image = image.unpremultiply();
         // Cast pixel values to integer
         if (sharp::Is16Bit(image.interpretation())) {
           image = image.cast(VIPS_FORMAT_USHORT);

test/unit/extract.js

@@ -114,7 +114,7 @@ describe('Partial image extraction', function () {
         if (err) throw err;
         assert.strictEqual(280, info.width);
         assert.strictEqual(380, info.height);
-        fixtures.assertSimilar(fixtures.expected('rotate-extract.jpg'), data, done);
+        fixtures.assertSimilar(fixtures.expected('rotate-extract.jpg'), data, { threshold: 6 }, done);
       });
   });
 

test/unit/gamma.js

@@ -52,7 +52,7 @@ describe('Gamma correction', function () {
         if (err) throw err;
         assert.strictEqual('png', info.format);
         assert.strictEqual(320, info.width);
-        fixtures.assertSimilar(fixtures.expected('gamma-alpha.jpg'), data, { threshold: 11 }, done);
+        fixtures.assertSimilar(fixtures.expected('gamma-alpha.jpg'), data, { threshold: 19 }, done);
       });
   });