Skip to content
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

[hlsl-out] Fix countOneBits and reverseBits for signed integers #1928

Merged
merged 1 commit into from
May 16, 2022

Conversation

hasali19
Copy link
Contributor

@hasali19 hasali19 commented May 16, 2022

This PR fixes usage of the HLSL countbits and reversebits functions with signed integers, by adding the necessary asuint and asint conversions.

Fixes #1904

Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Could you add HLSL to this list and generate the snapshot for it?

naga/tests/snapshots.rs

Lines 418 to 419 in 60ae549

"bits",
Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::WGSL,

@@ -2190,6 +2190,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
Asincosh { is_sin: bool },
Atanh,
Regular(&'static str),
UintWrapped(&'static str),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we name this NoIntOverload or MissingIntOverload? What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to think of a good name, that sounds much better!

@hasali19
Copy link
Contributor Author

Looks good! Could you add HLSL to this list and generate the snapshot for it?

naga/tests/snapshots.rs

Lines 418 to 419 in 60ae549

"bits",
Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::WGSL,

I noticed that previously and attempted to add it, but it looks like there are some other functions which aren't implemented yet causing it to fail:

thread 'convert_wgsl' panicked at 'HLSL write failed: Unimplemented("write_expr_math Pack4x8snorm")', tests/snapshots.rs:313:54
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@teoxoy
Copy link
Member

teoxoy commented May 16, 2022

Alright, we can enable the test later then. Thanks!

@teoxoy teoxoy merged commit 571302e into gfx-rs:master May 16, 2022
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.

[hlsl-out] Incorrect result for min(reverseBits(-1), 0)
2 participants