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

BPStructs: Gracefully handle out-of-range EFB copies #7950

Merged
merged 1 commit into from Apr 1, 2019

Conversation

3 participants
@stenzek
Copy link
Contributor

commented Mar 31, 2019

Since the copy X and Y coordinates/sizes are 10-bit, the game can configure a copy region up to 1024x1024. Hardware tests have found that the number of bytes written does not depend on the configured stride, instead it is based on the size registers, writing beyond the length of a single row. The data written for the pixels which lie outside the EFB bounds does not wrap around instead returning different colors based on the pixel format of the EFB.

This suggests it's not based on coordinates, but instead on memory addresses. The effect of a within-bounds size but out-of-bounds offset (e.g. offset 320,0, size 640,480) are the same.

As it would be difficult to emulate the exact behavior of out-of-bounds reads, instead of writing the junk data, we don't write anything to RAM at all for over-sized copies, and clamp to the EFB borders for over-offset copies.

Dolphin now matches my hardware tests (apart from the open-bus-like behavior trashing the next rows of textures).

This was previously causing errors in some arcade virtual console games, which perform these out-of-range copies.

BPStructs: Gracefully handle out-of-range EFB copies
Since the copy X and Y coordinates/sizes are 10-bit, the game can configure a
copy region up to 1024x1024. Hardware tests have found that the number of bytes
written does not depend on the configured stride, instead it is based on the
size registers, writing beyond the length  of a single row. The data written
for the pixels which lie outside the EFB bounds does not wrap around instead
returning different colors based on the pixel format of the EFB.

This suggests it's not based on coordinates, but instead on memory addresses.
The effect of a within-bounds size but out-of-bounds offset
(e.g. offset 320,0, size 640,480) are the same.

As it would be difficult to emulate the exact behavior of out-of-bounds reads,
instead of writing the junk data, we don't write anything to RAM at all for
over-sized copies, and clamp to the EFB borders for over-offset copies.

@stenzek stenzek force-pushed the stenzek:out-of-range-efb-copies branch from 6c39c6c to 378b605 Mar 31, 2019

@JMC47

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2019

Fixes Wreckless: The Yakuza Wars and 1942 + the other Wii VC Arcade Titles from a panic alert that then causes a crash if you dare try to ignore it.

srcRect.left, srcRect.top, destStride);

// Adjust the copy size to fit within the EFB. So that we don't end up with a stretched image,
// instead of clamping the source rectangle, we reduce it by the over-sized amount.

This comment has been minimized.

Copy link
@iwubcode

iwubcode Mar 31, 2019

Contributor

Just curious, any ideas why we reduce by the over-sized amount instead of just clamping? I'm assuming this matches your hardware tests, just seems odd

This comment has been minimized.

Copy link
@stenzek

stenzek Apr 1, 2019

Author Contributor

I kinda said that in the comment, but it wasn't very clear. Consider an oversized copy of 1024x480. If we clamped the size to 640x480, but left the rectangle intact, the first 640 pixels would cover only half the EFB copy, with the second half clamping to the border. By shrinking it by 640, we only create a 640 wide copy, and this matches 1:1 with the EFB pixels.

@iwubcode
Copy link
Contributor

left a comment

Tested a few EFB style Wii games. No regressions. Code seems fine. LGTM

@stenzek stenzek merged commit 32e330e into dolphin-emu:master Apr 1, 2019

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details

@stenzek stenzek deleted the stenzek:out-of-range-efb-copies branch Apr 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.