Android-Universal-Image-Loader icon indicating copy to clipboard operation
Android-Universal-Image-Loader copied to clipboard

Image flickering on notifyDataSetChanged()

Open vovkab opened this issue 11 years ago • 36 comments

Problems cause by this bug:

  1. image flickering
  2. if you load image grid, scroll down, then scroll up, images are reloaded again
  3. first batch of images consume TWICE as much memory.

It looks like there is a bug here:

/* com.nostra13.universalimageloader.core.ImageLoader:193 */
ImageSize targetSize = ImageSizeUtils.defineTargetSizeForView(imageView, configuration.maxImageWidthForMemoryCache, configuration.maxImageHeightForMemoryCache);
String memoryCacheKey = MemoryCacheUtil.generateKey(uri, targetSize);

First time when views is not created, it generates image cache key with screenWidth x screenHeight. And when views already created, it generates a new image cache key viewWidth x viewHeight.

Since keys is different both times, images are considered different so it loads the same image twice into memory.

So if we have a grid with 20 images, we will get:

  1. cache with 40 images
  2. grid flickering or image reloading on scrolling

It can be very easy reproduced with samples that comes with UIL:

  1. call notifyDataSetChanged one more time. or
  2. just scroll down then up.

If someone want a quick fix, replace cache key generator with static uri:

/* com.nostra13.universalimageloader.core.ImageLoader:194 */
String memoryCacheKey = uri; // MemoryCacheUtil.generateKey(uri, targetSize);

vovkab avatar Aug 20 '13 03:08 vovkab

First time when views is not created, it generates image cache key with screenWidth x screenHeight. And when views already created, it generates a new image cache key viewWidth x viewHeight.

You're right. First time view isn't drawn yet and its size is unknown. So UIL takes screen dimensions. And second time views already has their sizes and UIL loads in memory other Bitmaps which is smaller in common. So UIL tries to save memory by that. Not consume. There is no bug with cache keys actually. But I haven't yet find out a solution for this "bug".

nostra13 avatar Nov 20 '13 22:11 nostra13

Thanks for the answer.

The bug is - you cache the same image twice, because of wrong key, and this should't happens. I think it is a pretty serious bug, which exists for more then a year.

As a result we have performance issues:

  • more memory used
  • extra work to remove unused images
  • stuttering in scrolling
  • etc...

Btw, is it possible to configure UIL to show image "as is"?

vovkab avatar Nov 20 '13 23:11 vovkab

This "bug" was introduced on March 1st here - https://github.com/nostra13/Android-Universal-Image-Loader/commit/50751a0 - as improvement. This was pull request and I accepted it. view.getWidth() and view.getHeight() (actual view size) wasn't considered before that. So then, considering this values make this "bug" happens more frequently (actually it could happen before too but very rarely).

To prevent flickering and improve view size defining you should set layout_width and layout_height params if possible. If not then you should set maxWidth and maxHeight, approximately.

Anyway if you want to prevent twice-caching you should remove considering view.getWidth() and view.getHeight() from UIL. Actually you can do it by overriding ImageViewAware (methods getWidth() and getHeight()) since 1.9.0.

nostra13 avatar Nov 21 '13 07:11 nostra13

Hi guys. I solved this bug with a workaround in our app (I'm also thinking about make a pull request with this fix):

we save the url loaded in every single imageview. in this way we avoid to start the whole process if the url to load is the same. this avoided the flickering problem and helped to save few cycle of cpu in the main thread (every saving is good).

carlonzo avatar Nov 28 '13 11:11 carlonzo

I think this issue can be solved next way (1.9.0):

ImageAware imageAware = new ImageViewAware(imageView, false);
imageLoader.displayImage(imageUri, imageAware);

But I didn't tested it yet.

nostra13 avatar Nov 28 '13 21:11 nostra13

Ok, I investigated this question and I know the reason of flickering and some solutions to avoid it.

This problem is actual if your ImageViews haven't concrete defined size (android:layout_width or android:layout_height isn't set with value in dips).

The reason of flickering is that ImageLoader defines actual size of ImageView by default. When you display images first time ImageViews are not drawn yet so they haven't size. So ImageLoader creates bitmaps according device screen size. Then view reusing happens and ImageView was already drawn and it has defined size. So ImageLoader re-decode image to create bitmap according to this size. New bitmap is smaller than previous in common so it saves memory. But creating of new bitmap take some time so you can see flickering.

There are 3 ways to avoid this problem:

  1. Set android:layout_width and android:layout_height parameters for ImageViews in dips ('wrap_content' and 'match_parent' are not acceptable)
  2. Call ImageLoader after ImageView was drawn (in imageView.post(...):
imageView.post(new Runnable() {
        @Override
        public void run() {
            imageLoader.displayImage(imageUri, imageView);
        }
    });
  1. Pass ImageViewAware (instead of ImageView) which doesn't consider actual view size: Intead of:
imageLoader.displayImage(imageUri, imageView);

do following:

ImageAware imageAware = new ImageViewAware(imageView, false)
imageLoader.displayImage(imageUri, imageAware);

This way prevent memory economy because ImageLoader keeps bitmaps bigger than they should be in common.

These are solutions for now but I'll think about improving UIL to handle this case.

nostra13 avatar Dec 18 '13 19:12 nostra13

Yes, it is exactly what happens.

ImageView used in grids or list is almost never have a specific size. So it is more common use then you think. You can also see this bug in UIL samples app.

What if I don't care about sizes, and I want to keep images as-is, why resize is forced and keys contains image size in cache keys? Shouldn't resize be as part of configuration or something?

vovkab avatar Dec 18 '13 23:12 vovkab

So it is more common use then you think.

I don't think it's used rarely. I proposed solutions.

What if I don't care about sizes, and I want to keep images as-is, why resize is forced and keys contains image size in cache keys? Shouldn't resize be as part of configuration or something?

There is option DisplayImageOptions.imageScaleType(...) for that.

nostra13 avatar Dec 18 '13 23:12 nostra13

Thanks for answer.

I don't think it's used rarely. I proposed solutions.

Common :) The grid sample have this bug. List would to, if you need to make item not to be fixed by width or height.

There is option DisplayImageOptions.imageScaleType(...) for that.

Maybe I'm wrong but this will affect bitmap only and not cache keys. Flickering will still be happening and bitmaps duplicated. If I want to cache original bitmaps, why cache keys are different for different views, what the point if bitmap is the same?

Another use case: we have 2 views which is maybe few pixels different, so now you will create 2 bitmaps and consume twice memory then it actually needed.

The main problem that buggy version works by default. And you need to do extra work to fix it. That's why so many apps that uses UIL can be recognized from first check: because of flickering on first scroll.

vovkab avatar Dec 18 '13 23:12 vovkab

I have also facing same issue please help me. Above solution doesn't work for me.

maan-smith avatar Jan 08 '14 15:01 maan-smith

Anyone?

maan-smith avatar Jan 22 '14 16:01 maan-smith

The above solution doesnt work for me. I applied a workaround so solve the problem:

I save the loaded url into a tag in the imageview. before to start the loading with the imageloader, I check if the imageview is already showing the same url.

carlonzo avatar Jan 23 '14 08:01 carlonzo

    DisplayImageOptions options = new DisplayImageOptions.Builder()
            .showImageOnFail(R.drawable.avatar).displayer(new RoundedBitmapDisplayer(100))
            .imageScaleType(ImageScaleType.EXACTLY_STRETCHED)
            .cacheInMemory(true)
            .cacheOnDisc(true).bitmapConfig(Bitmap.Config.RGB_565).build();

I found a not very good way to solve this problem: cacheInMemory(true)

gonjay avatar Jan 29 '14 10:01 gonjay

Guys, problem still exists.

Please open your Sample app -> GridView, wait to load, scroll down, scroll up.

vovkab avatar Apr 08 '14 17:04 vovkab

Can confirm. Problem still there. using a activity with listivew and starting new activity. returning to the activity with the listview -> Images are flickering. Im now using Picasso which is handling this very well :)

chilliger avatar Apr 17 '14 10:04 chilliger

Hello!

I fixed this problem with helper-class (clear animation of ImageView and setting Bitmap from memory cache manually):

    protected static void loadImageByPath(DisplayImageOptions options, String path, ImageViewAware image, ImageLoadingListener listener) {
        ImageLoader.getInstance().cancelDisplayTask(image);
        image.getWrappedView().clearAnimation();
        image.setImageDrawable(null);

        final ImageSize size = ImageSizeUtils.defineTargetSizeForView(image, null);
        final String cacheKey = MemoryCacheUtil.generateKey(path, size);
        final List<Bitmap> cachedBitmaps = MemoryCacheUtil.findCachedBitmapsForImageUri(cacheKey, ImageLoader.getInstance().getMemoryCache());

        if (cachedBitmaps.size() > 0) {
            final Bitmap bitmap = cachedBitmaps.get(0); 

            // Yep, sometimes it is null
            if (bitmap != null) {
                if (listener != null) {
                    listener.onLoadingComplete(path, image.getWrappedView(), bitmap);
                }

                image.setImageBitmap(bitmap);

                return;
            }
        }

        ImageLoader.getInstance().displayImage(path, image, options, listener);
    }

My loading strategy is:

  1. Reset ImageView before loading (resetViewBeforeLoading(true));
  2. If I have Bitmap for item in memory cache - It will be set without fade;
  3. If I don't have Bitmap for item in memory cache - It will be downloaded and set in ImageView with fade. That's why that code is solution for me. My DisplayImageOptions:
final DisplayImageOptions fadeDisplayOptionsForList = new DisplayImageOptions.Builder()
    .resetViewBeforeLoading(true)
    .showImageOnLoading(R.color.color_image_loading)
    .showImageForEmptyUri(R.drawable.image_catalog_error)
    .showImageOnFail(R.drawable.image_catalog_error)
    .cacheInMemory(true)
    .cacheOnDisc(true)
    .postProcessor(null)
    .imageScaleType(ImageScaleType.EXACTLY)
    .bitmapConfig(Bitmap.Config.RGB_565)
    .displayer(new FadeInBitmapDisplayer(200))
    .build();

I use FadeInBitmapDisplayer in my DisplayImageOptions and I think that ImageLoader don't clear Animation after setting Bitmap. When you calls Adapter.notifyDataSetChanged(), AdapterView calls View.requestLayout() that is invalidates his children and restart Animation.

nfirex avatar May 08 '14 07:05 nfirex

Guys !!! For a gallery, with variable height images ....

Really you should GET THE WIDTH AND HEIGHT FROM THE SERVER. Indeed, you have to store them on the server when you save images!

Then use the ratio, and set the absolute size. THEN load the image using the magnificent AUIL.

smhk avatar Jun 03 '14 15:06 smhk

@carlonzo How you solved problem i tried all mentioned still flickering occurs.

This is my display image code.

public void displayImage(Drawable fallback, ImageView imageView,
        String imageURL) {
    ImageLoader imageLoader = ImageLoader.getInstance();
    ImageViewAware aware = new ImageViewAware(imageView, false);

    ImageLoader.getInstance().cancelDisplayTask(aware);
    aware.getWrappedView().clearAnimation();
    aware.setImageDrawable(null);

    DisplayImageOptions options = new DisplayImageOptions.Builder()
            .resetViewBeforeLoading(true).cacheOnDisk(true)
            .postProcessor(null).delayBeforeLoading(0).cacheInMemory(true)
            .showImageForEmptyUri(fallback).showImageOnFail(fallback)
            .showImageOnLoading(fallback)
            .bitmapConfig(Bitmap.Config.RGB_565)
            .displayer(new FadeInBitmapDisplayer(150))
            .imageScaleType(ImageScaleType.EXACTLY).build();
    final ImageSize size = ImageSizeUtils.defineTargetSizeForView(aware,
            new ImageSize(480, 800));
    final String cacheKey = MemoryCacheUtils.generateKey(imageURL, size);
    List<Bitmap> bitMaps = MemoryCacheUtils.findCachedBitmapsForImageUri(
            cacheKey, imageLoader.getMemoryCache());
    int length = bitMaps.size();
    if (bitMaps != null && length > 0) {

        if (bitMaps != null && length > 0 && bitMaps.get(0) != null) {
            if (lister != null) {
                lister.onLoadingComplete(imageURL, aware.getWrappedView(),
                        bitMaps.get(0));
            }
            imageView.setImageBitmap(bitMaps.get(0));
        } else {
            imageLoader.displayImage(imageURL, aware, options);

        }
    } else {
        imageLoader.displayImage(imageURL, aware, options);

    }
}

@nfirex i tried same code as yours not working on scroll images flickr Please help

cycorax12 avatar Oct 17 '14 07:10 cycorax12

It is a fix to stop the flicker ;)

@Override
public View getView(int position, View convertView, ViewGroup parent) {

    ...

    // Set image
    String imgUrl = getItem(position);
    String tag = null;
    Object obj = holder.image.getTag();
    if (obj != null) {
        tag = obj.toString();
    }

   // Check if the same url  
    if (!TextUtils.equals(imgUrl, tag)) {
        mImageLoader.cancelDisplayTask(holder.image);
        mImageLoader.displayImage(imgUrl, holder.image, mDiOptions);
        holder.image.setTag(imgUrl);
    }

    ...

}

gloria0021 avatar Nov 19 '14 10:11 gloria0021

It looks like this problem still exists with latest release of android lollipop. I'm using UIL v1.9.3 and image flickering happens only on Lollipop devices

ghost avatar Nov 22 '14 21:11 ghost

I solved this "bug" (it is definitely not) with a much simpler approach, - no need for custom helpers or solutions, just use View's tag and ViewHolder pattern properly:

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        ViewHolder viewHolder;
        if (convertView == null) {
            convertView = inflater.inflate(R.layout.row_layout, parent, false);
            viewHolder = new ViewHolder();

            viewHolder.title = (TextView) convertView.findViewById(R.id.row_layout_title);
            viewHolder.image = (ImageView) convertView.findViewById(R.id.row_layout_imgView);

            convertView.setTag(viewHolder);
        } else {
            viewHolder = (ViewHolder) convertView.getTag();
        }

        Item item = someAdapterArray.get(position);

        viewHolder.title.setText(item.getTitle());

        //lets prevent "blinking" by "saving URL tag" hack

        if (viewHolder.image.getTag() == null ||
                !viewHolder.image.getTag().equals(item.getImageUrl())) {

            //we only load image if prev. URL and current URL do not match, or tag is null

            ImageLoader.getInstance().displayImage(item.getImageUrl(), viewHolder.image, new SimpleImageLoadingListener());
            viewHolder.image.setTag(item.getImageUrl());
        }

        return convertView;
    }

Yeah, it's a lot of tags, but this will prevent your ImageViews from being reloaded upon any subtle change to a ListView/GridView state.

P.S. Take NavigationDrawer open/close event as an example of such a "subtle change" - this will be followed by notifyDataSetChanged() even if no change to adapter's underlying data occured.

ousenko avatar Dec 18 '14 13:12 ousenko

thank you ...

zhujunyu147 avatar May 18 '15 09:05 zhujunyu147

ImageAware imageAware = new ImageViewAware(imageView, false) imageLoader.displayImage(imageUri, imageAware);

Thanks its work for me

wdayanand avatar Aug 14 '15 06:08 wdayanand

guys. This library has been great. but now there are more evolved libraries. please check Picasso, Glide or Fresco out.

carlonzo avatar Aug 14 '15 08:08 carlonzo

Is there any fix or solution? I have tried the before solutions such like set ImageView a tag. There is a same problem using RecyclerView & notifidatasetchanged(); @nostra13

EDIT: It is my fault, I should call notifieddatainsert instead, then worked fine!

wsk900906 avatar Sep 14 '15 01:09 wsk900906

thank you @ousenko

nujnay avatar Sep 25 '15 06:09 nujnay

thanks @ousenko it works great )

xkokushox avatar Sep 28 '15 17:09 xkokushox

in recyclerView try to use: notifyItemChanged(position); in my code works.

OR:

try to get bitmap by method:

private static Bitmap getCachedBitmap(String url) { List<Bitmap> b = MemoryCacheUtils.findCachedBitmapsForImageUri(url, ImageLoader.getInstance().getMemoryCache()); if (b != null && b.size() > 0) return b.get(0); return null; }

then in code:

Bitmap cachedThumbnail = getCachedBitmap(url); if (cachedThumbnail != null) { image.setImageBitmap(cachedThumbnail); return; } ImageLoader.getInstance().displayImage(...);

ghost avatar Oct 26 '15 13:10 ghost

Hello Friend, Is this issue fixed?

SagarPanwala avatar Feb 27 '16 09:02 SagarPanwala

Hi @SagarPanwala i use v1.9.5 "bug" still exist ,u can use ousenko mentioned workaround

zhEdward avatar Mar 17 '16 06:03 zhEdward