GNU bug report logs - #25967
25.1; Support for ImageMagick 7

Previous Next

Package: emacs;

Reported by: Tej Chajed <tchajed <at> mit.edu>

Date: Sat, 4 Mar 2017 16:21:02 UTC

Severity: wishlist

Tags: patch

Found in version 25.1

Fixed in version 27.1

Done: Glenn Morris <rgm <at> gnu.org>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Sat, 04 Mar 2017 16:21:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Tej Chajed <tchajed <at> mit.edu>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sat, 04 Mar 2017 16:21:02 GMT) Full text and rfc822 format available.

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

From: Tej Chajed <tchajed <at> mit.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; Support for ImageMagick 7
Date: Sat, 4 Mar 2017 11:17:41 -0500
[Message part 1 (text/plain, inline)]
Are there plans to upgrade from ImageMagick 6 to 7? I ask because I’m fixing imagemagick support for the Emacs formula in Homebrew (pull #10477) and Homebrew has to depend on the legacy ImageMagick 6 to build Emacs with ImageMagick support.

It looks like there was a brief discussion on emacs-devel about this in a thread about the 64-bit Windows build, where the belief was that ImageMagick 7 is too unstable to use. If this is indeed the case it makes sense to stick with ImageMagick 6 but I don’t think there was much of an explicit discussion about upgrading in that thread.
[Message part 2 (text/html, inline)]

Added indication that bug 25967 blocks24655 Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Sun, 05 Mar 2017 17:57:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Fri, 10 Mar 2017 19:04:02 GMT) Full text and rfc822 format available.

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Tej Chajed <tchajed <at> mit.edu>
Cc: 25967 <at> debbugs.gnu.org
Subject: 25.1; Support for ImageMagick 7
Date: Fri, 10 Mar 2017 11:03:46 -0800
I don't know of anyone currently working on porting Emacs to ImageMagick 
7. It'd be nice if someone would volunteer to do that.





Severity set to 'wishlist' from 'normal' Request was from Paul Eggert <eggert <at> cs.ucla.edu> to control <at> debbugs.gnu.org. (Fri, 10 Mar 2017 19:05:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Mon, 24 Jul 2017 16:43:01 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Tej Chajed <tchajed <at> mit.edu>, 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Mon, 24 Jul 2017 12:41:59 -0400
Paul Eggert wrote:

> I don't know of anyone currently working on porting Emacs to
> ImageMagick 7. It'd be nice if someone would volunteer to do that.

There seems to be a patch at
http://lists.gnu.org/archive/html/emacs-devel/2017-01/msg00180.html

Maybe it is MS-Windows-specific though, I haven't examined it.

In the meantime, configure should explicitly reject versions > 6, to
avoid confusion.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Mon, 24 Jul 2017 22:16:02 GMT) Full text and rfc822 format available.

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Tej Chajed <tchajed <at> mit.edu>, 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Mon, 24 Jul 2017 15:15:36 -0700
[Message part 1 (text/plain, inline)]
Glenn Morris wrote:
> http://lists.gnu.org/archive/html/emacs-devel/2017-01/msg00180.html
> 
> Maybe it is MS-Windows-specific though, I haven't examined it.

It looks quite MS-Windows-specific, unfortunately.

> In the meantime, configure should explicitly reject versions > 6, to
> avoid confusion.

