Ensure manual flip, rotate, resize op order #3391

lovell · Oct 1, 2022 · eacb833 · eacb833
1 parent 99bf279
commit eacb833
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 28 deletions.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -4,6 +4,11 @@
 
 Requires libvips v8.13.2
 
+### v0.31.2 - TBD
+
+* Ensure manual flip, rotate, resize operation ordering (regression in 0.31.1)
+  [#3391](https://github.com/lovell/sharp/issues/3391)
+
 ### v0.31.1 - 29th September 2022
 
 * Upgrade to libvips v8.13.2 for upstream bug fixes.

diff --git a/src/pipeline.cc b/src/pipeline.cc
@@ -81,35 +81,47 @@ class PipelineWorker : public Napi::AsyncWorker {
       int pageHeight = sharp::GetPageHeight(image);
 
       // Calculate angle of rotation
-      VipsAngle rotation;
-      bool flip = FALSE;
-      bool flop = FALSE;
+      VipsAngle rotation = VIPS_ANGLE_D0;
+      VipsAngle autoRotation = VIPS_ANGLE_D0;
+      bool autoFlip = FALSE;
+      bool autoFlop = FALSE;
       if (baton->useExifOrientation) {
         // Rotate and flip image according to Exif orientation
-        std::tie(rotation, flip, flop) = CalculateExifRotationAndFlip(sharp::ExifOrientation(image));
+        std::tie(autoRotation, autoFlip, autoFlop) = CalculateExifRotationAndFlip(sharp::ExifOrientation(image));
+        image = sharp::RemoveExifOrientation(image);
       } else {
         rotation = CalculateAngleRotation(baton->angle);
       }
 
       // Rotate pre-extract
       bool const shouldRotateBefore = baton->rotateBeforePreExtract &&
-        (rotation != VIPS_ANGLE_D0 || flip || flop || baton->rotationAngle != 0.0);
+        (rotation != VIPS_ANGLE_D0 || autoRotation != VIPS_ANGLE_D0 ||
+          autoFlip || baton->flip || autoFlop || baton->flop ||
+          baton->rotationAngle != 0.0);
 
       if (shouldRotateBefore) {
-        if (rotation != VIPS_ANGLE_D0) {
-          image = image.rot(rotation);
+        if (autoRotation != VIPS_ANGLE_D0) {
+          image = image.rot(autoRotation);
+          autoRotation = VIPS_ANGLE_D0;
         }
-        if (flip) {
+        if (autoFlip) {
+          image = image.flip(VIPS_DIRECTION_VERTICAL);
+          autoFlip = FALSE;
+        } else if (baton->flip) {
           image = image.flip(VIPS_DIRECTION_VERTICAL);
+          baton->flip = FALSE;
         }
-        if (flop) {
+        if (autoFlop) {
           image = image.flip(VIPS_DIRECTION_HORIZONTAL);
+          autoFlop = FALSE;
+        } else if (baton->flop) {
+          image = image.flip(VIPS_DIRECTION_HORIZONTAL);
+          baton->flop = FALSE;
         }
-        if (rotation != VIPS_ANGLE_D0 || flip || flop) {
-          image = sharp::RemoveExifOrientation(image);
+        if (rotation != VIPS_ANGLE_D0) {
+          image = image.rot(rotation);
+          rotation = VIPS_ANGLE_D0;
         }
-        flop = FALSE;
-        flip = FALSE;
         if (baton->rotationAngle != 0.0) {
           MultiPageUnsupported(nPages, "Rotate");
           std::vector<double> background;
@@ -368,29 +380,16 @@ class PipelineWorker : public Napi::AsyncWorker {
       }
 
       // Flip (mirror about Y axis)
-      if (baton->flip || flip) {
+      if (baton->flip || autoFlip) {
         image = image.flip(VIPS_DIRECTION_VERTICAL);
-        image = sharp::RemoveExifOrientation(image);
       }
-
       // Flop (mirror about X axis)
-      if (baton->flop || flop) {
+      if (baton->flop || autoFlop) {
         image = image.flip(VIPS_DIRECTION_HORIZONTAL);
-        image = sharp::RemoveExifOrientation(image);
       }
-
       // Rotate post-extract 90-angle
       if (!baton->rotateBeforePreExtract && rotation != VIPS_ANGLE_D0) {
         image = image.rot(rotation);
-        if (flip) {
-          image = image.flip(VIPS_DIRECTION_VERTICAL);
-          flip = FALSE;
-        }
-        if (flop) {
-          image = image.flip(VIPS_DIRECTION_HORIZONTAL);
-          flop = FALSE;
-        }
-        image = sharp::RemoveExifOrientation(image);
       }
 
       // Join additional color channels to the image

diff --git a/test/unit/rotate.js b/test/unit/rotate.js
@@ -418,4 +418,29 @@ describe('Rotation', function () {
     assert.strictEqual(g, 73);
     assert.strictEqual(b, 52);
   });
+
+  it('Flip and rotate ordering', async () => {
+    const [r, g, b] = await sharp(fixtures.inputJpgWithPortraitExif5)
+      .flip()
+      .rotate(90)
+      .raw()
+      .toBuffer();
+
+    assert.strictEqual(r, 55);
+    assert.strictEqual(g, 65);
+    assert.strictEqual(b, 31);
+  });
+
+  it('Flip, rotate and resize ordering', async () => {
+    const [r, g, b] = await sharp(fixtures.inputJpgWithPortraitExif5)
+      .flip()
+      .rotate(90)
+      .resize(449)
+      .raw()
+      .toBuffer();
+
+    assert.strictEqual(r, 54);
+    assert.strictEqual(g, 64);
+    assert.strictEqual(b, 30);
+  });
 });