-
Couldn't load subscription status.
- Fork 20
Updated to zig 0.15.1 #20
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
Conversation
The library has syntax updated to the lastest zig version; however, because of the usingnamespace update, this might requires people who use emscripten to confirm the validity of the new implementation.
Suggested by https://ziglang.org/download/0.15.1/release-notes.html#Removal-of-BoundedArray, since std.BoundedArray is removed, the alternative is to use std.ArrayListUnmanaged; however, this type is also soon to be removed, in favor of std.ArrayList, but since this type require an allocator for appending the items; because the array is bounded, I decided to use a fixed buffer, with creating a fixedBufferAllocator for the arraylist.
|
Hmmm... I think I need some help this time, mainly about not familiar with ELF file, and I primary worked on windows. Tried to build the project with linux, and the same error pops up, but there is not much information about where it causes the issue. |
|
I'm new to zig, but I can at least get it to get through |
|
Thanks, added linkage to dynamic, it seems worked without the compile error. Tried to build the project from my testing triangle shader program, and it still works without any issues. Edit: the change has been committed, and let's see if this is a valid fix. |
Suggested by @Erdragh, seems like setting the linkage for the dawn library could fix the linux build problem, and not impacted to other platform because running a testing triangle shader program with zgpu on windows still works without any issue. Let's see if this version could pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the zgpu library from Zig 0.14.0 to 0.15.1, addressing breaking changes in the language and standard library. The update primarily involves:
- Updating calling convention syntax from
.Cto.c - Replacing deprecated
std.BoundedArraywithstd.ArrayListusing aFixedBufferAllocator - Refactoring the
wgpuDeviceTickfunction to use conditional compilation instead ofusingnamespace
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/zgpu.zig | Updated calling conventions, replaced BoundedArray with ArrayList using FixedBufferAllocator, refactored emscripten-specific code to use conditional compilation |
| src/wgpu.zig | Updated calling conventions in callback type definitions |
| build.zig.zon | Updated minimum Zig version requirement to 0.15.1 |
| build.zig | Added dynamic linkage configuration for zdawn library |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| var command_buffers = std.BoundedArray(wgpu.CommandBuffer, 32).init(0) catch unreachable; | ||
| command_buffers.append(stage_commands) catch unreachable; | ||
| command_buffers.appendSlice(commands) catch unreachable; | ||
| var buffer: [32]wgpu.CommandBuffer = undefined; |
Copilot
AI
Oct 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The buffer is left uninitialized (undefined). While this works with FixedBufferAllocator, it's better practice to initialize it with .{} for clarity and to avoid potential issues with future Zig versions that may have stricter safety checks.
| var buffer: [32]wgpu.CommandBuffer = undefined; | |
| var buffer: [32]wgpu.CommandBuffer = .{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the linkage change doesn't sit well on macOS. I guess want to avoid dynamically linking anyway. Otherwise changes look good
Thank you! So my concern to the dynamic linking was right, and we definitely need to get rid of it in some sort. According to the fork from @tristanpemble, could the use of llvm be the fix? Unfortunately, this might not be something I could fixed on my end because I don't have a Mac and well enough understanding to linux. Since I have seen some people forked my version recently, I wish to see if anyone could help or share some ideas on the mac and the linux side of the library. |
|
Yes I think we should set to use llvm here. I can test on mac and Linux. |
Due to the agreement to use llvm as a fix for the linux and potentially for the Mac, I have replaced the dynamic linkage with using llvm instead. On Windows, it compile fine, passed the test, and it works without any issues on my toy project that uses zgpu; however, futher testing is required to ensure linux and mac version of the library in working state.
|
Cool, with the llvm used, all three tests are now passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
The commit addresses the outdated zig version used in the library. While most of the code can use the same treatment by updating callconv(.c) and the build structure of build.zig; there are two changes might require follow up though:
As discussed in this issue, we can directly remove the usingnamespace keyword surrounding the function, but since emscripten_sleep(1) is exclusive for emscripten build, I ended up conditionally build the function by checking if the target platform is emscripten; otherwise, it will be an empty function.
Since I don't have any emscripten project on hands, I might need others to confirm if this implementation works.
Another change is the use of std.BoundedBuffer which is also deprecated in the 0.15.1, and suggested by the release notes, the substitution is to use ArrayListUnmanaged which will also be removed in the future version; thus, I have to use ArrayList, but because ArrayList requires an allocator for appending items, i ended up choosing fixedBufferAllocator like shown:
However, although this solution works and I have successfully to replicate the triangle example from the zig-gamedev with the updated library, the current solution doesn't address the TODO item.