Skip to content

Conversation

@MarijnS95
Copy link
Member

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
Copy link
Member

madsmtm commented Feb 1, 2026

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();

Comment on lines +141 to +143
// TODO: Validate that Android actually initializes (possibly with garbage) the buffer, or
// let softbuffer initialize it, or return MaybeUninit<Pixel>?
// i.e. consider age().
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Zero-copying on Android

4 participants