luisespinoza / LEColorPicker

A Cocoa-Touch system for iOS to get a color scheme in function of an image, like iTunes 11 does.
Other
648 stars 54 forks source link

Crashes when application is backgrounded #5

Closed rweichler closed 11 years ago

rweichler commented 11 years ago

When you try run the color picker while the app is backgrounded, a gpus_ReturnNotPermittedKillClient gets thrown, which is when OpenGL actions are triggered while an app is backgrounded.

luisespinoza commented 11 years ago

Thanks Again!, this is a very important issue. Just one question (to know how to reproduce the crash): are you periodically requesting color to LEColorPicker right?.

rweichler commented 11 years ago

Yeah, once every 3-4 minutes, like when I'm playing a song in the background and it switches to another song (and thus fetches the colors for its artwork), BOOM it crashes. At first I had no idea what it was but I eventually figured out it was from LEColorPicker. I'd just raise an exception or something if the user tries to pick colors and explain to them that it can't be called in the background or something

luisespinoza commented 11 years ago

OK, thank you for the feedback. I will fix it ASAP

luisespinoza commented 11 years ago

@rweichler , I have another question. Which background mode is using your app?, which app delegate method is calling the LEColorPicker code?

rweichler commented 11 years ago

I'm not using any App Delegate method to call the LEColorPicker code. I call it when a song is playing that needs to get colors from its album artwork.

On Fri, Aug 30, 2013 at 6:16 AM, Luis Enrique Espinoza Severino < notifications@github.com> wrote:

@rweichler https://github.com/rweichler , I have another question. Which background mode is using your app?, which app delegate method is calling the LEColorPicker code?

— Reply to this email directly or view it on GitHubhttps://github.com/luisespinoza/LEColorPicker/issues/5#issuecomment-23560095 .

alextrob commented 11 years ago

I'm getting the same issue, only when running on device.

In my case, the background color of a UITableViewCell is set (asynchronously) with LEColorPicker. So if I do a big scroll on the table view and press the home button to put the app in the background while it's still scrolling, it pretty much immediately crashes in the background on [LEColorPicker setupContext] (line 304).

luisespinoza commented 11 years ago

Ok @rweichler

luisespinoza commented 11 years ago

@alextrob thanks for the feedback. That will be easy for testing. The crash happens because apps should not use the GPU in background, as Apple docs explain. Since LEColorPicker uses GPU, is crashing your app. Sorry for the inconvenience. I hope to fix this before Sunday.

rweichler commented 11 years ago

Are you going to save all of the blocks in an array and call them when the Application re-enters the background or just throw an exception?

luisespinoza commented 11 years ago

Hi!, I've updated LEColorPicker trying to fix this issue. Please, try the code in branch "hotfix-issue5". I'll be waiting for your feedback before merge this changes into master.

Good Night

alextrob commented 11 years ago

I was still getting crashes at first, but realized it was because the notification observers weren't being removed. Once I added the following to LEColorPicker.m I stopped getting crashes.

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:self name:UIApplicationWillResignActiveNotification object:nil];
    [[NSNotificationCenter defaultCenter] removeObserver:self name:UIApplicationDidEnterBackgroundNotification object:nil];
    [[NSNotificationCenter defaultCenter] removeObserver:self name:UIApplicationWillEnterForegroundNotification object:nil];
}
luisespinoza commented 11 years ago

Sorry, I forgot. I will add those lines tonight. Thanks!

luisespinoza commented 11 years ago

@alextrob @rweichler correction done. Waiting for feedback. Try branch hotfix-issue5.

alextrob commented 11 years ago

Looking good for me so far. Nice work :)

luisespinoza commented 11 years ago

@alextrob @rweichler Changes merged. Thanks again for your input!