Skip to content

GS: Renderer Code Cleanup#14195

Open
TellowKrinkle wants to merge 6 commits intoPCSX2:masterfrom
TellowKrinkle:DepthFeedbackCleanup
Open

GS: Renderer Code Cleanup#14195
TellowKrinkle wants to merge 6 commits intoPCSX2:masterfrom
TellowKrinkle:DepthFeedbackCleanup

Conversation

@TellowKrinkle
Copy link
Copy Markdown
Member

Description of Changes

A bunch of semi-unrelated changes I made while working on the Metal implementation of depth feedback

  • Move some enums to a header that's shared between Metal and C++. Maybe we can find a way to share it with the other shading languages too, but for now at least that's one less place to desync.
  • Switched draw config dump to fmt from iostream and moved its definitions out of headers (not a huge fan of filling headers with private methods where not necessary, though I realize C++ seems to encourage this for some reason).
  • Cleaned trailing whitespace from some glsl and hlsl shaders
  • Removed some redundant ps selector bits
    • Not only were they redundant but the shaders were coded to make it seem like all sorts of absurd combinations were possible. What does it mean to do AFAIL_ZB_ONLY without COLOR_FEEDBACK? Might that ever happen? With the COLOR_FEEDBACK flag removed, the answer is now obvious: no, it would never have happened. The only flag that was ever actually used in multiple configurations was RGB_ONLY, so that one has been split into RGB_ONLY and RGB_ONLY_SW_Z. ZB_ONLY now implies COLOR_FEEDBACK, and FB_ONLY implies DEPTH_FEEDBACK.
  • Fixed a bug in the Metal shaders where we forgot to update when we switched the dual-source blend AFAIL handling to use RGB_ONLY_DSB instead of RGB_ONLY

Rationale behind Changes

Easier to work with

Suggested Testing Steps

Make sure I didn't break depth feedback on non-Metal

Did you use AI to help find, test, or implement this issue or feature?

No

@TJnotJT
Copy link
Copy Markdown
Contributor

TJnotJT commented Mar 24, 2026

The changes look good. The PS_*_FEEDBACK flags were redundant, so removing them makes sense. AA1 uses the depth feedback flag, but adapting it to these changes should be fairly straightforward.

@JordanTheToaster
Copy link
Copy Markdown
Member

Breaks accurate alpha test in D3D12.

@Mrlinkwii Mrlinkwii added this to the Release 2.8 milestone Mar 26, 2026
@TellowKrinkle TellowKrinkle force-pushed the DepthFeedbackCleanup branch 5 times, most recently from a4e75de to ce130bf Compare March 26, 2026 17:36
Copy link
Copy Markdown
Contributor

@TJnotJT TJnotJT left a comment

Choose a reason for hiding this comment

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

Maybe we can find a way to share it with the other shading languages too, but for now at least that's one less place to desync.

At some point, we could add a utility function in GSShaderEnums.h to generate the #defines, which could work for both HLSL and GLSL.

Switched draw config dump to fmt from iostream and moved its definitions out of headers.

Minor nit: the string representations are now parenthesized instead of following #, so they wouldn’t parse cleanly as YAML. Probably not important unless something relies on that.

@TellowKrinkle
Copy link
Copy Markdown
Member Author

TellowKrinkle commented Mar 29, 2026

Minor nit: the string representations are now parenthesized instead of following #, so they wouldn’t parse cleanly as YAML. Probably not important unless something relies on that.

Personally I think these should prioritize being human readable, and humans generally consider the enum name more important than the number. If you're worried about machine readability, I also changed the blend prints to print them as actual equations instead of a pile of enum cases.

If someone wants a machine readable version, we should just dump the hex number of the entire selector, which would require much less code to turn back into a selector. And wouldn't require a YAML parser, just a sscanf.

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.

5 participants