Support ImageMagick 7 #148
Comments
Since the earth was cooling and Xastir used ImageMagick, we've used the existence of the script "Magick-config" to conclude whether ImageMagick was installed or not, and if it was, we used this script to get Magick to tell us what CFLAGS, LDFLAGS, and LIBS to use to build with Magick support. Magick 6 deprecated this script in favor of one called MagickCore-config. It works exactly like the older one did and in ImageMagick 6 returns exactly the same values for all requested flags. Magick 7 deleted the old script. We now first look for MagickCore-config and set MAGIC_BIN to that path if we find it. If we don't find it, we look for Magick-config and set MAGIC_BIN to that path if we find it. This commit closes Xastir#147. It does not in fact allow Magick 7 to work with Xastir, but that's another issue (Xastir#148). This one had to be cleared before we could even start on the harder one.
|
Did a quick dig, and it looks like magick/api.h was also deprecated in ImageMagick6, and all it did was include magick/MagickCore.h. It is entirely possible that this whole thing could be fixed by just probing for magick/MagickCore.h and including it if it exists, and only probing for magick/api.h and including it if the other fails. There are a lot of places where this file is included, but if it is a correct fix, it's just a bunch of typing and a little more careful use of the macros that AC_CHECK_HEADER creates. |
|
Couple thoughts on this before I put it aside and go back to what I'm supposed to be doing today:
If we replace the AC_CHECK_HEADER call with an AC_CHECK_HEADERS that checks all of the appropriate files (magick/MagickCore.h and magick/api.h), we'll get a more complete picture of the Magick install and can better choose which thing to include, because we'll know which exists and which doesn't at the point of inclusion. This is better than the simple yes/no "do we have ImageMagick" local variable (which we can still set and use in configure itself). I hope to have some time to really play around with this later, but I had better knock it off for now. I do believe that we have already gotten the code in proper shape to work with IM7 by getting rid of the IM6 deprecation warnings as part of issues #23 and #81. I am hopeful that just fixing the include issue and the configure probe will be enough to close this. But you never know with IM. |
|
I was overoptimistic about IM6 deprecation warnings in the previous comments. I have the configure issues nailed, but we are still using some deprecated ImageMagick functions in map_geo.c, notably GetImagePixels. Fortunately, this very same usage has already been fixed in map_OSM.c with ifdefs that check Magick version, so they can be ported over fairly easily. But there is also a use of SyncImagePixels in map_geo.c that was introduced in commit ac6c812, and this function is also deprecated, in favor of SyncAuthenticPixels. This, like the GetImagePixels->GetAuthenticPixels deprecation, should just be a matter of calling the newer function with an extra exception pointer argument. Shouldn't be terrible to work around. More troubling is the use of InitializeMagick in main.c. This function is deprecated in Magick 6, and apparently the new one is called MagickCoreGenesis, which looks like it MIGHT be usable directly as a replacement if one passes in an extra argument of MagickTrue. |
|
Well, I'm able to get the ImageMagick 6 deprecations cleared out, but completely failed with ImageMagick 7. Too many changes between data types in ImageMagick6 and 7, and just about everything that uses Magick fails to compile. There are things like IndexPacket that no longer exist, functions that now take extra arguments, and even the QuantumDepth, which is defined by an ImageMagick header, is no longer an integer -- it's now a POINTER. I will create a pull request that fixes the IM 6 deprecation warnings, but will have to leave this bug open. We had just stopped talking about removing all ImageMagick support, precisely because the API breakage had slowed down (see #25). Might be time to talk about it again unless we really want to chase the massive changes in IM7. |
Configure can probe for MagickCore/MagickCore.h if it exists, and if it does, it should be used instead of magick/api.h. Note: this is intended to help with getting bug Xastir#148 going, but is way too little to get the job done. We do NOT work with ImageMagick7. Fixes Xastir#150
|
It may be the case that my real problems with ImageMagick 7 are due to the package on my system being built with HDRI, which was always known to be a problem for Xastir because it changes all sorts of datatypes into things Xastir can't use (doubles, pointers, whatever). On BSD, ImageMagick6 is NOT built that way, but ImageMagick 7 is. We have NO path forward or even plans for using Magick with HDRI (which is completely unnecessary for map images anyway). We have always told users that same thing when they encountered Magick builds on their systems that had HDRI enabled. Just sayin'. |
|
My issues with ImageMagick 7, while rendered impossible with HDRI support (which is now the DEFAULT for IM7 and has to be explicitly disabled), were not solely due to that, and this issue is going to be very difficult. They have really completely changed how pixel channels are accessed, and it may not be possible to wedge this support in. Most of our use of Magick data is in fact now outdated. https://imagemagick.org/script/porting.php Whether GraphicsMagick will go down this same path or not is a scary question to ponder. If so, we will have a great deal of work cut out for us. |
|
A thread about detecting HDRI: |
|
Or just use AC_CHECK_DECL for the MAGICKCORE_HDRI_SUPPORT symbol at configure time. That is set if HDRI is enabled, and AC_CHECK_DECL tests the existence of the symbol in the headers. Just a matter of picking the right arguments and includes for the test, which I'm not gonna look at today. |
Since the earth was cooling and Xastir used ImageMagick, we've used the existence of the script "Magick-config" to conclude whether ImageMagick was installed or not, and if it was, we used this script to get Magick to tell us what CFLAGS, LDFLAGS, and LIBS to use to build with Magick support. Magick 6 deprecated this script in favor of one called MagickCore-config. It works exactly like the older one did and in ImageMagick 6 returns exactly the same values for all requested flags. Magick 7 deleted the old script. We now first look for MagickCore-config and set MAGIC_BIN to that path if we find it. If we don't find it, we look for Magick-config and set MAGIC_BIN to that path if we find it. This commit closes Xastir#147. It does not in fact allow Magick 7 to work with Xastir, but that's another issue (Xastir#148). This one had to be cleared before we could even start on the harder one.
ImageMagick 7 is not backward compatible to ImageMagick 6, even with the issue of #147 fixed. The first, most obvious sign of this is that Xastir will not find "magick/api.h" at configure time, because they don't have that file anymore. Xastir absolutely depends on it.
Supporting ImageMagick 7 will involve digging through Xastir's ImageMagick usage and figuring out whether we're using any features that have been completely removed, and if so, what we're supposed to do instead. Some of this might have been taken care of by the older effort that fixed some of the deprecation warnings, but even if it was, we'll have to figure out the correct set of includes to use instead of the old "magick/api.h" include.
The text was updated successfully, but these errors were encountered: