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

Add partial support for khr_fragment_shading_rate #2574

Merged
merged 9 commits into from
Oct 19, 2024

Conversation

LDeakin
Copy link
Contributor

@LDeakin LDeakin commented Oct 14, 2024

This adds partial support for khr_fragment_shading_rate.

  • FragmentShadingRateState for GraphicsPipeline
  • DynamicState::FragmentShadingRate and set_fragment_shading_rate
  • Per-region support VkFragmentShadingRateAttachmentInfoKHR

Changelog:

### Additions
- Partial support for the `khr_fragment_shading_rate` extension.

@marc0246
Copy link
Contributor

I think it looks wonderful!

@marc0246
Copy link
Contributor

We should definitely add this to the task graph's command buffer (doesn't have to be in this PR), as any contribution to the old command buffer is instant legacy code. The old command buffer will be deprecated in the release after the next and eventually removed entirely together with GpuFuture.

@LDeakin LDeakin marked this pull request as ready for review October 18, 2024 11:02
@LDeakin
Copy link
Contributor Author

LDeakin commented Oct 18, 2024

This supports all but per-region fragment shading rate, which is a bit more involved. Perhaps a future PR?

@LDeakin LDeakin changed the title Add support for khr_fragment_shading_rate Add partial support for khr_fragment_shading_rate Oct 18, 2024
@Rua
Copy link
Contributor

Rua commented Oct 18, 2024

Looks good! Could you add a short documentation to the top of the fragment_shading_rate module, describing what it does?

@LDeakin
Copy link
Contributor Author

LDeakin commented Oct 18, 2024

In writing the docs, I realised I don't validate that primitive_fragment_shading_rate is enabled for per-primitive shading. But, I could not find an associated VUID for that.

Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com>
@marc0246
Copy link
Contributor

marc0246 commented Oct 19, 2024

Could you please format the validation messages like they are elsewhere? That is, use backticks around arguments/code and use line continuation to ensure they don't end up longer than 100 columns.

Copy link
Contributor

@marc0246 marc0246 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It LGTM; I just have some nits remaining.

Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com>
@marc0246
Copy link
Contributor

marc0246 commented Oct 19, 2024

Unfortunately, I don't have an answer for your earlier question. It looks as though there is no VUID specifically for primitive_fragment_shading_rate. There is the SPIR-V capability FragmentShadingRateKHR but it applies to all 3 device features; there is not one capability per device feature. I really wish there was, because this means that we do in fact have no way of validating them individually and the user has to know what they're doing :/

@marc0246 marc0246 merged commit 6981040 into vulkano-rs:master Oct 19, 2024
6 checks passed
@LDeakin LDeakin deleted the fragment_shading_rate branch October 19, 2024 05:35
@LDeakin
Copy link
Contributor Author

LDeakin commented Oct 19, 2024

Thanks for reviewing and getting this in so quick @marc0246 and @Rua!

marc0246 added a commit that referenced this pull request Oct 19, 2024
marc0246 added a commit that referenced this pull request Oct 19, 2024
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