Skip to content

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

Merged
Lokathor merged 1 commit intoLokathor:mainfrom
DJMcNab:spir-v-2
Jun 13, 2021
Merged

Spir-v: just gate the Display impl instead#69
Lokathor merged 1 commit intoLokathor:mainfrom
DJMcNab:spir-v-2

Conversation

@DJMcNab
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

DJMcNab commented Jun 13, 2021

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

@Lokathor
Copy link
Copy Markdown
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