Skip to content

Commit

Permalink
Improve forwards compatibility of input API
Browse files Browse the repository at this point in the history 
This adds a `#[doc(hidden)]` `__Unknown(u32)` variant to the various
enums to keep them extensible without requiring API breaks.

We need to consider that most enums that are based on Android SDK enums
may be extended across different versions of Android (i.e. effectively
at runtime) or extended in new versions of `android-activity` when we
pull in the latest NDK/SDK constants.

In particular in the case that there is some unknown variant we at least
want to be able to preserve the integer value to allow the values to be
either passed back into the SDK (it doesn't always matter whether we
know the semantics of a variant at compile time) or passed on to
something downstream that could be independently updated to know the
semantics.

We don't want it to be an API break to extend these enums in future
releases of `android-activity`.

It's not enough to rely on `#[non-exhaustive]` because that only really
helps when adding new variants in sync with android-activity releases.

On the other hand we also can't rely on a catch-all `Unknown(u32)` that
only really helps with unknown variants seen at runtime. (If code were
to have an exhaustive match that would include matching on `Unknown(_)`
values then they wouldn't be compatible with new versions of
android-activity that would promote unknown values to known ones).

What we aim for instead is to have a hidden catch-all variant that is
considered (practically) unmatchable so code is forced to have a
`unknown => {}` catch-all pattern match that will cover unknown variants
either in the form of Rust variants added in future versions or in the
form of an `__Unknown(u32)` integer that represents an unknown variant
seen at runtime.

Any `unknown => {}` pattern match can rely on `IntoPrimitive` to convert
the `unknown` variant to the integer that comes from the Android SDK in
case that values needs to be passed on, even without knowing it's
semantic meaning at compile time.

Instead of adding an `__Unknown(u32)` variant to the `Class` enum though
this enum has been removed in favour of adding methods like
`is_button_class()` and `is_pointer_class()` to the `Source` type, since
the class flags aren't guaranteed to be mutually exclusive and since
they are an attribute of the `Source`.

This removes some reliance `try_into().unwrap()` that was put in place
anticipating that we would support `into()` via `num_enum`, once we
could update our rust-version.
  • Loading branch information
@rib
rib committed Oct 15, 2023
1 parent a291e37 commit 37dc94a
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 116 deletions.
35 changes: 11 additions & 24 deletions android-activity/src/game_activity/input.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use std::convert::TryInto;

use crate::activity_impl::ffi::{GameActivityKeyEvent, GameActivityMotionEvent};
use crate::input::{
Axis, ButtonState, Class, EdgeFlags, KeyAction, KeyEventFlags, Keycode, MetaState,
MotionAction, MotionEventFlags, Pointer, PointersIter, Source, ToolType,
Axis, ButtonState, EdgeFlags, KeyAction, KeyEventFlags, Keycode, MetaState, MotionAction,
MotionEventFlags, Pointer, PointersIter, Source, ToolType,
};

// Note: try to keep this wrapper API compatible with the AInputEvent API if possible
Expand Down Expand Up @@ -50,14 +50,7 @@ impl<'a> MotionEvent<'a> {
#[inline]
pub fn source(&self) -> Source {
let source = self.ga_event.source as u32;
source.try_into().unwrap_or(Source::Unknown)
}

/// Get the class of the event source.
///
#[inline]
pub fn class(&self) -> Class {
Class::from(self.source())
source.into()
}

/// Get the device id associated with the event.
Expand All @@ -73,7 +66,7 @@ impl<'a> MotionEvent<'a> {
#[inline]
pub fn action(&self) -> MotionAction {
let action = self.ga_event.action as u32 & ndk_sys::AMOTION_EVENT_ACTION_MASK;
action.try_into().unwrap()
action.into()
}

/// Returns the pointer index of an `Up` or `Down` event.
Expand Down Expand Up @@ -275,7 +268,8 @@ impl<'a> PointerImpl<'a> {
#[inline]
pub fn axis_value(&self, axis: Axis) -> f32 {
let pointer = &self.event.ga_event.pointers[self.index];
pointer.axisValues[axis as u32 as usize]
let axis: u32 = axis.into();
pointer.axisValues[axis as usize]
}

#[inline]
Expand All @@ -294,7 +288,7 @@ impl<'a> PointerImpl<'a> {
pub fn tool_type(&self) -> ToolType {
let pointer = &self.event.ga_event.pointers[self.index];
let tool_type = pointer.toolType as u32;
tool_type.try_into().unwrap()
tool_type.into()
}
}

Expand Down Expand Up @@ -662,14 +656,7 @@ impl<'a> KeyEvent<'a> {
#[inline]
pub fn source(&self) -> Source {
let source = self.ga_event.source as u32;
source.try_into().unwrap_or(Source::Unknown)
}

/// Get the class of the event source.
///
#[inline]
pub fn class(&self) -> Class {
Class::from(self.source())
source.into()
}

/// Get the device id associated with the event.
Expand All @@ -685,13 +672,13 @@ impl<'a> KeyEvent<'a> {
#[inline]
pub fn action(&self) -> KeyAction {
let action = self.ga_event.action as u32;
action.try_into().unwrap()
action.into()
}

#[inline]
pub fn action_button(&self) -> KeyAction {
let action = self.ga_event.action as u32;
action.try_into().unwrap()
action.into()
}

/// Returns the last time the key was pressed. This is on the scale of
Expand Down Expand Up @@ -721,7 +708,7 @@ impl<'a> KeyEvent<'a> {
#[inline]
pub fn key_code(&self) -> Keycode {
let keycode = self.ga_event.keyCode as u32;
keycode.try_into().unwrap_or(Keycode::Unknown)
keycode.into()
}

/// Returns the number of repeats of a key.
Expand Down
2 changes: 2 additions & 0 deletions android-activity/src/game_activity/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,12 @@ impl AndroidAppInner {
}

pub fn enable_motion_axis(&mut self, axis: Axis) {
let axis: u32 = axis.into();
unsafe { ffi::GameActivityPointerAxes_enableAxis(axis as i32) }
}

pub fn disable_motion_axis(&mut self, axis: Axis) {
let axis: u32 = axis.into();
unsafe { ffi::GameActivityPointerAxes_disableAxis(axis as i32) }
}

Expand Down
140 changes: 76 additions & 64 deletions android-activity/src/input.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use bitflags::bitflags;
use num_enum::{IntoPrimitive, TryFromPrimitive};

pub use crate::activity_impl::input::*;
use crate::InputStatus;
Expand All @@ -10,7 +9,7 @@ pub use sdk::*;
/// An enum representing the source of an [`MotionEvent`] or [`KeyEvent`]
///
/// See [the InputDevice docs](https://developer.android.com/reference/android/view/InputDevice#SOURCE_ANY)
#[derive(Debug, Clone, Copy, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, num_enum::FromPrimitive, num_enum::IntoPrimitive)]
#[repr(u32)]
pub enum Source {
BluetoothStylus = 0x0000c002,
Expand All @@ -36,9 +35,36 @@ pub enum Source {
TouchNavigation = 0x00200000,
Trackball = 0x00010004,

Unknown = 0,
}

// We need to consider that the enum variants may be extended across
// different versions of Android (i.e. effectively at runtime) but at the
// same time we don't want it to be an API break to extend this enum in
// future releases of `android-activity` with new variants from the latest
// NDK/SDK.
//
// We can't just use `#[non-exhaustive]` because that only really helps
// when adding new variants in sync with android-activity releases.
//
// On the other hand we also can't rely on a catch-all `Unknown(u32)` that
// only really helps with unknown variants seen at runtime.
//
// What we aim for instead is to have a hidden catch-all variant that
// is considered (practically) unmatchable so code is forced to have
// a `unknown => {}` catch-all pattern match that will cover unknown variants
// either in the form of Rust variants added in future versions or
// in the form of an `__Unknown(u32)` integer that represents an unknown
// variant seen at runtime.
//
// Any `unknown => {}` pattern match can rely on `IntoPrimitive` to convert
// the `unknown` variant to the integer that comes from the Android SDK
// in case that values needs to be passed on, even without knowing it's
// semantic meaning at compile time.
#[doc(hidden)]
#[num_enum(catch_all)]
__Unknown(u32),
}

// ndk_sys doesn't currently have the `TRACKBALL` flag so we define our
// own internal class constants for now
bitflags! {
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
struct SourceFlags: u32 {
Expand All @@ -53,37 +79,31 @@ bitflags! {
}
}

/// An enum representing the class of a [`MotionEvent`] or [`KeyEvent`] source
///
/// See [the InputDevice docs](https://developer.android.com/reference/android/view/InputDevice#SOURCE_CLASS_MASK)
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Class {
None,
Button,
Pointer,
Trackball,
Position,
Joystick,
}

impl From<u32> for Class {
fn from(source: u32) -> Self {
let class = SourceFlags::from_bits_truncate(source);
match class {
SourceFlags::BUTTON => Class::Button,
SourceFlags::POINTER => Class::Pointer,
SourceFlags::TRACKBALL => Class::Trackball,
SourceFlags::POSITION => Class::Position,
SourceFlags::JOYSTICK => Class::Joystick,
_ => Class::None,
}
impl Source {
#[inline]
pub fn is_button_class(self) -> bool {
let class = SourceFlags::from_bits_truncate(self.into());
class.contains(SourceFlags::BUTTON)
}
}

impl From<Source> for Class {
fn from(source: Source) -> Self {
let source: u32 = source.into();
source.into()
#[inline]
pub fn is_pointer_class(self) -> bool {
let class = SourceFlags::from_bits_truncate(self.into());
class.contains(SourceFlags::POINTER)
}
#[inline]
pub fn is_trackball_class(self) -> bool {
let class = SourceFlags::from_bits_truncate(self.into());
class.contains(SourceFlags::TRACKBALL)
}
#[inline]
pub fn is_position_class(self) -> bool {
let class = SourceFlags::from_bits_truncate(self.into());
class.contains(SourceFlags::POSITION)
}
#[inline]
pub fn is_joystick_class(self) -> bool {
let class = SourceFlags::from_bits_truncate(self.into());
class.contains(SourceFlags::JOYSTICK)
}
}

Expand Down Expand Up @@ -174,7 +194,7 @@ impl From<ndk::event::MetaState> for MetaState {
///
/// See [the NDK
/// docs](https://developer.android.com/ndk/reference/group/input#anonymous-enum-29)
#[derive(Copy, Clone, Debug, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, num_enum::FromPrimitive, num_enum::IntoPrimitive)]
#[repr(u32)]
pub enum MotionAction {
Down = ndk_sys::AMOTION_EVENT_ACTION_DOWN,
Expand All @@ -190,19 +210,16 @@ pub enum MotionAction {
HoverExit = ndk_sys::AMOTION_EVENT_ACTION_HOVER_EXIT,
ButtonPress = ndk_sys::AMOTION_EVENT_ACTION_BUTTON_PRESS,
ButtonRelease = ndk_sys::AMOTION_EVENT_ACTION_BUTTON_RELEASE,
}

impl From<ndk::event::MotionAction> for MotionAction {
fn from(value: ndk::event::MotionAction) -> Self {
let inner: u32 = value.into();
inner.try_into().unwrap() // TODO: use into() when we bump the MSRV to 1.68
}
#[doc(hidden)]
#[num_enum(catch_all)]
__Unknown(u32),
}

/// An axis of a motion event.
///
/// See [the NDK docs](https://developer.android.com/ndk/reference/group/input#anonymous-enum-32)
#[derive(Copy, Clone, Debug, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, num_enum::FromPrimitive, num_enum::IntoPrimitive)]
#[repr(u32)]
pub enum Axis {
X = ndk_sys::AMOTION_EVENT_AXIS_X,
Expand Down Expand Up @@ -250,19 +267,16 @@ pub enum Axis {
Generic14 = ndk_sys::AMOTION_EVENT_AXIS_GENERIC_14,
Generic15 = ndk_sys::AMOTION_EVENT_AXIS_GENERIC_15,
Generic16 = ndk_sys::AMOTION_EVENT_AXIS_GENERIC_16,
}

impl From<ndk::event::Axis> for Axis {
fn from(value: ndk::event::Axis) -> Self {
let inner: u32 = value.into();
inner.try_into().unwrap() // TODO: replace with into() when we can bump MSRV to 1.68!
}
#[doc(hidden)]
#[num_enum(catch_all)]
__Unknown(u32),
}

/// The tool type of a pointer.
///
/// See [the NDK docs](https://developer.android.com/ndk/reference/group/input#anonymous-enum-48)
#[derive(Copy, Clone, Debug, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, num_enum::FromPrimitive, num_enum::IntoPrimitive)]
#[repr(u32)]
pub enum ToolType {
/// Unknown tool type.
Expand All @@ -284,6 +298,10 @@ pub enum ToolType {

/// The tool is a palm and should be rejected
Palm = ndk_sys::AMOTION_EVENT_TOOL_TYPE_PALM,

#[doc(hidden)]
#[num_enum(catch_all)]
__Unknown(u32),
}

/// A bitfield representing the state of buttons during a motion event.
Expand Down Expand Up @@ -382,25 +400,22 @@ impl From<ndk::event::MotionEventFlags> for MotionEventFlags {
/// Key actions.
///
/// See [the NDK docs](https://developer.android.com/ndk/reference/group/input#anonymous-enum-27)
#[derive(Copy, Clone, Debug, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, num_enum::FromPrimitive, num_enum::IntoPrimitive)]
#[repr(u32)]
pub enum KeyAction {
Down = ndk_sys::AKEY_EVENT_ACTION_DOWN,
Up = ndk_sys::AKEY_EVENT_ACTION_UP,
Multiple = ndk_sys::AKEY_EVENT_ACTION_MULTIPLE,
}

impl From<ndk::event::KeyAction> for KeyAction {
fn from(value: ndk::event::KeyAction) -> Self {
let inner: u32 = value.into();
inner.try_into().unwrap() // TODO: replace with into() when we can bump MSRV to 1.68!
}
#[doc(hidden)]
#[num_enum(catch_all)]
__Unknown(u32),
}

/// Key codes.
///
/// See [the NDK docs](https://developer.android.com/ndk/reference/group/input#anonymous-enum-39)
#[derive(Copy, Clone, Debug, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, num_enum::FromPrimitive, num_enum::IntoPrimitive)]
#[repr(u32)]
pub enum Keycode {
Unknown = ndk_sys::AKEYCODE_UNKNOWN,
Expand Down Expand Up @@ -692,13 +707,10 @@ pub enum Keycode {
ThumbsUp = ndk_sys::AKEYCODE_THUMBS_UP,
ThumbsDown = ndk_sys::AKEYCODE_THUMBS_DOWN,
ProfileSwitch = ndk_sys::AKEYCODE_PROFILE_SWITCH,
}

impl From<ndk::event::Keycode> for Keycode {
fn from(value: ndk::event::Keycode) -> Self {
let inner: u32 = value.into();
inner.try_into().unwrap() // TODO: replace with into() when we can bump MSRV to 1.68!
}
#[doc(hidden)]
#[num_enum(catch_all)]
__Unknown(u32),
}

/// Flags associated with [`KeyEvent`].
Expand Down
Loading

0 comments on commit 37dc94a

Please sign in to comment.