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

DRAFT - OAM Order Hijack Support (to fix #227 and similar) #258

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

andymcca
Copy link

@andymcca andymcca commented Aug 24, 2024

This is my attempt to support OAM Order Hijacking. The cause and effect of this is outlined in issue #227.

The issue that I see for gpsp in addressing this issue is that we store the Layer Priority at the Object/BG levels only. In order to correctly deal with OAM Order Hijacking in the same way as the hardware, we need to be able to check priority at a pixel-level and act accordingly.

My solution here is -

a) to check for OAM Order Hijacking at a row-level when we are ordering our objects (in order_obj), and mark each applicable row in the same way we do for rows that contain transparent objects.

b) when we come to render a scanline marked as hijacked, generate an OBJ-only scanline buffer first by rendering all available OBJ layers to this buffer in u16/INDXCOLOR mode, which gives us the most flexibility later on

c) when rendering 8-bit per pixel OBJ tiles, we check 4 pixels at a time for transparency (tilepix). In the current code, if all 4 pixels are transparent (i.e. 0), we move on to the next 4 pixels to complete the tile. But in my PR, we instead check each pixel to see if we have a non-zero value in the OBJ scanline buffer we created. If we do, we render the value to dest_ptr using the selected render mode.

I have tried to build in some optimisations here such as only creating the OBJ scanline buffer on scanlines affected by a hijack and only comparing to the scanline buffer when we have 4 consecutive transparent pixels as described above. The latter is sufficient to address the issues identified in Golden Sun as far as I can see, but complete/correct emulation would be to compare against EVERY transparent pixel.

Further ToDos:

  • 4bpp support (Now DONE - this also fixes areas in Golden Sun 2 and Zelda Minish Cap!)
  • Affine/Mosaic Objects support
  • Partial tile support - at the moment I'm only applying the buffer to full tiles, so tiles cut off by the screen on either side will still exhibit the previous behaviour

So this PR is still in draft at the moment for more discussion/testing before committing.

This is my attempt to support OAM Order Hijacking.  The cause and effect of this is outlined in issue libretro#227.

The issue that I see for gpsp in addressing this issue is that we store the Layer Priority at the Object/BG levels only.  In order to correctly deal with OAM Order Hijacking in the same way as the hardware, we need to be able to check priority at a pixel-level and act accordingly.

My solution here is - 

a) to check for OAM Order Hijacking at a row-level when we are ordering our objects (in order_obj), and mark each applicable row in the same way we do for rows that contain transparent objects.
b) when we come to render a scanline marked as hijacked, generate an OBJ-only scanline buffer first by rendering all available OBJ layers to this buffer in u16/INDXCOLOR mode, which gives us the most flexibility later on
c) when rendering 8-bit per pixel OBJ tiles, we check 4 pixels at a time for transparency (tilepix).  In the current code, if all 4 pixels are transparent (i.e. 0), we move on to the next 4 pixels to complete the tile.  But in my PR, we instead check each pixel to see if we have a non-zero value in the OBJ scanline buffer we created.  If we do, we render the value to dest_ptr using the selected render mode.

I have tried to build in some optimisations here such as only creating the OBJ scanline buffer on scanlines affected by a hijack and only comparing to the scanline buffer when we have 4 consecutive transparent pixels as described above.  The latter is sufficient to address the issues identified in Golden Sun as far as I can see, but complete/correct emulation would be to compare against EVERY transparent pixel.  

Further ToDos:
- 4bpp support
- Affine/Mosaic Objects support

So this PR is still in draft at the moment for more discussion/testing before committing.
Copy link
Author

@andymcca andymcca left a comment

Choose a reason for hiding this comment

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

Corrected a few comments which referred to older code

Added code to handle 4bpp OBJs - this fixes the similar issue in Golden Sun 2 with the ice shards in the Mars Lighthouse / other areas.
Correct a few comments in the code
@davidgfnet
Copy link
Collaborator

Will run a full romset test to verify this works on all roms. Then we can perhaps come up with a cleaner patch.
Thank you!

@mitchchn
Copy link

mitchchn commented Sep 3, 2024

Thanks for working on this @andymcca!

The fix puts everything in the right order in Golden Sun, but opening the menu (Start) causes sprites to peek through the menu. This only happens in scenes where the glitch would have previously occurred:

Corrected scene (previously the man by the temple was hidden in GPSP):

Screenshot 2024-09-03 at 7 24 21 PM

Opening the menu:

Screenshot 2024-09-03 at 7 25 14 PM

@andymcca
Copy link
Author

andymcca commented Sep 4, 2024

Thank you very much for this report @mitchchn !

So I will check this out, I did see this happen before when I was doing a few code tests prior to this PR. Seems odd that it's just parts of the Sprite that bleed through!! Very interesting, I will troubleshoot it once I have some time to work on this again - thanks again!

@andymcca
Copy link
Author

andymcca commented Dec 7, 2024

@mitchchn 3 months later...real life got in the way! So I've looked at this issue and checking with mgba frame inspector it looks like the OBJ transparency code is running when it shouldn't be i.e. on the pause screen sprites. I think the fix for this will be to clear the OBJ buffer I've created after I've rendered the last hijacking sprite but that seems to cause issues elsewhere on the frame. I will figure it out, just the code doesn't seem to be doing what I'm expecting it to do at the moment :-/

EDIT: found a bug - when the OBJ buffer is created I've realised that it currently ignores transparency which means lower priority objects will 'bleed' through to transparent areas in higher-priority objects in the buffer. Logically that would cause the behaviour above. But fixing that doesn't appear to solve it! More thought and code required.....

This fixes parts of sprites 'peeking through' on the Golden Sun pause menu.
@andymcca
Copy link
Author

@mitchchn @davidgfnet

Issue above is fixed. I tried a few different ways to fix it but the simplest and easiest is to clear the OBJ buffer when we start rendering OBJs with priority 0. It's a bit of a hack fix but to be honest this entire PR is one big hack 😆 trying to emulate a hardware wrinkle used in a handful of games.

@andymcca andymcca closed this Dec 17, 2024
@LibretroAdmin
Copy link

LibretroAdmin commented Dec 17, 2024

Hi there, sad to see this closed. Was no consensus reached to merge this or has it been incorporated into the code in some other way? Or is there a new PR that supersedes this?

@andymcca andymcca reopened this Dec 17, 2024
@andymcca
Copy link
Author

@LibretroAdmin I'm mystified as I didn't close this even though it says I did! I've re-opened it. I've left it as a draft for the time being because its unfinished and I'm not sure of what impact it has on other games. I think the concept is the correct way of doing it but I dont know that I've coded it in the best way tbh

@davidgfnet
Copy link
Collaborator

I will try to take a look at it. Dont wanna merge stuff that either breaks other games or makes the emulator too slow.

@andymcca
Copy link
Author

andymcca commented Dec 21, 2024 via email

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

Successfully merging this pull request may close these issues.

4 participants