Cleaned up the commandline ImageMagick support. The crop works now. Improved the path validation. Dumped the calls to the alter hook, I don't think it really works with the ImageAPI way of doing things.

Also fixed one bug in the toolkit selection form--when only a single toolkit is installed that's no GD the wrong variable was being set.

Marking this RTBC since the current version is pretty broken and because this is just a minimally changed backport of some code I've been using in D6.

CommentFileSizeAuthor
#2 imageapi_246203.patch7.67 KBdrewish
#1 imageapi_246203.patch8.56 KBdrewish
imageapi_imagemagick.patch8.54 KBdrewish

Comments

drewish’s picture

drewish commented
StatusFileSize
new imageapi_246203.patch8.56 KB

imageapi_imagemagick_image_crop() wasn't adjust the $image object's size like imageapi_imagemagick_image_resize().

drewish’s picture

drewish commented
StatusFileSize
new imageapi_246203.patch7.67 KB

Split the core imageapi settings bug fix of to #246336: Can't enable just one non-GD imagetoolkit kit..

Also fixes some typos in the openbasedir error message.

dopry’s picture

dopry commented
Status: Reviewed & tested by the community » Needs work

I've committed the variable set part, but I was using a a #validate on purpose... You will need to stick with checking the path in #validation for me to accept the rest of this patch.

dopry’s picture

dopry commented
Status: Needs work » Fixed

I've committed variations of the path validation/version reporting to HEAD, along with fixes to both crop and resize setting the height and width of the image object.

drewish’s picture

drewish commented
Status: Fixed » Active

we should really remove all that hook imagemagick alter stuff... i'll re-roll.

dopry’s picture

dopry commented
Status: Active » Fixed

oops I removed that alter stuff now that I understand why, and switched the error reporting in the exec function to watchdog instead of dsm and added a line about false positives on settings pages.

drewish’s picture

drewish commented
Status: Fixed » Active

i'm also not super into the way that you're doing the path validation... i think the after_build worked better.

drewish’s picture

drewish commented

whoops, forgot to update before posting... well while i'm bitching you dropped my change that passed the $convert_path into _imageapi_imagemagick_convert_exec() and replaced it with a hack to remove the errors...

dopry’s picture

dopry commented
Status: Active » Fixed

we'll have to agree to disagree yet again :). I don't want users to be able to add invalid paths and I don't want duplicate errors to show up, and I don't know why you would ever need a custom path to imagemagick... please enlighten me on the usefulness of that outside of your 'hack' to prevent errors from displaying. ;)

I cleaned up my hack.. the conditional in the after_build actual prevents the errors from displaying in the
_exec function....

oh and head has progressed some more.. mainly gd fixes for missing imagefilter and imagerotate on certain linux dists...

.darrel.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.