Also pass "boolean fromCache" to the Target interface

[csobrinho]

Hi @sjudd, any objection in doing a PR to slightly change the Target interface to add the fromCache boolean?

Before I was relying in the ImageReadyCallback to set a flag in the target like:

public static class CustomImageReadyCallback<T> implements ImageReadyCallback<T>
    {
        @Override
        public void onImageReady(T model, Target target, boolean fromCache)
        {
            ((CacheableAwareImageViewTarget) target).setFromCache(fromCache);
        }
    }

The proposed approach would be:

public interface Target {
    /**
     * The method that will be called when the image load has finished
     * @param bitmap The loaded image
     * @param fromCache If the bitmap was loaded from the cache
     */
    public void onImageReady(Bitmap bitmap, boolean fromCache);
    (...)
}

But now the callback is called after the target.onImageReady. Since I have custom animations using a TransitionDrawable with a cross fade the normal setAnimation scheme doesn't work anymore.

So my big question is, any big reason why the fromCache argument could not fit in terms of the structure you initially idealized?

As an example, my current ImageViewTarget is something like this:

public static class CustomImageViewTarget extends ImageViewTarget implements CacheableAwareImageViewTarget
    {
        private final ImageView mImageView;
        private final LoaderOptions mLoaderOptions;
        private boolean mFromCache;

        public CustomImageViewTarget(ImageView imageView, LoaderOptions loaderOptions)
        {
            super(imageView);

            mImageView = imageView;
            mLoaderOptions = loaderOptions;
        }

        @SuppressWarnings("deprecation")
        private void onImageReady(final Bitmap bitmap, final boolean isImmediate, boolean isInLayoutPass)
        {
            if (isImmediate && isInLayoutPass)
            {
                mImageView.post(new Runnable()
                {
                    @Override
                    public void run()
                    {
                        onImageReady(bitmap, isImmediate, false);
                    }
                });
                return;
            }

            if (mLoaderOptions.removePlaceholder && bitmap != null)
            {
                // Cannot use the #setPlaceholder method because that method only sets the setBackgroundDrawable if the
                // drawable is a NinePatchDrawable
                mImageView.setBackgroundDrawable(null);
            }

            if (isImmediate || !mLoaderOptions.fadeIn)
            {
                mImageView.setImageBitmap(bitmap);
                return;
            }

            final Drawable currentDrawable = mImageView.getDrawable();
            final Drawable initialDrawable = currentDrawable != null ? currentDrawable : sTransparentDrawable;

            // TODO: in some cases, the bitmap is streched! Drawing in a canvas will solve this?
            final BitmapDrawable bitmapDrawable = new BitmapDrawable(mImageView.getResources(), bitmap);

            // Use TransitionDrawable to fade in
            final TransitionDrawable td = new TransitionDrawable(new Drawable[] { initialDrawable, bitmapDrawable });
            td.setCrossFadeEnabled(true);

            mImageView.setImageDrawable(td);

            td.startTransition(UIUtils.ANIMATION_FADE_IN_TIME);
        }
        @Override
        public void onImageReady(Bitmap bitmap)
        {
            final boolean isInLayoutPass = mImageView instanceof LayoutAwareImageView ? ((LayoutAwareImageView) mImageView).isInLayout() : false;

            onImageReady(bitmap, mFromCache, isInLayoutPass);
        }

        @SuppressWarnings("deprecation")
        @Override
        public void setPlaceholder(Drawable placeholder)
        {
            if (placeholder instanceof NinePatchDrawable)
            {
                mImageView.setImageDrawable(null);
                mImageView.setBackgroundDrawable(placeholder);
            }
            else
                super.setPlaceholder(placeholder);
        }

        @Override
        public void setFromCache(boolean fromCache)
        {
            mFromCache = fromCache;
        }

Thanks!

[sjudd]

This sounds reasonable to me.

It seems like there is a lot of overlap between what people want to do in onImageReady in the target and what they want to do in onImageReady in the callback. I don't have any great ideas for resolving it thought.

[csobrinho]

What was your initial idea about the ResponseListener, Target and the ready callback?

[sjudd]

Ideally Target would be a very simple wrapper with little to no logic that basically just decouples the load from the thing receiving the load and the response listener would be where any more complex logic would go.

I think the problem is that it will always be difficult to get at the underlying object in the response listener which makes it hard to do anything more complex, like the crossfade animation. In turn that probably makes extending Target as you've done the best way to go.

[sjudd]

See my last comment here: #48