diff --git a/src/backends/android.rs b/src/backends/android.rs index 75e5bf90..d95bd365 100644 --- a/src/backends/android.rs +++ b/src/backends/android.rs @@ -12,16 +12,21 @@ use raw_window_handle::AndroidNdkWindowHandle; use raw_window_handle::{HasDisplayHandle, HasWindowHandle, RawWindowHandle}; use crate::error::InitError; -use crate::{util, BufferInterface, Pixel, Rect, SoftBufferError, SurfaceInterface}; +use crate::{BufferInterface, Pixel, Rect, SoftBufferError, SurfaceInterface}; /// The handle to a window for software buffering. #[derive(Debug)] pub struct AndroidImpl { + // Must be first in the struct to guarantee being dropped and unlocked before the `NativeWindow` reference + in_progress_buffer: Option>, native_window: NativeWindow, window: W, _display: PhantomData, } +// TODO: Move to NativeWindowBufferLockGuard? +unsafe impl Send for AndroidImpl {} + impl SurfaceInterface for AndroidImpl { type Context = D; type Buffer<'surface> @@ -41,6 +46,7 @@ impl SurfaceInterface for Android let native_window = unsafe { NativeWindow::clone_from_ptr(a.a_native_window.cast()) }; Ok(Self { + in_progress_buffer: None, native_window, _display: PhantomData, window, @@ -52,7 +58,7 @@ impl SurfaceInterface for Android &self.window } - /// Also changes the pixel format to [`HardwareBufferFormat::R8G8B8A8_UNORM`]. + /// Also changes the pixel format to [`HardwareBufferFormat::R8G8B8X8_UNORM`]. fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) -> Result<(), SoftBufferError> { let (width, height) = (|| { let width = NonZeroI32::try_from(width).ok()?; @@ -77,6 +83,12 @@ impl SurfaceInterface for Android } fn buffer_mut(&mut self) -> Result, SoftBufferError> { + if self.in_progress_buffer.is_some() { + return Ok(BufferImpl { + native_window_buffer: &mut self.in_progress_buffer, + }); + } + let native_window_buffer = self.native_window.lock(None).map_err(|err| { SoftBufferError::PlatformError( Some("Failed to lock ANativeWindow".to_owned()), @@ -99,12 +111,15 @@ impl SurfaceInterface for Android )); } - let buffer = - vec![Pixel::default(); native_window_buffer.stride() * native_window_buffer.height()]; + // SAFETY: We guarantee that the guard isn't actually held longer than this owned handle of + // the `NativeWindow` (which is trivially clonable), by means of having BufferImpl take a + // mutable borrow on AndroidImpl which owns the NativeWindow and LockGuard. + let native_window_buffer: NativeWindowBufferLockGuard<'static> = + unsafe { std::mem::transmute(native_window_buffer) }; + self.in_progress_buffer = Some(native_window_buffer); Ok(BufferImpl { - native_window_buffer, - buffer: util::PixelBuffer(buffer), + native_window_buffer: &mut self.in_progress_buffer, }) } @@ -116,8 +131,8 @@ impl SurfaceInterface for Android #[derive(Debug)] pub struct BufferImpl<'surface> { - native_window_buffer: NativeWindowBufferLockGuard<'surface>, - buffer: util::PixelBuffer, + // This Option will always be Some until present_with_damage() is called + native_window_buffer: &'surface mut Option>, } // TODO: Move to NativeWindowBufferLockGuard? @@ -125,20 +140,33 @@ unsafe impl Send for BufferImpl<'_> {} impl BufferInterface for BufferImpl<'_> { fn byte_stride(&self) -> NonZeroU32 { - NonZeroU32::new(self.native_window_buffer.stride() as u32 * 4).unwrap() + NonZeroU32::new(self.native_window_buffer.as_ref().unwrap().stride() as u32 * 4).unwrap() } fn width(&self) -> NonZeroU32 { - NonZeroU32::new(self.native_window_buffer.width() as u32).unwrap() + NonZeroU32::new(self.native_window_buffer.as_ref().unwrap().width() as u32).unwrap() } fn height(&self) -> NonZeroU32 { - NonZeroU32::new(self.native_window_buffer.height() as u32).unwrap() + NonZeroU32::new(self.native_window_buffer.as_ref().unwrap().height() as u32).unwrap() } #[inline] fn pixels_mut(&mut self) -> &mut [Pixel] { - &mut self.buffer + let native_buffer = self.native_window_buffer.as_mut().unwrap().bytes().unwrap(); + // assert_eq!( + // native_buffer.len(), + // self.native_window_buffer.stride() * self.native_window_buffer.height() + // ); + // TODO: Validate that Android actually initializes (possibly with garbage) the buffer, or + // let softbuffer initialize it, or return MaybeUninit? + // i.e. consider age(). + unsafe { + std::slice::from_raw_parts_mut( + native_buffer.as_mut_ptr().cast(), + native_buffer.len() / 4, + ) + } } #[inline] @@ -147,7 +175,7 @@ impl BufferInterface for BufferImpl<'_> { } // TODO: This function is pretty slow this way - fn present_with_damage(mut self, damage: &[Rect]) -> Result<(), SoftBufferError> { + fn present_with_damage(self, damage: &[Rect]) -> Result<(), SoftBufferError> { // TODO: Android requires the damage rect _at lock time_ // Since we're faking the backing buffer _anyway_, we could even fake the surface lock // and lock it here (if it doesn't influence timings). @@ -158,18 +186,9 @@ impl BufferInterface for BufferImpl<'_> { // when the enlarged damage region is not re-rendered? let _ = damage; - // Unreachable as we checked before that this is a valid, mappable format - let native_buffer = self.native_window_buffer.bytes().unwrap(); - - // Write RGB(A) to the output. - // TODO: Use `slice::write_copy_of_slice` once stable and in MSRV. - // TODO(madsmtm): Verify that this compiles down to an efficient copy. - for (pixel, output) in self.buffer.iter().zip(native_buffer.chunks_mut(4)) { - output[0].write(pixel.r); - output[1].write(pixel.g); - output[2].write(pixel.b); - output[3].write(pixel.a); - } + // The surface will be presented when it is unlocked, which happens when the owned guard + // is dropped. + self.native_window_buffer.take(); Ok(()) }