[webgpu] Add Surface API #21939
[webgpu] Add Surface API #21939
Conversation
beaufortfrancois
force-pushed
the
surface
branch
2 times, most recently
from
533d5f6
to
d6a8639
Compare
beaufortfrancois
force-pushed
the
surface
branch
2 times, most recently
from
139b253
to
5a31c92
Compare
src/library_webgpu.js
Outdated
| 'premultiplied', | ||
| 'unpremultiplied', | ||
| 'inherit', | ||
| ], |
There was a problem hiding this comment.
I don't think this is needed (see emscripten_no_enum_table in dawn.json)
There was a problem hiding this comment.
My bad, this is needed. I saw that it wasn't used and didn't think about the fact that it should be used.
However not all of the entries should be there, it should be something like:
CompositeAlphaMode: [
'opaque',
'opaque',
'premultiplied',
],
Thanks for starting the review @kainino0x. There are still some TODOs I'd like to solve such as presentModes, alphaModes, and the error returned by getCurrentTexture. |
There was a problem hiding this comment.
I addressed all my own comments, so LGTM now, but @beaufortfrancois PTAL before I land it.
@kainino0x I've just merged main branch and I've tried locally all your latest changes. LGTM. Thank you! |
|
Is there an IDL or a spec available to this Surface API? It doesn't seem to exist in the WebGPU spec at least. |
I believe webgpu-native/webgpu-headers#164 and webgpu-native/webgpu-headers#203 are where things got defined. |
The Web API only concerns itself with HTML Canvas, whereas Surface is a C-API-only abstraction over both HTML Canvas and native window system integration. The Surface API is very close to GPUCanvasContext though. |
|
What got me confused is that the JS side code but there is no such parameter defined in the IDL: https://www.w3.org/TR/webgpu/#dictdef-gpucanvasconfiguration (and no |
|
Ah, that's not supposed to be there. presentMode is native-only, Chrome doesn't support it in Blink, it just gets dropped on the floor because that's how WebIDL works. |
src/library_webgpu.js
Outdated
| 'premultiplied', | ||
| 'unpremultiplied', | ||
| 'inherit', | ||
| ], |
There was a problem hiding this comment.
My bad, this is needed. I saw that it wasn't used and didn't think about the fact that it should be used.
However not all of the entries should be there, it should be something like:
CompositeAlphaMode: [
'opaque',
'opaque',
'premultiplied',
],
| "usage": {{{ gpu.makeGetU32('config', C_STRUCTS.WGPUSurfaceConfiguration.usage) }}}, | ||
| // viewFormatCount | ||
| // viewFormats | ||
| "alphaMode": "opaque", |
There was a problem hiding this comment.
This should be either "opaque" or "premultiplied" depending on the configuration, and there should be an assert that it's one of the values that's valid in JS (auto, opaque, premultiplied). (Just look it up in the table above and then assert that the result is not undefined.)
Alternatively, assert the alphaMode is Opaque for now with a TODO, and implement this later if you'd like.
There was a problem hiding this comment.
I've assert alphaMode is Opaque with a TODO.
|
@sbc100 Is there any chance we can publish next Emscripten release (3.1.61 I believe) when Chrome 126 hits stable channel (June 10th) so that developers don't have to use the ToT version of Emscripten to use the Surface API in WebGPU? |
|
Sure, we can basically cut releases as we please. |
|
(assuming the tree is green and there are no known issues) |
That's great to hear! |
As raised in [1], the default value for surfaceConfiguration alphaMode should be "auto", not "opaque". [1]: emscripten-core/emscripten#21939 (comment) Bug: dawn:2320 Change-Id: I4201f6ab623ee62a2d206121d0fe2ae1eece67cc Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/190080 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Fr <beaufort.francois@gmail.com>
FIX #21745