hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.
https://www.ppsspp.org
Other
10.8k stars 2.12k forks source link

Black Rock Shooter speed and overbright issues #3680

Closed andutrache closed 10 years ago

andutrache commented 10 years ago

Using the latest v0.9.1-649-g515885e this game is now overbright with buffered rendering on. 0.9.1.502 good : http://postimg.org/image/lreynd9sh/ 0.9.1.649 bad : http://postimg.org/image/v6acqzigr/

Also there is this very annoying problem where the game starts playing in 2x speed in cutscenes and while walking around which started somewhere between 0.9.1-502 and 0.9.1-580. If you open the map the game resumes playing at normal speed until you change screens or about 3 minutes pass, the it goes fast forward again. Tried changing all settings to no avail nothing seems to fix it.

I would love to help you more but only 0.9.1-502(which works) and 0.9.1-580(which has the problem) are available for download. Apparently the buildbot was snoozing between these 2 revisions.

unknownbrackets commented 10 years ago

What does the FPS show when it runs too fast?

The bloom is a common problem in many games. I think it's due to stencil being enabled.

Additional builds should show up in that range within like an hour.

-[Unknown]

andutrache commented 10 years ago

Fps is always 100% and by that i refer to Speed: 100% being displayed on the corner at all times.

andutrache commented 10 years ago

Later edit: i set the fps display to Both in settings and now it shows 60/60 (100%) when its running fast and 30/30 (100%) when its running normally.

solarmystic commented 10 years ago

I can confirm every single one of @andutrache's findings.

Via bisecting:-

Issue 1:- The double speed issue

The responsible commit/build for the double speed issue is v0.9.1-505-ga8f100c https://github.com/hrydgard/ppsspp/commit/a8f100c94f5be47d1887d7d52e84262584f97001

505

Last one not to have it is v0.9.1-504-g7f1a615 https://github.com/hrydgard/ppsspp/commit/7f1a6154b20b2246e4f9150a355afca05b123c58

504

Issue 2:- The overbright/excessive bloom issue

The responsible commit/build for the overbright/excessive bloom issue with Buffered Rendering ON is v0.9.1-610-g2e8b475 https://github.com/hrydgard/ppsspp/commit/2e8b475789cff8eca7439765e912fc12d6f318e7

610

Last one not to have it with Buffered Rendering ON is v0.9.1-609-g3b323c4 https://github.com/hrydgard/ppsspp/commit/3b323c41765565c586171c4e4fbb1e0993382d11

609

@unknownbrackets

It shows 60/60 compared to the previously normal 30/30. But % wise 100% is being shown all the time.

Which is effectlvely double speed in this game, since it's supposed to run at 30 FPS internally, not 60 FPS.

Or in the older notation, Speed:60, FPS:60 as compared to Speed:60. FPS:30

unknownbrackets commented 10 years ago

So to confirm, if you comment out this line:

__KernelRegisterWaitTypeFuncs(WAITTYPE_VBLANK, __DisplayVblankBeginCallback, __DisplayVblankEndCallback); 

It goes away? The brightness issue like I said is probably bloom, most likely it was just missing the framebuffer/texture before by accident.

-[Unknown]

andutrache commented 10 years ago

after checking on my PSP, i can confirm that the game does indeed use bloom but not in the same way, it appears here everything is bloomed and is overbright because of it.

solarmystic commented 10 years ago

@unknownbrackets

Commenting out the aforementioned line as shown below, fixes the double speed issue:- capture

capture1

Bloom is still pretty much prevalent in that picture.

unknownbrackets commented 10 years ago

I think the "everything bloomed" thing is a stencil or depth handling issue. The fact that it's there is actually more correct than it being not rendered at all, even if it's not ideal.

Does it affect the speed at all if you remove these lines from Core/HLE/sceKernelThread.cpp?


    if (call->cbId != 0)
        g_inCbCount++;

...

    if (call->cbId != 0)
        g_inCbCount--;

It's a bit more involved than that, but I'm already preparing the changes necessary to do that correctly.

-[Unknown]

solarmystic commented 10 years ago

@unknownbrackets Just those 4 lines? Or everything else in between,including those 4 lines?

Removing just the 4 lines (by commenting them out or deleting them) does nothing to solve the double speed issue. (still getting 60/60)

Only commenting out that line earlier on https://github.com/hrydgard/ppsspp/issues/3680#issuecomment-24012943 helps.

capture capture1

capture

unknownbrackets commented 10 years ago

Can you export the map file? I'm wondering which vblank wait function it's using.

-[Unknown]

solarmystic commented 10 years ago

@unknownbrackets

In game map file from the latest 64bit release build based on v0.9.1-657-g865057c https://github.com/hrydgard/ppsspp/commit/865057c8b28c2dd85bc747aac7677d53dbbb99e2

https://gist.github.com/solarmystic/6481537

Direct download:-

http://www.mediafire.com/?8d2r4u705p87dvd

Hope I did it correctly, do tell me if there's something missing @unknownbrackets

andutrache commented 10 years ago

The intro of GTA Vice City Stories is also afected by the double-speed issue.

andutrache commented 10 years ago

well its v0.9.1-1114-g36fd5df which still has the problem revert the code to what works already.

Also GTA Vice City Stories now crashes on the intro where you talk to Jerry, but i think i saw an issue on this already.

raven02 commented 10 years ago

@andutrache , i think the code revert you mean this ?

__KernelRegisterWaitTypeFuncs(WAITTYPE_VBLANK, DisplayVblankBeginCallback, DisplayVblankEndCallback);

solarmystic commented 10 years ago

@andutrache

Yep, the crash on intro is common to GTA Vice CIty Stories and GTA Chinatown Wars in the latest revisions is due to the atrac3+ decoder. The issue report is here https://github.com/hrydgard/ppsspp/issues/3805 and the comment for VCS is here https://github.com/hrydgard/ppsspp/issues/3805#issuecomment-24659414

Play the game(s) without atrac3+ audio and it will not crash.

andutrache commented 10 years ago

@raven: yes that thing, the game worked before so if a change broke it then its not a good change, or does the doublespeed problem lie elsewhere?

andutrache commented 10 years ago

@solarmystic: yes i know disabling atrac3+ audio stops the crashes, but playing without sound sucks. Don't get me wrong, i have a PSP and if i want everything to work 100% ill play on but that won't help the emu get better, that's why i am posting here in the first place.

Also it usually pains me to see that games that were working before get broken, i understand that can't be avoided since it's a work in progress, if there are other reasons to keep that line of code as it is and the problem lies elsewhere at least say so, then ill understand that the problem lies elsewhere and will be fixed in the future (as were a lot of problems i had before).

daniel229 commented 10 years ago

Last Ranker has this speed issue too.

solarmystic commented 10 years ago

@andutrache

You're preaching to the choir mate, why do you think I'm responding to your issue report https://github.com/hrydgard/ppsspp/issues/3680#issuecomment-24012272 and helping you narrow down the issue further? We're all here to help improve this emulator further. I just suggested playing without atrac3+ audio as a temporary measure.

Also, as you've already mentioned, growing pains are to be expected with a project like this. You can always stay at an older build if you wish to keep previously working functionality in specific games that got broken in recent builds until the issue is resolved again.

andutrache commented 10 years ago

So it came out as preaching anyway XD . Sorry i couldn't express what i thought any differently or else i would have came out as bad bitching user, i have seen too many cases of that on the internet that i am now trying to avoid that like the plague.

I never said i am not thankful for your help, your help is appreciated and from what i have seen you do a 10x better job at reporting than me.

I also thought this issue was gonna be left out and no one will look at it and that's mostly the reason why i posted. Will wait for a fix :) .

