-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Revert TEV alpha lerp change and special-case alpha=1 in blending #10394
Conversation
JMC mentioned that NotesIn that, they have an object (1133) with an alpha value of 1, which is used to mask off some things to draw by updating the depth buffer for everything that shouldn't be drawn. In this case, they actually do use a texture with alpha in it: Here's that same texture with the alpha channel removed (it's an Here's the old depth buffer (using values of .99-1 in renderdoc): And here's the new depth buffer, where things work properly: Furthermore, here's an image from real hardware, via the hardware fifoplayer: Here's what Dolphin used to look like: And here's Dolphin's new result: It matches pretty closely, though real hardware has some dithering that Dolphin doesn't have; I don't know what's going on with that. I noticed that with Mario Kart Wii too, but haven't investigated further. One other thing is that the mesa fifoci instance had white lines, but those are gone now; Baten Kaitos seems to be using the alpha channel in the EFB for similar effects elsewhere in rendering, and I guess mesa handled that weirdly? I don't know why it's fixed, and I don't know why only mesa was affected. Here's the EFB with alpha enabled, via renderdoc (the hardware fifoplayer doesn't include alpha, at least not with how I'm generating the images): |
So, This is an improvement in all of our known cases then. |
fbb51b1
to
2194fa9
Compare
For reviewing purposes, here's FifoCI differences for the first commit and the second commit, in addition to combined differences for both. I haven't had time to go through everything, though. |
Here are the interesting fifoci diffs (this took a while to prepare). bk-tev and mkw-flags are fixed by this, and fortune-street-white-box does not regress. jb-shadow also has something invisible happen in it that should be investigated further, and a few lighting/transparency effects seem to have slightly changed. Mesa also has a problem that I'll need to investigate further, though this problem doesn't exist in the combined version; this only applies to llvmpipe (the software renderer for mesa) but it still should be reported to them.
|
@Pokechu22 - is this ready to go? You mention a few investigations. Are those going to happen in a different PR? |
I looked into the bond thing, and it seems like it just draws some fog/snow particle effects that are fairly transparent. The mesa issue isn't something that will be handled in a PR here; I just need to look into it and report it to them (in case it affects other stuff). So, this PR is ready for review (though it's not one that needs to be merged this month). I still need to investigate how blending works further, as well, but for the sake of this PR the second hacky commit is good enough (once blending is understood better, it will be easy to replace it in a new PR). |
It doesn't make sense for alpha to add the bias ONLY when dividing by 2, while color doesn't apply the bias for divide by 2 only; hardware testing indicates that alpha should have the bias. This fixes the menus in Mario Kart Wii (https://bugs.dolphin-emu.org/issues/11909) but reintroduces the white rectangle in Fortune Street. This reverts commit 5aaa514 (and several other matching changes elsewhere).
This removes the white box in fortune street again, without causing Mario Kart Wii to regress.
2194fa9
to
444f6fd
Compare
FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:
automated-fifoci-reporter |
I took a quick look at Gormiti (following my previous attempt with #9914). The main video is drawn with I mention this because that means that this change making Gormiti solid black instead of having color values of 0 or 1 in each channel is probably still fine; I don't think Gormiti is directly dependent on this behavior (instead it's dependent on whatever is going on with the near plane). It's also not like having values of 0/1 in each channel was enough to actually see the game; for all intents and purposes it was black before and it's still black now. |
This is approved and I tested the 3 known affected games. The code is also relatively simple, and the comments are very clear. There isn't a lot changed and part of it appears to be a revert. I think this is safe to merge with the approval and we're just past a beta. |
This also fixes shadows in Micro Machines, which seems to directly care about the lerping behavior. |
This was added in dolphin-emu#10394 for both the hardware and software backends to work around an issue with Mario Kart Wii, Fortune Street, and Baten Kaitos. However, it seems like the software renderer handles blending well enough that we don't need this (and in any case, it's easy to change blending in the software renderer). Some experimentation with dolphin-emu#11387 (not pushed) showed that the software renderer's logic would also produce correct results on the hardware backends with this hack removed, but would require fbfetch (currently); if a better solution is found the hack can also be removed from the hardware backends.
This was added in dolphin-emu#10394 for both the hardware and software backends to work around an issue with Mario Kart Wii, Fortune Street, and Baten Kaitos. However, it seems like the software renderer handles blending well enough that we don't need this (and in any case, it's easy to change blending in the software renderer). Some experimentation with dolphin-emu#11387 (not pushed) showed that the software renderer's logic would also produce correct results on the hardware backends with this hack removed, but would require fbfetch (currently); if a better solution is found the hack can also be removed from the hardware backends.
Fixes https://bugs.dolphin-emu.org/issues/11909, partially by reverting #4514. I still need to do further testing here; I'm fairly sure the first commit is correct (I was able to confirm that an alpha value of 1 is being generated by both Fortune Street and Mario Kart Wii), but am less sure about the second one.