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

attr: fix #[spirv(flat)] checking to match the Vulkan spec. #933

Merged
merged 1 commit into from
Oct 27, 2022

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented Oct 24, 2022

Rust-GPU validation for #[spirv(flat)] was added in #815 but it had both:

  • false positives: requiring it based on type alone, even in places not even allowed by the Vulkan spec
  • false negatives: not requiring on builtin fragment inputs (because of an unrelated early-return for bools)

The Vulkan spec validation rules relevant here are:

  • VUID-StandaloneSpirv-Flat-04670

    The Flat, NoPerspective, Sample, and Centroid decorations must only be used on variables with the Output or Input storage class

  • VUID-StandaloneSpirv-Flat-04744

    Any variable with integer or double-precision floating-point type and with Input storage class in a fragment shader, must be decorated Flat

  • VUID-StandaloneSpirv-Flat-06201

    The Flat, NoPerspective, Sample, and Centroid decorations must not be used on variables with the Output storage class in a fragment shader

  • VUID-StandaloneSpirv-Flat-06202

    The Flat, NoPerspective, Sample, and Centroid decorations must not be used on variables with the Input storage class in a vertex shader

This was prompted by spirv-val improving their own checks (blocking updating SPIRV-Tools in #928):

@eddyb eddyb requested a review from oisyn as a code owner October 24, 2022 23:58
@eddyb eddyb enabled auto-merge (rebase) October 24, 2022 23:58
@eddyb eddyb mentioned this pull request Oct 25, 2022
1 task
@eddyb eddyb merged commit 65f892e into EmbarkStudios:main Oct 27, 2022
@eddyb eddyb deleted the flat-and-flatter branch October 27, 2022 09:02
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