Loading tableview images from URL in background causes duplicates

Question

I have a tableview that is populated with the results of a search term. Many of the results have images that need to load from URLs, but not all. I originally had it grabbing the image from the URL in the cellForRowAtIndexPath method in the main thread, which worked perfectly, but it made the scrolling of the tableview choppy as it "stuck" on each image momentarily.

So, I decided to try loading the images in a background thread. Here is my cellForRowAtIndexPath method. The URLs are stored in the resultsArray, with the indices corresponding to the row of the cell.

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    [sBar resignFirstResponder];
    if (indexPath.row != ([resultsArray count])) 
    {
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"ResultCell"];
    UIImageView * bookImage = (UIImageView *)[cell viewWithTag:102];
        //set blank immediately so repeats are not shown
        bookImage.image = NULL;

        //get a dispatch queue
        dispatch_queue_t concurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
        //this will start the image loading in bg
        dispatch_async(concurrentQueue, ^{        
            NSData *image = [[NSData alloc] initWithContentsOfURL:[NSURL URLWithString:[[resultsArray objectAtIndex:indexPath.row] thumbnailURL]]];

            //this will set the image when loading is finished
            dispatch_async(dispatch_get_main_queue(), ^{
                bookImage.image = [UIImage imageWithData:image];

                if(bookImage.image == nil)
                            {
                                bookImage.image = [UIImage imageNamed:@"no_image.jpg"];
                            }
            });
        }); 

        return cell;
    }
    // This is for the last cell, which loads 10 additional items when touched
    // Not relevant for this question, I think, but I'll leave it here anyway
    else{
        UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"More"];
        UILabel *detailLabel = (UILabel *)[cell viewWithTag:101];
        detailLabel.text = [NSString stringWithFormat:@"Showing %d of %d results", [resultsArray count], [total intValue]];
        if ([UIApplication sharedApplication].networkActivityIndicatorVisible == NO) {
            cell.userInteractionEnabled = YES;
        }
        return cell;

    }
}

When the tableview scrolls slowly, the images all load in their appropriate cells, which I can confirm because the images match the titles of the cells, but I left out the setting of everything but the image to shorten it for this question. However when I scroll faster, especially when I simulate a slow internet connection, images start loading in the wrong cells. I think it has something to do with reusing the cells, because, if I a scrolling toward the top quickly, the image of the cell just leaving the view often ends up in the cell just entering. I thought the bookImage.image = NULL; line would ensure that didn't happen, but I guess not. I guess I don't understand background threading very well, when the image is finally loaded from the URL, has it lost track of which cell it was intended for? Do you know what may be happening? Thanks for any feedback!

score 1 · Answer 1 · answered Mar 28 '12 at 16:28

1

At a guess:

Cell A is loaded and begins asynchronous request A.
Table is scrolled and cell A scrolls offscreen before the request completes.
Cell A is recycled and dequeued as cell B.
Cell B begins asynchronous request B.
Asynchronous request A completes, updating cell A (now cell B)'s image view.

Fixing this requires keeping a handle to the asynchronous request and cancelling it when the cell is dequeued.

answered Mar 28 '12 at 16:28

jnic

8,695
3
33
47

1

Ok that makes sense, thanks for the feedback! But how do I keep track of which cell is being dequeued and which request corresponds to it? – Ryan Mar 29 '12 at 20:10

score 1 · Answer 2 · answered Mar 29 '12 at 18:15

My approach (relies on the AFNetworking library). Include classes from the following zip-file into your project:

http://dl.dropbox.com/u/6487838/imageloading.zip

Create your own custom cells. Code could look somewhat like the following (check the -setComment: and -willMoveToSuperview: methods):

#import "CommentCell.h"
#import "CommentCellView.h"
#import "MBImageLoader.h"


@interface CommentCell ()
@property (nonatomic, strong) UIImageView     *imageView;
@property (nonatomic, strong) CommentCellView *cellView;
@property (nonatomic, assign) CellStyle       style;
@end


@implementation CommentCell

@synthesize cellView, style = _style, imageView, comment = _comment, showCounter;

- (id)initWithStyle:(CellStyle)style reuseIdentifier:(NSString *)reuseIdentifier;
{
    self = [super initWithStyle:UITableViewCellStyleDefault reuseIdentifier:reuseIdentifier];
    if (self)
    {
        self.selectionStyle = UITableViewCellSelectionStyleNone;
        self.opaque = YES;
        self.style = style;
        self.showCounter = YES;


        CGFloat width = 53.0f;
        CGFloat x = (style == CellStyleRight) ? 320.0f - 10.0f - width : 10.0f;
        CGRect frame = CGRectMake(x, 10.0f, width, 53.0f);
        self.imageView = [[UIImageView alloc] initWithFrame:frame];
        imageView.image = [UIImage imageNamed:@"User.png"];
        [self.contentView addSubview:imageView];


        self.cellView = [[CommentCellView alloc] initWithFrame:self.frame cell:self];
        cellView.backgroundColor = [UIColor clearColor];
        [self.contentView addSubview:cellView];
    }
    return self;
}

- (void)dealloc
{
    self.comment = nil;
    self.cellView = nil;
    self.imageView = nil;
}

- (void)willMoveToSuperview:(UIView *)newSuperview
{
    [super willMoveToSuperview:newSuperview];

    if (!newSuperview)
    {
        [[MBImageLoader sharedLoader] cancelLoadImageAtURL:_comment.iconURL forTarget:self];
    }
}

- (void)setSelected:(BOOL)selected animated:(BOOL)animated
{
    [super setSelected:selected animated:animated];

    // Configure the view for the selected state
}

- (void)setFrame:(CGRect)newFrame 
{
    [super setFrame:newFrame];

    CGRect bounds = self.bounds;
    bounds.size.height -= 1; 
    cellView.frame = bounds;
}

- (void)setNeedsDisplay 
{
    [super setNeedsDisplay];

    [cellView setNeedsDisplay];
}

- (void)setComment:(MBComment *)comment
{
    if (_comment == comment) 
        return;

    _comment = comment;

    if (comment.icon == nil)
    {
        imageView.image = nil;

        [[MBImageLoader sharedLoader] loadImageForTarget:self withURL:comment.iconURL success:^ (UIImage *image) 
         {
             comment.icon = image;

             imageView.alpha = 0.0f;
             imageView.image = image;

             [UIView animateWithDuration:0.5f delay:0.0f options:UIViewAnimationOptionAllowUserInteraction animations:^ {
                 imageView.alpha = 1.0f;                 
             } completion:^ (BOOL finished) {}];             

         } failure:^ (NSError *error) 
         {
             DLog(@"%@", error);
         }];
    }
    else 
    {
        imageView.image = comment.icon;
    }

    [self setNeedsDisplay];
}

@end

score 0 · Answer 3 · answered Mar 29 '12 at 18:06

How I did this.

cellForRowAtIndexPath: method:

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath     *)indexPath {

static NSString *cellIdentifier = @"CellIdentificator";

UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:cellIdentifier];
if (cell == nil) {
    cell = [[[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:cellIdentifier] autorelease];
}
[cell.imageView setImage:nil];

NSData *const cachedImageData = [self.cache objectAtIndex:indexPath.row];
if ([cachedImageData isKindOfClass:[NSData class]]) {
    [cell.imageView setImage:[UIImage imageWithData:cachedImageData]];
} else {
    [self downloadAndCacheImageForIndexPath:indexPath];
}

return cell;

}

downloadAndCacheImageForIndexPath: method:

- (void)downloadAndCacheImageForIndexPath:(NSIndexPath *)indexPath {

dispatch_queue_t concurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
dispatch_async(concurrentQueue, ^{
    NSString *const imageStringURL = [NSString stringWithFormat:@"%@%.2d.png", IMG_URL, indexPath.row];

    NSData *image = [[NSData alloc] initWithContentsOfURL:[NSURL URLWithString:imageStringURL]];
    [self.cache replaceObjectAtIndex:indexPath.row withObject:image];

    dispatch_async(dispatch_get_main_queue(), ^{
        [self.tableView reloadRowsAtIndexPaths:[NSArray arrayWithObject:[NSIndexPath indexPathForRow:indexPath.row inSection:0]]withRowAnimation:UITableViewRowAnimationNone];
    });
    [image release];
});

}

self.cache is NSMutableArray which I use to store NSData for loaded images. self.cache preloaded with [NSNull null]s for amount of images

BR.
Eugene.

Loading tableview images from URL in background causes duplicates

3 Answers3