That's easy enough; I installed the attached.
[0001-Do-not-use-ImageMagick-7-and-later.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 25 Jul 2017 01:26:01 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Tej Chajed <tchajed <at> mit.edu>, 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Mon, 24 Jul 2017 21:25:13 -0400
Paul Eggert wrote:

>> In the meantime, configure should explicitly reject versions > 6, to
>> avoid confusion.
>
> That's easy enough; I installed the attached.

I think it would be nicer if:

   checking for IMAGEMAGICK... no

became

   checking for IMAGEMAGICK (version 6)... no




Removed indication that bug 25967 blocks Request was from Eli Zaretskii <eliz <at> gnu.org> to control <at> debbugs.gnu.org. (Sat, 02 Sep 2017 13:07:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Sat, 02 Sep 2017 13:08:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: tchajed <at> mit.edu, 25967 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Sat, 02 Sep 2017 16:06:36 +0300
unblock 24655 by 25967
thanks

> From: Glenn Morris <rgm <at> gnu.org>
> Date: Mon, 24 Jul 2017 21:25:13 -0400
> Cc: Tej Chajed <tchajed <at> mit.edu>, 25967 <at> debbugs.gnu.org
> 
> Paul Eggert wrote:
> 
> >> In the meantime, configure should explicitly reject versions > 6, to
> >> avoid confusion.
> >
> > That's easy enough; I installed the attached.
> 
> I think it would be nicer if:
> 
>    checking for IMAGEMAGICK... no
> 
> became
> 
>    checking for IMAGEMAGICK (version 6)... no

I agree that would be nicer, but I don't think this minor issue, nor
the more general one of supporting ImageMagick 7, should block the
release of Emacs 26.1.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Sun, 03 Sep 2017 01:12:01 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: tchajed <at> mit.edu, 25967 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Sat, 02 Sep 2017 21:10:49 -0400
Eli Zaretskii wrote:

> I agree that would be nicer, but I don't think this minor issue, nor
> the more general one of supporting ImageMagick 7, should block the
> release of Emacs 26.1.

My motivation was that at some point ImageMagick 7 will become the
default version in GNU/Linux distributions. For example, possibly as
soon as Fedora 28, next May, ref

https://pagure.io/fesco/issue/1766

In terms of Emacs releases, this kind of change isn't far away, and
quite possibly before Emacs 27. At some point Emacs will need to support it.

Reference porting info: https://www.imagemagick.org/script/porting.php




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 12 Dec 2017 17:01:02 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: tchajed <at> mit.edu, 25967 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Tue, 12 Dec 2017 11:59:57 -0500
Glenn Morris wrote:

> My motivation was that at some point ImageMagick 7 will become the
> default version in GNU/Linux distributions. For example, possibly as
> soon as Fedora 28, next May, ref
>
> https://pagure.io/fesco/issue/1766
>
> In terms of Emacs releases, this kind of change isn't far away, and
> quite possibly before Emacs 27. At some point Emacs will need to support it.

Seems to have happened already in Arch Linux.

https://git.archlinux.org/svntogit/packages.git/log/trunk?h=packages/imagemagick




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 12 Dec 2017 17:17:02 GMT) Full text and rfc822 format available.

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

From: Marcin Borkowski <mbork <at> mbork.pl>
To: Glenn Morris <rgm <at> gnu.org>
Cc: tchajed <at> mit.edu, Eli Zaretskii <eliz <at> gnu.org>, eggert <at> cs.ucla.edu,
 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Tue, 12 Dec 2017 18:15:59 +0100
On 2017-12-12, at 17:59, Glenn Morris <rgm <at> gnu.org> wrote:

> Glenn Morris wrote:
>
>> My motivation was that at some point ImageMagick 7 will become the
>> default version in GNU/Linux distributions. For example, possibly as
>> soon as Fedora 28, next May, ref
>>
>> https://pagure.io/fesco/issue/1766
>>
>> In terms of Emacs releases, this kind of change isn't far away, and
>> quite possibly before Emacs 27. At some point Emacs will need to support it.
>
> Seems to have happened already in Arch Linux.
>
> https://git.archlinux.org/svntogit/packages.git/log/trunk?h=packages/imagemagick

Yes, exactly this bit me today.  Is there a chance that this could be
fixed soon?  Can I help that happen?

-- 
Marcin Borkowski




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 12 Dec 2017 17:45:01 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Marcin Borkowski <mbork <at> mbork.pl>
Cc: tchajed <at> mit.edu, Eli Zaretskii <eliz <at> gnu.org>, eggert <at> cs.ucla.edu,
 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Tue, 12 Dec 2017 12:44:20 -0500
Marcin Borkowski wrote:

> Yes, exactly this bit me today.

(If you just want things to work, probably you could install "imagemagick6".
https://www.archlinux.org/packages/extra/x86_64/imagemagick6/ )




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Fri, 01 Jun 2018 19:19:02 GMT) Full text and rfc822 format available.

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

From: Karl Otness <karl <at> karlotness.com>
To: 25967 <at> debbugs.gnu.org
Subject: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Fri, 1 Jun 2018 14:17:32 -0500
[Message part 1 (text/plain, inline)]
I have attached a patch which adds support for ImageMagick version 7.
I have tried to keep compatibility with version 6 as well. I have
gotten clean builds against both versions on my machine (running Arch
and overriding PKG_CONFIG_PATH to test). I also did a quick build on a
Mac and it also seemed to link up to the ImageMagick 7 from homebrew.
On both machines I could open/resize/rotate images and page through
the frames of animated GIFs.

The changes that seem to be needed are doable with a #define and a
typedef to rename a function and type in image.c. The patch makes
these conditional on the major version that is found by pkg-config
during ./configure, trying version 7 first and falling back to 6.

I'm not very familiar with autoconf, but the conditional checking in
the patch seems to work and avoids checking for ImageMagick 6 if it
finds version 7 first. I used AS_IF for this rather than the action
arguments to EMACS_CHECK_MODULES since this way seems to avoid the
redundant checks.

Thanks,
Karl
[0001-Add-support-for-ImageMagick-7-Bug-25967.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Sat, 23 Jun 2018 10:10:02 GMT) Full text and rfc822 format available.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: 25967 <at> debbugs.gnu.org
Cc: Karl Otness <karl <at> karlotness.com>
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Sat, 23 Jun 2018 12:09:51 +0200
FWIW, I tried this patch on Tumbleweed (OpenSUSE's rolling-release
distribution) which only has ImageMagick version 7[^1].  I also tried it
on Debian stable, which only has ImageMagick version 6[^1].

Like Karl, I successfully compiled emacs's master branch, opened,
resized and rotated a few PNGs and JPEGs.  Are there other features to
try out to assess the patch's soundness?


[^1]: AFAICT from some hasty zypper/apt searching.




Added tag(s) patch. Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Sat, 23 Jun 2018 11:27:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Mon, 25 Jun 2018 21:09:01 GMT) Full text and rfc822 format available.

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

From: Karl Otness <karl <at> karlotness.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Mon, 25 Jun 2018 16:08:08 -0500
On Sat, Jun 23, 2018 at 5:09 AM, Kévin Le Gouguec
<kevin.legouguec <at> gmail.com> wrote:
> FWIW, I tried this patch on Tumbleweed (OpenSUSE's rolling-release
> distribution) which only has ImageMagick version 7[^1].  I also tried it
> on Debian stable, which only has ImageMagick version 6[^1].
>
> Like Karl, I successfully compiled emacs's master branch, opened,
> resized and rotated a few PNGs and JPEGs.  Are there other features to
> try out to assess the patch's soundness?

Thanks for trying out the patch. I really appreciate you testing it
out on some other systems. I've been using it in my Emacs for a while
and haven't really seen any issues. I'd also be happy to test other
steps out if there's something more specific I should check.

Thanks,
Karl




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 28 Aug 2018 02:11:01 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Karl Otness <karl <at> karlotness.com>
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Mon, 27 Aug 2018 22:10:27 -0400
Karl Otness wrote:

> I have attached a patch which adds support for ImageMagick version 7.

Thanks very much! It looks much simpler than I was expecting.

I applied it as 5729486, then tweaked it in bf1b147 (mainly to avoid
printing two identical "checking for imagemagick"); it would be good if you
check I did not break it. :)

We normally like to get copyright assignments for Emacs; see eg
https://www.gnu.org/licenses/why-assign.en.html

I think your patch is just small enought not to require this.
But if you would like to contribute more in future, or just would like
to, you can start the assignment process by filing out the form at

http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future

Thanks again!




Reply sent to Glenn Morris <rgm <at> gnu.org>:
You have taken responsibility. (Tue, 28 Aug 2018 02:13:02 GMT) Full text and rfc822 format available.

Notification sent to Tej Chajed <tchajed <at> mit.edu>:
bug acknowledged by developer. (Tue, 28 Aug 2018 02:13:02 GMT) Full text and rfc822 format available.

Message #57 received at 25967-done <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 25967-done <at> debbugs.gnu.org
Subject: Re: bug#25967: 25.1; Support for ImageMagick 7
Date: Mon, 27 Aug 2018 22:12:15 -0400
Version: 27.1

Added in 5729486.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 28 Aug 2018 20:33:01 GMT) Full text and rfc822 format available.

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

From: Karl Otness <karl <at> karlotness.com>
To: rgm <at> gnu.org
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Tue, 28 Aug 2018 15:32:26 -0500
I hate to say this, but it looks like the modified patch might not be
linking up quite right. It seems like the issue has to do with the new
"IMAGEMAGICK7" prefix for EMACS_CHECK_MODULES. Some of the downstream
tests, specifically looking for MagickRelinquishMemory try to use the
IMAGEMAGICK_CFLAGS which won't be defined if configure finds version
7. The test will then fail ImageMagick and it won't be linked.

I don't remember exactly, but I think this may have been the reason
why I had initially set it up to use the same HAVE_IMAGEMAGICK prefix
for both pkg-config checks, and then manually added the
IMAGEMAGICK_MAJOR definition to sort out which was actually found.

Using separate test variables can probably work, but will require
updating the AC_CHECK_FUNCS tests later on. When I put together the
initial patch, I think I wanted to touch as little of configure as
possible, so I left those alone.

It may be simpler to go back to using the same IMAGEMAGICK prefix for
both package checks, but I agree it is nicer to not re-use the same
pkg-config variables.

Thanks for helping get this checked in.

Karl
On Mon, Aug 27, 2018 at 9:10 PM Glenn Morris <rgm <at> gnu.org> wrote:
>
> Karl Otness wrote:
>
> > I have attached a patch which adds support for ImageMagick version 7.
>
> Thanks very much! It looks much simpler than I was expecting.
>
> I applied it as 5729486, then tweaked it in bf1b147 (mainly to avoid
> printing two identical "checking for imagemagick"); it would be good if you
> check I did not break it. :)
>
> We normally like to get copyright assignments for Emacs; see eg
> https://www.gnu.org/licenses/why-assign.en.html
>
> I think your patch is just small enought not to require this.
> But if you would like to contribute more in future, or just would like
> to, you can start the assignment process by filing out the form at
>
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future
>
> Thanks again!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Wed, 29 Aug 2018 03:22:02 GMT) Full text and rfc822 format available.

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

From: Karl Otness <karl <at> karlotness.com>
To: rgm <at> gnu.org
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Tue, 28 Aug 2018 22:21:07 -0500
On Mon, Aug 27, 2018 at 9:10 PM Glenn Morris <rgm <at> gnu.org> wrote:
> I applied it as 5729486, then tweaked it in bf1b147 (mainly to avoid
> printing two identical "checking for imagemagick")

