2

I have a UICollectionView in which I show either an image or an image of a video with a play button. The cells take up the entire width of the screen, so only one cell is visible on screen at the same time. Users can scroll left or right to show the next image or video. I can best compare it with the feed of Instagram.

So, there is always an image shown to the user when they scroll. However, when I scroll through the cells when there are, let's say, four or five images, they get mixed up. This is my code:

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
    let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "mediaSliderCell", for: indexPath) as! MediaSliderCell
    if(dataArray.count > 0) {
        if(dataArray[indexPath.item].mediaType == .photo) {
            let mediaURL = URL(string: "https://myURL.com/media/\(dataArray[indexPath.item].mediaURL)")
            cell.photoView.kf.setImage(with: mediaURL, options: [.targetCache(mediaCache)])
            cell.photoView.isHidden = false
            cell.videoView.isHidden = true
        } else if(dataArray[indexPath.item].mediaType == .video) {
            cell.photoView.isHidden = true
            cell.videoView.isHidden = false
            let mediaThumbURL = URL(string: "https://myURL.com/media/\(dataArray[indexPath.item].mediaThumbURL!)")
            let mediaURL = URL(string: "https://myURL.com/media/\(dataArray[indexPath.item].mediaURL)")!
            cell.videoView.placeholderView.kf.setImage(with: mediaThumbURL, options: [.targetCache(mediaCache)])
            cell.videoView.mediaURL = mediaURL
            cell.videoView.testLabel.text = "indexPath.item: \(indexPath.item)"
        }
    }
    return cell
}

I am aware of how dequeuing works. I've had issues with this in the past. I have also tried everything I know so far to avoid this issue:

  • set the imageView.image to nil in prepareForReuse()
  • use an if else statement and set the imageView.image to nil in cellForItemAt, but this returns a black screen (so there is no image then)

It happens when I use video. For photo, I have not seen this yet, but it could be the case as well. How can I fix this?

This is my MediaSliderCell:

class MediaSliderCell: UICollectionViewCell {
    override func prepareForReuse() {
        videoView.placeholderView.kf.cancelDownloadTask()
        videoView.placeholderView.image = UIImage()
        photoView.image = UIImage()
    }

    var photoView: UIImageView = {
        let photoView = UIImageView()
        photoView.translatesAutoresizingMaskIntoConstraints = false
        photoView.backgroundColor = .black
        photoView.isHidden = true
        return photoView
    }()

    var videoView: VideoView = {
        let videoView = VideoView()
        videoView.translatesAutoresizingMaskIntoConstraints = false
        videoView.backgroundColor = .black
        return videoView
    }()

    override init(frame: CGRect) {
        super.init(frame: frame)
        setupViews()
    }

    func setupViews() {
        addSubview(photoView)
        addSubview(videoView)
        photoView.topAnchor.constraint(equalTo: topAnchor).isActive = true
        photoView.bottomAnchor.constraint(equalTo: bottomAnchor).isActive = true
        photoView.leadingAnchor.constraint(equalTo: leadingAnchor).isActive = true
        photoView.trailingAnchor.constraint(equalTo: trailingAnchor).isActive = true
        videoView.topAnchor.constraint(equalTo: topAnchor).isActive = true
        videoView.bottomAnchor.constraint(equalTo: bottomAnchor).isActive = true
        videoView.leadingAnchor.constraint(equalTo: leadingAnchor).isActive = true
        videoView.trailingAnchor.constraint(equalTo: trailingAnchor).isActive = true
    }

    required init?(coder aDecoder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
}

VideoView is also custom, but this is simply the implementation of an AVPlayer and shouldn't influence the behavior of the cells. It's quite a lot of code so I rather not post it as it doesn't deal with the images being shown. Any ideas what is happening here? When I print the indexPaths and URLs in cellForItemAt, I always get the right URL of the image I should see. But the images get mixed up. I use KingFisher to download images and cache them for future use.

EDIT: Basically, as far as I understand the situation right now, in cellForItemAt, the right data gets returned (at least when I print it out). I do notice that cells are retrieved whenever iOS wants to retrieve them, so iOS could be fetching data for cell 0, then cell 1, then cell 2, then cell 1 again... but that should not be the issue.

First, I thought the issue could be in the KingFisher download method, because the download might finish later, when the cell is already changed. But then again, the cell also uses the wrong mediaURL, so not only the image is wrong, but also the mediaURL I link to it (mediaURL is for example https://myURL.com/media/test.mp4 and mediaThumbURL is for example https://myURL.com/media/thumb/test.jpeg). I think the problem lies somewhere else, maybe in the VideoView? Could this be re-used or wrongly associated somehow?

Might also be helpful to mention that my dataArray is built up like this:

struct MediaData: Codable {
    var mediaType: MediaType // enum .photo or .video
    var mediaURL: String
    var mediaThumbURL: String?
}

Next, I simply append all my items and thus I need to show them in the right order, which is why I rely on indexPath.item to pick them out of the array correctly.

EDIT2: I now notice the returned indexPath is sometimes wrong, too. I added a UILabel to my cell and I add cell.testLabel.text = indexPath.item. Sometimes, for the first cell (which should be indexPath item 0), I get 3 as a result. So the very first cell, after scrolling left and right, then has indexPath (0,3). Why is this and how can I fix it?

PennyWise
  • 595
  • 2
  • 12
  • 37

1 Answers1

0

After some research of KingsFisher library i found, that if the cache for your image is disk cache, then cancelDownloadTask() wouldn't prevent KingsFisher from retrieving image from disk cache. Also for disk cache it switch data obtaining to another DispatchQueue and kf.setImage would set image from disk cache after some little amount of time. This lead to interesting "behaviour"(i would say that it's not documented behaviour or maybe bug).
How to reproduce it:
1. Run your app.
2. Scroll to the end of your collection view.
3. Wait some time(until all images are download and cached to disk).
4. Relaunch your app.
5. Wait until image is loaded from disk cache and displayed in your first UICollectionViewCell.
6. Scroll right to second cell and return back to first cell without waiting.
7. You will see image from your second cell instead of first cell.
The reason is that if KingsFisher take image from memory cache synchronously. But for disk cache it make it asynchronously with another DispatchQueue.

// This url is loaded from disk cache
let url1 = URL(string: "some url 1")
// This url is loaded from memory cache
let url2 = URL(string: "some url 2")

// Because url1 is taken from disk cache
// it would be setted asynchronously in future.
imageView.kf.setImage(with: url1)

// Becase url2 is taken from memory cache
// it setted synchronously in present.
imageView.kf.setImage(with: url2)

// As result imageView constains image from url1 instead of image from url2.

/*
 One of the solutions is to use .fromMemoryCacheOrRefresh option
 that would igrnore disk cache, and load images synchronously from memory cache.

 imageView.kf.setImage(with: url1, options: [.fromMemoryCacheOrRefresh])
 imageView.kf.setImage(with: url2, options: [.fromMemoryCacheOrRefresh])

 imageView will show image from url2.
 */

Also you in setupViews you must place label and video view not on cell but on it's contentView.

contentView.addSubview(photoView)
contentView.addSubview(videoView)
Mark
  • 701
  • 6
  • 12
  • Would this also explain why mediaURL is wrong? Would it block the queue until its task is done and thus only then set the mediaURL? Will your suggested solution, fromMemoryCache, prevent downloading again so just wait until the image is available in memory? – PennyWise Nov 02 '19 at 17:10
  • 1) No it's not. Maybe there another problem with UICollectionView, because all code that your post looks good. 2) It would synchronously set url to image and perform next intsruction (setting mediaURL). 3) We need to call cancelDownloadTask in prepareForReuse or right before setting image in cellForRowAt to prevent from downloading. – Mark Nov 02 '19 at 18:26
  • I find another issue. You mast call super.prepareForReuse() in prepareForReuse – Mark Nov 02 '19 at 18:38