Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

WIP -- specialization constant support #1646

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

WIP -- specialization constant support #1646

wants to merge 10 commits into from

Conversation

chrisforbes
Copy link
Collaborator

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.

Copy link
Contributor

@tobine tobine left a 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?

@chrisforbes
Copy link
Collaborator Author

@tobine perhaps. It's easy to add one.

@chrisforbes
Copy link
Collaborator Author

Note there's still some issues here:

  • We're not using the optimized module yet.
  • Need the SPIRV-Tools team to fix their opt pass (currently doing string parsing that we don't want)
  • cmake needs some more work

@chrisforbes
Copy link
Collaborator Author

@tobine What do you think about landing 1-5 now, and the rest later?

@tobine
Copy link
Contributor

tobine commented Apr 12, 2017

@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.
@chrisforbes
Copy link
Collaborator Author

cmake for this is no good. I need to look at it a bit more.

@mark-lunarg
Copy link
Collaborator

Can anything be done with the work here, or should we just archive this for now?

@mark-lunarg
Copy link
Collaborator

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants