Skip to content
Draft
Changes from all commits
Commits
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
69 changes: 44 additions & 25 deletions src/backends/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<D, W> {
// Must be first in the struct to guarantee being dropped and unlocked before the `NativeWindow` reference
in_progress_buffer: Option<NativeWindowBufferLockGuard<'static>>,
native_window: NativeWindow,
window: W,
_display: PhantomData<D>,
}

// TODO: Move to NativeWindowBufferLockGuard?
unsafe impl<D, W> Send for AndroidImpl<D, W> {}

impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> for AndroidImpl<D, W> {
type Context = D;
type Buffer<'surface>
Expand All @@ -41,6 +46,7 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> 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,
Expand All @@ -52,7 +58,7 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> 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()?;
Expand All @@ -77,6 +83,12 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> for Android
}

fn buffer_mut(&mut self) -> Result<BufferImpl<'_>, 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()),
Expand All @@ -99,12 +111,15 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> 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) };
Comment on lines +117 to +118
Copy link
Member

Choose a reason for hiding this comment

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

Nit: When transmuting, I prefer:

Suggested change
let native_window_buffer: NativeWindowBufferLockGuard<'static> =
unsafe { std::mem::transmute(native_window_buffer) };
let native_window_buffer = unsafe {
std::mem::transmute::<
NativeWindowBufferLockGuard<'_>,
NativeWindowBufferLockGuard<'static>,
>(native_window_buffer)
};

To make it be a compile-time error if we made a mistake and forgot to unwrap the error from ANativeWindow::lock.

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,
})
}

Expand All @@ -116,29 +131,42 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> 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<NativeWindowBufferLockGuard<'static>>,
}

// TODO: Move to NativeWindowBufferLockGuard?
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<Pixel>?
// i.e. consider age().
Comment on lines +161 to +163
Copy link
Member

Choose a reason for hiding this comment

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

As long as android is using a swap chain of two or three buffers and we can get the buffer age, it probably makes the most sense for softbuffer to zero-initialize the buffers, if Android doesn't already do that.

It shouldn't really add any meaningful cost to zero each buffer once.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can somehow infer the amount of buffers with ANativeWindow_getBuffersDataSpace?

But I'd also be fine with zero-initializing in buffer_mut for now, and coming back to this later, this PR will be an improvement anyhow.

unsafe {
std::slice::from_raw_parts_mut(
native_buffer.as_mut_ptr().cast(),
native_buffer.len() / 4,
)
}
}

#[inline]
Expand All @@ -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).
Expand All @@ -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(())
}
Expand Down
Loading