guoyingtao / Mantis

An iOS Image cropping library, which mimics the Photo App written in Swift.
MIT License
921 stars 183 forks source link

Min size / max size options #175

Closed vanniktech closed 2 years ago

vanniktech commented 2 years ago

My backend wants avatars which have a 1:1 aspect ratio (doable), and as for the size it can be anything between 512x512 & 2048x2048.

I'd like to have an option to constrain the size in a given range.

TOCropViewController has support for a min size, although indirectly: https://github.com/TimOliver/TOCropViewController/issues/478#issuecomment-821913494

I'd be happy with doing the calculations myself too though and having a maximumZoomScale & minimumZoomScale API. Alternatively, something like maximumCropSize: CGSize & minimumCropSize: CGSize

guoyingtao commented 2 years ago

Thanks for the suggestion. I will think about this feature.

guoyingtao commented 2 years ago

@vanniktech The cropped image size is due to several reasons like updating cropping range, zooming the original image, rotating the original image etc. So when the cropped image size is not in the range like 512x512 - 2048x2048, do you want to disable some operations like not allowing user to update cropping range / zooming or rotating the original image? Or do you have other ideas to make sure the cropped image in the expected range?

vanniktech commented 2 years ago

Rotating (clockwise, counterclockwise, -1, +1 etc) should always work. They don't change the image size.

If the min size is defined as 512x512, you simply won't be allowed to zoom in any further. I guess somewhere there must already be some logic for this whether implicit oder explicit since I wan't seem to be able to get a 1x1 image out of this cropping library.

As for the max size 2048x2048, the initial zooming path needs to be the size of the max size if one is set and any further zooming out should not be allowed. (The latter also accounts for zooming in & out)

guoyingtao commented 2 years ago

When user updates cropping range, also simply don't allow user to change the cropping box?

vanniktech commented 2 years ago

Exactly. No matter which way the user zooms in or out, the crop needs to be in the range of the provided min / max sizes, in my case 512x512 and 2048x2048, meaning at some point the user is not able to zoom in or out further.

guoyingtao commented 2 years ago

Hi @vanniktech I did some tests. It looks not easy to limit crop size just by not allowing user to do something like changing the cropping box. Sometimes user can be just over the limitation a little bit but it is not easy to recover to the previous status. Also the minimumZoomScale is set again when changing the cropping box or rotating the image.

So I am thinking if it works that I provide the output image size and some property say isCroppedImageOverRange after each image crop change without disabling any user actions. Then it is up to you to give user hint like show the output image size and size range somewhere and/or show some alert message to user.

What do you think about it?

vanniktech commented 2 years ago

Sometimes user can be just over the limitation a little bit but it is not easy to recover to the previous status.

In which case can that happen?

The output image size, I can already get in cropViewControllerDidCrop using:

ImageSize(width: Int32(cropped.size.width), height: Int32(cropped.size.height))

Also the minimumZoomScale is set again when changing the cropping box or rotating the image.

Why is minimumZoomScale changed when rotating the image? Is it due to different aspect ratios and hence a new minimumZoomScale needs to be calculated?

guoyingtao commented 2 years ago

For minimumZoomScale, I have this function which is called after each updating crop box. The function is to keep the selection area after rotating.

func updateMinZoomScale() {
    minimumZoomScale = getBoundZoomScale()
}

If we don't rotate the image, the minimumZoomScale will always be the default one (1.0). After rotating, the minimumZoomScale could be less than 1.0 or greater than 1.0. It depends.

Before rotating - minimumZoomScale is 1.0 before rotating

After rotating - minimumZoomScale changed after rotating

guoyingtao commented 2 years ago

Yes, you can get output image size by using cropViewControllerDidCrop, but it takes time to do real crop work. I am thinking to provide a output size before cropping which can be used to show realtime output size.

vanniktech commented 2 years ago

Hmm but should not rotating the image yield this:

Screen Shot 2022-07-16 at 22 54 27

So basically, the image is shown entirely, it's zoomed out (to keep the image constraints).

guoyingtao commented 2 years ago

To prevent user from updating crop box out of range, I was trying to get output crop image size in touchesMoved function in CropView+Touches.swift, when detecting the output image size is over range, which need to update crop box first, so the crop box would be out of range, we need to restore it to a previous valid state, which may make things complicated.

Considering minimumZoomScale changes according to user's different operations, I am thinking maybe I just provide enough information after updating crop box / zooming / rotating, it is up to you to give users hint to tell them the current selection is valid or not.

Of course you can always check the final output image size when cropping the image.

guoyingtao commented 2 years ago

Hmm but should not rotating the image yield this:

Screen Shot 2022-07-16 at 22 54 27

So basically, the image is shown entirely, it's zoomed out (to keep the image constraints).

I just followed the same way the native Photo app does. After rotating, the crop box area will occupy the main area (aspectFit) as before rotating.

vanniktech commented 2 years ago

It just looks like in your example https://github.com/guoyingtao/Mantis/issues/175#issuecomment-1186288362 that the leafy parts of the image have disappeared completely after rotating.

You're doing aspectFit on the crop box area though (what the Photo App does too?) right rather than the complete image (what I expect), correct?

guoyingtao commented 2 years ago

@vanniktech Sorry my explanation is not very clear. The complete image is still there, you can move the image to see the leafy part. It is just the crop box part will automatically expand to occupy the main visible area.

The Photos App is the one made by Apple

image

Simulator Screen Recording - iPhone 13 Pro Max - 2022-07-17 at 06 45 07

vanniktech commented 2 years ago

Ah, now I know what you mean. I was always thinking in 1:1 aspect ratio and not in others (such as the one you were selecting). If you're only allowing 1:1 you won't have the problem with rotating and suddenly being over bounds.

guoyingtao commented 2 years ago

It seems not suitable to add crop size control logic only for 1:1 aspect in a library.

Actually there is already a example which can show cropped size after each size change operation. But it gets output size by doing real cropping work. I improved it and also made it show size change when rotating an image.

image

Simulator Screen Recording - iPhone 13 Pro Max - 2022-07-17 at 12 27 17

vanniktech commented 2 years ago

It seems not suitable to add crop size control logic only for 1:1 aspect in a library.

Yeah totally understandable. What if we just expose some minimum and maxiumum zoom scale? That way they can be calculated and used for this kind of min/max sizing.

guoyingtao commented 2 years ago

Sure I can expose minimum and maximum zoom scale, but since I plan to expose output size, do you think you still need them to do calculations by yourself?

Simulator Screen Recording - iPhone 13 Pro Max - 2022-07-17 at 12 27 17

vanniktech commented 2 years ago

If minimum & maximum scale are exposed, I will only need them.

guoyingtao commented 2 years ago

Does it work for you if you get minimum & Maximus scale from cropViewController in the delegate function cropViewControllerDidEndResize(_ cropViewController: CropViewController, original: UIImage, cropInfo: CropInfo)?

vanniktech commented 2 years ago

I thought I could set them in the options. Maybe in here

guoyingtao commented 2 years ago

Do you mean you set initial minimum & maximum scale in CropViewConfig, then I update them after size change operations?

guoyingtao commented 2 years ago

I think I can expose current zoomScale then you can check if the zoomScale is out of range. BTW I will expose image output size anyway, if you want to check if the output image size is out of range before cropping, you can always directly use it.

vanniktech commented 2 years ago

Do you mean you set initial minimum & maximum scale in CropViewConfig, then I update them after size change operations?

That would be really cool.

I think I can expose current zoomScale then you can check if the zoomScale is out of range. BTW I will expose image output size anyway, if you want to check if the output image size is out of range before cropping, you can always directly use it.

Sounds good!

guoyingtao commented 2 years ago

@vanniktech For your case (always 1:1 crop box, no rotation dial), I just realized you may just need to set minimum zoomScale and maximum zoomScale in the beginning, then you can control the final output image size.

I exposed minimumZoomScale & maximumZoomScale in CropViewConfig in PR #184 Let me know if it works for your case, then I can merge it into master.

vanniktech commented 2 years ago

For your case (always 1:1 crop box, no rotation dial), I just realized you may just need to set minimum zoomScale and maximum zoomScale in the beginning, then you can control the final output image size.

That's exactly what I was thinking too!

guoyingtao commented 2 years ago

Just close it for now because of merging of #184 You can reopen it or create a new issue if #184 does not work for you.

vanniktech commented 2 years ago

maximumZoomScale works well:

let minimumSize = imagePicker.imageRequirements.minImageSize()

if image.size.width > image.size.height {
    config.cropViewConfig.maximumZoomScale = max(
        image.size.width / CGFloat(minimumSize.width),
        image.size.height / CGFloat(minimumSize.height)
    )
} else {
    config.cropViewConfig.maximumZoomScale = min(
        image.size.width / CGFloat(minimumSize.width),
        image.size.height / CGFloat(minimumSize.height)
    )
}