Conversation
5ed5026 to
44f186a
Compare
There was a problem hiding this comment.
Code Review
This pull request updates the project's dependency stack, upgrading glam to version 0.32.1 and migrating rust-gpu components to registry-based versions (0.10.0-alpha.1). It also refines the Nix development environment by removing unnecessary packages and updating toolchains. Review feedback highlights a regression where BufferStruct implementations for glam::Quat and glam::DQuat were commented out; these should be restored with manual implementations if necessary or removed with a clear explanation.
| // glam_array!(Vec3A, [f32; 4]); | ||
| glam_array!(glam::Vec4, [f32; 4]); | ||
| glam_array!(glam::Quat, [f32; 4]); | ||
| // glam_array!(glam::Quat, [f32; 4]); |
There was a problem hiding this comment.
The implementation of BufferStruct for glam::Quat has been commented out. This is a regression in the library's support for shader buffer types. If this was due to compilation issues with glam 0.32 or rust-gpu 0.10, it is preferable to provide a manual implementation (similar to the Vec3A implementation at line 85) rather than disabling it, as Quat is a fundamental type for representing rotations in shader buffers. If it is truly no longer supported or needed, it should be removed entirely with a comment explaining the rationale.
There was a problem hiding this comment.
Quat is for rotations in 3D space, which you're not using. glam changed something in their implementation that makes their functions differ from the vector types.
| glam_array!(glam::DVec3, [f64; 3]); | ||
| glam_array!(glam::DVec4, [f64; 4]); | ||
| glam_array!(glam::DQuat, [f64; 4]); | ||
| // glam_array!(glam::DQuat, [f64; 4]); |
There was a problem hiding this comment.
1 issue found across 12 files
Confidence score: 4/5
- This PR is likely safe to merge from a code-risk perspective; the only reported issue is a process/compliance check rather than a runtime or functional defect.
- The most severe finding is PR title enforcement: the title should be changed from
update rust-gpu to0.10.0-alpha.1to `Update rust-gpu to `0.10.0-alpha.1to satisfy the required capitalized imperative format. - Given the issue’s nature, user-facing regression risk appears low, but it may still block merge automation until corrected.
- Pay close attention to
libraries/dyn-any/Cargo.toml- ensure the associated PR metadata/title policy check is addressed so merge is not blocked.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libraries/dyn-any/Cargo.toml">
<violation number="1" location="libraries/dyn-any/Cargo.toml:31">
P1: Custom agent: **PR title enforcement**
PR title must start with a capitalized imperative verb (sentence case). Change `update rust-gpu to `0.10.0-alpha.1`` to `Update rust-gpu to `0.10.0-alpha.1``.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
!build desktop (Run ID 24720128854) |
0.10.0-alpha.10.10.0-alpha.1
44f186a to
adee7fc
Compare
|
|
|
|
Desktop/Nix seems to work, generally LGTM |
|
I haven't tested them either. I also see the "enable webgpu" button got removed, but webgpu isn't enabled since the "Upload Texture" node is panicing, so how does one enable it nowadays? |
|
Thanks for this! Regarding the "enable webgpu" button, I do not recall that ever existing. Are you thinking of the button in the preferences to enable Vello? That is now enabled by default. |
0.10.0-alpha.1cargo-gputocargo-gpu-install, removing unnecessary dependencies likeclap,crosstermandinotifycrate-type = ["dylib"]declaration, now unnecessaryraster-nodes-shaders-entrypointcrate, I think it may be related to cargo bugs arounddylibhandling, so it could also be unnecessary now?spirv-unknown-naga-wgsl.json), now unnecessary0.30.8nix flake updateto have toolchainnightly-2026-04-11availablealias cargo='mold --run cargo'as it just errors withmold: fatal: unknown command line option: --runfor whatever reasoncargocode completion is still broken witherror: no such command: cargonix run .works locally