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
11.36k stars 2.18k forks source link

Games that still need "Framebuffer to memory" #6261

Closed hrydgard closed 6 years ago

hrydgard commented 10 years ago

Our new blocktransfer support (Thanks to @unknownbrackets for fixing tons of issues!) and related memcpy overrides fix most of the games that previously needed the "Framebuffer to memory" modes.

Are there any games that still need them? Otherwise I would like to remove or at least hide them in developer options, as they are slow and often cause instability.

#2713 #4739 #7695 #6880 #6547

solarmystic commented 10 years ago

EDIT:- Trails in the Sky was working fine all along, I just had function replacements disabled.

solarmystic commented 10 years ago

EDIT:- Same thing goes for GEB, function replacements resolved the issue. Buffered + Simulate Block Transfer fixes the flickering now.

hrydgard commented 10 years ago

Hm, that save screen thingy is curious. Not really critical though, the Trails one is worse (but has worked previously?)

solarmystic commented 10 years ago

@hrydgard Afaik the Trails one has always required Read Framebuffers to Memory enabled to work properly in all prior revisions.

unknownbrackets commented 10 years ago

@solarmystic do you have function replacements disabled? Gods Eater Burst and Trails both work. They both require function replacements (default on - FuncReplacements = True.)

-[Unknown]

solarmystic commented 10 years ago

@unknownbrackets Ahh, my bad, that was the culprit for both games. Thanks for the heads up.

hrydgard commented 10 years ago

Thanks for clearing that up :)

daniel229 commented 10 years ago

Rockman Dash https://github.com/hrydgard/ppsspp/issues/4739 Maybe Danganrunpo still investigate wrong thing on mobile without Framebuffer to memory.I heard.

unknownbrackets commented 10 years ago

Hmm, we have a Danganronpa hack directly, it should work everywhere...

For Rockman DASH, I was hoping the framebuffer texture offset thing would fix it (#6259). Does it detect it as an offset?

-[Unknown]

daniel229 commented 10 years ago

Yes, Rockman DASH is fixed.

unknownbrackets commented 10 years ago

Nice. So it doesn't need "read framebuffers to memory" anymore, right?

-[Unknown]

daniel229 commented 10 years ago

No need it any more.

daniel229 commented 10 years ago

Sakura Taisen video still need "read framebuffers to memory" ,graphics are fixed just like Rockman DASH.

without "read framebuffers to memory". 04

unknownbrackets commented 10 years ago

Is it using sceMpeg or scePsmf? Basically I'm wanting to know where it writes the video to, and how that's supposed to make it to the display. A debug log would probably do the trick.

-[Unknown]

daniel229 commented 10 years ago

Debug log(rename jpg to rar) ppsspplog

daniel229 commented 10 years ago

the silent hill games might need it.

13

12

unknownbrackets commented 10 years ago

14:49:561 user_main D[HLE]: HLE\sceDmac.cpp:89 sceDmacMemcpy(dest=04000000, src=098be820, size=524288) 14:49:561 user_main W[G3D]: GLES\Framebuffer.cpp:2048 Memcpy fbo upload 098be820 -> 04000000

That memcpy is either 512x256 or 480x272, but it should still theoretically work.

Est: 44088000

It's got its FBOs at 44. Maybe there's still an issue matching framebuffers. If you press "step frame" during the video, where is it texturing from? I assume it's texturing from 04000000 to draw the video to 44128000 / 44088000. Those are at least 320 apart so there should be space...

Hmm, might need more breakpoints in the silent kill game to see what it's doing.

-[Unknown]

daniel229 commented 10 years ago

Right,it's 0x04000000 01

unknownbrackets commented 10 years ago

Hmm. I wonder if the FBO has the wrong format. It's not drawing to it.

Render to texture with different formats 3 != 1

I wonder if we should swap the format if it gets used for too long like that without being rendered to. What if you change:

WARN_LOG_REPORT_ONCE(diffFormat1, G3D, "Render to texture with different formats %d != %d", entry->format, framebuffer->format);

Add below:

                if (framebuffer->last_frame_render + 10 < gpuStats.numFlips && entry->format <= 3) {
                    framebuffer->format = (GEBufferFormat)entry->format;
                }

(or, really, in this case we could also skip rendering from the texture. Which may also be safe...)

-[Unknown]

daniel229 commented 10 years ago

Not help.

daniel229 commented 10 years ago

If I click buffered rendering,it works fine.

daniel229 commented 10 years ago

So the sakura's video works with framebuffer to memory on game bootup,or changing buffer rendering mode("normal buffered rendering" to "normal buffered rendering",or "normal buffered rendering" to "framebuffer to memory") while the video playing.changing resolution does not help.

LunaMoo commented 10 years ago

5447 - macross games kind of needs it to show heat wave effect over the engines, however it get's slightly wrong color(?) which makes it look kind of bad anyway.

hrydgard commented 10 years ago

update: Macross no longer needs fb to memory, the effect mostly works without it.

LunaMoo commented 10 years ago

6411 reminded me that Me & My Katamari requires framebuffer to memory while naming an island.


59:57:053 user_main    I[SCEGE]: GLES\Framebuffer.cpp:986 Creating FBO for 00044000 : 480 x 272 x 1
59:57:152 user_main    I[SCEGE]: GLES\Framebuffer.cpp:986 Creating FBO for 00000000 : 480 x 272 x 1
59:57:156 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
59:57:199 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:05:977 user_main    I[HLE]: JitCommon\JitBlockCache.cpp:173 Adding proxy root 088a38a0 to block at 08823d18
00:05:979 user_main    I[SCEGE]: GLES\Framebuffer.cpp:986 Creating FBO for 00154000 : 128 x 128 x 3
00:05:979 user_main    W[SCEGE]: GLES\Framebuffer.cpp:1024 FBO using existing buffer as depthbuffer, 00154000/00000000 and 00000000/00088000
00:07:806 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:07:806 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:07:807 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:07:807 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:07:807 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:07:807 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:07:808 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture

^ this shows up in buffered rendering with simulate block transfer and the game doesn't allow to continue althrough doesn't crash or anything

changing to read framebuffers to memory allows to continue with:


00:38:802 user_main    I[G3D]: GLES\TextureCache.cpp:111 Texture cached cleared from 29 textures
00:38:855 idle0        I[SCEGE]: GLES\Framebuffer.cpp:2043 Destroying FBO for 00044000 : 480 x 272 x 1
00:38:856 idle0        I[SCEGE]: GLES\Framebuffer.cpp:2043 Destroying FBO for 00000000 : 480 x 272 x 1
00:38:856 idle0        I[SCEGE]: GLES\Framebuffer.cpp:2043 Destroying FBO for 00154000 : 128 x 128 x 3
00:38:864 user_main    I[SCEGE]: GLES\Framebuffer.cpp:986 Creating FBO for 00000000 : 480 x 272 x 1
00:38:887 user_main    I[SCEGE]: GLES\Framebuffer.cpp:986 Creating FBO for 00044000 : 480 x 272 x 1
00:38:971 user_main    I[SCEGE]: GLES\Framebuffer.cpp:986 Creating FBO for 00154000 : 128 x 128 x 3
00:38:972 user_main    W[SCEGE]: GLES\Framebuffer.cpp:1024 FBO using existing buffer as depthbuffer, 00154000/00000000 and 00000000/00088000
00:39:022 user_main    I[HLE]: JitCommon\JitBlockCache.cpp:173 Adding proxy root 088dd560 to block at 08823d18
00:39:078 user_main    I[SCEGE]: GLES\Framebuffer.cpp:2006 Decimating FBO for 00154000 (128 x 128 x 3), age 6
00:39:079 user_main    I[SCEGE]: GLES\Framebuffer.cpp:2027 Decimating FBO for 00154000 (128 x 128 x 3), age 6
00:39:672 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture
00:39:729 user_main    I[G3D]: GLES\TextureScaler.cpp:545 TextureScaler: early exit -- empty/flat texture

Edit: blah I don't know how to use code tags on github ;| ~ should be ok now.

achurch commented 10 years ago

As of 8b67f42, Brandish (ULJM05424) still needs "read framebuffer to memory" for both fading out to the utility menu (Start button) and generating save file screenshots; without it, the fade or save file image is some random image from an earlier point in time, or solid black shortly after startup. Changing the FuncReplacements setting doesn't seem to make a difference.

unknownbrackets commented 10 years ago

Can you set breakpoints to see where it's trying to read? It may just be a memcpy or similar that we don't recognize. Best way is to see where it fades from (via GE debugger) and how, and then set breakpoints there to see where that data is supposed to come from.

-[Unknown]

achurch commented 10 years ago

It looks like the GE debugger is only built for Windows? I'm on Linux and I can't get the Windows build to run under Wine (it needs the new VC runtime and that dies during install). I tried running under GDB and setting read watchpoints on 0x40{00,88}000 in the emulated memory region but didn't catch anything obvious, so I'm not sure what else I can do.

I'll see if I can trace back through the code that generates the save screenshot, since that seems to use the same framebuffer copy as the fades.

unknownbrackets commented 10 years ago

Unfortunately, that's true (although some effort was made to keep the breakpointing portable.)

I think it works in WINE, although the fonts aren't pretty (unless you have Lucida Console.) To get the runtime, you need to extract it and copy the dlls, iirc.

See here: http://forums.ppsspp.org/showthread.php?tid=9193

-[Unknown]

achurch commented 10 years ago

Thanks for the pointer re: the VC runtime. I still couldn't get the Windows build to work (it didn't render anything), but I was able to trace through the code to find the copy routine:

   3b100: li      v1,1
   3b104: sllv    v0,v1,s1    # s1 = 0 on entry
   3b108: sll     a3,v0,0x1
   3b10c: move    a0,s3       # s3: destination buffer (0x9d34940 in this run)
   3b110: sllv    t2,v1,s2    # s2 = 0 on entry
   3b114: move    t0,zero     # Y loop counter
   3b118: sll     v0,t0,0xa   # Top of Y loop
   3b11c: addu    a1,t1,v0    # t1: source buffer (0x4044000 in this run)
   3b120: addiu   a2,a1,1024
   3b124: sltu    v0,a1,a2
   3b128: beqzl   v0,0x3b14c
   3b12c: addu    t0,t0,t2
   3b130: lhu     v0,0(a1)    # Top of X loop
   3b134: addu    a1,a1,a3
   3b138: sltu    v1,a1,a2
   3b13c: sh      v0,0(a0)
   3b140: bnez    v1,0x3b130
   3b144: addiu   a0,a0,2     # End of X loop
   3b148: addu    t0,t0,t2
   3b14c: slti    v0,t0,272
   3b150: bnez    v0,0x3b11c  # End of Y loop
   3b154: sll     v0,t0,0xa   # (delay slot)

Does that help any?

unknownbrackets commented 10 years ago

Hmm, if WINE isn't working anymore that's good to know. Might have to bisect it later. I know it never worked well with nouveau.

Hmm, so looks like that's a custom copy routine (hardcoded to 512x272x2). We can add a hook for it.

If you open ppsspp.ini, and change FuncHashMap to True, you can then name that function in the debugger (hmm, might be Windows only... can't recall if the Qt one allows that.) Once it has a name, it'll show up in ~/.config/ppsspp/PSP/SYSTEM/knownfuncs.ini. Something like this:

08f70035620d9c39:128 = brandish_buffer_blit

We can then put that in Core/MIPS/MIPSAnalyst.cpp's hardcodedHashes, and then hook it in Core/HLE/ReplaceTables.cpp.

Obviously, this is really a hack, but using OpenGL to read/write individual u16's isn't very realistic, and we can't accurately predict the height. In the software renderer, which does this accurately, the hooks will do nothing.

In this specific case, we could probably fully replace it (instead of a hook like topx_create_saveicon) and treat it like a memcpy with a fixed size (seems like it copies even the bytes between the width and stride.)

If it's a function that does other things, or if the dst/src are hard to get to, can hook with offsets to get those reg values, would just need to hook entry and exit to download and then upload the buffers.

-[Unknown]

achurch commented 10 years ago

Unfortunately the copy loop is only part of the function (which is 134 instructions long and calls out to a couple of other functions as well). I'll see if I can write up a hook for it.

unknownbrackets commented 10 years ago

The offset is an offset in bytes from the function start. If you offset to 3b10c, you can use something like u32 fb_address = currentMIPS->r[MIPS_REG_S3]; for the destination, for example.

You'd want to offset to 3b158 or later to do the upload, it looks like t1 would still be valid at that point. Unfortunately, t1 is called MIPS_REG_A5 in that enum (since it is used as the 6th argument register.)

-[Unknown]

achurch commented 10 years ago

It's actually a download, not an upload (VRAM -> main RAM), so I need to hook in before the loop starts. That said, I've dug a little deeper and it looks like most of the function code is effectively dead because it's always called with the same arguments, so I may be able to replace the entire function after all. I'll give that a shot and see if it works.

achurch commented 10 years ago

I think I've got it: https://github.com/achurch/ppsspp/commit/33264a6b8f1a738c51fb823560d5767d686a108e I still need to check that it hasn't broken anything, but it does fix the fadeout and save screenshots.

hrydgard commented 10 years ago

Looks good to me, feel free to submit a pull request.

unknownbrackets commented 10 years ago

@achurch I meant that, theoretically, that function could be used to either download or upload (depending on what addresses it uses), right? But if it's always the same arguments, that's simpler.

Note that the function won't only be replaced in Brandish. So even if Brandish only calls it with static arguments, that doesn't necessarily mean it's safe to ignore the way the function works for other arguments (the developer may have reused the same function in other games.)

-[Unknown]

achurch commented 10 years ago

In this specific case, it's download only, because the function generates the source (VRAM) buffer address from a global flag selecting one of two framebuffers.

Fair point about possibly breaking other games. Though on a closer look, I'm not entirely clear on how ReplaceTables.cpp works -- does the native function replace the translated function or just insert additional behavior before running the original function code? If the latter, I can drop the memcpy() and just leave the PerformMemoryDownload() call there.

daniel229 commented 10 years ago

Zero no kiseki using the same function,but different offset.

{ 0x8126a59ffa504614, 540, "zeronokiseki_download_frame", },//Zero no Kiseki

just change the offset in @achurch 's code,It works great.

static int Hook_zeronokiseki_download_frame() {
    u32 fb_infoaddr;
    if (!GetMIPSStaticAddress(fb_infoaddr, 0x2c, 0x30)) {
        return 0;
    }
    const u32 fb_info = Memory::Read_U32(fb_infoaddr);
    const u32 fb_index = (Memory::Read_U32(fb_info + 0x4148) + 1) & 1;
    const u32 fb_address = 0x4000000 + (0x44000 * fb_index);
    const u32 dest_address = currentMIPS->r[MIPS_REG_A1];
    if (Memory::IsRAMAddress(dest_address)) {
        gpu->PerformMemoryDownload(fb_address, 0x00044000);
        memcpy(Memory::GetPointer(dest_address),
            Memory::GetPointer(fb_address), 0x00044000);
        CBreakPoints::ExecMemCheck(fb_address, true, 0x00044000, currentMIPS->pc);
        CBreakPoints::ExecMemCheck(dest_address, true, 0x00044000, currentMIPS->pc);
    }
    return 0;
}

02

achurch commented 10 years ago

Zero no kiseki using the same function,but different offset.

I've got a change in progress that will change the literal constant to parsing the lw instruction which should make the hook usable for both cases.

daniel229 commented 10 years ago

That's great.

unknownbrackets commented 10 years ago

REPFLAG_HOOKENTER causes it to only execute additional code when it hits the instruction at the specified offset. REPFLAG_HOOKEXIT causes it to run additional code before any jr ra (or their delay slots), and neither flag means it will replace the function with C++ code entirely.

-[Unknown]

achurch commented 10 years ago

@daniel229: Try https://github.com/achurch/ppsspp/commit/230bbad1c6585cf06000f339966a0b58982cbd7d and see if you can reuse the hook function.

@unknownbrackets: Thanks, I saw the comment on the pull request. I removed the memcpy() in https://github.com/hrydgard/ppsspp/pull/6836 so the hook should be safe for all uses now.

daniel229 commented 10 years ago

Zero no KIseki and Ao no kiseki work great.

unknownbrackets commented 10 years ago

Does the Katamari game still work better with it? How about Silent Hill?

-[Unknown]

daniel229 commented 10 years ago

Silent Hill seem does not use that function.For Katamari,I don't know where the problem is.

daniel229 commented 10 years ago

Growlanser IV missing saveicons.

01

Framebuffer to memory 02

I found this function https://gist.github.com/daniel229/34fe5ba95c937b072a6c add to replacefunction table,but get a different saveicon. { 0xc3089f66ee6f0a24, 464, "memcpy", }, 03

unknownbrackets commented 10 years ago

Hmm, well, I wonder what icon it's supposed to have? Can check some savedata anyone created from a PSP, it should have the right icon, or else try the game on a PSP.

Without looking too hard, that looks like a framebuf downloading func. memcpy() probably won't cut it, but it's probably hookable. Seems like the address should be t6 here: https://gist.github.com/daniel229/34fe5ba95c937b072a6c#file-growlanser-iv-saveicons-L43

And Memory::Read_U32(currentMIPS->r[MIPS_REG_SP]) would give the format, which should probably dictate the size of the download.

-[Unknown]

daniel229 commented 10 years ago

The save icons should be like the one using Framebuffer to memory.How to use Memory::Read_U32(currentMIPS->r[MIPS_REG_SP])

unknownbrackets commented 10 years ago

Try:

{ 0xc3089f66ee6f0a24, 464, "growlanser_create_saveicon", },

And then add to ReplaceTables (after brandish):

{ "growlanser_create_saveicon", &Hook_growlanser_create_saveicon, 0, REPFLAG_HOOKENTER, 0x7C},

Then add a function:

static int Hook_growlanser_create_saveicon() {
    const u32 fb_address = Memory::Read_U32(currentMIPS->r[MIPS_REG_SP] + 4)
    const u32 fmt = Memory::Read_U32(currentMIPS->r[MIPS_REG_SP])
    const u32 sz = fmt == GE_FORMAT_8888 ? 0x00088000 : 0x00044000;
    if (Memory::IsVRAMAddress(fb_address) && fmt <= 3) {
        gpu->PerformMemoryDownload(fb_address, sz);
        CBreakPoints::ExecMemCheck(fb_address, true, sz, currentMIPS->pc);
    }
    return 0;
}

-[Unknown]