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

Metal: vertex descriptor is built without using the location information #2324

Closed
zeux opened this issue Aug 15, 2018 · 2 comments
Closed

Metal: vertex descriptor is built without using the location information #2324

zeux opened this issue Aug 15, 2018 · 2 comments

Comments

@zeux
Copy link

zeux commented Aug 15, 2018

This is the code that creates Metal vertex descriptors:

let mtl_attribute_desc = vertex_descriptor
.attributes()
.object_at(i)
.expect("too many vertex attributes");

The expected behavior is that the location in the vertex shader matches the location in the attributes array; instead this code fills the array from 0 to count-1, which doesn't work for vertex shaders that have 3 input attributes with locations 0, 2, 4. The array index should instead use VkVertexInputAttributeDescription::location.

@kvark
Copy link
Member

kvark commented Aug 16, 2018

@zeux this is an unbelievably precise definition of a bug, almost carrying a fix in it self :). Thank you for chasing this down! Fix is on the way.

@zeux
Copy link
Author

zeux commented Aug 16, 2018

Thanks! FWIW based on cursory inspection CTS doesn’t seem to cover this, I filed KhronosGroup/VK-GL-CTS#121 for this as well.

bors bot added a commit that referenced this issue Aug 16, 2018
2325: [mtl] New vertex binding logic r=grovesNL a=kvark

Fixes #2324
Fixes #2307
Fixes #2058 (unrelated to the PR, but harmless)

PR checklist:
- [ ] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: metal
- [ ] `rustfmt` run on changed code


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
@bors bors bot closed this as completed in #2325 Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants