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

Introduce "Compatibility Flags". #8006

Merged
merged 2 commits into from
Sep 27, 2015
Merged

Introduce "Compatibility Flags". #8006

merged 2 commits into from
Sep 27, 2015

Conversation

hrydgard
Copy link
Owner

Compatibility fixes are controlled by assets/compat.ini.

Alternatively, if PSP/SYSTEM/compat.ini exists, it is merged on top, to enable editing the file on Android for tests.

This file is not meant to be user-editable, although is kept as a separate ini file instead of compiled into the code for debugging purposes.

The uses cases are strict:

  • Enable fixes for things we can't reasonably emulate without completely ruining
    performance for other games, such as the screen copies in Dangan Ronpa
  • Disabling accuracy features like 16-bit depth rounding, when we can't seem to
    implement them at all in a 100% compatible way
  • Emergency game-specific compatibility fixes before releases, such as the GTA
    music problem where every attempted fix has reduced compatibility with other games
  • Enable "unsafe" performance optimizations that some games can tolerate and
    others cannot. We do not currently have any of those.

This functionality should NOT be used for any of the following:

  • Cheats
  • Fun hacks, like enlarged heads or whatever
  • Fixing general compatibility issues. First try to find a general solution. Try hard.

We already have the Action Replay-based cheat system for such use cases.

Should help #8004, by disabling depth rounding in Fight Night round 3.

@unknownbrackets
Copy link
Collaborator

Hmm. Not sure how I feel about this.

-[Unknown]

@@ -96,6 +96,7 @@ set(SRCS
Util/PPGeDraw.cpp
CPU.cpp
CoreTiming.cpp
Compatibility.cpp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant to add this to /CMakeLists.txt. Can we delete Core/CMakeLists.txt?

-[Unknown]

@hrydgard
Copy link
Owner Author

I'm also not sure how I feel about this, but I see no other way out anymore for the depth stuff, I've done what I can.

@unknownbrackets
Copy link
Collaborator

Well, yeah, same here. Something might be possible on modern GL, like using a buffer for depth or etc., with probably significant perf impact... but it's true that depth is a hard problem...

-[Unknown]

These should be used very restrictively, see comment in Compatibility.h.

Should help #8004, by disabling depth rounding in Fight Night round 3.
@hrydgard
Copy link
Owner Author

I think this is ready for merge now. @unknownbrackets , if you think it's alright, I'm going to try to add one more compat setting to revert sceAtrac's behaviour to something that GTA is happy with and then push out 1.1 fairly soon with that. Hopefully we'll find a better fix later, but I really want to get this release out, given that it contains major stuff like Android TV and 64-bit ARM support.

@unknownbrackets
Copy link
Collaborator

Yeah, fair enough. Hopefully we can fix it better later. Really need an example of its syscall usage...

-[Unknown]

hrydgard added a commit that referenced this pull request Sep 27, 2015
Introduce "Compatibility Flags".
@hrydgard hrydgard merged commit 9b27ffb into master Sep 27, 2015
@hrydgard hrydgard deleted the compat-flags branch September 27, 2015 08:45
@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 8, 2015

@hrydgard @unknownbrackets do you guys think we could add those 2 lines from 4561440#commitcomment-6889647 as another compatibility flag?

I wouldn't really ask about this, but MechaBouncer on the forum in Armored Core game thread made a valid point that we knew what change broke those games from over a year ago.
It's similar situation, but in opposite to GTA hack which also implements old/broken code as a hack, this one is just a small change that wouldn't really block any code improvements. It also fixes affected series of games completely not just decreases their brokeness as apparently GTA hack is doing ~ see #7863 (comment)

I can add the new compatibility flag and open a pull request if you accept it, if not, no big deal, I didn't really posted on the forum that I'll ask about it so no pressure, nobody will feel disappointed.

@unknownbrackets
Copy link
Collaborator

The problem with GTA turns out to be that it's managing the data incorrectly. Recent changes made parts of the code manage data more correctly, but that breaks GTA because it depends on other parts managing the data incorrectly the same way (#4483 is a great example of this.)

I'm hoping the GTA thing can be removed very soon. It probably loops incorrectly and etc.

I'm fairly sure that the change in the commit you mentioned is not correct. I'm not really a fan of squirreling away whatever actual bug Armored Core has.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 8, 2015

Yeah I know it's just a hack, it's just that on the user end it's just a compatibility regression lasting for over a year and nobody was able to find real culprit or even a clue about it.

I tried to find out about PSP_SIGNAL_HANDLER_PAUSE which the hack changes to make a CWCheat workaround for the affected game I have at least(only Bleach), but can't find any info about it, I only found sceGuSignal on pspsdk which lists only 2 behaviours:

void    sceGuSignal (int signal, int behavior)
        Trigger signal to call code from the command stream.
Available behaviors are:
GU_BEHAVIOR_SUSPEND - Stops display list execution until callback function finished
GU_BEHAVIOR_CONTINUE - Do not stop display list execution during callback
Parameters
signal  - Signal to trigger
behavior    - Behavior type

is this http://psp.jim.sh/pspsdk-doc/index.html SDK incomplete or PPSSPP added something which should not exist for example on older firmware?

@unknownbrackets
Copy link
Collaborator

Yeah that documentation is very outdated. I think they added more handlers in later firmware versions, and also slightly changed the behavior... I think it was in 2.00?

Basically PAUSE just tells the list to keep going until FINISH, at which point it pauses. The essential process can be seen here:

https://github.com/hrydgard/pspautotests/blob/master/tests/gpu/signals/pause.cpp
https://github.com/hrydgard/pspautotests/blob/master/tests/gpu/signals/pause.expected

As you can see:

  • Pause acts the same on old and new firmware versions.
  • Pause does not call the signal callback immediately.
  • Pause doesn't actually pause right away; it pauses at the next FINISH command.
  • The signal callback is called when it pauses.
  • sceGeContinue() resumes the list and lets it complete.
  • Popularly, the signal callback may sceGeContinue(), and this must work to continue the list immediately. Many games that use PAUSE do this.

I guess it might be possible to try adding an sceUpdateStallAddr call here:
https://github.com/hrydgard/pspautotests/blob/master/tests/gpu/signals/pause.cpp#L61

If it unpauses it before Continue, then the previous behavior is correct. I think this might be covered by a separate test already, but might be clearer to add to this one or create a pause2 or something.

Anyway, at this point based on all my testing, my thinking of sceUpdateStallAddr is that it really just updates the stall address and doesn't do any other status monkeying. Someone told me once that its code looked like it was handcoded asm in ofw, since it is very important to preformance in some games.

Most likely, what's happening in Armored Core is that something ELSE is failing to sceGeContinue, or is doing so too early, or isn't getting scheduled, or whatever... and so this hack makes it work. From the perspective of correctness, it's just a little landmine reset switch that magically unpauses lists at unexpected but sometimes not that problematic times.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Oct 8, 2015

Given how games use it, I agree that sceUpdateStallAddr really just sets the address at which the GPU will stop reading, and nothing else. For example there are no guarantees about exactly where the GPU will be afterwards, except not having passed that address.

We incorrectly "rush" the GPU forwards to the stall address in single core mode, practically all games are quite happy with that though, and as the GPU has a really fast command reader, it's probably fairly close to what they see in practice except when the GPU is doing large draw calls.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 8, 2015

Thanks for lots of info:) will have something to digest(and will also know the sdk site is outdated;p). For now I checked what's happening right before the hang in Bleach game, it sets the signal to pause, then it calls sceGeContinue twice and finally hangs(only graphics) I assume that's the moment it get's the finish which triggers the pause.

