This pull request introduces a new option that enables inheriting of combined sampler bindings.

For an example, consider the original HLSL fragment:

Texture2D tex0 : register(t0);
Texture2D tex1 : register(t1);
Texture2D tex2 : register(t2);
Texture2D tex3 : register(t3);
SamplerState sampler0 : register(s0);
SamplerState sampler1 : register(s1);
SamplerState sampler2 : register(s2);
SamplerState sampler3 : register(s3);

And the output with the option disabled (default/old behaviour):

uniform sampler2D SPIRV_Cross_Combinedtex0sampler0;
uniform sampler2D SPIRV_Cross_Combinedtex2sampler2;
uniform sampler2D SPIRV_Cross_Combinedtex3sampler3;
uniform sampler2D SPIRV_Cross_Combinedtex1sampler1;

And compare with the output with the same option enabled:

layout(binding = 0) uniform sampler2D SPIRV_Cross_Combinedtex0sampler0;
layout(binding = 2) uniform sampler2D SPIRV_Cross_Combinedtex2sampler2;
layout(binding = 3) uniform sampler2D SPIRV_Cross_Combinedtex3sampler3;
layout(binding = 1) uniform sampler2D SPIRV_Cross_Combinedtex1sampler1;

Note that in this particular (real world) example, the bindings changed. This is because SPIRV-Cross generates the combined samplers in the order of post-optimization usage, and not order of original binding or declaration.

Without this option, it's impossible to write a generic system that will bind the GLSL textures to the same bindings as the original HLSL.

This patch merely invokes the util method already provided by SPIRV-Cross.

Hi Minmin Gong,

I added a new Disassemble option in TargetDesc to get disassemble Spirv for debug purposes. I hope that you find it useful to merge. Next step should be disassemble DXIL too.

Note. I linked spirv-tools for this feature.

Regards, Jorge

Is your feature request related to a problem? Please describe. My engine supports GLES2 / WebGL1 in addition to newer OpenGL versions and Vulkan/DX12/Metal. By default, the shaders generated don't play well with the legacy GLSL versions. Since it's still a major target, I suggest some improvements & conventions to make it work out of the box.

Describe the solution you'd like I suggest two improvements using SPRIVCross's shader reflection API:

• Make the vertex output names match the fragment input names (renaming out_var_X and in_var_X to varying_X)
• Append descriptor binding info into uniform names so it can be easily extracted when uniforms are enumerated after compilation/linking

Describe alternatives you've considered The minor changes I propose should cover most of the use cases, but there might be a way to do this more generically with callbacks etc, although I feel it would go against the super streamlined design of ShaderConductor and would leak implementation details into user code.

Additional context This is the code I'm using currently in ConvertBinary():

case ShadingLanguage::Glsl:
case ShadingLanguage::Essl:
    compiler = std::make_unique<spirv_cross::CompilerGLSL>(spirvIr, spirvSize);
    combinedImageSamplers = true;
    buildDummySampler = true;

    // Legacy GLSL fixups
    if (intVersion <= 300)
    {
        auto&& vars = compiler->get_active_interface_variables();
        for (auto& var : vars)
        {
            auto varClass = compiler->get_storage_class(var);
            
            // Make VS out and PS in variable names match
            if (source.stage == ShaderStage::VertexShader && varClass == spv::StorageClass::StorageClassOutput)
            {
                auto name = compiler->get_name(var);
                if (name.find("out_var") == 0)
                {
                    name.replace(0, 7, "varying");
                    compiler->set_name(var, name);
                }
            }
            else if (source.stage == ShaderStage::PixelShader && varClass == spv::StorageClass::StorageClassInput)
            {
                auto name = compiler->get_name(var);
                if (name.find("in_var") == 0)
                {
                    name.replace(0, 6, "varying");
                    compiler->set_name(var, name);
                }
            }
            
            // Encode binding info into variable name for uniform buffers, textures, samplers
            if (varClass == spv::StorageClass::StorageClassUniform || varClass == spv::StorageClass::StorageClassUniformConstant)
            {
                auto space = compiler->get_decoration(var, spv::Decoration::DecorationDescriptorSet);
				auto reg = compiler->get_decoration(var, spv::Decoration::DecorationBinding);
            
                char buf[128];
                sprintf(buf, "_reg_%d_space_%d", reg, space);
            
				auto type = compiler->get_type_from_variable(var);
 				auto typeName = compiler->get_name(type.self);
 				typeName.append(buf);
 				compiler->set_name(type.self, typeName);
            
                auto name = compiler->get_name(var);
                name.append(buf);
                compiler->set_name(var, name);
            }
        }
    }

    break;

Describe the bug

The Linux CI builds produce a libdxcompiler.so build that dynamically links to libtinfo.so.5. That's not something that is always installed on a user machine. I have libtinfo.so.6 installed as an ncurses dependency on Arch Linux). I managed to run the CI binaries after installing ncurses5-compat-libs from the AUR. That provides libtinfo.so.5, though when running I get the following message /usr/lib/libtinfo.so.5: no version information available.

Expected behavior

There is no libtinfo dependeny.

Additional context

libtinfo is an optional dependency of LLVM, though I don't see where it is enabled exactly. It looks like it's only compiled as a dependency if the LLVM_ENABLE_TERMINFO flag is set. A cmake file in DirectXShaderCompiler checks that flag and finds a lib providing terminfo (e.g. libtinfo) here. The flag is enabled by default here. But it also looks like terminfo is disabled here. Building locally on my machine I don't get this dependency, but I'm unsure why.

There are important files that Microsoft projects should all have that are not present in this repository. A pull request has been opened to add the missing file(s). When the pr is merged this issue will be closed automatically.

Microsoft teams can learn more about this effort and share feedback within the open source guidance available internally.

Merge this pull request

Hi @gongminmin ,

I added 4 new parameters in Options struct:

int shiftAllTexturesBindings: First binding number for Vulkan textures. int shiftAllSamplersBindings: First binding number for Vulkan samplers. int shiftAllCBuffersBindings: First binding number for Vulkan constant buffers. int shiftAllUABuffersBindings: First binding number for Vulkan unordered access buffers.

It is equivalent to use -fvk-{b|s|t|u}-shift in DXC command-line. That is a very important option when you compile HLSL to SPIRV HLSL register and Vulkan binding.

Finally, I inverted packMatricesInRowMajor option to fix the issue.

Resolve #37 Resolve #41

This is related to #28 but not required for that issue. Instead of invoking git from inside CMake project generation this change uses git submodules to accomplish the same thing.

Describe the bug CMake project generation requires git.exe to be in PATH in order to successfully run the UpdateExternalLib function defined in External/CMakeLists.txt. This requirement is not explicitly stated in README.md even though the instructions for CMake and Python both mention the PATH variable.

To Reproduce

1. Clone the repository
2. In a context where git is not in PATH run Build.py or cmake -G ... as instructed in README.md

Expected behavior Expected CMake project generation to complete successfully. Instead CMake silently fails when running the git commands in UpdateExternalLib and generates errors later when attempting to call add_subdirectory on the cloned external repositories and subsequent commands on targets in those repositories.

Additional context I hit this issue because I have git installed into the PATH of Git Bash for Windows only and tried to generate the project files in the developer command prompt because of an issue with CMake and Visual Studio 2019.

Not sure why git submodules aren't being used for this step.

Describe the bug SPIRV-Tools is compiled with the -WError flag. Building a clean clone of the repo with either clang or gcc fails because of the warning reported in KhronosGroup/SPIRV-Tools#1541. Not sure why CI builds don't run into this.

To Reproduce Run python BuildAll.py ninja gcc or python BuildAll.py ninja clang. Clang version: 7.0.0 (tags/RELEASE_700/final) Gcc version: 8.2.1 20181127 (GCC)

Expected behavior Cloning a build and running the python build script Just Works.

Additional context I don't think ShaderConductor needs to set -WError. This can be disabled by setting SPIRV_WERROR=OFF.

dxcompiler.dll is 17.7MB (7.7MB compressed), but libdxcompiler.so is 34.5MB (13.1MB compressed), and libdxcompiler.dylib is 27.1MB (10.0MB compressed). Need to have some understanding about why they are so different, and how to improve this.

I am using ShaderConductor library from C# so I created a wrapper project and C# pinvoke test project. I hope that you find it interesting to accept in your repo.

I tried getting the pre-compiled binaries listed on mainpage. But I'm getting a 503 error.

https://cdn.discordapp.com/attachments/745513646888714265/1009083625184182292/unknown.png

Note that this PR is on top of 69, 70, 71, and 72. Please review them first. This change needs to be rebased after those PRs go in. After this PR, we have both DXIL and SPIR-V reflections.

Refactor the test code for future extension.

Note that this PR is on top of https://github.com/microsoft/ShaderConductor/pull/69 and https://github.com/microsoft/ShaderConductor/pull/70. Please review them first.

Add VS2022 support.

Note that this PR is based on https://github.com/microsoft/ShaderConductor/pull/69 . Please review that one first. This change needs to be rebased.