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 GLFW and SDL2 examples #8381

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

Conversation

BrutPitt
Copy link

@BrutPitt BrutPitt commented Feb 7, 2025

As already anticipated here, I send you the PR with the example GLFW in which the Swap-Chain was removed in favor of SurfaceConfigure.

But given the little troubles with GLFW (Validation Layer messages), I wanted to check with SDL2 if everything worked well.
So I created a function to acquire the surface in a native way on SDL.
It works and has been tested (with ImGui SDL2 WebGPU example) in Windows, Linux (X11) and Emscripten
There is also the possibility with Linux (Wayland) , but I haven't tried it yet.

The ImGui SDL2 WebGPU example works well: the web version is live here (obviously a browser with webGPU capabilities is required)

In SDL2 everything works perfectly and there are no warnings from the Validation Layer: it was tested in Linux, Windows and EMSCRIPTEN*

@ocornut
Copy link
Owner

ocornut commented Feb 10, 2025

Hello,
Could you ensure the new symbols use matching code style?

  • surfaceConfiguration -> wgpu_surface_configuration

  • surfaceResize() -> ResizeSurface()

  • wgpuSurfaceConfigure( wgpu_surface, (WGPUSurfaceConfiguration *) &surfaceConfiguration ); -> wgpuSurfaceConfigure(wgpu_surface, (WGPUSurfaceConfiguration*)&surfaceConfiguration);

  • etc.

  • brace on new line

  • Can you clarify why the RequestAdapter() stuff needs to be changed? Is e.g. std::move() an absolute requirement (spoiler: I don't know what it does) ?

  • Could the new callback have declared signatures instead of using lambdas?

Thank you!

@ocornut
Copy link
Owner

ocornut commented Feb 10, 2025

In addition, if I try to compile example_glfw_wgpu/ BEFORE your change using Dawn, I get many other errors (which are not the one you are aiming to fix).

imgui_impl_wgpu.cpp(66,41): error C2187: syntax error: 'WGPUComputeState' was unexpected here [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(273,5): error C2065: 'WGPUShaderSourceWGSL': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(273,26): error C2146: syntax error: missing ';' before identifier 'wgsl_desc' [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(273,26): error C2065: 'wgsl_desc': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(273,36): error C3079: an initializer list cannot be used as the right operand of this assignment operator [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(274,5): error C2065: 'wgsl_desc': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(274,29): error C2065: 'WGPUSType_ShaderSourceWGSL': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(275,5): error C2065: 'wgsl_desc': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(275,37): error C2065: 'WGPU_STRLEN': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(283,62): error C2065: 'wgsl_desc': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(288,39): error C2065: 'WGPU_STRLEN': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(288,29): error C2440: '=': cannot convert from 'initializer list' to 'const char *' [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
      imgui_impl_wgpu.cpp(288,29):
      The initializer contains too many elements
imgui_impl_wgpu.cpp(405,13): error C2065: 'WGPU_STRLEN': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(432,13): error C2065: 'WGPU_STRLEN': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(545,55): error C2065: 'WGPU_STRLEN': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(545,26): error C2440: '=': cannot convert from 'initializer list' to 'const char *' [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
      imgui_impl_wgpu.cpp(545,26):
      The initializer contains too many elements

imgui_impl_wgpu.cpp(612,9): error C2065: 'WGPU_STRLEN': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
imgui_impl_wgpu.cpp(720,45): error C2065: 'WGPUOptionalBool_False': undeclared identifier [C:\Omar\Work\imgui\examples\example_glfw_wgpu\build\example_glfw_wgpu.vcxproj]
  imgui_widgets.cpp
  Generating Code...

Yet my copy of Dawn appears to be up to date, so I am quite confused about the build process it seems.

@BrutPitt
Copy link
Author

BrutPitt commented Feb 10, 2025

Hi Omar, let me start with your last message.

I tried to compile the GLFW example under Windows, again, and continue to get NO errors.
I tried also to:

  • delete DAWN and re-clone it entirely
  • delete all sources and re-clone they entirely
  • delete all CMake cache and previous generated files
  • in the end have compiled with: VS2022 (from CMakeFile), with CLion (from CMakeFile) and also directly from command line, and to do this I followed the directives contained in the CMakeFile.txt (file not touched by me) step by step, and which I report below:
# Building for desktop (WebGPU-native) with Dawn:
#  1. git clone https://github.com/google/dawn dawn
#  2. cmake -B build -DIMGUI_DAWN_DIR=dawn
#  3. cmake --build build

and always I get NO errors.

I add, for full completeness, after cloning DAWN I run python tools/fetch_dawn_dependencies.py --use-test-deps command (before to compile), as reported here (googlesource site)

I also noticed that in your report appears example_glfw_wgpu.vcxproj that I don't find from me, nor before, nor after.
I also opened imgui_examples.sln, but it does not contain example_glfw_wgpu.
So I also generated, from CMake, an imgui_example_glfw_wgpu.sln (and obviously also relative .vcxproj), and again get NO errors to compile it

Screenshot 2025-02-10 220006

 

  • For this I ask you which procedure did you follow to compile the example?

 

Moreover, I checked the state of the "undeclared identifier" that you report, and they exist (in my copy of dawn):
e.g. WGPUOptionalBool_False in webgpu.h

Screenshot at 2025-02-10 22-19-02

And, searching the net, I found that WGPUOptionalBool_False was declared lately, in DAWN, but already (at least) from September:
#7969

This seemed to me the most important thing to clarify as soon possible, later I reply to your questions

P.S.
In this reply I focused only on Windows, because of the .vcxproj extension, but obviously it compile with success also in Linux and emscripten

@BrutPitt
Copy link
Author

BrutPitt commented Feb 11, 2025

About your questions:

About code style

No problem to use your code style.

About changes in RequestAdapter/Device

RequestAdapter was changed because wgpuInstanceRequestAdapter has been changed from Google Dawn:

Currently this is the wgpuInstanceRequestAdapter prototype, it accepts THREE parameters:

WGPUFuture wgpuInstanceRequestAdapter(WGPUInstance instance, WGPU_NULLABLE WGPURequestAdapterOptions const * options, WGPURequestAdapterCallbackInfo callbackInfo) WGPU_FUNCTION_ATTRIBUTE;

In the previous code wgpuInstanceRequestAdapter used FOUR parameters :

 WGPUAdapter adapter;
    wgpuInstanceRequestAdapter(instance, nullptr, onAdapterRequestEnded, (void*)&adapter);
    return adapter;                                                            ^^^^^^^^^

The 4th parameter was removed from google DAWN

Same for wgpuInstanceRequestDevice

WGPU_EXPORT WGPUFuture wgpuAdapterRequestDevice(WGPUAdapter adapter, WGPU_NULLABLE WGPUDeviceDescriptor const * options, WGPURequestDeviceCallbackInfo callbackInfo) WGPU_FUNCTION_ATTRIBUTE;

About std::move

I make just a little example:

    std::string str1 {"AAAAAA"};
    std::string str2 {"BBBBBB"};

    std::string str1_out = str1;
    std::string str2_out = std::move(str2);

The first assignment call the copy constructor/operator: it copy the full string "AAAAAA" into str1_out.
The second move the reference of str2 into str2_out and simply set str2 in a valid but unspecified state
Both str1_out and str2_out will have what is expected, but str2_out with less work for the CPU.
So std::move, move the reference of an object into another, and this is very useful when a local object must to be moved/assigned to a global variable, when it's near to end its scope and then it will not be needed anymore (frequently used in lambda).

(I'm sorry for the short and simplistic explanation: std::move is a "bit" more complex)

  • I'm sorry, this had escaped me, so I add:

Absolute requirement, no.
I went to see the class constructor and the assignment operator and, using std::move, the "release" functions (e.g. WGPURelease) are avoided on an object which will soon be destroyed.

And again:
I tried with the simple assignment and all works (obviously), but I receive this warning from Clang-Tidy:
"Parameter 'adapter' is passed by value and only copied once; consider moving it to avoid unnecessary copies"

About callback

In principle, no problem, but (to understand well) you want that I remove all lambdas, not only the validation layers callbacks, right?

@PathogenDavid
Copy link
Contributor

PathogenDavid commented Feb 11, 2025

Just to give an extra data point, I was able to successfully build and run example_glfw_wgpu on Windows using @BrutPitt's instructions and google/dawn@2a84206.

I've never worked with WebGPU on this machine so the Dawn clone was as fresh as can be. I don't have depot_tools or any other Chromium-specific stuff installed.

This is with Windows 10 22H2, CMake 3.30.2, Python 3.12.5, Visual Studio 2022 17.12.4 (latest). I also have Visual Studio 2022 17.13.0p2.1 and Ninja 1.12.1 on this machine, but I don't think either were used while building the example.

I add, for full completeness, after cloning DAWN I run python tools/fetch_dawn_dependencies.py --use-test-deps command (before to compile), as reported here (googlesource site)

I also ran this as per your mention, although judging by our CMakeLists it shouldn't actually be necessary.

(Also for @ocornut's sake: If you're like me and habitually add --recursive to your git clone commands. Definitely don't do that with Dawn unless you hate bandwidth, time, and storage space. It also might not actually work since their tooling probably doesn't expect you to do it.)

@BrutPitt
Copy link
Author

BrutPitt commented Feb 11, 2025

@ocornut

About SDL2 WebGPU example attached.

As mentioned it uses a "made by me" module sdl2wgpu.cpp with SDL native calls to get SDL_SysWMinfo data to pass wgpuInstanceCreateSurface call.
And despite being a widely mentioned and used method to acquire the info from/of WM to create Surface, I know is not an official DAWN wgpu procedure.
*and currently I tested it only on Windows, Linux (X11) and emscripten, and the macOS procedure has yet to be written

But I made and attached it exclusively to "present" an example without messages from the Validation Layer and consider/test the correct procedure also/already used in GLFW.

So, give the premises, if you prefer, I take it off from the PR.
Possibly, after, I add it as "un-official" SDL2 imgui WGPU example on my account

@PathogenDavid

This is with Windows 10 22H2, CMake 3.30.2, Python 3.12.5, Visual Studio 2022 17.12.4 (latest). I also have Visual Studio 2022 17.13.0p2.1 and Ninja 1.12.1 on this machine, but I don't think either were used while building the example.

You did well to specify the developer tools releases and OS
Personally tested on/with:

  • Windows 11 24H2
  • CMake > 3.30: bundled compiler versions (CLion/VS) and also last official 3.31.5 release (for command line build and to generate .sln)
  • VS v17.12.3
  • Builder: bundled Ninja on CLion 2024.3.2, otherwise MSbuild

I add, for full completeness, after cloning DAWN I run python tools/fetch_dawn_dependencies.py --use-test-deps command (before to compile), as reported here (googlesource site)

I also ran this as per your mention, although judging by our CMakeLists it shouldn't actually be necessary.

Confirm that this is not necessary (on Linux I don't used it), and therefore is not necessary to insert this step in the CMake procedure, but given the doubts I wanted to follow all the "official" steps

…callbacks declarated as signature - Validation Layer callbacks moved from lambdas to standard functions
@BrutPitt
Copy link
Author

BrutPitt commented Feb 12, 2025

Last commit changes:

If you prefer RequestAdaper and RequestDevice declared as functions would have this code, in a #ifndef __EMSCRIPTEN__ block:

#ifndef __EMSCRIPTEN__
static wgpu::Adapter localAdapter;   // currently used as local declaration in InitWGPU, both
static wgpu::Device localDevice;

void RequestAdapter(wgpu::RequestAdapterStatus status, wgpu::Adapter adapter, wgpu::StringView message)
{
    if (status != wgpu::RequestAdapterStatus::Success)
    {
        printf("Failed to get an adapter: %s\n", message.data);
        return;
    }
    localAdapter = std::move(adapter);
}

void RequestDevice(wgpu::RequestDeviceStatus status, wgpu::Device device, wgpu::StringView message)
{
    if (status != wgpu::RequestDeviceStatus::Success)
    {
        printf("Failed to get an device: %s\n", message.data);
        return;
    }
    localDevice = std::move(device);
}
#endif

Added notes:

  • I unified the two #ifdef __EMSCRIPTEN__ blocks in IntWGPU: now - before (previous/current imgui WGPU example)

…er and RequestDevice callbacks declarated as signature - Validation Layer callbacks moved from lambdas to standard functions
… of all resources used (not present in original example)
@BrutPitt
Copy link
Author

BrutPitt commented Feb 17, 2025

Hi, Omar.

It's frustrating to work on Dawn: last branch, downloaded Saturday, causes "segmentation fault" on WebGPU Vulkan backend.
And also all the DAWN examples (HelloTriangle, Anemometer, etc) have the same problem: on my PCs this happens with NVidia RTX 3080 (Linux) and AMD RX 6700 XT (Windows 11).
I have included in the code a way to force a backendType change to D3D11 (or D3D12), for Windows, as workaround.
Currently, have no option for Linux where Vulkan is the only option.
This does not happens in Emscripten

Edit - current problem is reported here:
https://issues.chromium.org/issues/397448368

About the GLFW example

Exploring the Dawn Source Code (the documentation is poor) I made some optimizations and correction to the example:

A new function has been inserted to directly create the device, without the use of callbacks
It is not used anywhere, there is only this note: - Synchronous adapter.CreateDevice(const wgpu::DeviceDescriptor*) may be called.

Currently there is no similar option for Adapter, not for wgpu::Instance class, but only from dawn::native::Instance, this is an example of implementation:

	// Declare DAWN RAII instead of wgpu::Instance
    dawn::native::Instance instance(&instanceDescriptor);

    // Enumerate all devices
    std::vector<dawn::native::Adapter> adapterList = instance.EnumerateAdapters((WGPURequestAdapterOptions*) nullptr);
    assert(!adapterList.empty());

    // we get first available
    dawn::native::Adapter localAdapter { adapterList[0] };

But works only with DAWN (even if it does not involve the EMISCRIPTEN side), for this I currently did not use it

About GLFW viewport error:

It occurs only on window resize, not always, and only when enlarging.

Validation error: Viewport bounds (x: 0.000000, y: 0.000000, width: 1757.999878, height: 1259.000000) are not contained in the render target dimensions (1758 x 1256).
 - While encoding [RenderPassEncoder (unlabeled)].SetViewport(0.000000, 0.000000, 1757.999878, 1259.000000, 0.000000, 1.000000).
 - While finishing [CommandEncoder (unlabeled)].

This because, currently, GLFW does not manage the framebuffer, but only the frame size: it's the example that deals with the downsizing of the framebuffer/Surface, based on the FrameSize acquired through GLFW:

  • First, when the window is resized, the example resize the Surface (framebuffer).
  • Then, ImGui calls glfwGetFramebufferSize from ImGui_ImplGLFW_NewFrame to set io.DisplaySize, but really gets only the "frame size" of the window, which can also be varied in the meantime, during the window drag, respect to Surface resized previously from example.
  • End, ImGui_ImplWGPU_RenderDrawData calls wgpuRenderPassEncoderSetViewport with the DisplaySize parameters, and if the Viewport is bigger of the Surface dimensions, an error occurs.

To avoid a further bothersome inconvenience that happens in this situation:
Viewport bounds (x: 0.000000, y: 0.000000, width: 747.000000, height: 864.000061) are not contained in the render target dimensions (747 x 864).
e.g. Viewport width: 864.000061 vs Surface width: 864, that happens from this call: wgpuRenderPassEncoderSetViewport(ctx, 0, 0, draw_data->FramebufferScale.x * draw_data->DisplaySize.x, draw_data->FramebufferScale.y * draw_data->DisplaySize.y, 0, 1);
A workaround (without to distort ImGui behavior) is to set the Surface just a pixel bigger of that acquired by FrameSize

This not happens in SDL because the "frame size" not change asynchronously, but only after aSDL_PollEvent call, so two calls of SDL_GetWindowSize, in the same iteration cycle, get the same sizes

The only option that works with GLFW is the one in which the example passes the Surface sizes to ImGui_ImplGLFW_NewFrame, but this distorts the current ImGui method and moreover, sooner or later, GLFW will be adapted to the WebGPU platform.

Anyway here there is an online GLFW EMSCRIPTEN example where I have modified ImGui_ImplGLFW_NewFrame that acquires the correct FrameBufferSize from the example, and all works fine.
(tested with last Google Chrome Canary, with Unsafe WebGPU Support - Enabled)

Currently the SDL example hasn't been changed yet

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.

3 participants