raven02 commented 10 years ago

Last Ranker double speed issue seems to be be fixed

hrydgard commented 10 years ago

Is the overbrightness still a problem?

raven02 commented 10 years ago

@hrydgard , I didn't see any over brightness in-game.

@unknownbrackets , looks like the double speed issue because of this line in __DisplayVblankBeginCallback

if (vblankPausedWaits.find(pauseKey) != vblankPausedWaits.end()) {
    return;
}
unknownbrackets commented 10 years ago

@raven02, so that means it's got two in a row? The problem is handling the case that it waits inside a callback.

Can you do a log including the sceDisplayWaitVblankCB messages?

-[Unknown]

solarmystic commented 10 years ago

@hrydgard

The same commit https://github.com/hrydgard/ppsspp/commit/33efffe7594bb4a7e4e5e63382152a7e9984ba1c that heavily reduced the overbrightness in Gods Eater Burst and Wipeout also eliminated the overbrightness in Black Rock Shooter. Nicely done.

andutrache commented 10 years ago

reporting with the v0.9.5-848-g726327b : overbright issues fixed, speed issue still not fixed.

Keep at it.

hrydgard commented 10 years ago

@solarmystic , excellent, thanks for testing.

raven02 commented 10 years ago

@unknownbrackets , I logged the sceDisplayWaitVblankCB , DisplayVblankBeginCallback and DisplayVblankEndCallback with following messages

17:15:561 idle0 I[HLE]: HLE\sceDisplay.cpp:288 sceDisplayWaitVblankCB: Suspending vblank wait for callback 17:15:561 main I[HLE]: HLE\sceDisplay.cpp:308 sceDisplayWaitVblankCB: Resuming vblank wait from callback 17:15:561 main I[HLE]: HLE\sceDisplay.cpp:308 sceDisplayWaitVblankCB: Resuming vblank wait from callback 17:15:561 main I[HLE]: HLE\sceDisplay.cpp:308 sceDisplayWaitVblankCB: Resuming vblank wait from callback 17:15:563 main I[HLE]: HLE\sceDisplay.cpp:308 sceDisplayWaitVblankCB: Resuming vblank wait from callback

raven02 commented 10 years ago

If i remove the following , it is running in this way (one suspend and one resume consectively)

// This means two callbacks in a row.  PSP crashes if the same callback waits inside itself (may need more testing.)
// TODO: Handle this better?
if (vblankPausedWaits.find(pauseKey) != vblankPausedWaits.end()) {
    return;
}

22:17:588 idle0 I[HLE]: HLE\sceDisplay.cpp:286 sceDisplayWaitVblankCB: Suspending vblank wait for callback 22:17:588 main I[HLE]: HLE\sceDisplay.cpp:306 sceDisplayWaitVblankCB: Resuming vblank wait from callback 22:17:588 idle0 I[HLE]: HLE\sceDisplay.cpp:286 sceDisplayWaitVblankCB: Suspending vblank wait for callback 22:17:589 main I[HLE]: HLE\sceDisplay.cpp:306 sceDisplayWaitVblankCB: Resuming vblank wait from callback 22:17:589 idle0 I[HLE]: HLE\sceDisplay.cpp:286 sceDisplayWaitVblankCB: Suspending vblank wait for callback 22:17:589 main I[HLE]: HLE\sceDisplay.cpp:306 sceDisplayWaitVblankCB: Resuming vblank wait from callback

unknownbrackets commented 10 years ago

I can't remember if I specifically tested waiting a vblank within a vblank wait or not.

Anyway, this might overwrite the wait incorrectly, especially if two callbacks are called during a vblank wait. Is it actually calling sceDisplayWaitVblankCB in between the suspend and resume? Or Multi or anything?

There's no demo or anything of this game, is there?

-[Unknown]

thedax commented 10 years ago

A quick Google search didn't yield a demo, so I doubt it.

unknownbrackets commented 10 years ago

Hmm, there's definitely something broken here, I'll be able to look at it more tomorrow. It seems like vblank waits inside of callbacks running during a vblank wait should actually work, and aren't.

My quick 5-minute test:

#include <common.h>
#include <pspgu.h>
#include <psprtc.h>
#include <pspdisplay.h>
#include <pspthreadman.h>

extern "C" {
int sceDisplayWaitVblankStartMulti(int vblanks);
int sceDisplayWaitVblankStartMultiCB(int vblanks);
int sceDisplayIsVblank();
int sceDisplayIsVsync();
}

SceUID cb;
int vbase;
bool stop = false;

int cbFunc(int arg1, int arg2, void *common) {
    checkpoint("inside before vcount=%d", sceDisplayGetVcount() - vbase);
    checkpoint("inside: %08x", sceDisplayWaitVblankStartMultiCB(1));
    checkpoint("inside after vcount=%d", sceDisplayGetVcount() - vbase);
    checkpoint("cbFunc");
    return 0;
}

int threadFunc(SceSize args, void *argp) {
    checkpoint("threadFunc start");
    cb = sceKernelCreateCallback("cb", cbFunc, NULL);
    while (!stop) {
        checkpoint("before vcount=%d", sceDisplayGetVcount() - vbase);
        sceDisplayWaitVblankStartMultiCB(5);
        checkpoint("after vcount=%d", sceDisplayGetVcount() - vbase);
    }
    checkpoint("threadFunc done");
    return 0;
}

extern "C" int main(int argc, char *argv[]) {
    vbase = sceDisplayGetVcount();
    SceUID threadID = sceKernelCreateThread("test", &threadFunc, 0x1F, 0x2000, 0, NULL);
    sceKernelStartThread(threadID, 0, NULL);

    sceKernelDelayThread(300000);
    sceKernelNotifyCallback(cb, 1);
    sceKernelDelayThread(300000);
    stop = true;
    sceKernelDelayThread(300000);

    return 0;
}

-[Unknown]

raven02 commented 10 years ago

@unknownbrackets , it did use sceDisplayWaitVblankStartMultiCB() as well.

31:56:038 main I[HLE]: HLE\sceDisplay.cpp:764 sceDisplayWaitVblankStartMultiCB(2) 31:56:040 main I[HLE]: HLE\sceDisplay.cpp:764 sceDisplayWaitVblankStartMultiCB(2) 31:56:042 main I[HLE]: HLE\sceDisplay.cpp:764 sceDisplayWaitVblankStartMultiCB(2) 31:56:044 main I[HLE]: HLE\sceDisplay.cpp:764 sceDisplayWaitVblankStartMultiCB(2)

unknownbrackets commented 10 years ago

Oh right, that's just the problem where it doesn't wait inside callbacks still.

Anyway, d'oh, I'm super dumb, can't believe I didn't see the missing line...

-[Unknown]

unknownbrackets commented 10 years ago

Does that change help at all? It's definitely right, even if it doesn't solve this problem.

-[Unknown]

raven02 commented 10 years ago

Thanks @unknownbrackets .I will test it out tonight .

solarmystic commented 10 years ago

@unknownbrackets

https://github.com/hrydgard/ppsspp/commit/160ee740d23a4c91deae85f9221162fb38e088c7 did the trick and resolved the 60 FPS issue. The game now correctly runs at 30 FPS in the overworld, as it should.

Nicely done.

capture

Now if we can get the @andutrache or someone else in here to verify my findings and confirm them, we can close this issue and chalk it up as a case closed, since the overbright issue has also been resolved.

hrydgard commented 10 years ago

Great!

I'm gonna close this then.

raven02 commented 10 years ago

Thanks all !

andutrache commented 10 years ago

Tested v0.9.5-933-g4cd57b6, the double speed bug has been fixed.

Good Job :) .