-
Notifications
You must be signed in to change notification settings - Fork 137
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
Make fine.wgsl integer literals explicitly signed #418
Conversation
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.
I'm not going to merge, as I haven't run this change.
But if someone else can confirm this still executes as expected, that would be helpful.
Is there a corresponding Naga issue? i.e. tracking that the abstractint doesn't get maintained through unary minus (?)
@@ -311,7 +311,7 @@ fn fill_path_ms(fill: CmdFill, local_id: vec2<u32>, result: ptr<function, array< | |||
let mask1_exp = (mask_b >> 4u) & 0x1010101u; | |||
var mask1_signed = select(mask1_exp, u32(-i32(mask1_exp)), is_down); | |||
if is_bump { | |||
let bump_delta = select(u32(-0x1010101), 0x1010101u, is_down); | |||
let bump_delta = select(u32(-0x1010101i), 0x1010101u, is_down); |
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.
Good catch, thanks!
@@ -400,7 +400,7 @@ fn fill_path_ms(fill: CmdFill, local_id: vec2<u32>, result: ptr<function, array< | |||
} | |||
// packed_w now contains the winding numbers for a slice of 4 pixels, | |||
// each relative to the top left of the row. | |||
for (var i = 0u; i < local_id.y >> 2u; i++) { | |||
for (var i = 0u; i < (local_id.y >> 2u); i++) { |
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.
Is this consistent with elsewhere in our source?
I think the relational expression binds more tightly than the shift operation
Related: linebender/bevy_vello#17 |
Thanks for the PR! |
The previous literals were being interpreted as an AbstractInt, which according to the u32 constructor:
I made the literals explicit signed integers so the correct constructor overload is used.