Decouple glam from spirv-std, add const-generics feature flag. #476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
XAMPPRocky
force-pushed
the
decouple-glam
branch
2 times, most recently
from
b85d4f6 to
008ccc6
Compare
XAMPPRocky
force-pushed
the
decouple-glam
branch
2 times, most recently
from
0f48669 to
e34f2ec
Compare
XAMPPRocky
force-pushed
the
decouple-glam
branch
from
e34f2ec to
0baa4d7
Compare
XAMPPRocky
force-pushed
the
decouple-glam
branch
from
0baa4d7 to
5327419
Compare
khyperia
previously requested changes
Mar 11, 2021
XAMPPRocky
force-pushed
the
decouple-glam
branch
9 times, most recently
from
a3be4c6 to
f59221c
Compare
|
This should be ready for review. The main change is that I've added a new proc-macro called |
eddyb
reviewed
Mar 12, 2021
eddyb
reviewed
Mar 12, 2021
eddyb
reviewed
Mar 12, 2021
XAMPPRocky
force-pushed
the
decouple-glam
branch
from
f59221c to
d2e9d36
Compare
|
@khyperia This should be ready for a final review. |
khyperia
reviewed
Mar 16, 2021
XAMPPRocky
force-pushed
the
decouple-glam
branch
from
d2e9d36 to
1d0ca42
Compare
|
I need one of you to give me the green checkmark to merge. 😄 |
khyperia
approved these changes
Mar 16, 2021
| @@ -463,6 +463,9 @@ pub fn main() { | |||
| let vector = glam::Vec2::new(1.0, 2.0); | |||
| let element = unsafe { spirv_std::arch::vector_extract_dynamic(vector, 1) }; | |||
| assert!(2.0 == element); | |||
| let uvector = glam::UVec2::new(1, 2); | |||
| let element: u32 = unsafe { spirv_std::arch::vector_extract_dynamic(uvector, 1) }; | |||
There was a problem hiding this comment.
sorry, meant to include this in my comment too but didn't want to spam too many comments
Suggested change
| let element: u32 = unsafe { spirv_std::arch::vector_extract_dynamic(uvector, 1) }; | |
| let element = unsafe { spirv_std::arch::vector_extract_dynamic(uvector, 1) }; |
There was a problem hiding this comment.
Spam me with review comments 😄
| @@ -568,7 +575,7 @@ fn image_read() { | |||
| val(r#" | |||
| #[spirv(fragment)] | |||
| pub fn main(image: UniformConstant<StorageImage2d>, mut output: Output<glam::Vec2>) { | |||
| let coords = image.read(glam::IVec2::new(0, 1)); | |||
| let coords = image.read(glam::IVec2::new(0, 1)); | |||
There was a problem hiding this comment.
nit: formatting
Suggested change
| let coords = image.read(glam::IVec2::new(0, 1)); | |
| let coords = image.read(glam::IVec2::new(0, 1)); |
|
oh. silly me, didn't see the automerge enable, that's why I didn't approve in my previous comment :P |
|
I'll make a followup PR for those |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes
glamas a dependency fromspirv-std. Since we still use it for our tests, we currently have to patch it with a forked version ofglamthat contains the trait implementations we need untilglamcan depend on these changes directly. It also exportsderivative::derive_implas a hidden export so thatglamcan use it in its implementation.This PR also introduces a new
const-genericsfeature flag, and reintroduces some of the const generics in order to be able enforce the restrictions on these types through theVectortrait. Having these under a feature flag, will allow us to iterate and add condt generic code without affecting Ark until they are released on stable at the end of the month, and then we can remove it for primitives.glam branch diff