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

There is a small improvement that should prevent showing a wrong bitmap in the recycled view #85

Merged
merged 1 commit into from
Oct 28, 2012

Conversation

Vitaly-Olshevsky
Copy link
Contributor

Hi,

Sometimes when I scroll a ListView, wrong images appear in recycled views. These images appear just for a moment until actual images are loaded. It happens quite rarely, but it is still very irritating.

Here is the simple scheme of what is actually happening in the code:

  1. Some image is completely loaded in a background thread, it is checked for actuality and a new DisplayBitmapTask is posted to the UI.
  2. Some views are recycled. Among them is a view that is still linked with the image from item 1. At this point the image becomes not actual.
  3. The DisplayBitmapTask from item 1 is finally executed and unfortunately it shows the wrong image.

So I added a simple actuality check into the DisplayBitmapTask. Therefore there is no single chance for an inactual image to be shown. Please review this fix and apply it if you want.

@nostra13
Copy link
Owner

But there is still exists possibility that task becomes not actual after

if (memoryCacheKey.equals(currentCacheKey)) {

but before bitmap was set into ImageView in BitmapDisplayer :) Especially if BitmapDisplayer.display(...) do some long work before imageView.setImage... (like RoundedBitmapDisplayer).
But I agree it can reduce unexpected images displaying count.

Did you mention in your app that this improvement helps?

nostra13 added a commit that referenced this pull request Oct 28, 2012
There is a small improvement that should prevent showing a wrong bitmap in the recycled view
@nostra13 nostra13 merged commit c0dfb72 into nostra13:master Oct 28, 2012
@Vitaly-Olshevsky
Copy link
Contributor Author

But there is still exists possibility that task becomes not actual after

if (memoryCacheKey.equals(currentCacheKey)) {

but before bitmap was set into ImageView in BitmapDisplayer :) Especially if BitmapDisplayer.display(...) do some long work before imageView.setImage... (like RoundedBitmapDisplayer).

No, there is no such a possibility unless a client starts a new background thread in BitmapDisplayer.display(...). But it is a client's synchronization problem, so it doesn't matter here. At least, a client may be sure that parameters of the display(...) method are still actual at this point.

I'm telling that there is no such a possibility, because BitmapDisplayer.display(...) runs in the UI thread as well as an adapter recycles its views in this thread. There is no chance for actions to overlap.

Did you mention in your app that this improvement helps?

Yeah, I've checked that this improvement helps. I've reproduced the issue on the example app as well, but only with logging. It is not that noticeable in the example.

@nostra13
Copy link
Owner

I'm telling that there is no such a possibility, because BitmapDisplayer.display(...) runs in the UI thread as well as an adapter recycles its views in this thread. There is no chance for actions to overlap.

You're right :) Forgot that BitmapDisplayer runs on UI thread.
Thank you, Vitaly, for this improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants