replace WGPUSwapChain with methods for WGPUSurface #164

rajveermalviya · 2022-07-17T11:07:32Z

remove WGPUSwapChain as its already been removed from webgpu spec.

replaces wgpuDeviceCreateSwapChain with wgpuSurfaceConfigure
replaces wgpuSwapChainPresent with wgpuSurfacePresent
replaces wgpuSwapChainGetCurrentTextureView with wgpuSurfaceGetCurrentTexture
adds wgpuSurfaceUnconfigure

wgpu already supports most of these, except for wgpuSurfaceUnconfigure and WGPUSurfaceConfiguration.viewFormats

One question:

should WGPUSurfaceConfiguration also have alphaMode & colorSpace from GPUCanvasConfiguration?

Fixes #89
Fixes #88

Kangz

Overall direction to match the Web API sounds good, but I think we need more changes than just this.

Kangz · 2022-08-08T11:09:34Z

webgpu.h

-// Procs of SwapChain
-typedef WGPUTextureView (*WGPUProcSwapChainGetCurrentTextureView)(WGPUSwapChain swapChain);
-typedef void (*WGPUProcSwapChainPresent)(WGPUSwapChain swapChain);
+typedef void (*WGPUProcSurfaceConfigure)(WGPUSurface surface, WGPUDevice device, WGPUSurfaceConfiguration const * configuration);


nit: To match the Web API exactly, the Device would be in the WGPUSurfaceConfiguration.

Kangz · 2022-08-08T11:10:32Z

webgpu.h

-typedef void (*WGPUProcSwapChainPresent)(WGPUSwapChain swapChain);
+typedef void (*WGPUProcSurfaceConfigure)(WGPUSurface surface, WGPUDevice device, WGPUSurfaceConfiguration const * configuration);
+typedef void (*WGPUProcSurfaceUnconfigure)(WGPUSurface surface);
+typedef WGPUTexture (*WGPUProcGetCurrentTexture)(WGPUSurface surface, bool * suboptimal);


nit: Maybe it could be a separate call, but also optionally return some optimal parameters like the size of the texture?

wouldn't separate calls to wgpuTextureGetWidth & wgpuTextureGetHeight on the returned WGPUTexture suffice?

I don't think so. The texture you get should have exactly the size defined in WGPUSurfaceConfiguration otherwise the application doesn't really control when the resize happens and needs to be ready to resize other rendertargets any time. (plus if they forget to check then they'll get errors leading to rendering a black screen). At least in Dawn we have manual test that the resizing work, and it is implemented with a blit in the backends if it is needed (of course optimal would return false ^^)

Kangz · 2022-08-08T11:12:39Z

webgpu.h

+typedef void (*WGPUProcSurfaceConfigure)(WGPUSurface surface, WGPUDevice device, WGPUSurfaceConfiguration const * configuration);
+typedef void (*WGPUProcSurfaceUnconfigure)(WGPUSurface surface);
+typedef WGPUTexture (*WGPUProcGetCurrentTexture)(WGPUSurface surface, bool * suboptimal);
+typedef void (*WGPUProcSurfacePresent)(WGPUSurface surface);


There are other changes @cwfitzgerald made compared to the Web API for getting the supported formats and present mode. Maybe we should replicate some of them, and also re-discuss them in the context of a native API that's shared between projects.

#163 mentions wgpuSurfaceGetSupportedPresentModes.

for supported formats I think we can have

size_t wgpuSurfaceGetSupportedFormats(WGPUSurface surface, WGPUTextureFormats * formats);

Do we need some kind of option structure for example to ask low bit-depth vs. high bit-depth? It could be useful for HDR.

don't think wgpu currently provides a way to know that

~~(also we don't have hdr support in webgpu.h, yet, wgpu does though)~~

AstcChannel seems unrelated to the swapchain API TBH.

Ah yes, apologies!

Let's tackle this in another PR - maybe after a mini issue-rfc kind of deal - as there is a ton of complexity there that we aren't yet equipped to deal with here. For reference: gfx-rs/wgpu#2869 is our proposal internal to wgpu which would set the stage for the HDR api.

rajveermalviya · 2022-08-19T06:16:40Z

webgpu.h

@@ -575,6 +574,15 @@ typedef enum WGPUVertexStepMode {
    WGPUVertexStepMode_Force32 = 0x7FFFFFFF
 } WGPUVertexStepMode;

+typedef enum WGPUSurfaceStatus {


Replaced bool * suboptimal with an enum.
This matches SurfaceStatus in wgpu.

But still not sure if we should only have Good & Suboptimal and report others as errors.

I'm not sure what Outdated means in practice, but the enum sounds good.

I'm not sure what Outdated means in practice

in wgpu it is mapped to VK_ERROR_OUT_OF_DATE_KHR

after thinking more about it, WGPUSurfaceStatus can be

typedef enum WGPUSurfaceStatus { WGPUSurfaceStatus_Good = 0x00000000, // ~ VK_SUCCESS WGPUSurfaceStatus_Suboptimal = 0x00000001, // ~ VK_SUBOPTIMAL_KHR WGPUSurfaceStatus_Timeout = 0x00000002, // ~ VK_TIMEOUT WGPUSurfaceStatus_Force32 = 0x7FFFFFFF } WGPUSurfaceStatus;

removed Lost & Outdated, those can be reported as errors.

Kangz · 2022-08-26T15:09:03Z

webgpu.h

-typedef void (*WGPUProcSwapChainPresent)(WGPUSwapChain swapChain);
+typedef void (*WGPUProcSurfaceConfigure)(WGPUSurface surface, WGPUSurfaceConfiguration const * configuration);
+typedef void (*WGPUProcSurfaceUnconfigure)(WGPUSurface surface);
+typedef WGPUTexture (*WGPUProcSurfaceGetCurrentTexture)(WGPUSurface surface, uint32_t * width, uint32_t * height, WGPUSurfaceStatus * status);


I'm not sure why we need to return all of these by pointer. width and height is whatever the user configured and is also available with wgpuTextureGetWidth/Height. Status could just be wgpuSurfaceGetStatus?

@cwfitzgerald wdyt?

for width & height if I didn't misunderstand, didn't you propose to return them here.

for status I think we should return it via pointer to enforce users to explicitly check swapchain status

We return this immediately, so I think it's good to have it returned in the interface immediately - I would also like to discorage a glGetError kinda interface as much as we can.

I assume that by "this" you mean the status? That'd work, although maybe we should support passing NULL if you don't care. The pointer is slightly unfortunate but IDK how to make a C API better then this (Maybe the function returns the status and it is the WGPUTexture that's returned by pointer? Not sure).

lisanlow · 2022-09-19T03:51:21Z

@Kangz @rajveermalviya I am looking forward to the changes in this PR, may I know if there is currently a road block to its completion and is there any plan to get this change into the first version of webgpu release?

Kangz · 2022-09-22T15:37:12Z

I think we need to iterate a bit more. For example we don't have the notion of orientation which would be important on Android. Returning of optimal parameters is a bit wonky at the moment etc. My suggestion is that we start a branch that's swapchain-rework and start landing PRs against it, then when ready we merge it back. It'll make it easier to discuss various parts of the swapchain separately.

mwyrzykowski · 2023-01-24T00:14:25Z

@Kangz it looks like the last commit to swapchain-rework was 6 months ago. Do you know if these changes are still planned for v1?

It would align the web API more directly with WebGPU.h. Currently there is a bit of a disconnect between them with Surface and SwapChain concepts in WebGPU.h but they are bundled together in GPUCanvasContext in the web specification.

austinEng · 2023-01-24T16:55:49Z

@mwyrzykowski, our team is currently more focused on getting V1 of the JavaScript API out. We may not get to reworking the native swapchain API until later.

litherum · 2023-01-24T19:22:42Z

this is maybe a silly question, but is it really true that updating the webgpu-native/webgpu-headers is blocked on Dawn's implementation?

Can't this repo be updated in parallel?

austinEng · 2023-01-24T19:23:53Z

It certainly can - I was simply saying that our team is probably not going to do native swapchain changes just yet.

rajveermalviya · 2023-08-01T04:30:27Z

superseded by #203

rajveermalviya marked this pull request as ready for review July 19, 2022 15:02

Kangz reviewed Aug 8, 2022

View reviewed changes

replace WGPUSwapChain with methods for WGPUSurface

b9c96db

rajveermalviya force-pushed the remove-swapchain branch from 55625d5 to b9c96db Compare August 19, 2022 06:11

rajveermalviya commented Aug 19, 2022

View reviewed changes

Kangz reviewed Aug 26, 2022

View reviewed changes

chances mentioned this pull request Sep 4, 2022

feat(extensions/webgpu): swapchain support denoland/deno#10690

Closed

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

rajveermalviya closed this Aug 1, 2023

beaufortfrancois mentioned this pull request May 21, 2024

[webgpu] Add Surface API emscripten-core/emscripten#21939

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

replace WGPUSwapChain with methods for WGPUSurface #164

replace WGPUSwapChain with methods for WGPUSurface #164

rajveermalviya commented Jul 17, 2022

Kangz left a comment

Kangz Aug 8, 2022

rajveermalviya Aug 19, 2022

Kangz Aug 8, 2022

rajveermalviya Aug 8, 2022

Kangz Aug 8, 2022

rajveermalviya Aug 19, 2022

Kangz Aug 8, 2022

rajveermalviya Aug 8, 2022

Kangz Aug 8, 2022

rajveermalviya Aug 8, 2022 •

edited

Loading

Kangz Aug 8, 2022

rajveermalviya Aug 8, 2022

cwfitzgerald Aug 29, 2022

rajveermalviya Aug 19, 2022

Kangz Aug 26, 2022

rajveermalviya Aug 28, 2022

Kangz Aug 26, 2022

rajveermalviya Aug 28, 2022 •

edited

Loading

cwfitzgerald Aug 29, 2022 •

edited

Loading

Kangz Aug 29, 2022

lisanlow commented Sep 19, 2022

Kangz commented Sep 22, 2022

mwyrzykowski commented Jan 24, 2023

austinEng commented Jan 24, 2023

litherum commented Jan 24, 2023

austinEng commented Jan 24, 2023

rajveermalviya commented Aug 1, 2023

replace WGPUSwapChain with methods for WGPUSurface #164

replace WGPUSwapChain with methods for WGPUSurface #164

Conversation

rajveermalviya commented Jul 17, 2022

Kangz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajveermalviya Aug 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajveermalviya Aug 28, 2022 • edited Loading

Choose a reason for hiding this comment

cwfitzgerald Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lisanlow commented Sep 19, 2022

Kangz commented Sep 22, 2022

mwyrzykowski commented Jan 24, 2023

austinEng commented Jan 24, 2023

litherum commented Jan 24, 2023

austinEng commented Jan 24, 2023

rajveermalviya commented Aug 1, 2023

rajveermalviya Aug 8, 2022 •

edited

Loading

rajveermalviya Aug 28, 2022 •

edited

Loading

cwfitzgerald Aug 29, 2022 •

edited

Loading