Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using SDL_iPhoneSetAnimationCallback and Launch Screen causes crash #1728

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1
Reported for operating system, platform: iOS (All), iPhone/iPod touch

Comments on the original bug report:

On 2014-12-24 04:35:22 +0000, Diego wrote:

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.

On 2014-12-24 08:46:11 +0000, Eric wing wrote:

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

On 2014-12-24 15:08:11 +0000, Diego wrote:

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.

On 2014-12-24 17:51:55 +0000, Eric wing wrote:

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.

On 2014-12-24 20:31:27 +0000, Diego wrote:

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.

On 2014-12-24 20:33:12 +0000, Diego wrote:

Also slows down with or without SDL_iPhoneSetAnimationCallback

On 2014-12-25 01:33:01 +0000, Eric wing wrote:

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.)

On 2014-12-25 03:29:29 +0000, Diego wrote:

Good thinking. I can confirm the slow down happened after commit 8900afb. f0bd407 is good. 8900afb is slow.

On 2014-12-25 20:28:56 +0000, Eric wing wrote:

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.

On 2014-12-26 14:39:43 +0000, Diego wrote:

I opened a new bug.
https://bugzilla.libsdl.org/show_bug.cgi?id=2827

On 2015-02-19 04:27:47 +0000, Ryan C. Gordon wrote:

(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.

On 2015-02-19 05:22:16 +0000, Ryan C. Gordon wrote:

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!

On 2015-02-19 05:41:57 +0000, Alex Szpakowski wrote:

FWIW this issue might also be fixed by my large iOS patch1. I've used SDL_iPhoneSetAnimationCallback without issue when running the code in my patch.

On 2015-02-19 15:10:58 +0000, Diego wrote:

I've tested Alex's patch. This bug is fixed with Alex's patch.

On 2015-04-07 02:13:11 +0000, Ryan C. Gordon wrote:

(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.

On 2015-04-07 13:08:27 +0000, Eric wing wrote:

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.)

On 2015-04-07 13:58:20 +0000, Diego wrote:

I'm using Alex's branch of SDL for iOS. It does fix this problem.

On 2015-04-07 21:40:42 +0000, Diego wrote:

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.

On 2015-04-07 22:15:27 +0000, Alex Szpakowski wrote:

Created attachment 2108
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.

On 2015-04-07 23:40:36 +0000, Diego wrote:

Created attachment 2109
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.

On 2015-04-07 23:53:58 +0000, Alex Szpakowski wrote:

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.

On 2015-04-08 03:08:52 +0000, Diego wrote:

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.

On 2015-04-08 03:15:17 +0000, Alex Szpakowski wrote:

(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. :)

On 2015-04-10 02:45:21 +0000, Ryan C. Gordon wrote:

This bug should be fixed by the changes made for Bug # 2798. Please reopen if that's not the case.

Thanks!

--ryan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant