gpsp icon indicating copy to clipboard operation
gpsp copied to clipboard

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

Open andymcca opened this issue 1 year ago • 10 comments
trafficstars

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.

andymcca avatar Aug 24 '24 11:08 andymcca

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!

davidgfnet avatar Aug 24 '24 21:08 davidgfnet

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

mitchchn avatar Sep 03 '24 23:09 mitchchn

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 avatar Sep 04 '24 12:09 andymcca

@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.....

andymcca avatar Dec 07 '24 12:12 andymcca

@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 avatar Dec 14 '24 10:12 andymcca

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?

LibretroAdmin avatar Dec 17 '24 20:12 LibretroAdmin

@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

andymcca avatar Dec 17 '24 21:12 andymcca

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

davidgfnet avatar Dec 21 '24 15:12 davidgfnet

It's probably best if it's a core option David, and off by default, as it's really only needed in exceptional circumstances.

I won't do any more on this or the other draft PRs I've created now, will leave it for you and/or other contributors to review, improve on if necessary and decide how/if to implement in the live code.

andymcca avatar Dec 21 '24 15:12 andymcca

Found another instance of this, in Mario Golf.

image

This is from mgba and you can see that on the yard meter, the 'L' icon in the bottom left overlays the power indicator (where it says 'Normal'). In gpsp it is instead behind the power indicator. Looking at the sprite order in mgba, it is the same issue i.e. OAM order hijack, but in this instance the Priorities are set as 1 and 0.

The Clear OBJ buffer on Priority 0 commit I did (https://github.com/libretro/gpsp/pull/258/commits/36969582aa18c0bd0f45eee100308238414f4b0f) to 'fix' the Golden Sun menu issue described above, will unfortunately revert this to standard gpsp behaviour.

So I (or someone else) probably needs to rethink this to cover all scenarios. Annoying issue!

andymcca avatar Mar 23 '25 09:03 andymcca

Had some time recently to tinker with this again, this time using Cursor/Cluade to help me implement the changes.

I've tried several different implementations of this, all of which solve some issues but then either leave others unresolved, or worse - create new ones!

Fundamentally the issue is with the current renderer approach and how it differs from what actually happens on the GBA. With the current approach we render in 'layers' and in priority order. So each background is a layer and each set of OBJs grouped by background priority is a layer.

What we're trying to emulate here is a bug in the real GBA OBJ depth buffer process, which for transparent pixels incorrectly assigns that background priority to all OBJ pixels occupying the same spot. Hence the behaviour on real hardware. The problem with the current renderer is that we only buffer 2 pixels max during the layering process, so by the time you come to render that transparent pixel, the OBJ pixel that needs its priority 'elevated' and thus displayed has likely already been overwritten.

The correct approach would be to rewrite the current renderer to work more like the GBA e.g.

  1. render all backgrounds in layers as we do currently to the current scanline, and additionally store the background priority +1 of each pixel in a separate array (configure 240 entries to value 4 in the first instance)
  2. render objects to the scanline in OAM order 0 - 127. for each pixel rendered, check the current OBJ priority against the separate priority array value for that pixel - if the separate array value is less than or equal to then discard it. If it is greater then render the OBJ pixel and update the priority in the separate array we created in 1. If it is a transparent pixel, then don't render anything but simply update the priority in the separate array we created in 1.

I think that would solve this issue, likely at the cost of performance due to all the priority checking - plus you'd have to take into account all the affine objects, mosaic, video modes etc etc. Probably a lot of work!

Or a hybrid approach where the above only happens for scanlines where OAM hijacking is detected.

andymcca avatar Oct 05 '25 10:10 andymcca

Yeah you are right. The way this is done is on a per-pixel basis. A good reference implementation (not sure if it's perfect but I suspect it is quite close) is the machine implementation in some MISTER backend: https://github.com/MiSTer-devel/GBA_MiSTer/blob/17042c99591cef4f7b0d882f332456321a64152b/rtl/gba_drawer_merge.vhd#L378 And in fact most emulators follow a similar approach. GPSP uses a smart approach to save time. Rendering obj/tiles in groups and layers is not accurate but works way faster (I'm talking like 3-4x faster). In fact, the mosaic effect I added (which has specialized paths) is quite slow and shows slowdowns on most low-end platforms. However it was kind of useful since some games rely on it heavily.

As you say we could have a different rendering method if we detect this "hack", but given the number of games that abuse this, I'd say it's not worth the effort :(

davidgfnet avatar Oct 05 '25 17:10 davidgfnet

Well....I went ahead and finally got it fixed up anyway using my original approach (i.e. no fundamental renderer re-write!!). It does now work correctly in all instances I know about and I fixed the two remaining issues explained above (Mario Golf shot indicator and Golden Sun in-game menu 'peeping').

I've also made it a core option so you can turn it on/off, and even when it's on it will only create the scanline OBJ buffer if it detects that BG priority runs contrary to OAM order. The buffer is then only used for the specific OBJs that are 'hijacking'.

I think it gets used by more games than we think, just that the Golden Sun games are the really 'obvious' ones. I didn't know about Zelda until I saw it mentioned on Discord, and I found the Mario Golf one myself.

Anyway, I wanted to get it to a good place which I've done now, up to you if you think it's good enough to push.

andymcca avatar Oct 12 '25 06:10 andymcca

Tested in Golden Sun with the newest commits:

  • Waterfall glitch is fixed when the OAM core option is enabled, and still present when disabled
  • Start menu loads correctly with the OAM core option enabled
  • No apparent performance impact on low powered handhelds at least in this scene

Great stuff @andymcca! Even it only helps with a handful of games they're some of the most important ones on the system.

mitchchn avatar Oct 12 '25 16:10 mitchchn

Will check it out. Ideally this needs to be tested and find out:

  • Whether it works with every official game
  • How much performance is lost on average for most games.

davidgfnet avatar Oct 13 '25 22:10 davidgfnet