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

GS-HW: Use integers for depth conversion shaders #5518

Merged
merged 1 commit into from
Feb 15, 2022
Merged

GS-HW: Use integers for depth conversion shaders #5518

merged 1 commit into from
Feb 15, 2022

Conversation

stenzek
Copy link
Contributor

@stenzek stenzek commented Feb 12, 2022

Description of Changes

This improves the precision when reinterpreting textures to the depth buffer, reducing round-trip error which causes depth fighting in the games listed below.

Sadly, it doesn't improve precision in rendering alone (e.g. Quake 3), since that requires more than 23 bits of precision. For what it's worth, using unrestricted depth range and reinterpreting the unsigned 32-bit depth value as a float does fix Quake's rendering, but this will cause problems at the upper end of the range once it hits NaNs/inf, and requires slow depth/disables early-Z (writing gl_FragDepth).

Rationale behind Changes

Fixing bugs.
Fixes #4051
Fixes #4674
Fixes #2219

Suggested Testing Steps

Test games with z-fighting, make sure there's no regressions in others which use format conversions.

@refractionpcsx2
Copy link
Member

DBZ BT3 fixed as noted

Master:

image

PR:

image

Toro to Kyuujitsu as noted in #4674 fixed in Vulkan.

Master:

image

PR:

image

@ghost
Copy link

ghost commented Feb 12, 2022

Congratulation, you fixed a bug that existed since PCSX2 was create ^^:

image

@DonelBueno
Copy link

Congratulation, you fixed a bug that existed since PCSX2 was create ^^:

image

What game is it?

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Feb 12, 2022

What game is it?

It's a homebrew demo called Aura for Laura

@TellowKrinkle
Copy link
Member

I don't see how precision should be changed at all by multiplying by a power of 2 (like exp(-32)), that should only affect the exponent and not touch the mantissa at all.

Are you sure the accuracy improvement here isn't actually due to the changes to the depth ↔︎ color conversion functions?
(If so, we can get that accuracy without dealing with multiple depth representations across different APIs)

@TellowKrinkle
Copy link
Member

TellowKrinkle commented Feb 12, 2022

Okay I tested our current algorithm, and while I expected it to break on weird edge cases, I didn't expect it to break on literally every nonzero integer

Test and results here (Yes, it's in Metal. Sorry. Results are included so you don't need to run it.)

Looking back on it, the issue mainly stems from us treating values of 1/255 and 1/256 the same. The depth → rgba conversion grabs each 0-255 chunk divided by 256 and returns them. This works fine for values from 0 to 0x80, but for 0x81 to 0xff, the value is rounded down when being converted to unorm. I'm a dumb and missed the * 256 / 255
In the rgba → depth conversion, we multiply by 0xff and convert to integer. But the unorm → float algorithm doesn't give exact results, and it appears to (on my AMD GPU) give the float 0x3f008080 for 0x80, while the proper float for 128/255 is 0x3f008081. Because of this, multiplying by 0xff and converting to integer rounds down to 0x7f.

Edit: On an Nvidia and Intel GPU, the values were instead slightly too high for 192/255, at 0x3f40c0c2 instead of 3f40c0c1. Note that this is using the unpack_unorm4x8_to_float builtin instead of actually writing to a texture and reading a value back, so actual values may differ, but with this much variation here I don't expect great things from the latter either. At the end of the day, the lesson seems to be "Don't trust unorm values to be exactly xx/255. If you care, multiply by 255 and round to the nearest integer."

So we should definitely test fixing the conversion functions before we attempt unrestricted depth, as I don't think that's the issue

@stenzek
Copy link
Contributor Author

stenzek commented Feb 13, 2022

Refraction and I actually tested using integers for converting the incoming colour values and found that it wasn't sufficient for DTZ BT3 (at least with the conversions it uses; rgba8->float24). So while it does sound like there's an issue with the float-based conversion, I was trying to leave that path unchanged in this PR.

Copy link
Contributor

@lightningterror lightningterror left a comment

Choose a reason for hiding this comment

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

Forgot to change the dx11 equivalent shaders in tfx.fx.

@lightningterror
Copy link
Contributor

lightningterror commented Feb 13, 2022

Also fixes #4051

@stenzek stenzek changed the title GS/Vulkan: Use VK_EXT_unrestricted_depth_range where available GS/Vulkan: GS/HW: Use integers for depth conversion shaders Feb 13, 2022
@lightningterror lightningterror changed the title GS/Vulkan: GS/HW: Use integers for depth conversion shaders GS-HW: Use integers for depth conversion shaders Feb 13, 2022
Copy link
Contributor

@lightningterror lightningterror left a comment

Choose a reason for hiding this comment

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

Depth seems broken on dx11 on dbz bt3.

Fixes z-fighting in reflections in DBZ BT3, maybe others?
@ghost
Copy link

ghost commented Feb 14, 2022

Sadly it doesn't fix Sonic Heroes massive Z fighting issue with distant buildings, here is the multiframe dump:

https://drive.google.com/file/d/1uYlLEXnY6BFdVy21RFA3_38Py4hlr_Uf/view?usp=sharing

Edit: However this PR does fix The sims 2.

@refractionpcsx2
Copy link
Member

@gilderoylockhart01 yes this PR doesn't aim to fix MOST z-fighting problems, it's very specific ones, usually localised (so they appear as thin lines rather than missing polys).

@stenzek
Copy link
Contributor Author

stenzek commented Feb 14, 2022

I made a note of that in the OP. This only affects depth buffer round trips.

@refractionpcsx2
Copy link
Member

@gilderoylockhart01 what bug in The Sims 2? Have you got comparison shots?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants