Switch to libvips' resize, make fastShrinkOnLoad optional (#977)

lib/constructor.js

@@ -149,6 +149,7 @@ const Sharp = function (input, options) {
     kernel: 'lanczos3',
     interpolator: 'bicubic',
     centreSampling: false,
+    fastShrinkOnLoad: true,
     // operations
     background: [0, 0, 0, 255],
     flatten: false,

lib/resize.js

@@ -142,6 +142,11 @@ function resize (width, height, options) {
     if (is.defined(options.centreSampling)) {
       this._setBooleanOption('centreSampling', options.centreSampling);
     }
+
+    // Shrink on load
+    if (is.defined(options.fastShrinkOnLoad)) {
+      this._setBooleanOption('fastShrinkOnLoad', options.fastShrinkOnLoad);
+    }
   }
   return this;
 }

src/pipeline.cc

@@ -222,20 +222,27 @@ class PipelineWorker : public Nan::AsyncWorker {
       // If integral x and y shrink are equal, try to use shrink-on-load for JPEG and WebP,
       // but not when applying gamma correction, pre-resize extract or trim
       int shrink_on_load = 1;
+
+      int shrink_on_load_factor = 1;
+      // Leave at least a factor of two for the final resize step, when fastShrinkOnLoad: false
+      // for more consistent results and avoid occasional small image shifting
+      if (!baton->fastShrinkOnLoad) {
+        shrink_on_load_factor = 2;
+      }
       if (
-        xshrink == yshrink && xshrink >= 2 &&
+        xshrink == yshrink && xshrink >= 2 * shrink_on_load_factor &&
         (inputImageType == ImageType::JPEG || inputImageType == ImageType::WEBP) &&
         baton->gamma == 0 && baton->topOffsetPre == -1 && baton->trimTolerance == 0
       ) {
-        if (xshrink >= 8) {
+        if (xshrink >= 8 * shrink_on_load_factor) {
           xfactor = xfactor / 8;
           yfactor = yfactor / 8;
           shrink_on_load = 8;
-        } else if (xshrink >= 4) {
+        } else if (xshrink >= 4 * shrink_on_load_factor) {
           xfactor = xfactor / 4;
           yfactor = yfactor / 4;
           shrink_on_load = 4;
-        } else if (xshrink >= 2) {
+        } else if (xshrink >= 2 * shrink_on_load_factor) {
           xfactor = xfactor / 2;
           yfactor = yfactor / 2;
           shrink_on_load = 2;
@@ -282,23 +289,6 @@ class PipelineWorker : public Nan::AsyncWorker {
         }
         xfactor = static_cast<double>(shrunkOnLoadWidth) / static_cast<double>(targetResizeWidth);
         yfactor = static_cast<double>(shrunkOnLoadHeight) / static_cast<double>(targetResizeHeight);
-        xshrink = std::max(1, static_cast<int>(floor(xfactor)));
-        yshrink = std::max(1, static_cast<int>(floor(yfactor)));
-        xresidual = static_cast<double>(xshrink) / xfactor;
-        yresidual = static_cast<double>(yshrink) / yfactor;
-        if (
-          !baton->rotateBeforePreExtract &&
-          (rotation == VIPS_ANGLE_D90 || rotation == VIPS_ANGLE_D270)
-        ) {
-          std::swap(xresidual, yresidual);
-        }
-      }
-      // Help ensure a final kernel-based reduction to prevent shrink aliasing
-      if (xshrink > 1 && yshrink > 1 && (xresidual == 1.0 || yresidual == 1.0)) {
-        xshrink = xshrink / 2;
-        yshrink = yshrink / 2;
-        xresidual = static_cast<double>(xshrink) / xfactor;
-        yresidual = static_cast<double>(yshrink) / yfactor;
       }
 
       // Ensure we're using a device-independent colour space
@@ -364,13 +354,12 @@ class PipelineWorker : public Nan::AsyncWorker {
         }
       }
 
-      bool const shouldShrink = xshrink > 1 || yshrink > 1;
+      bool const shouldResize = xfactor != 1.0 || yfactor != 1.0;
-      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 shouldPremultiplyAlpha = HasAlpha(image) &&
-        (shouldShrink || shouldReduce || shouldBlur || shouldConv || shouldSharpen || shouldOverlayWithAlpha);
+        (shouldResize || shouldBlur || shouldConv || shouldSharpen || shouldOverlayWithAlpha);
 
       // Premultiply image alpha channel before all transformations to avoid
       // dark fringing around bright pixels
@@ -379,79 +368,21 @@ class PipelineWorker : public Nan::AsyncWorker {
         image = image.premultiply();
       }
 
-      // Fast, integral box-shrink
+      // Resize
-      if (shouldShrink) {
+      if (shouldResize) {
-        if (yshrink > 1) {
+        VipsKernel kernel = static_cast<VipsKernel>(
-          image = image.shrinkv(yshrink);
+          vips_enum_from_nick(nullptr, VIPS_TYPE_KERNEL, baton->kernel.data()));
-        }
-        if (xshrink > 1) {
-          image = image.shrinkh(xshrink);
-        }
-        // Recalculate residual float based on dimensions of required vs shrunk images
-        int shrunkWidth = image.width();
-        int shrunkHeight = image.height();
-        if (!baton->rotateBeforePreExtract &&
-          (rotation == VIPS_ANGLE_D90 || rotation == VIPS_ANGLE_D270)) {
-          // Swap input output width and height when rotating by 90 or 270 degrees
-          std::swap(shrunkWidth, shrunkHeight);
-        }
-        xresidual = static_cast<double>(targetResizeWidth) / static_cast<double>(shrunkWidth);
-        yresidual = static_cast<double>(targetResizeHeight) / static_cast<double>(shrunkHeight);
         if (
-          !baton->rotateBeforePreExtract &&
+          kernel != VIPS_KERNEL_NEAREST && kernel != VIPS_KERNEL_CUBIC && kernel != VIPS_KERNEL_LANCZOS2 &&
-          (rotation == VIPS_ANGLE_D90 || rotation == VIPS_ANGLE_D270)
+          kernel != VIPS_KERNEL_LANCZOS3
         ) {
-          std::swap(xresidual, yresidual);
+          throw vips::VError("Unknown kernel");
         }
-      }
 
-      // Use affine increase or kernel reduce with the remaining float part
+        image = image.resize(1.0 / xfactor, VImage::option()
-      if (xresidual != 1.0 || yresidual != 1.0) {
+          ->set("vscale", 1.0 / yfactor)
-        // Insert tile cache to prevent over-computation of previous operations
+          ->set("kernel", kernel)
-        if (baton->accessMethod == VIPS_ACCESS_SEQUENTIAL) {
+          ->set("centre", baton->centreSampling));
-          image = sharp::TileCache(image, yresidual);
-        }
-        // Perform kernel-based reduction
-        if (yresidual < 1.0 || xresidual < 1.0) {
-          VipsKernel kernel = static_cast<VipsKernel>(
-            vips_enum_from_nick(nullptr, VIPS_TYPE_KERNEL, baton->kernel.data()));
-          if (
-            kernel != VIPS_KERNEL_NEAREST && kernel != VIPS_KERNEL_CUBIC && kernel != VIPS_KERNEL_LANCZOS2 &&
-            kernel != VIPS_KERNEL_LANCZOS3
-          ) {
-            throw vips::VError("Unknown kernel");
-          }
-          if (yresidual < 1.0) {
-            image = image.reducev(1.0 / yresidual, VImage::option()
-              ->set("kernel", kernel)
-              ->set("centre", baton->centreSampling));
-          }
-          if (xresidual < 1.0) {
-            image = image.reduceh(1.0 / xresidual, VImage::option()
-              ->set("kernel", kernel)
-              ->set("centre", baton->centreSampling));
-          }
-        }
-        // Perform enlargement
-        if (yresidual > 1.0 || xresidual > 1.0) {
-          if (trunc(xresidual) == xresidual && trunc(yresidual) == yresidual && baton->interpolator == "nearest") {
-            // Fast, integral nearest neighbour enlargement
-            image = image.zoom(static_cast<int>(xresidual), static_cast<int>(yresidual));
-          } else {
-            // Floating point affine transformation
-            vips::VInterpolate interpolator = vips::VInterpolate::new_from_name(baton->interpolator.data());
-            if (yresidual > 1.0 && xresidual > 1.0) {
-              image = image.affine({xresidual, 0.0, 0.0, yresidual}, VImage::option()
-                ->set("interpolate", interpolator));
-            } else if (yresidual > 1.0) {
-              image = image.affine({1.0, 0.0, 0.0, yresidual}, VImage::option()
-                ->set("interpolate", interpolator));
-            } else if (xresidual > 1.0) {
-              image = image.affine({xresidual, 0.0, 0.0, 1.0}, VImage::option()
-                ->set("interpolate", interpolator));
-            }
-          }
-        }
       }
 
       // Rotate
@@ -1211,6 +1142,7 @@ NAN_METHOD(pipeline) {
   baton->kernel = AttrAsStr(options, "kernel");
   baton->interpolator = AttrAsStr(options, "interpolator");
   baton->centreSampling = AttrTo<bool>(options, "centreSampling");
+  baton->fastShrinkOnLoad = AttrTo<bool>(options, "fastShrinkOnLoad");
   // Join Channel Options
   if (HasAttr(options, "joinChannelIn")) {
     v8::Local<v8::Object> joinChannelObject = Nan::Get(options, Nan::New("joinChannelIn").ToLocalChecked())

src/pipeline.h

@@ -69,6 +69,7 @@ struct PipelineBaton {
   std::string kernel;
   std::string interpolator;
   bool centreSampling;
+  bool fastShrinkOnLoad;
   double background[4];
   bool flatten;
   bool negate;

test/fixtures/index.js

@@ -67,6 +67,7 @@ module.exports = {
   inputJpg320x240: getPath('320x240.jpg'), // http://www.andrewault.net/2010/01/26/create-a-test-pattern-video-with-perl/
   inputJpgOverlayLayer2: getPath('alpha-layer-2-ink.jpg'),
   inputJpgTruncated: getPath('truncated.jpg'), // head -c 10000 2569067123_aca715a2ee_o.jpg > truncated.jpg
+  inputJpgCenteredImage: getPath('centered_image.jpeg'),
 
   inputPng: getPath('50020484-00001.png'), // http://c.searspartsdirect.com/lis_png/PLDM/50020484-00001.png
   inputPngWithTransparency: getPath('blackbug.png'), // public domain

test/unit/extract.js

@@ -98,6 +98,7 @@ describe('Partial image extraction', function () {
     sharp(fixtures.inputPngWithGreyAlpha)
       .extract({ left: 20, top: 10, width: 380, height: 280 })
       .rotate(90)
+      .jpeg()
       .toBuffer(function (err, data, info) {
         if (err) throw err;
         assert.strictEqual(280, info.width);

test/unit/gamma.js

@@ -48,11 +48,12 @@ describe('Gamma correction', function () {
     sharp(fixtures.inputPngOverlayLayer1)
       .resize(320)
       .gamma()
+      .jpeg()
       .toBuffer(function (err, data, info) {
         if (err) throw err;
-        assert.strictEqual('png', info.format);
+        assert.strictEqual('jpeg', info.format);
         assert.strictEqual(320, info.width);
-        fixtures.assertSimilar(fixtures.expected('gamma-alpha.jpg'), data, { threshold: 20 }, done);
+        fixtures.assertSimilar(fixtures.expected('gamma-alpha.jpg'), data, done);
       });
   });
 

test/unit/resize.js

@@ -448,4 +448,34 @@ describe('Resize dimensions', function () {
           });
       });
   });
+
+  it('fastShrinkOnLoad: false ensures image is not shifted', function (done) {
+    return sharp(fixtures.inputJpgCenteredImage)
+      .resize(9, 8, {
+        fastShrinkOnLoad: false,
+        centreSampling: true
+      })
+      .png()
+      .toBuffer(function (err, data, info) {
+        if (err) throw err;
+        assert.strictEqual(9, info.width);
+        assert.strictEqual(8, info.height);
+        // higher threshold makes it pass for both jpeg and jpeg-turbo libs
+        fixtures.assertSimilar(fixtures.expected('fast-shrink-on-load-false.png'), data, { threshold: 7 }, done);
+      });
+  });
+
+  it('fastShrinkOnLoad: true (default) might result in shifted image', function (done) {
+    return sharp(fixtures.inputJpgCenteredImage)
+    .resize(9, 8, {
+      centreSampling: true
+    })
+    .png()
+    .toBuffer(function (err, data, info) {
+      if (err) throw err;
+      assert.strictEqual(9, info.width);
+      assert.strictEqual(8, info.height);
+      fixtures.assertSimilar(fixtures.expected('fast-shrink-on-load-true.png'), data, done);
+    });
+  });
 });