GS:HW: Unify TFX shaders among backends. - #14850
Conversation
c067222 to
8bea2fa
Compare
There was a problem hiding this comment.
Had a look though only the PS stuff
I was comparing against DX and VK shaders and seeing what differed code wise
I did notice that some of our constants vary across backends
I.e. 255.5f vs 255.0f, value255.5f instead of (value255f)+0.5f. etc
Perhaps we should consider what should be used instead of just using what Metal does, given that Metal is the least used backend and thus not as battle tested.
lightningterror
left a comment
There was a problem hiding this comment.
Currently gl is broken:
[ 38.2015] 0(489) : error C0000: syntax error, unexpected identifier, expecting reserved word or reserved word "in" or reserved word "out" or reserved word "uniform" at token "buffer"
[ 38.2018] 0(501) : error C0000: syntax error, unexpected identifier, expecting reserved word or reserved word "in" or reserved word "out" or reserved word "uniform" at token "buffer"
[ 38.2021] 0(511) : error C1503: undefined variable "index_buffer"
[ 38.2026] 0(516) : error C1503: undefined variable "vertex_buffer"
[ 38.2029] 0(548) : error C1503: undefined variable "index_buffer"
[ 38.2033] 0(553) : error C1503: undefined variable "vertex_buffer"
I also did a pass over the PS code and found a couple few other places that had some differences:
We should probably use a consistent constants throughout the shader, though for now it may be easier to match DX/VK to make checking the dump runs easier. |
7037169 to
d7e4176
Compare
Might be driver specific as I'm unable to repro on my end. The lines causing the issue appear to be the ones with but just to be sure could I have the full shader that is triggering the error? |
|
I had disable vertex shader expand toggled, that should break it. |
b31ddb0 to
daa3edb
Compare
Thanks, fixed it by only allowing the vertex/index SSBOs to be declared in shader if vertex expand shader is used. For the record: If VS expand shader is disabled we use GLSL 330 core on OpenGL, whereas SSBO needs 430 core (as noted by @JordanTheToaster) or ARB_shader_storage_buffer_object. |
5ebb352 to
5a87865
Compare
|
Last push separates pixel shader functions into different files as in #14583. Side note: we might want to look into using the constant buffer for alpha correction (selectors |
a78cd24 to
e13c50a
Compare
lightningterror
left a comment
There was a problem hiding this comment.
A couple of shader warnings on dx11/12:
bios
(1688,4-7): warning X3550: array reference cannot be used as an l-value; not natively addressable, forcing loop to unroll
(2385,3-25): warning X3206: implicit truncation of vector type
This doesn't output shader warning files, so this is somewhat a guess but; As for the truncation warning, no clue where that is from. |
e13c50a to
557d107
Compare
|
For the forcing loop to unroll warning, I added an UNROLL macro that expands to [unroll] on DX. The vector truncation warning was because the float4 output of the generic shader was being truncated to float for primid setup. To fix this, I just made the primid setup output float4, since the fixed function will truncate it anyway, which allowed removing a bit of messy macro logic the DX shader. Dump runs are clear in DX11/12 and VK after changing the color rounding logic to match the current DX shaders. OGL dump runs still needs to be done and Metal still needs to be tested. Edit: For standardizing the color rounding logic, that may be best to wait for a future PR as it's easier to match current master DX/VK for testing purposed in this one. |
557d107 to
69334a3
Compare
|
Dump run in OGL completed, nothing looks broken but about ~200 dumps are different. Metal was tested by @kamfretoz. |
|
Breaks SW blending with OpenGL FB fetch on Intel. |
69334a3 to
340026e
Compare
Status: draft until tested/reviewed.
Description of Changes
Unifies backend TFX (i.e. GS emulation) shaders:
tfx_defs.inc,tfx_vs.inc,tfx_ps.inc.tfx.fx(DX11/12),tfx.glsl(GL/VK),tfx.metal(Metal).The generic code was based on
tfx.metal, as it uses dynamic branch for selectors, which can be used later for spec constants in VK and uber shaders in all backends.Metal likely doesn't compile yet and needs some work.
Outline for this approach was partly due to @TellowKrinkle.
Details
ps_main()into a new helperps_shuffle().PS_CHANNEL_FETCHtoPS_CHANNELandPS_ANISOTRIPIC_FILTERINGtoPS_SW_ANSIOon non-Metal backends.VS_PROVOING_VERTEX_LASTmacro in VK (was unused).GS_) since we no longer use geometry shaders.max_depthconstants in VS and PS tomax_depth_vsandmax_depth_ps, since GL has issues with constants with the same name in different constant buffers.bin/resources/shaders/dx11tobin/resources/shaders/dx, since it's used for both DX11/12.bin/resources/shaders/vulkan_openglfolder since the language specific TFX shader code is now shared between VK/GL.Todo:
Rationale behind Changes
Reduce the amount of duplicated code in TFX shaders and make it easier to make changes.
Possible drawbacks:
Suggested Testing Steps
Use any HW renderer and make sure it works identically to master.
We should make sure there aren't any performance impacts on GPU or shader compilation.
So far, I've checked a couple optimized DX12/VK vertex/pixels shaders in renderdoc and they seem to be around the same size as master.
Did you use AI to help find, test, or implement this issue or feature?
Likely to ask general programming questions about C++, shading languages, etc.