diff --git a/docs/changelog.md b/docs/changelog.md index 510c73fa..9ace370c 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -18,6 +18,10 @@ Requires libvips v8.3.1 [#443](https://github.com/lovell/sharp/pull/443) [@lemnisk8](https://github.com/lemnisk8) +* Ensure scaling factors are calculated independently to prevent rounding errors. + [#452](https://github.com/lovell/sharp/issues/452) + [@puzrin](https://github.com/puzrin) + * Add --sharp-cxx11 flag to compile with gcc's new C++11 ABI. [#456](https://github.com/lovell/sharp/pull/456) [@kapouer](https://github.com/kapouer) diff --git a/src/pipeline.cc b/src/pipeline.cc index d671bd62..f9b01456 100644 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -219,34 +219,46 @@ class PipelineWorker : public AsyncWorker { // Scaling calculations double xfactor = 1.0; double yfactor = 1.0; + int targetResizeWidth = baton->width; + int targetResizeHeight = baton->height; if (baton->width > 0 && baton->height > 0) { // Fixed width and height - xfactor = static_cast(inputWidth) / (static_cast(baton->width) + 0.1); - yfactor = static_cast(inputHeight) / (static_cast(baton->height) + 0.1); + xfactor = static_cast(inputWidth) / static_cast(baton->width); + yfactor = static_cast(inputHeight) / static_cast(baton->height); switch (baton->canvas) { case Canvas::CROP: - xfactor = std::min(xfactor, yfactor); - yfactor = xfactor; + if (xfactor < yfactor) { + targetResizeHeight = static_cast(round(static_cast(inputHeight) / xfactor)); + yfactor = xfactor; + } else { + targetResizeWidth = static_cast(round(static_cast(inputWidth) / yfactor)); + xfactor = yfactor; + } break; case Canvas::EMBED: - xfactor = std::max(xfactor, yfactor); - yfactor = xfactor; + if (xfactor > yfactor) { + targetResizeHeight = static_cast(round(static_cast(inputHeight) / xfactor)); + yfactor = xfactor; + } else { + targetResizeWidth = static_cast(round(static_cast(inputWidth) / yfactor)); + xfactor = yfactor; + } break; case Canvas::MAX: if (xfactor > yfactor) { - baton->height = static_cast(round(static_cast(inputHeight) / xfactor)); + targetResizeHeight = baton->height = static_cast(round(static_cast(inputHeight) / xfactor)); yfactor = xfactor; } else { - baton->width = static_cast(round(static_cast(inputWidth) / yfactor)); + targetResizeWidth = baton->width = static_cast(round(static_cast(inputWidth) / yfactor)); xfactor = yfactor; } break; case Canvas::MIN: if (xfactor < yfactor) { - baton->height = static_cast(round(static_cast(inputHeight) / xfactor)); + targetResizeHeight = baton->height = static_cast(round(static_cast(inputHeight) / xfactor)); yfactor = xfactor; } else { - baton->width = static_cast(round(static_cast(inputWidth) / yfactor)); + targetResizeWidth = baton->width = static_cast(round(static_cast(inputWidth) / yfactor)); xfactor = yfactor; } break; @@ -259,23 +271,23 @@ class PipelineWorker : public AsyncWorker { } } else if (baton->width > 0) { // Fixed width - xfactor = static_cast(inputWidth) / (static_cast(baton->width) + 0.1); + xfactor = static_cast(inputWidth) / static_cast(baton->width); if (baton->canvas == Canvas::IGNORE_ASPECT) { - baton->height = inputHeight; + targetResizeHeight = baton->height = inputHeight; } else { // Auto height yfactor = xfactor; - baton->height = static_cast(round(static_cast(inputHeight) / yfactor)); + targetResizeHeight = baton->height = static_cast(round(static_cast(inputHeight) / yfactor)); } } else if (baton->height > 0) { // Fixed height - yfactor = static_cast(inputHeight) / (static_cast(baton->height) + 0.1); + yfactor = static_cast(inputHeight) / static_cast(baton->height); if (baton->canvas == Canvas::IGNORE_ASPECT) { - baton->width = inputWidth; + targetResizeWidth = baton->width = inputWidth; } else { // Auto width xfactor = yfactor; - baton->width = static_cast(round(static_cast(inputWidth) / xfactor)); + targetResizeWidth = baton->width = static_cast(round(static_cast(inputWidth) / xfactor)); } } else { // Identity transform @@ -429,19 +441,13 @@ class PipelineWorker : public AsyncWorker { // Swap input output width and height when rotating by 90 or 270 degrees std::swap(shrunkWidth, shrunkHeight); } - xresidual = static_cast(baton->width) / static_cast(shrunkWidth); - yresidual = static_cast(baton->height) / static_cast(shrunkHeight); - if (baton->canvas == Canvas::EMBED) { - xresidual = std::min(xresidual, yresidual); - yresidual = xresidual; - } else if (baton->canvas == Canvas::IGNORE_ASPECT) { - if (!baton->rotateBeforePreExtract && - (rotation == VIPS_ANGLE_D90 || rotation == VIPS_ANGLE_D270)) { - std::swap(xresidual, yresidual); - } - } else { - xresidual = std::max(xresidual, yresidual); - yresidual = xresidual; + xresidual = static_cast(targetResizeWidth) / static_cast(shrunkWidth); + yresidual = static_cast(targetResizeHeight) / static_cast(shrunkHeight); + if ( + !baton->rotateBeforePreExtract && + (rotation == VIPS_ANGLE_D90 || rotation == VIPS_ANGLE_D270) + ) { + std::swap(xresidual, yresidual); } } @@ -673,10 +679,10 @@ class PipelineWorker : public AsyncWorker { // use gravity in ovelay if(overlayImageWidth <= baton->width) { - across = static_cast(ceil(static_cast(baton->width) / overlayImageWidth)); + across = static_cast(ceil(static_cast(image.width()) / overlayImageWidth)); } if(overlayImageHeight <= baton->height) { - down = static_cast(ceil(static_cast(baton->height) / overlayImageHeight)); + down = static_cast(ceil(static_cast(image.height()) / overlayImageHeight)); } if(across != 0 || down != 0) { int left; @@ -684,10 +690,10 @@ class PipelineWorker : public AsyncWorker { overlayImage = overlayImage.replicate(across, down); // the overlayGravity will now be used to CalculateCrop for extract_area std::tie(left, top) = CalculateCrop( - overlayImage.width(), overlayImage.height(), baton->width, baton->height, baton->overlayGravity + overlayImage.width(), overlayImage.height(), image.width(), image.height(), baton->overlayGravity ); overlayImage = overlayImage.extract_area( - left, top, baton->width, baton->height + left, top, image.width(), image.height() ); } // the overlayGravity was used for extract_area, therefore set it back to its default value of 0 diff --git a/test/fixtures/expected/crop-entropy.jpg b/test/fixtures/expected/crop-entropy.jpg index 40107d2d..af12e8a3 100644 Binary files a/test/fixtures/expected/crop-entropy.jpg and b/test/fixtures/expected/crop-entropy.jpg differ diff --git a/test/fixtures/expected/embed-16bit.png b/test/fixtures/expected/embed-16bit.png index e5ce8c4e..f5981791 100644 Binary files a/test/fixtures/expected/embed-16bit.png and b/test/fixtures/expected/embed-16bit.png differ