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

Added projection matrix epsilon that fixes depth clipping issues in some games #1366

Merged
merged 2 commits into from Dec 3, 2014

Conversation

kayru
Copy link
Contributor

@kayru kayru commented Oct 22, 2014

I looked more closely at what values we in the projection matrix in Sonic, rawProjection 4 and 5 (3rd row z and w). They are -0.0101010110 and -1.01010108. From those, we can reconstruct near and far values. They turn out to be 1.00000012 and 100.000000. I found 1.00000012 to be suspicious. It happens to be 1 ULP over 1.0f. We can probably assume that the programmer wanted near and far to be exactly 1.0 and 100.0.

If we calculate what z and w should be for those near and far values, they come out as -0.0101010110 and -1.01010108 (0xbc257eb5 and 0xbf814afd). Values that we see coming in from Sonic are -0.0101010110 and -1.01010108 (0xbc257eb6 and 0xbf814afe), 1 ULP off.

I wrote a quick test that calculates and sets a projection matrix with those near and far values. I found that xfmem.projection was correct.

I don't understand how SU manages to produce a slightly-off matrix. It is also unclear why exactly it works on real hardware. Most likely, there is rounding happening somewhere that makes 1 ULP difference not important. It would be good to understand exactly what's going on....

There is a workaround/hack, though. It works in D3D and OGL.

@delroth
Copy link
Member

delroth commented Oct 22, 2014

Can we confirm that real hardware sets the same projection values and that
it's not a CPU emulation bug when computing these floating point values?

Do FIFO logs using the "wrong" values work on hardware?

On Thu, Oct 23, 2014 at 12:56 AM, kayru notifications@github.com wrote:

I looked more closely at what values we in the projection matrix in Sonic,
rawProjection 4 and 5 (3rd row z and w). They are -0.0101010110 and
-1.01010108. From those, we can reconstruct near and far values. They turn
out to be 1.00000012 and 100.000000. I found 1.00000012 to be suspicious.
It happens to be 1 ULP over 1.0f. We can probably assume that the
programmer wanted near and far to be exactly 1.0 and 100.0.

If we calculate what z and w should be for those near and far values, they
come out as -0.0101010110 and -1.01010108 (0xbc257eb5 and 0xbf814afd).
Values that we see coming in from Sonic are -0.0101010110 and -1.01010108
(0xbc257eb6 and 0xbf814afe), 1 ULP off.

I wrote a quick test that calculates and sets a projection matrix with
those near and far values. I found that xfmem.projection was correct.

I don't understand how SU manages to produce a slightly-off matrix. It is
also unclear why exactly it works on real hardware. Most likely, there is
rounding happening somewhere that makes 1 ULP difference not important. It
would be good to understand exactly what's going on....

There is a workaround/hack, though. It works in D3D and OGL.

You can merge this Pull Request by running

git pull https://github.com/kayru/dolphin orthographic_projection_epsilon

Or view, comment on, or merge it at:

#1366
Commit Summary

  • Added projection matrix epsilon that fixes depth clipping issues in
    some games

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#1366.

Pierre "delroth" Bourdon delroth@gmail.com
Software Engineer @ Zürich, Switzerland
http://code.delroth.net/

@kayru
Copy link
Contributor Author

kayru commented Oct 22, 2014

@neobrain said that fifos do work correctly on hardware.

@JMC47
Copy link
Contributor

JMC47 commented Oct 22, 2014

Fifologs using the wrong values do work on hardware. I'll run my recorded fifologs through the hardware fifoplayer just to be sure.

@JMC47
Copy link
Contributor

JMC47 commented Oct 23, 2014

Here you guys go. The Polygons are glitchy on Dolphin too; dunno what happened with the fifo-recording but it happens. Either way, this was taken from my fifo I sent @kayru.

hardwaresoniccolors

@Linktothepast
Copy link
Contributor

Tried F-ZERO GX, Eternal Darkness, SONIC COLOURS, Sonic Unleashed, Mario & Sonic at the Olympic Winter Games, Sonic and the Black Knight with all hw backends and they are fixed! Nice work there kayru. I found something for your previous pr though that makes things consistent between opengl and d3d, now it seems d3d to have inherited all d3d9 issues as well of the past... Custom Robo, Samba De Amigo, MARIO SPORTS MIX and FINAL FANTASY FABLES: Chocobo's Dungeon all behave like d3d9 behaved in the past, opengl is fine with those titles.

@JMC47
Copy link
Contributor

JMC47 commented Oct 23, 2014

Well, what's wrong with those games? Obviously it has nothing to do with this and rather the clipping, but I'd still like to know

I looked into it a bit; it's probably because the no-clipping bug is gone, that's all. Now we're finding more specific bugs.

@neobrain
Copy link
Member

+1 if we get a hwtest which shows that this does not cause any issues :)

@neobrain
Copy link
Member

Also uh, might be good to post your findings about the "expected" z/w values being off by FLT_EPSILON compared to those which actually are used by the game.

@kayru
Copy link
Contributor Author

kayru commented Oct 27, 2014

Is there any reason to not merge this?

@neobrain
Copy link
Member

@kayru See my second-last comment.

@JMC47
Copy link
Contributor

JMC47 commented Oct 27, 2014

The argument against merging it is simply that we don't know if this is an accurate solution. Merging it would probably make someone less apt to write a Hardware test for the behavior, I'm guessing neobrain would argue. We also don't want to end up having this reverted down the road due to finding out it's wrong, like the previous Sonic Unleashed Hack was.

The argument for merging it is that there are a ton of games broken in both backends now, and this fixes them without introducing any (known) regressions. We also have people using old/outdated/hacked builds because the games they play are broken in the latest master builds.

I can see both sides, but I sort of favor merging this based on the testing I did do. The fact is that the fifolog when played on hardware works fine... we just don't know for sure this is why, I guess.

It will come down to which side has more support unless a HW test shows it doesn't cause any problems.

@neobrain
Copy link
Member

My main concern about a missing hwtest is that there is absolutely no way for other contributors to comprehend in any way why this change would be necessary and to judge whether it is a good solution or not. What's more, it introduces doubt when working with the code base, because there's a chance future chances will break this hack in some way which might not be obvious.

A hwtest serves both a robustness and a documentary purpose here. And I'm pretty certain that a well-written test would help catch earlier a more generic range of potential bugs which we might introduce in the future.

@lioncash
Copy link
Member

lioncash commented Nov 3, 2014

For what it's worth, I'm in favor of the hardware test, since it's a more bulletproof aid in regards to testing.

@kayru
Copy link
Contributor Author

kayru commented Nov 3, 2014

What should the hwtest for this actually do?

@JMC47
Copy link
Contributor

JMC47 commented Nov 3, 2014

I was wondering this myself, as well.

@neobrain
Copy link
Member

neobrain commented Nov 3, 2014

@kayru Given your extensive debugging I was thinking you would be the best to come up with that :)

At the bare minimum it should copy the behavior of the games this PR fixes though, I guess.

@kayru
Copy link
Contributor Author

kayru commented Nov 3, 2014

@neobrain yeah, copying the specific game cases is the only thing I could think of so far. I have it implemented, so will do a hwtest pr soon.

@neobrain
Copy link
Member

neobrain commented Nov 3, 2014

@kayru Would be nice if you included a variation of the description text of this PR in the test, ideally with an explicit calculation if that's feasible.

@Buddybenj
Copy link
Contributor

Bump. Anything stopping this from being merged?

skidau added a commit that referenced this pull request Dec 3, 2014
Added projection matrix epsilon that fixes depth clipping issues in some games
@skidau skidau merged commit bfc62d2 into dolphin-emu:master Dec 3, 2014
@PatrickFerry
Copy link
Contributor

Yes, it's merged.
untitled

@JMC47
Copy link
Contributor

JMC47 commented Dec 4, 2014

This causes a regression in Metal Gear Solid: Twin Snakes. Damn. Main menu gets all messed up.

@kayru
Copy link
Contributor Author

kayru commented Dec 4, 2014

I shall investigate mgs issue!

@JMC47
Copy link
Contributor

JMC47 commented Dec 4, 2014

It may be a CPU OR GPU problem. delroth suggested it could be a CPU bug in IRC, that Dolphin's CPU could be feeding the GPU bad information and flipper on console can just handle it somehow. Thus, to continue my testing, I tested a fifolog of the MGS issue on console. The menu is unfortunately some kind of framebuffer effect, and it's very, very glitchy and broken on console fifoplayer. The suggestion that maybe Flipper can just handle the inaccuracy fed to it by the CPU is a possibility, though I don't know how realistic you would think it is. I felt it had to be mentioned.

So assuming I'm seeing the right thing, with a Float Epsilon, both work on console, without the float epsilon, both work on console.

@JMC47
Copy link
Contributor

JMC47 commented Dec 4, 2014

Maybe I'm totally over-thinking it as well. Please take my guesses with a grain of salt, my brain is beyond friend on this issue; I just wanted it dead!

Edit: Went in game, and nothing 3D renders, the minimap is broken, bad things happen. Changing IR gets some of the graphics to show up for one frame. Continuing after that causes some of the trippiest stuff ever. It's great and horrifying at the same time.

@Linktothepast
Copy link
Contributor

It's like a framebuffer effect that doesn't get cleared, and it's not just the menu, happens ingame too. Basically i managed to momentarily fix it ingame by exiting fullscreen for example.

@JMC47
Copy link
Contributor

JMC47 commented Dec 4, 2014

Starfox Assault's loading screens are broken as well.

@JMC47
Copy link
Contributor

JMC47 commented Dec 4, 2014

Sonic Heroes is another one; same problem as MGS:TS

@JMC47
Copy link
Contributor

JMC47 commented Dec 5, 2014

In case you need more games to test on, it appears that MegaMan Network Transmission has no graphics at all. If you don't need more examples and you'd rather me not post 'em let me know.

@kayru
Copy link
Contributor Author

kayru commented Dec 5, 2014

Well, the good news is that you should be able to just undo dc08de0 to fix (all?) issues.

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