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

Add epsilon hack to work around rounding issue in texture lookup #9407

Closed

Conversation

dkacperski97
Copy link

It is based on the solution (hack) proposed by @endrift in #3807. It fixes the issue with VP6 videos and possibly some others.

Since it's not a perfect solution, I added it as a togglable hack. As mentioned in the original PR, an ideal solution to this problem would be to integerize texture lookups, but it would be much slower. I added a comment about it in the shader code. I also updated the game settings files accordingly, based on this wiki page.

I tested it with the GTX 1080 ti on Windows (drivers version 460.89) and with the Mali-G71 on Android 9 (Galaxy S8+) in several games.

@iwubcode
Copy link
Contributor

iwubcode commented Dec 31, 2020

I'm sure others will have opinions, my main concern is that this is turned on for games in general to fix a bug on some vendor GPUs but might negatively impact the GPUs that work correctly.

I wonder if we should create a driver issue and only turn it on for some drivers (ex: Nvidia)?

EDIT: we don't have any game specific driver bugs at the moment, so...

@mbc07
Copy link
Contributor

mbc07 commented Jan 1, 2021

I can't test this at the moment but this PR might also fix dancer corruption in earlier Just Dance titles too. I'm not completely sure if the Just Dance corruption affects all GPU vendors or just NVIDIA, though...

@dkacperski97
Copy link
Author

@iwubcode In shader generators, I could just check if the hack is enabled and if the drivers have a bug. After such a change, I should probably add information in the description that the hack does not affect the image on properly working GPUs. I'm just not sure which cards are affected by this bug. Should I enable it for all user-reported configurations on the issue page? This bug also occurs on the Mali-G71 that I have access to, and from what I can see, no one has reported this bug for this GPU.

@mbc07 I don't have access to Just Dance so unfortunately I can't verify how the hack affects it, but the issue description sounds very similar. If anyone could confirm that turning on the hack fixes it, I can add a settings file for this game

@mbc07
Copy link
Contributor

mbc07 commented Jan 3, 2021

Tested Just Dance, Just Dance 2 and Just Dance 3 and indeed this epsilon hack affects them. I tested on Intel Ivy Bridge (very old, only the DX11 backend still works with it) and on NVIDIA Kepler:

Intel, without the hack

  • Just Dance: no corruptions
  • Just Dance 2: no corruptions on loading screens, few corruptions on dancers, appears as horizontal lines only
  • Just Dance 3: no corruptions

NVIDIA, without the hack

  • Just Dance: few corruptions on dancers, appears as vertical lines only
  • Just Dance 2: strong corruption both on dancers and loading screens, appears as both vertical and horizontal lines
  • Just Dance 3: very few corruptions on dancers, appears as vertical lines only (the corruption is so minor on this game that it can't even be seen depending of the selected song)

Intel, with the epsilon hack

  • Just Dance: no corruptions
  • Just Dance 2: no corruptions
  • Just Dance 3: no corruptions

NVIDIA, with the epsilon hack

  • Just Dance: no corruptions
  • Just Dance 2: no corruptions on loading screens, few corruptions on dancers, appears as horizontal lines only
  • Just Dance 3: strong corruptions on dancers, appears as both vertical and horizontal lines

Different games seems to need different epsilon values, considering that the current 0.25 value actually made things worse in JD3 with NVIDIA, despite fixing the corruption on JD and games with VP6 videos, and greatly reducing it on JD2. So, in case this PR goes further, could the epsilon value be made adjustable at runtime? Perhaps keep the hack toggle available on the UI (or even remove it completely, rendering the hack only accessible through the INI) but have an additional option (INI only too) to set the desired epsilon value?

@dkacperski97
Copy link
Author

Thanks for the tests. I think being able to change values is a good idea.

In GUI, I can change the checkbox to a slider, and in the description, mention that the value should be as low as possible.

@dkacperski97
Copy link
Author

I looked at tests of various games on the wiki and in some games the problem with VP6 movies was also reported for AMD graphics cards (eg Red Steel 2). All cards seem to be affected, but in some games it is almost invisible. So I don't think enabling the hack only for some gpu is correct.

@JMC47
Copy link
Contributor

JMC47 commented Jan 4, 2021

@dolphin-emu-bot rebuild

@dkacperski97
Copy link
Author

After discussing this PR on #dolphin-dev, the slider is too complicated.

@Extrems mentioned something that could be a valid solution to the texture lookup problem, but I don't have enough knowledge to implement it yet. Below I add his messages, maybe they will be helpful for someone:

21:47 https://github.com/extremscorner/libogc-rice/blob/master/libogc/gx.c#L1751 + https://github.com/extremscorner/emgba/blob/main/source/gx.c#L65-L92 is what I needed to cancel the error on real hardware.
22:09 The first half is the pixel center fix, the second half is using 1:1 texture coordinates and setting up a post-transform matrix with a 1/64 offset.
22:12 With this, I'm able to set up indirect texturing to make bilinear filtering give bitexact results to nearest neighbor at integer scales.
22:14 (both the texture and indirect texture are bilinear filtered)

@DJRM2021
Copy link

Any update on this PR?

@mbc07
Copy link
Contributor

mbc07 commented Jan 25, 2021

Someone on Reddit detailed another possible solution a few days ago. I'm not entirely sure on where the code snippet should go for a proper test build, so I'll just leave the link there...

@Miksel12
Copy link
Contributor

Miksel12 commented Jan 25, 2021

Someone on Reddit detailed another possible solution a few days ago. I'm not entirely sure on where the code snippet should go for a proper test build, so I'll just leave the link there...

That should replace this line: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/PixelShaderGen.cpp#L1204
This is only the non-ubershader path but should suffice for testing.
I don't have any vp6 games so I can't test unfortunately.

@JMC47
Copy link
Contributor

JMC47 commented Jan 25, 2021

If we made a PR build I could test it. Or in a few hours I could setup a build here and try it out.

@Miksel12
Copy link
Contributor

If we made a PR build I could test it. Or in a few hours I could setup a build here and try it out.

#9473

@mbc07 mbc07 mentioned this pull request Jan 26, 2021
@Miksel12 Miksel12 mentioned this pull request Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants