daltoniam / Skeets

Fetch, cache, and display images via HTTP in Swift.
Apache License 2.0
191 stars 21 forks source link

Images mixed up while using with UICollectionView #3

Closed mahmed8003 closed 9 years ago

mahmed8003 commented 9 years ago

While using library with UICollectionView, inside UICollectionViewDataSource

 func collectionView(collectionView: UICollectionView, cellForItemAtIndexPath indexPath: NSIndexPath) -> UICollectionViewCell {
        let cell = collectionView.dequeueReusableCellWithReuseIdentifier(reuseIdentifier, forIndexPath: indexPath) as PartCollectionViewCell

        cell.backgroundColor = UIColor.whiteColor()

        //fetch the image
        ImageManager.sharedManager.fetch(imagePath,
            progress: { (status: Double) in
            },success: { (data: NSData) in
                cell.ivIcon.image = UIImage(data: data)
            }, failure: { (error: NSError) in
                cell.ivIcon.image = nil
        })

        return cell
    }

When collectionview reuses the view then images get mixed up. Let suppose there is cell A, and code made a request for IMG_URL_1 that is 43KB It will take time to load CollectionView while scrolling reuses cell A again and this time code made a request for IMG_URL_6 for this cell, and this image is only of 6KB, so it will load faster while IMG_URL_1 still in loading process. First view gets updated with IMG_URL_6 image because it loaded early, but after few mili seconds IMG_URL_1 also get loaded so cell will get updated again with IMG_URL_1 this time. But it should display IMG_URL_6 image instead of IMG_URL_1 image.

daltoniam commented 9 years ago

Yeah that will be a tricky one to work around. Good part is after the first request, the caching should kick in. I just checked in a cancel method that should help. Try calling that before a cell gets reused and let me know if that fixes it.

mahmed8003 commented 9 years ago

I think its hard to get previous url that I need to cancel before loading new url on reused cell. Please check above sample how can I cancel previous url?

daltoniam commented 9 years ago

where you are getting the imagePath variable from? Also have you seen the prepareForReuse method?

https://developer.apple.com/library/ios/documentation/UIKit/Reference/UICollectionReusableView_class/#//apple_ref/doc/uid/TP40012178-CH1-SW2

mahmed8003 commented 9 years ago
class MaterialCollectionViewDataSource : NSObject, UICollectionViewDataSource {

    private let reuseIdentifier = "MaterialCell"
    private var materials : [Material] = []

    init(materials:[Material]) {
        self.materials = materials
    }

    func collectionView(collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
        return self.materials.count
    }

    func collectionView(collectionView: UICollectionView, cellForItemAtIndexPath indexPath: NSIndexPath) -> UICollectionViewCell {
        let cell = collectionView.dequeueReusableCellWithReuseIdentifier(reuseIdentifier, forIndexPath: indexPath) as MaterialCollectionViewCell
        cell.backgroundColor = UIColor.whiteColor()
        // Configure the cell

        let material = self.materials[indexPath.row]

        //fetch the image
        ImageManager.sharedManager.fetch(material.imagePath,
            progress: { (status: Double) in
            },success: { (data: NSData) in
                cell.ivIcon.image = UIImage(data: data)
            }, failure: { (error: NSError) in
                cell.ivIcon.image = nil
        })

        return cell
    }

    func numberOfSectionsInCollectionView(collectionView: UICollectionView) -> Int {
        return 1
    }

    func setMaterials(materials:[Material]) {
        self.materials = materials
    }

}

I think func prepareForReuse() will not help for getting previous url either.

daltoniam commented 9 years ago

Ah I see how it works now. Maybe try adding a imageUrl property onto your MaterialCollectionViewCell? That way you can know which URL that cell is loading right now and cancel it if it is still running.

daltoniam commented 9 years ago

Where you able to add that property and test the cancel method?

mahmed8003 commented 9 years ago

Yes I tested this way but now it takes longer time and app started to crash frequently with issue https://github.com/daltoniam/Skeets/issues/4 The I reverted back the code to previous point where images mixup but app not crashed frequently.

daltoniam commented 9 years ago

ok cool. I think we got this one sorted out, just have to get #4 resolved and will be good to go.