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

GameSettings: Disable "Store EFB Copies to Texture Only" for Final Fantasy Crystal Chronicles #9329

Merged
merged 1 commit into from Dec 15, 2020

Conversation

smurf3tte
Copy link
Contributor

@smurf3tte smurf3tte commented Dec 15, 2020

This fixes the crash at Goblin Wall: https://bugs.dolphin-emu.org/issues/9915

A patch (for the US release only) that fixes the game's buffer overrun bug is included as an alternative with lower performance cost. It is disabled by default.

This game makes use of EFB to RAM copies for various post-processing effects. When doing so, it first attempts to copy texture data in RGBA8 format (8 bits per channel), presumably to maintain the best possible image quality. If the destination buffer is too small, it will fall back on RGB565, which only requires half the space.

Unfortunately, when calculating the required space, the game fails to take into account the padding introduced by tiling on the GPU. There is a helper function provided by the SDK for this purpose which the game does make use of elsewhere. In one particular function (address 0x8001792c in the US release) it does not, and as a result it sometimes underestimates the size of the copy, leading to a classic buffer overrun.

In the Goblin Wall area of the game, there is a post-boss cutscene that triggers the buffer overrun, trashing a data structure used by the game's memory manager. This structure is later accessed when the player opens their Moogle Mail in the cutscene that follows. In Dolphin, this led to multiple invalid read/write errors and ultimately the game would crash.

Initially, I suspected the game was being saved from this corruption on real hardware by the CPU's data cache, as has been the case with other games (e.g. #9316). Because the GPU copies straight to main memory, the corruption would not be observed so long as the affected data was already loaded into a CPU cache. However, in this case the affected data is last touched long before the corruption occurs, making it unlikely to still be cached by that time.

Later tests confirmed the data cache does not protect the game on real hardware; it observes memory corruption at the same addresses as under Dolphin! Why, then, does it not crash in the same way? As it turns out, the texture data that is written follows a certain pattern, and the first byte that is read back by the CPU has the value 0xFF. The game interprets this to have a particular meaning and doesn't bother reading the rest of the data. Lucky break.

By default, as an optimization Dolphin does not fully emulate the effects of EFB to RAM copies. Instead, it zeroes the bytes in main RAM, to disastrous effect in the case of Crystal Chronicles. When the game reads back 0x00, it sets down a path that tries to interpret the remaining zeros as pointers, which leads to a whole bunch of invalid reads and writes. By disabling this optimization, parity is restored with real hardware... or it would be, were it not for yet another optimization.

Even when EFB to RAM copies are enabled, they are deferred by default, meaning they do not occur until later than they would on real hardware, and in some cases are skipped entirely. As it happens, the buffer overrun at Goblin Wall is an example of the latter, which means the memory corruption is prevented from occurring at all - even though it happens on real hardware! Disabling the setting to defer EFB copies leads to more hardware accurate behavior, though it is of dubious value in this case. Instead I have left it enabled, as disabling it would mean taking a performance hit for the benefit of corrupting memory more often.

One of the first things I tried, upon discovering the bug in the game code, was to correct the logic to no longer overrun the destination of the copy. This can be done most trivially by accounting for the smallest texture that can trigger a buffer overrun. The inputs are bounded by the frame buffer size (640x448), so the worst case is a texture of size 569x373, for which the game will calculate a space requirement of 0xCF434 bytes, as opposed to the correct answer of 0xD2080 bytes. The destination buffer is 0xD2000 bytes, so a comparison using the incorrect size can be made safe by adjusting the limit downward by 0x2BCC+1 bytes.

This adjustment is precisely what the included patch does, as an alternative to enabling EFB to RAM copies. The downside is that in some cases near the boundary, textures that would have fit will be needlessly downgraded from RGBA8 to RGB565. Still, some users may find this solution preferable, as it has a minimal effect on performance.

@Pokechu22
Copy link
Contributor

Should there also be a comment in the INI about the EFB deferring leading to the corruption not happening at all (in a console-inaccurate way)? Right now it implies that EFBToTextureEnable = False is enough to make it faithful to hardware, not just to stop the crash.

…ntasy Crystal Chronicles

This fixes the crash at Goblin Wall: https://bugs.dolphin-emu.org/issues/9915

A patch (for the US release only) that fixes the game's buffer overrun bug is included as an alternative with lower performance cost. It is disabled by default.
@JMC47 JMC47 merged commit 5a5c22d into dolphin-emu:master Dec 15, 2020
@MayImilae
Copy link
Contributor

@smurf3tte I'm one of the writers for the Dolphin Blog. We're have this change as a section but there are a few things we're not clear about. Would you mind coming to dolphin-dev and answering a few of my questions? Alternatively we could do it here if you'd prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants