winit icon indicating copy to clipboard operation
winit copied to clipboard

Android: `request_redraw()` blocks `send_event()`

Open Rodrigodd opened this issue 3 years ago • 1 comments

In android, Window::request_redraw() can block UserEvents send by EvenLoopProxy::send_event to reach the event loop callback.

I think this happens because of the way that request_redraw is injected in the Looper. In request_redraw the request is store in the static INTERNAL_EVENT, and the Looper is wakened:

https://github.com/rust-windowing/winit/blob/4dd2b66aaa3a338d7569d108444da609ee675b43/src/platform_impl/android/mod.rs#L676-L679

And in Poll::Wake, the request is injected in the Looper, taking priority over EventSource::User:

https://github.com/rust-windowing/winit/blob/4dd2b66aaa3a338d7569d108444da609ee675b43/src/platform_impl/android/mod.rs#L220-L226

And send_event also wakes up the Looper:

https://github.com/rust-windowing/winit/blob/4dd2b66aaa3a338d7569d108444da609ee675b43/src/platform_impl/android/mod.rs#L583-L587

This means that if both calls to loop.wake() creates a single Poll::Wake, request_event will always be blocking send_event (as happens in my device).

Reproducing the issue

I can reproduce this bug by adding the following example to the examples folder: https://gist.github.com/Rodrigodd/379a24ea39c6bead98a8b5dad5e0753e

And running in android, as instructed in the README.md.

// add this to Cargo.toml
[[example]]
name = "android_send_event_block"
crate-type = ["cdylib"]
cargo apk run --example android_send_event_block
adb logcat RustStdoutStderr:D *:S

Reproducible on Android 11.

Rodrigodd avatar May 22 '22 21:05 Rodrigodd

For reference, I recently updated the winit Android backend to support a different glue layer that I've been working on and as part of that I re-worked how redraws were queued since I was also concerned with the current approach.

Here's a link to what I did there: https://github.com/rib/winit/blob/8adc3fe461ce78202b4464bfb9bddebd505721c4/src/platform_impl/android/mod.rs#L255

Actually I wonder if a similar pattern could also be adopted in other backends, since I e.g. saw that the X backend uses and mpsc queue for redraw requests which doesn't really seem appropriate for something that needs to act more like a flag.

I think that solution could potentially be ported across to the current backend and might fix this issue.

rib avatar May 29 '22 14:05 rib