-
Notifications
You must be signed in to change notification settings - Fork 172
WIP -- specialization constant support #1646
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything here LGTM. Do we want/need a positive test similar to CreatePipelineVsFsArraySizeSpecialization that uses specialization constant to adjust array size to correct size?
@tobine perhaps. It's easy to add one. |
Note there's still some issues here:
|
@tobine What do you think about landing 1-5 now, and the rest later? |
@chrisforbes Yes, landing 1-5 asap makes sense. Spin off into separate PR and run through our farm to be safe. Assuming clean it has my +1. |
Currently we don't even look at these, and instead assume `1`, which is completely bogus.
These shaders are fine without the specialization -- but with it applied, we should get a type mismatch on the array sizes.
The SC code was previously reaching directly into the layer_data, which we'd rather not do. Also removes some iterator clutter from SC.
This was done long ago.
We're about to actually apply specializations, so let's do this work /before/ broken specializations will cause us to crash.
cmake for this is no good. I need to look at it a bit more. |
Can anything be done with the work here, or should we just archive this for now? |
@chrisforbes, this repository will close for write access on Sunday, 5/13/2018. If it is pushed before that time it will be present in the follow-on Vulkan-ValidationLayers repository on Monday, otherwise a new PR will be required in the new repo. |
Patches 1-5 do some housekeeping in the test framework in order to make writing this test easier.
Patches 6-7 are adding new test cases (which currently fail).
Still to come: actual implementation to make the tests pass.