Download and cache images in UITableViewCell
Asked Answered
H

2

6

Note: Please no libraries. This is important for me to learn. Also, there are a variety of answers on this but none that I found solves the issue nicely. Please don't mark as duplicate. Thanks in advance!

The problem I have is that if you scroll really fast in the table, you will see old images and flickering.

  • The solution from the questions I read is to cancel the URLSession data request. But I do not know how to do that at the correct place and time. There might be other solutions but not sure.

This is what I have so far:

Image cache class

class Cache {

    static let shared = Cache()

    private let cache = NSCache<NSString, UIImage>()
    var task = URLSessionDataTask()
    var session = URLSession.shared

    func imageFor(url: URL, completionHandler: @escaping (image: Image? error: Error?) -> Void) {
            if let imageInCache = self.cache.object(forKey: url.absoluteString as NSString)  {
                completionHandler(image: imageInCache, error: nil)
                return
            }

            self.task = self.session.dataTask(with: url) { data, response, error in

                if let error = error {
                    completionHandler(image: nil, error: Error)
                    return
                }

                let image = UIImage(data: data!)

                    self.cache.setObject(image, forKey: url.absoluteString as NSString)
                    completionHandler(image: image, error: nil)
                }

            self.task.resume()
    }
}

Usage

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let cell = tableView.dequeueReusableCell(withIdentifier: "cell", for: indexPath)
    let myImage = images[indexPath.row]

    if let imageURL = URL(string: myImage.urlString) {
        photoImageView.setImage(from: imageURL)
    }
    return cell
}

Any thoughts?

Hotspur answered 25/6, 2017 at 5:49 Comment(2)
Take a look at Apple's LazyTableImagesTokay
By the way, your dataTask closure should presumably dispatch it's completion handlers back to the main queue if you're going to update UI objects in that closure. You're asynchronous dispatching the issuing of the request to the main queue, but that doesn't affect the completion handler queue. So dispatch UI updates to the main queue.Hessite
H
6

A couple of issues:

  1. One possible source of flickering is that while you're updating the image asynchronously, you really want to clear the image view first, so you don't see images for prior row of reused/dequeued table view cell. Make sure to set the image view's image to nil before initiating the asynchronous image retrieval. Or, perhaps combine that with "placeholder" logic that you'll see in lots of UIImageView sync image retrieval categories.

    For example:

    extension UIImageView {
    
        func setImage(from url: URL, placeholder: UIImage? = nil) {
            image = placeholder               // use placeholder (or if `nil`, remove any old image, before initiating asynchronous retrieval
    
            ImageCache.shared.image(for: url) { [weak self] result in
                switch result {
                case .success(let image):
                    self?.image = image
    
                case .failure:
                    break
                }
            }
        }
    }
    
  2. The other issue is that if you scroll very quickly, the reused image view may have an old image retrieval request still in progress. You really should, when you call your UIImageView category's async retrieval method, you should cancel and prior request associated with that cell.

    The trick here is that if you're doing this in a UIImageView extension, you can't just create new stored property to keep track of the old request. So you'd often use "associated values" to keep track of prior requests.

Hessite answered 25/6, 2017 at 6:13 Comment(15)
Thanks for responding so quickly! I read many of your answers on this and understand the concept but not sure how to implement. Is there a quick, not looking for perfect way, to adjust code I have so far to make this work. What I thought of doing is cancelling pending requests if the cell is not on screen or something. What do you think? Thanks again!Hotspur
@Hotspur See revised answer with updated setImage method that makes sure to reset image before initiating asynchronous request.Hessite
Just changed my code to this now, but it still has similar issue. The images keep replacing each other over and over until it finally stops changing. I think its due to all of the dataRequest's that get made when scrolling really fast, like super fast. Is there a simple (not saying perfect) way to fix this? would be great to learn itHotspur
Yep, that's a second problem, that you should keep a reference to the old image's URLSessionTask and cancel it before starting a new request.Hessite
With the architecture I have in the code above, I just don't know how to do that, it's hard for me to implement this; although conceptually I understand it. I created the property private var task = URLSessionDataTask() inside of ImageCache for this reason but don't know how to cancel it or if this is the way to do it ?Hotspur
First, don't define vars like that. It would be private var task: URLSessionTask?. Second, this saved URLSessionTask is saved at the UIImageView extension, not at the ImageCache. When initiating a new request for an image view, you want to cancel prior requests associated with this image view, only. The hassle here is that you can't define new stored properties in extensions, so you have to use the "associated value" API of NSObject. See nshipster.com/associated-objects.Hessite
If you want to see practical implementation of associated objects for UIImageView async image retrieval categories, see how KingFisher uses associated objects to keep track of the previous request for that image view. github.com/onevcat/Kingfisher/blob/master/Sources/….Hessite
Thanks! This is getting very tricky, I see KingFisher has a associated about private var imageTaskKey: Void? which could be what they use to track the taskHotspur
Do you see this becoming something that will require another 100 lines of code to do, or am I simply missing like 5 or 10 lines of code to complete this?Hotspur
Yes, to do this properly is a bit tricky, which is why we generally say "don't try to write this yourself; use an existing library like AlamofireImage or KingFisher to do this." It's not rocket science, but it's non-trivial, too. I might suggest that you first do this in a UIImageView subclass (where having an additional stored property is trivial ... tho it's not as elegant because you now have to replace UIImageView with your custom subclass in IB), and once you've got that working, then research how to do this in an extension to UIImageView using associated values. Small steps.Hessite
I see, thanks for explaining! Last question: If there was a quick fix without a library, do you know what it would be? Meaning its kind of a hack but would do the job in an ok way without writing 100 lines of code. I'm accepting your answering tooHotspur
I wouldn’t describe doing this with a UIImageView subclass rather than an extension as a “hack”. It’s just not as elegant. Consider it version 0.1 of your eventual solution. Save a weak reference to the URLSessionTask in your UIImageView subclass and your setImage would then just task?.cancel(). And make sure your dataTask closure doesn’t report cancellation as failure. But, honestly, with no offense intended, I’m not inclined to write this all for you, esp when there are libraries that already do all of this already. Give it a go yourself. Good luck and happy coding!Hessite
To clarify, I didn't call your idea a hack; I asked if there was a hack that could work in this case --- Just wanted to clear that up case I did not want to offend : ) You've helped enough and will take a look at all you said again - you're a true pro!Hotspur
The hack you could consider is keeping the URLSessionTask in the cell, itself (where having that weak stored property is easy). It’s not the right logical place to keep track of prior requests issued by the extension, but it would work. Personally, I’d probably do the UIImageView subclass approach, but both would work.Hessite
That sounds great! By the way, I also like you answer for other SO question because I finally understand it clearly after your explanations above. This is your answer: #16664118Hotspur
L
7

Swift 3:

Flickering can be avoided by this way:

Use the following code in public func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell

cell.photoImageView.image = nil //or keep any placeholder here
cell.tag = indexPath.row
let task = URLSession.shared.dataTask(with: imageURL!) { data, response, error in
    guard let data = data, error == nil else { return }

    DispatchQueue.main.async() {
        if cell.tag == indexPath.row{
            cell.photoImageView.image = UIImage(data: data) 
        }
    }
}
task.resume()

By checking cell.tag == indexPath.row, we are assuring that the imageview whose image we are changing, is the same row for which the image is meant to be. Hope it helps!

Lengthen answered 25/6, 2017 at 8:16 Comment(0)
H
6

A couple of issues:

  1. One possible source of flickering is that while you're updating the image asynchronously, you really want to clear the image view first, so you don't see images for prior row of reused/dequeued table view cell. Make sure to set the image view's image to nil before initiating the asynchronous image retrieval. Or, perhaps combine that with "placeholder" logic that you'll see in lots of UIImageView sync image retrieval categories.

    For example:

    extension UIImageView {
    
        func setImage(from url: URL, placeholder: UIImage? = nil) {
            image = placeholder               // use placeholder (or if `nil`, remove any old image, before initiating asynchronous retrieval
    
            ImageCache.shared.image(for: url) { [weak self] result in
                switch result {
                case .success(let image):
                    self?.image = image
    
                case .failure:
                    break
                }
            }
        }
    }
    
  2. The other issue is that if you scroll very quickly, the reused image view may have an old image retrieval request still in progress. You really should, when you call your UIImageView category's async retrieval method, you should cancel and prior request associated with that cell.

    The trick here is that if you're doing this in a UIImageView extension, you can't just create new stored property to keep track of the old request. So you'd often use "associated values" to keep track of prior requests.

Hessite answered 25/6, 2017 at 6:13 Comment(15)
Thanks for responding so quickly! I read many of your answers on this and understand the concept but not sure how to implement. Is there a quick, not looking for perfect way, to adjust code I have so far to make this work. What I thought of doing is cancelling pending requests if the cell is not on screen or something. What do you think? Thanks again!Hotspur
@Hotspur See revised answer with updated setImage method that makes sure to reset image before initiating asynchronous request.Hessite
Just changed my code to this now, but it still has similar issue. The images keep replacing each other over and over until it finally stops changing. I think its due to all of the dataRequest's that get made when scrolling really fast, like super fast. Is there a simple (not saying perfect) way to fix this? would be great to learn itHotspur
Yep, that's a second problem, that you should keep a reference to the old image's URLSessionTask and cancel it before starting a new request.Hessite
With the architecture I have in the code above, I just don't know how to do that, it's hard for me to implement this; although conceptually I understand it. I created the property private var task = URLSessionDataTask() inside of ImageCache for this reason but don't know how to cancel it or if this is the way to do it ?Hotspur
First, don't define vars like that. It would be private var task: URLSessionTask?. Second, this saved URLSessionTask is saved at the UIImageView extension, not at the ImageCache. When initiating a new request for an image view, you want to cancel prior requests associated with this image view, only. The hassle here is that you can't define new stored properties in extensions, so you have to use the "associated value" API of NSObject. See nshipster.com/associated-objects.Hessite
If you want to see practical implementation of associated objects for UIImageView async image retrieval categories, see how KingFisher uses associated objects to keep track of the previous request for that image view. github.com/onevcat/Kingfisher/blob/master/Sources/….Hessite
Thanks! This is getting very tricky, I see KingFisher has a associated about private var imageTaskKey: Void? which could be what they use to track the taskHotspur
Do you see this becoming something that will require another 100 lines of code to do, or am I simply missing like 5 or 10 lines of code to complete this?Hotspur
Yes, to do this properly is a bit tricky, which is why we generally say "don't try to write this yourself; use an existing library like AlamofireImage or KingFisher to do this." It's not rocket science, but it's non-trivial, too. I might suggest that you first do this in a UIImageView subclass (where having an additional stored property is trivial ... tho it's not as elegant because you now have to replace UIImageView with your custom subclass in IB), and once you've got that working, then research how to do this in an extension to UIImageView using associated values. Small steps.Hessite
I see, thanks for explaining! Last question: If there was a quick fix without a library, do you know what it would be? Meaning its kind of a hack but would do the job in an ok way without writing 100 lines of code. I'm accepting your answering tooHotspur
I wouldn’t describe doing this with a UIImageView subclass rather than an extension as a “hack”. It’s just not as elegant. Consider it version 0.1 of your eventual solution. Save a weak reference to the URLSessionTask in your UIImageView subclass and your setImage would then just task?.cancel(). And make sure your dataTask closure doesn’t report cancellation as failure. But, honestly, with no offense intended, I’m not inclined to write this all for you, esp when there are libraries that already do all of this already. Give it a go yourself. Good luck and happy coding!Hessite
To clarify, I didn't call your idea a hack; I asked if there was a hack that could work in this case --- Just wanted to clear that up case I did not want to offend : ) You've helped enough and will take a look at all you said again - you're a true pro!Hotspur
The hack you could consider is keeping the URLSessionTask in the cell, itself (where having that weak stored property is easy). It’s not the right logical place to keep track of prior requests issued by the extension, but it would work. Personally, I’d probably do the UIImageView subclass approach, but both would work.Hessite
That sounds great! By the way, I also like you answer for other SO question because I finally understand it clearly after your explanations above. This is your answer: #16664118Hotspur

© 2022 - 2024 — McMap. All rights reserved.