Skip to content

input: Split out InputEventJava into Motion and Key with Deref #503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2025

Conversation

MarijnS95
Copy link
Member

Fixes #502

When I implemented the from_java() constructors for MotionEvent and KeyEvent in #456, I used a single InputEventJava wrapper since that could wrap the existing enum InputEvent and only need a single destructor calling AInputEvent_release() (which must only be called when the input event was created from Java).

This however requires existing callers to MotionEvent::from_java() and KeyEvent::from_java() to unwrap/unpack that nested enum InputEvent again in order to get access to the underlying native methods, despite already calling a specific constructor method (since Android makes a distinction between both types).

Not that that is even reachable, since the nested InputEvent member was private and there was no Deref into &InputEvent anywhere making it impossible to use this API in any meaningful way.

Solve both issues by splitting the struct into a Motion and Key variant, and implement Deref on both to their respective type. No wrapper InputEventJava remains since there does not appear to be any reason to pass both of these types into a single API.

@MarijnS95 MarijnS95 requested a review from rib April 25, 2025 16:11
@MarijnS95 MarijnS95 added this to the 0.10.0 release milestone Apr 25, 2025
@MarijnS95
Copy link
Member Author

I'm going to refactor this PR a little bit further, at least to remove the Java moniker and change it to a proper *Ref type (as we should also do with some other types: #309). As it turns out at least SurfaceControl's new AInputReceiver (that I'm adding on top of #499 / #500) callbacks are documented to require a call to AInputEvent_release() on the key/motion events that these callbacks receive:

https://developer.android.com/ndk/reference/group/native-activity#ainputreceiver_onkeyevent
https://developer.android.com/ndk/reference/group/native-activity#ainputreceiver_onmotionevent

When I implemented the `from_java()` constructors for `MotionEvent`
and `KeyEvent` in #456, I used a single `InputEventJava` wrapper since
that could wrap the existing `enum InputEvent` and only need a single
destructor calling `AInputEvent_release()` (which must only be called
when the input event was created from Java).

This however requires existing callers to `MotionEvent::from_java()` and
`KeyEvent::from_java()` to _unwrap_/unpack that nested `enum InputEvent`
again in order to get access to the underlying native methods, despite
already calling a specific constructor method (since Android makes a
distinction between both types).

Not that that is even reachable, since the nested `InputEvent` member
was private and there was no `Deref` into `&InputEvent` anywhere making
it impossible to use this API in any meaningful way.

Solve both issues by splitting the `struct` into a `Motion` and `Key`
variant, and implement `Deref` on both to their respective type.  No
wrapper `InputEventJava` remains since there does not appear to be any
reason to pass both of these types into a single API.
@MarijnS95 MarijnS95 force-pushed the input-event-java-make-reachable branch from 20fc5e6 to 8b9f422 Compare June 23, 2025 20:45
@MarijnS95 MarijnS95 added the impact: breaking API/ABI-breaking change label Jun 23, 2025
@MarijnS95 MarijnS95 merged commit 69bb80d into master Jun 23, 2025
38 checks passed
@MarijnS95 MarijnS95 deleted the input-event-java-make-reachable branch June 23, 2025 20:48
@MarijnS95
Copy link
Member Author

I may or may not end up renaming it in a followup. AInputEvent_release() still documents it specifically takes Java Key/MotionEvents, so perhaps these callbacks specifically wrap Java components. After all, the necessary AInputTransferToken is also specific to Java-land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: breaking API/ABI-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate InputEventJava into Key/Motion Java structs to Deref into (and provide methods on) their native variants
1 participant