rust-mobile / ndk

Rust bindings to the Android NDK
Apache License 2.0
1.14k stars 112 forks source link

Unify lifetime handling for `NativeWindow` and `HardwareBuffer` #309

Open MarijnS95 opened 2 years ago

MarijnS95 commented 2 years ago

See https://github.com/rust-windowing/android-ndk-rs/pull/296#issuecomment-1153288154, there are multiple ways we can approach this.

Something based on borrows (Like how String + &str work together) has my preference, but we can't give any sensible lifetime to for example what fn native_window() returns (which we currently have to forcibly _acquire to make it valid). Alas, we'll probably remove those static getters anyway in light of recent suggestions and to support multi-Activity, so this specific case wouldn't affect us anymore. At which point a borrow-based approach might as well work out :)

MarijnS95 commented 3 months ago

Looking into this again, handing out &'_ Type would be preferred (i.e. to match Rust's &str vs String deisgn), but we need storage for Type in this case which is impossible. Hence HardwareBufferRef solves this by simply owning a HardwareBuffer and implementing Drop for itself.

We could possibly improve this API by adding a lifetime to HardwareBuffer, which HardwareBufferRef hides by setting it to 'static internally because it controls the lifetime. That would allow us to derive Clone, Copy on the non-refcounted HardwareBuffer, as long as Deref for HardwareBufferRef returns a target of (&'a self) -> &'a HardwareBuffer<'a> and not 'static. On the other hand the current implementation (without lifetime on HardwareBuffer) already captures a very similar semantic when &'_ HardwareBuffer exists, except that HardwareBuffer::from_ptr() gives you an owned and lifetime-less HardwareBuffer.