Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Upgrade to ndk-sys 0.6.0 and ndk 0.9.0 #155

Merged
merged 1 commit into from
Apr 26, 2024
Merged

Upgrade to ndk-sys 0.6.0 and ndk 0.9.0 #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions android-activity/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ log = "0.4"
jni-sys = "0.3"
cesu8 = "1"
jni = "0.21"
ndk-sys = "0.5.0"
ndk = { version = "0.8.0", default-features = false }
ndk-sys = "0.6.0"
ndk = { version = "0.9.0", default-features = false }
ndk-context = "0.1"
android-properties = "0.2"
num_enum = "0.7"
Expand Down
12 changes: 4 additions & 8 deletions android-activity/src/native_activity/input.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,8 @@ impl<'a> PointerImpl<'a> {
#[inline]
pub fn axis_value(&self, axis: Axis) -> f32 {
let value: u32 = axis.into();
if let Ok(ndk_axis) = value.try_into() {
self.ndk_pointer.axis_value(ndk_axis)
} else {
// FIXME: We should also be able to query `Axis::__Unknown(u32)` values
// that can't currently be queried via the `ndk` `Pointer` API
0.0f32
}
let value = value as i32;
self.ndk_pointer.axis_value(value.into())
Comment on lines +265 to +266
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I wondered if maybe the repr type should have been fixed as game-activity is converting them as well. That matches what I did in the NDK to match the enum type to the function(s) that consume/generate it, not to the default u32 that happens to be assigned to untyped enums.

At some point that'll be fixed upstream anyway as they're retroactively fixing things to be enum foo : int32_t now that the NDK is always compiled with clang.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if I read things like this in android-activity again:

        // XXX: we use `AInputEvent_getSource` directly (instead of calling
        // ndk_event.source()) since we have our own `Source` enum that we
        // share between backends, which may also capture unknown variants
        // added in new versions of Android.

It feels like most if not all enums that are redefined are not to capture differences with game-activity, but merely because the ndk crate was lagging behind Android (what the comment literally says).

Instead of tediously spending a lot of time doing the same "type rectification" here, I'd rather extend the ndk with all the missing variants (which we're now allowed to do non-breaking thanks to using the exact same #[non_exhaustive] enum setup as android-activity), compare with android-activity to make sure we're not missing out, and finally reduce some NIH :)

}

#[inline]
Expand All @@ -283,7 +278,8 @@ impl<'a> PointerImpl<'a> {

#[inline]
pub fn tool_type(&self) -> ToolType {
let value: u32 = self.ndk_pointer.tool_type().into();
let value: i32 = self.ndk_pointer.tool_type().into();
let value = value as u32;
value.into()
}
}
Expand Down
1 change: 1 addition & 0 deletions android-activity/src/native_activity/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ impl<'a> InputIteratorInner<'a> {
ndk::event::InputEvent::KeyEvent(e) => {
input::InputEvent::KeyEvent(input::KeyEvent::new(e))
}
_ => todo!("NDK added a new type"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, we probably shouldn't panic here.

It would be a backwards compatible change to extend the ndk enum, and if we don't know about some new event in android-activity then we should skip calling callback()

I guess for now lets be lazy and just keep this todo!() though since it's maybe a bit of a faff to rejig to be able to skip the callback() but still call queue.finish_event()

and it would presumably make sense to handle new events here but without knowing what kind of event might be added it's not obvious that a panic is reasonable here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah fair enough, we're currently only mapping Key and Motion here, and disregarding Focus/Capture/Drag/TouchMode. Those don't have any custom functions, I'm not sure if they're used at all or only with the very limited generic AInputEvent_ functions that provide next to no info?

};

// `finish_event` needs to be called for each event otherwise
Expand Down
4 changes: 2 additions & 2 deletions examples/agdk-mainloop/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ edition = "2021"
log = "0.4"
android_logger = "0.11.0"
android-activity = { path="../../android-activity", features = ["game-activity"] }
ndk-sys = "0.5.0"
ndk = "0.8.0"
ndk-sys = "0.6.0"
ndk = "0.9.0"

[lib]
name="main"
Expand Down
4 changes: 2 additions & 2 deletions examples/na-mainloop/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ edition = "2021"
log = "0.4"
android_logger = "0.11.0"
android-activity = { path="../../android-activity", features = [ "native-activity" ] }
ndk-sys = "0.5.0"
ndk = "0.8.0"
ndk-sys = "0.6.0"
ndk = "0.9.0"

[lib]
#name="na_mainloop"
Expand Down
Loading