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

Fix no text in Phantasy Star Series #7912

Closed
wants to merge 1 commit into from

Conversation

sum2012
Copy link
Collaborator

@sum2012 sum2012 commented Aug 19, 2015

I remember someone write good code but performance decrease many in God Eater
So that we must make code like Dangan Ronpa hack.
Fix #3777 , #7662

@hrydgard
Copy link
Owner

So, from the start we've been very reluctant to do hacky workarounds like this because once you do it, pressure is off to find a "proper" fix, and also it can hide issues that might affect more games.

But, we already do function patching for framebuffer copies and as main development of the PSP emulation is pretty much "done", I think introducing this kind of game-specific fixes for the worst edge cases (especially those that rely on subtle GPU rendering differences) makes more sense now than it has before.

However, I'd like to do it in a more systematic way. We should detect these game IDs at game startup and set a flag, possibly driven by .INI files, and not do the string comparisons in the draw state code.

I will be back next week, and I'll think of something.

@sum2012
Copy link
Collaborator Author

sum2012 commented Aug 19, 2015

OK ,I have to wait :)

@thedax
Copy link
Collaborator

thedax commented Aug 19, 2015

Instead of resorting to a hack, we should really verify if the PSP's transform unit indeed only works on 24-bit values, as @ikoul's comment suggested in #3777.

@hrydgard
Copy link
Owner

The PSP has a 16-bit Z buffer and some chips we run on only support 24-bit Z, and we use 24-bit Z where available anyway. So it's possible that we might not be able to solve all cases completely - like, depths could round to the same value on the PSP and not on some chips we run on - which could cause z-fighting issues like this, regardless of if the transform unit only uses 24-bit floating point.

@unknownbrackets
Copy link
Collaborator

I really feel like these depth issues are affecting a bunch of games, and I don't like the idea of hacking just these games and ignoring the other affected games.

It's not a situation where dividing by another number will "always fix it", so it's not even as if we can assume all these other games will be improved by this hack.

-[Unknown]

@hrydgard
Copy link
Owner

Do we have a list of games that are affected by depth precision issues?

For things where we don't really expect to have a full solution soon, this type of hack is not really that much worse than the Dangan Ronpa fix or the other block copy patches... and if properly isolated, can be easily removed once we have a real solution. In the meantime, this makes Phantasy Star quite playable.

While I'm not really happy with adding hacks like this, the alternative is likely to be patched versions of the emulator floating around forums which is not really much better...

@unknownbrackets
Copy link
Collaborator

We just have #6660, not depth precision specifically. I know Phantom Brave for sure has what looks like a very similar issue, although it's not as prevalent.

I feel like the Danganronpa hack (+ block transfer behavior) represents something with no other firm solution that is acceptably performant. Segfaults may work but may also reduce performance overall, even when games don't use these features. Plus, although it's a bit ugly and direct, at least it makes a clean attempt at simulating the exact correct behavior.

In this case, dividing by the other number is just another way of being very incorrect that happens to work. IIRC, it's because through and non-through end up scaling Z values differently so the value happens to fall into a range that works. It doesn't more closely approximate behavior, it's just an ugly wrong hack that happens to work.

The Danganronpa hack is not an ugly wrong hack that just happens to work by doing things another wrong way. The block transfer hacks are not just ugly wrong hacks that happen to work by doing things other wrong ways.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Aug 23, 2015

There's also #6485 possibly having similar problem to PSP1, althrough not affected by any of the few existing hackish solutions.

I think giving a green light for game specific hacks will be bad in longer period of time even if it makes lots of people happy on the start. Hacks are easier to make and not affect performance much, so people will concentrate on them instead figuring out proper solutions which in the long run means less popular games might never get fixed.:| Already happened in lots of older emulators.

@hrydgard
Copy link
Owner

Okay, fair enough. A slightly different scale factor was what fixed this issue in Dolphin though, but of course for some reason (as discussed, possibly transform calculation precision) it doesn't seem to fully solve the problem for us, and there it obviously was global, not per game.

There's another interesting possible use for per-game fixed "hack flags" though - enabling "dangerous" recompiler optimizations, like having the regalloc obey the ABI rules for what registers to keep on a RET which we know won't work on all games but will work on some. So it might still make sense to eventually add such a mechanism, even if I, thinking some more about it, mostly agree that this particular use is a step too far...

@unknownbrackets
Copy link
Collaborator

Well, I'm not saying I can't be wrong... but so far all my testing shows that depth values are strictly between 0 and 65535 and I've measured the way they round and scale in different modes. In fact that effort fixed bugs in some games. So I don't think transform mode is scaled this way.

-[Unknown]

@hrydgard
Copy link
Owner

Closing in favour of #7920

@hrydgard hrydgard closed this Aug 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants