bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
14.66k stars 1.92k forks source link

Incorrect DxbcOpcodeInfo entries? #2709

Open aleiby opened 2 years ago

aleiby commented 2 years ago

Testing bgfx's dxbc parsing I ran into an issue which appears to be due to incorrect DxbcOpcodeInfo entries. How were these generated?

Specifically, DCL_INPUT_SIV which has numOperands=1, but numValues=0.

Digging into d3d11TokenizedProgramFormat.hpp:

// ----------------------------------------------------------------------------
// Input Register Declaration w/System Interpreted Value
// (see separate declarations for Pixel Shaders)
//
// OpcodeToken0:
//
// [10:00] D3D10_SB_OPCODE_DCL_INPUT_SIV
// [23:11] Ignored, 0
// [30:24] Instruction length in DWORDs including the opcode token.
// [31]    0 normally. 1 if extended operand definition, meaning next DWORD
//         contains extended operand description.  This dcl is currently not
//         extended.
//
// OpcodeToken0 is followed by 2 operands:
// (1) Operand, starting with OperandToken0, defining which input
//     v# register (D3D10_SB_OPERAND_TYPE_INPUT) is being declared,
//     including writemask.  For Geometry Shaders, the input is 
//     v[vertex][attribute], and this declaration is only for which register 
//     on the attribute axis is being declared.  The vertex axis value must 
//     be equal to the # of vertices in the current input primitive for the GS
//     (i.e. 6 for triangle + adjacency).
// (2) a System Interpreted Value Name (NameToken)

It looks like numValues should be 1. This would also match the existing entry for DCL_INPUT_SGV.

I was getting instruction.length=5 but then read was only parsing 4 DWORDs.

Similarly, it looks like DCL_OUTPUT_SGV also needs numValues=1. The other DCL INPUT/OUTPUT entries look good.

I have not gone through all of these to verify correctness, so I was wondering if you originally just did this by hand and maybe these got missed, or if something else is going on.

bkaradzic commented 2 years ago

I have not gone through all of these to verify correctness, so I was wondering if you originally just did this by hand and maybe these got missed, or if something else is going on.

Yes, I did this by hand.

Table is here: https://github.com/bkaradzic/bgfx/blob/a2ad0667397d82b921907870de45ca47d3e2526d/src/shader_dxbc.cpp#L116

If you have test, you could fix this and send PR.

zeroxer commented 2 years ago

I meet a problem with direct11 which may related with the problem @aleiby have found.

Platform

vec3 a_position : POSITION; vec4 a_color0 : COLOR0;


- v_simple.sc

$input a_position, a_color0 $output v_color0

include

void main() { gl_Position = mul(u_modelViewProj, vec4(a_position, 1.0) ); v_color0 = a_color0; }

- f_simple.sc

$input v_color0

include

void main() { gl_FragColor = v_color0; }

- LoadShader
```cpp
bgfx::ShaderHandle loadShader(const char* _name) {
    char* data = new char[2048];
    std::ifstream file;
    size_t fileSize = 0;
    if (!std::filesystem::exists(_name)) {
        std::filesystem::path cwd = std::filesystem::current_path();
        std::cout << "Current dir is: " << cwd << ". Cannot find file: " << _name << std::endl;
        return bgfx::ShaderHandle{ bgfx::kInvalidHandle };
    }
    file.open(_name);
    if(file.is_open()) {
        file.seekg(0, std::ios::end);
        fileSize = file.tellg();
        file.seekg(0, std::ios::beg);
        file.read(data, fileSize);
        file.close();
    }
    const bgfx::Memory* mem = bgfx::copy(data, fileSize+1);
    mem->data[mem->size - 1] = '\0';
    bgfx::ShaderHandle handle = bgfx::createShader(mem);
    bgfx::setName(handle, _name);
    return handle;
}

Direct11

@REM DirectX
..\..\.Build\bgfx.cmake\Debug\shaderc.exe -f v_simple.sc -o v_simple.bin --stdout --varyingdef varying.def.sc --platform windows -p vs_5_0 --type vertex --verbose -i ..\..\3rdparty\bgfx.cmake\bgfx\src
..\..\.Build\bgfx.cmake\Debug\shaderc.exe -f f_simple.sc -o f_simple.bin --stdout --varyingdef varying.def.sc --platform windows -p ps_5_0 --type fragment --verbose -i ..\..\3rdparty\bgfx.cmake\bgfx\src

As the debugger shows: instruction.length = 8, but read size is 28. read size/4 =7, not equals to instruction.length. I have no idea about this error. Would you have some time to check this?

OpenGL

@REM OpenGL
..\..\.Build\bgfx.cmake\Debug\shaderc.exe -f v_simple.sc -o v_simple.bin --varyingdef varying.def.sc --platform windows --type vertex --verbose -i ..\..\3rdparty\bgfx.cmake\bgfx\src
..\..\.Build\bgfx.cmake\Debug\shaderc.exe -f f_simple.sc -o f_simple.bin --varyingdef varying.def.sc --platform windows --type fragment --verbose -i ..\..\3rdparty\bgfx.cmake\bgfx\src
pezcode commented 2 years ago

@zeroxer You should read shader .bin files as binary, they're not text files: file.open(_name, std::ifstream::in|std::ifstream::binary)

zeroxer commented 2 years ago

Thanks. It works now.