flutter_crop icon indicating copy to clipboard operation
flutter_crop copied to clipboard

Pre-Cropping

Open K-Y-Johnson opened this issue 5 years ago • 32 comments
trafficstars

We are using your widget to create square avatars. When we do so, it pre-crops the original image.

Here is an image.

Given this original image, we placed it inside your example app, changing only line 106 to reference this image (also adding it as an asset in the pubspec.yaml).

When we run the app and set a 1:1 aspect ratio, which is the functionality we want, here's what we end up with. We can't see the person of interest because of the pre-crop that occurs due to the fit: BoxFit.cover.

Below is the desired result.

K-Y-Johnson avatar Jul 23 '20 00:07 K-Y-Johnson

Crop defaults to center of the image when the aspect ratios are not equal. You'll never know where in the image your users want to select and crop. What if the user wants to crop the horse. Or even the flower? How do you know? You may not predict what user wants.

xclud avatar Jul 23 '20 07:07 xclud

We can't see the person of interest because of the pre-crop that occurs.

It's not your task to select the human face. Users should do it.

xclud avatar Jul 23 '20 08:07 xclud

I agree. Users should be able to select whatever they want for their avatar, and it's not the programmer's job to select the human face.

The issue is that due to fit: BoxFit.cover, the image is pre-cropped, and the user is literally unable to select anything that isn't shown in the image below. The human figure isn't able to be shown, as the data isn't there since it was cropped out.

The image is exactly the same as shown in my previous message, simply zoomed out.

K-Y-Johnson avatar Jul 24 '20 00:07 K-Y-Johnson

The issue is that due to fit: BoxFit.cover, the image is pre-cropped, and the user is literally unable to select anything

Can't you pan the image?

Where did the last screenshot come from? I mean the one with black bars.

xclud avatar Jul 24 '20 15:07 xclud

No, we are unable to pan the image. That data has been cropped off.

The last screenshot came from me zooming out with two fingers and holding the image while I took a screenshot. After I released the image, it snapped back to look like the second image in my first post.

K-Y-Johnson avatar Jul 24 '20 22:07 K-Y-Johnson

We have run this situation on iOS simulator, Android emulator and iPhone Xs, and the results are the same.

K-Y-Johnson avatar Jul 24 '20 22:07 K-Y-Johnson

I see the same problem, using BoxFit.contain instead of BoxFit.cover seems to solve the problem. The Image is shown with "missing parts" if the Controller aspect ratio is different from image aspect ratio. However users can then achieve cropping of any part. Hope this helps @K-Y-Johnson

0xshipthecode avatar Aug 04 '20 21:08 0xshipthecode

Thank you for your help, @vejmelkam.

We did discuss this possible solution in #24, however it then results in showing the user "missing parts" or black bars, which the user is then able to crop into their image. Allowing the user to do this is not part of our desired functionality, though it is a possible workaround.

K-Y-Johnson avatar Aug 07 '20 21:08 K-Y-Johnson

@xclud , here is a video demonstration of the issue:

crop_cutting.zip

In this case, I'd like to crop the image to 16:9, but I want to capture the upper portion of the image. However, after selecting a 16:9 crop aspect ration, I am unable to pan and therefore unable to capture the upper portion of the image.

Note: I've tried setting fit: Boxfit.contains as suggested above on my image to no avail

nwparker avatar Nov 19 '20 09:11 nwparker

@nwparker Thank you. Now I am able to reproduce the bug. Will let you know when I fix this. Contributions are welcome :)

xclud avatar Nov 19 '20 10:11 xclud

@nwparker Thank you. Now I am able to reproduce the bug. Will let you know when I fix this. Contributions are welcome :)

I haven't had luck fixing this altogether, but it turns out out switching to BoxFit.contain vs BoxFit.cover (as mentioned above) does actually change the situation ( https://github.com/nwparker/flutter_crop/commit/ede69395d62ccc68032090d90677c79990b09fd9 )

However, it's not quite right as it allows cropping of areas outside of the image

nwparker avatar Nov 22 '20 07:11 nwparker

@nwparker Thank you. Now I am able to reproduce the bug. Will let you know when I fix this. Contributions are welcome :)

I haven't had luck fixing this altogether, but it turns out out switching to BoxFit.contain vs BoxFit.cover (as mentioned above) does actually change the situation ( nwparker@ede6939 )

However, it's not quite right as it allows cropping of areas outside of the image

We can't simply change this parameter. I think we should change the original zoom factor, which should be determined by the size of the image and the target clipping size.

SF-Simon avatar Nov 24 '20 09:11 SF-Simon

@HeebeLee correct that scaling should be adjusted too. See the comment in related closed issue https://github.com/xclud/flutter_crop/issues/24#issuecomment-660728778 that provides one workaround:

final imageAspectRatio = image.width / image.height;
final minScale = imageAspectRatio < 1.0 ? 1 / imageAspectRatio : imageAspectRatio;
cropController.scale = minScale;

This is not a complete solution since it doesn't constrain the user's panning/zooming to not enter the "empty" image parts.

rich-j avatar Nov 24 '20 19:11 rich-j

No, we are unable to pan the image. That data has been cropped off.

The last screenshot came from me zooming out with two fingers and holding the image while I took a screenshot. After I released the image, it snapped back to look like the second image in my first post.

Hi, I am still facing this issue. Any fix?

momoDragon avatar May 12 '21 11:05 momoDragon

@momoDragon I am rewriting a part of calculations at the moment. Will publish a new version when done. This bug is over a year old now and we should get rid of it.

xclud avatar May 12 '21 12:05 xclud

@momoDragon I am rewriting a part of calculations at the moment. Will publish a new version when done. This bug is over a year old now and we should get rid of it.

xclud avatar May 12 '21 12:05 xclud

@momoDragon I am rewriting a part of calculations at the moment. Will publish a new version when done. This bug is over a year old now and we should get rid of it.

I'm waiting to solve this issue and enjoy more, I have same issue that cropper select the center of image and I can't select another part of image to crop. thanks

amirucefi avatar May 21 '21 20:05 amirucefi

My cat needs help. https://vimeo.com/557199490

michaldev avatar May 31 '21 14:05 michaldev

@michaldev Sorry for the inconvenience. My own cat needs help too!. This issue is blocking version 1.0 release and i hate this.

xclud avatar May 31 '21 17:05 xclud

@xclud Hello is there any progress on that bug? Is there any branch you are working on? I could try to help you with that if you are busy.

Cheers

margorski avatar Aug 03 '21 12:08 margorski

Hi @margorski,

I appreciate your contribution. I am working on hotfix-crop-bug branch.

xclud avatar Aug 03 '21 18:08 xclud

I really appreciate the work that has been done on this package. I recognize you are doing all this work out of good will. I just wondered if any progress has been made on this issue?

jasonJamEther avatar Sep 08 '21 22:09 jasonJamEther

Hello @xclud I did a fix for that on the forked repository. Here is the pull request: https://github.com/xclud/flutter_crop/pull/71

I admit that I didn't do any heavy tests. Using the example app I checked all aspect ratios and it looks that it works. One thing I changed is a clamping area for the rotated pictures. If I remember correctly in the previous implementation it was clamping panning of the image to the inside of the picture. Now I allow panning on boundaries of the rotated image. Take a look at pictures to see how it works: image

It could be changed but it is suitable for my project so I will keep that on the forked repo.

In the next day, I will be integrating that into our app so if I will encounter any problems I will add fixes on the branch. For now, you could take a look if that is anything that will work for you.

margorski avatar Nov 16 '21 12:11 margorski

Still need some work with vertical images so not ready yet :)

margorski avatar Nov 16 '21 12:11 margorski

Ok, it looks like it is ready now. Review and see if that is a good solution to merge into your library.

margorski avatar Nov 16 '21 13:11 margorski

@margorski Thank you for the great job on this bug. You definitely make a a lot of people happy and you will make the package go to version 1.0. I appreciate your work on behalf on everyone using this package.

xclud avatar Nov 17 '21 17:11 xclud

Hello @xclud thanks for the awesome work!

Do you know when it will be available version 1.0 with this bug fixed?

FerBueroTrebino avatar Mar 08 '22 13:03 FerBueroTrebino

In case anybody needs a ready-to-go solution, I manually applied all the outstanding PRs #66, #68, and #71, to my own fork of this repo (the PRs don't apply cleanly, because the main branch has changed too much).

https://github.com/lukehutch/flutter_crop

This solves the problem for me (at least with rotation disabled, I haven't tested with rotation enabled).

lukehutch avatar Feb 28 '23 20:02 lukehutch

Actually I spoke too soon -- @margorski your PR #71 doesn't work when cropping a 16:9 aspect ratio photo using aspectRatio: 9/16. You can't select all the way to the sides of the image, and you can select a black area above and below the image.

lukehutch avatar Feb 28 '23 20:02 lukehutch

@margorski I fixed the issue in your PR, but there were more issues with rotation, e.g. the bounding box calculation and minimum scale calculations were wrong.

Since I don't need rotation, I just need a working crop widget, I removed rotation support from my fork (linked in a previous comment). That version is now fully functional. Somebody else is welcome to take that version and add rotation back in, if they're game to wrap their mind around the needed trignonometry.

lukehutch avatar Mar 01 '23 00:03 lukehutch