I noticed also two things:

  • with activated hack that changes pause to suspend it doesn't even trigger those 2 continues,(so I guess they are for the pause, but trigger too quickly?)

  • with the above hack and multithreading activated, the game still sets pause and calls the 2 continues, not really sure what's happening with that, maybe it doesn't do the update stall where the hack is placed so guess that's another confirmation it's a dirty hack working accidently:].

    Thanks again, I might have a chance to do it via CWCheat hack now and maybe learn something more in the process, at worst would at least temporarily make a bunch of people happy.

@sum2012
Copy link
Collaborator

sum2012 commented Oct 8, 2015

Many China people use this hack (from I compiled ppsspp) to play
Bleach: Soul Carnival (1 and 2).
I had some search in jpcsp source in
#1782 (comment)
but still fail.Hope @LunaMoo Good luck

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 9, 2015

Well I spent a lot of time on this yesterday trying to understand how it works or rather why it doesn't work without much luck, althrough one thing kind of bothers me, is the FINISH directly after signal pause setting it?
bleach
Because if so, then sceGeContinue happens after this and yet it's still paused.

Either way at least I managed to make CWCheat workarounds for affected games and posted a guide how to get info I need to port it for versions of those games which I didn't had any access to, so hopefully no more ppsspp polluting hacks are required.

@unknownbrackets
Copy link
Collaborator

Hmm. So wait, it's like that - SIGNAL, FINISH, END? That's weird and seems like a bug in the game. I don't think I've ever tested that sequence - it should normally be SIGNAL, END, FINISH, END. It could definitely be that we handle that particular sequence differently than we should.

-[Unnown]

@unknownbrackets
Copy link
Collaborator

Oh, sorry, I misread - signal, end, finish, end, is what's there. Yes, that's when it should trigger the callback. Our debug output could be clearer...

-[Unknown]

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.

4 participants