Cannot use ShaderCI.FilePath to load byte code

[hzqst]

I'm trying to fill ShaderCI.FilePath with bytecode path (e.g. shader/cache/DrawWorldSurface.spirv / shader/cache/DrawWorldSurface.dxbc) from a manifest file to take advantage of RENDER_STATE_CACHE_FILE_HASH_MODE_BY_NAME,

however ShaderVkImpl::Initialize , CompileD3DBytecode and GetD3D12ShaderModel always treats ShaderCI.FilePath as text source input:

void ShaderVkImpl::Initialize(const ShaderCreateInfo& ShaderCI,
                              const CreateInfo&       VkShaderCI)
{
    if (ShaderCI.Source != nullptr || ShaderCI.FilePath != nullptr)
    {
        DEV_CHECK_ERR(ShaderCI.ByteCode == nullptr, "'ByteCode' must be null when shader is created from source code or a file");

        SHADER_COMPILER ShaderCompiler = ShaderCI.ShaderCompiler;
        if (ShaderCompiler == SHADER_COMPILER_DXC)
        {
            IDXCompiler* pDXCompiler = VkShaderCI.pDXCompiler;
            if (pDXCompiler == nullptr || !pDXCompiler->IsLoaded())
            {
                LOG_WARNING_MESSAGE("DX Compiler is not loaded. Using default shader compiler");
                ShaderCompiler = SHADER_COMPILER_DEFAULT;
            }
        }

        switch (ShaderCompiler)
        {
            case SHADER_COMPILER_DXC:
                m_SPIRV = CompileShaderDXC(ShaderCI, VkShaderCI);
                break;

            case SHADER_COMPILER_DEFAULT:
            case SHADER_COMPILER_GLSLANG:
                m_SPIRV = CompileShaderGLSLang(ShaderCI, VkShaderCI);
                break;

            default:
                LOG_ERROR_AND_THROW("Unsupported shader compiler");
        }

        if (m_SPIRV.empty())
        {
            LOG_ERROR_AND_THROW("Failed to compile shader '", m_Desc.Name, '\'');
        }
    }
    else if (ShaderCI.ByteCode != nullptr)
    {
        DEV_CHECK_ERR(ShaderCI.ByteCodeSize != 0, "ByteCodeSize must not be 0");
        DEV_CHECK_ERR(ShaderCI.ByteCodeSize % 4 == 0, "Byte code size (", ShaderCI.ByteCodeSize, ") is not multiple of 4");
        m_SPIRV.resize(ShaderCI.ByteCodeSize / 4);
        memcpy(m_SPIRV.data(), ShaderCI.ByteCode, ShaderCI.ByteCodeSize);
    }
    else
    {
        LOG_ERROR_AND_THROW("Shader source must be provided through one of the 'Source', 'FilePath' or 'ByteCode' members");
    }

• Possible fix A:

Add a new flag SHADER_COMPILE_FLAG_BYTECODE to always treats the input ShaderCI.FilePath as bytecode file and skip the shader compiling. (also leave ShaderCI.ByteCode for backward compatibility)

• Possible fix B:

check if ShaderCI.FilePath ends with ".dxbc", ".dxil", ".spv", ".spirv" and skip the shader compiling for them

[TheMostDiligent]

What is the issue here? The RENDER_STATE_CACHE_FILE_HASH_MODE_BY_NAME flag ensures that nothing is loaded from the file system when computing the ShaderCI hash. This holds true when the byte code is provided through the ShaderCI.ByteCode . Hashing byte blob is fast, unlike parsing source files to find includes and load them recursively.

Reading byte code from file is only a couple lines of code:

std::vector<Uint8> FileData;
FileWrapper::ReadWholeFile("path/to/file", FileData);

There is no need to have this functionality in the engine.

[hzqst]

What is the issue here? The RENDER_STATE_CACHE_FILE_HASH_MODE_BY_NAME flag ensures that nothing is loaded from the file system when computing the ShaderCI hash. This holds true when the byte code is provided through the ShaderCI.ByteCode . Hashing byte blob is fast, unlike parsing source files to find includes and load them recursively.

Reading byte code from file is only a couple lines of code:

std::vector FileData;
FileWrapper::ReadWholeFile("path/to/file", FileData);
There is no need to have this functionality in the engine.

1. I have to add extra code to support bytecode-based shader for D3D/VK, which breaks the behavior consistency between D3D/VK and other backends:

//procedure
if (GetRenderDevice() == D3D11 || GetRenderDevice() == D3D12 || GetRenderDevice() == VULKAN){
  ByteCode = FileWrapper::Read("...");
  ShaderCI.ByteCode = ByteCode->GetConstDataPtr();
  ShaderCI.ByteCodeSize = ByteCode->GetSize();
}

if (GetRenderDevice() == D3D11 || GetRenderDevice() == D3D12 || GetRenderDevice() == VULKAN){
  RenderStateCacheCI.flags &= ~RENDER_STATE_CACHE_FILE_HASH_MODE_BY_NAME;//not supported with bytecode-based shader yet
}

2. ByteCode = FileWrapper::Read(ByteCodeFilePath) still introduce redundant file IOs when RenderStateCache with RENDER_STATE_CACHE_FILE_HASH_MODE_BY_NAME actually don't need them at all

[TheMostDiligent]

OK, I see.
A possible solution here is to allow both FileName and ByteCode members to be non-null.
In this case, the FileName can be used as key when loading, and ByteCode can be null.
The file name does not need to be a real name if ByteCode or Source are not null

[hzqst]

OK, I see. A possible solution here is to allow both FileName and ByteCode members to be non-null. In this case, the FileName can be used as key when loading, and ByteCode can be null. The file name does not need to be a real name if ByteCode or Source are not null

we will still have to read bytecode from file first, which is what I want to avoid

[TheMostDiligent]

Second time ByteCode can be null

[hzqst]

Second time ByteCode can be null

What if the cached shader was not found in RSC due to something like accident crash before saving RSC.bin? iirc RenderDevice->CreateShader will be used as fallback, and it will not be working with non-real (and also bytecode-based) ShaderCI.FileName.

my workflow:

1. read shaderName,shaderFormat,shaderEntry,shaderByteCodePath from manifest json -> feed RSC with what we got from manifest json -> let RSC do the remaining stuffs -> exit game -> save RSC to RSC.bin.

2. if (1) failed: slang -> bytecode(dxbc/spirv) -> saved as *.dxbc/*.spirv -> create a manifest json with shaderName,shaderFormat,shaderEntry,shaderByteCodePath -> feed RSC with what we got from manifest json -> let RSC do the remaining stuffs -> exit game -> save RSC to RSC.bin.

[TheMostDiligent]

Yes, these are valid concerns. Need to think what is the best way to resolve this.

[hzqst]

Yes, these are valid concerns. Need to think what is the best way to resolve this.

SHADER_COMPILE_FLAG_LOAD_FILE_AS_BYTECODE do resolve my problem with minimal impact to existing code base.

[hzqst]

confirm solved!