Kingfisher image render recycling cells

Viewed 5840

I'm using Kingfisher to download and cache my images. I'm using the custom class below CustomImageView that extends ImageView. I also am using my custom UITableViewCell to call the loadImage method within the didSet property in my CustomCell class file.

The below method loadImage is where all the magic happens.

class CustomImageView : UIImageView {
var lastUrlLoaded: String?

func loadImage(url:String) {
    lastUrlLoaded = url
    ImageCache.default.retrieveImage(forKey: url, options: nil) { (image, cacheType) in
        if let image = image {
            self.image = image
            return
        }
        else {
            guard let url = URL(string: url) else { return }
            self.kf.setImage(with: url, placeholder: nil, options: nil, progressBlock: nil) {
                (image, error, cacheTye, _) in
                if let err = error {
                    self.kf.indicatorType = .none
                    DispatchQueue.main.async {
                      self.image = #imageLiteral(resourceName: "no_image_available") 
                    }
                    print ("Error loading Image:", err)
                    return
                }

                if url.absoluteString != self.lastUrlLoaded { return }
                if let image = image {
                    ImageCache.default.store(image, forKey: url.absoluteString)
                    DispatchQueue.main.async {
                        self.image = image
                    }
                }
            }
        }
    }
}

// Custom TableViewCell -- NewsCell.file
 class NewsCell: UITableViewCell {
var article: Article? {
    didSet{
        guard let image_url = article?.image_url else { return }
        _image.kf.indicatorType = .activity
        _image.loadImage(url: image_url, type: "news")
      }
   }
}

 // METHOD in my NewsController
 override func tableView(_ tableView: UITableView, cellForRowAt indexPath: 
  IndexPath) -> UITableViewCell {        
   let cell = tableView.dequeueReusableCell(withIdentifier: cellId, for: indexPath) as! NewsCell
      cell.article = articles[indexPath.item]
return cell
}

I'm having an issue when scrolling fast through my cells, some images that have loaded are being used for the wrong cells. I've researched and read other post but no answers helped me. I understanding it's dealing with the background task but just not sure how to fix it. If someone can help or point me in the right direction it would be much appreciated.

Solutions I've seen on stackoverflow and tried:

  1. prepareForReuse in custom cell setting the image to nil.
  2. setting the image to nil at cellforrowatindexpath.
3 Answers
override func prepareForReuse() {
    super.prepareForReuse()
    imageView.kf.cancelDownloadTask() // first, cancel currenct download task
    imageView.kf.setImage(with: URL(string: "")) // second, prevent kingfisher from setting previous image
    imageView.image = nil
}

Problem

When you scroll fast, you are reusing cells. The reused cells contain KF images which may have a running download task (initiated from the call to kf.setImage), for the image in the recycled article. If that task is allowed to finish, it will update your new cell with the old image. I will suggest two ways to fix it.

Answer 1 - Kill Recycled Download Tasks

Refer to the Kingfisher cheat sheet, Canceling a downloading task.

The cheat sheet shows how to stop those tasks in didEndDisplaying of your table view. I quote it here:

// In table view delegate
func tableView(_ tableView: UITableView, didEndDisplaying cell: 
UITableViewCell, forRowAt indexPath: IndexPath) {
    //...
    cell.imageView.kf.cancelDownloadTask()
}

You could do it there or in the didSet observer in your cell class. You get the idea.

Answer 2 - Don't kill them, just ignore the result

The other approach would be to allow the download task to finish but only update the cell image if the downloaded URL matches the currently desired URL. Perhaps you were trying to do something like that with your lastUrlLoaded instance variable.

You have this in the completion handler of kf.setImage:

if url.absoluteString != self.lastUrlLoaded { return }

This is kind of what I mean, except you are not setting that variable properly to make it work. Let's imagine it was called "desiredURL" instead, for sake of explanation. Then you would set it in the didSet observer:

_image.desiredURL = image_url

And test it in the completion handler of kf.setImage:

// phew, finished downloading, is this still the image that I want?
if url != self.desiredURL { return }

Notes

If it were me, I would definitely go with the second approach (it has the positive side effect of caching the downloaded image, but also I don't trust canceling tasks).

Also, it seems that you are trying to do the hard way what kf.setImage does for you, i.e. loading the cache etc. I'm sure that you have your reasons for that, and either way you would still need to deal with this issue.

in your model check if the url not exist set it as nil, then in cell check your url if it is exist set it's image, else set a placeholder for that.

if article?.image_url != nil{
  _image.kf.indicatorType = .activity
  _image.loadImage(url: image_url, type: "news")
}else{
  _image = UIImage(named: "placeholder")
}
Related