Better legacy GLSL compatibility

[tklajnscek]

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;

[WJsjtu]

I've met the same problem. And this works for me when I try to use it for es 300 in WebGL2 since Separate Shader Objects feature is not supported until 310.

[gongminmin]

Thanks. I'll add your change. It looks like appending descriptor binding info into uniform names is a temporally workaround for extracting information. It can be replaced by a future reflection APIs.

[gongminmin]

I have to remove the appending descriptor binding info into uniform names. Because the spv::Decoration::DecorationBinding is different on macOS. The test can't pass with it.

[gongminmin]

A regression happens after update the SPIRV-Cross. The "in_var_xxx" is changed to "in.var.xxx". Need to adopt this change.