flokosiol / kirby-focus

Better image cropping for Kirby CMS
184 stars 14 forks source link

Crop doesn't respect image boundaries #6

Closed malvese closed 8 years ago

malvese commented 8 years ago

The crop doesn't seem to take image boundaries into account:

the-sea-400x300-5-44

medienbaecker commented 8 years ago

I think the second problem is a different issue. Every image's width gets scaled up a bit. I just noticed that in my installation. You wrote it only happens when the "crop is narrow", can you check if it happens with a different focus point too? If there's enough room considering the focus point, the image borders should be visible.

Full image (scaled down): thumb

Cropped image (1500x600): _38a1173_web-1500x600-31-38

Cropped image with Photoshop (1500x600): photoshop

Why does it crop the width if the cropping ratio fits into 100% width? Only the upper and lower parts should be cropped.

It looks like the changed line from https://github.com/flokosiol/kirby-focus/issues/2 is the problem. It may not be stretched now, but it's still not really fixed. The first part of the line crops the original image, the second part crops it again to be the specified height and width. Maybe it should first resize it to the specified dimensions and then crop it? The coordinate variables may not be correct then, though...

medienbaecker commented 8 years ago

We could do $img->fit_to_height($thumb->options['height']); and $img->fit_to_width($thumb->options['width']); for the two fitting modes. The $x1, $x2, $y1, $y2 need to be converted from the original image to the new dimensions.

Do you have an idea @flokosiol?

medienbaecker commented 8 years ago

Yeah, I think finally found the error. It's here: https://github.com/flokosiol/kirby-focus/blob/master/focus.php#L91 ...and here: https://github.com/flokosiol/kirby-focus/blob/master/focus.php#L110

The dimension that's not fitting inside the cropped dimension is not calculated correctly. It should be: $height = floor($dimensions->width() * $thumb->options['ratio']); and $width = $dimensions->height() / $thumb->options['ratio'];

I've added a pull request.