Re: Re: [PATCH] Image_Transform IM driver crop() implementation

On Thursday 27 November 2003 12:03 am, Peter Bowyer wrote: > At 17:14 26/11/2003 -0800, Ian Eure wrote: > >While working with Image_Transform, I noticed that none of the drivers had > > a crop() implementation. I needed one for IM, so I wrote it, and here it > > is. > > Thanks Ian - I've committed it to CVS. > Great. While doing some more testing, I found that scaling the image after a crop didn't work correctly, and resulted in the image aspect being distorted. Plane resize() worked fine, but using scaleByLength() or another scaling function which preserves the image aspect did not. They were using the original image dimensions instead of the cropped dimensions. This patch against CVS fixes the problem. It seems to work fine, as I've tried various combinations of crop, rotate, and scale, and everything works as expected. Is it ok to alter img_x/img_y directly like I'm doing here? --- Driver/IM.php 27 Nov 2003 08:02:23 -0000 1.18 +++ Driver/IM.php 28 Nov 2003 20:29:24 -0000 @@ -128,7 +128,7 @@ } // End rotate /** - * Crop image + * Crop image * * @author Ian Eure <ieure@websprockets.com> * @@ -143,6 +143,11 @@ // Do we want a safety check - i.e. if $width+$x > $this->img_x then we // raise a warning? [and obviously same for $height+$y] $this->command['crop'] = "-crop {$width}x{$height}+{$x}+{$y}"; + + // I think that setting img_x/y is wrong, but scaleByLength() & friends + // mess up the aspect after a crop otherwise. + $this->new_x = $this->img_x = $width - $x; + $this->new_y = $this->img_y = $height - $y; } /**