splittingred / phpThumbOf

A secure phpthumb output filter for MODx Revolution
http://rtfm.modx.com/display/addon/phpthumbof/
23 stars 17 forks source link

Other ZC parameters besides "1" don't work #19

Closed Martijn1981 closed 12 years ago

Martijn1981 commented 13 years ago

I've tried using :phpthumbof=w=520&h=200&zc=T, but the ZC parameter won't budge. It only seems to accept the "1" or "C" parameter.

I'm trying to use the other ones that are listed here: http://phpthumb.sourceforge.net/demo/docs/phpthumb.readme.txt

tinpixel commented 13 years ago

I'm also getting this issue with 1.2.0pl on a Modx 2.1.1pl install - it defaults to Center regardless of ZC setting.

I've got another site running on the same server using phpThumbOf 1.1.0pl on a Modx 2.0.8pl install and it's works perfectly - especially coupled with TV dropdown to allow clients to specify the crop position of their images.

Any quick fixes or updates would be most appreciated. Thanks in advance.

bertoost commented 13 years ago

I'm also getting this issue and would be great to fix it!

splittingred commented 13 years ago

Try using 'far'. This seems to be an issue with phpThumb.

bertoost commented 13 years ago

I've tried "far" but didn't help either

Martijn1981 commented 13 years ago

Same here.

tinpixel commented 13 years ago

Sorry, not working here either...

lithiumlab commented 13 years ago

Not working, this is a must for cropping correctly. Can't crop from top for example, image always cropped from center.

elz64 commented 13 years ago

REally neede now. How to satisfy customers ??

MichaelvanLaar commented 12 years ago

I noticed a similar, but not the esact same behavior. I have two websites, both running phpthumbof 1.2.2. The strange thing is that the parameter combination w=100&h=100&zc=1 works like a charm on one of the sites. On the other website the zc parameter is completely ignored. I have no idea, why!

As a workaround for the second website, I added a 1px white border via “&fltr[]=bord|1|0|0|ffffff” which makes the zc parameter work. But of course this is a pretty lame workaround.

splittingred commented 12 years ago

This should be fixed in MODX 2.2+.

MichaelvanLaar commented 12 years ago

Great :-) Gotta try Revo 2.2 tomorrow, anyway.

miduku commented 12 years ago

Tried it with Revo 2.2.0-pl2 (advanced) and phpThumbOf 1.3.1 and it still doesn't seem to work properly.

The other options like &q, &far, etc. are also not working. The workaround from MichaelvanLaar works... but only for &zc

The Gallery component has also problems outputting these options right...

lithiumlab commented 12 years ago

This one finally got solved?

Martijn1981 commented 12 years ago

I don't think so unfortunately...

oo12 commented 12 years ago

Have you checked that phpThumb is using ImageMagick to generate the image? If you don't have IM installed or phpThumb doesn't detect it due to some configuration weirdness then it'll fall back to GD, which doesn't support options other than C for zoom crop. The problem may not be related to MODX at all, but rather to your IM install.

I ran into the same problem (MODX 2.2.5) and tracked it down to this. The server has ImageMagick installed. which convert returns /usr/bin/convert. It's indeed in that directory but file_exists('/usr/bin/convert') returns false. I don't know why. PHP Safe Mode is off. Anyway, this causes phpThumb to think ImageMagick isn't installed, since on line 1116 of core/model/phpthumb/phpthumb.class.php it checks this:

