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

spirv: impl Debug + Display, fix Affine::from_cols_array() #559

Merged
merged 3 commits into from
Oct 16, 2024

Conversation

Firestar99
Copy link
Contributor

I would like to get a 👍 from the rust-gpu maintainers before this is merged, but feel free to already review this. Thanks :D

Added features for the spirv target:

  • rust-gpu v0.9.0 added support for Debug and Display traits, this PR removes all the #[cfg(not(target_arch = "spirv"))] hiding the traits impls for the spirv target.
  • Make Affine::from_cols_array() not use any slices internally and use arrays instead, as this caused compilation errors when these methods were used on the spirv target.

I tested this PR against embark's rust-gpu master branch and the latest v0.9.0 release using their compiletest suite.
(Note: test member_ref_arg-broken may fail on v0.9.0 due to a diff in stdout caused by newer spirv tools)

@eddyb
Copy link

eddyb commented Oct 7, 2024

rust-gpu v0.9.0 added support for Debug and Display traits

To be clear, running the derived impls isn't really supported, there's just some minimal panic! format string conversion to debugPrintf, mainly for simple scalars but I guess vectors are also supported?
(i.e. the only reason to have these fmt::{Debug,Display} impls is so that e.g. panic!("{some_vec4}") type-checks, before it can get transformed into the equivalent debugPrintf)

@Firestar99 Firestar99 marked this pull request as ready for review October 7, 2024 09:27
@Firestar99
Copy link
Contributor Author

@bitshifter could we get this merged? Thanks :D

@bitshifter bitshifter merged commit 2a3f6d4 into bitshifter:main Oct 16, 2024
18 checks passed
@bitshifter
Copy link
Owner

Sorry for the delay, GitHub didn't notify me that the PR had moved out of draft.

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.

3 participants