search

marcosgriselli / SwipeableTabBarController

UITabBarController with swipe interaction between its tabs.
MIT License
1.54k stars 107 forks source link

Very quick swipes cause app to freeze #52

Open jyc66 opened 5 years ago

jyc66 commented 5 years ago

Issuehunt badges

On iOS 12.0.1, iPhone X with SwipeableTabBarController 1.4.0. Initially, produced in my app and then reproduced on the sample app.

Issue: Swipe very quick (velocity.x > 1500, even up to 4000) causes the app to sometime freeze. The tabs may still work (they select but nothing else changes).

Reproduction steps:

  1. Make this change in the sample app to make it easier to reproduce: In TabBarController.swift, line 17, change selectedViewController = viewControllers![1] to selectedViewController = viewControllers![0]

  2. Run the app on a real device

  3. Slowly swipe from right to left so that the screen tracks your finger.

  4. From the middle screen, very quickly swipe from right to left. You need to be quick enough so that the tab changes without tracking your finger. It seems to happen most often when velocity.x is over 1500.

  5. If the bug does not occur, you must kill the app and restart as the issue will not occur once each screen has been displayed.

I have not been able to fix this myself as of yet but I have noticed that UIView.animate has been called and the completion handler is never fired when the issue occurs.

IssueHunt Summary ### Backers (Total: $150.00) - [marcosgriselli marcosgriselli](https://issuehunt.io/u/marcosgriselli) ($150.00) #### [Become a backer now!](https://issuehunt.io/r/marcosgriselli/SwipeableTabBarController/issues/52) #### [Or submit a pull request to get the deposits!](https://issuehunt.io/r/marcosgriselli/SwipeableTabBarController/issues/52) ### Tips - Checkout the [Issuehunt explorer](https://issuehunt.io/r/marcosgriselli/SwipeableTabBarController/) to discover more funded issues. - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on IssueHunt to raise funds.
marcosgriselli commented 5 years ago

I've been fighting this issue since the release of the library. It happens because it is trying to start a transition before the previous one finished. I'll see if I can come up with a final solution on the upcoming days.

jyc66 commented 5 years ago

Thanks, one note is that if the view has already appeared I haven't been able to reproduce the issue. Ie it can only happen the first time you swipe to a particular tab.

VitaliyR commented 5 years ago

Wouch saw it also. Reproducible simple without changing library code - just be fast enough (even on the simulator using a trackpad)

marcosgriselli commented 5 years ago

Just spent some time digging into this and I gotta admit this issue is strange. All the steps seem to be executed correctly but the transition simply doesn't happen. I'll take some time closer to the end of the week or weekend to refactor a few implementations that should fix this.

jyc66 commented 5 years ago

Looking forward to it, let me know if I can help. (Admittedly, I already spent a bit of time and could not solve it myself)

jyc66 commented 5 years ago

Hello @marcosgriselli did you end up looking at this issue this weekend?

marcosgriselli commented 5 years ago

@jyc66 Hey! good news I'm about to finish refactoring a lot of the transition behavior based on some Apple sample code which I believe works fine. Are you using an animation other than sideBySide? I still need to adjust the frames set up for all the different supported animations.

marcosgriselli commented 5 years ago

You can check the refactor branch momentarily although it is missing some features that will get added on the upcoming days.

jyc66 commented 5 years ago

Hey this is great, I'll check tomorrow when I'm at the office. Thanks

marcosgriselli commented 5 years ago

@jyc66 perfect, let me know how it goes. There are some features still missing like completing the transition when you flick quickly but it shouldn't be too hard to add.

jyc66 commented 5 years ago

I tried it out yesterday using the example app and wasn't able to reproduce the issue however in my actual app I end up with a crash on load. I haven't had a chance to investigate the cause. Our app is rather complicated and has a lot of legacy code so I'm not sure where the issue lies. I'll spend a bit more time later this week on it. Thanks

marcosgriselli commented 5 years ago

Ok, keep me posted on that

marcosgriselli commented 5 years ago

@jyc66 did you had any luck with finding the crash origin?

jyc66 commented 5 years ago

I managed to fix the crash and things appear to work now although I will need to continue testing on a real device. I can see you put a ton of work into v2, incredible! Thanks!

marcosgriselli commented 5 years ago

Great! I pushed v2.0 today so you can try it out already. It might need a tweak here and there but the core functionality and all animations are there.

marcosgriselli commented 5 years ago

Closing this for now.

CameraPost commented 4 years ago

There are some features still missing like completing the transition when you flick quickly but it shouldn't be too hard to add.

Hi Marco @marcosgriselli , thank you for this library!! Follow-up about your post: "There are some features still missing like completing the transition when you flick quickly but it shouldn't be too hard to add."

What features did you have in mind for that? Is that something such as to complete the prior transition if you're flipping quickly to avoid a crash? Appears that quick swiping is causing freezing -- but possibly also something else I haven't figured out yet as well. Thanks!

marcosgriselli commented 4 years ago

@CameraPost First I need to fix that crash, then I'll probably add that flick to transition feature. I'm a bit short of time these past months so progress has been slow.

CameraPost commented 4 years ago

@marcosgriselli Thanks! I understand! I can add that .sideBySide appeared to freeze very often -- so far .overlap hasn't frozen though I haven't tested it thoroughly.

Just many better ways than what I'm adding for now below to the TabBarController using SwipeableTabBarController which seemed to reduce but not stop the crashes with .sideBySide. Attempt was to try to wait for the animation to complete.

@objc func panned(recognizer: UIPanGestureRecognizer) {
        let isDragged: Bool

        switch recognizer.state {
        case .began, .changed:
            isDragged = true
        case .ended:
            isDragged = false
            isSwipeEnabled = false
            isManualDelayForTransitionToFinishStillHappening = true
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { [weak self] in
                self?.isSwipeEnabled = true
                self?.isManualDelayForTransitionToFinishStillHappening = false
            }
        default:
            isDragged = false
        }

        viewControllers?.forEach({ (vc) in
            (vc as? Draggable)?.isDragged = isDragged
        })
    }

And:

    override func tabBar(_ tabBar: UITabBar, didSelect item: UITabBarItem) {
        if isManualDelayForTransitionToFinishStillHappening == true {
            return
        }

        let selectedIndex = tabBar.items?.firstIndex(of: item) ?? 0
        self.updateUploadTint(profileSelected: selectedIndex == 3)

        if selectedIndex == self.selectedIndex {
            (viewControllers?[safe: selectedIndex] as? Reselectable)?.didReselect(in: self)
        }
    }
marcosgriselli commented 4 years ago

Thanks for the code samples @CameraPost! I'll be looking into this next week. I'm quite busy with BA:Swiftable this week.

If you want to discuss implementation details or work on it together like pair-programming feel free to reach out via twitter or email. Always happy to join :).

eliaxyz commented 4 years ago

Hi Marco, your controller is nearly perfect, but.... I get regularly strange freezes by using the controller and all views stop reacting to touch interactions. At the same time the views can still be controlled by code, but not by touch. I tried to debug that error, and I found out, that the processor is at 5 % and my app is completely working normal, but the views (e.g. buttons) are not reacting any more, and also no swipe is possible anymore to change the tab. I would be fine with a workaround, e.g. refreshing all views or something like this, but it seems impossible to detect the freeze programmatically because everything seems to run completely normal. I would be very very thankful, if you could find a solution for this bug, because in the end I cannot use the controller anymore, when it doesn't stop freezing.

marcosgriselli commented 4 years ago

@eliaxyz yes, this has been a persisting issue with this library. I'm not able to replicate the problem consistently in order to find a solution. If you have any sample code that reproduces the issue consistently I'd really appreciate it.

eliaxyz commented 4 years ago

Hi Marcos,

just wondering, if there is a way to answer to you privately… because I would not like to publish parts of my current project to everyone. I’m going to create a test project with some racing conditions to produce the crash now…. … still don’t know, what happens with this message, sent from my email client :-)

Am 17.02.2020 um 14:22 schrieb Marcos Griselli notifications@github.com:

@eliaxyz https://github.com/eliaxyz yes, this has been a persisting issue with this library. I'm not able to replicate the problem consistently in order to find a solution. If you have any sample code that reproduces the issue consistently I'd really appreciate it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/marcosgriselli/SwipeableTabBarController/issues/52?email_source=notifications&email_token=AOIZEXBJQKV2FV27P5YV5DLRDKFSVA5CNFSM4GQ4GVV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL6MNQQ#issuecomment-586991298, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOIZEXCTHJPRQQPJBFZFGV3RDKFSVANCNFSM4GQ4GVVQ.

marcosgriselli commented 4 years ago

Yes, you can write me to my username at gmail or via twitter (same username)

eliaxyz commented 4 years ago

While creating different test-projects with race-condition, I startet with the following code, in which a freeze always occurs:

class ViewController: UIViewController {

var currentVC = 0
var x = 0

override func viewDidLoad() {
    super.viewDidLoad()

           Timer.scheduledTimer(withTimeInterval: 0.1, repeats: true) { timer in
           self.timerFunc()
       }
}

func timerFunc(){

    if(x > 10){
        return
    }

    currentVC = abs(currentVC - 1)
    x = x + 1

    print(x, ": ", currentVC)
    tabBarController?.selectedIndex = currentVC
}
eliaxyz commented 4 years ago

There are 2 different errors possible with this test-code:

  1. No view becomes visible at all
  2. The ViewController freezes and doesn't react to movements anymore after starting with some fast "swipes"

I guess, the SwipeableTabBarController should be able to handle this stress-situation without error.

marcosgriselli commented 4 years ago

@eliaxyz this is great! I just tried your code and managed to reproduce the issue consistently. I might have an idea for a fix. I'll be working on this in the upcoming days. Thank you very much!

eliaxyz commented 4 years ago

Maybe it could be a good idea to introduce a new custom function (eg. swipeToIndex) for changing between tabs animated. And letting the property-change of "selectedIndex" doing the same without animation (change to the new tab immediately)...?

marcosgriselli commented 4 years ago

That might be a good idea. I'm trying to keep the library as close as UITabBarController as possible so you just subclass it and everything works with animations. If I'm not able to come up with a solution I think that's the path we will take.

eliaxyz commented 4 years ago

Hey Marcos,

I didn't take a deeper look at your code yet, but it's clear, that the problem is caused by a wrong timing. (When to start animation, from where to start it, and when to set "selectedIndex" - which should not start any animation, but should only be set indirectly when swipe-animation already arrived at the associated item!)

You can modify my testing code by setting: withTimeInterval: 1 and in SwipeTransitionAnimator.swift set: animationDuration: TimeInterval = 1 and you'll get the same issue...

I hope this helps and I'm looking forward to your solution 👍

eliaxyz commented 4 years ago

Hi Marcos,

I modified the former "crash-test" with the following:

var vc: ViewController?
vc = self // in didLoad

    override func viewDidAppear(_ animated: Bool) {
           super.viewDidAppear(animated)

        let jetzt = Int(round(Double(DispatchTime.now().uptimeNanoseconds/1000000)))
        let dauer = jetzt - zuletzt
        zuletzt = jetzt

           print("index:",  (tabBarController?.selectedIndex)!,
                 "          Appear: 0 -- +", dauer)
       }

    override func viewDidDisappear(_ animated: Bool) {
      super.viewDidDisappear(animated)

        let jetzt = Int(round(Double(DispatchTime.now().uptimeNanoseconds/1000000)))
        let dauer = jetzt - zuletzt
        zuletzt = jetzt

        print("index:",  (tabBarController?.selectedIndex)!,
              "          Appear: 0 -- +", dauer)
    }

    func timerFunc(){
        if(x > 10){
            return
        }

        currentVC = abs(currentVC - 1)
        x = x + 1
        let jetzt = Int(round(Double(DispatchTime.now().uptimeNanoseconds/1000000)))
        let dauer = jetzt - zuletzt
        zuletzt = jetzt

        print(x, ":",  (tabBarController?.selectedIndex)!, "--> selectedIndex =", currentVC,  "-- +", dauer)
        tabBarController?.selectedIndex = currentVC
    }

And I did the same for the second ViewController like this:


       override func viewDidAppear(_ animated: Bool) {
           super.viewDidAppear(animated)

        let jetzt = Int(round(Double(DispatchTime.now().uptimeNanoseconds/1000000)))
        let dauer = jetzt - vc!.zuletzt
        vc?.zuletzt = jetzt

           print("index:",  (tabBarController?.selectedIndex)!,
                 "          Appear: 1 -- +", dauer)
       }

    override func viewDidDisappear(_ animated: Bool) {
      super.viewDidDisappear(animated)

        let jetzt = Int(round(Double(DispatchTime.now().uptimeNanoseconds/1000000)))
        let dauer = jetzt - vc!.zuletzt
        vc?.zuletzt = jetzt

        print("index:",  (tabBarController?.selectedIndex)!,
              "          Appear: 1 -- +", dauer)

    }

As result you can see what happens and when it happens: (the numbers behind the + is the time after last event in mili-seconds):

index: 0           Appear: 0 -- + 64
1 : 0 --> selectedIndex = 1 -- + 972
2 : 1 --> selectedIndex = 0 -- + 1000
index: 0           Appear: 1 -- + 239
index: 0           Appear: 0 -- + 0 // this really should never happen, because it means, that the second ViewController "appeared" de facto at the same time as the first one (+0)
3 : 0 --> selectedIndex = 1 -- + 760
index: 1           Appear: 0 -- + 241
index: 1           Appear: 1 -- + 0   // and again there is no time between the didAppear from the 2 different ViewControllers!

And as logical result the view crashed with the error:

2020-02-24 18:00:48.666482+0100 SwipeRace[2116:266350] Unbalanced calls to begin/end appearance transitions for <SwipeRace.ViewController2: 0x7fc8a3e02d90>.
2020-02-24 18:00:48.666686+0100 SwipeRace[2116:266350] Unbalanced calls to begin/end appearance transitions for <SwipeRace.ViewController: 0x7fc8a7101a30>

This error is well documented...

... But you have really to consider: even if the controller would not be crashing, the timing of everything is really messed up!

Independently from all the different ways, the controller can be crashed... it might always be helpful, to abort the current animation, before another one gets initiated in general.

marcosgriselli commented 4 years ago

@eliaxyz great! Thanks for all the feedback. I was out last week but I'll be able to catch up with this issue this week. I'll keep you posted.

IssueHuntBot commented 4 years ago

@marcosgriselli has funded $80.00 to this issue.

joshkautz commented 4 years ago

I've had the same issue with swiping quickly - spent some time trying to identify the source of the issue, but no luck.

issuehunt-oss[bot] commented 4 years ago

@marcosgriselli has funded $70.00 to this issue.

jorgealegre commented 3 years ago

Hello :D Is this still an issue? I couldn't reproduce running the sample app on an iOS 14.3 iPhone. If someone knows how to reproduce this and in what context (lib version, iOS version, etc.), I'd happily look into it.

marcosgriselli commented 3 years ago

@jorgealegre is probably the only important issue the library always had. I recently refactor part of the transition code to use UIViewPropertyAnimator to enable interruptible animations since the black screen was caused by transitions being triggered before an old transition finished. Doing so reduced the number of times the black screen appears but a user reported that it still happened rarely and that other smaller issues were introduced. You can check the last comments on #94.

Help is always appreciated and I'm here to clear any doubt that might arise. Also, this issue has $150 assigned on IssueHunt which you can claim if you come up with a proven working fix.