Make softbuffer window contexts own the pixel buffers.

[i509VCB]

At the moment all backends need to perform a copy to present. Some backends such as Wayland (always) and X11 (with the SHM extensions) can simply share the pixel memory with the server. On Windows and Mac this saving wouldn't be possible, but it would help on other platforms.

Not sure what redox does in this case.

[ids1024]

Should be possible with dumb/gbm buffers for https://github.com/rust-windowing/softbuffer/issues/42 too, right? Though the implications of working with references into mmaped gbm buffers may be subtler than just shared memory.

Also to provide a slice into an shm buffer, we technically trust the compositor to not modify it for soundness. This is probably fine (and less a concern than the reverse) but it's something to be aware of.

[i509VCB]

Should be possible with dumb/gbm buffers for rust-windowing/softbuffer#42 too, right? Though the implications of working with references into mmaped gbm buffers may be subtler than just shared memory.

Also to provide a slice into an shm buffer, we technically trust the compositor to not modify it for soundness. This is probably fine (and less a concern than the reverse) but it's something to be aware of.

With dumbuffers I believe that would be possible. GBM I guess mapping memory is weird there.

[jackpot51]

Redox supports memory mapping window buffers. In fact it is the only way to draw anything. The only problem is the memory needs to be remapped on window resizes, which may complicate the API.

[i509VCB]

If we need to ensure that the buffer can't be resized while drawing, we could use a closure in the function to access the buffer which takes an &mut reference.

[notgull]

Also to provide a slice into an shm buffer, we technically trust the compositor to not modify it for soundness. This is probably fine (and less a concern than the reverse) but it's something to be aware of.

According to the people that I've talked to about this, it may not actually be sound. On the X11 end, it's entirely possible for someone to write an implementation of the X server that maliciously modifies that buffer, and then run that server using our Rust program as a client. While the protocol says that the buffer can't be modified mid-transmission (or at least, not without the appropriate X11 events being delivered), there's nothing really stopping the server from disobeying it. I'm not sure about the Wayland end but I'd imagine that a similar problem exists.

A copy may unfortunately be necessary in the name of safety, in this case.

[i509VCB]

Yes the server can clobber the entire buffer on Wayland. But every client that has damage relies on the compositor not clobbering said buffer.

No client I've seen however assumes the server will trash the buffer unless given to something like wlr-screencopy which writes to the shm buffer.

[ids1024]

This is the same concern I mentioned earlier. There are a few considerations here:

• With backends like DRM, presumably this is guaranteed to avoid mutation barring a bug in the kernel or graphics driver. And we rely on a correct and non-malicious kernel for soundness anyway.
• With Wayland this could be a concern, but as far as malicious modification, the server normally has the same or greater privileges than the client, so trusting the compositor might be reasonable?
  • Similarly with X. Or in remote use cases you wouldn't be able to use shm anyway.
• I don't think this is likely to lead to exploitable vulnerabilities in practice even with a malicious server, but that's hard to be sure of. And more complicated in a library since the user could potentially be doing things other then merely writing bytes without reading anything.
• If we don't trust the server for soundness, we could still provide such an API, but it couldn't provide a slice into memory. But it is possible to provide a custom helper type with a custom Index implementation, etc.
  • If so, we'd probably want to provide this no-copy interface in addition to the simpler set_buffer API, rather than only providing it.

[i509VCB]

There is a significant usability harm if we aren't exposing slices. Many libraries like tiny-skia's PixmapMut takes a mutable slice.

[ids1024]

Yes, for a lot of APIs you would want the buffer or a part of it as a slice, so this could be quite limiting. While for some uses it would mostly work alright.

[notgull]

Given that we're probably moving in this direction, I figure we should come up with an API for this. Here's some considerations:

• How do we expose the buffer to the user? Do we have a buffer()/buffer_mut() method set that directly returns a slice? Or, do we want to make access more restrictive (maybe a with_buffer method that takes a closure that operations on a slice).
• How do we handle resizing the buffer?
• How should the buffer be pushed to the server?

[ids1024]

For the Wayland backend, at least, we could possibly hand out an owned object that DerefMuts to [u8]. Then the client transfers it back to display the buffer on the screen. That's basically how the current WaylandBuffer type works. But maybe that doesn't make as much sense in other contexts, and isn't actually a more convenient API.

I think buffer/buffer_mut is fine and with_buffer is unnecessary. As long as the method that submits/swaps the buffer takes &mut self, that will ensure the caller no longer has any references to the buffer.

[ids1024]

Another consideration is that set_buffer takes a width/height, while that's needed to allocate/resize buffers before we can provide a slice.

One possible API for this, and https://github.com/rust-windowing/softbuffer/issues/37:

Context::new<T: HasRawDisplayHandle>(display: &T) -> Self;
Surface::new<T: HasRawWindowHandle>(context: &Context, window: &T, width: u32, height: u32); 
Surface::resize(&mut self, width: u32, height: u32);
Surface::size(&self) -> (u32, u32);
Surface::buffer(&self) -> &[u8];
Surface::buffer_mut(&mut self) -> &mut [u8];
Surface::swap_buffers(&mut self);

(Perhaps make all of these return a Result.)

Considering the Wayland implementation, which is what I'm familiar with:

• Context/Surface/resize/swap_buffers generally mirrors Glutin and other libraries for hardware accelerated rendering. Just instead of OpenGL calls to draw, we have .buffer_mut().
• This could use either triple or double buffering (double requires waiting for the compositor to release the previous buffer at the end of swap_buffers, or during buffer/buffer_mut). This could also be configurable.
• buffer/buffer_mut create slices into mmapped shared memory, and trusts the compositor not to mutate the memory for soundness.
• We could still have a set_buffer function like the current one on top of these methods. With double buffering it could perform basically the same way. (Assuming we block like in https://github.com/rust-windowing/softbuffer/pull/55).
  • But is this really more ergonomic for an uses? Otherwise it's best to leave out if broken API changes are happening anyway.

For other platforms:

• If we aren't able to provide a slice into the memory that's sent to the server or it isn't sent over shared memory (for example on X without XShm), the backend can simply have a Vec<u8> for the front buffer, and copy the data over on swap in the same way set_buffer currently does
  • The performance and behavior should be comparable to any other solution assuming the application is able to render into the buffer without storing its own copy.
  • "swap buffers" isn't such an accurate description in this case.
• What are the contents of the buffer initially? Should we document it as being unspecified, since it may or may not be the same buffer used in the last, second to last, etc. draw, but we also don't want to have to zero it each swap?

[notgull]

My thoughts:

• I think we should zero out the buffer. Having an &mut [MaybeUninit<u8>] instead of an &mut [u8] probably isn't ergonomic.
• Should we also incorporate #36 into this?
• I see that you use an &[u8] instead of an &[u32] to meet the use case where formats use a quantum that fits into less than 32 bits. How are we representing formats in this case? An enum? A trait object? If the latter, we could (theoretically) have a Quantum associated type that abstracts over this, and we could instead use &mut [Quantum].

[john01dav]

I disagree with the proposal to use &[u8]. For formats that nicely fit into &[u32], like we exclusively use right now, it's very convenient to have this type — and all associated alignment requirements — specified in the type.

One way that we could nonetheless support different types is to have a Surface trait with multiple implementations, and then a SurfaceU8, SurfaceU32, etc. trait for each type. For example, you can have SurfaceRgbU32 implements Surface, SurfaceU32, and SurfaceU8 (because &[u32] can be bytemuck-casted to &[u8]), and others as applicable. But, SurfaceGreyscaleU8 would just implement SurfaceU8. You could also do something like the image crate where you have Surface or Surface instead of fully separate classes.

[ids1024]

I think we should zero out the buffer.

I mean we'd zero the buffer when it is allocated or grown, but after swapping buffers it would have the contents of a previous draw. But with single/double/triple buffering that isn't guaranteed either.

Oh right, I forgot the API is currently using &[u32]. Is that what drawing libraries like tiny-skia expect? It could be good to have a way to get the buffer as either type, though the caller can just use something like bytemuck. As for formats other that aren't 32-bit per pixel... not sure what sort of API we'd want for that. We'd not want to represent it as a &[u32], but we'd also need some API for negotiating what format the backend supports. Though even with just 32-bit RGB formats, we want some way to choose RGBA vs RGBX (https://github.com/rust-windowing/softbuffer/issues/17).

SurfaceU8/SurfaceU32/ect could be handled with a bytes per pixel const generic and such, though you quickly get into arcane type errors and things Rust may not support yet.

For https://github.com/rust-windowing/softbuffer/issues/36, we want a way to read the pixels that are in the front buffer, essentially? For Wayland this is easy enough. For other backends it might not be available in an easily readable way client-side, especially with a no-copy API. (If the pixel data was sent over a socket, or uploaded to GPU.)

[john01dav]

It could be good to have a way to get the buffer as either type, though the caller can just use something like bytemuck.

This may seem obvious to some, but I think that it's important to point out that u8 is not required to be aligned to u32 boundaries. Bytemuck does runtime checks for alignment when casting, but there's no guarantee unless we use the larger types. But, of course, the larger types are guaranteed to be aligned compatibly with the smaller types. That's why I want SurfaceU32/SurfaceU8 etc. That way, we can auto-implement the smaller types for the larger ones via bytemuck, thus exposing all relevant alignment guarantees, and being flexible to users.

[notgull]

Closed by #65