bug#22591: 25.0.50; [PATCH] imagemagick orientation EXIF tag is respected

bug#22591: 25.0.50; [PATCH] imagemagick orientation EXIF tag is respected

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

Hi. Here are two small patches to improve handling of images loaded with
the imagemagick backend. They are:

1. Accept integer :rotation parameters. Previously only floating-point
ones worked

2. If an EXIF orientation tag exists and no explicit :rotation exists,
use the orientation. This happens early in the processing, so all the
data (width, height, etc) that emacs sees applies to the rotated image.

The motivation for this was that I was tired of seeing upside-down
photos in my mu4e mail.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-imagemagick-images-accept-integer-rotation-specs.patch --]
[-- Type: text/x-diff, Size: 1004 bytes --]

From 8e7e3d54f6202be3dbff34fd822d259d394fa318 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Sun, 7 Feb 2016 10:37:31 -0800
Subject: [PATCH 1/2] imagemagick images accept integer :rotation specs

* src/image.c (imagemagick_load_image):
previously we only accepted floating-point specifications, even though
the most common rotations are 90, 180 and 270. Now integers work too.
---
 src/image.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/image.c b/src/image.c
index 144fe30..133d696 100644
--- a/src/image.c
+++ b/src/image.c
@@ -8646,7 +8646,7 @@ imagemagick_load_image (struct frame *f, struct image *img,
     image_spec_value (img->spec, QCbackground, NULL); if (!STRINGP
     (specified_bg).  */
   value = image_spec_value (img->spec, QCrotation, NULL);
-  if (FLOATP (value))
+  if (FLOATP (value) || INTEGERP (value))
     {
       rotation = extract_float (value);
       status = MagickRotateImage (image_wand, bg_wand, rotation);
-- 
2.1.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-imagemagick-respects-the-orientation-EXIF-tag.patch --]
[-- Type: text/x-diff, Size: 1259 bytes --]

From b070fb5f8b4e9cb84e15a8a7970e4dcb25696d1c Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Sun, 7 Feb 2016 14:43:24 -0800
Subject: [PATCH 2/2] imagemagick respects the orientation EXIF tag

* src/image.c (imagemagick_load_image):
images that have an orientation given in EXIF and have no explicit
:rotation tag are now pre-rotated.  All information such as
width/height is reported for the rotated image
---
 src/image.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/image.c b/src/image.c
index 133d696..45abfc7 100644
--- a/src/image.c
+++ b/src/image.c
@@ -8546,6 +8546,16 @@ imagemagick_load_image (struct frame *f, struct image *img,
       return 0;
     }
 
+  /* If no :rotation is explicitly specified, apply the automatic
+     rotation from EXIF. */
+  if (NILP (image_spec_value (img->spec, QCrotation, NULL)))
+    if (MagickAutoOrientImage (image_wand) == MagickFalse)
+      {
+        image_error ("Error applying automatic orientation in image `%s'", img->spec);
+        DestroyMagickWand (image_wand);
+        return 0;
+      }
+
   if (ino < 0 || ino >= MagickGetNumberImages (image_wand))
     {
       image_error ("Invalid image number `%s' in image `%s'", image, img->spec);
-- 
2.1.4

bug#22591: 25.0.50; [PATCH] imagemagick orientation EXIF tag is respected

[Lars Ingebrigtsen]

Dima Kogan <dima@secretsauce.net> writes:

> Hi. Here are two small patches to improve handling of images loaded with
> the imagemagick backend. They are:
>
> 1. Accept integer :rotation parameters. Previously only floating-point
> ones worked
>
> 2. If an EXIF orientation tag exists and no explicit :rotation exists,
> use the orientation. This happens early in the processing, so all the
> data (width, height, etc) that emacs sees applies to the rotated image.
>
> The motivation for this was that I was tired of seeing upside-down
> photos in my mu4e mail.

Looks good.  I wonder whether this should go in the trunk, though?  It's
a new feature of sorts.  But we could consider not showing images
correctly rotated a bug...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#22591: 25.0.50; [PATCH] imagemagick orientation EXIF tag is respected

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Dima Kogan <dima@secretsauce.net> writes:
>
>> Hi. Here are two small patches to improve handling of images loaded with
>> the imagemagick backend. They are:
>>
>> 1. Accept integer :rotation parameters. Previously only floating-point
>> ones worked
>>
>> 2. If an EXIF orientation tag exists and no explicit :rotation exists,
>> use the orientation. This happens early in the processing, so all the
>> data (width, height, etc) that emacs sees applies to the rotated image.
>>
>> The motivation for this was that I was tired of seeing upside-down
>> photos in my mu4e mail.
>
> Looks good.  I wonder whether this should go in the trunk, though?  It's
> a new feature of sorts.  But we could consider not showing images
> correctly rotated a bug...

I applied it to emacs-25.  Not respecting EXIF rotations is kinda a bug.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#22591: 25.0.50; [PATCH] imagemagick orientation EXIF tag is respected

[Glenn Morris]

This causes a build failure on RHEL 7.2, with ImageMagick-6.7.

  image.c:8552: undefined reference to `MagickAutoOrientImage'

bug#22591: 25.0.50; [PATCH] imagemagick orientation EXIF tag is respected

[Lars Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

> This causes a build failure on RHEL 7.2, with ImageMagick-6.7.
>
>   image.c:8552: undefined reference to `MagickAutoOrientImage'

Oops.  I'll revert the patch in emacs-25, reapply it to the trunk, and
then add autoconf tests for it there.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no