search

libretro / virtualjaguar-libretro

Hard fork of Virtual Jaguar (abandoned project) to Libretro
31 stars 34 forks source link

Replace bitwise operands with Structs for performance #58

Closed JoeMatt closed 2 years ago

inactive123 commented 2 years ago

Black screen with Rayman (World). Corrupted graphics and black screen with Tempest 2000. Black screen with Doom (World).

JoeMatt commented 2 years ago

Ready for review Tested Doom, NBA Jam and Tempest 2000

inactive123 commented 2 years ago

Doom, Tempest 2000, Wolfenstein 3D and others still don't work with this PR.

I suggest we start all over from scratch with this and split this up into separate PRs. It's not wise to do so many changes in one big omnibus PR without proper regression testing.

JoeMatt commented 2 years ago

This is already split up into the smallest denomination of changes to not lose my sanity.

I had all those roms working on my end and had the bugs down to 1 line in 1 commit, a program counter that got overwritten.

someone else can split this up but I'm going to continue to work on this PR as-is on my own.

I have other obligations this week, I'll get back to it shortly.

Edit: I'm still having no issues on iOS or Mac M1 with those 3 roms.

I need to setup an intel build environment, it might just be a simple endian bug in one of the structs (though Provenance in iOS simulator runs it fine which would be AMD64)

inactive123 commented 2 years ago

I'm testing it on Windows 10/11, on an intel x64 CPU.

inactive123 commented 2 years ago

This is simply a no-go in its current state. Too many things are changed, not enough testing has been done outside of Mac/iOS, the regressions that already exist with this are already way too high and the chance for future regressions even higher still.

This entire PR needs to be entirely redone from the ground up and split up into bite-sized chunks that each get tested on at least Windows, Linux, and Mac. These PRs should each be rather isolated and not try to do too much things at once, so that we have a better way of chasing after any potential regressions. Also, the testing surface area needs to be bigger Just testing it on Mac alone and then only on ARM is simply not going to lead to results that can eventually be merged, libretro targets way more systems than just that.

JoeMatt commented 2 years ago

I'm well aware of that. This PR is the last part of the already split up larger PR. I have testing environments for other arches. I've just been busy with holidays and job interviews.

There's a reason this is still marked draft.

Note, I've also tested intel since I have Provenance running on MacOS catalyst and the simulator builds x86 native. I know that works fine here.

Also I think this PR is as small as it goes, it's a change in the memory data struct and the code that calls it. I don't think it's possible to go any smaller but I'll look into it when I get back to this.

edit: I actually think this can be split into 3 parts.

  1. Const additions
  2. GPUControl struct
  3. Blitter commands as structs
inactive123 commented 2 years ago

OK, it being split up in 3 parts like that would be better, and would allow us to test easier for regressions across platforms.