I did a quick run of configure and I'm not sure if this is a
difference in version of autoconf (mine reports version 2.69), but on
my machine it seems to use the module definition when printing. I see:
"checking for MagickWand >= 7... yes"

I wonder if that might help the situation possibly using the same
prefix for both pkg-config checks?

Thanks,
Karl




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Thu, 30 Aug 2018 18:01:01 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Karl Otness <karl <at> karlotness.com>
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Thu, 30 Aug 2018 14:00:04 -0400
Sorry, my change was garbage. I hope I fixed it in 3cc42bb...

> I did a quick run of configure and I'm not sure if this is a
> difference in version of autoconf (mine reports version 2.69), but on
> my machine it seems to use the module definition when printing. I see:
> "checking for MagickWand >= 7... yes"

Looks like this changed in 37b2f9f when m4/pkg.m4 was synced with
pkg-config 0.29.2 earlier this year. PKG_CHECK_MODULE used to print
$1, now it prints $2. Progress!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Thu, 30 Aug 2018 21:54:02 GMT) Full text and rfc822 format available.

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

From: Karl Otness <karl <at> karlotness.com>
To: rgm <at> gnu.org
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Thu, 30 Aug 2018 16:53:35 -0500
On Thu, Aug 30, 2018 at 1:00 PM Glenn Morris <rgm <at> gnu.org> wrote:
> Looks like this changed in 37b2f9f when m4/pkg.m4 was synced with
> pkg-config 0.29.2 earlier this year. PKG_CHECK_MODULE used to print
> $1, now it prints $2. Progress!

That's good to know, nice that it should be consistent and give a good
description of the check.

> Sorry, my change was garbage. I hope I fixed it in 3cc42bb...

No problem, it is tricky to try the configure step with different
versions. I just did a new build off of master and I ended up with an
Emacs linked to libmagick version 7 and it opened images just fine.
Seems like everything is working.

Thanks again for your help getting this in.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Mon, 03 Sep 2018 22:58:02 GMT) Full text and rfc822 format available.

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

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Mon, 03 Sep 2018 23:57:05 +0100
[Message part 1 (text/plain, inline)]
On Fri 01 Jun 2018, Karl Otness wrote:

> I have attached a patch which adds support for ImageMagick version 7.
> I have tried to keep compatibility with version 6 as well. I have
> gotten clean builds against both versions on my machine (running Arch
> and overriding PKG_CONFIG_PATH to test). I also did a quick build on a
> Mac and it also seemed to link up to the ImageMagick 7 from homebrew.
> On both machines I could open/resize/rotate images and page through
> the frames of animated GIFs.

As a followup, here is a patch to add ImageMagick support for Windows.
The patch was initially developed for Imagemagick 6, but I've updated it
to support ImageMagick 7.

I've given this some light testing on a 64bit mingw64 (MSYS2) and 64bit
cygwin builds, both of which use Imagemagick 7.

Please test, and report if it breaks anything on other platforms.

    AndyM


[imagemagick7.patch (text/x-patch, inline)]
diff --git a/configure.ac b/configure.ac
index 6f3d7338c3..25e42fdbaf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2533,6 +2533,10 @@ AC_DEFUN
 		      MagickMergeImageLayers MagickAutoOrientImage])
       CFLAGS=$OLD_CFLAGS
       LIBS=$OLD_LIBS
+      # Windows loads libmagickwand dynamically
+      if test "${opsys}" = "mingw32"; then
+        IMAGEMAGICK_LIBS=
+      fi
       # Check that ImageMagick links.  It does not link on Fedora 25
       # with './configure CC=clang', as pkg-config outputs flags like
       # -lomp that work for GCC but not Clang.
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index dc57160d04..e07a011de5 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -277,7 +277,8 @@ libgnutls-version
        '(libxml2 "libxml2-2.dll" "libxml2.dll")
        '(zlib "zlib1.dll" "libz-1.dll")
        '(lcms2 "liblcms2-2.dll")
-       '(json "libjansson-4.dll")))
+       '(json "libjansson-4.dll")
+       '(imagemagick "libMagickWand-7.Q16HDRI-6.dll")))
 
 ;;; multi-tty support
 (defvar w32-initialized nil
diff --git a/src/image.c b/src/image.c
index 24decbc099..3faa3c6b3c 100644
--- a/src/image.c
+++ b/src/image.c
@@ -8277,8 +8277,8 @@ imagemagick_image_p (Lisp_Object object)
 #ifdef HAVE_IMAGEMAGICK7
 # include <MagickWand/MagickWand.h>
 # include <MagickCore/version.h>
-/* ImageMagick 7 compatibility definitions.  */
-# define PixelSetMagickColor PixelSetPixelColor
+/* ImageMagick 7 compatibility definitions.  API functions are
+   redefined later, after the w32 runtime linking support code.  */
 typedef PixelInfo MagickPixelPacket;
 #else
 # include <wand/MagickWand.h>
@@ -8292,6 +8292,255 @@ extern WandExport void PixelGetMagickColor (const PixelWand *,
 					    MagickPixelPacket *);
 #endif
 
+#if defined HAVE_NTGUI && defined WINDOWSNT
+
+/* Imagemagick MagickWand library functions.  */
+
+DEF_DLL_FN (MagickWand *, CloneMagickWand, (const MagickWand *));
+DEF_DLL_FN (MagickWand *, DestroyMagickWand, (MagickWand *));
+DEF_DLL_FN (PixelIterator *, DestroyPixelIterator, (PixelIterator *));
+DEF_DLL_FN (PixelWand *, DestroyPixelWand, (PixelWand *));
+DEF_DLL_FN (MagickBooleanType, MagickCropImage,
+            (MagickWand *, const size_t, const size_t, const ssize_t,
+             const ssize_t));
+DEF_DLL_FN (MagickBooleanType, MagickExportImagePixels,
+            (MagickWand *, const ssize_t, const ssize_t, const size_t,
+             const size_t, const char *, const StorageType, void *));
+#ifndef HAVE_MAGICKMERGEIMAGELAYERS
+DEF_DLL_FN(MagickWand *, MagickFlattenImages, (MagickWand *));
+#endif
+DEF_DLL_FN (char *, MagickGetException, (const MagickWand *, ExceptionType *));
+DEF_DLL_FN (MagickWand *, MagickGetImage, (MagickWand *));
+DEF_DLL_FN (size_t, MagickGetImageDelay, (MagickWand *));
+DEF_DLL_FN (DisposeType, MagickGetImageDispose, (MagickWand *));
+DEF_DLL_FN (size_t, MagickGetImageHeight, (MagickWand *));
+DEF_DLL_FN (MagickBooleanType, MagickGetImagePage,
+             (MagickWand *, size_t *, size_t *, ssize_t *, ssize_t *));
+DEF_DLL_FN (char *, MagickGetImageSignature, (MagickWand *));
+DEF_DLL_FN (size_t, MagickGetImageWidth, (MagickWand *));
+DEF_DLL_FN (size_t, MagickGetNumberImages, (MagickWand *));
+#ifdef HAVE_MAGICKMERGEIMAGELAYERS
+DEF_DLL_FN (MagickWand *, MagickMergeImageLayers,
+            (MagickWand *, const LayerMethod));
+#endif
+DEF_DLL_FN (char **, MagickQueryFormats, (const char *, size_t *));
+DEF_DLL_FN (MagickBooleanType, MagickReadImage, (MagickWand *, const char *));
+DEF_DLL_FN (MagickBooleanType, MagickReadImageBlob,
+            (MagickWand *, const void *, const size_t));
+DEF_DLL_FN (void *, MagickRelinquishMemory, (void *));
+DEF_DLL_FN (MagickBooleanType, MagickRotateImage,
+            (MagickWand *, const PixelWand *, const double));
+DEF_DLL_FN (MagickBooleanType, MagickScaleImage,
+            (MagickWand *, const size_t, const size_t));
+DEF_DLL_FN (MagickBooleanType, MagickSetFilename, (MagickWand *, const char *));
+DEF_DLL_FN (MagickBooleanType, MagickSetImageBackgroundColor,
+            (MagickWand *,const PixelWand *));
+DEF_DLL_FN (MagickBooleanType, MagickSetIteratorIndex,
+            (MagickWand *, const ssize_t));
+DEF_DLL_FN (void, MagickWandGenesis, (void));
+DEF_DLL_FN (void, MagickWandTerminus, (void));
+DEF_DLL_FN (MagickWand *, NewMagickWand, (void));
+DEF_DLL_FN (PixelIterator *, NewPixelIterator, (MagickWand *));
+DEF_DLL_FN (PixelWand *, NewPixelWand, (void));
+DEF_DLL_FN (double, PixelGetAlpha, (const PixelWand *));
+DEF_DLL_FN (void, PixelGetMagickColor, (const PixelWand *, MagickPixelPacket *));
+DEF_DLL_FN (PixelWand **, PixelGetNextIteratorRow, (PixelIterator *, size_t *));
+DEF_DLL_FN (void, PixelSetBlue, (PixelWand *, const double));
+DEF_DLL_FN (void, PixelSetGreen, (PixelWand *, const double));
+DEF_DLL_FN (MagickBooleanType, PixelSetIteratorRow,
+            (PixelIterator *, const ssize_t));
+#ifdef HAVE_IMAGEMAGICK7
+DEF_DLL_FN (void, PixelSetPixelColor, (PixelWand *, const MagickPixelPacket *));
+#else
+DEF_DLL_FN (void, PixelSetMagickColor, (PixelWand *, const MagickPixelPacket *));
+#endif
+DEF_DLL_FN (void, PixelSetRed, (PixelWand *, const double));
+DEF_DLL_FN (MagickBooleanType, PixelSyncIterator, (PixelIterator *));
+
+DEF_DLL_FN (MagickBooleanType, MagickSetSize,
+            (MagickWand *, const size_t, const size_t));
+DEF_DLL_FN (MagickBooleanType, MagickSetDepth,
+            (MagickWand *, const size_t));
+#ifdef HAVE_MAGICKAUTOORIENTIMAGE
+DEF_DLL_FN (MagickBooleanType, MagickAutoOrientImage, (MagickWand *));
+#endif
+
+static bool
+init_imagemagick_functions (void)
+{
+  HMODULE library = NULL;
+
+  if (!(library = w32_delayed_load (Qimagemagick)))
+    {
+      return 0;
+    }
+
+  LOAD_DLL_FN (library, CloneMagickWand);
+  LOAD_DLL_FN (library, DestroyMagickWand);
+  LOAD_DLL_FN (library, DestroyPixelIterator);
+  LOAD_DLL_FN (library, DestroyPixelWand);
+  LOAD_DLL_FN (library, MagickCropImage);
+  LOAD_DLL_FN (library, MagickExportImagePixels);
+#ifndef HAVE_MAGICKMERGEIMAGELAYERS
+  LOAD_DLL_FN (library, MagickFlattenImages);
+#endif
+  LOAD_DLL_FN (library, MagickGetException);
+  LOAD_DLL_FN (library, MagickGetImage);
+  LOAD_DLL_FN (library, MagickGetImageDelay);
+  LOAD_DLL_FN (library, MagickGetImageDispose);
+  LOAD_DLL_FN (library, MagickGetImageHeight);
+  LOAD_DLL_FN (library, MagickGetImagePage);
+  LOAD_DLL_FN (library, MagickGetImageSignature);
+  LOAD_DLL_FN (library, MagickGetImageWidth);
+  LOAD_DLL_FN (library, MagickGetNumberImages);
+#ifdef HAVE_MAGICKMERGEIMAGELAYERS
+  LOAD_DLL_FN (library, MagickMergeImageLayers);
+#endif
+  LOAD_DLL_FN (library, MagickQueryFormats);
+  LOAD_DLL_FN (library, MagickReadImage);
+  LOAD_DLL_FN (library, MagickReadImageBlob);
+  LOAD_DLL_FN (library, MagickRelinquishMemory);
+  LOAD_DLL_FN (library, MagickRotateImage);
+  LOAD_DLL_FN (library, MagickScaleImage);
+  LOAD_DLL_FN (library, MagickSetFilename);
+  LOAD_DLL_FN (library, MagickSetImageBackgroundColor);
+  LOAD_DLL_FN (library, MagickSetIteratorIndex);
+  LOAD_DLL_FN (library, MagickWandGenesis);
+  LOAD_DLL_FN (library, MagickWandTerminus);
+  LOAD_DLL_FN (library, NewMagickWand);
+  LOAD_DLL_FN (library, NewPixelIterator);
+  LOAD_DLL_FN (library, NewPixelWand);
+  LOAD_DLL_FN (library, PixelGetAlpha);
+  LOAD_DLL_FN (library, PixelGetMagickColor);
+  LOAD_DLL_FN (library, PixelGetNextIteratorRow);
+  LOAD_DLL_FN (library, PixelSetBlue);
+  LOAD_DLL_FN (library, PixelSetGreen);
+  LOAD_DLL_FN (library, PixelSetIteratorRow);
+#ifdef HAVE_IMAGEMAGICK7
+  LOAD_DLL_FN (library, PixelSetPixelColor);
+#else
+  LOAD_DLL_FN (library, PixelSetMagickColor);
+#endif
+  LOAD_DLL_FN (library, PixelSetRed);
+  LOAD_DLL_FN (library, PixelSyncIterator);
+
+  LOAD_DLL_FN (library, MagickSetSize);
+  LOAD_DLL_FN (library, MagickSetDepth);
+#ifdef HAVE_MAGICKAUTOORIENTIMAGE
+  LOAD_DLL_FN (library, MagickAutoOrientImage);
+#endif
+
+  return 1;
+}
+
+#undef CloneMagickWand
+#undef DestroyMagickWand
+#undef DestroyPixelIterator
+#undef DestroyPixelWand
+#undef MagickCropImage
+#undef MagickExportImagePixels
+#undef MagickFlattenImages
+#undef MagickGetException
+#undef MagickGetImage
+#undef MagickGetImageDelay
+#undef MagickGetImageDispose
+#undef MagickGetImageHeight
+#undef MagickGetImagePage
+#undef MagickGetImageSignature
+#undef MagickGetImageWidth
+#undef MagickGetNumberImages
+#undef MagickMergeImageLayers
+#undef MagickQueryFormats
+#undef MagickReadImage
+#undef MagickReadImageBlob
+#undef MagickRelinquishMemory
+#undef MagickRotateImage
+#undef MagickScaleImage
+#undef MagickSetFilename
+#undef MagickSetImageBackgroundColor
+#undef MagickSetIteratorIndex
+#undef MagickWandGenesis
+#undef MagickWandTerminus
+#undef NewMagickWand
+#undef NewPixelIterator
+#undef NewPixelWand
+#undef PixelGetAlpha
+#undef PixelGetMagickColor
+#undef PixelGetNextIteratorRow
+#undef PixelSetBlue
+#undef PixelSetGreen
+#undef PixelSetIteratorRow
+#undef PixelSetMagickColor
+#undef PixelSetPixelColor
+#undef PixelSetRed
+#undef PixelSyncIterator
+
+#undef MagickSetSize
+#undef MagickSetDepth
+#undef MagickAutoOrientImage
+
+#define CloneMagickWand fn_CloneMagickWand
+#define DestroyMagickWand fn_DestroyMagickWand
+#define DestroyPixelIterator fn_DestroyPixelIterator
+#define DestroyPixelWand fn_DestroyPixelWand
+#define MagickCropImage fn_MagickCropImage
+#define MagickExportImagePixels fn_MagickExportImagePixels
+#ifndef HAVE_MAGICKMERGEIMAGELAYERS
+#define MagickFlattenImages fn_MagickFlattenImages
+#endif
+#define MagickGetException fn_MagickGetException
+#define MagickGetImage fn_MagickGetImage
+#define MagickGetImageDelay fn_MagickGetImageDelay
+#define MagickGetImageDispose fn_MagickGetImageDispose
+#define MagickGetImageHeight fn_MagickGetImageHeight
+#define MagickGetImagePage fn_MagickGetImagePage
+#define MagickGetImageSignature fn_MagickGetImageSignature
+#define MagickGetImageWidth fn_MagickGetImageWidth
+#define MagickGetNumberImages fn_MagickGetNumberImages
+#define MagickMergeImageLayers fn_MagickMergeImageLayers
+#define MagickQueryFormats fn_MagickQueryFormats
+#define MagickReadImage fn_MagickReadImage
+#define MagickReadImageBlob fn_MagickReadImageBlob
+#define MagickRelinquishMemory fn_MagickRelinquishMemory
+#define MagickRotateImage fn_MagickRotateImage
+#define MagickScaleImage fn_MagickScaleImage
+#define MagickSetFilename fn_MagickSetFilename
+#define MagickSetImageBackgroundColor fn_MagickSetImageBackgroundColor
+#define MagickSetIteratorIndex fn_MagickSetIteratorIndex
+#define MagickWandGenesis fn_MagickWandGenesis
+#define MagickWandTerminus fn_MagickWandTerminus
+#define NewMagickWand fn_NewMagickWand
+#define NewPixelIterator fn_NewPixelIterator
+#define NewPixelWand fn_NewPixelWand
+#define PixelGetAlpha fn_PixelGetAlpha
+#define PixelGetMagickColor fn_PixelGetMagickColor
+#define PixelGetNextIteratorRow fn_PixelGetNextIteratorRow
+#define PixelSetBlue fn_PixelSetBlue
+#define PixelSetGreen fn_PixelSetGreen
+#define PixelSetIteratorRow fn_PixelSetIteratorRow
+#ifdef HAVE_IMAGEMAGICK7
+#define PixelSetPixelColor fn_PixelSetPixelColor
+#else
+#define PixelSetMagickColor fn_PixelSetMagickColor
+#endif
+#define PixelSetRed fn_PixelSetRed
+#define PixelSyncIterator fn_PixelSyncIterator
+
+#define MagickSetSize fn_MagickSetSize
+#define MagickSetDepth fn_MagickSetDepth
+#ifdef HAVE_MAGICKAUTOORIENTIMAGE
+#define MagickAutoOrientImage fn_MagickAutoOrientImage
+#endif
+
+# endif /* HAVE_NTGUI WINDOWSNT */
+
+#ifdef HAVE_IMAGEMAGICK7
+/* ImageMagick 7 compatibility definitions.  Redefine API functions
+   here, after the w32 runtime linking support code.  */
+# define PixelSetMagickColor PixelSetPixelColor
+#endif
+
+
 /* Log ImageMagick error message.
    Useful when an ImageMagick function returns the status `MagickFalse'.  */
 
@@ -8411,7 +8660,7 @@ imagemagick_get_animation_cache (MagickWand *wand)
       pcache = &cache->next;
     }
 
-  DestroyString (signature);
+  MagickRelinquishMemory (signature);
   cache->update_time = current_timespec ();
   return cache;
 }
@@ -8985,13 +9234,14 @@ and `imagemagick-types-inhibit'.  */)
 {
   Lisp_Object typelist = Qnil;
   size_t numf = 0;
-  ExceptionInfo *ex;
   char **imtypes;
   size_t i;
 
-  ex = AcquireExceptionInfo ();
-  imtypes = GetMagickList ("*", &numf, ex);
-  DestroyExceptionInfo (ex);
+  if (imagemagick_type.init && !imagemagick_type.init ())
+    return Qnil;
+
+  MagickWandGenesis ();
+  imtypes = MagickQueryFormats ("*", &numf);
 
   for (i = 0; i < numf; i++)
     {
@@ -9001,6 +9251,8 @@ and `imagemagick-types-inhibit'.  */)
     }
 
   MagickRelinquishMemory (imtypes);
+  MagickWandTerminus ();
+
   return Fnreverse (typelist);
 }
 

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 04 Sep 2018 17:13:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andy Moreton <andrewjmoreton <at> gmail.com>
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Tue, 04 Sep 2018 20:11:49 +0300
> From: Andy Moreton <andrewjmoreton <at> gmail.com>
> Date: Mon, 03 Sep 2018 23:57:05 +0100
> 
> As a followup, here is a patch to add ImageMagick support for Windows.
> The patch was initially developed for Imagemagick 6, but I've updated it
> to support ImageMagick 7.
> 
> I've given this some light testing on a 64bit mingw64 (MSYS2) and 64bit
> cygwin builds, both of which use Imagemagick 7.
> 
> Please test, and report if it breaks anything on other platforms.

Thanks.  A couple of minor comments:

This needs a NEWS entry.

> +       '(imagemagick "libMagickWand-7.Q16HDRI-6.dll")))

Is this DLL name fixed for all the supported versions?  It sounds
like it's only for Imagemagick v7, and so the DLL for version 6 will
be named differently.

Also, AFAIU, there are binary incompatibilities between v6 and v7, so
an Emacs compiled with one of them should not attempt to load DLLs
from another, is that right?

For these two reasons, I think we should have a Lisp variable that
provides the version of Imagemagick with which Emacs was built, and we
need the dispatch in w32-win.el for loading the correct DLLs based on
that variable, like we do with libgif etc.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Tue, 04 Sep 2018 19:04:01 GMT) Full text and rfc822 format available.

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

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Tue, 04 Sep 2018 20:03:07 +0100
On Tue 04 Sep 2018, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton <at> gmail.com>
>> Date: Mon, 03 Sep 2018 23:57:05 +0100
>> 
>> As a followup, here is a patch to add ImageMagick support for Windows.
>> The patch was initially developed for Imagemagick 6, but I've updated it
>> to support ImageMagick 7.
>> 
>> I've given this some light testing on a 64bit mingw64 (MSYS2) and 64bit
>> cygwin builds, both of which use Imagemagick 7.
>> 
>> Please test, and report if it breaks anything on other platforms.
>
> Thanks.  A couple of minor comments:
>
> This needs a NEWS entry.
>
>> +       '(imagemagick "libMagickWand-7.Q16HDRI-6.dll")))
>
> Is this DLL name fixed for all the supported versions?  It sounds
> like it's only for Imagemagick v7, and so the DLL for version 6 will
> be named differently.

Indeed. v6 is legacy (and not available on any platform I currently test
on). The DLL name is from the current package for MSYS2.

> Also, AFAIU, there are binary incompatibilities between v6 and v7, so
> an Emacs compiled with one of them should not attempt to load DLLs
> from another, is that right?

Correct.

> For these two reasons, I think we should have a Lisp variable that
> provides the version of Imagemagick with which Emacs was built, and we
> need the dispatch in w32-win.el for loading the correct DLLs based on
> that variable, like we do with libgif etc.

Agreed. Do you use Imagemagick on mingw.org builds ? Is there a packaged
library available there ?

    AndyM





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25967; Package emacs. (Wed, 05 Sep 2018 02:40:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andy Moreton <andrewjmoreton <at> gmail.com>
Cc: 25967 <at> debbugs.gnu.org
Subject: Re: bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
Date: Wed, 05 Sep 2018 05:38:45 +0300
> From: Andy Moreton <andrewjmoreton <at> gmail.com>
> Date: Tue, 04 Sep 2018 20:03:07 +0100
> 
> >> +       '(imagemagick "libMagickWand-7.Q16HDRI-6.dll")))
> >
> > Is this DLL name fixed for all the supported versions?  It sounds
> > like it's only for Imagemagick v7, and so the DLL for version 6 will
> > be named differently.
> 
> Indeed. v6 is legacy (and not available on any platform I currently test
> on).

I see a precompiled binary on the Imagemagick site.

> The DLL name is from the current package for MSYS2.

How frequently does the name of the DLL change?  If it changes with
each update, we should find a way of allowing users to still use
previous or next compatible DLLs.  We don't want to require them to
upgrade all the time.

> > For these two reasons, I think we should have a Lisp variable that
> > provides the version of Imagemagick with which Emacs was built, and we
> > need the dispatch in w32-win.el for loading the correct DLLs based on
> > that variable, like we do with libgif etc.
> 
> Agreed. Do you use Imagemagick on mingw.org builds ? Is there a packaged
> library available there ?

No, not that I know of.  I think we should provide compatibility to
MSYS2 packages and to packages provided by Imagemagick themselves.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 03 Oct 2018 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 42 days ago.

Previous Next


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