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

fix fragment/vertex shader descriptor set layout mismatch #8140

Merged

Conversation

pixelflinger
Copy link
Collaborator

The SSR variant uses a custom fragment shader but the generic vertex shader. For this reason their descriptor set layout for the "per view" set end-up being different (the fragment shader one has less descriptors in it and the descriptor itself is create from the SSR variant data). In the end, we end up with a descriptor set that's too small for the vertex shader and Metal was rightly complaining.

We fix this by:

  • removing the descriptors unused in the vertex shader in the generic case
  • adding the remaining descriptors in the SSR version of the layout

In practice this only adds one UBO (for shadows). That UBO is actually not even used in the SSR variants, but the (generic) vertex shader layout has it, so it must be provided. WE do this by providing a dummy UBO, which will never be used anyways.

In addition, we add a check to matc which will make sure such mismatch won't happen again.

The SSR variant uses a custom fragment shader but the generic vertex
shader. For this reason their descriptor set layout for the "per view"
set end-up being different (the fragment shader one has less
descriptors in it and the descriptor itself is create from the SSR
variant data).  In the end, we end up with a descriptor set that's too
small for the vertex shader and Metal was rightly complaining.

We fix this by:
- removing the descriptors unused in the vertex shader in the generic case
- adding the remaining descriptors in the SSR version of the layout

In practice this only adds one UBO (for shadows). That UBO is actually
not even used in the SSR variants, but the (generic) vertex shader 
layout has it, so it must be provided. WE do this by providing a
dummy UBO, which will never be used anyways.

In addition, we add a check to matc which will make sure such mismatch
won't happen again.
@pixelflinger pixelflinger added the internal Issue/PR does not affect clients label Sep 19, 2024
filament/src/ds/SsrPassDescriptorSet.cpp Outdated Show resolved Hide resolved
@pixelflinger pixelflinger merged commit 1ed4777 into ma/descriptor-sets-1.54.3 Sep 19, 2024
1 check passed
@pixelflinger pixelflinger deleted the ma/fix-descriptor-set-layout-mismatch branch September 19, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Issue/PR does not affect clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants