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

Add example renderer for WebGPU #3632

Closed
wants to merge 11 commits into from
Closed

Conversation

bfierz
Copy link
Contributor

@bfierz bfierz commented Dec 1, 2020

This change adds a sample implementation of a WebGPU backend for ImGui including an example. The code is compatible with a recent version of Google's Dawn library (1.12.2020), and Emscpripten 2.0.11.
Note that to date WebGPU is a moving target, which will require future adaptations to this sample.

The example uses GLFW as a window toolkit. Support for initializing GLFW in a WebGPU compatible way was added with 2.0.11 (PR: emscripten-core/emscripten#12907).
Here's a screenshot of running the example in chrome canary version with WebGPU enabled:
image

@ocornut
Copy link
Owner

ocornut commented Dec 2, 2020

Hello @bfierz,

Thanks for the PR!

The example uses GLFW as a window toolkit, however, due to a missing feature, it is not yet fully compatible with Emscripten

Could you clarify this comment?
Do we expect this to be able to be usable then?

I suggest rebasing this on latest, as backends were moved to the backends/ folder since 1.79.

I'll add some notes now.

@ocornut
Copy link
Owner

ocornut commented Dec 2, 2020

With d20f2bc I have renamed the existing example_emscripten to example_emscripten_opengl3 in prevision for this.

imgui_impl_wgpu feedback:

  • It doesn't seem to support texture binding yet the header on top of .h/.cpp says so.
  • Header state it is 64-bit only, I don't think that's true? WGPUTexture appears to be a pointer so it would always fit in default void*.
  • Header suggest Win32 as a suggested platform binding, should probably say SDL or GLFW.

Examples feedback:

  • It would be good to add CI rules for this (extend on the Emscripten CI build).
  • The output file should be example_emscripten_wgpu.*
  • Afaik neither the html or js file should be committed to git as they are compiler outputs.
  • examples/example_emscripten_webgpu/main.cpp should be modeled after examples/example_emscripten_opengl3/main.cpp, aka the only different lines should be related to the renderer usage. This enable us to facilitate further maintenance of the many examples. (Right now it has arbitrary differences.)

Documentation feedback:

Generally it looks good. I must also say that peeking at webgpu.h is looks remarkably slick and well-contained!

@bfierz
Copy link
Contributor Author

bfierz commented Dec 2, 2020

Hi @ocornut,

Thanks for the feedback. I will address the issues you've raised. Two points I'd like to mention already:

  • I am currently working on making GLFW for Emscripten to properly support this example by allowing it to support bypassing GL context creation (which is supported by GLFW)
  • As for the commited HTML I will investigate more. At the moment the way emcc works with html templates is not compatible with WebGPU, so for the time being, I need this precrafted HTML file.
  • I can restructure examples/example_emscripten_webgpu/main.cpp according to examples/example_emscripten_opengl3/main.cpp. The diff would then be the usage of GLFW instead of SDL and WebGPU instead of opengl3.

@bfierz
Copy link
Contributor Author

bfierz commented Dec 3, 2020

HI @ocornut,

I think I cleaned up PR according to your suggestions. A few notes:

  • You were absolutely right, I messed up the custom texture binding part. As it is not straight forward to do I would like to defer this to a later PR. Do you have a suggestion on how to do this: As I need to create to allocate custom data per texture, I would need some sort of map store this. What would you recommend for this?
  • I've aligned the example as much as possible. There WebGPU code is a lot more verbose, so there is quite a significant diff

@bfierz
Copy link
Contributor Author

bfierz commented Jan 7, 2021

Updated the initial description to reflect the fact that the GLFW integration is now compatible with Emscripten 2.0.11

@bfierz
Copy link
Contributor Author

bfierz commented Jan 9, 2021

@ocornut, in this branch in my repository (https://github.com/bfierz/imgui/commits/feature/webgpu-custom-textures - the top 3 commits), I've added support for user-defined textures. If reasonable, I can add these commits to this PR. What do you think?

@ocornut
Copy link
Owner

ocornut commented Jan 11, 2021

Thank you for the update! I'm not sure when I'll be able to tackle this in details but after shipping 1.80.

, in this branch in my repository (https://github.com/bfierz/imgui/commits/feature/webgpu-custom-textures - the top 3 commits), I've added support for user-defined textures. If reasonable, I can add these commits to this PR. What do you think?

Do you see a reason not to?

Q: Is the reason you started using imgui_internal.h solely for the ImPool<WGPUBindGroup> ?
As I understand since WGPUBindGroup is a pointer, like most webgpu types, you could use public-facing ImGuiStorage instead of ImPool<> since it can store void* pointers. In fact, for this particular use cases I see no value in using ImPool over ImGuiStorage, ImPool will only add extra layer of indirection here.

(
As a super minor other thing, we tweaked the other Emscripten backend Makefile so you may carry the same change, but otherwise I'll report them myself later when merging:
31a2f0c...master
)

@bfierz
Copy link
Contributor Author

bfierz commented Jan 11, 2021

Thank you for briefly checking the commits.

Q: Is the reason you started using imgui_internal.h solely for the ImPool ?

Yes, the reason is for using ImPool. However, since it is using ImGuiStorage internally, I will just replace it and push the changes to this branch.

I'll also check the changes on the Makefile.

@ocornut
Copy link
Owner

ocornut commented Jan 26, 2021

I have tried this today and could build it (with a simple makefile update) but couldn't figure out how to run it on Firefox or Chrome.

May be good in the Readme to link to e.g.
https://github.com/gpuweb/gpuweb/wiki/Implementation-Status
And provide some details about enabling.

With Firefox 84.0.2 with dom.webgpu.enabled=true and gfx.webrender.all=true I get:
Uncaught (in promise) DOMException: WebGPU is not enabled!

With Firefox Canari 86.0b1 with dom.webgpu.enabled=true and gfx.webrender.all=true I alo get:
Uncaught (in promise) DOMException: WebGPU is not enabled!

With Chrome 87.0.4280.141 I get:
Cannot read property 'requestAdapter' of undefined

With Chrome 90.0.4400.0 (Official Build) canary (64-bit) it runs but:
image

I am hosting with:
python3 -m http.server -d .

Not sure what I doing wrong?

Happy to merge this once you make the change for ImPool and texture support.

@bfierz
Copy link
Contributor Author

bfierz commented Jan 26, 2021

Unfortunately, you didn't do anything wrong. A few days ago @Henauxg reported the same issue and with the latest version of chrome I also have the same problems. We believe that the issue lies with Emscripten not being up to date with chrome canary, but it needs investigation.
I never actually tested Firefox as their WebGPU implementation is not up to date at all.
The implementation is correct as it still renders correctly when using a native build using Google Dawn directly. As it is not straightforward to build I didn't include any instructions, but I can try to update the example and readme to include instructions to work with Dawn directly and do a native build.

Never the less I will update the readme to be more specific on this issue.

@ocornut
Copy link
Owner

ocornut commented Jan 26, 2021

Either way since WebGPU is experimental I don't mind merging even as slightly clunky, since I'm hoping the availability of this can help you and others advance work on WebGPU.

I just noticed that the work e.g. "Replace ImPool with ImGuiStorage" was already done in your other branch but not in this PR. If you push them to the same PR I'll merge, and I can fix the Makefile bits.

@bfierz
Copy link
Contributor Author

bfierz commented Jan 26, 2021

I will push it as soon as I get to it, thank you for fixing the makefile afterward

@bfierz
Copy link
Contributor Author

bfierz commented Jan 26, 2021

The new run seems to fail due to a missing '-s ERROR_ON_UNDEFINED_SYMBOLS=0' in the makefile. @ocornut would you mind adding this too, while fixing the makefile?

ocornut pushed a commit that referenced this pull request Jan 28, 2021
@Kangz
Copy link
Contributor

Kangz commented Feb 10, 2021

Just saw this in @ocornut's tweet, it's quite exciting! The use of WebGPU looks good but the implementation could probably be simplified a bit. Is this a good place to make some comments?

Also feel free to drop in one of the two Matrix channels listed in Dawn's README.md if you ever have difficulties with WebGPU or the emscripten implementation.

@bfierz
Copy link
Contributor Author

bfierz commented Feb 11, 2021

Hi @Kangz, I am happy to hear any feedback you've got. Feel free to post it here, or send me an email.
The current implementation roughly follows the D3D12 implementation to keeps them consistent. As my experience with WebGPU is small, I am eager to learn more.

Thanks for mentioning the Matrix channels. I should join as I experience some weird problems when trying to resize swap chains.

@iocafe
Copy link

iocafe commented Feb 23, 2021

To second the Kangz, it is exiting. If this works out we could run Imgui application in cloud server, etc and have chrome or Firefox render the UI at client end? As can be expected, my 20 minute quick try to set this up with limited knowledge was not successful. It has been long time dream to be able to write applications with "game like" response, which run on desktop and mobile platforms. ImGui has already made this feasible, although mobile support requires a bit of customization. If the same application can in future be run in browser by remote client, I start to run out of musical wishes:)

@Kangz
Copy link
Contributor

Kangz commented Feb 23, 2021

Whoops, this went to the wrong inbox, sorry for the late reply. Again, this is overall pretty good and using the API correctly, which is non-trivial there are not good WebGPU resources (yet).

In imgui_impl_wgpu.cpp, things would be overall simpler if you were using webgpu_cpp.h which provides good enough C++ RAII wrappers around the WebGPU objects. It would remove the need for SafeRelease and you could just assign nullptr to the object. It also includes a bunch of defaults that would no longer be required to be specified on the various descriptors.

WGSL is still evolving, but when it stabilizes I think IMGUI should use it instead of hardcoding compiled SPIRV: it will make code more readable / easier to modify, and it will also map better on the Web were only WGSL will be available. (SPIRV will keep being available in native). In the GLSL, the usage of gl_PerVertex is a bit weird, you can use gl_Position directly. Likewise the use of the struct for varyings is a bit uncommon: you can just have two varyings. With WGSL you'll also be able to put both entrypoints in the same wgpu::ShaderModule.

ImGui_ImplWGPU_CreateDeviceObjects could take advantage of the default values being set for most descriptor fields in C++ but also should use wgpu::RenderPipeline::GetBindGroupLayout to reflect the bind group layout, instead of creating it directly. (it will also avoid the need for the EMSCRIPTEN_VERSION check there, in general shouldn't IMGUI just target the latest emscripten while the API is still evolving a bit?). The separation between the two bindgroups is great though!

ImGui_ImplWGPU_CreateFontsTexture could take advantage of the default values in the texture descriptor, and the texture view could just be equal to texture.CreateView(): the view descriptor is optional and defaults to using the full view, which is what the function does. wgpu::Queue::WriteTexture could be used instead of creating a buffer, filling it, encoding the copy command and submitting. This also removes the need for ImGui_ImplWGPU_CreateBufferFromData (which as a nitpick should have used mappedAtCreation = true instead of WriteBuffer)

ImGui_ImplWGPU_RenderDrawData's reallocation looks good (although you could call .Destroy() so that buffers are freed as soon as they are unused instead of waiting for the JS GC). Note however that on OOM WebGPU stills returns a non-null buffer pointer (this is because the OOM happens in a different process in an async manner in web browsers). If there's an OOM the error will propagate but there will be no crash.

As nitpicks in ImGui_ImplWGPU_SetupRenderState, it seems we always use the full size of vertex and index buffers. Maybe we could remove offfset and use WGPU_WHOLE_SIZE for the size argument?

example_emscripten_wgpu/main.cpp could maybe also use webgpu_cpp.h

@bfierz
Copy link
Contributor Author

bfierz commented Feb 24, 2021

@iocafe, I agree. It will be an interesting setup to distribute 3D heavy applications without needing to write a new frontend

@Kangz, thank you for the review.

C++ API

I fully agree it would be a lot cleaner to use it. As I was mainly working with dawn directly, I wasn't sure what the future of the C++ API will be, how it gets from dawn to emscripten. Reuse with e.g. wgpu-native for Firefox was another reason (though it turned out that their API is still very different).

Shaders and WGSL

For simplicity, the shader is a direct copy of the Vulkan part. So far I didn't pay much attention to WGSL as it is still in development. But I agree, the shader code should be replaced once WGSL is mature enough. If it already is, I'll give it a try.

ImGui_ImplWGPU_CreateFontsTexture

Thanks for these hints. Writing data to textures and buffers, was my biggest struggle. I am not sure if I missed what you mentioned, or if at the time of writing things weren't available yet. I've opened the PR rather late; the main development actually dates back to late summer.

ImGui_ImplWGPU_RenderDrawData

Thanks for the hint. I'll try to use .Destroy

@Kangz
Copy link
Contributor

Kangz commented Feb 25, 2021

I fully agree it would be a lot cleaner to use it. As I was mainly working with dawn directly, I wasn't sure what the future of the C++ API will be, how it gets from dawn to emscripten. Reuse with e.g. wgpu-native for Firefox was another reason (though it turned out that their API is still very different).

The C++ API is a pure wrapper over the C API so you can use it in emscripten builds. I think it's even shipped in emscripten today?

For simplicity, the shader is a direct copy of the Vulkan part. So far I didn't pay much attention to WGSL as it is still in development. But I agree, the shader code should be replaced once WGSL is mature enough. If it already is, I'll give it a try

I should have thought of that :) Sounds good. Not that WGSL is still not very stable so YMMV with it. It might be best to wait a little bit more.

I've opened the PR rather late; the main development actually dates back to late summer.

Gotcha, I think we were in the process of implementing WriteTexture last summer so this checks out.

@bfierz
Copy link
Contributor Author

bfierz commented Feb 26, 2021

The C++ API is a pure wrapper over the C API so you can use it in emscripten builds. I think it's even shipped in emscripten today?

Yes, it is. Although it is slightly modified compared to the one shipped with dawn. I am going to give it a try.

shouldn't IMGUI just target the latest emscripten while the API is still evolving

In this particular case, I was working with latest emscripten while only 2.0.13 was out. I agree that whoever is using WebGPU now, should be aware that it is not stable yet. The macro is maybe a bit over the top :)

While implementing the other proposed changes, two questions came up:

ImGui_ImplWGPU_CreateDeviceObjects

I am afraid I don't quite understand how to work with GetBindGroupLayout. From what I gathered it will give you access to the bind groups in an existing pipeline object, but at some point, somebody has to do the initial setup of the bind groups. Did I miss a reflection API?

WGPU_WHOLE_SIZE

This maps to 0xfffff... (at least in my version). However, only when using 0 for both offset and size, I get through the validation.

@Kangz
Copy link
Contributor

Kangz commented Mar 4, 2021

I am afraid I don't quite understand how to work with GetBindGroupLayout. From what I gathered it will give you access to the bind groups in an existing pipeline object, but at some point, somebody has to do the initial setup of the bind groups. Did I miss a reflection API?

wgpu::RenderPipelineDescriptor::layout is optional. If you set it to nullptr then the pipeline will compute its default layout (the minimal one that's enough for the pipeline), and then it pipeline.GetBindGroupLayout(N) will return the inferred layout. It helps avoid having to deal with layouts for simple cases.

This maps to 0xfffff... (at least in my version). However, only when using 0 for both offset and size, I get through the validation.

Interesting, that might be a missing thing in emscripten.

@iocafe
Copy link

iocafe commented Mar 5, 2021

Bfierz, when you get to point where testing is needed, I would be interested to take part in that.

@ocornut
Copy link
Owner

ocornut commented May 6, 2021

WebGPU example starting failing to build on CI today:

https://github.com/ocornut/imgui/runs/2517585708?check_suite_focus=true

main.cpp:205:5: error: unknown type name 'WGPURenderPassColorAttachmentDescriptor'; did you mean 'WGPURenderPassColorAttachment'?
    WGPURenderPassColorAttachmentDescriptor color_attachments = {};
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    WGPURenderPassColorAttachment
/home/runner/work/imgui/imgui/emsdk-master/upstream/emscripten/cache/sysroot/include/webgpu/webgpu.h:868:3: note: 'WGPURenderPassColorAttachment' declared here
} WGPURenderPassColorAttachment;
  ^
main.cpp:209:23: error: no member named 'attachment' in 'WGPURenderPassColorAttachment'
    color_attachments.attachment = wgpuSwapChainGetCurrentTextureView(wgpu_swap_chain);
    ~~~~~~~~~~~~~~~~~ ^
main.cpp:224:23: error: use of undeclared identifier 'wgpuDeviceGetDefaultQueue'; did you mean 'wgpuDeviceGetQueue'?
    WGPUQueue queue = wgpuDeviceGetDefaultQueue(wgpu_device);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
                      wgpuDeviceGetQueue
/home/runner/work/imgui/imgui/emsdk-master/upstream/emscripten/cache/sysroot/include/webgpu/webgpu.h:1227:23: note: 'wgpuDeviceGetQueue' declared here
WGPU_EXPORT WGPUQueue wgpuDeviceGetQueue(WGPUDevice device);
                      ^
3 errors generated.

@Kangz
Copy link
Contributor

Kangz commented May 6, 2021

Ah you update emscripten directly? The following changes are needed: https://hackmd.io/BtRPZXulQICjbcZi4tLCdg

Are you able to suppress the failure until this is fixed?

@ocornut
Copy link
Owner

ocornut commented May 6, 2021

We get emscripten latest:
wget -q https://github.com/emscripten-core/emsdk/archive/master.tar.gz
https://github.com/ocornut/imgui/blob/master/.github/workflows/build.yml#L442

A bit living on the edge but since we seldomly use it ourselves it is a good way to detect changes.

If any of you think you can fix it within a few days I'll leave CI as is, otherwise I'll disable it from CI after that.

WebGPU seems so edgy when I google for wgpuDeviceGetDefaultQueue() there are FOUR matches (two being from imgui_impl_wgpu).

@bfierz
Copy link
Contributor Author

bfierz commented May 7, 2021

Thanks for notifying, I didn't get to work with emscripten recently. I'll try to update the code base over the weekend.

@bfierz
Copy link
Contributor Author

bfierz commented May 7, 2021

I've updated my open branch containing some improvements to the WebGPU API usage that were suggested by @Kangz and added the new render pipeline descriptor: https://github.com/bfierz/imgui/tree/feature/update-wgpu-emsdk-2.0.20
The work is not yet finished as I am trying to understand some smaller issues. Once ready I'll open a PR.

@ocornut
Copy link
Owner

ocornut commented Aug 24, 2021

WebGPU backend failing on CI today:
https://github.com/ocornut/imgui/runs/3411015414?check_suite_focus=true

../../backends/imgui_impl_wgpu.cpp:547:5: error: unknown type name 'WGPURenderPipelineDescriptor2'; did you mean 'WGPURenderPipelineDescriptor'?
    WGPURenderPipelineDescriptor2 graphics_pipeline_desc = {};
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    WGPURenderPipelineDescriptor
/home/runner/work/imgui/imgui/emsdk-master/upstream/emscripten/cache/sysroot/include/webgpu/webgpu.h:936:3: note: 'WGPURenderPipelineDescriptor' declared here
} WGPURenderPipelineDescriptor;
  ^
../../backends/imgui_impl_wgpu.cpp:613:23: error: use of undeclared identifier 'wgpuDeviceCreateRenderPipeline2'; did you mean 'wgpuDeviceCreateRenderPipeline'?
    g_pipelineState = wgpuDeviceCreateRenderPipeline2(g_wgpuDevice, &graphics_pipeline_desc);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                      wgpuDeviceCreateRenderPipeline
/home/runner/work/imgui/imgui/emsdk-master/upstream/emscripten/cache/sysroot/include/webgpu/webgpu.h:1220:32: note: 'wgpuDeviceCreateRenderPipeline' declared here
WGPU_EXPORT WGPURenderPipeline wgpuDeviceCreateRenderPipeline(WGPUDevice device, WGPURenderPipelineDescriptor const * descriptor);

@Kangz
Copy link
Contributor

Kangz commented Aug 24, 2021

Should be fixed #4472 This is the first time I use CodeSpaces to do a PR so hopefully it is ok

@Kangz
Copy link
Contributor

Kangz commented Aug 24, 2021

Note that more breakage should be incoming, see https://hackmd.io/OxDovqjoTXqC_r_WY-aM1Q

@ocornut
Copy link
Owner

ocornut commented Aug 24, 2021

Thank you Corentin, much appreciated! Merged now.

@bfierz
Copy link
Contributor Author

bfierz commented Aug 25, 2021

Note that more breakage should be incoming, see https://hackmd.io/OxDovqjoTXqC_r_WY-aM1Q

Thanks for fixing and providing the link. I am regularly updating the version of dawn that I am using. I will check the deprecations and provide a pull request.

@ocornut
Copy link
Owner

ocornut commented Feb 28, 2022

FYI for anyone interested, WebGPU backend broke again with latest:
https://github.com/ocornut/imgui/runs/5357933672?check_suite_focus=true

ocornut added a commit that referenced this pull request Mar 15, 2022
sergeyn pushed a commit to sergeyn/imgui that referenced this pull request Mar 19, 2022
sergeyn pushed a commit to sergeyn/imgui that referenced this pull request Mar 19, 2022
sergeyn pushed a commit to sergeyn/imgui that referenced this pull request Mar 19, 2022
ocornut added a commit that referenced this pull request Apr 22, 2022
…PassColorAttachment::clearColor to ::clearValue (#3632)
ocornut pushed a commit that referenced this pull request Jan 25, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants