gfx-rs / portability

Vulkan Portability Implementation
Mozilla Public License 2.0
382 stars 25 forks source link

Port to the new swapchain model #210

Closed kvark closed 3 years ago

kvark commented 4 years ago

Closes #208 Fixes #212 Also has general fixes to:

The PR is inviting for discussion: @grovesNL @msiglreith I'm not 100% sure what to do yet.

The Good:

  1. Being able to remove the old swapchain from gfx-rs COMPLETELY. It was hacky on all the backends other than Vulkan, quite complicated on Metal, and didn't work very in general.
  2. Fullscreen support! It's not ideal yet (occasional hitches), but it was pretty much non-functional before. In general, stuff that runs is more robust this change.

The Bad:

  1. Portability implementation is a bit more complex now, but not extraordinary so. In general, I think it's a win if we move some complication out of gfx-rs into gfx-portability, since gfx-rs has more users, it's more important to keep clean.
  2. We can't support recording a render pass that uses a swapchain image if either:
    • the image is not acquired at the moment of recording
    • the command buffer is re-usable. We expect the users to strictly acquire-record-submit-present.

The Ugly:

  1. Our KHR swapchain implementation is more limited: no support for any usage other than COLOR_ATTACHMENT. This is technically valid in Vulkan, but for some reason many apps expect to transfer to/from swapchain images. So this is fine for the matter of CTS and VkPI, but can introduce some friction in real world testing.
  2. The new swapchain model itself needs to be evolved a bit more, according to https://github.com/gfx-rs/gfx/issues/3184 . We don't know how exactly: there is both a risk (that we'll need to redo something here) and an opportunity (that we'll support other usages, for example).
kvark commented 4 years ago
Compatibility list: app status changes needed
LunarG cube :no_entry: non-acquired use of swapchain
dota2 :no_entry: https://github.com/ValveSoftware/Dota-2-Vulkan/issues/335
FPS Infinite :no_entry: non-acquired use of swapchain
gzDoom :white_check_mark:
vkQuake1 :white_check_mark:
vkQuake2 :white_check_mark: https://github.com/kondrak/vkQuake2/issues/90
vkQuake3 :construction: https://github.com/suijingfeng/vkQuake3/issues/8
filament :white_check_mark: https://github.com/google/filament/issues/2342
VSand :white_check_mark: https://github.com/Ealrann/VSand/issues/21
Diligent engine :white_check_mark: https://github.com/gfx-rs/gfx/pull/3206 https://github.com/gfx-rs/gfx/pull/3207
RPCS3 :construction: many features missing
Dolphin :white_check_mark:
msiglreith commented 4 years ago

How does this interact with present modes? In particular, out of order presentation and acquiring multiple swapchain images?

kvark commented 4 years ago

out of order presentation

Should be unaffected.

acquiring multiple swapchain images

multiple images of the same swapchain - not supported by this path

kvark commented 4 years ago

Given the amount of testing I'm doing right now, will be confident to release portability-0.8 right after this merges, if we proceed 😄

kvark commented 3 years ago

We can't hold this off forever. bors r+

bors[bot] commented 3 years ago

Timed out.

kvark commented 3 years ago

bors retry

bors[bot] commented 3 years ago

Timed out.