GNU bug report logs - #22591
25.0.50; [PATCH] imagemagick orientation EXIF tag is respected

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 22591 in the body.
You can then email your comments to 22591 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dima Kogan <dima <at> secretsauce.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50; [PATCH] imagemagick orientation EXIF tag is respected
Date: Sun, 07 Feb 2016 14:54:39 -0800

[Message part 1 (text/plain, inline)]

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!

[0001-imagemagick-images-accept-integer-rotation-specs.patch (text/x-diff, inline)]

From 8e7e3d54f6202be3dbff34fd822d259d394fa318 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima <at> 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

[0002-imagemagick-respects-the-orientation-EXIF-tag.patch (text/x-diff, inline)]

From b070fb5f8b4e9cb84e15a8a7970e4dcb25696d1c Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima <at> 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

Message #8 received at 22591 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dima Kogan <dima <at> secretsauce.net>
Cc: 22591 <at> debbugs.gnu.org
Subject: Re: bug#22591: 25.0.50;
 [PATCH] imagemagick orientation EXIF tag is respected
Date: Tue, 09 Feb 2016 14:02:49 +1100

Dima Kogan <dima <at> 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

Message #11 received at 22591 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dima Kogan <dima <at> secretsauce.net>
Cc: 22591 <at> debbugs.gnu.org
Subject: Re: bug#22591: 25.0.50;
 [PATCH] imagemagick orientation EXIF tag is respected
Date: Wed, 10 Feb 2016 15:40:25 +1100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Dima Kogan <dima <at> 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

Message #18 received at 22591 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 22591 <at> debbugs.gnu.org, Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#22591: 25.0.50;
 [PATCH] imagemagick orientation EXIF tag is respected
Date: Wed, 10 Feb 2016 11:35:03 -0500

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

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

Message #21 received at 22591 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 22591 <at> debbugs.gnu.org, Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#22591: 25.0.50;
 [PATCH] imagemagick orientation EXIF tag is respected
Date: Thu, 11 Feb 2016 13:28:00 +1100

Glenn Morris <rgm <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.