Imagick / imagick

🌈 The Imagick PHP extension 🌈
http://pecl.php.net/imagick
Other
532 stars 135 forks source link

Constants and their supported versions need updating. #287

Open Danack opened 5 years ago

Danack commented 5 years ago

The documenation needs updating for which versions support which constants.

Danack commented 5 years ago

@SkepticaLee To point you in the right direction - the constants in Imagick are defined in this file

All of them have this format:

IMAGICK_REGISTER_CONST_LONG("COMPOSITE_COLORDODGE", ColorDodgeCompositeOp);

where the string is the string that they will be available as in Imagick, and the second value is a C enum value defined in one of the ImageMagick headers.

For ones that are only available in certain versions of ImageMagick, they are wrapped with an #if macro function. e.g.

#if MagickLibVersion >= 0x701
    IMAGICK_REGISTER_CONST_LONG("FILTER_HANNING", HannFilter);
    IMAGICK_REGISTER_CONST_LONG("FILTER_HANN", HannFilter);
#else
    IMAGICK_REGISTER_CONST_LONG("FILTER_HANNING", HanningFilter);
#endif

In this example, the C enum name was changed in ImageMagick, so we added the new constant name, but kept the old spelling also defined to prevent a BC break.

Currently keeping the docs up-to-date with the code is a manual process (which is why sometimes it doesn't happen).

There's a pretty good set of instructions of how to currently edit the PHP documentation here but as I said elsewhere, it might be easier to make notes of what needs to be changed, and check in a few weeks if the documentation is on github which will make it a lot easier to edit.

Also, it might be a good idea to refactor how the constants are managed, to make it so that the version info for the docs can be generated from (or maybe sync'ed with) the source code.

SkepticaLee commented 5 years ago

@Danack

All the current, valid constants can be listed with: var_dump ((new ReflectionClass ("imagick"))->getConstants ())

The question is: Do we need to list the deprecated constants, with their sometimes confusing dependencies, in the main list, or would a separate list of deprecated constants be more to the point?

Danack commented 5 years ago

To be clear, I'm not sure.

All the current, valid constants can be listed with: You should look in the code, so you can see that information depends on which version of ImageMagick you're using, and some information which is hard-coded in the Imagick source files.

For Imagick the choice of deprecating stuff is slightly different than other libraries. In most libraries that design their own constants and methods, they can choose a sensible way of deprecating stuff.

Because the constants and methods come from the underlying ImageMagick library, the constants and methods that will be available to end users will depend on which version of ImageMagick they compile against.

So up until we drop support for old versions of ImageMagick, it doesn't really matter which version of Imagick people use, they need to look at which version of ImageMagick they are choosing to install to see what will be available or deprecated.

In an ideal world, we wouldn't need to do much for this. That information about which versions stuff is available in would come from ImageMagick itself. However currently that project doesn't maintain that data in an easy to see format.

I think what would make sense (and would be better than the current way of doing things) is something like the following.

Have a PHP file that lists the minimum and maximum version for constants and methods. So something like:

<?php

$constants = [
    "FILTER_HANNING", "HannFilter", 0x701, null
];

Which defines that the Imagick constant FILTER_HANNING comes from the ImageMagick library enum HannFilter if the ImageMagick library is at least version 0x701 and currently there is no upper version limit (i.e. it hasn't been removed yet).

From that we could generate the code needed for the file where the constants are listed in Imagick, https://github.com/Imagick/imagick/blob/209fe54148ed6068bc2d1e41a64fbd4bc95b4ae5/imagick_helpers.c#L978-L1246

We could also then generate the documentation page that shows which versions of ImageMagick the constants and methods would be available for.....

Actually we could do something cooler than that.

We could make a page on phpimagick.com that allows people to pick two versions of ImageMagick, and show them what constants + methods have been removed or added between those versions.

So when someone goes to upgrade from one of the myriad versions of ImageMagick they could use that page to see compatibility notes between the versions.

Actually that reminds me this https://abi-laboratory.pro/index.php?view=timeline&l=imagemagick is a tool that show the differences between ImageMagick versions. Theoretically it might be possible to use that to generate the version info.....but it's not something I've though of doing before.

So.....thoughts?

SkepticaLee commented 5 years ago

I have also noticed that resizeImage() now no longer performs sharpening/blurring after resizing, so the third parameter float $blur could be left out of the documentation.

SkepticaLee commented 4 years ago

The following functions no longer work with the latest versions ("undefined method"):

setImageBias medianFilterImage autoGammaImage

Also waveletDenoiseImage no longer seems to be available.