keepingCropAspectRatio doesn't keep aspect ratio if crop area is shrunk too much

[mariusc]

I took the demo code and forced it to keep the aspect ratio and gave it a hardcoded CGRect for the controller.imageCropRect. You can see the source in https://github.com/mariusc/PEPhotoCropEditor/tree/d892059bb09f870152856d727cfc170b8fe2a913/DemoApp

Generally it works ok, but if I shrink the cropping area too much, the cropRect stops shrinking in height (if height < width) and only changes its width, so the aspect ratio is lost. It seems that it either tends to go towards a square cropRect, or a cropRect with the reversed aspect ratio (interchanges width with height). I tried to debug it for some time, but gave up in the end.

Added a screen capture to my fork showing the bug. Not sure if the movie works in browser, you might have to download the archive or clone the repo. https://github.com/mariusc/PEPhotoCropEditor/blob/a15321913e86d798876217c82cdc775f807c10d1/cropBug.mov

Later edit: I think one of the problem is in PECropRectView.m, cropRectMakeWithResizeControlView:, where it checks if the current rect is too small (line 294 - ..). But there may be other issues, because if I comment out from line 294 to the end of the function - without the return statement-, it doesn't change the aspect ratio. But I can still notice a change in the orientation of the cropping rect (width/height = aspectRatio or width/height = 1/aspectRatio). If I manage to track down the bug, I'll come back with a pull request :)

Later later edit: The other problem was in function constrainedRectWithRectBasisOfWidth: aspectRatio: from PECropRectView, line 327. In my opinion, the condition in that 'if' should check the relation between the width and height of the original image, not the scaled cropping rectangle. Usually, if I want to crop something and preserve its aspect ratio, I also want to preserve its orientation. I'll come back these days with a pull request, and then it's your choice if you think it's ok or not. Anyway, good job with this pod. Despite the fact that I stayed a couple of hours to figure this out, it still saved me a LOT of time :). Thanks

[mariusc]

Issued the pull request https://github.com/kishikawakatsumi/PEPhotoCropEditor/pull/52

[stoneark]

I came across the same issue, mariusc's pull request https://github.com/kishikawakatsumi/PEPhotoCropEditor/pull/52 have solved this issue perfectly.

[padamthapa]

@mariusc @stoneark In "freeform" mode, frames of crop rect gets restricted. Also sometimes during aspect ratio mode; at some size while resizing, portrait rect suddenly jumps and becomes landscape and vice versa. Have you come across that ?

Thanks for the pull.

[padamthapa]

@mariusc After looking into changes in this pull on PECropRectView, I did make change on constrainedRectWithRectBasisOfHeight method too. I Replaced if (width < height) with if (self.initialRect.size.width < self.initialRect.size.height) {

Now this issue for portrait aspect mode seems fixed. Only issue is crash happens only on 64 bit device when it is resized to minimum size it could get (Portrait aspect ratio mode only). Thanks.