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

WebGPU example - support for WebGPU-native/Dawn #7435

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

eliasdaler
Copy link
Contributor

@eliasdaler eliasdaler commented Mar 24, 2024

Inspired by #7132

Also removed Emscripten's Makefile as we now have CMake and using emcmake with it is much easier than maintaining a build script + it works on Windows.

Build instructions

Building for desktop (WebGPU-native) with Dawn:

git clone https://github.com/google/dawn dawn
cmake -B build -DIMGUI_DAWN_DIR=dawn
cmake --build build

The resulting binary will be found at one of the following locations:

  • build/Debug/example_emscripten_wgpu[.exe]
  • build/example_emscripten_wgpu[.exe]

Building for Emscripten:

  1. Install Emscripten SDK following the instructions: https://emscripten.org/docs/getting_started/downloads.html
  2. Install Ninja build system
  3. Build:
emcmake cmake -G Ninja -B build
cmake --build build
emrun build/index.html

@ocornut
Copy link
Owner

ocornut commented Mar 24, 2024

Hello,
I’d prefer if the makefile was not removed because it makes it clear and explicit what needs to be done.

So this is for building on desktop with Dawn, but still using Emscripten on a desktop? Could the
WebGPU backend be used alongside an existing platform backend eg SDL or GLFW?

@eliasdaler
Copy link
Contributor Author

eliasdaler commented Mar 24, 2024

Hello,
I’d prefer if the makefile was not removed because it makes it clear and explicit what needs to be done.

Restored. Also this will fix the CI - I'm also not sure how Dawn build should be handled. It'll take a lot of time to build, probably (since it pulls many dependencies and compiles a lot of stuff).

Plus, I'm not really good at writing CI scripts, so I'll leave that to someone else.

So this is for building on desktop with Dawn, but still using Emscripten on a desktop? Could the
WebGPU backend be used alongside an existing platform backend eg SDL or GLFW?

No. Emscripten is only used for web build, on Desktop you build with Dawn and glfw. It's possible to use Dawn with SDL, but it requires a lot more effort to get the surface, e.g.:

Would be easier once libsdl-org/SDL#8912 is done.

Right now you need to do platform-specific things like these: https://github.com/eliasdaler/webgpu-learning/blob/479fb1ce14e5b2b9788bea1ab3b1fa42843f29a8/src/util/SDLWebGPU.cpp#L10

@meshula
Copy link

meshula commented Mar 24, 2024

@eliasdaler I haven't updated to latest in several months, but I have a CI friendly dawn/tint build script here.

https://github.com/meshula/labslang

Note that this elides many of the slow building dependencies for dawn/tint and components unneeded for building such as the testing infrastructure, in order to get the build down into the realm of tens of seconds.

@eliasdaler
Copy link
Contributor Author

It's up for Omar to decide (and the CI for Dawn build should probably be done in the separate PR), but I feel like supporting a custom build/integration of Dawn is too much work since Dawn's dependencies can change at any time and their CMake build should remain stable even if it's somewhat slow.

I also removed build of various unimportant Dawn's components from the build to reduce the build time - I'm not sure more things can be removed.

@ypujante
Copy link

@eliasdaler I am personally a bit confused about this PR. You are changing the example that is called example_emscripten_wgpu to not build with emscripten. Shouldn't it be its own example instead? Like example_dawn_wgpu?

For example:

# from CMake
if(EMSCRIPTEN)
  target_link_options(example_emscripten_wgpu PRIVATE
    "-sUSE_WEBGPU=1"
    "-sUSE_GLFW=3"
    "-sWASM=1"
    "-sALLOW_MEMORY_GROWTH=1"
    "-sNO_EXIT_RUNTIME=0"
    "-sASSERTIONS=1"
    "-sDISABLE_EXCEPTION_CATCHING=1"
    "-sNO_FILESYSTEM=1"
  )
  set_target_properties(example_emscripten_wgpu PROPERTIES OUTPUT_NAME "index")
  # copy our custom index.html to build directory
  add_custom_command(TARGET example_emscripten_wgpu POST_BUILD
    COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_CURRENT_LIST_DIR}/web/index.html" $<TARGET_FILE_DIR:example_emscripten_wgpu>
  )
endif()

Or from the code:

#ifndef __EMSCRIPTEN__
        // Tick needs to be called in Dawn to display validation errors
        wgpuDeviceTick(wgpu_device);
#endif

I personnally think that it is very confusing and make the emscripten example far too complicated.

@eliasdaler
Copy link
Contributor Author

eliasdaler commented Mar 25, 2024

Other backends have Emscripten implementations too. And the desktop part is considered the main one, while the Emscripten part is just an addition to the existing backend. See this, for example:

#ifdef __EMSCRIPTEN__

Ideally this example should just be named "example_glfw_wgpu", but I didn’t rename it yet because it would be hard to see the changes to the files - this should probably be done after this PR is done. I don’t think Emscripten+WGPU should be it’s own example as otherwise there would be too much code repetition.

You are changing the example that is called example_emscripten_wgpu to not build with emscripten

It still supports the build with Emscripten.

@ypujante
Copy link

I was pointing out that it is confusing and I still feel that way. If this example is no longer the emscripten/wgpu example as it has been since its creation then indeed I would suggest at the very minimum to rename it.

In my mind this example was also implying that it was web only (because of emscripten). This PR flips this on its head.

I'm not saying this is a bad PR. I'm just saying it's confusing and I feel like the confusion should be lifted one way or another.

Hopefully @ocornut will have a better grasp on which direction he wants to go.

@ypujante
Copy link

Hey @eliasdaler. I have been keeping this in the back of my head

I guess the main question about this specific example is:

  • is it supposed to be an example about the renderer WebGPU?
  • is it supposed to be an example about how to use emscripten?

I have always considered this example in the latter category (an example about emscripten)

But since it is true that there are 2 other examples that also have emscipten support (example_glfw_opengl3 and example_sdl2_opengl3), then I feel like your suggestion of renaming this example into example_glfw_wgpu is probably the best approach and would remove the confusion.

@eliasdaler
Copy link
Contributor Author

eliasdaler commented Mar 30, 2024

I think that this example should have had both desktop and Emscripten support from the start, just like other examples do. However, it didn’t and #7132 was created. However, it was doing a bit too much (building for desktop and adding multi-viewport support) and Omar asked me to only leave "building for Dawn/webgpu-native" part.

The example will later be renamed to example_glfw_wgpu - as I said before, I didn’t do it to make this PR easier to read (otherwise Git could have shown renamed files as new files and didn’t show just my changes)

@JulesFouchy
Copy link
Contributor

Hi! Instead of requiring users to manually download dawn, maybe we should use CMake's FetchContent feature to download it automatically when someone tries to build the example with dawn? This would also have the benefit of specifying in the CMakeLists.txt which version (commit hash) of dawn to use. Since the API is still changing a bit, this would prevent the example from breaking in the future, and we can manually update the commit hash when we update the example to support new API changes.

For example this repository provides a CMake script to download Dawn. It also has the advantage to only download the minimum required (way less than a git clone), and builds as little things as possible. I made the changes and sent you a pull request @eliasdaler.

@ocornut
Copy link
Owner

ocornut commented Apr 6, 2024

It’s not our job to provide librairies nor even build systems to get them, that’d be opening a pandora box. But we can put some effort documenting ways to get things, I think that’s more valuable.

@ocornut
Copy link
Owner

ocornut commented Apr 16, 2024

Q: can you explain the location for calling wgpuDeviceTick() ? shouldn't it be lower or higher in the loop?

Q: can you explain how the 4 calls to wgpuTextureViewRelease(), wgpuRenderPassEncoderRelease(), etc didn't exist in Emscripten version and why that didn't cause problems?

Q: Why has this comment been removed? (I don't fully understand it tbh)

    // Use C++ wrapper due to misbehavior in Emscripten.
    // Some offset computation for wgpuInstanceCreateSurface in JavaScript
    // seem to be inline with struct alignments in the C++ structure

Thanks!

@ocornut
Copy link
Owner

ocornut commented Apr 16, 2024

I have merged this now. Thank you!

  • I am going to rename this to example_glfw_webgpu shortly (on it).
  • Could you explain the location for calling wgpuDeviceTick() ? shouldn't it be lower or higher in the loop?
  • Could you explain how the 4 calls to wgpuTextureViewRelease(), wgpuRenderPassEncoderRelease(), etc didn't exist in Emscripten version and why that didn't cause problems?
  • Why has this comment been removed? (I don't fully understand it tbh) (see post above)
  • I am getting those warnings when running:
Warning: Layer VK_LAYER_OBS_HOOK uses API version 1.2 which is older than the application specified API version of 1.3. May cause issues.
Warning: The explicit creation of a SwapChain object is deprecated and should be replaced by Surface configuration.
  • mainly, I am rather unhappy that this is pulling and building dawn. This is how the VS project looks like and it initially make my VS freeze on build:

image

Assuming there are no Dawn binaries (FFS...), Is there any way we can split up the responsibility aka have a build file to conveniently build Dawn separately, and then make the main file assume Dawn is available?

Thanks a lot.

@ypujante
Copy link

As an FYI, if you look at the "LearnWebGPU" tutorial, there is a section about the various implementations. Dawn is one, wgpu-native is another. I believe when using wgpu-native there are pre-built binaries so you don't build it yourself...

I am currently investigating migrating my ImGui application to WebGPU and in my proof of concept, I have been using Dawn and one thing I noticed is that the resulting binary file is huge (~60Mb for a "hello world"). It seems that it is a lot smaller with wgpu-native. I have not investigated yet why it is so big with Dawn and if there are ways to reduce the size. (The project I would like to port is ~5MB when compiled using ImGui/OpenGL).

@Zelif
Copy link

Zelif commented Apr 18, 2024

Q: can you explain how the 4 calls to wgpuTextureViewRelease(), wgpuRenderPassEncoderRelease(), etc didn't exist in Emscripten version and why that didn't cause problems?

@ocornut
Memory leaks using C wgpu calls.
The C++ objects will take care of releasing the given objects under the hood, so when using just the C implementation you are required to do this. The example used a mixture of C/C++ objects that resulted in not needing this. Also the old example might of been leaking memory every frame if they were using just the C bindings.

When implementing multiviewport I had run into this when in control of the rendering was controlled for each window.

// Use C++ wrapper due to misbehavior in Emscripten.
// Some offset computation for wgpuInstanceCreateSurface in JavaScript
// seem to be inline with struct alignments in the C++ structure

As for this my guess is, @eliasdaler is using all C api to do this, then the top comment wont be needed. Unsure with the others. I was never sure what the comment was referencing so I had left it.

Dawn is a pain in the ass for when I have used it, as some parts are behind google internal git (but are not required), this is why I wanted to try get wgpu-native to compile since they offer up precompiled libs. As @ypujante pointed out @eliemichel 's Tutorials have a cut down dawn implementation or setup via wgpu-native.

Unfortunately there are differences in both( I think around surface and swapchain ), which is blocked atm on dawn's end.
@waywardmonkeys had pointed this out in the PR I had started.

When pulling dawn for the first time prepare to wait A LONG time(for me some PCs took 1h+ to get everything). I think why it bloats the exe as well is just due to the sheer amount debug handling in dawn as well.

Sorry to ping everyone, I hope I added some clarity to some of the questions.

P.S. The way swapchain is done in the example does lead to crashes and corruption(when resizing) on linux with the vulkan backend, I get no such corruption or crash with windows/DirectX. Hope they push the swapchain stuff into the surface soon so we can avoid these issues.

@eliasdaler
Copy link
Contributor Author

eliasdaler commented Apr 18, 2024

I’ll answer other questions later, but need to clarify this:

When pulling dawn for the first time prepare to wait A LONG time(for me some PCs took 1h+ to get everything)

You don’t need to pull it with submodules. You can do a shallow clone of Dawn and it will pull needed dependencies on build automatically.

And yeah, Dawn and wgpu are not identical in their APIs… I’m personally not interested in writing many ifdefs to handle both and I also think that depending on pre-built libraries to build an example is not a good idea. Requiring user to install a Rust toolchain and build Rust code is another not-so-great idea…

I guess, if someone makes a PR which makes the backend compatible with wgpu, then it’ll be useful. But I don’t think that a glfw example needs this (will increase complexity a lot possibly)

@Zelif
Copy link

Zelif commented Apr 19, 2024

@eliasdaler
Everytime I tried to do that, it still took ages or missed things.( Probs me just being horrible at cmake and what no xD ) I have some figuring out to do.

I'm personally not interested in writing many ifdefs

I'd agree, for this time right now just getting the example working with dawn's api seemed to be the best way imo, anyone wanting to adapt it would to me seem a trivial task if they are already using wgpu-native.

My main concern was I had not tested any of the latest dawn stuff either and had been told there was work to remove the management of swapchain as much. But if that hasn't come through the release away and PR it later when it comes down the pipe ;D

@ypujante
Copy link

I wanted to do a follow up on my comment about size

I am currently investigating migrating my ImGui application to WebGPU and in my proof of concept, I have been using Dawn and one thing I noticed is that the resulting binary file is huge (~60Mb for a "hello world"). It seems that it is a lot smaller with wgpu-native. I have not investigated yet why it is so big with Dawn and if there are ways to reduce the size. (The project I would like to port is ~5MB when compiled using ImGui/OpenGL).

I rebuilt my project in Release mode, and I am down to 8.6Mb. So it looks like indeed when building in Debug mode there is a lot of extra code and debugging information that is not present in an optimized build. Which is really good.

@eliemichel
Copy link
Contributor

Hey thanks for pinging me here, replying to some points mentionned above:

It's possible to use Dawn with SDL, but it requires a lot more effort to get the surface

There is an appendix of LearnWebGPU that is dedicated to that, and it is actually failrly straightforward by using the sdl2webgpu I wrote for this and which plays a similar role as glfw3webgpu for GLFW. [Resulting code]

Q: can you explain the location for calling wgpuDeviceTick() ? shouldn't it be lower or higher in the loop?

This works a bit like glfwPollEvents, it just has to be called somewhere in the loop, and this is what invokes awaiting callbacks when they are ready to run (the most common case being calling the error callback)

Q: can you explain how the 4 calls to wgpuTextureViewRelease(), wgpuRenderPassEncoderRelease(), etc didn't exist in Emscripten version and why that didn't cause problems?

JavaScript has a Garbage Collector that takes care of releasing whatever gets out of scope. In order to be compatible with non-GC languages such as C/C++, the native API exposes an explicit Release method for all objects.

Some C++ wrapper follow the RAII idiom in order to automatically release objects, but I believe that for didactic purposes the examples should stick with the raw C API in this case. Dawn and emscripten provide a pretty similar C++ wrapper (they are both auto-generated through the same procedure) but this is non-standard (and in particular not provided with wgpu-native).

I am getting those warnings when running

These come from custom Vulkan validation layers that were installed by OBS when you isntalled it (because in your case Dawn runs on top of Vulkan).

mainly, I am rather unhappy that this is pulling and building dawn. This is how the VS project looks like and it initially make my VS freeze on build:

I agree this is annoying, I ended up adding to the FetchDawn.cmake script settings to put all third party targets in a common Dawn folder. The very list of target changes from time to time so I need to update it regularily...

Assuming there are no Dawn binaries

Sadly there are no official Dawn binaries, although their continuous integration scripts build it thousands of times a day ^^'

Honestly @ocornut I would go for wgpu-native official binaries in the case of a simple example like you intend to maintain, instead of pulling the whole Dawn codebase. It is either reasonable to ask people to get the dll/dylib/so that fits their platform (or again use a CMake script like FetchWgpuNative.cmake to automate the job but afaik you're not a big fan of maintaing build systems, which I totally understand because C++ ecosystem is a mess).

Unfortunately there are differences in both( I think around surface and swapchain )

The one lagging behind was actually Dawn/emscripten (emscripten is based on Dawn), and since a couple of days Dawn daily releases are now up to date (i.e., they provide the Surface Configuration API that replaces the SwapChain API - now deprecated). The next release of emscripten (~2 weeks maybe?) should also include this new unified API.

I'm pretty happy with this, and as I'm writing the new version of the guide I realize there is only 1 single #ifdef for now. We can really expect that on the long run the API gets fully unified, this is for sure a goal for both Dawn and wgpu-native teams, and I think it is good to make sure imgui's examples are not tied to a specific backend.

@ocornut
Copy link
Owner

ocornut commented Apr 22, 2024

This works a bit like glfwPollEvents, it just has to be called somewhere in the loop, and this is what invokes awaiting callbacks when they are ready to run (the most common case being calling the error callback)

Then shouldn't be called higher in the loop? It feels strange to be raising events at a random "middle of main loop" location

I am getting those warnings when running
These come from custom Vulkan validation layers that were installed by OBS when you isntalled it (because in your case Dawn runs on top of Vulkan).

Shouldn't they be fixed then? or there is no way to fix them without modyfing Dawn?

Honestly @ocornut I would go for wgpu-native official binaries in the case of a simple example like you intend to maintain, instead of pulling the whole Dawn codebase. It is either reasonable to ask people to get the dll/dylib/so that fits their platform (or again use a CMake script like FetchWgpuNative.cmake to automate the job but afaik you're not a big fan of maintaing build systems, which I totally understand because C++ ecosystem is a mess).

If there is a way to make the onlooker/user experience more lightweight I would appreciate, as long as the code ultimately supports both.

PS: This is entirely merged, it's just that cherry-pick+amending commit doesn't mark PR as merged on GitHub frontend, but it is. I left this open to discuss those remaining issues.

@eliemichel
Copy link
Contributor

Then shouldn't be called higher in the loop? It feels strange to be raising events at a random "middle of main loop" location

Agreed, I tend to pu this either at the very beginning or very end (the end makes sense because it's primary job is to report errors -- note that with wgpu-native error reporting is instantaneous instead of upon polling btw).

Shouldn't they be fixed then? or there is no way to fix them without modyfing Dawn?

Good question, I'm not expert in Vulkan layers, I noticed similar messages coming from other validation layers present on my system.

If there is a way to make the onlooker/user experience more lightweight I would appreciate, as long as the code ultimately supports both.

The current native part of the code is Dawn-specific for the following reasons:

  • It uses webgpu_glfw which is a Dawn thing. I'd suggest glfw3webgpu for a portable solution (also handling the emscripten case without the need for #ifdef), which hopefully will get merged into GLFW one day.
  • It uses a bizarre mix of raw C API and C++ wrapper, the latter being Dawn-specific. I'd suggest sticking with the standard C API for such an example.
  • It uses wgpuDeviceTick, which is Dawn-specific. The kind of equivalent on wgpu-native side is wgpuDevicePoll, which is located in webgpu/wgpu.h because wgpu-native clearly splits non-standard extensions in its own header. Implementation-specific #ifdef is annoying to do because there is no official way; so in my WebGPU-distribution I took the liberty to add WEBGPU_BACKEND_WGPU and WEBGPU_BACKEND_DAWN to the CMake targets. This issue has been reported upstream and a more official way should come up eventually.
  • It uses the soon-to-be-deprecated explicit SwapChain (already dropped from wgpu-native)

As I'm writing this I realize it is worth a PoC, so here it is: #7523

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

7 participants