webgpu-native / webgpu-headers

https://webgpu-native.github.io/webgpu-headers/
BSD 3-Clause "New" or "Revised" License
379 stars 44 forks source link

proposal: swapchain & presentation rework #197

Closed rajveermalviya closed 1 year ago

rajveermalviya commented 1 year ago

Context

Currently the WGPUSwapChain API diverts from the js spec; this proposal tries to match the js spec while also maintaining the functionality that is required in the native context.

Keep in mind this proposal is mostly what wgpu-rs already exposes so it tries to match it.

Update present mode enum to add automatic modes

// matches wgpu
// https://docs.rs/wgpu-types/latest/wgpu_types/enum.PresentMode.html
typedef enum WGPUPresentMode {
    WGPUPresentMode_Fifo = 0x00000000, // default
    // VK_PRESENT_MODE_FIFO_RELAXED_KHR
    WGPUPresentMode_FifoRelaxed = 0x00000001, // new
    WGPUPresentMode_Immediate = 0x00000002,
    WGPUPresentMode_Mailbox = 0x00000003,
    WGPUPresentMode_Force32 = 0x7FFFFFFF
} WGPUPresentMode;

Remove WGPUSwapChain (and merge it's interface into Surface api)

In the js spec this was done a while ago. Fixes: https://github.com/webgpu-native/webgpu-headers/issues/88

// matches wgpu
// https://docs.rs/wgpu-types/latest/wgpu_types/enum.CompositeAlphaMode.html
typedef enum WGPUCompositeAlphaMode {
    // Chooses between Opaque & Inherit whichever is available.
    WGPUCompositeAlphaMode_Auto = 0x00000000,
    WGPUCompositeAlphaMode_Opaque = 0x00000001,
    WGPUCompositeAlphaMode_PreMultiplied = 0x00000002,
    WGPUCompositeAlphaMode_UnPreMultiplied = 0x00000003,
    WGPUCompositeAlphaMode_Inherit = 0x00000004,
    WGPUCompositeAlphaMode_Force32 = 0x7FFFFFFF
} WGPUCompositeAlphaMode;

// Replaces WGPUSwapChainDescriptor
typedef struct WGPUSurfaceConfiguration {
    WGPUChainedStruct const * nextInChain;
    WGPUDevice device;
    WGPUTextureFormat format;
    WGPUTextureUsage usage;
    size_t viewFormatCount;
    WGPUTextureFormat const * viewFormats;
    WGPUCompositeAlphaMode alphaMode;

    uint32_t width;
    uint32_t height;
    WGPUPresentMode presentMode;
} WGPUSurfaceConfiguration;

// Replaces wgpuDeviceCreateSwapChain
void wgpuSurfaceConfigure(WGPUSurface surface, WGPUSurfaceConfiguration const * config);
// This function probably should be skipped but we need to match js spec.
void wgpuSurfaceUnconfigure(WGPUSurface surface);

Provide current Texture instead of TextureView

Fixes: https://github.com/webgpu-native/webgpu-headers/issues/89

typedef enum WGPUSurfaceGetCurrentTextureError {
    WGPUSurfaceGetCurrentTextureError_NoError = 0x00000000,
    WGPUSurfaceGetCurrentTextureError_Timeout = 0x00000001,
    WGPUSurfaceGetCurrentTextureError_Outdated = 0x00000002,
    WGPUSurfaceGetCurrentTextureError_Lost = 0x00000003,
    WGPUSurfaceGetCurrentTextureError_OutOfMemory = 0x00000004,
    WGPUSurfaceGetCurrentTextureError_DeviceLost = 0x00000005,
    WGPUSurfaceGetCurrentTextureError_Force32 = 0x7FFFFFFF
} WGPUSurfaceGetCurrentTextureError;

typedef struct WGPUSurfaceTexture {
    WGPUTexture texture;
    // If suboptimal is true presentation will still succeed,
    // but the internal swapchain is no longer configured optimally
    // for the surface it targets. Applications should recreate
    // their swapchain at the next convenient opportunity.
    bool suboptimal;
    WGPUSurfaceGetCurrentTextureError error;
} WGPUSurfaceTexture;

// Replaces wgpuSwapChainGetCurrentTextureView
void wgpuSurfaceGetCurrentTexture(WGPUSurface surface, WGPUSurfaceTexture * surfaceTexture);

Provide a way to query information about the surface

typedef struct WGPUSurfaceCapabilities {
    WGPUChainedStructOut * nextInChain;
    size_t formatCount;
    WGPUTextureFormat * formats;
    size_t presentModeCount;
    WGPUPresentMode * presentModes;
    size_t alphaModeCount;
    WGPUCompositeAlphaMode * alphaModes;
} WGPUSurfaceCapabilities;

void wgpuSurfaceGetCapabilities(WGPUSurface surface, WGPUAdapter adapter, WGPUSurfaceCapabilities * capabilities);
void wgpuSurfaceCapabilitiesFreeMembers(WGPUSurfaceCapabilities capabilities);

Note: wgpuSurfaceGetCapabilities could replace current wgpuSurfaceGetPreferredFormat by guaranteeing that first format is the preferred one. But removing wgpuSurfaceGetPreferredFormat will divert from js spec.

Move Present function to Surface

// DevicePushErrorScope/DevicePopErrorScope can be used to catch errors
void wgpuSurfacePresent(WGPUSurface surface);

open discussion: presentation extensions

frame pacing & damage support ref: https://github.com/gfx-rs/wgpu/issues/2869

Kangz commented 1 year ago

This seems like a pretty good direction, anything not commented on looks good!

Update present mode enum to add automatic modes

Why is FifoRelaxed in AutoVsync? It can result in visible tearing in some cases, is that ok? Can we guarantee Fifo is always supported?

Remove WGPUSwapChain (and merge it's interface into Surface api)

Why do we need all these various alpha modes? Are any guaranteed to be present? +1 for ColorSpaces, they could be used in other places like helper functions to implement importExternalTexture and copyEI2T. In general for higher level language bindings it would be nice if most things could have default values. (except present mode, format and width/height?). Keeping Configure still makes sense as it removes a reference to the WGPUDevice and lets the CPU allocation be freed in some cases. (similar reason to why we have this in JS).

Provide current Texture instead of TextureView

Why do we have an errorMsg? Can't this use the regular facilities for errors on the WGPUDevice? Also the FreeMembers would depend on the resolution to that.

Provide a way to query information about the surface

Why not use a ChainedStructOut and a FreeMembers method? It seems likely that we are going to add things to this structure over time. IMHO we could have a duplicate method for GetPreferredFormat just to match JS.

rajveermalviya commented 1 year ago

Why is FifoRelaxed in AutoVsync? It can result in visible tearing in some cases, is that ok?

Up for debate

Can we guarantee Fifo is always supported?

Vulkan requires it

Why do we need all these various alpha modes?

It matches VkCompositeAlphaFlagBitsKHR.

Are any guaranteed to be present?

No, hence Auto; it will choose between Opaque & Inherit, whichever is available. (adding this info in proposal) Vulkan on Android only advertises Inherit

Why do we have an errorMsg? Can't this use the regular facilities for errors on the WGPUDevice? Also the FreeMembers would depend on the resolution to that.

Re:Question: What do you think about the alternative API proposed in that section, it's much simpler and removes the struct?

Why not use a ChainedStructOut and a FreeMembers method?

Yeah, I wanted to keep the API simpler (one shot). Guess we should use FreeMembers there to keep things consistent. (updating the proposal).

Kangz commented 1 year ago

Re:Question: What do you think about the alternative API proposed in that section, it's much simpler and removes the struct?

Maybe it could be a return enum with also SurfaceLost and DeviceLost, but that would work!

kainino0x commented 1 year ago

Why do we have an errorMsg? Can't this use the regular facilities for errors on the WGPUDevice? Also the FreeMembers would depend on the resolution to that.

Re:Question: What do you think about the alternative API proposed in that section, it's much simpler and removes the struct?

I think we liked the pattern we described here for GetMappedRange: https://github.com/webgpu-native/webgpu-headers/issues/143#issuecomment-1612208345 So maybe same for GetCurrentTexture?

kainino0x commented 1 year ago

There was a lot of discussion on this today so I'll paste the raw meeting notes (not totally complete but should have captured the important stuff):

rajveermalviya commented 1 year ago

(updated the proposal)

kainino0x commented 1 year ago

Looks good so far! Filed #200 for color spaces.

I think the big thing left to discuss is alpha modes. We might want to split that one to a new issue as well, if we don't find the answers yet.

kainino0x commented 1 year ago
rajveermalviya commented 1 year ago

Updated the proposal and #203 incorporates changes in the header.