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

Simpsons crash on GLES fixed #16483

Merged
merged 1 commit into from
Dec 3, 2022
Merged

Simpsons crash on GLES fixed #16483

merged 1 commit into from
Dec 3, 2022

Conversation

lvonasek
Copy link
Contributor

@lvonasek lvonasek commented Dec 2, 2022

Tested on Pico Neo 3 and Pico 4 VR headsets. Loading saved game renders one frame and then it crashes on texture destroying with a broken pointer (0x1FFF...FFF). No idea why is it happening, I only found the commit from which it started happening and reverted it.

This reverts commit cbfa4bf.

@unknownbrackets
Copy link
Collaborator

Since all of the ClearCacheNextFrame() methods are the same, what if you just put that on GPUCommon? Does that still fix the crash, or does it somehow need to be virtual?

If it only fixes it when it's virtual, it makes me worry there's some heap corruption happening and this only "fixes" it because it moves memory around and makes the thing crashing dodge that corruption (but some other unknown thing is getting corrupted now.)

-[Unknown]

@lvonasek
Copy link
Contributor Author

lvonasek commented Dec 2, 2022

Since all of the ClearCacheNextFrame() methods are the same, what if you just put that on GPUCommon? Does that still fix the crash, or does it somehow need to be virtual?

I tried to move it into gstate_c, it still crashes.

@hrydgard
Copy link
Owner

hrydgard commented Dec 2, 2022

This one is quite hard to justify heh, like, are we somehow saving/loading the vtable, or how could that possibly matter? That can't possibly work with ASLR...

@@ -1197,7 +1197,6 @@ bool TextureCacheCommon::GetCurrentFramebufferTextureDebug(GPUDebugBuffer &buffe
}

void TextureCacheCommon::NotifyConfigChanged() {
clearCacheNextFrame_ = true;
Copy link
Owner

@hrydgard hrydgard Dec 2, 2022

Choose a reason for hiding this comment

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

What if all you do is remove this line move this to CheckConfigChanged? This actually didn't seem to happen before the same way, though I don't see why it would be crashy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested removing and that fixes the crash. Just turned everything off, I can try to move it tomorrow.

@hrydgard
Copy link
Owner

hrydgard commented Dec 2, 2022

Actually this is a small change in behavior. If my suggestion above doesn't help, I'm ok with reverting this, I'll do a second version of the cleanup later on top.

@hrydgard hrydgard merged commit d6b9f39 into hrydgard:master Dec 3, 2022
@lvonasek lvonasek deleted the hotfix_simpsons_crash branch December 3, 2022 09:49
hrydgard added a commit that referenced this pull request Dec 3, 2022
hrydgard added a commit that referenced this pull request Dec 3, 2022
@hrydgard
Copy link
Owner

hrydgard commented Dec 12, 2022

Going back, I think this only accidentally fixed stuff - the real fix was #16533 or the bug fixes in #16531. Anyway, doesn't really matter now.

@hrydgard hrydgard added this to the v1.14.0 milestone Dec 12, 2022
@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Dec 12, 2022
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.

3 participants