AlchemyCMS / alchemy_cms

Alchemy is the Open Source Rails CMS framework for the component based web that can be used as classic server side rendered or headless CMS.
https://www.alchemy-cms.com
BSD 3-Clause "New" or "Revised" License
837 stars 313 forks source link

Image cropper selection is not reflecting the default centered cropping mask for new pictures. #567

Closed tvdeyen closed 10 years ago

tvdeyen commented 10 years ago

If one wants to crop a picture of a newly created EssencePicture, then the default cropping mask is not centered. It sits at the left edge of the image.

This line of code is definitely wrong and the whole picture.rb#default_mask method needs to be refactored.

mamhoff commented 10 years ago

I've looked into it, and it's actually not so much wrong as wrongly specified I think.

First, since Alchemy::Picture doesn't have cropping data, I think the function should really go in to Alchemy::EssencePicture (which is also the only place from where the function is called).

Second, picture.rb#default_mask crops like ImageMagick would with the # argument - it first resizes the mask and then applies the cropping. Hence the 0 in those lines of code.

Should the behaviour rather be like this:

If the mask that's provided is smaller than the image on both dimensions, crop without resize from center. If the mask that's provided is bigger on at least one dimension, then resize the mask so it fits the image?

tvdeyen commented 10 years ago
  1. The EssencePicture model does not have the informations we need from the image_file we need, so the method has to be in the Picture model.
  2. In the current implementation picture.rb#default_mask does not crop like ImageMagick does. It always places the selection at the left edge of the image, hence the 0 in https://github.com/magiclabs/alchemy_cms/blob/master/app/models/alchemy/picture.rb#L172

The desired behaviour is:

When the mask is smaller than the rendered preview image (Not the original image: it gets automatically downsized in the view), then the selection should be placed in the center of the image, regardless of the orientation of the image, so we have the same result as from ImageMagick.

This is very tricky, espacially because the JCrop library has a lot of options and it automatially sizes the selection dependent from ratio, image size, etc.

I guess, we need to find a simple solution to provide the default mask and let JCrop handle the rest.

mamhoff commented 10 years ago

Would #585 do the trick?

tvdeyen commented 10 years ago

Fixed with https://github.com/magiclabs/alchemy_cms/pull/590