Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

empty bitmap file creation #173

Closed
truthofmatthew opened this issue Aug 14, 2016 · 6 comments
Closed

empty bitmap file creation #173

truthofmatthew opened this issue Aug 14, 2016 · 6 comments
Labels

Comments

@truthofmatthew
Copy link

truthofmatthew commented Aug 14, 2016

bug 😭
@Yalantis
This bug is shown when:
1- we chose an image from google drive app.
2- we chose an image from external sd card.

Waz goin on:
1- we open the app
2- we chose an image from gdrive, here two things will happen
1- if we crop the image, its okay and it will work because ucrop copy the bitmap into where we set
2- if we do not crop it, it will not work cuz the app didnt copy the image into the path we set as default, i was looking at the source code, if we dont crop the app will say: cropped needed? false. then its just create an empty bitmap file and will load that empty file.

Solution i Think:
this is what i think is right, for the files on internal or some external cards the behaviour is right and works well, but on some devices and gdrive files will not work, but this idea i have maybe can be helpful.

1- if we do not crop, u crop should copy the selected image into the path we set in any situation.
2- or put an option to ask the user, do you want to copy or use the original?

and here is the stacktrace:
onFailure: setImageUri java.lang.IllegalArgumentException: Bounds for bitmap could not be retrieved from the Uri: [file:///data/user/0/com.yalantis.ucrop.sample/cache/SampleCropImage.jpg] at com.yalantis.ucrop.task.BitmapLoadTask.doInBackground(BitmapLoadTask.java:113) at com.yalantis.ucrop.task.BitmapLoadTask.doInBackground(BitmapLoadTask.java:41) at android.os.AsyncTask$2.call(AsyncTask.java:295) at java.util.concurrent.FutureTask.run(FutureTask.java:237) at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:234) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588) at java.lang.Thread.run(Thread.java:818)

and i think the bug is in the copying stage:
the class BitmapCropTask.java
line 148: FileUtils.copyFile(mImageInputPath, mImageOutputPath);
because everything works fine if we comment: boolean shouldCrop = shouldCrop(width, height);
its not the solution but it can make it work because of the cropping solutions and...

the FileUtils.java: copyFile()...
i didnt have time to look into the codes and find the main reason and best solution, but for now its good to know this kinda things is there.

thanks a lot, ill be happy to know more about this.

@PGMacDesign
Copy link
Contributor

PGMacDesign commented Aug 16, 2016

I believe this is related to the copy utils bug. If so, I wrote a fix for it 2 weeks ago and am still waiting for the devs to merge the branch/ code....

The main issue (I believe) is just as you mentioned, in the copying stage, wherein it is breaking the old photo, which prevents it from being used in secondary tests. The photo gets 'erased' in the sense that its source data is deleted, but it is still cached as a thumbnail and therefore shows when you try to select it.

Pull request is here if you want to take a look at the simple fix: #162

@shliama
Copy link
Contributor

shliama commented Sep 8, 2016

@mtkarimi @PGMacDesign yeah guys, so sorry it took me so long to come and fix it :(
But yes, @PGMacDesign is absolutely right and his fix already merged. I'll prepare new library build soon and post here when it's ready!

@shliama shliama added the bug label Sep 8, 2016
@PGMacDesign
Copy link
Contributor

Thanks for the merge @shliama ! Appreciate it and thoroughly love this library.

PGMac

@shliama
Copy link
Contributor

shliama commented Sep 8, 2016

Please check the latest uCrop version :octocat:
Now you can choose between:

Lightweight general solution
compile 'com.yalantis:ucrop:2.2.0' 
Get power of the native code to preserve image quality (+ about 1.5 MB to an apk size)
compile 'com.yalantis:ucrop:2.2.0-native'

@truthofmatthew
Copy link
Author

Aaaww Yeaah 👍
Thanks a lot @shliama
its even much more faster as im using the native one, importing from everywhere is working awesome.
even the result W/H is very useful

@TeeRawk
Copy link
Contributor

TeeRawk commented Mar 13, 2017

resolved

@TeeRawk TeeRawk closed this as completed Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants