diff --git a/docs/changelog.md b/docs/changelog.md index b68325c8e..afbb4f8f7 100644 --- a/docs/changelog.md +++ b/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) diff --git a/src/operations.cc b/src/operations.cc index 20adfd40a..3984235a3 100644 --- a/src/operations.cc +++ b/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 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]; diff --git a/src/pipeline.cc b/src/pipeline.cc index 06093d856..617874065 100644 --- a/src/pipeline.cc +++ b/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); diff --git a/test/fixtures/expected/crop-strategy.png b/test/fixtures/expected/crop-strategy.png index 1c8a3780f..bec5122ed 100644 Binary files a/test/fixtures/expected/crop-strategy.png and b/test/fixtures/expected/crop-strategy.png differ diff --git a/test/fixtures/expected/embed-16bit-rgba.png b/test/fixtures/expected/embed-16bit-rgba.png index 06a53d2cb..eb31604d6 100644 Binary files a/test/fixtures/expected/embed-16bit-rgba.png and b/test/fixtures/expected/embed-16bit-rgba.png differ diff --git a/test/fixtures/expected/embed-2channel.png b/test/fixtures/expected/embed-2channel.png index ca6f220ae..a15d715d1 100644 Binary files a/test/fixtures/expected/embed-2channel.png and b/test/fixtures/expected/embed-2channel.png differ diff --git a/test/fixtures/expected/gamma-alpha.jpg b/test/fixtures/expected/gamma-alpha.jpg index aabde8a92..e7eea6274 100644 Binary files a/test/fixtures/expected/gamma-alpha.jpg and b/test/fixtures/expected/gamma-alpha.jpg differ diff --git a/test/fixtures/expected/rotate-extract.jpg b/test/fixtures/expected/rotate-extract.jpg index d27753cef..fcd3b95a6 100644 Binary files a/test/fixtures/expected/rotate-extract.jpg and b/test/fixtures/expected/rotate-extract.jpg differ diff --git a/test/fixtures/expected/sharpen-rgba.png b/test/fixtures/expected/sharpen-rgba.png index 841511cac..e4a29845a 100644 Binary files a/test/fixtures/expected/sharpen-rgba.png and b/test/fixtures/expected/sharpen-rgba.png differ diff --git a/test/unit/extract.js b/test/unit/extract.js index e339a9d21..22a70b746 100644 --- a/test/unit/extract.js +++ b/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); }); }); diff --git a/test/unit/gamma.js b/test/unit/gamma.js index d049bd926..c06ebff7f 100644 --- a/test/unit/gamma.js +++ b/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); }); });