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

Alternative swapchain model #2863

Closed
kvark opened this issue Jun 24, 2019 · 6 comments · Fixed by #2882
Closed

Alternative swapchain model #2863

kvark opened this issue Jun 24, 2019 · 6 comments · Fixed by #2882

Comments

@kvark
Copy link
Member

kvark commented Jun 24, 2019

Situation

We have a lot of swapchain-related issues:

  • it doesn't map to Metal:
    • the current solution is hacky and occasionally fires back, with things like full-screen, or orientation change on iOS
  • it doesn't map to most GL WSI
    • GL doesn't let you us VkImage of the framebuffer, although may be possible in some configurations (i.e. Wayland, or IOSurface-backed WSI)
  • it doesn't map nice to DX12
    • users aren't allowed to do anything with any other image than currently acquired

it's really the most rough API piece of all the things we have... and the problem comes from how Vulkan exposes the swapchain. Good thing is - designers of Vulkan anticipated the WSI being the pain spot and excluded it from the core specification.

Idea

What if we expose a very simple (and limited) API for the swapchain: you get the next frame as as a structure that can be borrowed as Framebuffer, you draw to it (only using one-time-submit command buffers), and you pass it back by value on present(). That's it. No enumeration of images ahead of time, no semaphores, less possible errors on the way and definitely less pain for the backends to implement it.

My bet is that this simple model would work for most of the client apps. I'd like to hear from our users if it wouldn't work. The only known use that doesn't it this scenario is gfx-portability... so my hope is to structure that simple API in such a way that the current complex logic of Metal backend (mapping Vulkan to Metal) is basically moved and adopted there, in gfx-portability. It could in fact even be simpler since at this level we have the leverage of providing more hidden state info from the backends.

Note: having render-only swapchain images is certainly more limited than regular Vulkan WSI, where you could ask for supported usages and potentially copy to/from the swapchain images, even bind them as storage, etc. However, there is evidence that presentation works most efficiently if it's only rendered to: Metal has a special flag "isFramebufferOnly" indicating possible optimizations, and DXGI is known to operate in a different mode if you need readbacks and such.

@zakarumych
Copy link

zakarumych commented Jun 24, 2019

What about use cases where surface is not the only attachment?
On mobiles we hope to use single render pass with few subpasses where last one will render final image to surface, so there will be lot of attachments (image views) bound to one framebuffer.

@mtak-
Copy link
Contributor

mtak- commented Jun 24, 2019

No enumeration of images ahead of time.

I am hugely in favor of this. iOS doesn't work well with the attempts to acquire all of the swapchain images ahead of time. On orientation change, iOS often hands out unique, seemingly one time use drawables. This causes swapchain invalidation to occur a couple times per orientation change, and the blitting from the swapchain image at index 0 to occasionally blit garbage onto the screen (all red). The fact that swapchain recreation requires the presentation of a drawable users have no control over makes me sad.

I'd like to hear from our users if it wouldn't work.

For us it's the opposite. The current situation doesn't work well.

@kvark
Copy link
Member Author

kvark commented Jun 24, 2019

Given feedback so far, the API candidate looks like:

trait B::NewSwapchain {
  // Get a new image or throw an error if the dimensions change
  unsafe fn acquire_image(&mut self) -> Result<B::SwapchainImage, AcquireError>;
}

trait B::Queue {
      // Notice the by-value move of the swapchain image
      // Side note: I don't think there is much value in presenting multiple things at once... It's only supported natively in Vulkan and I haven't yet seen a case where it would make a difference.
      unsafe fn present<'a, S, Iw>(
        &mut self,
        swapchain: &B::NewSwapchain,
        image: B::SwapchainImage,
        wait_semaphores: Iw,
    ) -> Result<Option<Suboptimal>, PresentError>
    where
        S: 'a + Borrow<B::Semaphore>,
        Iw: IntoIterator<Item = &'a S>;
}

trait B::SwapchainImage: Borrow<B::ImageView> {
  // Backend would promise to return images with recently used X number of unique IDs *most often* (but not every time), where X is the swapchain image count.
  fn unique_id(&self) -> u64;
}

And we'd keep the old API for a while, until it's more clear what the perspectives are.

@mtak-
Copy link
Contributor

mtak- commented Jun 24, 2019

fn unique_id(&self) -> u64; Is this likely just a casted pointer value? Sounds good to me!

@zakarumych
Copy link

No. Pointer value won't be unique.

@msiglreith
Copy link
Contributor

@kvark Thanks for the draft, looks good to me so far. Creation/resizing will be interesting.

  • My immediate thought would be how the vulkan backend would handle the acquistion semaphores.
  • I'm wondering if there id should maybe be splitted into a generation and id segment by design? This might be easier to handle for the user for dealing with swapchain image depending resources like framebuffers.

@kvark kvark removed the api: hal label Jun 26, 2019
bors bot added a commit that referenced this issue Aug 13, 2019
2882: Presentation Surface API r=msiglreith a=kvark

Fixes #2863
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: Metal
- [x] `rustfmt` run on changed code


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
bors bot added a commit that referenced this issue Aug 13, 2019
2882: Presentation Surface API r=msiglreith a=kvark

Fixes #2863
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: Metal
- [x] `rustfmt` run on changed code


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
bors bot added a commit that referenced this issue Aug 14, 2019
2882: Presentation Surface API r=msiglreith a=kvark

Fixes #2863
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: Metal
- [x] `rustfmt` run on changed code


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
bors bot added a commit that referenced this issue Aug 14, 2019
2882: Presentation Surface API r=msiglreith a=kvark

Fixes #2863
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: Metal
- [x] `rustfmt` run on changed code


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
@bors bors bot closed this as completed in #2882 Aug 14, 2019
@kvark kvark unpinned this issue Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants