ashfurrow / Collection-View-in-a-Table-View-Cell

Sample code for my tutorial
http://ashfurrow.com/blog/putting-a-uicollectionview-in-a-uitableviewcell-in-swift/
MIT License
394 stars 81 forks source link

Low performance in TableView's scroll. #14

Closed PPacie closed 4 years ago

PPacie commented 8 years ago

First of all, great tutorial. Thanks for sharing it with the community as usual =)

Wanted to notice that the approach implemented in this issue works but has a performance side effect. I am using it in the following app, you can test it there.

When you scroll vertically in the Home screen you will notice the lags due to the reloadData() in the setCollectionViewDataSourceDelegate. I have tested commenting that line out and the vertical scroll becomes smooth.

I have an iPhone 5S, perhaps you don't notice it in most recent devices. But still that reloadData() is super expensive there.

PS: I just noticed it, in case I come up with a solution will let you know.

ludvigeriksson commented 8 years ago

I had a similar problem in my app where I also use UICollectionViews inside of UITableViewCells. It didn't lag, but you could actually see the cell before it reloaded, and during the time it took to reload it was unresponsive to user input.

The problem seemed to be that reloadItemsAtIndexPaths(_:), which I use instead of reloadData(), is animating the changes. To stop this I added the following inside tableView(_:willDisplayCell:forRowAtIndexPath:):

UIView.setAnimationsEnabled(false)
cell.collectionView.reloadItemsAtIndexPaths(cell.collectionView.indexPathsForVisibleItems())
UIView.setAnimationsEnabled(true)
PPacie commented 8 years ago

HI Ludving,

Unfortunately that approach didn't work for me. Using the reloadItemsAtIndexPaths() made the app to crash.

I've came up with a more efficient approach in the setCollectionViewDataSourceDelegate():

if collectionView.tag != row {
   collectionView.tag = row
   // Stops collection view if it was scrolling.
   collectionView.setContentOffset(collectionView.contentOffset, animated:false)
   collectionView.reloadData()
 }  

So I just reloadData() when the row changes. Still you can notice (subtle blink) when the Collection redraws that row but the overall experience is better than the original approach.

My next step is to look for a way to avoid reloading the data of the collectionView at all. Not sure if will be able to just load the data once using the approach of this tutorial.

PPacie commented 8 years ago

Wanted to tell you that I have completely changed the approach of the tutorial because of those TableView "scrolling blinks".

What did I do?

I could manage to load the TableView Cells just once, so the CollectionView is not being updated (aka calling reloadData()) every time you scroll up and down through the TableView.

Following the data model structure of the example, I have implemented the following method after getting the model created:

var model = [[Tweets]]()
var reusableCells = [TweetsTableViewCell]()

func createTableViewCells() {        
    for tweets in model {
        let cell = tableView.dequeueReusableCellWithIdentifier("Cell") as! TweetsTableViewCell
        //Here I set the model of the TableViewCell that will be used by the CollectionView.
        cell.tweets = tweets
        reusableCells.append(cell)
    }
    tableView.reloadData()
}

Then in the TableView Datasource:

override func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell {

    let cell = reusableCells[indexPath.section]
    return cell
}

Maybe this approach is not as elegant as the one of the tutorial, but now the performance of the TableView scrolling is way better, totally smooth.

Cheers, Feedback is also welcome.

ashfurrow commented 8 years ago

Clever approach! I hadn't thought of that. Possibly an even better solution would be to create the table views on-demand, as they are displayed for the first time. They could even be stored in an NSCache so that if the system gets low on memory, it can drop the table views and they'll get recreated the next time they're visible.

farzadshbfn commented 8 years ago

@PPacie Your app did not have any bugs? If there are lots of [Tweets] dequeue method will dequeue same cells. and then your setting models all wrong.

juliensagot commented 5 years ago

Has a solution been found for that particular issue? reloadData() is indeed really slow and has a major impact on scroll performances.

ashfurrow commented 5 years ago

@juliensagot not that I'm aware, but I haven't been heads-down in UIKit for a few years so it's possible someone else might know of a workaround.

moKelani commented 4 years ago

any solutions yet?

PPacie commented 4 years ago

The solution to this particular issue was https://github.com/ashfurrow/Collection-View-in-a-Table-View-Cell/issues/14#issuecomment-213578879

Nowadays, there are many ways to approach this UI by using Compositional Layouts or even SwiftUI.