if ($which_convert && ($which_convert{0} == '/') && @file_exists($which_convert)) {

I changed this line to

if ($which_convert && ($which_convert{0} == '/') && @stream_resolve_include_path($which_convert)) {

Which performs the same check, only it finds convert on my server and the other zoom crop options now work.

Well, almost. If you find your server loads spiking and PHP hitting its execution time limit before it's able to resize several images, your ImageMagick version may have this problem. Mine reports its version as

Version: ImageMagick 6.5.4-7 2012-05-07 Q16 OpenMP http://www.imagemagick.org

By the way, that OpenMP in there throws off phpThumb's version detection, line 1150 of the same file

`if (preg_match('#^Version\: [^0-9]*([ 0-9\\.\\:Q/]+) (http|file)\:#i', $versionstring[1], $matches)) {`

You could fix that by changing it to

if (preg_match('#^Version\: [^0-9]*([ 0-9\\.\\:Q/]+)#i', $versionstring[1], $matches)) {
opengeek commented 11 years ago

@oo12 Unfortunately, this change will only work in PHP >= 5.3.2 and MODX needs to support back to at least 5.2.x; currently it supports back to 5.1.x with certain exceptions.

I'll see if there is another way to address this without the 5.3.2+ dependency on the stream_resolve_include_path function.

oo12 commented 11 years ago

Perhaps go the opposite direction and add an option for disabling this extra check if the user knows that convert definitely installed and executable. Maybe if it's explicitly specified in phpthumb_imagemagick_path then assume it's really there. After all, by this time phpThumb has already run which convert and checked that it's returned something reasonable, so file_exists() is something of a double check, and one that's generating false negatives on some systems.

opengeek commented 11 years ago

What if we do something like this:

if ($which_convert && ($which_convert{0} == '/') && (version_compare(phpversion(), '5.3.2', '>=') ? @stream_resolve_include_path($which_convert) : @file_exists($which_convert))) {

At least it will resolve the issue on systems running newer versions. Thoughts?

oo12 commented 11 years ago

Yeah, this ought to save some folks a headache, since PHP >= 5.3.2 is pretty prevalent these days. Probably the whole process of checking for convert could be more streamlined though, since phpThumb does seem a bit paranoid about making double dang sure it's there :-) Likely there's a good reason for that however. Anyway, it seems like splitting hairs to worry about it much more.

itskingori commented 11 years ago

I'm using 2.2.6 (phpthumb zoom crop is still ignored even after setting to '1') this is after I've just moved from 2.2.4 (where zc='1' was working fine) ... so after digging around I found this comment in the bug tracker ...

http://tracker.modx.com/issues/9006#note-6

He suggests that the line that was added to 2.2.6 doesn't solve the problem (which is true in my case ...) after reverting the line back to ...

if ($which_convert && ($which_convert{0} == '/') && @file_exists($which_convert)) {

... there was an improvement in quality in the thumbnails and I could tell there was some zoom-cropping going on.

Also checked an image that i had set height=390, width=800, zc=1 but it was zoom-cropped to 800x800 (note that the original file was large, seems like height was ignored)

neoneddy commented 11 years ago

I tried opengeek's edit with no luck.
PHP Version 5.4.10 imagick module version 3.1.0RC2

midito commented 11 years ago

Any updates on this? Tried ZC with value different than "1" and doesn't work. This is an issue reported 2 years ago!!!!

midito commented 11 years ago

I get this error message: [9] => ImageMagickThumbnailToGD() aborting because cannot find convert in $this->config_imagemagick_path (), and which convert returned (which: no convert in ((null))) in file "phpthumb.class.php" on line 1131

It's really strange because it's configured in system settings and "which convert" works fine when accessing from ssh. I have also other scripts working fine with ImageMagick.

$this->config_imagemagick_path is returning an empty string and which convert is null...

As a temporary hack, you can replace the function ImageMagickCommandlineBase until this bug is fixed:

function ImageMagickCommandlineBase() { // Manually set the ImageMagick path in phpthumb $IMpath = '/usr/bin/convert' return $IMpath; }

midito commented 11 years ago

You can follow my last discussion about this in forums: http://forums.modx.com/thread/49190/phpthumbof-zoom-crop-with-t-or-tl-not-working

aktimale commented 7 years ago

MODX Revolution 2.5.1-pl, phpThumbOf 1.4.0-pl Find core/model/phpthumb/phpthumb.class.php line 1728:

switch (strtoupper($this->zc)) {
                                case 'T':
                                    $commandline .= ' -gravity north';
                                    break;
                                case 'B':
                                    $commandline .= ' -gravity south';
                                    break;
...

Write all "gravity" parameters in uppercase: North, South, West, East, NorthWest, NorthEast, etc:

switch (strtoupper($this->zc)) {
                                case 'T':
                                    $commandline .= ' -gravity North';
                                    break;
                                case 'B':
                                    $commandline .= ' -gravity South';
                                    break;
... 

Save phpthumb.class.php and enjoy. PhpThumb use imagemagick http://www.imagemagick.org/script/command-line-options.php#gravity. I think, "gravity" parameters case sensitive.

frogabog commented 5 years ago

It's been a while since this was visited, and I've applied the uppercase solution aktimale proposed but am not getting any results other than what I get with zc=1 for all gravity options. It seems this is closed without a solution, at least here since my version of phpthumb.class.php had lowercase gravity parameters still. Any assistance is very appreciated.