Android: Change default implementation to ignore volume keys and let operating system handle them

[Hoodad]

The Issue

Current Android implementation will always mark the volume keys as handled by the application (window) overriding the default operating system behavior. This is a bit unexpected I would argue the opposite would be expected as default, to opt in to override default operating system behavior.

The Change

1. By default volume keys Volume Up/Down & Mute will all be marked as unhandled by the application. Letting the operating system handle them.
2. Added a new function EventLoopBuilderExtAndroid::handle_volume_keys for applications that want to manually handle the volume keys.

Remarks

This could be considered a breaking change from a users point of view, even though I would argue this is the "correct" default. So I'm not sure how or if that should be reflected in the CHANGELOG.md.

This PR is heavily inspired by the PR #1919

• Tested on all platforms changed
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

How to make the app handle volume keys

#[cfg(target_os = "android")]
use winit::platform::android::activity::AndroidApp;

#[cfg(target_os = "android")]
#[no_mangle]
fn android_main(app: AndroidApp) {
    use winit::platform::android::EventLoopBuilderExtAndroid;

    android_logger::init_once(android_logger::Config::default().with_min_level(log::Level::Trace));

    let event_loop = EventLoopBuilder::with_user_event()
        .with_android_app(app)
        .handle_volume_keys()   // New function
        .build();
    _main(event_loop);
}

[rib]

Yeah, this seems reasonable to me.

By default it's very unlikely an application wants to intercept the volume keys so this is pretty terrible behaviour atm.

It seems like ideally Winit would also support the notion that an application might not handle a key event and would have a way for apps to give that feedback which we could just forward to android-activity instead of us making assumptions or needing extension APIs like this. Without that though - this makes sense to me.

One thing I'm not 100% sure of atm is the status of any embargo from wanting to land the Keyboard 2.0 changes in Winit? Would we be able to land a change like this atm @kchibisov / @msiglreith ?

I think the Keyboard 2.0 changes need some reworking for Android anyway though, so I tend to think it would be reasonable to land this now.

[kchibisov]

It seems like ideally Winit would also support the notion that an application might not handle a key event and would have a way for apps to give that feedback which we could just forward to android-activity instead of us making assumptions or needing extension APIs like this. Without that though - this makes sense to me.

