Compiling on windows for android errors #1886 #2118
Conversation
| Event::Pause => { | ||
| call_event_handler!( | ||
| event_handler, | ||
| self.window_target(), | ||
| control_flow, | ||
| event::Event::Suspended | ||
| ); | ||
| self.running = false; | ||
| } | ||
| Event::Resume => { | ||
| call_event_handler!( | ||
| event_handler, | ||
| self.window_target(), | ||
| control_flow, | ||
| event::Event::Resumed | ||
| ); | ||
| self.running = true; | ||
| } |
There was a problem hiding this comment.
Can you please add some information about why this is necessary, maybe with a few links to the Android docs? WindowDestroyed already sends Suspended, why is that not enough?
There was a problem hiding this comment.
rust-windowing/glutin#904
rust-windowing/glutin#1313
Check this comment:
rust-windowing/glutin#1313 (comment)
Android has some special windows context handling, where context cannot be resused after resuming. Data has to be reinitialized. Therefore we need to handle those events.
Old version just sets flags:
Event::Pause => self.running = false,
Event::Resume => self.running = true
There was a problem hiding this comment.
More discussion on this:
rust-mobile/ndk#117
There was a problem hiding this comment.
I doubt that is correct: https://developer.android.com/guide/components/activities/activity-lifecycle#onpause
There was a problem hiding this comment.
I doubt that is correct: https://developer.android.com/guide/components/activities/activity-lifecycle#onpause
Why? We need to handle pause and resume events on android side. The point is, no one can tell how long the app was paused, maybe it was send to background for several hours... We need to free graphics resources here. If the app resumes, it's possible that original window context is not valid anymore. We cannot keep rendering to that destroyed window.
Just settings flags is not an option. These both events are the point where graphics need to be handled. I've made an consumer of these events, that is working fine:
match event {
Event::Resumed => {
// ANDROID: only create if native window is available
#[cfg(target_os = "android")]
{
// enable immersive mode
enable_immersive();
// create graphics context (mabye it still exists)
if self.device_ctx.is_none() && ndk_glue::native_window().is_some() {
self.device_ctx = Some(DeviceContext::new(_event_loop));
if let Some(device_ctx) = self.device_ctx.as_mut() {
// call create device callback
self.runner.create_device(&device_ctx.gl);
}
}
}
// call resume callback
paused = false;
}
Event::Suspended => {
// call pause callback
paused = true;
// ANDROID: only destroy if native window is available
#[cfg(target_os = "android")]
{
if self.device_ctx.is_some() && ndk_glue::native_window().is_some() {
if let Some(device_ctx) = self.device_ctx.as_mut() {
// call destroy device callback
self.runner.destroy_device(&device_ctx.gl);
}
self.device_ctx = None;
}
}
}
...There was a problem hiding this comment.
The linked docs are pretty clear on this:
You can also use the onPause() method to release system resources, handles to sensors (like GPS), or any resources that may affect battery life while your activity is paused and the user does not need them. However, as mentioned above in the onResume() section, a Paused activity may still be fully visible if in multi-window mode. As such, you should consider using onStop() instead of onPause() to fully release or adjust UI-related resources and operations to better support multi-window mode.
There was a problem hiding this comment.
In other words: while I do agree it might be nice to forward these events through winit, the Suspended and Resumed events are not right as they serve a different purpose.
onPause and onResume should result in a WindowEvent::Focused.
Perhaps we should rename Suspended and Resumed to more closely represent the graphics state needing to be forcibly destroyed and recreated, respectively?
There was a problem hiding this comment.
I agree, maybe I abused these events to get my context handling correctly. The main point is, that we must not lose important android events. We need to be able to react on window changes.
When do resourced need to be created, when do they need to be destroyed.
And yes you are right, it would be much more coherent, if these events would be of type WindowEvent. Like i'm handling the resize event:
Event::WindowEvent { event, .. } => match event {
WindowEvent::Resized(physical_size) => {
if let Some(device_ctx) = self.device_ctx.as_mut() {
device_ctx.window_context.resize(physical_size);
// call resize device callback
self.runner.resize_device(
&device_ctx.gl,
physical_size.width,
physical_size.height,
);
}
}
....There was a problem hiding this comment.
I checked the android behaviour:
Focus can not be used to create the device, as i get following events when starting the main loop:
- resume
- resize
- focus
There is no window create event or such. The focus event is too late to use it for creation, as resize event is already executed.
There was a problem hiding this comment.
Focus can not be used to create the device
Nor is the android documentation suggesting to. Swapchain creation and deletion "must" be tied to window creation and destroying: you can do it whenever you like, as long as it is created after WindowCreated and destroyed before or during WindowDestroyed. Applications are free to perform this elsewhere, as long as they synchronize properly with the events (ie. hold on to the lock from native_window(), and destroy your resources followed by releasing this lock whenever WindowDestroyed is called).
There is no window create event or such.
At what level are you logging this? Is logging initialized early enough (ie. in ndk_glue::main)? If Android doesn't give your application a window there's nothing to render to.
My best guess is "resume" in your list is referring to event::Event::Resumed, ie. triggered on ndk_glue::Event::WindowCreated?
| Event::Destroy => { | ||
| let event = event::Event::WindowEvent { | ||
| window_id: window::WindowId(WindowId), | ||
| event: event::WindowEvent::CloseRequested, | ||
| }; | ||
| call_event_handler!( | ||
| event_handler, | ||
| self.window_target(), | ||
| control_flow, | ||
| event | ||
| ); | ||
| } |
There was a problem hiding this comment.
It seems wrong to send CloseRequested when the application is destroyed; maybe it should be the Destroyed event instead?
There was a problem hiding this comment.
If I get everthing correctly, there is no direct destroy in android. There is just a request to destroy the app. It's send to background, to make reopening faster. If there are not enough resources available, the app gets destroyed, but those events are handled by OS.
I will check this again!
There was a problem hiding this comment.
I checked this. I'm open to change this to WindowEvent::Destroyed. I think it's some sort of interpretation:
Is it asking for destroying or is it force destroying.
When android receives this event, render context is still alive and can be saved or released gracefully, therefore i thought it as "the window is requested to be closed". You can also see this as forced shutdown.
If this is the only problem, i will change it.
There was a problem hiding this comment.
Event::Destroy happens when the entire activity is destroyed (https://developer.android.com/guide/components/activities/activity-lifecycle#ondestroy), theoretically the window should have been destroyed prior, with Event::WindowDestroyed?
This is dispatched as a winit Suspended event though. Android''s Suspend/Pause and Resume events represent something different than winit's Suspended / Resumed (specific for Android backends iirc).
There was a problem hiding this comment.
It would be best, if we can strictly divide Event and WindowEvent. The questio is, if winit fires all those events correctly, also in android environment. Again, we must not lose any important events for graphics handling.
I will rework the complete event section for android. To make some nice mapping:
// starting app
Android: Start - Winit: ???
Android: Resume - Winit: ???
Android: InputQueueCreated - Winit: ???
Android: WindowCreated - Winit: ???
Android: WindowResized - Winit: ???
Android: ContentRectChanged - Winit: ???
Android: WindowHasFocus - Winit: ???
Android: WindowRedrawNeeded - Winit: ???
// send to background
Android: Pause - Winit: ???
Android: WindowLostFocus - Winit: ???
Android: SaveInstanceState - Winit: ???
Android: Stop - Winit: ???
Android: WindowDestroyed - Winit: ???
// close
Android: InputQueueDestroyed - Winit: ???
Android: Destroy - Winit: ???
There was a problem hiding this comment.
Again, we must not lose any important events for graphics handling.
As said it'd be great if this event is handled properly in winit, but we must not diverge from winit patterns too far. It is already a stretch to have Suspended and Resumed (whether their naming is useful/accurate is still up for debate) as this makes winit applications for other platforms not immediately runnable on Android without refactoring swapchain-lifetime logic to adhere to these events.
(Tangent: theoretically it'd be nice if every platform could emit these events for swapchain creation and destruction, making the whole thing more generic. Even better: we provide the RawWindowHandle only in the Resumed [or otherwise named] (Window)Event so that crates are forced to handle this event instead of calling fn raw_window_handle at will.)
I think it's some sort of interpretation:
Is it asking for destroying or is it force destroying.
Regarding this prior question: Whenever this function/event is called, you are supposed to release all resources related to the window (again: the swapchain) and return from this function (ndk_glue does that after the lock from fn native_window() is released, presuming the caller read the documentation and is already holding on to this lock while they have resources created on the window). In that sense you could keep using and rendering to the swapchain without ever returning from on_window_destroyed, but I don't know what Android will do in this case (it's already out of focus and most likely also out of view, so it'll probably result in an ANR, and no new events will arrive).
|
It should be noted, I don't know anything about Android and haven't actually read the linked issues, so maybe you've already answered the comments there. |
|
I removed the duplicated run_return function. You're right, it can be easily aquired by using the trait. Currently the biggest Problem is, that pause and resume need to be handled on android side, graphics data need to be reinitialized. Currently these events are lost. |
Removed empty line
|
Does this have a future? I also made PRs, but simpler #2297 |
|
I don't think so. We could pick up some of the review comments in smaller followup steps but this seems to have gone stale. |
|
I've picked up this If someone wishes to proceed with the suggested renaming of events to be more in-line with Android naming and/or make it more consistent with other platforms, please open a new PR which clearly states that intent. Feel free to also add proper events for |
cargo fmthas been run on this branchcargo docbuilds successfullyCHANGELOG.mdif knowledge of this change could be valuable to usersThis merge request adds functionality to implement a working solution for glutin on android.
Everthing starts here (Glutin build failure for android targets)
rust-windowing/glutin#1307
I fixed this, by customizing winit and glutin:
https://github.com/Kaiser1989/winit
https://github.com/Kaiser1989/glutin
Working examples with both fixes can be found here:
https://github.com/Kaiser1989/rust-android-example
https://github.com/Kaiser1989/game-gl
Several messages arrived me, that i should pls try to merge my changes to the original crates, so these projects can stay up-to-date.
PS: Do not check the single commits, just check the 3 files. There aren't that much changes, mainly adding resume and suspend events, and brining back run_return function.
Best regards
Philipp