unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25967: 25.1; Support for ImageMagick 7
@ 2017-03-04 16:17 Tej Chajed
  2017-03-10 19:03 ` Paul Eggert
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Tej Chajed @ 2017-03-04 16:17 UTC (permalink / raw)
  To: 25967

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

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.

[-- Attachment #2: Type: text/html, Size: 994 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-03-04 16:17 bug#25967: 25.1; Support for ImageMagick 7 Tej Chajed
@ 2017-03-10 19:03 ` Paul Eggert
  2017-07-24 16:41   ` Glenn Morris
  2018-06-01 19:17 ` bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967) Karl Otness
  2018-08-28  2:12 ` bug#25967: 25.1; Support for ImageMagick 7 Glenn Morris
  2 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-03-10 19:03 UTC (permalink / raw)
  To: Tej Chajed; +Cc: 25967

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.






^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-03-10 19:03 ` Paul Eggert
@ 2017-07-24 16:41   ` Glenn Morris
  2017-07-24 22:15     ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-24 16:41 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Tej Chajed, 25967

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.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-07-24 16:41   ` Glenn Morris
@ 2017-07-24 22:15     ` Paul Eggert
  2017-07-25  1:25       ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-07-24 22:15 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Tej Chajed, 25967

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

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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-use-ImageMagick-7-and-later.patch --]
[-- Type: text/x-patch; name="0001-Do-not-use-ImageMagick-7-and-later.patch", Size: 1002 bytes --]

From bc78bd0be41a2438a522df9fa3f46b64589403dc Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 24 Jul 2017 15:13:26 -0700
Subject: [PATCH] Do not use ImageMagick 7 and later

Suggested by Glenn Morris (Bug#25967#15).
* configure.ac (IMAGEMAGICK_MODULE): Reject 7 and later.
---
 configure.ac | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 5e6dbda..648da99 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2507,7 +2507,8 @@ AC_DEFUN
   if test "${with_imagemagick}" != "no"; then
     ## 6.3.5 is the earliest version known to work; see Bug#17339.
     ## 6.8.2 makes Emacs crash; see Bug#13867.
-    IMAGEMAGICK_MODULE="Wand >= 6.3.5 Wand != 6.8.2"
+    ## 7 and later have not been ported to; See Bug#25967.
+    IMAGEMAGICK_MODULE="Wand >= 6.3.5 Wand != 6.8.2 Wand < 7"
     EMACS_CHECK_MODULES([IMAGEMAGICK], [$IMAGEMAGICK_MODULE])
 
     if test $HAVE_IMAGEMAGICK = yes; then
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-07-24 22:15     ` Paul Eggert
@ 2017-07-25  1:25       ` Glenn Morris
  2017-09-02 13:06         ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-25  1:25 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Tej Chajed, 25967

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





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-07-25  1:25       ` Glenn Morris
@ 2017-09-02 13:06         ` Eli Zaretskii
  2017-09-03  1:10           ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-09-02 13:06 UTC (permalink / raw)
  To: Glenn Morris; +Cc: tchajed, 25967, eggert

unblock 24655 by 25967
thanks

> From: Glenn Morris <rgm@gnu.org>
> Date: Mon, 24 Jul 2017 21:25:13 -0400
> Cc: Tej Chajed <tchajed@mit.edu>, 25967@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.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-09-02 13:06         ` Eli Zaretskii
@ 2017-09-03  1:10           ` Glenn Morris
  2017-12-12 16:59             ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-09-03  1:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tchajed, 25967, eggert

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





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-09-03  1:10           ` Glenn Morris
@ 2017-12-12 16:59             ` Glenn Morris
  2017-12-12 17:15               ` Marcin Borkowski
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-12-12 16:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tchajed, 25967, eggert

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





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-12-12 16:59             ` Glenn Morris
@ 2017-12-12 17:15               ` Marcin Borkowski
  2017-12-12 17:44                 ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Marcin Borkowski @ 2017-12-12 17:15 UTC (permalink / raw)
  To: Glenn Morris; +Cc: tchajed, eggert, 25967


On 2017-12-12, at 17:59, Glenn Morris <rgm@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





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-12-12 17:15               ` Marcin Borkowski
@ 2017-12-12 17:44                 ` Glenn Morris
  0 siblings, 0 replies; 23+ messages in thread
From: Glenn Morris @ 2017-12-12 17:44 UTC (permalink / raw)
  To: Marcin Borkowski; +Cc: tchajed, eggert, 25967

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/ )





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2017-03-04 16:17 bug#25967: 25.1; Support for ImageMagick 7 Tej Chajed
  2017-03-10 19:03 ` Paul Eggert
@ 2018-06-01 19:17 ` Karl Otness
  2018-06-23 10:09   ` Kévin Le Gouguec
                     ` (2 more replies)
  2018-08-28  2:12 ` bug#25967: 25.1; Support for ImageMagick 7 Glenn Morris
  2 siblings, 3 replies; 23+ messages in thread
From: Karl Otness @ 2018-06-01 19:17 UTC (permalink / raw)
  To: 25967

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

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

[-- Attachment #2: 0001-Add-support-for-ImageMagick-7-Bug-25967.patch --]
[-- Type: text/x-patch, Size: 3959 bytes --]

From 2d9b9e611e1c08240830cbb99fe445a99cc93594 Mon Sep 17 00:00:00 2001
From: Karl Otness <karl@karlotness.com>
Date: Thu, 31 May 2018 19:23:21 -0500
Subject: [PATCH] Add support for ImageMagick 7 (Bug#25967)

* configure.ac: Allow linking to ImageMagick 7 and expose the major
version found to the rest of build.
* src/image.c [MagickWand >= 7]: Define functions and types for
ImageMagick 7 compatibility.
---
 configure.ac | 17 +++++++++++++----
 src/image.c  | 15 ++++++++++++---
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index a11abc1b65..c3a5401570 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2549,9 +2549,16 @@ AC_DEFUN
 
     ## 6.3.5 is the earliest version known to work; see Bug#17339.
     ## 6.8.2 makes Emacs crash; see Bug#13867.
-    ## 7 and later have not been ported to; See Bug#25967.
-    IMAGEMAGICK_MODULE="Wand >= 6.3.5 Wand != 6.8.2 Wand < 7"
-    EMACS_CHECK_MODULES([IMAGEMAGICK], [$IMAGEMAGICK_MODULE])
+    IMAGEMAGICK7_MODULE="MagickWand >= 7"
+    IMAGEMAGICK6_MODULE="Wand >= 6.3.5 Wand != 6.8.2"
+    # As we check for ImageMagick 7 then 6 track which version we find
+    EMACS_CHECK_MODULES([IMAGEMAGICK], [$IMAGEMAGICK7_MODULE])
+    AS_IF([test $HAVE_IMAGEMAGICK = yes],
+      [IMAGEMAGICK_MAJOR=7],
+      [
+        EMACS_CHECK_MODULES([IMAGEMAGICK], [$IMAGEMAGICK6_MODULE])
+        AS_IF([test $HAVE_IMAGEMAGICK = yes], [IMAGEMAGICK_MAJOR=6])
+      ])
 
     if test $HAVE_IMAGEMAGICK = yes; then
       OLD_CFLAGS=$CFLAGS
@@ -2571,6 +2578,8 @@ AC_DEFUN
     fi
     if test $HAVE_IMAGEMAGICK = yes; then
       AC_DEFINE([HAVE_IMAGEMAGICK], 1, [Define to 1 if using ImageMagick.])
+      AC_DEFINE_UNQUOTED([IMAGEMAGICK_MAJOR], [$IMAGEMAGICK_MAJOR],
+                         [ImageMagick major version number (from configure).])
     else
       IMAGEMAGICK_CFLAGS=
       IMAGEMAGICK_LIBS=
@@ -5458,7 +5467,7 @@ AC_DEFUN
   Does Emacs use -lrsvg-2?                                ${HAVE_RSVG}
   Does Emacs use cairo?                                   ${HAVE_CAIRO}
   Does Emacs use -llcms2?                                 ${HAVE_LCMS2}
-  Does Emacs use imagemagick (version 6)?                 ${HAVE_IMAGEMAGICK}
+  Does Emacs use imagemagick?                             ${HAVE_IMAGEMAGICK}
   Does Emacs support sound?                               ${HAVE_SOUND}
   Does Emacs use -lgpm?                                   ${HAVE_GPM}
   Does Emacs use -ldbus?                                  ${HAVE_DBUS}
diff --git a/src/image.c b/src/image.c
index 992b225d7b..3c1366f5ce 100644
--- a/src/image.c
+++ b/src/image.c
@@ -8272,11 +8272,20 @@ imagemagick_image_p (Lisp_Object object)
 /* The GIF library also defines DrawRectangle, but its never used in Emacs.
    Therefore rename the function so it doesn't collide with ImageMagick.  */
 #define DrawRectangle DrawRectangleGif
-#include <wand/MagickWand.h>
+
+#if IMAGEMAGICK_MAJOR == 6
+# include <wand/MagickWand.h>
+# include <magick/version.h>
+#else
+# include <MagickWand/MagickWand.h>
+# include <MagickCore/version.h>
+/* ImageMagick 7 compatibility definitions */
+# define PixelSetMagickColor PixelSetPixelColor
+typedef PixelInfo MagickPixelPacket;
+#endif
 
 /* ImageMagick 6.5.3 through 6.6.5 hid PixelGetMagickColor for some reason.
    Emacs seems to work fine with the hidden version, so unhide it.  */
-#include <magick/version.h>
 #if 0x653 <= MagickLibVersion && MagickLibVersion <= 0x665
 extern WandExport void PixelGetMagickColor (const PixelWand *,
 					    MagickPixelPacket *);
@@ -8814,7 +8823,7 @@ imagemagick_load_image (struct frame *f, struct image *img,
 #endif /* HAVE_MAGICKEXPORTIMAGEPIXELS */
     {
       size_t image_height;
-      MagickRealType color_scale = 65535.0 / QuantumRange;
+      MagickRealType color_scale = 65535.0 / (MagickRealType) QuantumRange;
 #ifdef USE_CAIRO
       data = xmalloc (width * height * 4);
       color_scale /= 256;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-06-01 19:17 ` bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967) Karl Otness
@ 2018-06-23 10:09   ` Kévin Le Gouguec
  2018-06-25 21:08     ` Karl Otness
  2018-08-28  2:10   ` Glenn Morris
  2018-09-03 22:57   ` Andy Moreton
  2 siblings, 1 reply; 23+ messages in thread
From: Kévin Le Gouguec @ 2018-06-23 10:09 UTC (permalink / raw)
  To: 25967; +Cc: Karl Otness

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.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-06-23 10:09   ` Kévin Le Gouguec
@ 2018-06-25 21:08     ` Karl Otness
  0 siblings, 0 replies; 23+ messages in thread
From: Karl Otness @ 2018-06-25 21:08 UTC (permalink / raw)
  To: Kévin Le Gouguec; +Cc: 25967

On Sat, Jun 23, 2018 at 5:09 AM, Kévin Le Gouguec
<kevin.legouguec@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





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-06-01 19:17 ` bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967) Karl Otness
  2018-06-23 10:09   ` Kévin Le Gouguec
@ 2018-08-28  2:10   ` Glenn Morris
  2018-08-28 20:32     ` Karl Otness
  2018-08-29  3:21     ` Karl Otness
  2018-09-03 22:57   ` Andy Moreton
  2 siblings, 2 replies; 23+ messages in thread
From: Glenn Morris @ 2018-08-28  2:10 UTC (permalink / raw)
  To: Karl Otness; +Cc: 25967

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!





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: 25.1; Support for ImageMagick 7
  2017-03-04 16:17 bug#25967: 25.1; Support for ImageMagick 7 Tej Chajed
  2017-03-10 19:03 ` Paul Eggert
  2018-06-01 19:17 ` bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967) Karl Otness
@ 2018-08-28  2:12 ` Glenn Morris
  2 siblings, 0 replies; 23+ messages in thread
From: Glenn Morris @ 2018-08-28  2:12 UTC (permalink / raw)
  To: 25967-done

Version: 27.1

Added in 5729486.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-08-28  2:10   ` Glenn Morris
@ 2018-08-28 20:32     ` Karl Otness
  2018-08-30 18:00       ` Glenn Morris
  2018-08-29  3:21     ` Karl Otness
  1 sibling, 1 reply; 23+ messages in thread
From: Karl Otness @ 2018-08-28 20:32 UTC (permalink / raw)
  To: rgm; +Cc: 25967

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@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!





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-08-28  2:10   ` Glenn Morris
  2018-08-28 20:32     ` Karl Otness
@ 2018-08-29  3:21     ` Karl Otness
  1 sibling, 0 replies; 23+ messages in thread
From: Karl Otness @ 2018-08-29  3:21 UTC (permalink / raw)
  To: rgm; +Cc: 25967

On Mon, Aug 27, 2018 at 9:10 PM Glenn Morris <rgm@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





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-08-28 20:32     ` Karl Otness
@ 2018-08-30 18:00       ` Glenn Morris
  2018-08-30 21:53         ` Karl Otness
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2018-08-30 18:00 UTC (permalink / raw)
  To: Karl Otness; +Cc: 25967


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!





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-08-30 18:00       ` Glenn Morris
@ 2018-08-30 21:53         ` Karl Otness
  0 siblings, 0 replies; 23+ messages in thread
From: Karl Otness @ 2018-08-30 21:53 UTC (permalink / raw)
  To: rgm; +Cc: 25967

On Thu, Aug 30, 2018 at 1:00 PM Glenn Morris <rgm@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.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-06-01 19:17 ` bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967) Karl Otness
  2018-06-23 10:09   ` Kévin Le Gouguec
  2018-08-28  2:10   ` Glenn Morris
@ 2018-09-03 22:57   ` Andy Moreton
  2018-09-04 17:11     ` Eli Zaretskii
  2 siblings, 1 reply; 23+ messages in thread
From: Andy Moreton @ 2018-09-03 22:57 UTC (permalink / raw)
  To: 25967

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

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



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: imagemagick7.patch --]
[-- Type: text/x-patch, Size: 12694 bytes --]

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);
 }
 

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-09-03 22:57   ` Andy Moreton
@ 2018-09-04 17:11     ` Eli Zaretskii
  2018-09-04 19:03       ` Andy Moreton
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2018-09-04 17:11 UTC (permalink / raw)
  To: Andy Moreton; +Cc: 25967

> From: Andy Moreton <andrewjmoreton@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.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-09-04 17:11     ` Eli Zaretskii
@ 2018-09-04 19:03       ` Andy Moreton
  2018-09-05  2:38         ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Moreton @ 2018-09-04 19:03 UTC (permalink / raw)
  To: 25967

On Tue 04 Sep 2018, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@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






^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967)
  2018-09-04 19:03       ` Andy Moreton
@ 2018-09-05  2:38         ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2018-09-05  2:38 UTC (permalink / raw)
  To: Andy Moreton; +Cc: 25967

> From: Andy Moreton <andrewjmoreton@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.





^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-09-05  2:38 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-04 16:17 bug#25967: 25.1; Support for ImageMagick 7 Tej Chajed
2017-03-10 19:03 ` Paul Eggert
2017-07-24 16:41   ` Glenn Morris
2017-07-24 22:15     ` Paul Eggert
2017-07-25  1:25       ` Glenn Morris
2017-09-02 13:06         ` Eli Zaretskii
2017-09-03  1:10           ` Glenn Morris
2017-12-12 16:59             ` Glenn Morris
2017-12-12 17:15               ` Marcin Borkowski
2017-12-12 17:44                 ` Glenn Morris
2018-06-01 19:17 ` bug#25967: [PATCH] Add support for ImageMagick 7 (Bug#25967) Karl Otness
2018-06-23 10:09   ` Kévin Le Gouguec
2018-06-25 21:08     ` Karl Otness
2018-08-28  2:10   ` Glenn Morris
2018-08-28 20:32     ` Karl Otness
2018-08-30 18:00       ` Glenn Morris
2018-08-30 21:53         ` Karl Otness
2018-08-29  3:21     ` Karl Otness
2018-09-03 22:57   ` Andy Moreton
2018-09-04 17:11     ` Eli Zaretskii
2018-09-04 19:03       ` Andy Moreton
2018-09-05  2:38         ` Eli Zaretskii
2018-08-28  2:12 ` bug#25967: 25.1; Support for ImageMagick 7 Glenn Morris

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).