square / picasso

A powerful image downloading and caching library for Android
https://square.github.io/picasso/
Apache License 2.0
18.71k stars 3.97k forks source link

Custom Target sometimes work,sometime doesn't #251

Closed laaptu closed 11 years ago

laaptu commented 11 years ago

This is my scenario

// on FragmentActivity Class
    Picasso picasso = Picasso.with(this);
    String[] imageUrls = {
            "http://digital-photography-school.com/wp-content/uploads/2013/03/Acorn256.png",
            "http://stereo-ssc.nascom.nasa.gov/beacon/latest_256/behind_euvi_195_latest.jpg",
            "http://littleblackapp.blob.core.windows.net/product3071/img10020_0.png" };

    private void initLoadImages() {
        for (int i = 0; i < imageUrls.length; i++) {
            String imageUrl = imageUrls[i];
            picasso.getSnapshot().dump();
            CustomTarget target = new CustomTarget(imageView, progressBar,
                    index);
            picasso.load(imageUrl).into(target);

        }

    }

    // this class is within the Activity class
    public class CustomTarget implements Target {
        public ImageView imageView;
        public ProgressBar progressBar;

        public CustomTarget(ImageView imageView, ProgressBar progressBar,
                int position) {
            this.imageView = imageView;
            this.progressBar = progressBar;
        }

        @Override
        public void onBitmapLoaded(final Bitmap bitmap, LoadedFrom from) {
            Log.i("bitmap loaded", "bitmap loaded");
            imageView.setImageBitmap(bitmap);

        }

        @Override
        public void onBitmapFailed() {

        }

    }

The custom target for first image will be always called but for other images it will be sometimes called and sometimes doesn't. Have I implemented it in wrong way or am I missing something. I have used latest Picasso along with okHttp 1.2.1.

dnkoutso commented 11 years ago

Your CustomTarget instance gets gc'ed. You need to hold a strong reference to it.

JakeWharton commented 11 years ago

You need to hold a strong reference to targets yourself.

            CustomTarget target = new CustomTarget(imageView, progressBar,
                    index);
            picasso.load(imageUrl).into(target);

after this line target has nothing referencing it so it will be garbaged collected.

JakeWharton commented 11 years ago

This is exactly what into(ImageView, Callback) is for. Use the callbacks to show/hide other views like your progress bar.

laaptu commented 11 years ago

@JakeWharton: Here is my scenario

So for that I can't rely on into(ImageView,Callback).For that I assume that I need a custom Callback. Right now as you and @dnkoutso has suggested,I need to reference it somewhere in my class. For that I have done the following and it is working

  ArrayList<CustomTarget> targetList =new ArrayList<CustomTarget>();
  private void initLoadImages() {
        for (int i = 0; i < imageUrls.length; i++) {
            String imageUrl = imageUrls[i];
            picasso.getSnapshot().dump();
            CustomTarget target = new CustomTarget(imageView, progressBar,
                    index);
            targetList.add(target);
            picasso.load(imageUrl).into(target);

        }

    }

I don't know it is a efficient way to do or not. But right now it is working. I am loving Picasso ,AWESOME TOOL ;)

dnkoutso commented 11 years ago

Remember to remove the instance from the ArrayList once the download is complete (either successful or failed).

You should initialize your ArrayList with a capacity since it appears you already know the length imageUrls.length.

You should also call cancelRequest() in onDestroy() or in onPause() (with isFinishing() check) or in onStop() for each element in the array to ensure Picasso does not invoke them after the user has left the screen.

You wouldn't Picasso to invoke your callback after the user has left the screen...

laaptu commented 11 years ago

@dnkoutso Thanks a million for showing the efficient way. As per your advice I have implemented the following

public class ImageLoadWithCallback extends FragmentActivity {

    String[] imageUrls = {
            "http://digital-photography-school.com/wp-content/uploads/2013/03/Acorn256.png",
            "http://stereo-ssc.nascom.nasa.gov/beacon/latest_256/behind_euvi_195_latest.jpg",
            "http://littleblackapp.blob.core.windows.net/product3071/img10020_0.png" };

    private Picasso picasso;
    private ArrayList<CustomTarget> targetList;

    @Override
    protected void onCreate(Bundle arg0) {
        super.onCreate(arg0);
        setContentView(R.layout.activity_main);
        picasso = Picasso.with(this);
        initLoadImages();
    }

    private void initLoadImages() {
        // for holding strong reference to Target so that it won't be GC'ed
        targetList = new ArrayList<ImageLoadWithCallback.CustomTarget>(3);
        for (int i = 0; i < imageUrls.length; i++) {
            String imageUrl = imageUrls[i];
            picasso.getSnapshot().dump();
            CustomTarget target = new CustomTarget(imageView, progressBar, i);
            targetList.add(i, target);
            picasso.load(imageUrl).into(target);
        }

    }

    @Override
    protected void onDestroy() {
        super.onDestroy();
        if (isFinishing()) {
            for (CustomTarget target : targetList)
                picasso.cancelRequest(target);

            targetList.clear();
            targetList = null;
        }

    }

    public class CustomTarget implements Target {
        public ImageView imageView;
        public ProgressBar progressBar;
        private int index = -1;

        public CustomTarget(ImageView imageView, ProgressBar progressBar,
                int position) {
            this.imageView = imageView;
            this.progressBar = progressBar;
                      this.index =position;
        }

        @Override
        public void onBitmapLoaded(final Bitmap bitmap, LoadedFrom from) {
            imageView.setImageBitmap(bitmap);
            releaseTargetReference();
        }

        @Override
        public void onBitmapFailed() {
            releaseTargetReference();
        }

        private void releaseTargetReference() {
            if (index != -1)
                targetList.remove(index);
        }

    }

}
hakimrie commented 10 years ago

@laaptu you forgot to update variable index value in your CustomTarget class (constructor). just saying :)

laaptu commented 10 years ago

@hakimrie : Thanks for mentioning. I have updated the variable index now