Add genvulkan target definition #2439
Conversation
|
|
||
| <!-- Comma separated list of target" specifiers to indicate which | ||
| code generators to use. --> | ||
| <input name="targets" type="string" value="genglsl,genosl,genmdl,genessl,genmsl" /> |
There was a problem hiding this comment.
I don't think this will actually do anything since "genvulkan" is not a known target for any generator.
Pinging @ashwinbhat on how I think Vulkan works currently.
- There is a
VkShaderGeneratorwhich has a "genglsl" target. - It is not sufficient to generate proper code since you need to create a binding context for Vulkan. I'm not sure what happens if you don't do this ? This is shown in
Python/Scripts/generateshader.py.
I think to have this target recognized, the VkShaderGenerator needs to change it's TARGET to genvulkan, and I'd also suggest that the context requirements be automatically added within the generator, then users of this generator do not need to add it in (for Python, C++ and WASM/JS).
BTW, I'm not even show how the genessl option causes anything different to occur since it also does not seem to be a recogized target -- but perhaps getting Vulkan correct first can be used as a template to have genessl recognized ?
There was a problem hiding this comment.
For changing the VkShaderGenerator target, I believe this was originally done in parallel with PR#2399 with the intention of switching the targets in that PR. But I can do that here.
Thanks for the feedback - I'll need to take a look at Python/Scripts/generateshader.py to get a better understanding of how the binding context should be implemented.
This PR adds the genvulkan target definition, to be used as a unique identifier for the VkShaderGenerator.
I ran through the MaterialX unit tests and generated a
genglsl_glsl_vulkan_generate_testoutput from the executable. I did not see anything out of place - please let me know if I need to add something for complete test coverage.