Skip to content

GS:HW: Unify TFX shaders among backends. - #14850

Open
TJnotJT wants to merge 3 commits into
PCSX2:masterfrom
TJnotJT:gs-shader-unify
Open

GS:HW: Unify TFX shaders among backends.#14850
TJnotJT wants to merge 3 commits into
PCSX2:masterfrom
TJnotJT:gs-shader-unify

Conversation

@TJnotJT

@TJnotJT TJnotJT commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Status: draft until tested/reviewed.

Description of Changes

Unifies backend TFX (i.e. GS emulation) shaders:

  • Put generic TFX code in files tfx_defs.inc, tfx_vs.inc, tfx_ps.inc.
  • Put language specific code in files tfx.fx (DX11/12), tfx.glsl (GL/VK), tfx.metal (Metal).
  • The language specific files includes the generic files and contain macros that allow the generic code to compile correctly in HLSL/GLSL/MSL.
  • The includes in language specific files are manually inlined at runtime (other than Metal, which compiles at build time).

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

  • Refactored the shuffle handling into a function from ps_main() into a new helper ps_shuffle().
  • Changed macros PS_CHANNEL_FETCH to PS_CHANNEL and PS_ANISOTRIPIC_FILTERING to PS_SW_ANSIO on non-Metal backends.
  • Removed VS_PROVOING_VERTEX_LAST macro in VK (was unused).
  • Removed all geometry shader macros (i.e. those prefixed by GS_) since we no longer use geometry shaders.
  • Change max_depth constants in VS and PS to max_depth_vs and max_depth_ps, since GL has issues with constants with the same name in different constant buffers.
  • Change folder name bin/resources/shaders/dx11 to bin/resources/shaders/dx, since it's used for both DX11/12.
  • Add bin/resources/shaders/vulkan_opengl folder since the language specific TFX shader code is now shared between VK/GL.

Todo:

  • There are some indentation issues that need to be resolved.
  • DX has a several warnings about uninitialized variables that must be dealt with.
  • Future work: We could possibly unify other shaders with duplicated code.

Rationale behind Changes

Reduce the amount of duplicated code in TFX shaders and make it easier to make changes.

Possible drawbacks:

  • The shader code is less readable due to macro usage.
  • Debug shaders on VK, DX, GL now contains the dead code from inactive selectors. This seems to break renderdoc source level debugging on VK (although assembly debugging works).
  • Debugging compilation issues might be a bit more complicated since the shaders on disk won't have the same line numbers as in error messages. However, we dump the shader with includes inlined to disk on compile errors.
  • Dump runs on VK/DX12 (so far) show many differences with master, likely due to small numerical difference in the shader code. The differences should not be visually apparent, however.

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.

@TheLastRar TheLastRar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/dx/tfx.fx Outdated
Comment thread bin/resources/shaders/common/tfx_ps.inc Outdated
Comment thread bin/resources/shaders/dx/tfx.fx Outdated

@lightningterror lightningterror left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

@TJnotJT

TJnotJT commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

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, value_255.5f instead of (value_255f)+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.

I also did a pass over the PS code and found a couple few other places that had some differences:

  • fetch_raw_depth(): In non-Metal it interacts with PS_TEX_IS_FB. SincePS_TEX_IS_FB is only used when tex == rt (not ds) it should be fine to omit this.
  • ps_fbmask(): Metal uses 255.5f instead of * 255.0f + 0.1f to denormalize RGB, and uses 255.f for alpha denorm instead of 65525.f when PS_COLCLIP_HWis true.
  • ps_blend(): In Metal the current color is modified for all subsequent steps if there is a shuffle. In non-Metal the current color is only modified in the ps_blend() scope. In Metal/VK the shuffle code runs any time the RT is used, while in DX it is run only when the blend uses the RT.
  • Z clamp, AA1 discard, and complex alpha test discards are done in different orders in Metal (probably no impact).

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.

@TJnotJT

TJnotJT commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Currently gl is broken:

Might be driver specific as I'm unable to repro on my end. The lines causing the issue appear to be the ones with readonly buffer ... here:

// Vertex buffer for expand shaders (sprites, upscaled lines, AA1 edges, etc.)
#if PCSX2_VULKAN
layout(std140, set = 0, binding = 2)
#elif PCSX2_OPENGL
layout(std140, binding = 2)
#endif
readonly buffer VertexBuffer
{
	VSRawVertex vertex_buffer[];
};

// Index buffer for rearranging vertices in AA1 expand shader.
// Warning: use std430 instead of std140 so that the ints are tightly packed.
#if PCSX2_VULKAN
layout(std430, set = 0, binding = 3) 
#elif PCSX2_OPENGL
layout(std430, binding = 3)
#endif
readonly buffer IndexBuffer
{
	uint index_buffer[];
};

but just to be sure could I have the full shader that is triggering the error?

@lightningterror

Copy link
Copy Markdown
Contributor

I had disable vertex shader expand toggled, that should break it.

@TJnotJT
TJnotJT force-pushed the gs-shader-unify branch 3 times, most recently from b31ddb0 to daa3edb Compare August 18, 2026 20:35
@TJnotJT

TJnotJT commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

I had disable vertex shader expand toggled, that should break it.

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.

@TJnotJT

TJnotJT commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

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 PS_RTA_CORRECTION and PS_RTA_SRC_CORRECTION), since they only affects code through a couple constants and may not lead to much optimization.

@lightningterror lightningterror left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@TheLastRar

Copy link
Copy Markdown
Contributor

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

This doesn't output shader warning files, so this is somewhat a guess but;
In master, the for loop in sample_color has the [unroll] attribute, but that is absent in this PR (I'm also not sure what the equivalent attributes would be for other languages is).
Guess it's harmless here, since FXC forces the unroll anyway

As for the truncation warning, no clue where that is from.

@TJnotJT

TJnotJT commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

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.

@TJnotJT

TJnotJT commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Dump run in OGL completed, nothing looks broken but about ~200 dumps are different. Metal was tested by @kamfretoz.

@TJnotJT
TJnotJT marked this pull request as ready for review August 24, 2026 14:44
@TJnotJT

TJnotJT commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Breaks SW blending with OpenGL FB fetch on Intel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants