Skip to content

Commit 08b4242

Browse files
lovelllovell
committed
Ensure autoOrient occurs before non-90 rotation #4425
- Separate orient vs rotate ordering logic - Simplify EXIF auto-orient by using only rotate and/or flop
1 parent 67462be commit 08b4242

File tree

6 files changed

+72
-51
lines changed

6 files changed

+72
-51
lines changed

‎docs/src/content/docs/changelog/v0.34.4.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,8 @@ title: v0.34.4 - TBD
33
slug: changelog/v0.34.4
44
---
55

6+
* Ensure `autoOrient` occurs before non-90 angle rotation.
7+
[#4425](https://github.com/lovell/sharp/issues/4425)
8+
69
* Ensure `autoOrient` removes existing metadata after shrink-on-load.
710
[#4431](https://github.com/lovell/sharp/issues/4431)

‎lib/constructor.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ const Sharp = function (input, options) {
230230
angle: 0,
231231
rotationAngle: 0,
232232
rotationBackground: [0, 0, 0, 255],
233-
rotateBeforePreExtract: false,
233+
rotateBefore: false,
234+
orientBefore: false,
234235
flip: false,
235236
flop: false,
236237
extendTop: 0,

‎lib/resize.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ const mapFitToCanvas = {
107107
* @private
108108
*/
109109
function isRotationExpected (options) {
110-
return (options.angle % 360) !== 0 || options.input.autoOrient === true || options.rotationAngle !== 0;
110+
return (options.angle % 360) !== 0 || options.rotationAngle !== 0;
111111
}
112112

113113
/**
@@ -343,7 +343,7 @@ function resize (widthOrOptions, height, options) {
343343
}
344344
}
345345
if (isRotationExpected(this.options) && isResizeExpected(this.options)) {
346-
this.options.rotateBeforePreExtract = true;
346+
this.options.rotateBefore = true;
347347
}
348348
return this;
349349
}
@@ -490,9 +490,12 @@ function extract (options) {
490490
// Ensure existing rotation occurs before pre-resize extraction
491491
if (isRotationExpected(this.options) && !isResizeExpected(this.options)) {
492492
if (this.options.widthPre === -1 || this.options.widthPost === -1) {
493-
this.options.rotateBeforePreExtract = true;
493+
this.options.rotateBefore = true;
494494
}
495495
}
496+
if (this.options.input.autoOrient) {
497+
this.options.orientBefore = true;
498+
}
496499
return this;
497500
}
498501

@@ -566,7 +569,7 @@ function trim (options) {
566569
}
567570
}
568571
if (isRotationExpected(this.options)) {
569-
this.options.rotateBeforePreExtract = true;
572+
this.options.rotateBefore = true;
570573
}
571574
return this;
572575
}

‎src/pipeline.cc

Lines changed: 34 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -92,45 +92,43 @@ class PipelineWorker : public Napi::AsyncWorker {
9292
// Calculate angle of rotation
9393
VipsAngle rotation = VIPS_ANGLE_D0;
9494
VipsAngle autoRotation = VIPS_ANGLE_D0;
95-
bool autoFlip = false;
9695
bool autoFlop = false;
9796

9897
if (baton->input->autoOrient) {
9998
// Rotate and flip image according to Exif orientation
100-
std::tie(autoRotation, autoFlip, autoFlop) = CalculateExifRotationAndFlip(sharp::ExifOrientation(image));
99+
std::tie(autoRotation, autoFlop) = CalculateExifRotationAndFlop(sharp::ExifOrientation(image));
101100
}
102101

103102
rotation = CalculateAngleRotation(baton->angle);
104103

105-
// Rotate pre-extract
106-
bool const shouldRotateBefore = baton->rotateBeforePreExtract &&
107-
(rotation != VIPS_ANGLE_D0 || autoRotation != VIPS_ANGLE_D0 ||
108-
autoFlip || baton->flip || autoFlop || baton->flop ||
109-
baton->rotationAngle != 0.0);
110-
111-
if (shouldRotateBefore) {
112-
image = sharp::StaySequential(image,
113-
rotation != VIPS_ANGLE_D0 ||
114-
autoRotation != VIPS_ANGLE_D0 ||
115-
autoFlip ||
116-
baton->flip ||
117-
baton->rotationAngle != 0.0);
104+
bool const shouldRotateBefore = baton->rotateBefore &&
105+
(rotation != VIPS_ANGLE_D0 || baton->flip || baton->flop || baton->rotationAngle != 0.0);
106+
bool const shouldOrientBefore = (shouldRotateBefore || baton->orientBefore) &&
107+
(autoRotation != VIPS_ANGLE_D0 || autoFlop);
118108

109+
if (shouldOrientBefore) {
110+
image = sharp::StaySequential(image, autoRotation != VIPS_ANGLE_D0);
119111
if (autoRotation != VIPS_ANGLE_D0) {
120112
if (autoRotation != VIPS_ANGLE_D180) {
121113
MultiPageUnsupported(nPages, "Rotate");
122114
}
123115
image = image.rot(autoRotation);
124116
autoRotation = VIPS_ANGLE_D0;
125117
}
126-
if (autoFlip != baton->flip) {
118+
if (autoFlop) {
119+
image = image.flip(VIPS_DIRECTION_HORIZONTAL);
120+
autoFlop = false;
121+
}
122+
}
123+
124+
if (shouldRotateBefore) {
125+
image = sharp::StaySequential(image, rotation != VIPS_ANGLE_D0 || baton->flip || baton->rotationAngle != 0.0);
126+
if (baton->flip) {
127127
image = image.flip(VIPS_DIRECTION_VERTICAL);
128-
autoFlip = false;
129128
baton->flip = false;
130129
}
131-
if (autoFlop != baton->flop) {
130+
if (baton->flop) {
132131
image = image.flip(VIPS_DIRECTION_HORIZONTAL);
133-
autoFlop = false;
134132
baton->flop = false;
135133
}
136134
if (rotation != VIPS_ANGLE_D0) {
@@ -145,6 +143,7 @@ class PipelineWorker : public Napi::AsyncWorker {
145143
std::vector<double> background;
146144
std::tie(image, background) = sharp::ApplyAlpha(image, baton->rotationBackground, false);
147145
image = image.rotate(baton->rotationAngle, VImage::option()->set("background", background)).copy_memory();
146+
baton->rotationAngle = 0.0;
148147
}
149148
}
150149

@@ -183,8 +182,7 @@ class PipelineWorker : public Napi::AsyncWorker {
183182
// When auto-rotating by 90 or 270 degrees, swap the target width and
184183
// height to ensure the behavior aligns with how it would have been if
185184
// the rotation had taken place *before* resizing.
186-
if (!baton->rotateBeforePreExtract &&
187-
(autoRotation == VIPS_ANGLE_D90 || autoRotation == VIPS_ANGLE_D270)) {
185+
if (autoRotation == VIPS_ANGLE_D90 || autoRotation == VIPS_ANGLE_D270) {
188186
std::swap(targetResizeWidth, targetResizeHeight);
189187
}
190188

@@ -206,7 +204,7 @@ class PipelineWorker : public Napi::AsyncWorker {
206204
// - input colourspace is not specified;
207205
bool const shouldPreShrink = (targetResizeWidth > 0 || targetResizeHeight > 0) &&
208206
baton->gamma == 0 && baton->topOffsetPre == -1 && baton->trimThreshold < 0.0 &&
209-
baton->colourspacePipeline == VIPS_INTERPRETATION_LAST && !shouldRotateBefore;
207+
baton->colourspacePipeline == VIPS_INTERPRETATION_LAST && !(shouldOrientBefore || shouldRotateBefore);
210208

211209
if (shouldPreShrink) {
212210
// The common part of the shrink: the bit by which both axes must be shrunk
@@ -398,7 +396,6 @@ class PipelineWorker : public Napi::AsyncWorker {
398396
image = sharp::StaySequential(image,
399397
autoRotation != VIPS_ANGLE_D0 ||
400398
baton->flip ||
401-
autoFlip ||
402399
rotation != VIPS_ANGLE_D0);
403400
// Auto-rotate post-extract
404401
if (autoRotation != VIPS_ANGLE_D0) {
@@ -408,7 +405,7 @@ class PipelineWorker : public Napi::AsyncWorker {
408405
image = image.rot(autoRotation);
409406
}
410407
// Mirror vertically (up-down) about the x-axis
411-
if (baton->flip != autoFlip) {
408+
if (baton->flip) {
412409
image = image.flip(VIPS_DIRECTION_VERTICAL);
413410
}
414411
// Mirror horizontally (left-right) about the y-axis
@@ -515,7 +512,7 @@ class PipelineWorker : public Napi::AsyncWorker {
515512
}
516513

517514
// Rotate post-extract non-90 angle
518-
if (!baton->rotateBeforePreExtract && baton->rotationAngle != 0.0) {
515+
if (!baton->rotateBefore && baton->rotationAngle != 0.0) {
519516
MultiPageUnsupported(nPages, "Rotate");
520517
image = sharp::StaySequential(image);
521518
std::vector<double> background;
@@ -656,21 +653,16 @@ class PipelineWorker : public Napi::AsyncWorker {
656653
if (composite->input->autoOrient) {
657654
// Respect EXIF Orientation
658655
VipsAngle compositeAutoRotation = VIPS_ANGLE_D0;
659-
bool compositeAutoFlip = false;
660656
bool compositeAutoFlop = false;
661-
std::tie(compositeAutoRotation, compositeAutoFlip, compositeAutoFlop) =
662-
CalculateExifRotationAndFlip(sharp::ExifOrientation(compositeImage));
657+
std::tie(compositeAutoRotation, compositeAutoFlop) =
658+
CalculateExifRotationAndFlop(sharp::ExifOrientation(compositeImage));
663659

664660
compositeImage = sharp::RemoveExifOrientation(compositeImage);
665-
compositeImage = sharp::StaySequential(compositeImage,
666-
compositeAutoRotation != VIPS_ANGLE_D0 || compositeAutoFlip);
661+
compositeImage = sharp::StaySequential(compositeImage, compositeAutoRotation != VIPS_ANGLE_D0);
667662

668663
if (compositeAutoRotation != VIPS_ANGLE_D0) {
669664
compositeImage = compositeImage.rot(compositeAutoRotation);
670665
}
671-
if (compositeAutoFlip) {
672-
compositeImage = compositeImage.flip(VIPS_DIRECTION_VERTICAL);
673-
}
674666
if (compositeAutoFlop) {
675667
compositeImage = compositeImage.flip(VIPS_DIRECTION_HORIZONTAL);
676668
}
@@ -1402,21 +1394,20 @@ class PipelineWorker : public Napi::AsyncWorker {
14021394
Calculate the angle of rotation and need-to-flip for the given Exif orientation
14031395
By default, returns zero, i.e. no rotation.
14041396
*/
1405-
std::tuple<VipsAngle, bool, bool>
1406-
CalculateExifRotationAndFlip(int const exifOrientation) {
1397+
std::tuple<VipsAngle, bool>
1398+
CalculateExifRotationAndFlop(int const exifOrientation) {
14071399
VipsAngle rotate = VIPS_ANGLE_D0;
1408-
bool flip = false;
14091400
bool flop = false;
14101401
switch (exifOrientation) {
14111402
case 6: rotate = VIPS_ANGLE_D90; break;
14121403
case 3: rotate = VIPS_ANGLE_D180; break;
14131404
case 8: rotate = VIPS_ANGLE_D270; break;
1414-
case 2: flop = true; break; // flop 1
1415-
case 7: flip = true; rotate = VIPS_ANGLE_D90; break; // flip 6
1416-
case 4: flop = true; rotate = VIPS_ANGLE_D180; break; // flop 3
1417-
case 5: flip = true; rotate = VIPS_ANGLE_D270; break; // flip 8
1405+
case 2: flop = true; break;
1406+
case 7: flop = true; rotate = VIPS_ANGLE_D270; break;
1407+
case 4: flop = true; rotate = VIPS_ANGLE_D180; break;
1408+
case 5: flop = true; rotate = VIPS_ANGLE_D90; break;
14181409
}
1419-
return std::make_tuple(rotate, flip, flop);
1410+
return std::make_tuple(rotate, flop);
14201411
}
14211412

14221413
/*
@@ -1641,7 +1632,8 @@ Napi::Value pipeline(const Napi::CallbackInfo& info) {
16411632
baton->angle = sharp::AttrAsInt32(options, "angle");
16421633
baton->rotationAngle = sharp::AttrAsDouble(options, "rotationAngle");
16431634
baton->rotationBackground = sharp::AttrAsVectorOfDouble(options, "rotationBackground");
1644-
baton->rotateBeforePreExtract = sharp::AttrAsBool(options, "rotateBeforePreExtract");
1635+
baton->rotateBefore = sharp::AttrAsBool(options, "rotateBefore");
1636+
baton->orientBefore = sharp::AttrAsBool(options, "orientBefore");
16451637
baton->flip = sharp::AttrAsBool(options, "flip");
16461638
baton->flop = sharp::AttrAsBool(options, "flop");
16471639
baton->extendTop = sharp::AttrAsInt32(options, "extendTop");

‎src/pipeline.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ struct PipelineBaton {
115115
int angle;
116116
double rotationAngle;
117117
std::vector<double> rotationBackground;
118-
bool rotateBeforePreExtract;
118+
bool rotateBefore;
119+
bool orientBefore;
119120
bool flip;
120121
bool flop;
121122
int extendTop;

‎test/unit/rotate.js

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -565,9 +565,9 @@ describe('Rotation', function () {
565565
.raw()
566566
.toBuffer();
567567

568-
assert.strictEqual(r, 60);
569-
assert.strictEqual(g, 73);
570-
assert.strictEqual(b, 52);
568+
assert.strictEqual(r, 61);
569+
assert.strictEqual(g, 74);
570+
assert.strictEqual(b, 51);
571571
});
572572

573573
it('Flip and rotate ordering', async () => {
@@ -648,6 +648,27 @@ describe('Rotation', function () {
648648
assert.strictEqual(orientation, undefined);
649649
});
650650

651+
it('Auto-orient and rotate 45', async () => {
652+
const data = await sharp(fixtures.inputJpgWithLandscapeExif2, { autoOrient: true })
653+
.rotate(45)
654+
.toBuffer();
655+
656+
const { width, height } = await sharp(data).metadata();
657+
assert.strictEqual(width, 742);
658+
assert.strictEqual(height, 742);
659+
});
660+
661+
it('Auto-orient, extract and rotate 45', async () => {
662+
const data = await sharp(fixtures.inputJpgWithLandscapeExif2, { autoOrient: true })
663+
.extract({ left: 20, top: 20, width: 200, height: 100 })
664+
.rotate(45)
665+
.toBuffer();
666+
667+
const { width, height } = await sharp(data).metadata();
668+
assert.strictEqual(width, 212);
669+
assert.strictEqual(height, 212);
670+
});
671+
651672
it('Invalid autoOrient throws', () =>
652673
assert.throws(
653674
() => sharp({ autoOrient: 'fail' }),

0 commit comments

Comments
 (0)