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

taito/taito_f3_v.cpp: major rewrite #11811

Merged
merged 90 commits into from
Apr 23, 2024
Merged

taito/taito_f3_v.cpp: major rewrite #11811

merged 90 commits into from
Apr 23, 2024

Conversation

y-ack
Copy link
Contributor

@y-ack y-ack commented Dec 2, 2023

  • 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, moriguchi hiroko no quiz de hyuu!hyuu!)
  • 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)
  • emulate blending priority conflict (darius gaiden, bubble memories, grid seeker)

MT#8697, MT#2527, MT#1922, MT#26, MT#2733, MT#3741, MT#2002, MT#1907, MT#7321, MT#2097, MT#1924, MT#1923, MT#8827, GH #10033

Systems promoted to working

Moriguchi Hiroko no Quiz de Hyuu!Hyuu! [ywy, 12Me21]

y-ack and others added 5 commits December 2, 2023 05:56
* nicer sprite data access

* clean up bitmasks, x/y position reading

* simplify logic, eliminate per-game hack "dariusg kludge"
@y-ack
Copy link
Contributor Author

y-ack commented Dec 2, 2023

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

  • basic sprite drawing
  • basic tilemap drawing
  • basic pivot layer drawing
  • flipscreen
  • more accurate sprite scaling behavior
  • line alternate tilemaps
  • line column scroll
  • line clipping
  • line pivot control -- 24.4.2: total mystery, needs HW testing
  • line blend
  • 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
  • line pivot alpha select bit (theorized dark select bit ?)
  • line pivot priority
  • line sprite clipping
  • line sprite brightness alpha select bit
  • line sprite priority
  • line playfield scaling
  • more accurate playfield x scaling -- 24.4.2: needs HW testing
  • line playfield palette addition
  • line playfield rowscroll
  • line playfield priority
  • line playfield clipping
  • layer additive blending
  • line subsection bank switching (bubblem)
  • 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
Copy link
Contributor Author

y-ack commented Apr 5, 2024

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
Copy link
Contributor Author

y-ack commented Apr 5, 2024

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?

u16 maybe_sync_reg{0};
bool no_opaque_dest{false};
// 6200
u8 blend[4]{0}; // less 0 - 8 more
Copy link
Member

@happppp happppp Apr 5, 2024

Choose a reason for hiding this comment

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

FYI initializing arrays like this, it will only initialize the 1st index, and fill the rest with 0. Best use {} like you did elsewhere. Same result in this specific case but less misleading.

see:
int m_array[4] { 1 };
for (int i = 0; i < 4 ; i++) printf("%d ",m_array[i]);
-> 1 0 0 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks; fixed.

@happppp
Copy link
Member

happppp commented Apr 5, 2024

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.

@cuavas
Copy link
Member

cuavas commented Apr 5, 2024

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.

@y-ack
Copy link
Contributor Author

y-ack commented Apr 5, 2024

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?)

@TheCoolPup
Copy link

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

@y-ack
Copy link
Contributor Author

y-ack commented Apr 7, 2024

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
Copy link
Contributor Author

y-ack commented Apr 9, 2024

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 y-ack requested a review from cuavas April 9, 2024 22:09
@angelosa
Copy link
Member

angelosa commented Apr 10, 2024

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

@12Me21
Copy link

12Me21 commented Apr 10, 2024

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

@angelosa
Copy link
Member

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.

@y-ack
Copy link
Contributor Author

y-ack commented Apr 13, 2024

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)

@angelosa angelosa merged commit 563b63f into mamedev:master Apr 23, 2024
5 checks passed
@dinkc64
Copy link

dinkc64 commented Apr 25, 2024

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

@CPS42069
Copy link

CPS42069 commented May 7, 2024

0000

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

@12Me21
Copy link

12Me21 commented May 7, 2024

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)

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.

None yet

8 participants