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

Vertex state correctness #4109

Merged
merged 2 commits into from
Dec 20, 2024
Merged

Conversation

greggman
Copy link
Contributor

Refactor these tests so they don't use storage buffers.

The tests were using one uniform or storage buffer per
vertex attribute. Switched to using just a single uniform
buffer for all attributes.

Note: There was a note about increasing the vertex count
by +1 in vertex_buffer_used_multiple_times_interleaved.
I don't know if that's old because removing the +1 there
and a few lines below does not break the test nor generate
any validation errors.

The reason I needed to remove it was the code that packs
the data into a single buffer needed the size of each buffer
to match kVertexCount but that particular test was passing
+1 in some places and not in others.

@greggman greggman requested a review from kainino0x December 19, 2024 05:57
Copy link
Collaborator

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

lgtm, that was a pleasantly simpler review than I thought given how complex I remember this test being :)

Refactor these tests so they don't use storage buffers.

The tests were using one uniform or storage buffer per
vertex attribute. Switched to using just a single uniform
buffer.

Note: There was a note about increasign the vertex count
by +1 in vertex_buffer_used_multiple_times_interleaved.
I don't know if that's old because removing the +1 there
are a few lines below does not break the test nor generate
any validation errors.

The reason I needed to remove it was the code that packs
the data into a single buffer needed the size of each buffer
to match kVertexCount but that particular test was passing
+1 in some places and not in others.
This test might break on devices with higher limits
but in that case the test should be fixed.
@greggman greggman force-pushed the vertex-state-correctness branch from eceff4e to ed883c4 Compare December 20, 2024 09:50
@greggman greggman enabled auto-merge (squash) December 20, 2024 09:50
@greggman greggman merged commit 6fd9914 into gpuweb:main Dec 20, 2024
1 check passed
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