Yalantis / uCrop

Image Cropping Library for Android
https://yalantis.com/blog/introducing-ucrop-our-own-image-cropping-library-for-android/
11.89k stars 2.16k forks source link

"Freestyle" cropping does not maintain aspect ratio #135

Closed yukuku closed 7 years ago

yukuku commented 8 years ago

I'm using version 1.5.0 (because I don't want the native libraries)

When I enable the freestyle cropping, it seems to ignore the aspect ratio I set (1:1).

shliama commented 8 years ago

That is what freestyle cropping should do - it lets user to set any aspect ratio :) Therefore it's useless to combine this feature with pre-set aspect ratio.

yukuku commented 8 years ago

Not really — even if the freestyle cropping is enabled (which means user can drag the crop rectangle instead of moving the image), aspect ratio can still be maintained. See another library's demo gif: https://raw.githubusercontent.com/ArthurHub/Android-Image-Cropper/master/art/demo.gif

shliama commented 8 years ago

Oh, that's great 👍 Please fork and pull-request, looking forward to add this feature :octocat:

yukuku commented 8 years ago

Is it ok if I fork and make changes on 1.5.0? I do really like 1.5.0 more than 2.x because:

  1. No native lib
  2. content:// uris are not resolved to file path. See https://commonsware.com/blog/2016/03/14/psa-file-scheme-ban-n-developer-preview.html for why it is important not to get file path from content:// uris.

On Tue, Jun 28, 2016 at 4:28 PM, Oleksii notifications@github.com wrote:

Oh, that's great 👍 Please fork and pull-request, looking forward to add this feature [image: :octocat:]

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Yalantis/uCrop/issues/135#issuecomment-228985198, or mute the thread https://github.com/notifications/unsubscribe/ABV7Xa0-tTOMbhZMRytFYPg9eH6EVDciks5qQNssgaJpZM4I_0U7 .

shliama commented 8 years ago

I don't see anything wrong with the native libraries... however if you check the code, you may notice that I try to resolve a file path from content:// uris (and it still works for a large amount of cases), but if it fails - I use content:// uri directly via InputStream. I got the image file one way or another.

yukuku commented 8 years ago

Native lib is subjective. I just don't want to make the APK bigger.

However, reading the file from content:// uri will fail if the app does not have the permission to read external storage (or if the user denies the permission on M). It crashes when I integrate the library without giving that permission.

shliama commented 8 years ago

The sample handles permission and works well on any platform. I don't think uCrop should handle permissions for you.

yukuku commented 8 years ago

The sample requires read external storage permission even before the crop operation starts. It is not actually needed to select a photo using another app.

suau commented 8 years ago

@yukuku Agreed with the permissions. I started using uCrop 2.1.1 and I believe it's the best cropping library out there, however that permission thing struck me, as the permission isn't necessary when directly loading the passed in Uri. With the native libraries concern however, I believe (didn't verify) uCrop is one of the few libraries which can crop large images without downsampling due to the native library, which doesn't need to hold the complete image in memory to crop (streaming ? again not verified @shliama are my assumptions correct ?).

TeeRawk commented 7 years ago

outdated