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

rsx: Shader decompiler improvements #5860

Merged
merged 13 commits into from Apr 25, 2019
Merged

rsx: Shader decompiler improvements #5860

merged 13 commits into from Apr 25, 2019

Conversation

kd-11
Copy link
Contributor

@kd-11 kd-11 commented Apr 18, 2019

  • Fix fp16 emulation. Simple clamping is not enough as infinity and nan need preserving and nvidia cards clamp NaN to the lower bounds which is wrong.
  • Use native fp16 types if the hardware or driver supports it. In OpenGL most modern hardware will report this feature but for vulkan only Volta, Turing and Vega report this functionality. Should help lower the overhead of the fp16 emulation.
  • Fix shadow compare operations on vulkan. In OpenGL, GL_ARB_shadow specifies that compare_R_to_texture clamps R to (0-1) before comparing. Vulkan behaves more like the older GL_SGIX_shadow extension and R is not necessarily clamped. This is particularly true when the depth buffer is in floating point format such as D32F_S8. Fixes broken shadows when using AMD cards or NV cards when the 'High precision Z' option is enabled.
  • Fix poor attribute interpolation precision on Nvidia which causes spurious failure of floating point comparisons in fragment shaders and leads to visual noise.
  • Properly enforces IEEE-754 rules for all opcodes with the exception of DIVSQ which is provably broken on realhw as well and is now done per-channel to match realhw. LG2 and RSQ seem to ignore the sign bit of the input arguments but otherwise behave in an IEEE compliant way.

Fixes:
#5830
#4174
#5343
#4799
#5567

- Emulating f16 with f32 is not ideal and requires a lot of value clamping
- Using native data type can significantly improve performance and accuracy
- With openGL, check for the compatible extensions NV_gpu_shader5 and
AMD_gpu_shader_half_float
- With Vulkan, enable this functionality in the deviceFeatures if
applicable. (VK_KHR_shader_float16_int8 extension)
- Temporarily disable hw fp16 for vulkan
- Improve support for float16_t by minimizing mixed inputs to functions
(ambiguous overloads)
- Minimize amount of downcasts in code by using opcode flags
- Re-enable float16_t support for vulkan
…rmats are used

- The fixed-point D24S8 format does special Z clamping during compare which matches PS3 behaviour
- D32S8 is a floating point format and comparison with Dref > 1 always fails causing black edges/borders
- The hw generates inaccurate values when doing perspective-correct
  interpolation of vertex output attributes and makes the comparison (a ==
  b) fail even when they are a fixed constant value.
- Increase equality tolerance when doing comparisons in fragment
  shaders for NV cards only to work around this issue.
- Teepo fix
- While mul(0, nan) = nan and 0 / 0 = nan, 0 / sqrt(0) = 0 because of hw
gremlins. normalize(0) is also nan so this behaviour does not work
around that particular case either which makes it even more baffling.
@MarioSonic2987
Copy link
Contributor

MarioSonic2987 commented Apr 18, 2019

One Piece: Pirate Warriors has regressed. Now shows black textures in Vulkan and throw a shader compilation error with OpenGL.

Master

image
image

This PR

PR OpenGL:

E {RSX [0x02192f0]} RSX: Failed to compile shader: 0(322) : error C7011: implicit cast from "int" to "float16_t"

F {RSX [0x02192f0]} RSX: class gl::glsl::link_exception thrown: linkage failed: 'Fragment info
-------------
0(322) : error C7011: implicit cast from "int" to "float16_t"
(0) : error C2003: incompatible options for link
'

PR Vulkan:

image

Log OPPW (OpenGL PR)

Shaderlog OPPW (PR)

In Grand Theft Auto IV, there's a weird circle around lights.

GTA IV

image
image

In Grand Theft Auto V, there's a weird shading on characters with Vulkan (with OpenGL it's the same as Vulkan in master).

GTA V

image
image

@arcadee1977
Copy link

Thanks for your great work! All of Nvidia card's graphic issue have been fixed.

@incognitoh
Copy link

incognitoh commented Apr 18, 2019

RR7 basically looks perfect on NVIDIA now. Nice

rr7rpcs3nv

@stride21
Copy link

stride21 commented Apr 18, 2019

This PR causes black squares to appear in Battlefield: Bad Company.

PR: OpenGL:
bf opengl pr
PR Vulkan:
bf pr vulkan'
Master OpenGL:
bf open master
Master Vulkan:
vulkan bf master

RPCS3.log.gz

Motorstorm when using Vulkan also regressed in this PR. The black on the cars, trees and other objects seem to only happen when they're in the shade.
PR Vulkan:
vulkan PR
Master Vulkan:
master motors vulkan

RPCS3.log.gz

@jenci8888
Copy link

jenci8888 commented Apr 18, 2019

Progression and regression other games.

-GT5/6 no more flickering on car anymore, even still present rainbow, but more or less.
-GT5 tries to load night tracks when gets Vulkan (out of memory) error.
-More unstable ingame GT6, It can hangs ingame/intro with Vulkan error. (the error same as master)
-NFS Most Wanted has regression black car (not including traffic vehicles)
-NFS Rivals tries to load main menu but I get this:
E {RSX [0x05648f0]} RSX: Unsupported depth conversion (0x9A) then,
F {RSX [0x055a8ec]} RSX: class std::runtime_error thrown: Assertion Failed! Vulkan API call failed with unrecoverable error: Device lost (Driver crashed with unspecified error or stopped responding and recovered) (VK_ERROR_DEVICE_LOST) (in file c:\projects\rpcs3\rpcs3\emu\rsx\vk\vkhelpers.h:964) (same as GT5)

Screenshots:
image
image
image

Log:
RPCS3-GT5.gz (crash when loads night tracks including out of memory error as well)
RPCS3.log.gz (NFS Rivals)

@ActualMandM
Copy link

ActualMandM commented Apr 18, 2019

Regression with Sonic's model in Sonic Unleashed

Image

2019-04-18_16-06-52

RPCS3.log.gz

@RainbowCookie32
Copy link
Contributor

Beyond Two Souls is back to normal in this PR (seems like it broke at some point in master)

b2s-master

b2s-pr

@kd-11
Copy link
Contributor Author

kd-11 commented Apr 19, 2019

Need some kind of trace to debug any of these. RRC or RDC will do.

@ActualMandM
Copy link

If you think you could trace the issue with Unleashed with the RSX capture looking like this, then I can give you one.

2019-04-18_16-10-59

I believe this is an RSX capture only issue. Either that or something went wrong on my end when loading the capture.

@MarioSonic2987
Copy link
Contributor

MarioSonic2987 commented Apr 19, 2019

RSX Capture (OPPW VK Master)

RSX Capture (OPPW VK PR)

@stride21
Copy link

MotorStorm Vulkan RSX Captures

Battlefield Bad Company OpenGL RSX Captures

Battlefield Bad Company Vulkan RSX Captures

@kd-11
Copy link
Contributor Author

kd-11 commented Apr 20, 2019

@slashiee Don't worry about RSX capture not looking correct in replay, it usually still draws stuff properly but breaks post-processing which often relies on Cell-side intervention to look right.

- Properly test for NaN and Inf when clamping down to fp16
- Optimize divsq a bit; mix(vec, vec, bvec) emits OpSelect which is what
we want here, instead of component-wise selection which is much slower.
@kd-11
Copy link
Contributor Author

kd-11 commented Apr 20, 2019

Retest, improved clamp16 and divsq a bit.
As for OpenGL refusing to compile some fp16-native code, it seems to be a driver bug when the driver is looking for a function override; investigating possible workarounds.

@MarioSonic2987
Copy link
Contributor

MarioSonic2987 commented Apr 20, 2019

One Piece: Pirate Warriors and GTA V regressions are now fixed.
image
image

But in GTA IV it's the same:
image

@kd-11
Copy link
Contributor Author

kd-11 commented Apr 20, 2019

Check the OpenGL crash; should be fixed now. For GTA4 I'll need comparison RDC to debug further. Vulkan vs master should be enough.
The differences between OpenGL and Vulkan in some cases is because OpenGL supports float16 'natively'. In reality most vulkan drivers also do support float16 but they do not advertise this feature properly. The differences appear when the native float16 implementation is more accurate than the one we emulate with the shader. A config option "disable native float16 support" exists in config.yml to force use of the shader emulation path and should also 'break' OpenGL when enabled.

@Xcedf
Copy link

Xcedf commented Apr 20, 2019

Sonic Unleashed regression fixed

@MarioSonic2987
Copy link
Contributor

RDC Vulkan GTA IV Master
RDC Vulkan GTA IV PR

OPPW crash with OpenGL is now fixed, by the way.

@kd-11
Copy link
Contributor Author

kd-11 commented Apr 23, 2019

Added a workaround for the problematic instruction. It no longer behaves like real hardware but we cannot match exactly the PS3's TMU so I think going out of spec there is fine. Basically the texture filtering does not seem to match the PS3 exactly pixel-for-pixel which is expected. Since texturing is completely fixed function (non-programmable) a workaround is the only way to get around this. I can make the hotspots dance around the screen by altering the filtering even very slightly causes the hotspots to migrate. More research on this topic will be carried out at a later date, but likely only for academic curiosity rather than improving any games (NaNs are rather useless as they are often substituted with 0 anyway).

@stride21
Copy link

stride21 commented Apr 23, 2019

Glowy things are fixed with this PR but now shooting with a weapon causes a fatal error.

Master:
BF master

PR:
battlefield bad company

PR error:
BF error
RPCS3 log.zip

@kd-11 kd-11 force-pushed the master branch 2 times, most recently from 51007d7 to e939e3a Compare April 23, 2019 19:19
@stride21
Copy link

Battlefield Bad Company fatal error is now fixed.

@Kravickas
Copy link
Contributor

Since u are working on shaders, I leave this here if u wanted to look in to that too even tho its regression from another time.
RPCS3.log.gz
Far Cry 2 when loading ingame and generating shaders

·E 0:04:32.978121 {RSX [0x008ad10]} RSX: ERROR: 0:330: '' :  syntax error, unexpected EQUAL
ERROR: 1 compilation errors.  No code generated.


·E 0:04:32.978128 {RSX [0x008ad10]} RSX: 
·F 0:04:32.978395 {RSX [0x008ad10]} RSX: class std::runtime_error thrown: Failed to compile vertex shader
(in file c:\projects\rpcs3\rpcs3\emu\rsx\vk\vkhelpers.h:2982)

Let it be if its completly different issue or hard to fix.

@kd-11
Copy link
Contributor Author

kd-11 commented Apr 23, 2019

If you don't have the shaderlog then you have to submit an opengl log. Vulkan backend does not write any shader info in the log file.

@Kravickas
Copy link
Contributor

OGL log RPCS3.log.gz

@kd-11
Copy link
Contributor Author

kd-11 commented Apr 24, 2019

Enable 'Log shader programs' in debug settings and then after the crash (disable async shader compiler beforehand) attach shaderlog/vp_analyser.bin file.

TODO: Investigate the _s input modifier behaviour further, in case it can avoid generating zeroes from a MAD instruction.
x = MAD(+ve, -ve, -ve) with _s input modifier in BFBC expects result to be Non-zero
@Kravickas
Copy link
Contributor

Kravickas commented Apr 24, 2019

OGL, strict enabled, async shaders disabled
vp_analyser.zip
i hope thats it

vulkan, strict enabled, async shaders disabled
vp_analyser.zip

- When moving to CC, the operation has VEC flag disabled and also temp
regs disabled. Looks to be the catch-all ELSE in the selection logic.
@kd-11
Copy link
Contributor Author

kd-11 commented Apr 24, 2019

Far Cry 2 crash should be fixed now.

@EwoutH
Copy link

EwoutH commented Apr 24, 2019

Has someone benchmarked this PR on a Nvidia Turing (GTX 16 or RTX 20 series) or AMD Vega (Radeon Vega 56/64 or VII) yet? Those GPUs offer double the FP16 performance, would be interesting to see the gains.

@kd-11
Copy link
Contributor Author

kd-11 commented Apr 24, 2019

The version of glslang we use to emit SPIR-V may not have the optimizations built in to properly support fp16 as it is a relatively recent thing in the vulkan core spec, although the older AMD extension VK_AMD_gpu_shader_half_float seems to work correctly (there should be no difference to the emitter in theory, but you never know). A Vulkan tools update is coming soon to bring us to newer SDK versions, but it could be a few weeks before that is done.

@Kravickas
Copy link
Contributor

Far Cry 2 fixed, n1 thanks

@Xcedf
Copy link

Xcedf commented Apr 24, 2019

Confirm Far Cry 2 fixed

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

Successfully merging this pull request may close these issues.

None yet