neos-ui
neos-ui copied to clipboard
FEATURE: Adds pixel snapping support to aspect ratio cropping
Description: This aims to prevent cropping issues by cropping areas that will lead to rounding errors.
What I did
This change adds a new configuration option to the ImageEditor.
image:
ui:
inspector:
editorOptions:
crop:
aspectRatio:
pixelSnapping: true
With this setting set to true, image cropping that is handled in the Ui will snap to pixels ranges. For example, it will no longer be possible to select a 170px x 96px
crop area when the ratio is set to 16:9 as 16 is not a divider of 170.
Usually it would lead to rounding and sometimes losing pixels in the process. This is especially annoying when creating thumbnails in different resolutions as you would sometimes end up with 160px x 90px
and 320px x 179px
images.
Large values in custom aspect ratios, for example 400:300, are also supported as it gets calculated the same way as 4:3.
How I did it
Modify the response from react-image-crop by manipulating the values in cropArea before calling the onComplete callback passed to the ImageCropper component.
I also tried to implement onChange behaviour which worked nicely on CodePen with a react-image-crop at version 9. We're still using version 2 in here and the onChange callback is a bit buggy in version 2 so I skipped that part.
How to verify it
Crop an image with 16:9 aspect ratio and display the final resource with and height. You will get width and height values that are not multiples of the aspect ratio's values, for example 1922x1080 (When calculating paddings to reserve space for the image, you will end up with 56.21313% instead of 56.25% which could also break pixel perfect display for fixed 16:9 images.)
When cropping the image with snapping enabled, it now is 56.25% as expected. I attached a video to display the aspect ratio problem:
https://user-images.githubusercontent.com/41791606/157550570-b4800edd-84b6-471e-9945-cd1d0297fca1.mp4
Thanks for providing this improvement. Resolved some linter issues. Will test soonish :)
I would argue for setting the pixelSnapping option to true by default with the next major :)
i agree, is there any place where we dont want this?