Skip to content

[WIP] update to wgpu-native v25.0.2.1 #718

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Conversation

Vipitis
Copy link
Contributor

@Vipitis Vipitis commented May 24, 2025

as usual, trying to get this done. Barely anything new - but changes to Dx12 which might need some fixes.
also thinking about how to make InstanceExtras available to the user.


useful links:

Preparations

  • Perhaps warp up outstanding PR's.
  • Might be a good time to roll a release of wgpu-py.
  • Perhaps for pygfx too.

Upstream

  • wgpu-native is updated to a recent wgpu-core.
  • wgpu-native uses a recent webgpu.h. (it hasn't changed since v24)
  • wgpu-native has a release with these changes.

Update to latest wgpu-native

The lines below can be copied in the PR's top post.

  • Run python tools/download_wgpu_native.py --version xx to download the latest webgpu.h and DLL.
  • Run python codegen to apply the automatic patches to the code.
  • It may be necessary to tweak the hparser.py to adjust to new formatting.
  • Diff the report for new differences to take into account.
  • Diff wgpu_native/_api.py to get an idea of what structs and functions have changed.
  • Go through all FIXME comments that were added in _api.py:
    * Apply any necessary changes.
    * Remove the FIXME comment if no further action is needed, or turn into a TODO for later.
  • Run python codegen again to validate that all is well. Repeat the steps above if necessary.
  • Make sure that the tests run and provide full coverage. (d3d12 crashes with some tests, but isn't tested on CI)
  • Make sure that the examples all work.
  • Make sure that pygfx works (again).
  • Update release notes for changes in behavior.

Wrapping up

  • Update pygfx.
  • Release new wgpu-py.
  • Release new pygfx.
  • This can be a good moment to deal with certain issues, or perhaps some issues can be closed.

@almarklein
Copy link
Member

It sounds like you can even target a specific shader model.

Can you share some references?

@Vipitis
Copy link
Contributor Author

Vipitis commented May 26, 2025

I managed to do use the instance extras to specify Dxc with a hard coded path for the .dll which resulted I. Shader model 6.5. the default is fxc which does like shader model 5.1 or 5.2.

But I haven't been successful in doing the instance flag for debug, is is meant to set the compiler to output debug symbols for the shader module... Not quite sure where it's failing. It needs to reach here https://github.com/gfx-rs/wgpu/blob/f35cf942af1a3bb6f48aa9185f4d2bcee809f814/wgpu-hal/src/dx12/shader_compilation.rs#L297

via what's explained here gfx-rs/wgpu-native#327

But I am not sure if there is like an order to it.

I will maybe paste or push the C and python version to do this, it's really janky right now. As Dxc isn't packaged you have to point to two files manually, in the Future only one of them. Maybe we can consider setting these with WGP_DEBUG=1 or have some better API design.

@Vipitis
Copy link
Contributor Author

Vipitis commented May 27, 2025

while CI is green but directx is crashing on tests that request native limits for push constants.... I will try to gather my memory from last update there this already caused some trouble. Could be one of those pointer dereferences again - but wouldn't make much sense as Vulkan and OpenGL work...

I put the C code that changed directx compiler for me here: https://github.com/Vipitis/wgpu-native/tree/instance-extras
the python code that calls essentially the same value is currently in _api.py I am not sure how this would be best exposed to the user, as the globale instance in _helpers gets called during request adapter and enumerate adapter, so you would need an arg to both for example. As you need to provide like several paths, it wouldn't make sense to do this just via env var either.

@Vipitis
Copy link
Contributor Author

Vipitis commented May 27, 2025

current suspicion. D3D12 is running into another issue (gfx-rs/wgpu#5285) and therefore is using the Default DownlevelLimits, which set max-push-constants to 0. Oddly enough if I don't request the limits by commenting out this line I no longer crash but get incorrect results. Not sure what to think about this but might just be an upstream issue because it doesn't seem like we can influence these downlevel flags at all.

I also saw that Vulkan complained about these tests with the InstanceFlag for validation (proving they seem to work at least).

Running test_normal_push_constants ...
Forcing backend: Vulkan (6)
VALIDATION [VUID-VkGraphicsPipelineCreateInfo-layout-10069 (0xadf5e51f)]
        vkCreateGraphicsPipelines(): pCreateInfos[0].pStages[0] SPIR-V (VK_SHADER_STAGE_VERTEX_BIT) has a push constant buffer Block with range [0, 80] which outside the VkPushConstantRange of [0, 40].
The Vulkan spec states: If a push constant block is declared in a shader, the block must be contained inside the push constant range in layout that matches the stage (https://vulkan.lunarg.com/doc/view/1.4.313.0/windows/antora/spec/latestchapters/pipelines.html#VUID-VkGraphicsPipelineCreateInfo-layout-10069)
        objects: (type: SHADER_MODULE, hndl: 0xb000000000b, name: ?), (type: PIPELINE_LAYOUT, hndl: 0xa000000000a, name: ?)
VALIDATION [VUID-VkGraphicsPipelineCreateInfo-layout-10069 (0xadf5e51f)]
        vkCreateGraphicsPipelines(): pCreateInfos[0].pStages[1] SPIR-V (VK_SHADER_STAGE_FRAGMENT_BIT) has a push constant buffer Block with range [0, 80] which outside the VkPushConstantRange of [40, 80].
The Vulkan spec states: If a push constant block is declared in a shader, the block must be contained inside the push constant range in layout that matches the stage (https://vulkan.lunarg.com/doc/view/1.4.313.0/windows/antora/spec/latestchapters/pipelines.html#VUID-VkGraphicsPipelineCreateInfo-layout-10069)
        objects: (type: SHADER_MODULE, hndl: 0xc000000000c, name: ?), (type: PIPELINE_LAYOUT, hndl: 0xa000000000a, name: ?)

@Vipitis Vipitis changed the title [WIP] update to wgpu-native v25.0.0.1 [WIP] update to wgpu-native v25.0.2.1 May 27, 2025
@Vipitis
Copy link
Contributor Author

Vipitis commented May 28, 2025

An idea for the API design. Add a function to wgpu.extras that can set instance flags so global _the_instance keeps hold of these. Maybe set_instance_flags() with a python friendly kwargs and documentation.

Now subsequent calls to get_wgpu_instance() as currently is should make use of them. only limitation I see is that these have to be set before anything else calls it. And the actual helper tools like new_struct or to_c_string_view() are currently in _api.py.

Will try to implement that tomorrow night, let me know if there is other suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants