Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nice Buffer API #349

Open
3 tasks
freesig opened this issue Jun 14, 2019 · 0 comments
Open
3 tasks

Nice Buffer API #349

freesig opened this issue Jun 14, 2019 · 0 comments

Comments

@freesig
Copy link
Collaborator

freesig commented Jun 14, 2019

Problem

There is currently no way to copy a buffer of pixel data to the frame and there is no way to copy the contents of the swapchain to a buffer for recording the previous frame. This is possible using raw vulkan but a nice api would mean that it could be simple for users to do this.

Other Issues / Goals

  • Minimum of copies. We don't want an api that forces users to copy data when they don't have to. ie. exposing copy(my_data: &[u8]) is not a good idea because there data might not be in [u8] format and then they need to do a loop through the frame to make it [u8]. Then we copy it internally again. Instead expose the internal buffer that we use like copy() -> &mut [u8] and allow the user to do the copy.
    Actually on second thought this is actually adding an additional copy because the inner buffer is then copied again to the GPU buffer but I don't think we can expose the GPU buffer to the user as it needs to be sync'd. This could be done with buffer pools see Avoid allocating new buffers each frame for vertex #347
  • Frame could be in MSAA. On most platforms the frame is in 4xMSAA so there are 4x as many pixels per screen pixel. We either make the user fill a 4x buffer or we do some sort of upscalling for them. I think they should probably just fill the buffer in whatever msaa level they have as they probably want to just fill it at the higher resolution. But it could be useful to supply some helper functions to take an image and upscale it on copy.
    This is not an issue for screen capture because the swapchain image is already resolved.
  • BLIT issue. The swapchain image and therefor the frame are using in BGRA format (but not always) so we need a way to detect this (not currently possible in vulkano) and a way for an efficient BLIT to happen. vkCmdBlitImage is not guaranteed to be supported on all systems so we need a way to supply a back up. I have a compute shader that can do it in the screenshot pr
  • It's worth mentioning that gstreamer can handle most of these issues so when it comes to video we probably won't need to do it ourselves. But it might be worth profiling to see whos is faster.

Solution

Copy to the frame

I'm thinking something like

for (frame_pixel, my_pixel) in frame
  .copy_to_screen()
  .iter_mut()
  .zip(my_buffer.iter()) {
*frame_pixel = *my_pixel
}

Where copy_to_screen(&mut self) -> &mut [[u8; 4]]
The obvious issue here is frame can't do &mut but we could probably use a RefCell

Copy from the frame

frame
  copy_from_screen(my_buffer)

where copy_from_screen(&self, buffer: Vec<[u8; 4]>)

The other issue is we haven't got a way in this API to show that the buffers are ready. So this will probably need to be wrapped in something like ScreenCopy<Vec<[u8; 4]>>.
This would then impl get_inner(self) -> Result<Vec<[u8; 4]>> which blocks and try_inner which doesn't block. Although you could easily deadlock by calling get_inner() on the view().
You would probably need to pass it in like:

let my_buffer = ScreenCopy::new(some_vec_of_data);
frame
  copy_from_screen(my_buffer.clone())

where the clone is just cloning an Arc internally.

I've use [u8; 4] here for pixels but we could somehow take anything that impl IntoPixels but that might be too complex.

There's a lot here. Would love some thoughts from people when they have a chance.

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

No branches or pull requests

1 participant