Use EventLoopBuilderExtAndroid to add such a logic to it(Or on a window, I don't know at which point you should opt-in/out). On other platforms there's no such thing in general, and it's either forwarded to you or caught by compositor, there's though, a special protocol for keyboard input grabbing used by virtual machines guis(qemu), which captures everything and allows set of binding to work, but I'll go into implementing it not soon.

To sum up, for now abuse the extensions for Android, they should just work for you.

One thing I'm not 100% sure of atm is the status of any embargo from wanting to land the Keyboard 2.0 changes in Winit? Would we be able to land a change like this atm @kchibisov /
@msiglreith ?

I general I wanted to merge the keyboard input v2 PR, given that it has some approvals, was used for a long time in some projects, and had only minor issues which were trivial to resolve(or look trivial to resolve). However the things got stalled due to MIA of its authors and I can't take over it myself really.

I think the Keyboard 2.0 changes need some reworking for Android anyway though, so I tend to think it would be reasonable to land this now.

As long as you can submit android patches for keyboard v2 yourself you can merge stuff. I'll do something like that with Wayland anyway.

P.s.

I swear I've replied to it, but I have no clue where my reply is, given that you like post similar things on multiple issues(which is good, but I listen to all the issue and process them in a queue) I might already replied to you in the past.

[rib]

It seems like ideally Winit would also support the notion that an application might not handle a key event and would have a way for apps to give that feedback which we could just forward to android-activity instead of us making assumptions or needing extension APIs like this. Without that though - this makes sense to me.

Use EventLoopBuilderExtAndroid to add such a logic to it(Or on a window, I don't know at which point you should opt-in/out). On other platforms there's no such thing in general, and it's either forwarded to you or caught by compositor, there's though, a special protocol for keyboard input grabbing used by virtual machines guis(qemu), which captures everything and allows set of binding to work, but I'll go into implementing it not soon.

Yeah, as you say other platforms don't generally let apps return a filter status like on Android, so returning a status probably wouldn't be a great fit for other platforms.

This kind of BuilderExt extension should be fine for addressing this I think.

It's surely pretty uncommon to want to change the behaviour of your volume keys, so it makes sense that this PR changes the default behaviour but still provides a way to intercept the volume keys if really needed.

You could imagine some situations where you might want to be able to change this dynamically but we could add an extension for that later if necessary.

I think the Keyboard 2.0 changes need some reworking for Android anyway though, so I tend to think it would be reasonable to land this now.

As long as you can submit android patches for keyboard v2 yourself you can merge stuff. I'll do something like that with Wayland anyway.

Yeah, I'd be happy to provide updated Android changes for the keyboard 2.0 work if needed.

P.s.

I swear I've replied to it, but I have no clue where my reply is, given that you like post similar things on multiple issues(which is good, but I listen to all the issue and process them in a queue) I might already replied to you in the past.

Right, sorry if I missed something. Was it something about Keyboard 2.0 changes? I haven't really followed the Keyboard 2.0 work that closely. The only thing I can really recall commenting on related to keyboard 2.0 was the Android code that surprisingly dug into native-activity and game-activity internal details for KeyEvent. I left a commented about 3 weeks ago under https://github.com/rust-windowing/winit/pull/2662/files/4a566b72bcd8a4b7fefff3f278f0c0b485e899b0#diff-9169a22d6397a250be741006cd857b8a575f804c74cc07e3a4fb8f3341606d9b (src/platform_impl/android/mod.rs)

My impression was that it should be (hopefully) straight forward to address this - and I don't mind doing that at some point if there's a consensus about landing this PR.

[msiglreith]

On Windows there is a similar use case regarding handling of system keybinds (Alt keys) where reporting if we want to handle these or not would be interesting. That's why I'm hesitant on this as it feels more a like a band-aid then a permanent solution to it.
On the other side it's also tricky to shove the responsiblity to the user to know what are system key commands.

[kchibisov]

The thing is that we'd likely want keyboard v2 to have a way to pass some identity of the key events to the winit or so.

Is windows have a per-key basis for passthrough, or is it more like all or nothing?

[msiglreith]

Is windows have a per-key basis for passthrough, or is it more like all or nothing?

Windows would be on a per key basis similar to Android. Looking at the v2 branch, a key event extension might be useful.

[Hoodad]

I'm not aware of what new functionality the new Keyboard 2.0 The PR does not describe what it does?

That's why I'm hesitant on this as it feels more a like a band-aid then a permanent solution to it.
On the other side it's also tricky to shove the responsiblity to the user to know what are system key commands.

I personally feel that the new functionality introduced here is so limited in scope that it can easily be removed if it turns out that Keyboard 2.0 does indeed solve this functionality later on. And having the ability to configure more advanced use-cases using a BuilderExt feels like a natural step of making the API more mature, imho.

[kchibisov]

Keyboard v2 defines key related functions and key codes in crossplatform way, so you can at least use them to pass into window builder.

[Hoodad]

If it makes ppl more at ease I can also remove the .handle_volume_keys() and just have the code always mark the buttons as unhandled which is the preferred default. This is where I started my work but figured it would be bad practice to remove functionality with having a way to opt-in to it...
If that is the preferred way when the Keyboard 2.0 gets in it may become possible to mark the buttons as handled somehow (maybe with a little bit of work).

[rib]

I still tend to think that this change is pretty much reasonable as is - even though I also raised the idea that maybe it would be ideal if Winit had a way for apps to report when a key was handled or not.

The default behaviour for volume keys is basically broken for most Android apps currently, and this PR addresses that.

An Android-specific Builder extension to just preserve a way to access the previous behaviour seems reasonable too. This kind of extension would be easy to drop in the future if a portable API for reporting the status was added in the future. (its most likely that nothing will ever even use the extension)

Seems a bit like one of those "don't let perfect be the enemy of good" things. Ideally we wouldn't need to wait for keyboard 2.0 or a redesign of the input APIs to address this issue in Android for now.

[msiglreith]

fine for me as we also have a rough outline how to adjust it in the future once keyboard v2 lands

[Hoodad]

fine for me as we also have a rough outline how to adjust it in the future once keyboard v2 lands

Yeah when that lands changes are expected.

Seeing that it has been reviewed, what is the process for this to be merged? I also see that a few of the checks are failing due to unrelated things.

I'm also unsure if this counts as a breaking change or not. I would in this case not view it as a breaking change as this behavior is the expected default behavior. But if you rely on having the audio be handled by the app this is definitely a change you will notice as it may break the experience...

[msiglreith]

sry for the delay! With the keyboard event refactoring merged would you mind rebasing?

[Hoodad]

@msiglreith I just rebased now

[MarijnS95]

By default it's very unlikely an application wants to intercept the volume keys

I wouldn't call it unlikely, we rely on it for very simple use-cases.

Unfortunately I only got pinged about this just now that #1919 was closed. Wasn't keyboard v2 (which is also now merged) supposed to fix this proper?

[kchibisov]

Well, properly is more like, we have a way to refer to keys now, it's just default is flipped, so you can opt-in.

[Hoodad]

@MarijnS95 in your case you would need to specify the following when creating the event loop.

let event_loop = EventLoopBuilder::with_user_event()
        .with_android_app(app)
        .handle_volume_keys()   // Add this line to make your app handle the volume keys
        .build();

Then things should work as they did before this change. Or do you for see that this will cause any other problems?

[MarijnS95]

@Hoodad I know what to do to fix it, I just don't like having to fix it like that.

But that's part of a bigger issue where we can supposedly still not ad-hoc decide whether or not to handle a winit input event? Sometimes I'd like to pass things through, other times I'd like to mark things as handled.

[kchibisov]

@MarijnS95 can't we have an extension on a KeyEvent now for android handling your issue? I'm sure you could sort of communicate back or provide a way to set a key_event_filter mask listing virtual codes, so you passthrough and could adjust this filter at the runtime.