[WIP] update to wgpu-native v25.0.2.1
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:
- diff wgpu-core
- diff wgpu-native
Preparations
- [ ] Perhaps warp up outstanding PR's.
- [x] Might be a good time to roll a release of wgpu-py.
- [ ] Perhaps for pygfx too.
Upstream
- [x] wgpu-native is updated to a recent wgpu-core.
- [ ] wgpu-native uses a recent
webgpu.h. (it hasn't changed since v24) - [x] 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.
- [x] Run
python tools/download_wgpu_native.py --version xxto download the latest webgpu.h and DLL. - [x] Run
python codegento apply the automatic patches to the code. - [ ] It may be necessary to tweak the
hparser.pyto adjust to new formatting. - [x] Diff the report for new differences to take into account.
- [x] Diff
wgpu_native/_api.pyto get an idea of what structs and functions have changed. - [x] 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. - [x] Run
python codegenagain 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)
- [x] 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.
It sounds like you can even target a specific shader model.
Can you share some references?
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 https://github.com/gfx-rs/wgpu-native/issues/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.
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.
current suspicion. D3D12 is running into another issue (https://github.com/gfx-rs/wgpu/issues/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: ?)
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_extras() 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.
as far as I can tell the instance extras work, like you can request debug mode from fxc and vulkan for sure (shows up in renderdoc). While dxc works for compile, it seems to not work for debug flags, but that also happens in C, so I filed it as an issue there: https://github.com/gfx-rs/wgpu-native/issues/487
still stuck on the dx12 crash with max-push-constants limit requested (due to the downlevel thing). One potential issue I learned about is the WGPUNativeLimits requires out and in at times for different tasks: https://webgpu-native.github.io/webgpu-headers/StructChaining.html#OutStructChainError and the difference here is if it's a pointer or const pointer... although I couldn't make the alternative work (and vulkan still works!).
I stepped through this a lot of times and noticed that the logger callback triggers before the crash to output the warning:
Forcing backend: D3D12 (4)
Missing downlevel flags: DownlevelFlags(VERTEX_AND_INSTANCE_INDEX_RESPECTS_RESPECTIVE_FIRST_VALUE_IN_INDIRECT_DRAW)
The underlying API or device in use does not support enough features to be a fully compliant implementation of WebGPU. A subset of the features can still be used. If you are running this program on native and not in a browser and wish to limit the features you use to the supported subset, call Adapter::downlevel_properties or Device::downlevel_properties to get a listing of the features the current platform supports.
DownlevelCapabilities {
flags: DownlevelFlags(
COMPUTE_SHADERS | FRAGMENT_WRITABLE_STORAGE | INDIRECT_EXECUTION | BASE_VERTEX | READ_ONLY_DEPTH_STENCIL | NON_POWER_OF_TWO_MIPMAPPED_TEXTURES | CUBE_ARRAY_TEXTURES | COMPARISON_SAMPLERS | INDEPENDENT_BLEND | VERTEX_STORAGE | ANISOTROPIC_FILTERING | FRAGMENT_STORAGE | MULTISAMPLED_SHADING | DEPTH_TEXTURE_AND_BUFFER_COPIES | WEBGPU_TEXTURE_FORMAT_SUPPORT | BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED | UNRESTRICTED_INDEX_BUFFER | FULL_DRAW_INDEX_UINT32 | DEPTH_BIAS_CLAMP | VIEW_FORMATS | UNRESTRICTED_EXTERNAL_TEXTURE_COPIES | SURFACE_VIEW_FORMATS | NONBLOCKING_QUERY_RESOLVE,
),
limits: DownlevelLimits,
shader_model: Sm5,
}
And it's theoretically possible to detect this and then raise an exception to avoid a crash... so I will have a look at the logger based error handling over the weekend.
I think I have finally figured out what caused the crash and can finally continue to get it working.
for reference (mostly myself if this happens again next update). Creating the adapter calls _get_limits() which stores limits in adapter._limits as a dictionary. This includes native only limits - so you can't set these into c_required_limits as now that struct has additional fields.
it seems like a bunch of other stuff broke so I will spent my time in the next few days to get it back in order and hopefully have this ready for review this week.
I do still have some issues with dx12 remaining regarding various features:
Vulkan and even OpenGL seem to work.
No crashes but incorrect values for push constants and indirect draw. the later might be an upstream issue that is fixed for next release: https://github.com/gfx-rs/wgpu/pull/7535
I am marking this for ready for review, I can take out the set_instance_extra feature and make that a standalone PR with added example and documentation (maybe helper script for dxc download) if that helps!
Is this ready to go from your end?
Is this ready to go from your end?
yeah, perhaps a changelog entry and some kind of notice that Dx12 with native features can produce incorrect numbers in compute shaders.
@almarklein should be good to go now!
for the remaining errors( https://github.com/pygfx/wgpu-py/pull/718#issuecomment-3006617272), the stencil warnings are due to how the test is setup (hijacking the logger) and Dx12 outputs additional warnings on my system. The functionality seems to work. I also checked on main with v24 and saw the same wrong numbers with dx12 for these two native only features.
wgpu-core released v26 today and that seems to include a fix for indirect draw on dx12, not sure if push constants are also fixed... will keep a close eye on wgpu-native for the next update.
The only changes since the last release are updated examples. @Korijn let's merge this and do a release, then get the imgui fix merged and do another release.