We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 2826

Summary: Using SDL_iPhoneSetAnimationCallback and Launch Screen causes crash
Product: SDL Reporter: Diego <diegoacevedo91>
Component: videoAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: critical    
Priority: P2 CC: amaranth72, ewmailing, icculus
Version: HG 2.1Keywords: triage-2.0.4
Hardware: iPhone/iPod touch   
OS: iOS (All)   
Attachments: Modified happy.c demo which uses SDL_iPhoneSetAnimationCallback
Modified crashing happy.c with SDL_iPhoneSetAnimationCallback

Description Diego 2014-12-24 04:35:22 UTC
I'm testing the newest clone from Mercurial. I added a new iOS launch screen to the iOS demo apps. Then modified happy.c to run the main loop in a callback using SDL_iPhoneSetAnimationCallback. The app crashes.
Comment 1 Eric wing 2014-12-24 08:46:11 UTC
I reproduced the problem. It looks like the launch screen code is some how mysteriously interfering with iOS's behavior when SDL_iPhoneSetAnimationCallback is used. Stepping through the debugger, I don't see the transition point, but something inside iOS is trying to send a message (selector) to a CGImage (which doesn't make sense to me because CGImage is a C type and not an Obj-C type (unless it is toll-free bridged, but I don't think it is.)


The best solution is to remove all the SDL launch screen code entirely. This discussion came up in my discussion with Alex Szpakowski after my previous patch for iOS 8 launch nib support.
                                                                                
My concern at the time was removing this code would break legacy behavior for iOS 7 and below, or more specifically, launch screens would not work.
                                                                                
However, upon more testing with the iOS 7.1 legacy simulator, it appears that the legacy iOS 7 launch screen behavior will work if you conform to Apple's UILaunchImages array rules as shown here:
http://stackoverflow.com/questions/18976412/launch-screens-supporting-ios6-and-ios7-forced-to-splash-screen
                                                                                
I do not know what will happen with iOS 6 or earlier. However, as it stands today, iOS 8 has a 64% adoption rate and iOS 7 has 32%, leaving 4% of earlier. So I don't think legacy iOS versions are a big issue at this point.
http://arstechnica.com/apple/2014/12/apple-dashboard-says-ios-8-now-runs-on-about-two-thirds-of-devices/
                                                                                
The one minor downside is that SDL's legacy behavior did not fully conform to Apple's rules, particularly the iOS 7 UILaunchImages array. It appears the generic Default.png trick still works in my testing. However, there could be some subtle differences in behavior for the iOS 7 case. Still this is probably the better choice because SDL never quite did the right thing and now users can access the full power of the official iOS 7 convention if they need it. And it is unlikely anybody will notice any breakage at this point due to high iOS 8 adoption rates and my guess is Apple is not scrutinizing iOS 7 launch screens in app review any more.

Additionally, this is a much better fix because we are no longer hacking around Apple's launch code, and instead directly using built in behavior. I think this also solves some other issues Alex was working on.
              
                                      
Patch is here:
https://bitbucket.org/ewing/sdl_ios8launchscreen2
Comment 2 Diego 2014-12-24 15:08:11 UTC
Your patch worked for me. The launch screen worked as expected on iOS 8. On iOS 7 I do not directly change anything in the plist. I use a launch image asset catalog, and the image files don't follow any specific naming convection. I just ensure the catalog is reporting the images in their correct positions. This also works with your patch. I don't support iOS 6 for my apps anymore. So I did no testing with that. Also, building and using the libs from your patch drastically slowed down my apps on my device, 4S iOS 8, but not on the simulators. I'm not sure why. I haven't looked into it.
Comment 3 Eric wing 2014-12-24 17:51:55 UTC
Would you look into why it is slowing down your build and apps? That is very surprising because the patch only deletes code and doesn't add any. Additionally, the code in question is only run once at start up and doesn't get re-run every frame.
Comment 4 Diego 2014-12-24 20:31:27 UTC
I'm looking into it. I haven't been able to figure it out yet. I am running the unaltered happy.c from within the Xcode-iOS Demos project from both your patch and the Mercurial clone from yesterday. I can confirm it is running slow on a 4S and 5C running iOS 8, with and without a launch screen or launch images. The slowdown occurs deep within SDL_RenderCopy. There is no slowdown from the Mercurial branch.
Comment 5 Diego 2014-12-24 20:33:12 UTC
Also slows down with or without SDL_iPhoneSetAnimationCallback
Comment 6 Eric wing 2014-12-25 01:33:01 UTC
Would you revert to the immediate commit before my patch (e3ee6ba)? I want to prove that this patch doesn't affect the performance so we're not barking up the wrong tree. (Maybe some other commit altered the performance.)
Comment 7 Diego 2014-12-25 03:29:29 UTC
Good thinking. I can confirm the slow down happened after commit 8900afb. f0bd407 is good. 8900afb is slow.
Comment 8 Eric wing 2014-12-25 20:28:56 UTC
Okay, here's my recommendation:

- This patch gets accepted into mainline. (Sam?)

- Open a new bug about the performance regression (including the revision hash numbers you mention here)

- Please include a reproducible sample case or a way to clearly see the problem. (I don't notice anything running happy.c on my iPad mini.)

- Try to get Ryan's attention. The commit that broke it seems to be the Emscripten commit. It's kind of a large commit, so he might have a better sense of what may have affected this.
Comment 9 Diego 2014-12-26 14:39:43 UTC
I opened a new bug.
https://bugzilla.libsdl.org/show_bug.cgi?id=2827
Comment 10 Ryan C. Gordon 2015-02-19 04:27:47 UTC
(In reply to Eric wing from comment #8)
> - Try to get Ryan's attention. The commit that broke it seems to be the
> Emscripten commit. It's kind of a large commit, so he might have a better
> sense of what may have affected this.

We fixed the slowdown in Bug #2827 (Emscripten did something iOS disliked in the GLES renderer).

I haven't looked at this patch, but I'll put it on the triage list for 2.0.4.

--ryan.
Comment 11 Ryan C. Gordon 2015-02-19 05:22:16 UTC
Marking a large number of bugs with the "triage-2.0.4" keyword at once. Sorry if you got a lot of email from this. This is to help me sort through some bugs in regards to a 2.0.4 release. We may or may not fix this bug for 2.0.4, though!
Comment 12 Alex Szpakowski 2015-02-19 05:41:57 UTC
FWIW this issue might also be fixed by my large iOS patch[1]. I've used SDL_iPhoneSetAnimationCallback without issue when running the code in my patch.

[1]: https://bugzilla.libsdl.org/show_bug.cgi?id=2798
Comment 13 Diego 2015-02-19 15:10:58 UTC
I've tested Alex's patch. This bug is fixed with Alex's patch.
Comment 14 Ryan C. Gordon 2015-04-07 02:13:11 UTC
(In reply to Diego from comment #13)
> I've tested Alex's patch. This bug is fixed with Alex's patch.

Okay, I'm putting Alex's patch in for a number of unrelated reasons; Eric, if there's still something in your fix I should grab, let me know, otherwise, I'll resolve this bug soon.

Thanks!

--ryan.
Comment 15 Eric wing 2015-04-07 13:08:27 UTC
If Alex's latest patch removes all the special handling launch screen code like this does, then I think we're good. (I was under that impression that he went this direction in the end.)
Comment 16 Diego 2015-04-07 13:58:20 UTC
I'm using Alex's branch of SDL for iOS. It does fix this problem.
Comment 17 Diego 2015-04-07 21:40:42 UTC
Well, it did. Alex merged the default branch into his branch yesterday and it no longer fixes this problem. I don't know the specifics of Alex's patch with regards to why his branch is no longer working. Hopefully Alex can comment.
Comment 18 Alex Szpakowski 2015-04-07 22:15:27 UTC
Created attachment 2108 [details]
Modified happy.c demo which uses SDL_iPhoneSetAnimationCallback

I can't reproduce the crash (using my branch, post-merge.) I've attached my modified happy.c which uses SDL_iPhoneSetAnimationCallback but doesn't crash for me.

I'm using Xcode 6.2, and I've tested on the iOS simulator (simulating an iPhone 4s running iOS 8.2), and on my physical iPhone 6 running iOS 8.2. I tested using a Launch Screen nib as well as the SDL launch image.

I haven't tested using the main SDL branch code yet.
Comment 19 Diego 2015-04-07 23:40:36 UTC
Created attachment 2109 [details]
Modified crashing happy.c with SDL_iPhoneSetAnimationCallback

The program must return from the main function to use the iOS animation run loop that SDL_iPhoneSetAnimationCallback engages. The attached happy.c does this and crashes on your newest merge, but not pre newest merge.
Comment 20 Alex Szpakowski 2015-04-07 23:53:58 UTC
Your attached code doesn't crash for me either. What Xcode version / SDK / iOS version are you using? Are you running the SDL code from my branch unmodified? (e.g. a direct download of https://bitbucket.org/slime73/sdl-experiments/get/iOS-improvements.zip )


(In reply to Diego from comment #19)
> The program must return from the main function to use the iOS animation run
> loop that SDL_iPhoneSetAnimationCallback engages.

SDL_PollEvent calls SDL_PumpEvents which executes the run loop that calls the callback. So both our methods are valid ways to use SDL_iPhoneSetAnimationCallback, since my code calls SDL_PollEvent regularly.

Your method will continuously build up events in SDL's event queue without popping any though, since you never call SDL_PollEvent or SDL_FlushEvent but the run loop will keep pushing events to SDL's event queue.
Comment 21 Diego 2015-04-08 03:08:52 UTC
Please excuse my ignorance of Bitbucket. I was unknowingly downloading the default branch instead of your iOS-improvements branch. Everything works well in your branch.
I'm sorry if I caused you any trouble. Thanks for working with me!

I never knew you could use SDL_iPhoneSetAnimationCallback like that.

I purposefully didn't add SDL_PollEvent for the simplicity of the demo.
Comment 22 Alex Szpakowski 2015-04-08 03:15:17 UTC
(In reply to Diego from comment #21)
> I'm sorry if I caused you any trouble. Thanks for working with me!

No problem! More testing can only be a good thing, in my opinion. :)
Comment 23 Ryan C. Gordon 2015-04-10 02:45:21 UTC
This bug should be fixed by the changes made for Bug #2798. Please reopen if that's not the case.

Thanks!

--ryan.