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

Spir-v: just gate the Display impl instead #69

Merged
merged 1 commit into from
Jun 13, 2021

Conversation

DJMcNab
Copy link
Contributor

@DJMcNab DJMcNab commented Jun 12, 2021

The old version didn't compile (because of the extra semicolon).

It is extremely unlikely that anyone would ever want to use this impl on gpu, at least whilst the compiler is in its current state - if the impls did start working, they could just use the Debug impl.

This is not a breaking change, since all prior versions didn't compile on spir-v anyway.

As a small note, there is no expectation for you to proactively support the spir-v target; beyond checking PRs which fixup any breakage - bytemuck stopping compiling would be the least breaking change in the entire rust-gpu ecosystem.

The old version didn't compile (because of the extra semicolon)
@Lokathor
Copy link
Owner

reasonable, and this can be "undone" later if necessary by making the impl be everywhere with conditional compilation moving back inside.

@Lokathor Lokathor merged commit 291b2d2 into Lokathor:main Jun 13, 2021
@DJMcNab DJMcNab deleted the spir-v-2 branch June 13, 2021 14:14
@DJMcNab
Copy link
Contributor Author

DJMcNab commented Jun 13, 2021

Yeah, exactly. Moving the strange behaviour to compile time is useful.

@Lokathor
Copy link
Owner

released as 1.6.3

DJMcNab added a commit to DJMcNab/rust-gpu that referenced this pull request Jun 13, 2021
Lokathor/bytemuck#69 landed

That version also works 🎉
DJMcNab added a commit to DJMcNab/compute-shader-101 that referenced this pull request Jun 13, 2021
as of Lokathor/bytemuck#69

No longer depend on it conditionally
khyperia pushed a commit to EmbarkStudios/rust-gpu that referenced this pull request Jun 14, 2021
* Use bytemuck for the push constants

* Use the released version of bytemuck

Lokathor/bytemuck#69 landed

That version also works 🎉
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.

2 participants