[WIP] VideoCommon: Add epsilon to work around Nvidia rounding issue in texture lookup #3807
Conversation
|
I've been testing this and it doesn't seem to break anything. It only fixes a bunch of weird video problems. Excite Truck and EA VP6, for instance. |
|
We may need to change the tag to "merge immediately." I mean, what's one more epsilon hack? |
|
@JMC47: Take another look at: I'd say that looks bad. The question is if it's a fifo log issue, or if it's as bad when playing the game. (or if it's even proper emulation...) |
|
Twilight Princess is fine. It's a fifolog issue. |
endrift
changed the title
|
Odd that this doesn't actually break anything. In that case, it might make sense to merge for 5.0 if we're feeling daring, but otherwise, we should fix it properly post-5.0 by integerizing texture lookups (e.g. with texelFetch) |
| @@ -836,7 +836,7 @@ static void WriteStage(T& out, pixel_shader_uid_data* uid_data, int n, API_TYPE | |||
| uid_data->SetTevindrefTexmap(i, texmap); | |||
|
|
|||
| out.Write("\ttextemp = "); | |||
| SampleTexture<T>(out, "float2(tevcoord.xy)", texswap, texmap, ApiType); | |||
| SampleTexture<T>(out, "(float2(tevcoord.xy) + 0.05)", texswap, texmap, ApiType); | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Talked to @degasus about this in -dev, and while I think that going the integerization approach would be more correct, we agreed that it would be much slower, to the point of not being worthwhile. As such, an epsilon that's no more than 1/2 (according to degasus) or 1/4 (according to me, to avoid over-correcting the rounding), would be the fastest approach and might still give correct-enough results. An epsilon of 1/20 (what's in this patch) does appear to fix the issue (at least on Nvidia; haven't tried Intel), without noticeable ill effect, so this might be the "right enough" fix. I should probably try it on Intel, but I don't have anything newer than Ivy Bridge on Windows or Linux, (I have Haswell on my Mac and Skylake on my FreeBSD box, but the former is a proprietary driver and the latter doesn't have an IGP driver at all) which will hopefully be new enough to test on. Any other opinions from GPU people? |
|
I'd rather have it working somewhat now. We have The Simpson's game to remind us that we need to do this perfectly. |
|
Are we sure that The Simpson's game's bug is the same bug? I know that this epsilon patch fixed Excite Truck as well, but I thought it didn't affect anything else? |
|
I believe @magumagu could answer as he looked into it a long time ago? I'm 90% sure it's the same bug... |
|
The Simpsons thing is very closely related; see magumagu@f96aab6 . That hack shoves the texture sampling coordinate 1/128 of a pixel the other way. |
|
Is there a fifolog for the Simpsons bug? Which platforms does it affect? |
|
See https://bugs.dolphin-emu.org/issues/4570 . All platforms, as far as I know. I think the simpsons dff on fifoci contains the issue; the server is acting up so I can't double-check. |
endrift
force-pushed
the
tex-lookup-epsilon
branch
from
88e6557
to
9b58529
Compare
|
I just updated to master, and upped the epsilon to 0.25, as discussed above. |
|
@dolphin-emu-bot rebuild |
1 similar comment
|
@dolphin-emu-bot rebuild |
|
FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:
automated-fifoci-reporter |
|
Any progress on this PR? |
|
I believe there was another PR relating to this sort of thing that wasn't an epsilon hack, but I don't know where it is or if it got merged. |
|
I think it got abandoned. @degasus is this worth pursuing so we can make EA games look less broken on NVIDIA for a while? |
Fixes ea-vp6 on Nvidia (at least on OS X). Also a terrible hack.
Probably breaks a lot of stuff.I'm mostly posting this to see how much fifoci complains, and also to try and spur conversation about how to actually fix it.This change is