mame icon indicating copy to clipboard operation
mame copied to clipboard

taito/taito_f3_v.cpp: major rewrite

Open y-ack opened this issue 1 year ago • 1 comments

[wip]

  • simplify sprite logic, eliminate game-specific kludge
  • simplify line ram access
  • simplify draw loop
  • fix sprite bankswitch - riding fight sprites now 60fps
  • more accurate sprite scaling
  • x mosaic effect (ring rage, riding fight, command war)
  • screen raw parameters based on others' measurements and mosaic quirk
  • identify mosaic flags for sprites and pivot layer
  • re-derive blending rules (riding fight, space invaders dx, elevator action returns, recalhorn, gekirindan, bubble memories, cleopatra fortune, puchi carat, land maker)
  • identify sprite and pivot blend value select (arabian magic, grid seeker)
  • line ram alternate subsections (bubble memories)
  • sprite framebuffer unblit disable (sprite trails) (darius gaiden)

todo: cleanups and fixing oversights

y-ack avatar Dec 02 '23 22:12 y-ack

very rough draft pr for tracking, not ready for review. progress may be slow until late december.

  • [x] basic sprite drawing
  • [x] basic tilemap drawing
  • [x] basic pivot layer drawing
  • [x] flipscreen
  • [x] more accurate sprite scaling behavior
  • [x] line alternate tilemaps
  • [x] line column scroll
  • [x] line clipping
  • [ ] line pivot control -- 24.4.2: total mystery, needs HW testing
  • [x] line blend
  • [x] line x sample/mosaic
  • [ ] line x forward blend -- 24.4.2: near palette interpretation
  • [ ] line bg palette -- 24.4.2: unknown use
  • [ ] line pivot enable -- 24.4.2: total mystery, needs HW testing
  • [x] line pivot alpha select bit (theorized dark select bit ?)
  • [x] line pivot priority
  • [x] line sprite clipping
  • [x] line sprite ~~brightness~~ alpha select bit
  • [x] line sprite priority
  • [x] line playfield scaling
  • [ ] more accurate playfield ~~x~~ scaling -- 24.4.2: needs HW testing
  • [x] line playfield palette addition
  • [x] line playfield rowscroll
  • [x] line playfield priority
  • [x] line playfield clipping
  • [x] layer additive blending
  • [x] line subsection bank switching (bubblem)
  • [x] bubblem/spcinvdj sprite blending/late subsection bug..

first pass forgoes optimizations in order to later debug blending behavior. when closer to ready, performance regression should be evaluated and optimization passes may be necessary. notable optimizations that have currently been lost:

  • [ ] priority buffer / opaque layer skip
  • [ ] line draw grouping by constant effect states

y-ack avatar Dec 02 '23 22:12 y-ack

What's the state of this? I do believe it's already better to what's in bleeding edge so can be marked as ready?

angelosa avatar Mar 22 '24 22:03 angelosa

status is.. not 100% correct (even blending) but code in a place where it can reasonably be worked on. not totally opposed to putting one or two hacks in place to get this through and allow smaller iterative prs on top of it...

  • mosaic implemented (ringrage, ridingf, commandw, quizhuhu etc.)
    • promotes quizhuhu to working
    • ~~doesn't yet emulate the column 366 counter reset quirk~~
  • sprite scaling improved (accuracy and code)
    • closes mt#8697 (kaiserknj)
    • not 100% hardware accurate in some cases?
  • sprite rendering improvements: gets rid of dariusg kludge, fixes ridingf sprite framerate, closes mt#2527 (rayforce), mt#1922 (dariusg)
  • blending improvements
  • implemented alternate line ram subsections: closes mt#7321 (bubblem)
  • implemented blending priority conflict quirk: closes mt#7321 (bubblem), mt#1924 (dariusg)

