Skip to content
This repository has been archived by the owner on Jun 17, 2023. It is now read-only.

Crash in modal view controller when swiping up on the controller. #27

Closed
BubblyNetDev opened this issue Sep 11, 2020 · 13 comments
Closed

Comments

@BubblyNetDev
Copy link

Crashes when swiping up on the controller when having a collectionView with this layout presented modally. Only happens when the view is initially loaded and the first thing you do is swipe up.

Will not crash when you swipe to a different card and swipe up. When you swipe up and it doesn't crash you get other cards reloading from the top left and expanding back into place every time you swipe.

If you comment out viewDidLayoutSubviews code it will not crash but other issues are present.

Error:
[UICollectionViewData isIndexPathValid:validateItemCounts:]: message sent to deallocated instance

@amirdew
Copy link
Owner

amirdew commented Sep 12, 2020

Hi Bob,
I think the crash is not related to the layout but is related to the collection view lifecycle,
could you share a sample code?

I tried to reproduce the issue in the sample projects by presenting view controllers modally but it didn't happen:
on Samples project:

MainViewController.swift:

 @IBAction private func stackButtonTouched() {
        present(ShapesViewController.instantiate(viewModel: ShapesViewModel(layouts: .stack)), animated: true, completion: nil)
    }

@BubblyNetDev
Copy link
Author

I have a UINavigationController with a UIViewController that has a UIBarButtonItem that when selected presents the UIViewController with a CollectionView modally.

BasicSnapshotCollectionViewCell.swift:

class BasicSnapshotCollectionViewCell: UICollectionViewCell, SnapshotTransformView {
    
    var snapshotOptions = SnapshotTransformViewOptions(
        pieceSizeRatio: .init(width: 0.25, height: 0.10),
        piecesCornerRadiusRatio: .static(0.00),
        piecesAlphaRatio: .aggregated([.rowOddEven(
            0.20,
            0.00,
            increasing: false
            ), .columnOddEven(
                0.00,
                0.20,
                increasing: false
            )], +),
        piecesTranslationRatio: .rowOddEven(
            .init(x: -0.15, y: 0.00),
            .init(x: 0.15, y: 0.00),
            increasing: false
        ),
        piecesScaleRatio: .columnOddEven(
            .init(width: 0.10, height: 0.40),
            .init(width: 0.40, height: 0.10),
            increasing: false
        ),
        containerScaleRatio: 0.20,
        containerTranslationRatio: .init(x: 1.00, y: 0.00)
    )
    
    override init(frame: CGRect) {
        super.init(frame: frame)
    }
    
    required init?(coder: NSCoder) {
        super.init(coder: coder)
    }
}

ViewController.swift:

class ViewController: UIViewController, UICollectionViewDataSource {
        
    @IBOutlet weak var collectionView: UICollectionView!
    
    override func viewDidLoad() {
        super.viewDidLoad()
        setupCollectionView()
    }
    
    private func setupCollectionView() {
        let layout = CollectionViewPagingLayout()
        collectionView.collectionViewLayout = layout
        collectionView.isPagingEnabled = true
        collectionView.dataSource = self
    }
    
    override func viewDidLayoutSubviews() {
        super.viewDidLayoutSubviews()
        collectionView?.performBatchUpdates({
            self.collectionView.collectionViewLayout.invalidateLayout()
        })
    }
    
    func collectionView(
        _ collectionView: UICollectionView,
        numberOfItemsInSection section: Int
    ) -> Int {
       return 10
    }
    
    func collectionView(
        _ collectionView: UICollectionView,
        cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        
        if indexPath.row % 2 == 0 {
                    return collectionView.dequeueReusableCell(withReuseIdentifier: "redCell", for: indexPath) as! redCollectionViewCell
        } else {
                    return collectionView.dequeueReusableCell(withReuseIdentifier: "blueCell", for: indexPath) as! blueCollectionViewCell
        }
    }
}

@amirdew
Copy link
Owner

amirdew commented Sep 12, 2020

I made a new project with Xcode 11.6, added the library, and this code:
still don't see any crash at least on the simulator, could you help me with finding the crash?

import UIKit
import CollectionViewPagingLayout

@UIApplicationMain
class AppDelegate: UIResponder, UIApplicationDelegate {

    
    var window: UIWindow?


    func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
        
        self.window = UIWindow(frame: UIScreen.main.bounds)
        let nav = UINavigationController()
        nav.viewControllers = [MainVC()]
        window?.rootViewController = nav
        window?.makeKeyAndVisible()
        return true
    }

}


class MainVC: UIViewController {
    override func viewDidLoad() {
        super.viewDidLoad()
        self.view.backgroundColor = .green
        navigationItem.rightBarButtonItem = UIBarButtonItem(title: "present", style: .done, target: self, action: #selector(presentVC))
    }
    
    @objc func presentVC() {
        present(ViewController(), animated: true, completion: nil)
    }
}


class ViewController: UIViewController, UICollectionViewDataSource {

    weak var collectionView: UICollectionView!
    
    override func viewDidLoad() {
        super.viewDidLoad()
        let collectionView = UICollectionView(frame: .zero, collectionViewLayout: CollectionViewPagingLayout())
        view.addSubview(collectionView)
        self.collectionView = collectionView
        setupCollectionView()
    }
    
    private func setupCollectionView() {
        collectionView.isPagingEnabled = true
        collectionView.dataSource = self
        collectionView.register(BasicSnapshotCollectionViewCell.self, forCellWithReuseIdentifier: "redCell")
    }
    
    override func viewDidLayoutSubviews() {
        super.viewDidLayoutSubviews()
        collectionView.frame = .init(x: 0, y: 0, width: view.frame.width, height: view.frame.height)
        collectionView?.performBatchUpdates({
            self.collectionView.collectionViewLayout.invalidateLayout()
        })
    }
    
    func collectionView(
        _ collectionView: UICollectionView,
        numberOfItemsInSection section: Int
    ) -> Int {
       return 10
    }
    
    func collectionView(
        _ collectionView: UICollectionView,
        cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        
        collectionView.dequeueReusableCell(withReuseIdentifier: "redCell", for: indexPath) as! BasicSnapshotCollectionViewCell
    }

}


class BasicSnapshotCollectionViewCell: UICollectionViewCell, SnapshotTransformView {
    
    var snapshotOptions = SnapshotTransformViewOptions(
        pieceSizeRatio: .init(width: 0.25, height: 0.10),
        piecesCornerRadiusRatio: .static(0.00),
        piecesAlphaRatio: .aggregated([.rowOddEven(
            0.20,
            0.00,
            increasing: false
            ), .columnOddEven(
                0.00,
                0.20,
                increasing: false
            )], +),
        piecesTranslationRatio: .rowOddEven(
            .init(x: -0.15, y: 0.00),
            .init(x: 0.15, y: 0.00),
            increasing: false
        ),
        piecesScaleRatio: .columnOddEven(
            .init(width: 0.10, height: 0.40),
            .init(width: 0.40, height: 0.10),
            increasing: false
        ),
        containerScaleRatio: 0.20,
        containerTranslationRatio: .init(x: 1.00, y: 0.00)
    )
    
    override init(frame: CGRect) {
        super.init(frame: frame)
        setupView()
    }
    
    required init?(coder: NSCoder) {
        super.init(coder: coder)
        setupView()
    }
    
    
    func setupView() {
        let view = UIView()
        contentView.addSubview(view)
        view.backgroundColor = .red
        view.frame = .init(x: 100,y: 100,width: 100,height: 100)
    }
}

@BubblyNetDev
Copy link
Author

I used the Mac app to create the layout clicked save as a project (Xcode 11.7). I then added a new storyboard and then created the controllers and UI elements in Storyboard. Build and run the project and when the collectionView loads drag from the bottom of the view to the top.

If I keep everything the same and just change the collection view layout it will not crash.

From:

let layout = CollectionViewPagingLayout()

To:

let layout = UICollectionViewFlowLayout()

@amirdew
Copy link
Owner

amirdew commented Sep 13, 2020

We cannot say the problem is related to the layout because by changing the layout it's fixable, the flow layout behaves differently, for instance, there might be no need to access to CollectionView in the flow layout.

anyway, it's still not reproducible for me, but I think the problem is coming from viewDidLayoutSubviews,
so could you comment out that, and if that works, try to keep all the references weak to the collection view

     weak var collectionView: UICollectionView!
    
    override func viewDidLayoutSubviews() {
        super.viewDidLayoutSubviews()
        DispatchQueue.main.async { [weak self] in
            self?.collectionView?.performBatchUpdates({ [weak self] in
                self?.collectionView?.collectionViewLayout.invalidateLayout()
            })
        }
    }

@BubblyNetDev
Copy link
Author

Interesting it doesn't happen for you, I can get it to happen in every project I add it to, whether running in a simulator or on a physical device. As mentioned above I did narrow it down to viewDidLayoutSubviews when this is commented out and with the code above is added instead of what is used in the examples and generated from the mac app it will not crash. The cards still animate back into place when you pull up, but no crash so this is good.

@amirdew
Copy link
Owner

amirdew commented Sep 14, 2020

Cool, I'll change the code in the samples and the generated code!
cards animating back is expected since we invalidate the layout but we can improve it by checking the old bounds and new bounds and invalidate the layout only if needed!

can you share a simple project here or via email: Khorsandi@me.com?
I don't know why it's not happening for me! I'm probably missing something!

@BubblyNetDev
Copy link
Author

Okay, thank you. I will send the project soon. Another question, is there a way to avoid invalidating the layout when it first launches? I am not having crashes anymore but I see the view partially come up from the bottom as its about to present and then fully come up. So it looks like it glitches almost.

@amirdew
Copy link
Owner

amirdew commented Sep 18, 2020

Are you sure that is because of invalidating layout?
you can keep old bounds and only invalidate layout if the width changes if your layout doesn't need to be invalidated by changing the height.
I would like to see the code there might be another reason behind it!

@BubblyNetDev
Copy link
Author

I have sent an email with the sample project I modified. It will crash when you pull up on the initial controller without first paging to any other cell. The issue where the view starts to be presented and then the layout invalidates and is presented again is not visible in this sample. In my main project that creates and presents everything the same way but has a more complex layout it is visible.

@BubblyNetDev
Copy link
Author

Again the crash is fixed with this change to invalidateLayout in previous comments.

@amirdew
Copy link
Owner

amirdew commented Sep 27, 2020

Thanks for the email, I can replicate the issue now.
I can tell now the problem is coming from resizing the cell! when we swipe up the cell's contentView height changes!
in the snapshot layout, every time the size of the original view changes it takes a new snapshot! and process it again.
and that's a heavy task and changes the layout!
so I'm gonna provide a way that the developer can decide when it should take a new snapshot!
that would be the easiest way I think

meanwhile, you can set the cell height manually instead of constraint to the superview

@amirdew
Copy link
Owner

amirdew commented Apr 25, 2021

There is a new method for addressing this issue where you can implement and specify when a snapshot can be reused.

public protocol SnapshotTransformView: TransformableView {
 
    /// Check if the snapshot can be reused
    func canReuse(snapshot: SnapshotContainerView) -> Bool
}

check the new version 1.0.0

@amirdew amirdew closed this as completed Apr 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants