Navigation Menu

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

Jump to bottom

Large 2D sprites that use a tile index close to the right edge should wrap without advancing to next row #2443

Closed
velipso opened this issue Feb 12, 2022 · 4 comments
Closed

Large 2D sprites that use a tile index close to the right edge should wrap without advancing to next row #2443

velipso opened this issue Feb 12, 2022 · 4 comments
Labels
hardware:2D Issues in the 2D/pixel pipeline subsystem platform:GBA Game Boy Advance-related issues
Milestone
mGBA 0.10.0

Comments

@velipso
Copy link

velipso commented Feb 12, 2022

If a sprite is 16x8, and set to tile index 63, then the right half of the sprite should be taken from tile index 32. However, mGBA pulls from tile index 64.

I've included a small test ROM. On mGBA 0.9.3-0.10, it will display a white+red sprite, but on hardware it displays a white+green sprite.

obj.gba.zip
@endrift endrift added hardware:2D Issues in the 2D/pixel pipeline subsystem platform:GBA Game Boy Advance-related issues labels Feb 13, 2022
@velipso
Copy link
Author

velipso commented Feb 13, 2022

This fixes the test ROM... there's probably a better way to do this by having charBase mean the start of the row. I also have no clue how to fix the GL renderer, or deal with other sprite cases (256, windowing, etc). At least it's a start:

diff --git a/src/gba/renderers/software-obj.c b/src/gba/renderers/software-obj.c
index 3fb364f56..371a95957 100644
--- a/src/gba/renderers/software-obj.c
+++ b/src/gba/renderers/software-obj.c
@@ -74,7 +74,7 @@
 #define SPRITE_YBASE_16(localY) unsigned yBase = (localY & ~0x7) * stride + (localY & 0x7) * 4;
 
 #define SPRITE_DRAW_PIXEL_16_NORMAL(localX) \
-	LOAD_16(tileData, ((yBase + charBase + xBase) & 0x7FFE), vramBase); \
+	LOAD_16(tileData, ((((charBase & ~1023) + yBase + (((charBase & 1023) + xBase) & 1023)) & 0x7FFE)), vramBase); \
 	tileData = (tileData >> ((localX & 3) << 2)) & 0xF; \
 	current = renderer->spriteLayer[outX]; \
 	if ((current & FLAG_ORDER_MASK) > flags) { \

@endrift endrift added this to the mGBA 0.10.0 milestone Feb 21, 2022
@endrift
Copy link
Member

endrift commented Jun 8, 2022

Do you have a repro for 256 color sprites? It'd be nice to have one for both

@velipso
Copy link
Author

velipso commented Jun 8, 2022

sure, here you go, I modified the original to have both 16 and 256 color sprites... mGBA displays white+red, but hardware displays white+green.

obj.gba.zip

@endrift
Copy link
Member

endrift commented Jun 8, 2022

Perfect, thanks. I have a patch prepared, but I'm testing it to make sure it doesn't break anything. I noticed that this does not apply to 1D-mapped sprites, only 2D-mapped, which makes a lot of sense, so I'm glad I checked.

@velipso velipso mentioned this issue Jun 8, 2022
Closed
@endrift endrift closed this as completed in 7a07b14 Jun 8, 2022
rtomasa pushed a commit to rtomasa/mgba that referenced this issue Sep 15, 2023
@endrift @rtomasa
GBA Video: Fix OBJ tile wrapping with 2D char mapping (fixes mgba-emu…
e50f3d9 
…#2443)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
hardware:2D Issues in the 2D/pixel pipeline subsystem platform:GBA Game Boy Advance-related issues
Projects
None yet
Milestone
mGBA 0.10.0
Development

No branches or pull requests
2 participants
@endrift @velipso