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

Swapchain / Surface questions #24

Open
Kangz opened this issue Nov 12, 2019 · 10 comments
Open

Swapchain / Surface questions #24

Kangz opened this issue Nov 12, 2019 · 10 comments
Assignees
Labels
has resolution Issue is resolved, just needs to be done presentation Presenting images to surfaces like windows and canvases

Comments

@Kangz
Copy link
Collaborator

Kangz commented Nov 12, 2019

I've started to implement swapchains in Dawn and ran into a bunch of questions.

Q: How do we report surface creation errors?
Tentative-A: We return a nullptr surface? But it doesn't allow reporting a string message to explain what went wrong. Alternatively surface creation can never go wrong and errors are always exposed when the swapchain is created on the surface.

Q: Can we have multiple swapchains on the same surface?
A: No, there is always a single current swapchain. Creating a new swapchain on the surface invalidates the previous one (this includes destroying the "current texture").

Q: Can we have subsequent swapchains on a surface be on different backends?
Tentative-A: No, it is a validation error. (this is to not deal with backend-compatibility stuff)

Q: Do we have the equivalent of GPUCanvasContext.getPrefferredSwapChainFormat?
A: Maaaybe? But then is it synchronous or async? Or we could just have a list of formats that we guarantee always work (and do blits)?

Q: How do we handle window resizes and minimization?
A: No clue, Vulkan has the "outdated" swapchain concept, but there's no clear way to do this in webgpu.h that would also match the Web where the application sets the size of the canvas directly. Also we should take care on Windows, where Vulkan for example requires the size of the swapchain to match exactly the size of the window.

Q: Do we do things for the user like resize blits, format blits etc?

@kainino0x kainino0x added the presentation Presenting images to surfaces like windows and canvases label Jun 29, 2023
@kainino0x
Copy link
Collaborator

@rajveermalviya there are a few things left in this issue that I think are not answered by #203, you might be interested in trying to resolve them

@kainino0x
Copy link
Collaborator

A note from the meeting:

  • RM: Another thing left out is what to do if the user wants to skip presenting a frame. What happens if you GetCurrentTexture but then don’t Present?

@kainino0x kainino0x added the !discuss Needs discussion (at meeting or online) label Nov 10, 2023
@rajveermalviya
Copy link
Collaborator

Most of these have already been answered and implmented but coming back to it today,
there may be different answers for some.

How do we report surface creation errors?

Without changes in the header, we can't do more than returning null and log the error, or defer reporting error till surface configure.

But if we do allow changes then:

Can we have multiple swapchains on the same surface?

Current webgpu.h doesn't expose swapchains anymore.

Do we have the equivalent of GPUCanvasContext.getPrefferredSwapChainFormat?

It's already in webgpu.h as wgpuSurfaceGetPreferredFormat. In wgpu it prefers srgb supported formats over linear.

How do we handle window resizes and minimization?

webgpu.h currently contains WGPUSurfaceGetCurrentTextureStatus which has Timeout, Outdated and Lost - but AFAIK only mappable to vulkan.
For resizes some rules that [could be]/[are already] enforced:

  • Validation error for surface.configure() if width/height == 0
  • Validation error for getCurrentTexture() if configured surface size != actual window size. (expensive, AppKit doesn't like touching surface from non-main thread)

Do we do things for the user like resize blits, format blits etc?

Need more context here but in general, IMO prefer avoiding implicit behavior, but makes sense from webgpu POV.

@kainino0x
Copy link
Collaborator

Nov 16 meeting:

  • Do we do things for the user like resize blits, format blits etc?
    • RM: Need more context here but in general, IMO prefer avoiding implicit behavior, but makes sense from webgpu POV.
    • KN: May not have to deal with this for now, if we error, can see if there is developer demand for doing fixups like these
    • CF: Feel both of those situations are things we don’t want to deal with, shouldn’t blit things to clean up the mess of resizing
    • RM: There’s no race condition in Wayland, but there is in X
    • CF: Don’t think we have anyone who is using Linux regularly
    • RM: I did, but now using mac.
  • RM: Another thing left out is what to do if the user wants to skip presenting a frame. What happens if you GetCurrentTexture but then don’t Present?

    • CF: Had a plan for this for wgpu. On D3D12 you can put a texture back since acquiring isn’t a real operation. In Vulkan you can’t do that. Was thinking if you drop a surface texture, we save it off to return next time you acquire again.
    • KN: Wouldn’t even have to do anything on drop because we’re supposed to return the same thing again anyway
    • CF: May not be possible on native
    • CF: Does Mac let surfaces get promoted to scanout?
    • KN/AE: it should, e.g. with framebufferOnly
    • KN: Do we want to try to implement the Web thing of returning the same surface texture until present?
    • CF: Native APIs … Vulkan has vkAcquireNextImageKHR
    • KN: Could return null (error) and the application is expected to reuse the texture they got before
    • CF: Acquire-acquire-present feels like an error. Acquire-drop-acquire-present should be OK
    • CF: There’s also incremental present in GL and DXGI (but not new Win11 API)
      • RM: Wayland also has it
      • Vulkan has VK_KHR_incremental_present
      • CF: A situation where people need to know exactly which swapchain image they’re dealing with
    • Acquire-acquire returns null with implementation defined logging
    • Acquire-drop will not present a frame (old frame stays live)
      • Drop = ?
    • AE: Drop the texture and all the bind groups and command buffers that reference it?
    • CF: No… just drop the object
      • KN: Destroy?
    • CF: D3D can’t use swapchain textures as storage etc
    • CF: texture views need to be destroyed
    • AE: technically no because CreateRenderTargetView just gives you a CPU_DESCRIPTOR_HANDLE
    • CF: oh, wgpu holds a strong ref to the texture next to the CPU_DESCRIPTOR_HANDLE to avoid use-after-free
    • CF: I think I just realized wgpu has a race. Acquire, create view, present, write into the view.
    • AE: Destroy it on present
    • KN: These textures have to be destroyable even if the destroy extension isn’t available
    • CF: OK because “destroy” is not actually freeing memory. just setting an atomic bool
    • in vk(/d3d?) you CAN acquire two frames. also you can encode before acquiring.
    • Unify this with the Web API. Dropping the surface texture does nothing. You can GetCurrentTexture multiple times and get the same thing each time until you call Present (or Present is done implicitly by the browser).
    • (discussion of some other stuff)

@kainino0x
Copy link
Collaborator

How do we report surface creation errors?

Without changes in the header, we can't do more than returning null and log the error, or defer reporting error till surface configure.

I think this is sufficient since it should be possible to write a program that never hits an error condition. Maybe there can be dynamic errors but nullptr is probably enough for an application to know it needs to retry.

Do we have the equivalent of GPUCanvasContext.getPrefferredSwapChainFormat?

It's already in webgpu.h as wgpuSurfaceGetPreferredFormat. In wgpu it prefers srgb supported formats over linear.

One note about this - in the JS API, to avoid breaking content, getPreferredCanvasFormat will only ever choose from rgba8unorm and bgra8unorm. There will likely be some way you can opt into getting other formats, for example navigator.gpu.getPreferredCanvasFormat({ hdr: true }) that could return rgba16float or rgb10a2unorm, or { srgb: true } which could return rgba8unorm-srgb or bgra8unorm-srgb. Or maybe it looks more like getPreferredCanvasFormat(['rgba8unorm-srgb', 'bgra8unorm-srgb']) or getPreferredCanvasFormat(GPUPreferredFormat.SRGB | GPUPreferredFormat.HDR).

Regardless, we are likely to need some configurability here in the future that we don't have right now. If wgpu already wants to return srgb formats here (which differs from the JS API) then maybe we should make it configurable now so the default behavior can match.

@kainino0x kainino0x added has resolution Issue is resolved, just needs to be done and removed !discuss Needs discussion (at meeting or online) labels Dec 14, 2023
@Kangz
Copy link
Collaborator Author

Kangz commented May 22, 2024

Additional question from #295

Should a WGPUTextureUsage be added to WGPUSurfaceCapabilities with guarantees like RENDER_ATTACHMENT|TEXTURE_BINDING|COPY_SRC|COPY_DST?

Is the following defined to always work without additional waits? At least on Vulkan and D3D12 it will require waiting for the GPU on WGPUSurfaceRelease:

surface = wgpuInstanceCreateSurface(sameParams);
wgpuSurfaceConfigure(surface, ...);
wgpuSurfaceRelease(surface);

surface2 = wgpuInstanceCreateSurface(sameParams);

@kainino0x kainino0x added !discuss Needs discussion (at meeting or online) and removed has resolution Issue is resolved, just needs to be done labels May 22, 2024
@kainino0x
Copy link
Collaborator

Should a WGPUTextureUsage be added to WGPUSurfaceCapabilities with guarantees like RENDER_ATTACHMENT|TEXTURE_BINDING|COPY_SRC|COPY_DST?

Could the supported usages ever change according to other settings, like which format/alphaMode/presentMode is used? Or should there be no additional restrictions over what the device can do (e.g. whether it has bgra8unorm-storage)?

@Kangz
Copy link
Collaborator Author

Kangz commented May 29, 2024

I can only see the format causing restrictions. My assumption is that we'd validate the combination for format + usage the same way we do it for regular textures. Note that neither D3D12 nor Metal have a query for what a swapchain can do, and that Vulkan has the query return the allowed usages independently from the formats.

@kainino0x kainino0x removed the !discuss Needs discussion (at meeting or online) label Jun 6, 2024
@kainino0x
Copy link
Collaborator

May 23 meeting (sorry for delay):

  • Need to wait for idle before re-creating a surface?
    • CF: User wait for idle, or we do?
    • KN: Think that’s basically the question
    • CF/KN: Think it’s better if we deal with it for them. Impl block if needed

@kainino0x
Copy link
Collaborator

Jun 6 meeting:

  • “Should a WGPUTextureUsage be added to WGPUSurfaceCapabilities?”
    • KN: probably yes - Corentin has done this for Dawn already
    • KN: question if there are interactions with other things - like yes, you can get the usage, but not with format X or Y. Corentin seems to think it’s fine.
    • AE: Does the surface have to be configured before GetCapabilities? (Is that an input to the result?)
    • KN: Shouldn’t, you need this info to configure.
    • AE: Could be combinations not valid on the device.
    • KN: True, have info they need to figure that out though.
    • AE: Can add an extra GetCapabilities thing if we turn out to need more.
    • Sounds good let’s do it

@kainino0x kainino0x added the has resolution Issue is resolved, just needs to be done label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has resolution Issue is resolved, just needs to be done presentation Presenting images to surfaces like windows and canvases
Projects
None yet
Development

No branches or pull requests

3 participants