protonscr

Respect maxGeometryTotalOutputComponents limit

dxvkclosed bug
doitsujin/dxvk#1121 · opened 2019-07-11 by liam-middlebrook · updated 2019-07-24 · 8 comments · github
Lliam-middlebrook 2019-07-11 github

Currently DXVK runs into issues with geometry shaders on some titles on NVIDIA hardware where it creates SPIR-V modules which exceed the hardware defined limit of 1024 (on all current vulkan-supported NVIDIA hardware) for maxGeometryTotalOutputComponents. This issue is can arise from two potential factors:

  1. DXVK adds an additional Output variable gs_vertex_out which it associates with SV_POSITION and that becomes a pointer to the output variable that gets created for the DXBC dcl_output_siv for SV_POSITION.

  2. If an application passed a DXBC blob to DXVK which violated this limit, DXVK should be able to provide a more accessible error message to the developer.

Let's dive a bit deeper on the first factor. This additional Output variable is currently generated by DXVK to cover cases where applications don't have dcl_input_siv directives in a fragment shader which match with the dcl_output_siv directives in a geometry shader [1]. After a discussion that @jeffbolznv and I had with a member of our D3D team we learned that applications may additionally pass in this system value information through an ISGN entry.

One of the geometry shaders in Justice (the Chinese MMO) over-extends the limit when translated through DXVK. This pipeline this shader is used in raises the same concern as above, where the geometry shader's dcl_output_siv for SV_POSITION not having a matching dcl_input_sivin the fragment shader for the pipeline. I've written out some patches (which are available in PR #1120 ) which allow for debug-logging of the ISGN, OSGN, PSGN entries for a shader.

Here's the OSGN and ISGN for the geometry and fragment shaders I was looking at in Justice. Note the systemValue, for registerId=3.


debug: Output Signature for - GS_f65d0f8030f8da306b336b69230ad1999c5442f1

debug: SGN Entry:
        semanticName: TEXCOORD
        semanticIndex: 0
        registerId: 0
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::None
        streamId: 0


debug: SGN Entry:
        semanticName: TEXCOORD
        semanticIndex: 1
        registerId: 1
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::None
        streamId: 0


debug: SGN Entry:
        semanticName: TEXCOORD
        semanticIndex: 2
        registerId: 2
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::None
        streamId: 0


debug: SGN Entry:
        semanticName: SV_Position
        semanticIndex: 0
        registerId: 3
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::Position
        streamId: 0

debug: Input Signature for - FS_a9f8f8cfa79a54a3f28b6889af2e0098b7050b83

debug: SGN Entry:
        semanticName: TEXCOORD
        semanticIndex: 0
        registerId: 0
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::None
        streamId: 0


debug: SGN Entry:
        semanticName: TEXCOORD
        semanticIndex: 1
        registerId: 1
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::None
        streamId: 0


debug: SGN Entry:
        semanticName: TEXCOORD
        semanticIndex: 2
        registerId: 2
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::None
        streamId: 0


debug: SGN Entry:
        semanticName: SV_Position
        semanticIndex: 0
        registerId: 3
        componentMask: xyzw
        componentType: DxbcScalarType::Float32
        systemValue: DxbcSystemValue::Position
        streamId: 0


debug: SGN Entry:
        semanticName: SV_IsFrontFace
        semanticIndex: 0
        registerId: 4
        componentMask: x
        componentType: DxbcScalarType::Uint32
        systemValue: DxbcSystemValue::IsFrontFace
        streamId: 0

So I think the proper fix for factor one will be to check against the ISGN and OSGN for system values (in case a geometry shader doesn't dcl_output_siv even though the corresponding fragment shader uses dcl_input_siv), and then apply the spv::BuiltInPosition decoration (or other built-ins as appropriate).

Factor two will probably be much less common and practically can be covered by @jeffbolznv's change adding the maxGeometryTotalOutputComponents check to the validation layers.

Ddoitsujin maintainer 2019-07-11 github

The problem with this issue is that we don't know which fragment shader a geometry shader will be used with at the time we do the DXBC->SPIR-V translation. I'm concerned that performing this translation at draw time is going to make the stuttering issues that DXVK has due to pipeline compilation even worse than they already are, not to mention that doing so would require a major rework.

Another option might be to patch the generated SPIR-V at the time the pipeline gets compiled and change unused output locations to private variables, but that's also rather tricky.

Lliam-middlebrook 2019-07-12 github

We don't need to know which fragment shader a geometry shader will be used with at translation-time.

We know that a geometry shader should have at least one dcl_output or dcl_output_siv. When DXVK parses these declarations in DxbcCompiler::emitDclOutput() and encounters the SV_POSITION output declaration (be it through dcl_output_siv or matching the registerId from the OSGN) it should emit a variable that is decorated with spv::BuiltInPosition.

I spent some time prototyping this out today. I'll try and get my patches for a fix cleaned up to send over to you in the next few days / week.

Ddoitsujin maintainer 2019-07-12 github

The question is, what is supposed to happen if a game uses the geometry shader with a fragment shader which doesn't have the declaration? As far as I know, this is technically undefined behaviour in D3D11, but in practice, many games do rely on the exact ways things like this are handled in Windows drivers.

Ddoitsujin maintainer 2019-07-12 github

838372d0f9eae520f26a45591e34db9baa0269be (currently in a side branch) should be sufficient to fix this issue, but I'll have to test if this may cause any regressions.

Lliam-middlebrook 2019-07-12 github

I don't think that covers the case where a the SHDR section doesn't specify any _siv variants, but the paired shader expects a system value to be associated with an output. I have a patch #1123 that should resolve that.

Ddoitsujin maintainer 2019-07-12 github

Is that even possible? It would be extremely weird if the HLSL compiler wrote that info into the signatures but not the instruction stream, and DXVK doesn't take that into account when mapping the o# registers to SPIR-V built-ins right now anyway so something should be broken if there are shaders like this in the wild.

Ddoitsujin maintainer 2019-07-13 github

For some reason, this breaks terrain rendering in Assassin's Creed Odyssey, even with your additional patch. Not sure what's going on there just yet.

Ddoitsujin maintainer 2019-07-13 github

Turns out that this is actually the problem you described, but DXVK does not load system values in TCS/GS stages.

I don't think this can be fixed for those stages. Vulkan wants us to put all built-ins that are passed between stages in a block where the VS output block must be identical to the TCS input block, but we don't necessarily know which ones we need since the TCS ISGN may only be a subset of the VS OSGN. Limiting this to geometry shaders only should work though.

Upstream links