Ensure conv kernel scale is clamped to min val of 1 #561

docs/changelog.md

@@ -4,6 +4,12 @@
 
 Requires libvips v8.3.3
 
+#### v0.16.1 - TBD
+
+* Ensure convolution kernel scale is clamped to a minimum value of 1.
+  [#561](https://github.com/lovell/sharp/issues/561)
+  [@abagshaw](https://github.com/abagshaw)
+
 #### v0.16.0 - 18<sup>th</sup> August 2016
 
 * Add pre-compiled libvips for OS X, ARMv7 and ARMv8.

index.js

@@ -503,22 +503,25 @@ Sharp.prototype.blur = function(sigma) {
   Convolve the image with a kernel.
 */
 Sharp.prototype.convolve = function(kernel) {
-  if (!isDefined(kernel) || !isDefined(kernel.kernel) ||
-      !isDefined(kernel.width) || !isDefined(kernel.height) ||
-      !inRange(kernel.width,3,1001) || !inRange(kernel.height,3,1001) ||
+  if (!isObject(kernel) || !Array.isArray(kernel.kernel) ||
+      !isInteger(kernel.width) || !isInteger(kernel.height) ||
+      !inRange(kernel.width, 3, 1001) || !inRange(kernel.height, 3, 1001) ||
       kernel.height * kernel.width != kernel.kernel.length
      ) {
     // must pass in a kernel
     throw new Error('Invalid convolution kernel');
   }
-  if(!isDefined(kernel.scale)) {
-    var sum = 0;
-    kernel.kernel.forEach(function(e) {
-      sum += e;
-    });
-    kernel.scale = sum;
+  // Default scale is sum of kernel values
+  if (!isInteger(kernel.scale)) {
+    kernel.scale = kernel.kernel.reduce(function(a, b) {
+      return a + b;
+    }, 0);
   }
-  if(!isDefined(kernel.offset)) {
+  // Clamp scale to a minimum value of 1
+  if (kernel.scale < 1) {
+    kernel.scale = 1;
+  }
+  if (!isInteger(kernel.offset)) {
     kernel.offset = 0;
   }
   this.options.convKernel = kernel;

test/unit/convolve.js

@@ -9,18 +9,17 @@ describe('Convolve', function() {
 
   it('specific convolution kernel 1', function(done) {
     sharp(fixtures.inputPngStripesV)
-      .resize(320, 240)
-      .convolve(
-        {
-          'width': 3,
-          'height': 3,
-          'scale': 50,
-          'offset': 0,
-          'kernel': [ 10, 20, 10,
+      .convolve({
+        width: 3,
+        height: 3,
+        scale: 50,
+        offset: 0,
+        kernel: [ 10, 20, 10,
                    0,  0,  0,
                   10, 20, 10 ]
       })
       .toBuffer(function(err, data, info) {
+        if (err) throw err;
         assert.strictEqual('png', info.format);
         assert.strictEqual(320, info.width);
         assert.strictEqual(240, info.height);
@@ -30,16 +29,15 @@ describe('Convolve', function() {
 
   it('specific convolution kernel 2', function(done) {
     sharp(fixtures.inputPngStripesH)
-      .resize(320, 240)
-      .convolve(
-        {
-          'width': 3,
-          'height': 3,
-          'kernel': [ 1, 0, 1,
+      .convolve({
+        width: 3,
+        height: 3,
+        kernel: [ 1, 0, 1,
                   2, 0, 2,
                   1, 0, 1 ]
       })
       .toBuffer(function(err, data, info) {
+        if (err) throw err;
         assert.strictEqual('png', info.format);
         assert.strictEqual(320, info.width);
         assert.strictEqual(240, info.height);
@@ -47,35 +45,47 @@ describe('Convolve', function() {
       });
   });
 
-  it('invalid kernel specification: no data', function() {
-    assert.throws(function() {
-      sharp(fixtures.inputJpg).convolve(
-        {
-          'width': 3,
-          'height': 3,
-          'kernel': []
-        });
+  it('horizontal Sobel operator', function(done) {
+    sharp(fixtures.inputJpg)
+      .resize(320, 240)
+      .convolve({
+        width: 3,
+        height: 3,
+        kernel: [ -1, 0, 1,
+                  -2, 0, 2,
+                  -1, 0, 1 ]
+      })
+      .toBuffer(function(err, data, info) {
+        if (err) throw err;
+        assert.strictEqual('jpeg', info.format);
+        assert.strictEqual(320, info.width);
+        assert.strictEqual(240, info.height);
+        fixtures.assertSimilar(fixtures.expected('conv-sobel-horizontal.jpg'), data, done);
       });
   });
 
-  it('invalid kernel specification: bad data format', function() {
+  describe('invalid kernel specification', function() {
+    it('missing', function() {
       assert.throws(function() {
-      sharp(fixtures.inputJpg).convolve(
-        {
-          'width': 3,
-          'height': 3,
-          'kernel': [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
+        sharp(fixtures.inputJpg).convolve({});
       });
     });
-  });
-  
-  it('invalid kernel specification: wrong width', function() {
+    it('incorrect data format', function() {
       assert.throws(function() {
-      sharp(fixtures.inputJpg).convolve(
-        {
-          'width': 3,
-          'height': 4,
-          'kernel': [1, 2, 3, 4, 5, 6, 7, 8, 9]
+        sharp(fixtures.inputJpg).convolve({
+          width: 3,
+          height: 3,
+          kernel: [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
+        });
+      });
+    });
+    it('incorrect dimensions', function() {
+      assert.throws(function() {
+        sharp(fixtures.inputJpg).convolve({
+          width: 3,
+          height: 4,
+          kernel: [1, 2, 3, 4, 5, 6, 7, 8, 9]
+        });
       });
     });
   });