From a2cac61209774d93a8d02ae24a9738f8c556a02b Mon Sep 17 00:00:00 2001
From: Kleis Auke Wolthuizen <github@kleisauke.nl>
Date: Tue, 15 Aug 2023 08:56:07 +0200
Subject: [PATCH] Simplify 90/270 orient-before-resize logic (#3762)

---
 src/common.cc       | 10 +---------
 src/common.h        |  7 ++-----
 src/pipeline.cc     | 15 +++++++++------
 test/unit/rotate.js | 17 +++++++++++++++++
 4 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/common.cc b/src/common.cc
index 54354edb..b7b78ceb 100644
--- a/src/common.cc
+++ b/src/common.cc
@@ -964,15 +964,7 @@ namespace sharp {
   }
 
   std::pair<double, double> ResolveShrink(int width, int height, int targetWidth, int targetHeight,
-    Canvas canvas, bool swap, bool withoutEnlargement, bool withoutReduction) {
-    if (swap && canvas != Canvas::IGNORE_ASPECT) {
-      // Swap input width and height when requested.
-      std::swap(width, height);
-      if (canvas == Canvas::MAX) {
-        std::swap(targetWidth, targetHeight);
-      }
-    }
-
+    Canvas canvas, bool withoutEnlargement, bool withoutReduction) {
     double hshrink = 1.0;
     double vshrink = 1.0;
 
diff --git a/src/common.h b/src/common.h
index 9916f96a..df6bdcf6 100644
--- a/src/common.h
+++ b/src/common.h
@@ -362,13 +362,10 @@ namespace sharp {
   VImage EnsureAlpha(VImage image, double const value);
 
   /*
-    Calculate the shrink factor, taking into account auto-rotate, the canvas
-    mode, and so on. The hshrink/vshrink are the amount to shrink the input
-    image axes by in order for the output axes (ie. after rotation) to match
-    the required thumbnail width/height and canvas mode.
+    Calculate the horizontal and vertical shrink factors, taking the canvas mode into account.
   */
   std::pair<double, double> ResolveShrink(int width, int height, int targetWidth, int targetHeight,
-    Canvas canvas, bool swap, bool withoutEnlargement, bool withoutReduction);
+    Canvas canvas, bool withoutEnlargement, bool withoutReduction);
 
   /*
     Ensure decoding remains sequential.
diff --git a/src/pipeline.cc b/src/pipeline.cc
index 47f6bf9e..00879af1 100644
--- a/src/pipeline.cc
+++ b/src/pipeline.cc
@@ -157,15 +157,18 @@ class PipelineWorker : public Napi::AsyncWorker {
       int targetResizeWidth = baton->width;
       int targetResizeHeight = baton->height;
 
-      // Swap input output width and height when rotating by 90 or 270 degrees
-      bool swap = !baton->rotateBeforePreExtract &&
-        (rotation == VIPS_ANGLE_D90 || rotation == VIPS_ANGLE_D270 ||
-          autoRotation == VIPS_ANGLE_D90 || autoRotation == VIPS_ANGLE_D270);
+      // When auto-rotating by 90 or 270 degrees, swap the target width and
+      // height to ensure the behavior aligns with how it would have been if
+      // the rotation had taken place *before* resizing.
+      if (!baton->rotateBeforePreExtract &&
+        (autoRotation == VIPS_ANGLE_D90 || autoRotation == VIPS_ANGLE_D270)) {
+        std::swap(targetResizeWidth, targetResizeHeight);
+      }
 
       // Shrink to pageHeight, so we work for multi-page images
       std::tie(hshrink, vshrink) = sharp::ResolveShrink(
         inputWidth, pageHeight, targetResizeWidth, targetResizeHeight,
-        baton->canvas, swap, baton->withoutEnlargement, baton->withoutReduction);
+        baton->canvas, baton->withoutEnlargement, baton->withoutReduction);
 
       // The jpeg preload shrink.
       int jpegShrinkOnLoad = 1;
@@ -299,7 +302,7 @@ class PipelineWorker : public Napi::AsyncWorker {
       // Shrink to pageHeight, so we work for multi-page images
       std::tie(hshrink, vshrink) = sharp::ResolveShrink(
         inputWidth, pageHeight, targetResizeWidth, targetResizeHeight,
-        baton->canvas, swap, baton->withoutEnlargement, baton->withoutReduction);
+        baton->canvas, baton->withoutEnlargement, baton->withoutReduction);
 
       int targetHeight = static_cast<int>(std::rint(static_cast<double>(pageHeight) / vshrink));
       int targetPageHeight = targetHeight;
diff --git a/test/unit/rotate.js b/test/unit/rotate.js
index b5758082..aee20ead 100644
--- a/test/unit/rotate.js
+++ b/test/unit/rotate.js
@@ -192,6 +192,23 @@ describe('Rotation', function () {
       });
   });
 
+  it('Auto-rotate by 270 degrees, rectangular output ignoring aspect ratio', function (done) {
+    sharp(fixtures.inputJpgWithLandscapeExif8)
+      .resize(320, 240, { fit: sharp.fit.fill })
+      .rotate()
+      .toBuffer(function (err, data, info) {
+        if (err) throw err;
+        assert.strictEqual(320, info.width);
+        assert.strictEqual(240, info.height);
+        sharp(data).metadata(function (err, metadata) {
+          if (err) throw err;
+          assert.strictEqual(320, metadata.width);
+          assert.strictEqual(240, metadata.height);
+          done();
+        });
+      });
+  });
+
   it('Rotate by 30 degrees, rectangular output ignoring aspect ratio', function (done) {
     sharp(fixtures.inputJpg)
       .resize(320, 240, { fit: sharp.fit.fill })