ArthurHub / Android-Image-Cropper

Image Cropping Library for Android, optimized for Camera / Gallery.
Apache License 2.0
6.4k stars 1.38k forks source link

Doesn't work if i choose second image after the first image has been cropped #183

Open mehrosh opened 8 years ago

mehrosh commented 8 years ago

I am using the activity flow of cropping an image. I start the cropper activity by using

CropImage.activity(uri)
                            .setGuidelines(CropImageView.Guidelines.ON)
                            .setOutputUri(outputUri)
                            .start(this);

and gets the cropped image which i then passes on to another which renders it perfectly. but when i get back to the previous activity which is responsible for using the image, i again starts the cropper activity using the above code. The cropper activity launches but it shows a blank screen with a loader in the center which keeps on loading for like infinity. capture _2016-10-26-17-56-58

ArthurHub commented 8 years ago

Is there an error in logcat?

mehrosh commented 8 years ago

nothing in the logcat. But its not producing since this morning. will post if it happens again, now the loader gets displayed but eventually after some seconds image gets loaded. I'll test on further devices to reproduce this issue

AvatarQing commented 7 years ago

This is because BitmapLoadingWorkerTask is extend AsyncTask, and every instance of AsyncTask is using the same serial executor by default. When our app use AsyncTask first in other place, then open crop page, BitmapLoadingWorkerTask has to wait to execute until other instance of AsyncTask's execution finished. I think the author should execute BitmapLoadingWorkerTask on a custom non-serial executor.

ArthurHub commented 7 years ago

That make sense, awesome you found this, thank you.

According to Google: "AsyncTask should ideally be used for short operations (a few seconds at the most.)...". This perfectly fit the usage in the cropper, a short task to execute in the background while the UI is waiting for that task.

On the other hand, it sounds like your app is executing long task that is unrelated to the currently shown UI, that work should not be executed on async task.

Also, it's a small library, I don't want to introduce more complexity to async code execution.

AvatarQing commented 7 years ago

In my case, I didn't use AsyncTask by myself, I used facebook ad sdk to show an ad in previous page of cropper, the loading of the ad was slow, and it blocked the cropper’s loading, so I guess it used AsyncTask too, I can't control it.

I suggest you can call AsyncTask's executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR) which can simply execute tasks in parallel.

ArthurHub commented 7 years ago

You have a point there... will check it out, thx!

mehrosh commented 7 years ago

thanks for the help guys. You were right my app downloads data in the background asynchronously and durring that downloading the image cropper doesn't work. So implemented the parallel executor in the setImageUriAsync method. changed the following method in CropImageView.java

public void setImageUriAsync(Uri uri) {
        if (uri != null) {
            BitmapLoadingWorkerTask currentTask = mBitmapLoadingWorkerTask != null ? mBitmapLoadingWorkerTask.get() : null;
            if (currentTask != null) {
                // cancel previous loading (no check if the same URI because camera URI can be the same for different images)
                currentTask.cancel(true);
            }

            // either no existing task is working or we canceled it, need to load new URI
            clearImageInt();

            int NUMBER_OF_CORES = Runtime.getRuntime().availableProcessors();
            ThreadPoolExecutor executor = new ThreadPoolExecutor(
                    NUMBER_OF_CORES * 2,
                    NUMBER_OF_CORES * 2,
                    60L,
                    TimeUnit.SECONDS,
                    new LinkedBlockingQueue<Runnable>()
            );

            mCropOverlayView.setInitialCropWindowRect(null);
            mBitmapLoadingWorkerTask = new WeakReference<>(new BitmapLoadingWorkerTask(this, uri));
            mBitmapLoadingWorkerTask.get().executeOnExecutor(executor);
            setProgressBarVisibility();
        }
    }

Works like a charm, image gets loaded instanteously but when the crop button is pressed it takes some time to finally give the result back to the calling activity