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

Update framebuffer from memory when game manipulate framebuffer in memory(Open Season). #13326

Closed
wants to merge 4 commits into from

Conversation

shenweip
Copy link
Contributor

This will fix #13252 .Some games may write directly to PSP VRAM to update the framebuffer, but the virtual framebuffer will not be updated. Maybe the right thing to do is to use PSP VRAM instead of virtual framebuffers, but may need some refactoring.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Aug 23, 2020

I guess this is similar case to DarkStalkers and many homebrew where we use software rendering.
Isn't hashing too slow for that?

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Aug 23, 2020
@shenweip
Copy link
Contributor Author

I've tested it on PC, and it has almost no effect on speed, but, yes, it is likely to affect low performance devices.

@unknownbrackets
Copy link
Collaborator

If we can, it'd be better to hook the function that makes the change.

The trouble with this change is that some games clear the screen using the CPU (which we may even hook and detect), and then draw on top. Hashing alone in that case would make the screen black. I realize the plan is to individually manage this flag for each of 1400+ games one by one.

A lot of the time, games will modify and memcpy in data. We already detect this often, and many games will use the same memcpy signature. If the function that writes to VRAM fits in this category, we could hook its jr ra and upload to the GPU then.

-[Unknown]

@shenweip
Copy link
Contributor Author

@unknownbrackets The game seems to use this function to decode data, and it will call multiple times to complete the decoding. I agree that managing the flag for each games is unsatisfactory, is it better now?

@shenweip shenweip changed the title Update framebuffer from memory when games manipulate framebuffer in memory. Update framebuffer from memory when game manipulate framebuffer in memory(Open Season). Aug 24, 2020
@sum2012
Copy link
Collaborator

sum2012 commented Sep 2, 2020

@hrydgard @unknownbrackets how about this now ?

@unknownbrackets
Copy link
Collaborator

What's this func look like that you need to ignore some jr ras?

-[Unknown]

@shenweip
Copy link
Contributor Author

shenweip commented Sep 2, 2020

It look like:
0x00000000 addiu sp,sp -0x380(func entry)
  ⋮     ⋮
0x00000010 jal 0x00000030
  ⋮     ⋮
0x00000020 jal 0x00000060
  ⋮     ⋮
0x00000030 ???????????????
  ⋮     ⋮
0x00000040 jr ra(not real func exit)
  ⋮     ⋮
0x00000050 jr ra(real func exit)
0x00000054 addiu sp,sp 0x380
  ⋮     ⋮
0x00000060 ???????????????
  ⋮     ⋮
0x00000070 jr ra(not real func exit)

@unknownbrackets
Copy link
Collaborator

But are those separate functions that we're just misdetecting as part of the function, or are they actually other valid exits?

I'm worried that we might break an existing hook with this, since I remember at least one being a func that legitimately had two exits. But it's probably just that one and I think it was a leaf (no add sp.)

-[Unknown]

@shenweip
Copy link
Contributor Author

shenweip commented Sep 2, 2020

I think those aren't separate functions because some of them are located in front of the real exit. Um... do you mean having a func that pushes stack(has add sp) at the start of func but not pop stack(no add sp) at the exit? This is interesting, it seems likely to break the stack frame?

@unknownbrackets
Copy link
Collaborator

unknownbrackets commented Sep 3, 2020

No, I've seen cases where it does the add sp out of order when it has two exits. It's very uncommon, I just don't know if any of these hooked functions do it. I'm most worried about the simd optimized memcpys, but I think they are okay. It's likely not an issue.

For clarity: out of order meaning i.e. the addiu sp,sp,0x20 before the jr ra.

-[Unknown]

@shenweip
Copy link
Contributor Author

shenweip commented Sep 3, 2020

Got it, so let's specify func exit in this case for the compatibility.
Thanks @unknownbrackets

@sum2012
Copy link
Collaborator

sum2012 commented Oct 29, 2020

@hrydgard how about this? ( allthough change need rebase)

@sum2012
Copy link
Collaborator

sum2012 commented Dec 20, 2020

@hrydgard how about this? ( although change need rebase)

@unknownbrackets
Copy link
Collaborator

Is this function called in a loop, or is it called separately? Wondering if we could combine into one upload.

Also, rather than a new method on GPUCommon and using UpdateFromMemory, I recommend using gpu->PerformMemoryUpload(...);. The only trouble is that it'll want the transfer to be aligned to the framebuffer stride.

Perhaps we could just do something like...

static int Hook_openseason_data_decode() {
	static u32 firstWritePtr = 0;

	u32 curWritePtr = currentMIPS->r[MIPS_REG_A0];
	u32 endPtr = currentMIPS->r[MIPS_REG_A1];
	u32 writeBytes = currentMIPS->r[MIPS_REG_V0];
	u32 startPtr = curWritePtr - writeBytes;
	if (Memory::IsVRAMAdress(startPtr) && (firstWritePtr == 0 || startPtr < firstWritePtr)) {
		firstWritePtr = startPtr;
	}
	if (Memory::IsVRAMAdress(endPtr) && curWritePtr == endPtr) {
		gpu->PerformMemoryUpload(firstWritePtr, endPtr - firstWritePtr);
		firstWritePtr = 0;
	}
	return 0;
}

That would combine it into one upload, which should perform much better. Would that work?

We wouldn't need to change any files other than Core/MIPS/MIPSAnalyst.cpp and Core/HLE/ReplaceTables.cpp this way either. No need to add extra methods when we already have code to do this for other games.

-[Unknown]

@sum2012
Copy link
Collaborator

sum2012 commented Jan 26, 2021

@unknownbrackets I don't understand what your last said.Can you make another pull request for me to test ? Thanks.

@sum2012
Copy link
Collaborator

sum2012 commented Feb 21, 2021

Finally I figure that what is the @unknownbrackets chnage,
I will open another pull request

sum2012 added a commit to sum2012/ppsspp that referenced this pull request Feb 21, 2021
@sum2012 sum2012 mentioned this pull request Feb 21, 2021
@hrydgard
Copy link
Owner

Closing in favor of #14192 .

@hrydgard hrydgard closed this Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GE emulation Backend-independent GPU issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Open Season Title Screen does not display
5 participants