currently known regressions:

  • ✅~~arabianm, after stage 3 boss, sprite block attribute locking causes corrupted scaled sprites. understanding this edge case properly is expected to take a lot of testing. a very simple unobtrusive game-specific hack for it is possible.~~ 63d0306
  • gseeker hangar shadow (mt#1187), the hack that put the plane over the shadow was eliminated, so the plane is now underneath the shadow layer.
  • ✅~~we also disabled the 'sprite lag' behavior which should probably be re-enabled for now.~~ 2e857cc

notable known remaining issues (not regressions):

  • mt#1923
    • ~~some blending assumption is still wrong, but its effects are limited to this and gekiridn?~~ 11b190e
  • scfinals 'replay' is choppy
    • something about sprite ram write timing? fbneo doesn't have this issue.
  • mt#8230 (rayforce shadows), mt#1187 (gseeker shadows)
    • my current belief is that there are bits at 7400(for sp) and 6400(for pfs) that can cause a layer to be darkened by a layer underneath it... /separately/ from the priority conflict that dariusg uses for a similar effect
  • playfield scaling accuracy (ridingf ending, scfinals name entry)
  • palette interpretation bits must be respected for the gseeker intro zoom

other mametesters issues: mt#1172 (kaiserkn shadows) - ??? they are green, just dark, possibly gamma issue ? but need to check the blending again just in case mt#956 (trstar attract text) - don't believe in this without pcb proof mt#1171 - btanb mt#7776 - dariusg zone D - btanb??? behavior is on pcb ?

performance regression: in a not-at-all-controlled-environment average-of-3 test of the windows build artifact: mame.exe -w -resolution 320x232 ridingf -bench 120 this @ 929d445: 329.26% bleeding edge: 494.04%

(unacceptable by itself, but also totally unexplored.)

y-ack avatar Mar 26 '24 11:03 y-ack

fyi I don't mind if this will end up having a (relatively negligible) performance hit.

angelosa avatar Mar 26 '24 11:03 angelosa

okay, this is probably ready for review now.

show recordings of most visible changes note: gamma is set to 0.8 in all recordings

https://github.com/mamedev/mame/assets/12588017/465ff6f9-e902-4105-befd-8b5797a5ab2c

https://github.com/mamedev/mame/assets/12588017/ff3a0a98-7649-48ee-aa9f-18096adffb89

https://github.com/mamedev/mame/assets/12588017/38f90533-512c-4882-96ea-cf410ceb6e08

https://github.com/mamedev/mame/assets/12588017/25840463-6c5e-410c-92db-2a957a793e24

https://github.com/mamedev/mame/assets/12588017/3c74c668-b1e7-4255-a90e-75198de81f3b

https://github.com/mamedev/mame/assets/12588017/29af0f0b-c8d9-4580-ad68-47137eb7210e

https://github.com/mamedev/mame/assets/12588017/5183b615-d0c8-4b73-9d8a-de15d3f55d23

https://github.com/mamedev/mame/assets/12588017/77658f30-d9bb-4212-82f5-4cb0d58ea340

https://github.com/mamedev/mame/assets/12588017/badf5415-c4f4-477d-b287-1e4b14b4d01e

https://github.com/mamedev/mame/assets/12588017/a3779d27-28fb-43c9-b1b1-02127640a544

https://github.com/mamedev/mame/assets/12588017/13415844-298d-486c-96c7-1b4657243042

https://github.com/mamedev/mame/assets/12588017/35d6388d-ecd2-498c-8430-e9a0ada81261

https://github.com/mamedev/mame/assets/12588017/45a6c05c-e885-42e1-a81c-bf9727a564a8

https://github.com/mamedev/mame/assets/12588017/fe8f60a5-53e0-4c3c-afc9-41a6375b4022

y-ack avatar Apr 02 '24 16:04 y-ack

I guess it's safe that you can promote quizhuhu already from this very PR. A commit with:

Systems promoted to working
---------------------------
Moriguchi Hiroko no Quiz de Hyuu!Hyuu! [<your preferred username handle>]

Should be enough to be catched later by MAME release script, cfr. whatsnew patterns at https://github.com/mamedev/build/blob/master/whatsnew/whatsnew_template.txt

angelosa avatar Apr 03 '24 17:04 angelosa

Regression for Elevator Action Returns, the airplane section is broken now Elevator Action Returns (Ver 2 2O 1995_02_20)  elvactr  - MAME 0 264 (LLP64) 4_4_2024 4_17_16 PM Elevator Action Returns (Ver 2 2O 1995_02_20)  elvactr  - MAME 0 264 (LLP64) 4_4_2024 5_09_05 PM

TheCoolPup avatar Apr 04 '24 21:04 TheCoolPup

Regression for Elevator Action Returns, the airplane section is broken now

hi @TheCoolPup, thanks for finding this. it looks like playfields and sprites get disabled when set to the 'wrong' opaque mode.

y-ack avatar Apr 05 '24 02:04 y-ack

Before anything else, please clean up your changes to match the existing code in the files you’re working on.

Please Allman formatting consistently (opening braces on their own lines).

@cuavas we've rewritten 90% of the code already, it's easier at this point to switch to (consistently) k&r. my understanding is that no one else has seriously worked on the driver in years, i hope it's ok?

y-ack avatar Apr 05 '24 03:04 y-ack

If it is as you say and 90% of the file is yours (refactors don't count), then yes source code style choice is yours. Also, good time to include yourself to copyright holders.

happppp avatar Apr 05 '24 11:04 happppp

I don’t want it to end up with different formatting for the different Taito F3 source files, though. There’s taito_f3.cpp as well.

cuavas avatar Apr 05 '24 13:04 cuavas

thanks for the comments. i agree that having different styles across the taito f3 files should be avoided. taito_f3.cpp has only 4 functions that would need reformatting. we hadn't touched it yet because there is so much to clean up there* (and heavily mame-specific idioms that require finding modern driver examples) that it seemed like the increased scope warranted focusing on in a separate PR. this one was supposed to address the video rendering primarily.

[*] for example:

  • the cpu clocks are wrong (as noted elsewhere in the file, it's 15.23809MHz, not 16)
  • we need to do more hardware testing to figure out how the interrupts are generated (there seems to be an hblank interrupt which not all games enable)
  • tile decoding wants refactoring?
  • repetitive init routines for almost all sets feels really bad (though maybe that's just necessary and normal for mame?)

y-ack avatar Apr 05 '24 15:04 y-ack

Forgot to take screenshots but two other bugs i noticed is that in stage 4 when you are underground when you go through a subway car it doesn’t show you in it, also when you stop the missile at the end of the final stage it’s supposed to fade to white but instead it just turns black until the credits start

TheCoolPup avatar Apr 07 '24 15:04 TheCoolPup

in stage 4 when you are underground when you go through a subway car it doesn’t show you in it, also when you stop the missile at the end of the final stage it’s supposed to fade to white but instead it just turns black until the credits start

@TheCoolPup: the first one was the same bug as before; the second is the 'background palette' effect (so probably not a regression). both should be fixed in the latest commit.

y-ack avatar Apr 07 '24 20:04 y-ack

if the brace formatting not matching allman in mame core really is the blocking issue here, i will change it, but i push back in the interest of reducing friction for my co-contributor and i to continue working on the driver comfortably and maintain pace, not from a place of disrespect.

y-ack avatar Apr 09 '24 22:04 y-ack

The alternative would be "go to the other end and use K&R everywhere in these files, as long as it's consistent".

angelosa avatar Apr 10 '24 00:04 angelosa

The alternative would be "go to the other end and use K&R everywhere in these files, as long as it's consistent".

she preemptively tentatively did that in 37ad667 and fce4bd9

12Me21 avatar Apr 10 '24 00:04 12Me21

imo taito_f3_v.cpp will ultimately require a split into device(s) so it makes sense to have that with whatever style you prefer. As for regular taito_f3.cpp :

  • f3_coin_r/f3_control_r/_w/f3_analog_r all belongs to TC0640FIO, which we already have a core of;
  • sound_reset_0_w/sound_reset_1_w can clearly be expressed as lambda nowadays;
  • Not sure about that kirameki soundbank interaction, by face value I guess it can be removed by loading sample ROMs properly (while hoping the game has a sound test that covers those);
  • gfxdecode, set_raw etc. belongs to the video device, so original point stands;

All of this is non-blocking for the PR, and there are roughly two weeks for the next release.

angelosa avatar Apr 10 '24 11:04 angelosa

i think our roadmap from here is:

  • performance work (this is already done, but want to split into another pr to handle any contentiousness separately, and if this one is "approach readability to fix blending"), could be this cycle.
  • device-ify (and timings) (this might take a little longer given relative unfamiliarity, but it's something i've had in the back of my mind the whole time)
    • maybe rework for partial screen updates at this time too
    • the existing FIO device may need work
    • not sure if we want to consider the FDP and FDA as separate units, since as far as we can tell the FDA would just be a single function (color lookup/blending) and it's not known what the inputs to that look like outside of the context of the FDP.
    • have to consider seoul symphony bootleg board in this?
    • kirameki sound banking - we don't have the cart pcb and would like to take a look at it first to make sure, but this probably really is banking.
  • miscellaneous fixes: playfield scaling/scroll correctness (ridingf/scfinals etc.), palette interpretation, secret third way to generate shadows (rayforce/gseeker), sprite ram write timing(?) (scfinals)

y-ack avatar Apr 13 '24 19:04 y-ack

y-ack, I missed this 2 weeks ago, but, seriously: congrats y-ack & 12Me21! What an awesome feat! :)

dinkc64 avatar Apr 25 '24 12:04 dinkc64

0000

the classic 5th boss explosion bug in Grid Seeker is still there, tested on latest nightly build.

CPS42069 avatar May 07 '24 16:05 CPS42069

the classic 5th boss explosion bug in Grid Seeker is still there, tested on latest nightly build.

yes, this is caused by the game writing sprites past the end of the sprite list and then never clearing them. from hardware testing a few days ago, it seems like only the first ~864 sprites are rendered, rather than 1024 like was previously assumed. (it should be fixed in a future pr, after we find the exact limit)

12Me21 avatar May 07 '24 16:05 12Me21