pcsx2 icon indicating copy to clipboard operation
pcsx2 copied to clipboard

GS: Backup texture to upper 8 bits of Z during format conversion (WIP)

Open TJnotJT opened this issue 4 months ago • 4 comments

Status: Waiting on this PR https://github.com/PCSX2/pcsx2/pull/13159 (refactors to make implmentations cleaner)

Draft PR to address issue noted here: https://github.com/PCSX2/pcsx2/issues/13037. Only OpenGL has been implemented and some other things are also unfinished (see below). Any feedback/criticism on whether this is a feasible approach is much appreciated.

All renderers except OpenGL are currently broken in the PR (including software since its uses Vulkan shaders).

Description of Changes

Add a new member to GSTexture to keep track of 32 bit Z when Z32 is converted to Z24, and the code needed to make sure that the upper 8 bits of Z are preserved when converting from Z24 back to Z32 or to C32/24.

Unfinished things:

  1. Conversions from Z24 -> Z32 to a 16 (rather than 32) bit format.
  2. Not sure yet how to handle clears, whether the upper 8 bits should be preserved or cleared also when Z buf is in Z24.
  3. Handling other conversions like Z32 -> Z24 then using upper 8 bits as P8H, etc. (not sure if games actually do this).

Rationale behind Changes

Allows preserving the upper 8 bits of Z that might help the issue above.

Suggested Testing Steps

Might affects a lot of code so testing any game for bugs (there are probably a few) would help. Haven't yet done a fully dump run.

Army Men - Major Malfunction_SLES-53996_20250806151204.zip

Master (OpenGL): 00102_f00001_fr-1_02800_C_16S

PR (OpenGL): 00102_f00001_fr-1_02800_C_16S

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

GitHub copilot.

TJnotJT avatar Aug 13 '25 03:08 TJnotJT

Not really a fan how the shader copy is handled, is there a more cleaner way?

Yea, the shader code and the StretchRect() code is a bit messy right now. I'll try to clean it up a bit. Anything specific you had in mind? One option is to add a single #define to the shader in GetShaderSource() to switch between single depth and double depth to avoid having defined(...) || defined (....) ... etc.

TJnotJT avatar Aug 17 '25 02:08 TJnotJT

I was thinking if possible to avoid touching the StretchRects since it looks messy now, or try to minimize it, could also be just on the final StretchRect.

lightningterror avatar Aug 17 '25 03:08 lightningterror

I was thinking if possible to avoid touching the StretchRects since it looks messy now, or try to minimize it, could also be just on the final StretchRect.

Yea, some things can be localized to a single StretchRect(), preferably in GSDevice so that it doesn't have to be duplicated in all renderers.

I can also remove the extra argument to GSDeviceOGL::StretchRect() for using the upper-8-bit backup since that info is passed in the texture anyway.

TJnotJT avatar Aug 17 '25 04:08 TJnotJT

Fixed the shader compilation issue with the int/uint overloading ambiguity so should hopefully compile on all drivers.

Still have to do the other fixes.

TJnotJT avatar Aug 17 '25 14:08 TJnotJT