ndk: Bump MSRV from 1.64 to 1.66 (#431)

It is older than 6 months and now required for the `toml_edit` dependency (used by `num_enum_derive`). At the same time `num_enum` relies on arbitrary enum discriminants (for discriminated enums with tuple/struct variants) introduced by Rust 1.66 in order to implement `#[num_enum(catch_all)]`. This feature comes in use to replace the manual `match` blocks when implementing conversions for `HardwareBufferFormat` when also having an `Unknown(u32)` to catch valid private/vendor values that won't ever be described inside the NDK. This change effectively reverts #407 to its initial state, where a `catch_all` implementation was used. For the latter the intent is however to use this feature sparingly. In most APIs new values are few and far between, so treating these as an `Err` via `TryFromPrimitive` is desired to provoke upstream issue reports and quick turnaround on new values. Same for `enum`s that are used to pass values into functions: it is desired to only pass known values (by this `ndk` crate) into those, anything else should similarly be reported and added upstream. In these cases a `#[non_exhaustive]` allows us to do so with a tiny non-breaking patch release.
rust-mobile · Sep 15, 2023 · eb445c6 · eb445c6
1 parent 7abf3e4
commit eb445c6
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 123 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -38,12 +38,12 @@ jobs:
         run: cargo check -p ndk-sys --all-targets --all-features --target aarch64-linux-android
 
   check_msrv:
-    name: Check overall MSRV (1.64.0)
+    name: Check overall MSRV (1.66.0)
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v1
 
-      - uses: dtolnay/rust-toolchain@1.64.0
+      - uses: dtolnay/rust-toolchain@1.66.0
         with:
           target: aarch64-linux-android
 

diff --git a/ndk/CHANGELOG.md b/ndk/CHANGELOG.md
@@ -5,7 +5,7 @@
 - event: Add `tool_type` getter for `Pointer`. (#323)
 - input_queue: Allow any non-zero return code from `pre_dispatch()` again, as per documentation. (#325)
 - asset: Use entire asset length when mapping buffer. (#387)
-- Bump MSRV to 1.64 for `raw-window-handle 0.5.1`. (#388)
+- Bump MSRV to 1.66 for `raw-window-handle 0.5.1`, `num_enum`'s `catch_all` with arbitrary enum discriminants. (#388,#431)
 - Bump optional `jni` dependency for doctest example from `0.19` to `0.21`. (#390)
 - **Breaking:** Upgrade to [`ndk-sys 0.5.0`](../ndk-sys/CHANGELOG.md#050-TODO). (#370)
 - **Breaking:** Upgrade `bitflags` crate from `1` to `2`. (#394)

diff --git a/ndk/Cargo.toml b/ndk/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "ndk"
 version = "0.8.0-beta.0"
-authors = ["The Rust Windowing contributors"]
+authors = ["The Rust Mobile contributors"]
 edition = "2021"
 description = "Safe Rust bindings to the Android NDK"
 license = "MIT OR Apache-2.0"
@@ -10,7 +10,7 @@ readme = "../README.md"
 documentation = "https://docs.rs/ndk"
 homepage = "https://github.com/rust-mobile/ndk"
 repository = "https://github.com/rust-mobile/ndk"
-rust-version = "1.64"
+rust-version = "1.66"
 
 [features]
 all = ["audio", "bitmap","media", "api-level-30"]

diff --git a/ndk/src/asset.rs b/ndk/src/asset.rs
@@ -4,11 +4,12 @@
 //! [`AAssetDir`]: https://developer.android.com/ndk/reference/group/asset#aassetdir
 //! [`AAssetManager`]: https://developer.android.com/ndk/reference/group/asset#aassetmanager
 
-use std::ffi::{CStr, CString};
-use std::io;
-// TODO: Import from std::os::fd::{} since Rust 1.66
-use std::os::unix::io::{FromRawFd, OwnedFd};
-use std::ptr::NonNull;
+use std::{
+    ffi::{CStr, CString},
+    io,
+    os::fd::{FromRawFd, OwnedFd},
+    ptr::NonNull,
+};
 
 /// A native [`AAssetManager *`]
 ///

diff --git a/ndk/src/hardware_buffer.rs b/ndk/src/hardware_buffer.rs
@@ -13,9 +13,8 @@ use std::{
     mem::MaybeUninit,
     ops::Deref,
     os::{
+        fd::{AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd},
         raw::c_void,
-        // TODO: Import from std::os::fd::{} since Rust 1.66
-        unix::io::{AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd},
     },
     ptr::NonNull,
 };
@@ -211,7 +210,7 @@ impl HardwareBuffer {
             width: desc.width,
             height: desc.height,
             layers: desc.layers,
-            format: ffi::AHardwareBuffer_Format(desc.format).into(),
+            format: desc.format.into(),
             usage: HardwareBufferUsage(ffi::AHardwareBuffer_UsageFlags(desc.usage)),
             stride: desc.stride,
         }
@@ -497,7 +496,7 @@ impl HardwareBufferDesc {
             width: self.width,
             height: self.height,
             layers: self.layers,
-            format: ffi::AHardwareBuffer_Format::from(self.format).0,
+            format: self.format.into(),
             usage: self.usage.0 .0,
             stride: self.stride,
             rfu0: 0,

diff --git a/ndk/src/hardware_buffer_format.rs b/ndk/src/hardware_buffer_format.rs
@@ -2,121 +2,62 @@
 //!
 //! [`AHardwareBuffer_Format`]: https://developer.android.com/ndk/reference/group/a-hardware-buffer#ahardwarebuffer_format
 
+use num_enum::{FromPrimitive, IntoPrimitive};
+
 /// Buffer pixel formats.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[repr(u32)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, FromPrimitive, IntoPrimitive)]
 #[allow(non_camel_case_types)]
 pub enum HardwareBufferFormat {
-    /// Matches deprecated [`ffi::ANativeWindow_LegacyFormat::WINDOW_FORMAT_RGBA_8888`].
-    R8G8B8A8_UNORM,
-    /// Matches deprecated [`ffi::ANativeWindow_LegacyFormat::WINDOW_FORMAT_RGBX_8888`].
-    R8G8B8X8_UNORM,
+    /// Matches deprecated [`ffi::ANativeWindow_LegacyFormat::WINDOW_FORMAT_RGBA_8888`].0.
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM")]
+    R8G8B8A8_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM.0,
+    /// Matches deprecated [`ffi::ANativeWindow_LegacyFormat::WINDOW_FORMAT_RGBX_8888`].0.
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM")]
+    R8G8B8X8_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM.0,
     #[cfg(feature = "api-level-26")]
-    R8G8B8_UNORM,
-    /// Matches deprecated [`ffi::ANativeWindow_LegacyFormat::WINDOW_FORMAT_RGB_565`].
-    R5G6B5_UNORM,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM")]
+    R8G8B8_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM.0,
+    /// Matches deprecated [`ffi::ANativeWindow_LegacyFormat::WINDOW_FORMAT_RGB_565`].0.
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM")]
+    R5G6B5_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM.0,
     #[cfg(feature = "api-level-26")]
-    R16G16B16A16_FLOAT,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT")]
+    R16G16B16A16_FLOAT = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT.0,
     #[cfg(feature = "api-level-26")]
-    R10G10B10A2_UNORM,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM")]
+    R10G10B10A2_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM.0,
     #[cfg(feature = "api-level-26")]
-    BLOB,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_BLOB")]
+    BLOB = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_BLOB.0,
     #[cfg(feature = "api-level-26")]
-    D16_UNORM,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_D16_UNORM")]
+    D16_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_D16_UNORM.0,
     #[cfg(feature = "api-level-26")]
-    D24_UNORM,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_D24_UNORM")]
+    D24_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_D24_UNORM.0,
     #[cfg(feature = "api-level-26")]
-    D24_UNORM_S8_UINT,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_D24_UNORM_S8_UINT")]
+    D24_UNORM_S8_UINT = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_D24_UNORM_S8_UINT.0,
     #[cfg(feature = "api-level-26")]
-    D32_FLOAT,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_D32_FLOAT")]
+    D32_FLOAT = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_D32_FLOAT.0,
     #[cfg(feature = "api-level-26")]
-    D32_FLOAT_S8_UINT,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_D32_FLOAT_S8_UINT")]
+    D32_FLOAT_S8_UINT = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_D32_FLOAT_S8_UINT.0,
     #[cfg(feature = "api-level-26")]
-    S8_UINT,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_S8_UINT")]
+    S8_UINT = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_S8_UINT.0,
     #[cfg(feature = "api-level-26")]
-    Y8Cb8Cr8_420,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_420")]
+    Y8Cb8Cr8_420 = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_420.0,
     #[cfg(feature = "api-level-26")]
-    YCbCr_P010,
+    #[doc(alias = "AHARDWAREBUFFER_FORMAT_YCbCr_P010")]
+    YCbCr_P010 = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_YCbCr_P010.0,
     #[cfg(feature = "api-level-26")]
-    R8_UNORM,
-    Unknown(ffi::AHardwareBuffer_Format),
-}
-
-impl From<ffi::AHardwareBuffer_Format> for HardwareBufferFormat {
-    fn from(value: ffi::AHardwareBuffer_Format) -> Self {
-        use ffi::AHardwareBuffer_Format as AFormat;
-        use HardwareBufferFormat::*;
-        match value {
-            AFormat::AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM => R8G8B8A8_UNORM,
-            AFormat::AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM => R8G8B8X8_UNORM,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM => R8G8B8_UNORM,
-            AFormat::AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM => R5G6B5_UNORM,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT => R16G16B16A16_FLOAT,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM => R10G10B10A2_UNORM,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_BLOB => BLOB,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_D16_UNORM => D16_UNORM,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_D24_UNORM => D24_UNORM,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_D24_UNORM_S8_UINT => D24_UNORM_S8_UINT,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_D32_FLOAT => D32_FLOAT,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_D32_FLOAT_S8_UINT => D32_FLOAT_S8_UINT,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_S8_UINT => S8_UINT,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_420 => Y8Cb8Cr8_420,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_YCbCr_P010 => YCbCr_P010,
-            #[cfg(feature = "api-level-26")]
-            AFormat::AHARDWAREBUFFER_FORMAT_R8_UNORM => R8_UNORM,
-            _ => Unknown(value),
-        }
-    }
-}
-
-impl From<HardwareBufferFormat> for ffi::AHardwareBuffer_Format {
-    fn from(value: HardwareBufferFormat) -> Self {
-        use ffi::AHardwareBuffer_Format as AFormat;
-        use HardwareBufferFormat::*;
-        match value {
-            R8G8B8A8_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM,
-            R8G8B8X8_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM,
-            #[cfg(feature = "api-level-26")]
-            R8G8B8_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM,
-            R5G6B5_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM,
-            #[cfg(feature = "api-level-26")]
-            R16G16B16A16_FLOAT => AFormat::AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT,
-            #[cfg(feature = "api-level-26")]
-            R10G10B10A2_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM,
-            #[cfg(feature = "api-level-26")]
-            BLOB => AFormat::AHARDWAREBUFFER_FORMAT_BLOB,
-            #[cfg(feature = "api-level-26")]
-            D16_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_D16_UNORM,
-            #[cfg(feature = "api-level-26")]
-            D24_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_D24_UNORM,
-            #[cfg(feature = "api-level-26")]
-            D24_UNORM_S8_UINT => AFormat::AHARDWAREBUFFER_FORMAT_D24_UNORM_S8_UINT,
-            #[cfg(feature = "api-level-26")]
-            D32_FLOAT => AFormat::AHARDWAREBUFFER_FORMAT_D32_FLOAT,
-            #[cfg(feature = "api-level-26")]
-            D32_FLOAT_S8_UINT => AFormat::AHARDWAREBUFFER_FORMAT_D32_FLOAT_S8_UINT,
-            #[cfg(feature = "api-level-26")]
-            S8_UINT => AFormat::AHARDWAREBUFFER_FORMAT_S8_UINT,
-            #[cfg(feature = "api-level-26")]
-            Y8Cb8Cr8_420 => AFormat::AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_420,
-            #[cfg(feature = "api-level-26")]
-            YCbCr_P010 => AFormat::AHARDWAREBUFFER_FORMAT_YCbCr_P010,
-            #[cfg(feature = "api-level-26")]
-            R8_UNORM => AFormat::AHARDWAREBUFFER_FORMAT_R8_UNORM,
-            Unknown(x) => x,
-        }
-    }
+    R8_UNORM = ffi::AHardwareBuffer_Format::AHARDWAREBUFFER_FORMAT_R8_UNORM.0,
+    #[num_enum(catch_all)]
+    Unknown(u32),
 }
 
 impl HardwareBufferFormat {

diff --git a/ndk/src/looper.rs b/ndk/src/looper.rs
@@ -11,9 +11,10 @@
 
 use bitflags::bitflags;
 use std::mem::ManuallyDrop;
-use std::os::raw::c_void;
-// TODO: Import from std::os::fd::{} since Rust 1.66
-use std::os::unix::io::{AsRawFd, BorrowedFd, RawFd};
+use std::os::{
+    fd::{AsRawFd, BorrowedFd, RawFd},
+    raw::c_void,
+};
 use std::ptr;
 use std::time::Duration;
 use thiserror::Error;

diff --git a/ndk/src/media/image_reader.rs b/ndk/src/media/image_reader.rs
@@ -16,8 +16,7 @@ use std::{
 };
 
 #[cfg(feature = "api-level-26")]
-// TODO: Import from std::os::fd::{} since Rust 1.66
-use std::os::unix::io::{FromRawFd, IntoRawFd, OwnedFd};
+use std::os::fd::{FromRawFd, IntoRawFd, OwnedFd};
 
 #[cfg(feature = "api-level-26")]
 use crate::hardware_buffer::{HardwareBuffer, HardwareBufferUsage};

diff --git a/ndk/src/native_activity.rs b/ndk/src/native_activity.rs
@@ -219,7 +219,9 @@ impl NativeActivity {
         unsafe {
             ffi::ANativeActivity_setWindowFormat(
                 self.ptr.as_ptr(),
-                ffi::AHardwareBuffer_Format::from(format).0 as i32,
+                u32::from(format)
+                    .try_into()
+                    .expect("i32 overflow in set_window_format()"),
             )
         }
     }

diff --git a/ndk/src/native_window.rs b/ndk/src/native_window.rs
@@ -81,7 +81,7 @@ impl NativeWindow {
     pub fn format(&self) -> HardwareBufferFormat {
         let value = unsafe { ffi::ANativeWindow_getFormat(self.ptr.as_ptr()) };
         let value = u32::try_from(value).unwrap();
-        HardwareBufferFormat::from(ffi::AHardwareBuffer_Format(value))
+        value.into()
     }
 
     /// Change the format and size of the window buffers.
@@ -99,9 +99,13 @@ impl NativeWindow {
         height: i32,
         format: Option<HardwareBufferFormat>,
     ) -> Result<()> {
-        let format = format.map_or(0, |f| ffi::AHardwareBuffer_Format::from(f).0);
+        let format = format.map_or(0, |f| {
+            u32::from(f)
+                .try_into()
+                .expect("i32 overflow in set_buffers_geometry")
+        });
         let status = unsafe {
-            ffi::ANativeWindow_setBuffersGeometry(self.ptr.as_ptr(), width, height, format as i32)
+            ffi::ANativeWindow_setBuffersGeometry(self.ptr.as_ptr(), width, height, format)
         };
         status_to_io_result(status)
     }
@@ -179,7 +183,7 @@ impl<'a> NativeWindowBufferLockGuard<'a> {
     /// The format of the buffer. One of [`HardwareBufferFormat`].
     pub fn format(&self) -> HardwareBufferFormat {
         let format = u32::try_from(self.buffer.format).unwrap();
-        HardwareBufferFormat::from(ffi::AHardwareBuffer_Format(format))
+        format.into()
     }
 
     /// The actual bits.