Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

upgrade to wgpu 24.0.0#455

Merged
cwfitzgerald merged 53 commits intogfx-rs:trunkgfx-rs/wgpu-native:trunkfrom
ygdrasil-io:feature/upgrade-to-wgpu-24.0.0Copy head branch name to clipboard
Feb 14, 2025
Merged

upgrade to wgpu 24.0.0#455
cwfitzgerald merged 53 commits intogfx-rs:trunkgfx-rs/wgpu-native:trunkfrom
ygdrasil-io:feature/upgrade-to-wgpu-24.0.0Copy head branch name to clipboard

Conversation

@ygdrasil-io
Copy link
Copy Markdown
Contributor

This PR is based on #445 to upgrade wgpu to 24.0.0 and remove the memory leak.

Feel free to comment regarding the last changes.

I found one memory leak here: eliemichel@452b21e, but there is another one.

After 5-10 minutes of running, this decreases the memory leak by 50%, but it is still leaking, while the same sample is memory stable on the wgpu Rust repository.

PJB3005 and others added 30 commits September 19, 2024 17:53
This goes up to webgpu-native/webgpu-headers@2b59747

Things I *didn't* do:

* I didn't update the library to make sure "instance dropped" callback
  error codes are guaranteed to happen, like they seem to be in Dawn.

List of changes (roughly in order of header commits):

Various enum and struct renames

Updated callbacks to use the new *CallbackInfo structs and 2-userdata system. Also updated functions to return WGPUFuture, though the WGPUFuture thing is just stubbed out at the moment as I don't think wgpu-core has the necessary functionality for it. wgpuInstanceWaitAny is unimplemented!()

DepthClipControl merged into PrimitiveState, related code simplified.

Updated depthWriteEnabled to use an optional bool, mostly matters due to added validation.

Add TODOs for missing features (sliced 3D compressed textures)

*Reference() became *AddRef()

Added unorm10-10-10-2 vertex format

Usage field in TextureViewDescriptor, just used for validation as wgpu-core doesn't allow specifying it anyways.

Removed maxInterStageShaderComponents

Added clang_macro_fallback to bindgen config, since the headers switched to using UINT32_MAX etc. UINT64_MAX still doesn't work so I had to manually define those.

Renamed flags enums. Added a conversion helper function to convert them from u64 -> u32 for mapping. (means added direct dependency on bitflags crate)

Removed device argument from (unimplemented) wgpuGetProcAddress

Suboptimal surface texture acquisition moved to enum return value, was easy since wgpu-core already returns it like that.

"Undefined" present mode added, it just selects FIFO.
This enum was replaced with WGPUMapAsyncStatus, but I didn't quite notice. The error codes map different.
Replaces *EnumerateFeatures with *GetFeatures.

Also fixes CI due to fix in headers.
Upstream removed the "Flags" suffix from flags types and moved them to no longer be C enums. This matches that change. WGPUInstanceFlag still has "Flag" in the name because, well, there'd be nothing left to distinguish it from WGPUInstance, and it makes sense for it.
Also updates wgpu.h to use WGPUStringView everywhere.
All stubs, since we don't have WaitAny at the moment.
Update to webgpu-native/webgpu-headers@f1cdc3f

Also went through a bunch of existing enum conversions and fixed up some seemingly spec-incorrect cases of undefined enums not being handled properly.

As part of this, I made a new helper map_enum_with_undefined!() which distinguishes undefined and unknown enum values. Previously, much code relied on undefined being caught in the same net as an unknown value. This is no longer the case.
This updates the headers to webgpu-native/webgpu-headers@af63d34

These changes are exclusively in the header enum values, so no Rust code needs changing. Making this a separate commit for easier review.
Update headers to webgpu-native/webgpu-headers@b7656d0

Adds dual source blending. Other two features (float32 blendable and clip distances feature in wgsl) are not supported by wgpu.

Also made map_blend_factor use the shorter macro form, as all the enum names match (they didn't 3 years ago when this code was originally written, according to Git history).
Oops that's not how these chained structs work.
Updates headers to webgpu-native/webgpu-headers@6f549cc

Also made matching changes to wgpu.h
Replaced `ImageCopy*` types with `TexelCopy*` equivalents for consistency with updated API naming. Adjusted feature detection to use experimental ray tracing and ray query feature names. These changes improve clarity and align with updated standards.
    Simplifies the code by eliminating the unnecessary `return` keyword. This improves readability and aligns with idiomatic Rust practices. No functionality is affected by this change.
Reorganized load/store op mapping to include clear values and introduced `map_load_op_and_color` for better readability. Updated DX12 compiler handling to differentiate between dynamic and static Dxc paths. These changes improve code clarity and make handling of load/store operations and DX12 configurations more robust.
Corrects the incorrect use of `dxilPath` for `dxcPath` in DXC path assignment. Refactors instance descriptor to use structured `BackendOptions` for improved clarity and maintainability.
Simplified and streamlined the syntax for handling callbacks and closures. This enhances readability and consistency in the code by removing unnecessary wrapping and renaming variables for better alignment with Rust conventions.
Simplified the code by eliminating unnecessary `return` keywords before `NULL_FUTURE`. This improves code readability and aligns with Rust's idiomatic style for returning expressions. No functional changes were introduced.
Simplify conditional branches by reducing nested matches and removing redundant code. This improves readability and maintains the intended functionality for surface texture management and resource cleanup.
Updated the shader stage type from WGPUShaderStageFlags to WGPUShaderStage to align with API requirements. Adjusted conversion logic to handle the new type safely and ensure compatibility with associated functionality.
Corrected the initial state of `open` and adjusted its updates to ensure proper command buffer lifecycle management. This addresses potential inconsistencies in how command buffers are handled during operations.
@ygdrasil-io
Copy link
Copy Markdown
Contributor Author

With 70a8705 memory is stable. If someone could test on other OSs and architectures than Mac Arm.

Comment thread src/conv.rs
Refactored code to use `map_load_op` directly, replacing the removed `map_load_op_and_color` function. Adjusted call sites to explicitly map clear values, improving clarity and reducing redundant abstractions.
@ygdrasil-io ygdrasil-io marked this pull request as ready for review January 26, 2025 00:35
@Capati
Copy link
Copy Markdown

Capati commented Jan 26, 2025

With 70a8705 memory is stable. If someone could test on other OSs and architectures than Mac Arm.

Thanks for this!

I just tested a simple example (rotating cube) on Windows 11 x64, and it appears to be stable now. The memory usage no longer increases over time. I recall previous tests where the global report indicated numerous command buffers were not being released, but the issue seems to be resolved:

Global_Report {
  Surfaces:
        Surfaces:num_allocated = 0
        Surfaces:num_kept_from_user = 0
        Surfaces:num_released_from_user = 1
        Surfaces:element_size = 8
  Overview:
        adapters.num_allocated = 0
        adapters.num_kept_from_user = 0
        adapters.num_released_from_user = 1
        adapters.element_size = 8
        ----------
        devices.num_allocated = 0
        devices.num_kept_from_user = 0
        devices.num_released_from_user = 1
        devices.element_size = 8
        ----------
        pipeline_layouts.num_allocated = 0
        pipeline_layouts.num_kept_from_user = 0
        pipeline_layouts.num_released_from_user = 0
        pipeline_layouts.element_size = 16
        ----------
        shader_modules.num_allocated = 0
        shader_modules.num_kept_from_user = 0
        shader_modules.num_released_from_user = 1
        shader_modules.element_size = 16
        ----------
        bind_group_layouts.num_allocated = 0
        bind_group_layouts.num_kept_from_user = 0
        bind_group_layouts.num_released_from_user = 1
        bind_group_layouts.element_size = 16
        ----------
        bind_groups.num_allocated = 0
        bind_groups.num_kept_from_user = 0
        bind_groups.num_released_from_user = 1
        bind_groups.element_size = 16
        ----------
        command_buffers.num_allocated = 0
        command_buffers.num_kept_from_user = 0
        command_buffers.num_released_from_user = 1
        command_buffers.element_size = 8
        ----------
        render_bundles.num_allocated = 0
        render_bundles.num_kept_from_user = 0
        render_bundles.num_released_from_user = 0
        render_bundles.element_size = 16
        ----------
        render_pipelines.num_allocated = 0
        render_pipelines.num_kept_from_user = 0
        render_pipelines.num_released_from_user = 1
        render_pipelines.element_size = 16
        ----------
        compute_pipelines.num_allocated = 0
        compute_pipelines.num_kept_from_user = 0
        compute_pipelines.num_released_from_user = 0
        compute_pipelines.element_size = 16
        ----------
        query_sets.num_allocated = 0
        query_sets.num_kept_from_user = 0
        query_sets.num_released_from_user = 0
        query_sets.element_size = 16
        ----------
        textures.num_allocated = 0
        textures.num_kept_from_user = 0
        textures.num_released_from_user = 2
        textures.element_size = 16
        ----------
        texture_views.num_allocated = 0
        texture_views.num_kept_from_user = 0
        texture_views.num_released_from_user = 2
        texture_views.element_size = 16
        ----------
        samplers.num_allocated = 0
        samplers.num_kept_from_user = 0
        samplers.num_released_from_user = 0
        samplers.element_size = 16
}

@ygdrasil-io
Copy link
Copy Markdown
Contributor Author

Build available here: https://github.com/ygdrasil-io/wgpu-native/releases/tag/v24.0.0.0
Currently testing it on my kotlin binding and seem to work fine.

@laytan
Copy link
Copy Markdown

laytan commented Feb 11, 2025

I've tested this on a few of my projects on macos. It is working well and am not seeing any apparent leaks in my use cases.

One thing I noticed is that webgpu-headers was updated and now does ShaderSourceWGSL instead of ShaderModuleWGSLDescriptor. It would probably make sense to reflect that change with the shader possibilities of the wgpu-native specific header. ShaderModuleGLSLDescriptor -> ShaderSourceGLSL for example.

@cwfitzgerald
Copy link
Copy Markdown
Member

Unless anyone has a big complaint, I think we should merge this so it doesn't linger too long and if there are any issues we can fix it after landing.

@almarklein
Copy link
Copy Markdown
Collaborator

I think we should merge this so it doesn't linger too long

I Agree.

@cwfitzgerald cwfitzgerald merged commit f29ebee into gfx-rs:trunk Feb 14, 2025
@ygdrasil-io ygdrasil-io deleted the feature/upgrade-to-wgpu-24.0.0 branch February 18, 2025 18:05
@abocado18
Copy link
Copy Markdown

Now tgat it is merged, will there be a new release soon?

@almarklein
Copy link
Copy Markdown
Collaborator

almarklein commented Feb 20, 2025

Release done!

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.

8 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.