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

Screen shot #326

Closed
wants to merge 17 commits into from
Closed

Screen shot #326

wants to merge 17 commits into from

Conversation

freesig
Copy link
Collaborator

@freesig freesig commented May 31, 2019

Attempt at a screen shot

@freesig
Copy link
Collaborator Author

freesig commented Jun 2, 2019

Ok I've spent the last two days on this and I keep running into things that prevent it from working.
Keep in mind I'm new to vulkan
Also image crate only accepts &[u8]
The main blocking issue
Your swapchain is usually in BGRA and the image crate only works with RGBA (Its says it does BGRA but that is a lie for png).
You could do this on the cpu but that's slow as.
You can us vkCmdBlitImage but this isn't guaranteed to even be supported on the gpu.
This I've tried:

  • Create a render pass that takes the image and then rights it two an another image with the right order and then copy to a buffer. This just plain didn't work (I tried to debug it but the pixels weren't being written)
  • Use a Texel buffer in the render pass so I can write directly two it. Couldn't get the Texel buffer to work as u8.
  • I though maybe these issues were because I was trying to use a renderpass without presenting so I tried the trusty compute. Well GLSL doesn't have u8 only u32. So I cant just write to a u8 buffer (Atleast I coudnt figure out how to).
  • So I try the same thing but to an image and then run a copy from the image to the buffer. All I get is an obscure Unsupported Format from vulkano in reference to the image. It's just got transfer_source and storage so I don't see why that doesn't work.

Obviously the explicitness of vulkan is awesome but dam this is frustrating. I just want a way to get the dam image into a buffer.

@freesig
Copy link
Collaborator Author

freesig commented Jun 2, 2019

Ok so I've learnt one thing. An image copy or image to buffer copy will not take care of the format transitions. So you can't go from uint to float for example.

@freesig
Copy link
Collaborator Author

freesig commented Jun 2, 2019

I think the best option here is to figure out how to go from floats to 8 bit uints in glsl.
this might help

@freesig
Copy link
Collaborator Author

freesig commented Jun 2, 2019

Also another way to solve this would be to figure out how to get the image crate to save an image that isn't 8 bit without running through the whole image data again.
GLSL doesn't have bitwise shift.
Does anyone have any idea how you are supposed to take an image in float format and convert it to an image / buffer in uint format?

  • Copys don't do conversion
  • GLSL can't even talk about an 8bit uint so I can't use that a compute.
  • Apparently a storage image will do the conversion on imageStore() but if I try and create a rgba8ui then vulkano throws a panic in the shader macro saying that it isn't a float type. Event though the image that is attached to that framebuffer is R8G8B8A8Uint. So maybe it's a bug in vulkano?
    Seems weird that you wouldn't be able to use a uint storage image in compute shaders.
  • The only way I can think of is using the image as a color attachment on a render pass as what it draws into. This didn't work last time but maybe I had something wrong.
    Ok even just setting up a vanilla renderpass that sets f_color = vec4(1.0); to draw to 8bit image just gives all zeros.

@freesig
Copy link
Collaborator Author

freesig commented Jun 3, 2019

Ok I was about to give up and just use the vkCmdBlitImage function and only support cards that support this but apparently event vkCmdBlitImage doesn't support converting from float to uint.
I'm actually stuck on this now.

@freesig
Copy link
Collaborator Author

freesig commented Jun 3, 2019

Ok I have a working screen shot with the latest commit using compute.
Still a few issues to solve:

  • Timing is out and I still get access errors but I'll dig into this.
  • Need to fush the images incase only one is taken.
  • Need to update sizes in shader and work groups on resize
  • The colors are off. I suspect this is the type of colors being used @mitchmindtree do you know how this might be fixed. Heres what it looks like. Actually I fixed it.
    Screenshot from 2019-06-03 14-28-46
    The top left is nannou the bottom right is the screenshot
    This is how I get the colours now:
uvec4 col = uvec4(uint(255*t.x), uint(255*t.y), uint(255*t.z), uint(255*t.w));
  • Need to actually wait for write to file to finish on exit
  • no hard coded dimensions

@freesig freesig marked this pull request as ready for review June 3, 2019 11:12
@freesig
Copy link
Collaborator Author

freesig commented Jun 3, 2019

Ok this is working pretty well now.
There's a few this that aren't ideal but that's due to not having access to the inner fence.
Also vulkano doesn't give use access to query the supported image formats so I've just had to guess that these formats will work for everyone.

@freesig freesig mentioned this pull request Jun 14, 2019
3 tasks
@tpltnt
Copy link
Contributor

tpltnt commented Jun 28, 2019

When can we merge this?

@tpltnt
Copy link
Contributor

tpltnt commented Jul 5, 2019

ping

@freesig
Copy link
Collaborator Author

freesig commented Jul 10, 2019

I'm not even sure if this should be merged to master. I think it's good for a reference implementation but ultimately it would be better to include most of the functionality into nannou directly.
I like making examples as a way of testing functionality but I'm not sure if it's always a good idea to include them into nannou master as they will eventually become out-of-date and I'm not sure how useful they would be as an "example" but I could be wrong.
What do people think?

@freesig
Copy link
Collaborator Author

freesig commented Aug 1, 2019

This fails on an debug_assert because the resolve renderpass contains images with different formats.
But I need the image that copies to the buffer to be Uint and the swapchain is usually in Float.
Any Ideas??

@freesig
Copy link
Collaborator Author

freesig commented Aug 2, 2019

It also looks like this isn't copying the colors correctly. I think it's probably related to the above issue of different formats.
Todo

  • Find out the correct way to convert Float to Uint
  • Implement it and check that the colors are fixed

@mitchmindtree
Copy link
Member

Hey @freesig, seems like this is getting close at least!

Find out the correct way to convert Float to Uint

Are you sure the image requires the R8G8B8A8Uint format? E.g. the images used for the vk_image examples, the microbe mirrors particles and this current project are being loaded as R8G8B8A8Srgb - maybe it's worth trying this format instead? From my understanding images are almost always stored in non-linear sRGB except for special cases.

By the way, in case it helps, the frame to swapchain image writing process looks like this:

Image Colour Space Format
MSAA frame image Linear sRGBA vk::Format::R16G16B16A16Unorm
Resolved frame image Linear sRGBA vk::Format::R16G16B16A16Unorm
Swapchain image Almost always Non-linear sRGBA Almost always vk::Format::B8G8R8A8Srgb

I haven't looked closely at the current implementation, but I'm imagining the screenshot approach would look kind of similar where you add commands to resolve the MSAA frame image to a non-MSAA image and then blit it to the target storage image?

@freesig
Copy link
Collaborator Author

freesig commented Aug 2, 2019

Yeh I need Uint format for the PNG writer because it writes as &[u8]

I haven't looked closely at the current implementation, but I'm imagining the screenshot approach would look kind of similar where you add commands to resolve the MSAA frame image to a non-MSAA image and then blit it to the target storage image?

Yeh that's exactly what's happening.
The problem is in the resolve step. It's not happy resolving to a different format but as I'm writing this it has occurred to me that I should just sample the MSAA image.
Although I don't think you can sample a MSAA image so maybe I need two subpasses.

  1. Resolve
  2. Sample resolved image as UInt

@mitchmindtree
Copy link
Member

so maybe I need two subpasses.

  • Resolve
  • Sample resolved image as UInt

This makes sense to me!

@freesig
Copy link
Collaborator Author

freesig commented Aug 18, 2019

Todo:

  • Make resolve pass.
  • Add intermediate image
  • Sample from intermediate image (Add sampler)
  • Add sampling to uint pass
  • Copy from storage image to buffer
  • Change storage image to regular image
  • Check if unorm to uint needs blit
  • Plug resolve image into blit that dst is a inter image
  • Merge WIP branch into this branch

Found:

  • Can't use subpasses because the BLIT must happen between them and BLIT is outside rp only.

@mitchmindtree
Copy link
Member

I just ran this locally and the color is looking fine to me! Awesome stuff @freesig :)

One thing I did notice is that it looks like the texture coordinates are double what they should be. Here's a demonstration of the screenshots I'm getting:

screenshot5

screenshot4

If I change

tex_coords = position + vec2(0.5);

to

tex_coords = position * 0.5 + vec2(0.5);

then it looks as I'd expect:

screenshot1

I think my modification makes sense, as the incoming position coords are -1 to 1, and after position * 0.5 + vec2(0.5) they're 0 to 1 as the sampler2D would expect.

It looks like maybe the frag shader was copied from the vk_image example, which is also incorrect by the looks of it, however it's not as noticeable in that example as the edges are black.

Did you or @MacTuitui run into this at all?

@tpltnt
Copy link
Contributor

tpltnt commented Aug 18, 2019

This looks very encouraging :)

@MacTuitui
Copy link
Contributor

Works flawlessly with the tex_coords fix on my MacBook Pro (using @freesig vulkano patch for disccrete GPUs). That is a huge win for me. Thanks a lot!
If further optimization is needed, one thing that might be worth looking into is the DPI scaling as right now I end up with images twice the size of the window (retina display here), and maybe spawn a thread for saving the png as it should be completely independent from the other stuff (I'll see if I can manage that on my own!).

@freesig
Copy link
Collaborator Author

freesig commented Aug 20, 2019

@mitchmindtree Hey nice catch, I did indeed copy that code. I'll push a fix.
@MacTuitui I don't have a high dpi screen to test this, I think @mitchmindtree does, did you notice any larger pics with high dpi?

maybe spawn a thread for saving the png as it should be completely independent from the other stuff

This is kind of true. It's already on another thread however you can't really make them independent because if you just allow the rendering thread to run ahead of the saving thread then you will end up with a huge ever growing buffer of images waiting to be saved. It would probably crash your program. You either have to make the rendering thread wait or skip frames.
One thing that would be a huge win is getting gstreamer connected because then we could skip the whole png step but that will take a bit of work.

@mitchmindtree
Copy link
Member

Closing now that we have the draw_capture.rs and draw_capture_hi_res.rs examples working with wgpu.

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

Successfully merging this pull request may close these issues.

4 participants