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

Hackfix for debug cubes #9532

Merged
merged 3 commits into from Jun 10, 2021
Merged

Hackfix for debug cubes #9532

merged 3 commits into from Jun 10, 2021

Conversation

Pokechu22
Copy link
Contributor

@Pokechu22 Pokechu22 commented Feb 23, 2021

This change hides the debug cubes in Super Mario Sunshine, while showing the other things that were previously broken by that change (reflections in Super Monkey Ball, trucks in Monster Jam, and course elements in modded Mario Kart Wii courses). It does this by using a game-specific setting for the default color channel value when the color is not included in the vertex.

#9122 previously used the heuristic of setting alpha to 0 if the color format was ARGB (Color0Elements == 1), and setting alpha to 255 if the color format was RGB (Color0Elements == 0). The R, G, and B components were always set to 255. However, this heuristic turned out not to be correct, as it broke other games.

My various experiments have found that the cubes are actually colored transparent black (ARGB = 0x00000000) instead of transparent white (ARGB = 0x00FFFFFF), by setting xfmem.alpha[0].matsource to 0 (which means that the material color register's alpha value, which is 0xFF, replaces the previous color) and observing that the cubes showed up but were black. Thus, this change makes the (invisible) cubes more hardware accurate, though this can only be seen by using the same patch I used in testing.

Note that since the fifo player does not use game-specific settings, the cubes will still show up in it (and thus on fifo.ci). A proper fix would not use game-specific settings and thus would have them be invisible in the fifo player, but the exact conditions that make the cubes invisible are still unknown.

This PR is submitted for later use if I can't figure out what actually causes the cubes to be invisible, but shouldn't be merged unless it seems like that's not going to be solved at all. (In addition, the shader UID version needs to be bumped, which I haven't done yet; when the shader UID version is bumped I'd also want to remove some of the backwards-compatibility functionality in #9497 which has not yet been merged.)

@Pokechu22 Pokechu22 force-pushed the debug-cube-hack branch 3 times, most recently from 06a8df0 to 3f0ac53 Compare March 7, 2021 06:54
@phire
Copy link
Member

phire commented May 24, 2021

If we are going to do a hack-fix, I think a "enabled by default" cheat-code which patches the game code to not emit ill-formed GX commands might be a better approach.

At least that would get baked into any future fifologs that get recorded.

@JMC47
Copy link
Contributor

JMC47 commented May 24, 2021

What would we do about romhacks and the such that don't use conventional GameIDs or boot from dols/homebrew channel?

@phire
Copy link
Member

phire commented May 24, 2021

Those are equally ugly with the per-game config hack approach.

This is why we really don't like per game fixes.

@JMC47
Copy link
Contributor

JMC47 commented May 24, 2021

Right now we could go back to the old behavior except for Super Mario Sunshine and everything would work. To our knowledge. So a code for Super Mario Sunshine might be enough, but this behavior is theoretically correct for Super Mario Sunshine...?

@Pokechu22
Copy link
Contributor Author

I don't think it's feasible to do a game patch, as from what I remember the misconfiguration happens in level data and not in code, and I don't think there's one place in the code where it can be detected (for my testing, I forced use of the material color for everything, but that has side effects for other things and we'd also need to change the current material color to transparent black if we wanted to use it to hide the cubes.) It might be possible, but it'd require a fair bit of research.

I don't think romhacks are a concern; if a hack has a new gameid, it probably has different levels, so the debug cubes in one specific level shouldn't be a problem.

@phire
Copy link
Member

phire commented May 24, 2021

We can totally patch level data too, but that might be getting a bit on the hard side.

@Rumi-Larry
Copy link

Out of curiosity, does the PAL version of the game behave the same?

@Pokechu22
Copy link
Contributor Author

I haven't tested it (as I only own a US copy), and bug 8059 doesn't say. But probably, the US, PAL, Japanese, and Korean versions are all affected, as the Nintendo Switch version was affected and that would probably be built from the latest codebase.

@Pokechu22 Pokechu22 force-pushed the debug-cube-hack branch 4 times, most recently from 87438c7 to 7471cab Compare June 9, 2021 04:45
@Pokechu22 Pokechu22 marked this pull request as ready for review June 9, 2021 05:03
@Pokechu22 Pokechu22 changed the title (Do not merge) Hackfix for debug cubes Hackfix for debug cubes Jun 9, 2021
Source/Core/Core/Config/GraphicsSettings.cpp Outdated Show resolved Hide resolved
@iwubcode
Copy link
Contributor

Note that since the fifo player does not use game-specific settings, the cubes will still show up in it (and thus on fifo.ci). A proper fix would not use game-specific settings and thus would have them be invisible in the fifo player

If the fifologs stored the game id inside the file structure, we could technically read that and use it as the game id for specific graphic settings? Not sure what else that would break though

@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • mkw-bridge on ogl-lin-mesa: diff
  • smb-mirror on ogl-lin-mesa: diff
  • sms-gc on ogl-lin-mesa: diff
  • mkw-bridge on sw-lin-mesa: diff
  • smb-mirror on sw-lin-mesa: diff
  • sms-gc on sw-lin-mesa: diff
  • mkw-bridge on ogl-lin-radeon: diff
  • smb-mirror on ogl-lin-radeon: diff
  • sms-gc on ogl-lin-radeon: diff
  • mkw-bridge on uberogl-lin-radeon: diff
  • smb-mirror on uberogl-lin-radeon: diff
  • sms-gc on uberogl-lin-radeon: diff

automated-fifoci-reporter

@JMC47 JMC47 merged commit a51d01b into dolphin-emu:master Jun 10, 2021
11 checks passed
@Drflash55
Copy link

Oh, cool to see that a fix for this was finally able to be done and implemented into Rev 14402.
Finally, we can see the Monster Trucks again!

image

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