Skip to content

Handle android activity life cycle and event loop#868

Merged
tomaka merged 1 commit intorust-windowing:masterfrom
MortimerGoro:android_improvements
Mar 6, 2018
Merged

Handle android activity life cycle and event loop#868
tomaka merged 1 commit intorust-windowing:masterfrom
MortimerGoro:android_improvements

Conversation

@MortimerGoro
Copy link
Copy Markdown
Contributor

Handle android activity life cycle and event loop

Related PR: rust-windowing/winit#153

@MortimerGoro
Copy link
Copy Markdown
Contributor Author

I did a rebase and adapted the code to the new EventLoop API

@MortimerGoro
Copy link
Copy Markdown
Contributor Author

@tomaka I see that the API has changed and EventLoop has been moved to winit. Now I can't create the EventLoop override to handle EGL life cycle events on Android:

  • If I create a EventsLoop override in Glutin (like this PR did), it can't be used for the glutin::GlWindow::new(window, gl_context, &events_loop).unwrap()creation because the argument uses winit::EventLoop type
  • If I create a EventsLoop override in Winit, I don't have access to the gl_context defined in winit.

We need the android life cycle merged to unfork glutin in Servo. What's the best way to do this now?

@tomaka
Copy link
Copy Markdown
Contributor

tomaka commented Aug 1, 2017

Err I knew there would be a quirk with exposing winit.

@tomaka
Copy link
Copy Markdown
Contributor

tomaka commented Aug 1, 2017

I thought a bit about this, and considered the following solutions:

  • Letting the user handle the situation explicitly when they receive a Suspend event.
  • Wrapping around the EventsLoop again.
  • Try automatically detecting this in make_current and is_current.

But in the end I think the best thing to do is make a tweak to the winit's EventsLoop to make it possible for glutin to register a callback when a suspend event happens.
This is a bit of a hack, but I think that it doesn't have any downside.

The solution would be specific to Android (with a trait hidden with #[cfg(target_os = "android")]) and it would only be possible for suspend events.
Do you think that's ok?

@MortimerGoro
Copy link
Copy Markdown
Contributor Author

MortimerGoro commented Aug 1, 2017

Yes, I though doing a similar thing too but with all events. Doing with suspend event only is ok. I don't think that we'll need to handle other events directly in Glutin.

@paulrouget
Copy link
Copy Markdown
Contributor

@tomaka Is what you describe here #868 (comment) still the approach you recommend?

@tomaka
Copy link
Copy Markdown
Contributor

tomaka commented Feb 8, 2018

@paulrouget The discussion has more or less moved there: rust-windowing/winit#231
There is still no good solution for this right now.

@paulrouget
Copy link
Copy Markdown
Contributor

@MortimerGoro winit 0.11.1 has been published.

@MortimerGoro
Copy link
Copy Markdown
Contributor Author

@paulrouget thanks for the notfication. PR updated!

@tomaka We need to update winit dependency from 10.x to 11.x here. Do you want a separate PR for that change?

@tomaka
Copy link
Copy Markdown
Contributor

tomaka commented Feb 22, 2018

@MortimerGoro Yes please.

@MortimerGoro MortimerGoro force-pushed the android_improvements branch 2 times, most recently from 8ab7897 to 7507ce2 Compare February 28, 2018 19:38
@MortimerGoro
Copy link
Copy Markdown
Contributor Author

@tomaka r? PR ready

@tomaka
Copy link
Copy Markdown
Contributor

tomaka commented Mar 6, 2018

Thanks, looks good except for a few style issues.

@tomaka tomaka merged commit af67d29 into rust-windowing:master Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants