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

Add compat flag to not load CLUTs from old framebuffers #17965

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

hrydgard
Copy link
Owner

Fixes #17373 by adding a compat flag to avoid loading CLUTs (palettes) from framebuffers not rendered to during the current frame.

#17373 is a LoadCLUT issue (where we load a palette from a framebuffer). Doesn't persist after a save/reload state, so it seems we end up reading a palette from a framebuffer that contains outdated data, maybe due to sticking around from a previous frame while in reality the game has replaced that memory with the correct data. Checking for changes in the memory area covered by framebuffers might be necessary to fix this without flags, which gets too risky to include in the upcoming release.

I don't think it's that common though that we need LoadCLUT from framebuffers from old frames, so perhaps we should eventually invert this flag, and make it the default - risk seems fairly high to erroneously load CLUTs from outdated framebuffers lingering around. I'll do that after the release though.

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Aug 24, 2023
@hrydgard hrydgard added this to the v1.16.0 milestone Aug 24, 2023
@hrydgard hrydgard merged commit fb84444 into master Aug 24, 2023
18 checks passed
@hrydgard hrydgard deleted the load-clut-old-framebuffers branch August 24, 2023 12:24
@@ -1355,7 +1355,8 @@ void TextureCacheCommon::LoadClut(u32 clutAddr, u32 loadBytes) {
// the framebuffer with the smallest offset. This is yet another framebuffer matching
// loop with its own rules, eventually we'll probably want to do something
// more systematic.
if (matchRange && !inMargin && offset < (int)clutRenderOffset_) {
bool okAge = !PSP_CoreParameter().compat.flags().LoadCLUTFromCurrentFrameOnly || framebuffer->last_frame_used == gpuStats.numFlips;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably at least account for last_frame_render if it becomes the default?

By the way, this is already in an if (matchRange) {. I could see a game rendering a clut and then reading from it every frame, how old is the buffer in Syphon Filter? Heurstics like this will probably just get changed later when you figure "it's probably not an issue anymore" unless they have stronger logic to them.

-[Unknown]

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.

Syphon Filter: Logan's Shadow (US & EUR) Menu's Wrong Colors
2 participants