Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reinterpret between 32 and 16 bit texture formats #15907

Merged
merged 36 commits into from
Aug 28, 2022

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Aug 26, 2022

Fixes #15898 (Rendering issues in Tantalus games), more details in that issue.

Fixes #9850
Fixes #9604
Fixes #9018

Fixes the God of War shadow regression reported in comments to #15888.

Hopefully this also gets us closer to universally enabling reinterpret.

Not too afraid of merging this despite its size, can't find any regressions but if there are, they should be easy to fix, easier than before due to many things being handled in ways that are easier to reason about.

Spongebob now renders near perfectly, the other games have minor issues at 2x+ resolution, and apparently some subtle off by 1. Good enough for a first merge.

So far, Spongebob is rendering better but still broken due to two issues:

  • Need to implement the depth texture swizzle of +200000 addresses, since the game expects it
    • However, the current implementation somehow breaks Me and My Katamari's depth fog
      • It seems that the color format affects the 200000-type swizzle. It may not be present when the color format of the framebuffer is 16-bit. This will need hardware testing (does make sense though to have different swizzles for different color depth, to have the depth access pattern as different as possible from color).
  • Something is wrong with the depth texture depal, seems the shift is wrong, or somehow the input to it, something, as it causes an annoying repeating gradient EDIT: Turns out that the game uses a stencil/alpha trick to clear alpha, making this not apply where it's out of range.
    CLUT mode is set to CLUT16, mask 0xFF, shift 2. Depth data is 16-bit, so that just takes bits 2-10. The high bits 11-16 are unused in the lookup, causing a repeated gradient, which is weird and looks bad.

Other things to do:

  • Cars / MX vs ATV games have some different issues
    • Framebuffers cloned for reinterpret created too small
    • Half the screen still corrupt
    • Minor display corruption all over (UV precision issue?)
    • Thin stripes across the screen (maybe higher res is not possible? but something else is 1px off..)
  • GE dumps of Spongebob don't render correctly, might be interesting to figure out why and fix
  • Double check the first two commits

image
image

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Aug 26, 2022
@hrydgard hrydgard added this to the v1.14.0 milestone Aug 26, 2022
@hrydgard hrydgard force-pushed the reinterpret-between-32-and-16 branch from d82d824 to 6aecbef Compare August 26, 2022 14:06
@hrydgard hrydgard marked this pull request as ready for review August 27, 2022 14:39
@hrydgard
Copy link
Owner Author

hrydgard commented Aug 27, 2022

I've tested this on mobile now, and it works just fine on Adreno, except that other than Spongebob, the games render a bit dark, for whatever reason. I'm gonna leave that to later fixes.

Also dark on Mali, so likely not a driver bug, rather some configuration thing or extension check..

This now also fixes the God of War shadow regression.

Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

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

I feel like I'm missing something that makes the depth swap safe - what if a game were using that configuration of parameters but actually intended to draw color? Maybe left from a previous draw.

Also feel like we should probably just add those cmds to the depth dirty, or else we'll find little bugs...

-[Unknown]

GPU/Common/FramebufferManagerCommon.cpp Outdated Show resolved Hide resolved
GPU/Common/FramebufferManagerCommon.cpp Outdated Show resolved Hide resolved
GPU/Common/FramebufferManagerCommon.cpp Outdated Show resolved Hide resolved
GPU/D3D11/StateMappingD3D11.cpp Outdated Show resolved Hide resolved
@hrydgard
Copy link
Owner Author

As for adding the blend stuff to depth dirty, yes, technically it's needed. Still I'm reluctant because that is common enough state that it might have some impact, and setting stencil to KEEP/KEEP/ZERO like this is quite unusual so unlikely to be leftover state from some previous draw... But I might end up eating those words, heh.

Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

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

State tracking bugs are always annoying, though you're right it should be pretty rare with these checks.

-[Unknown]

GPU/Common/FramebufferManagerCommon.cpp Outdated Show resolved Hide resolved
GPU/Common/GPUStateUtils.h Outdated Show resolved Hide resolved
@hrydgard hrydgard merged commit 1653dcd into master Aug 28, 2022
@hrydgard hrydgard deleted the reinterpret-between-32-and-16 branch August 28, 2022 07:49
hrydgard added a commit that referenced this pull request Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GE emulation Backend-independent GPU issues
Projects
None yet
2 participants