SkyEmu icon indicating copy to clipboard operation
SkyEmu copied to clipboard

GBA: vertical wraparound bug for 128px tall sprites not emulated

Open fleroviux opened this issue 1 year ago • 1 comments

GBATEK mentions:

Caution: A very large OBJ (of 128 pixels vertically, ie. a 64 pixels OBJ in a Double Size area) located at Y>128 will be treated as at Y>-128, the OBJ is then displayed parts offscreen at the TOP of the display, it is then NOT displayed at the bottom.

SkyEmu currently does not emulate this correctly.

This edge-case can be reproduced on hardware inside a demo from the Tonc programming tutorial. I think I may have also figured out why/how this happens in hardware.

How to reproduce

  1. Run obj_aff.zip
  2. Press A until PA (top-left matrix entry) is close to 0080
  3. Press B until PD (bottom-right matrix entry) is close to 0080
  4. Press Start to set the 'Double Size' flag
  5. Press Select+Down to move the sprite downward

What happens on hardware?

As the sprite moves downward and its Y-coordinate reaches values of 128 and higher, the sprite wraps around at the top of the screen, but disappears at the bottom of the screen.

https://user-images.githubusercontent.com/13669774/218339351-cd431229-b65d-46cb-a292-867efe19c6d2.mp4

You may also want to reference VisualBoyAdvance or NO$GBA for comparison. Ironically, they were the only mainstream emulators until now that handle this correctly!

Why does this happen?

For each enabled sprite the PPU has to check that the sprite (vertically) intersects the current scanline.

A naive way to do this would be compare the current scanline against the lowest and highest y-Coordinates of the sprite:

uint y_min = oam_attr_0 & 255u;
uint y_max = y_min + height;

if(scanline >= y_min && scanline < y_max) {
  // sprite is visible...
}

However y_min may be between 160 and 255 and those values should wraparound to -96 to -1 (y_min - 256). One way to fix this would be to adjust y_min accordingly:

int y_min = oam_attr_0 & 255;

if(y_min >= 160) y_min -= 256;

int y_max = y_min + height;

if(scanline >= y_min && scanline < y_max) {
  // sprite is visible...
}

Using this logic, our sprite (with height=128px) at e.g. y=140 would be rendered at the bottom of the screen and not at the top. That contradicts the results from hardware though.

The thing is that all this arithmetic had to be done with a limited number of bits in hardware, and it happens to be 8-bit (which makes sense). But that means that hardware cannot use the same fix we used to handle wraparound, since 256 does not fit into 8-bits. Well, curiously if y_min is >= 160 then, due to wraparound, the only way that the sprite can be on-screen is if y_max exceeds 255 (since values between 160 and 255 would still be off-screen). Due to the 8-bit arithmetic, y_max would overflow in that case, meaning that it will be smaller than y_min (in hardware this could be detected using the adder's carry output).

Well, this opens a new opportunity for us to detect when the sprite wraps around right? If y_max overflows, then the sprite wraps around! And we can ignore the result of the scanline >= y_min check because y_min would be off-screen anyway.

In an emulator we could implement it like this:

uint y_min = oam_attr_0 & 255u;
uint y_max = (y_min + height) & 255u;

if((scanline >= y_min || y_max < y_min) && scanline < y_max) {
  // sprite is visible...
}

This appears to be how the hardware does it. Unfortunately this causes our edge-case. It is correct that y_max overflows when the sprite is (partially) visible but y_min is off-screen. But conversely 'If y_max overflows, then the sprite wraps around!' is not always correct. When height=128 then y_max will even overflow when y_min is between 128 and 159. y_max would then be between 0 and 31, meaning that the scanline < y_max check passes at the top of the screen and fails at the bottom of the screen.

The logic in hardware works correctly except when the sprite's height is 128 px.

In an emulator we don't have to use 8-bit arithmetic all the time, so it may be simpler to achieve the same effect using signed integers (as y_min may be used subsequently to calculate texture coordinates).

int y_min = oam_attr_0 & 255;
int y_max = (y_min + height) & 255u;

if(y_max < y_min) y_min -= 256;

if(scanline >= y_min && scanline < y_max) {
  // sprite is visible...
}

fleroviux avatar Feb 12 '23 23:02 fleroviux

Wow! Awesome bug report. Thanks for documenting this, I'll work on fixing this.

skylersaleh avatar Feb 13 '23 00:02 skylersaleh