diff --git a/docs/changelog.md b/docs/changelog.md index dea6e7129..6f7f4af53 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -4,6 +4,12 @@ Requires libvips v8.6.1. +#### v0.20.3 - TBD + +* Prevent possible rounding error when using shrink-on-load and 90/270 degree rotation. + [#1241](https://github.com/lovell/sharp/issues/1241) + [@anahit42](https://github.com/anahit42) + #### v0.20.3 - 29th May 2018 * Fix tint operation by ensuring LAB interpretation and allowing negative values. diff --git a/src/pipeline.cc b/src/pipeline.cc index b99de41f9..0516557a7 100644 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -281,15 +281,17 @@ class PipelineWorker : public Nan::AsyncWorker { } } // Recalculate integral shrink and double residual - int shrunkOnLoadWidth = image.width(); - int shrunkOnLoadHeight = image.height(); + int const shrunkOnLoadWidth = image.width(); + int const shrunkOnLoadHeight = 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(shrunkOnLoadWidth, shrunkOnLoadHeight); + // Swap when rotating by 90 or 270 degrees + xfactor = static_cast(shrunkOnLoadWidth) / static_cast(targetResizeHeight); + yfactor = static_cast(shrunkOnLoadHeight) / static_cast(targetResizeWidth); + } else { + xfactor = static_cast(shrunkOnLoadWidth) / static_cast(targetResizeWidth); + yfactor = static_cast(shrunkOnLoadHeight) / static_cast(targetResizeHeight); } - xfactor = static_cast(shrunkOnLoadWidth) / static_cast(targetResizeWidth); - yfactor = static_cast(shrunkOnLoadHeight) / static_cast(targetResizeHeight); } // Ensure we're using a device-independent colour space @@ -381,7 +383,6 @@ class PipelineWorker : public Nan::AsyncWorker { ) { throw vips::VError("Unknown kernel"); } - image = image.resize(1.0 / xfactor, VImage::option() ->set("vscale", 1.0 / yfactor) ->set("kernel", kernel)); diff --git a/test/unit/crop.js b/test/unit/crop.js index 5073a3c5b..cfa80afa4 100644 --- a/test/unit/crop.js +++ b/test/unit/crop.js @@ -177,25 +177,6 @@ describe('Crop', function () { }); }); - it('Clamp before crop when one post-resize dimension is below target', function () { - return sharp(fixtures.inputJpg) - .resize(1024, 1034) - .toBuffer() - .then(function (input) { - return sharp(input) - .rotate(270) - .resize(256) - .crop(sharp.strategy.entropy) - .toBuffer({ resolveWithObject: true }) - .then(function (result) { - assert.strictEqual(256, result.info.width); - assert.strictEqual(253, result.info.height); - assert.strictEqual(0, result.info.cropOffsetLeft); - assert.strictEqual(0, result.info.cropOffsetTop); - }); - }); - }); - describe('Entropy-based strategy', function () { it('JPEG', function (done) { sharp(fixtures.inputJpg) diff --git a/test/unit/resize.js b/test/unit/resize.js index 179ea4689..1bb2004b8 100644 --- a/test/unit/resize.js +++ b/test/unit/resize.js @@ -130,6 +130,25 @@ describe('Resize dimensions', function () { }); }); + it('JPEG shrink-on-load with 90 degree rotation, ensure recalculation is correct', function (done) { + sharp(fixtures.inputJpg) + .resize(1920, 1280) + .toBuffer(function (err, data, info) { + if (err) throw err; + assert.strictEqual(1920, info.width); + assert.strictEqual(1280, info.height); + sharp(data) + .rotate(90) + .resize(533, 800) + .toBuffer(function (err, data, info) { + if (err) throw err; + assert.strictEqual(533, info.width); + assert.strictEqual(800, info.height); + done(); + }); + }); + }); + it('TIFF embed known to cause rounding errors', function (done) { sharp(fixtures.inputTiff) .resize(240, 320)