Improved estimation of vertex attribute buffer count when reserving for implicit buffer #2425
+18
−5
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
…ding To be consistent with initReservedVertexAttributeBufferCount. This should not cause any visible effect.
…ing implicit buffer In vertex shader, vertex attribute buffer and implicit buffer occupy the same buffer binding space, but they are bound using different methods. When an implicit buffer is used, it should mark the vertex (attribute) buffer at the same buffer index as dirty so that it will be correctly bound when activated.
…ving The current vertex attribute buffer reservation is too generous. According to addVertexInputToPipeline method, we don't need synthetic binding buffer if the vertex attribute offset is 0.
cdavis5e
reviewed
Jan 21, 2025
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.
Currently, the vertex attribute buffer count estimation (to reserve binding for implicit buffers) is too generous. This merge request improves the estimation by introducing one case from the actual allocation logic,
addVertexInputToPipeline, i.e., synthetic binding buffer if the vertex attribute offset is 0. This helps in reducing the required binding slots the majority of pipelines generated by ANGLE and allows vertex shaders that uses more than 10 attribute inputs to work.In the meantime, the change brings bindings of implicit buffers and vertex attributes into close vicinity, and exposes a bug where implicit buffer interferes with vertex attribute at the same binding index across draw calls in the same render pass. This is masked by the previous over-conservative estimation. The second change fix the bug by adding dirty marks.
The only other (first) change is trivial style fix, and does not affect the visible behavior of the program.