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

VU0: fix for Street Fighter EX3 and R: Racing Evolution #1835

Merged
merged 1 commit into from Feb 23, 2017

Conversation

Projects
None yet
10 participants
@volodymyrkutsenko
Contributor

volodymyrkutsenko commented Feb 23, 2017

Hi guys!

Remember the Street Fighter EX3 clothing issue #954 and this thread http://forums.pcsx2.net/Thread-R-Racing-Evolution-invisible-cars describing the lack of the cars on the race track in R Racing Evolution?
Well, the dark days for these games seem finally to be over as there is a fix for them!

sfex3
rri

In short, there is something peculiar about the CFC2 instruction (VU0 Macro) when it copies the value from the TPC register.
These two mentioned games use the code like below for running the VU0 micro subroutine:

...
cfc2 t4, TPC
ctc2 t4, CMSAR0
callmsr
...

The official VU coding manual requires the value in the CMSAR0 to be divided by 8 before running the VCALLMSR instruction.
Apparently CFC2 divides the value by 8 itself because all the other games I have found also using the VCALLMSR instruction (24: The Game, GTA LCS, GTA VCS and FFXII) indeed divide the value by 8 in code before running the micro subroutine:

(From the FFXII game)

...
lw v0, 0x14(v1)
sra v0, 0x03
ctc2 v0, CMSAR0
...
callmsr
...

The only difference is that they never fetch the value from the TPC directly.

I have also run some homebrew tests on my PS2 and discovered that the CFC2 instruction really ends up with the value already divided by 8.
There are basically two possibilities: either CFC2 also divides by 8 while fetching from the TPC register or the TPC register always works with the values already divided by 8.
The latter seems unlikely because TPC is the Program Counter register that should hold the memory address of the instruction.
Sadly there seems to be no way to see what is really inside the TPC register before fetching from it by CFC2 because CFC2 is the only way to retrieve its value (so says the manual).
In any case PCSX2 already implements TPC as the instruction address pointer so sticking to CFC2 dividing by 8 looks like the correct way for the fix.

(I left a descriptive comment directly in code describing the same, fixed for both the EE Interpreter and the Recompiler)

It was possible for me to test some games apart from all of the mentioned above and there seem to be no regression.
However this needs to be tested on a bigger amount of games as obviously the impact may be pretty significant.
Who knows, it might even fix some issues related to VU0 in other games! :)

Please take a look at your earliest convenience.

VU0: added a special case to the CFC2 instruction if it copies the value
from the TPC register (fixes Street Fighter EX3 #954 and R Racing
Evolution the invisible cars issue)
@MarcoEstevez

This comment has been minimized.

MarcoEstevez commented Feb 23, 2017

I cant believe it just happen today, Thanks

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Feb 23, 2017

Good job.

Disclaimer I don't know anything on VU. But from an hardware point of view, if the TPC is always aligned on 8B, you don't need register for the 3 lsb. Otherwise a division by 8 is free in the HW. So it makes sense 👍

@FlatOutPS2 FlatOutPS2 changed the title from VU0: fix for Street Fighter EX3 and Ridge Racing Evolution to VU0: fix for Street Fighter EX3 and R: Racing Evolution Feb 23, 2017

@tabnk

This comment has been minimized.

tabnk commented Feb 23, 2017

Waiting a long time for this fix. Street Fighter EX3 is near perfect :)

@FlatOutPS2

This comment has been minimized.

Member

FlatOutPS2 commented Feb 23, 2017

I can confirm it fixes R: Racing, but it does have a performance impact. Not caused by the fix, it's just that the cars slow down OpenGL Hardware(required for best results) with AMD drivers. :p

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Feb 23, 2017

Nice job! That does all make sense, certainly something simple that got completely overlooked. I wonder if it fixes the Superman game with the insane SPS as well. But I'm happy with this :)

@refractionpcsx2 refractionpcsx2 merged commit 489a89a into PCSX2:master Feb 23, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@FlatOutPS2

This comment has been minimized.

Member

FlatOutPS2 commented Feb 23, 2017

It doesn't fix the missing characters in My Street though.

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Feb 23, 2017

No I know what causes that, it is the T bit/macro interlocking. The same thing with Mike Tysons Boxing. I did managed to hack it to a point My Street looked mostly OK and the animation worked in Mike Tyson (although textures were a black mess, but it was messy and did cause some other issues.

Here's some screenshots :P

mystreet
miketyson

@sudonim1

This comment has been minimized.

Contributor

sudonim1 commented Feb 23, 2017

Fun fact: the cfc2, ctc2, vcallmsr sequence is precisely what the manual says to do (in 7.3.1), although it doesn't outright provide an assembly listing for it.

@Nobbs66

This comment has been minimized.

Contributor

Nobbs66 commented Feb 23, 2017

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Feb 23, 2017

Wonder if this will fix Sega superstars tennis.

It might do

@FlatOutPS2

This comment has been minimized.

Member

FlatOutPS2 commented Feb 23, 2017

The Sega Superstarts Tennis issue reminded me of the Amplitude character issue. But the Amplitude issue is unaffected by this PR.

@Nobbs66

This comment has been minimized.

Contributor

Nobbs66 commented Feb 23, 2017

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Feb 23, 2017

Damn, didn't fix Superman Returns, oh well nevermind! It fixed 2 games which needed fixing so good job :)

@volodymyrkutsenko

This comment has been minimized.

Contributor

volodymyrkutsenko commented Feb 23, 2017

Surprisingly enough I looked at Superman Returns too while searching for the games that use VCALLMSR. But the game uses VCALLMS instead to run the vu0 micro subroutines so sadly it's unrelated to this particular fix.
Maybe some other time I'll get to it too :)

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Feb 23, 2017

Indeed, not sure what causes the SPS on that, it is the same across all renderers, which is kinda good i guess??? But it's a bit of a mystery for now, could be a COP2/FPU problem, but I'm just guessing.

If you do work it out, you get bonus internet points for being the first to solve it :P

@awichen

This comment has been minimized.

awichen commented Apr 11, 2017

I got a problem...

When Guile use "Sonic Boom Typhoon", there's still something wrong
If i choose Renderer "Hardware", i can't see the tornado.
sonic2

If i choose Renderer 'Software", i can see it...
sonic1

Does anyone know how to fix it??

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Apr 12, 2017

It isn't the subject of this PR. Otherwise it won't work neither in SW. No clue what happen here but HW renderer is far from perfect.

@ssakash ssakash added this to the Release 1.6 milestone May 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment