android: Make the backend zero-copy

[MarijnS95]

WIP but tested change that closes #318.

Still need to discuss how to handle MaybeUninit, and the now side-effect presenting in-progress modifications to pixels_mut() if the caller ended up dropping their Buffer half-way through, which is an unfortunate caveat of having a locked buffer for the surface that unlocks and presents on drop. The turning point is that it's not allowed to lock() a surface twice before unlock()(ing) and inherently presenting it.

Are other platforms affected by such a lock-unlock kind of API? As hinted in #318 ASurfaceControl+ASurfaceTransaction+AHardwareBuffer completely obviate this issue, but that has very high Android requirements (the API's are there for a while, but I seem to have been the first one ever using it on the root Surface, the one you get from NativeActivity, and it didn't work until a bug report and fix since Android 15 (API 35)).

[madsmtm]

Are other platforms affected by such a lock-unlock kind of API?

On macOS, the locking/unlocking operation that IOSurface does is expensive, because it tells the MMU to make the region of memory available to other parts of the system. But you can do that over and over again, because nobody should be using that IOSurface yet.

So I don't think there are other platforms where this is a problem, Android is a bit special in "managing" the buffer(s) for us like this.

---

Maybe an option would be to store the locked buffer on the surface? Something like:

struct AndroidImpl {
    native_window: NativeWindow,
    in_progress_buffer: Option<NativeWindowBufferLockGuard<'static>>, // + unsafe magic
}

fn buffer_mut() {
    if let Some(native_window_buffer) = self.in_progress_buffer {
        return BufferImpl { native_window_buffer };
    }
    
    let native_window_buffer = self.native_window.lock(None)?;
    BufferImpl { native_window_buffer, surface_in_progress_buffer: &mut self.in_progress_buffer }
}

impl Drop for BufferImpl {
    fn drop(&mut self) {
        let buffer = self.native_window_buffer.take();
        *self.surface_in_progress_buffer = Some(buffer);
    }
}

That would allow the following to work the same as on other platforms:

let mut buffer = surface.buffer_mut();
buffer.pixels().fill(Pixel::rgb(0, 0, 0)); // Clear
drop(buffer);
let mut buffer = surface.buffer_mut();
draw(&mut buffer);
buffer.present();

[ids1024]

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.

[madsmtm]

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.

[MarijnS95]

@madsmtm something like that would work. I dropped the Some reassignment in fn drop() since the mutable lifetime that BufferImpl has on AndroidImpl already guarantees unique ownership, so it can already access the Option directly.

Unfortunately that requires various unwraps (like your suggested self.native_window_buffer.take()), but at least should not be susceptible to std::mem::forget() scenarios (which are safe) where unlock is not called and AndroidImpl::in_progess_buffer remains None?

[madsmtm]

Unfortunately that requires various unwraps

I'm totally fine with a bit of unwrapping, especially after #313, where we don't need to make pixels_mut zero-cost.

should not be susceptible to std::mem::forget() scenarios (which are safe) where unlock is not called and AndroidImpl::in_progess_buffer remains None?

I don't think my solution would've caused unsoundness either if you forget the buffer? It'd just cause a panic because you'd try to lock the buffer twice.

Anyhow, forgetting the buffer is definitely unsupported, as long as things are sound I don't care what the behavior of it is - I'll leave that up to whatever is easiest for you to maintain.

[madsmtm]

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.