Skip indirect operation for out of bounds indirect stages #9757
Conversation
|
I did not port this to the software renderer. The software renderer currently just skips reading the texture in this case, using whatever was loaded before; looking at my hardware test results, this is actually fairly close to correct behavior. I could try to port this change to it, but I don't know if that's needed. |
|
I don't think making Software Renderer less accurate is a good idea. I would like to know why this hack is necessary versus the software renderer solution or a more accurate solution, though. I'm sure there's a good reason why, I just would like to documented here if I merge this. |
|
Based on my understanding, it's not feasible to use values from previous pixels in shaders, since that means values can't be computed in parallel, so the way the software renderer currently works (which wasn't something that was intentionally developed as far as I can tell) wouldn't be usable there. (If there is a way of doing it, I don't know enough about shaders to implement it. And I'm not 100% sure I understand why the software renderer is producing what it is.) Furthermore, the exact behavior on hardware isn't clear. Here are some examples from my hardware test; you can see that the undefined case is visually similar to the defined case, but it doesn't match exactly.
|
This fixes rendering issues in Viewtiful Joe (https://bugs.dolphin-emu.org/issues/12525), but it is not entirely hardware accurate, as hardware testing showed other, more complex behavior in this case. However, it should be good enough for our purposes.
|
FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:
automated-fifoci-reporter |
This fixes rendering issues in Viewtiful Joe (bug 12525). The issue was introduced in #9651, specifically by the "always apply fb_addprev and tex coord wrapping" aspect; with that, I also always applied the indirect operation. Doing that is is not hardware accurate, but skipping it (which is what this PR does) is not either; this test case (specifically the files ending in 0Y) trigger this undefined behavior on console. However, skipping gives a result that's close enough to avoid visual issues in Viewtiful Joe.
Based on the fifolog there, Viewtiful Joe tends to set the indirect configuration at the start of most objects, by clearing it for TEV stages 0 and 1 (with BP commands 10 and 11). On Object 78, it does that, and then later updates the configuration for TEV stage 0 to enable indirect functionality (at offset 242a2). It also updates the BP genmode to have 1 indirect stage enabled (at 2431c). The next object triggers an EFB copy, and then draws the copied EFB to screen a few times. However, it does not clear the indirect configuration, but does set the number of indirect stages to 0 (at 2451f); this means that TEV stage 0's previous indirect configuration applies, but now the indirect stage is not enabled. This triggers undefined behavior. I'm guessing it wasn't particularly evident on console due to the copies already being drawn multiple times, and a small matrix being used. Things went wrong in Dolphin because the indirect stage had bias enabled, and was using ITF_8 (0) as its format; this resulted in the computed coordinate being offset by 128 units, which is much more obvious.