I am currently using Kingfisher library to load thumbnail images for collectionview cells and if it is already recorded in the cache, it would load from the cache. Seldomly some images are loaded for wrong cell which leads to duplicate images in the collectionview. When I try to scroll down and then dismiss/dequeue again, images are then correctly placed. This issue is not present if I do not use functionality of retrieving image from the cache also.
func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "SimpleThumbnailView", for: indexPath) as! ThumbnailsCollectionViewCell
cell.thumbnailImage.kf.cancelDownloadTask()
cell.prepareForReuse()
if indexPath.row < thumbnails.count {
let urlString = thumbnails[indexPath.row].thumbnail ?? ""
ImageCache.default.retrieveImageInDiskCache(forKey: urlString) { result in
switch result {
case .success(let image):
if image == nil {
if let url = URL(string: urlString) {
DispatchQueue.main.async {
let resource = ImageResource(downloadURL: url, cacheKey: urlString)
cell.thumbnailImage.kf.setImage(with: resource, placeholder: UIImage(named: "DefaultImage"), options: [])
}
}
} else {
DispatchQueue.main.async {
cell.thumbnailImage.image = image
}
}
case .failure(let error):
print(error)
}
}
cell.titleLabel.text = thumbnails[indexPath.row].title
cell.authorLabel.text = StringBuilder.authorLikes(author: thumbnails[indexPath.row].writer, likes: thumbnails[indexPath.row].likeCount)
cell.articleId = thumbnails[indexPath.row].articleId
cell.authorId = thumbnails[indexPath.row].writer
if cell.articleId != -1 {
cell.removePlaceHolder()
}
} else {
thumbnailCollectionView.reloadData()
}
return cell
}
and this is my prepareForReuse()
override func prepareForReuse() {
super.prepareForReuse()
thumbnailImage.kf.cancelDownloadTask()
thumbnailImage.image = UIImage(named: "DefaultImage")
}
I also tried using cancelDownloadTask() function about anywhere I could think of but this didn't worked either. Could someone have an idea how to solve this problem?
Understanding the reuse system used in UICollectionView/UITableView is important, and that's why you dequeue cells.
When a cell disappear, it goes on the "available cells pool" in memory.
When you need to show a cell, it's check if there is an available cell in the previously mentioned pool, if not it creates one.
That's for optimisation. Why? Let's imagine that you have 10k cells to show, but only can show 5 at the same time on the screen, let's add 1 more cell (when scrolling, you see the bottom only of the first one, and the top of the new one). Do you think it'd be efficient to create 10k cells? Not, let's just create 6, and reuse them. That's the whole optimization behind reusing cells (in Android SDK, it's "Recycling", for Recycle View, the concept being the same).
But when the cells goes out of screen, it's put in the pool as "such", with all the customization it had before (like label values, colors, subview, etc.). There it will call prepareForReuse() to put it in a "blank" state (do not call it yourself). It's not needed to override prepareForReuse() if you have for instance only one UILabel and in collectionView(_:cellForItemAt:) you change its value each time (it'd be label.text = nil in prepareForReuse() directly changed in collectionView(_:cellForItemAt:) into label.text = "myValue", which is extra work not necessary.
Once you understood this, the current issue with your code is this one:
func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "SimpleThumbnailView", for: indexPath) as! ThumbnailsCollectionViewCell
//First Async call (I guess it's async)
ImageCache.default.retrieveImageInDiskCache(forKey: urlString) { result in
//Second async call
DispatchQueue.main.async {
cell.thumbnailImage.image = image
}
}
}
You use two async call (but one is enough to create the issue), so when the closure is called and when you want to set the value, the cell might already have been reused for another indexPath. Hence the issue you get having images at wrong index paths cells when scrolling.
Now, you can say that setImage(with url:) from KingFisher/Alamofire+Image/SDWebImage and other libs are using a "hidden" closure too and they don't have the issue. It's because they use associated_object, which let them to add an extra value to an object (the UIImageView). That extra value is an id (the URL of the image), so when they want to set the image, they check if the imageView associated id is still the correct one, if not, the cell has already been reused.
That's a simplification of what's done.
So in your case, you could add extra value to the cell, and check if it's really the same inside the closure, but in fact it's not needed, because you're overthinking the issue with KingFisher.
Depending on how you use KingFisher (there are extra parameters : force refresh etc), but by default it will cache itself the images, and check in its cache if it's there or not and avoid a web call.
Here, I'd suggest remove all of the cache check code (since it's already done by the lib):
func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "SimpleThumbnailView", for: indexPath) as! ThumbnailsCollectionViewCell
if indexPath.row < thumbnails.count {
let urlString = thumbnails[indexPath.row].thumbnail ?? ""
cell.thumbnailImage.kf.setImage(with: urlString, placeholder: UIImage(named: "DefaultImage"), options: []) //Might need a if let url here
cell.titleLabel.text = thumbnails[indexPath.row].title
cell.authorLabel.text = StringBuilder.authorLikes(author: thumbnails[indexPath.row].writer, likes: thumbnails[indexPath.row].likeCount)
cell.articleId = thumbnails[indexPath.row].articleId
cell.authorId = thumbnails[indexPath.row].writer
if cell.articleId != -1 {
cell.removePlaceHolder()
}
return cell
}
Now, if you want to check for yourself, you can put breakpoint inside the lib, of use the setImage() with the completion parameter. I didn't test it, but I guess the answer from where is coming the image, cache or not might be in the RetrieveImageResult value.
setImage(with: urlString, placeholder: UIImage(named: "DefaultImage"), options: [], completionHandler: { result in
switch result {
case .failure(let error):
print(error)
case success(let imageResult):
//check imageResult, maybe cacheType, source, etc.
}
}
Another issue you had was reloading of the whole collectionView.