If you upload images from a cell phone or modern digital camera, the EXIF Orientation flag is used to rotate the image according to the orientation of the camera. Sadly, Firefox and Chrome don't honor the flag, and so images can appear to be rotated improperly on the site. There is a drupal module, autorotate that adds an autorotate feature to fix this, but it doesn't work properly with the imagemagick toolkit because it rotates the image but leaves the EXIF Orient flag unchanged.

In trying to fix the issue I discovered it would be easier for imagemagick to support it directly, since ImageMagick has an -auto-orient flag that takes care of rotation and EXIF Orient flags in one step. Attached is a patch that adds the functionality (and fixes some comment/help strings which are wrong in imagemagick_advanced.module).

I think this is a useful feature, and hope you will adopt it into the code base. Thanks.

Comments

sun’s picture

Version: 7.x-1.0-alpha2 » 7.x-1.x-dev
Status: Active » Needs work

Nice!

Can you provide a regular unified git diff, please?

mikel1’s picture

Sorry, I'm a git newbie. You mean just a "diff --git" of what against 7.x-1.0-alpha2 or a "git diff" against the head of the repository? Attaching both. Hope that's good.

sun’s picture

The git diff one is the right one. :) This applies to all patches on drupal.org. Btw, you might be interested in reading the Configuring Git handbook page (and possibly related pages). :)

That's not meant to confuse you though - the .git.diff patch from #2 is fine. :) (That said, we commonly use .patch as file extension ;))

This looks pretty good already and I think we're close. I only have some smaller nitpicks:

+++ b/imagemagick_advanced/imagemagick_advanced.module
@@ -144,10 +158,15 @@ function image_imagemagick_watermark(stdClass $image, $watermark) {
+    'help' => t('Strip metadata from an image.'),
...
+    'help' => t('Automatically rotates image according to orientation flag set by many phones and digital cameras.'),

s/image/images/

On the first, also:

s/Strip/Strips/ (third-person form)

+++ b/imagemagick_advanced/imagemagick_advanced.module
@@ -144,10 +158,15 @@ function image_imagemagick_watermark(stdClass $image, $watermark) {
+    'label' => t('Auto orient'),

Hm — "Auto orient" is not really a term or phrase.

While "Auto-orient" would probably be more correct, that would still be very technical, and I think that the labels are more focused on user interaction.

How about one of these?

1) Automatically correct orientation
2) Fix orientation
3) Adjust orientation

What do you think?

I think I'd opt for the last one, but you definitely have more experience with this effect :)

+++ b/imagemagick_advanced/imagemagick_advanced.module
@@ -175,3 +194,27 @@ function imagemagick_advanced_strip_effect($image) {
+ * Image effect callback; Auto orients an image resource.

s/Auto orients/Automatically adjusts the orientation of/

mikel1’s picture

Well, I don't have a lot of experience with it. I only discovered yesterday that - surprise! - browsers don't support the flag. There is a multi-year argument about whether to modify Firefox to support it. Ugh.

Anyway, your changes are good. I chose "Auto orient" because I thought it made clear to the user what it was - you can look it up in the ImageMagick documentation. However, you are right in that it should be geared more towards web site designers, not developers. Out of the suggestions you list, I think I'd prefer "Automatically correct orientation" as I think it's the most descriptive. "Autorotate" would be fine, too. The drupal autorotate module already uses that term, so I don't know if that's a plus or minus (what if the user enabled both modules?).

So how does this go from here. Do I need to take any further action? Am I supposed to post a "final" patch? Are you going to commit the change?

sun’s picture

Would be great if you could provide a final patch that incorporates the tweaks outlined in #3.

mikel1’s picture

StatusFileSize
new2.64 KB

OK, here it is

sun’s picture

Title: Support imagemagick auto-orient operation » Auto-orient effect support
Status: Needs work » Fixed

Thanks for reporting, reviewing, and testing! Committed to 7.x-1.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.