konami/mystwarr_v.cpp, k053246_k053247_k055673.cpp: improve sprite blending #13328
Conversation
mark sprites with their attribute bits for blending
-move variable declarations closer to use -use appropriate variable types -mark applicable variables as const -move "cull off-screen objects" early out check earlier in function
|
I'm not gonna judge the changes here (they looks reasonable to me), you may give a shot at |
|
Good call on dadandrn! I had trouble finding real HW footage so I didn't dig into it earlier. Found a video here to compare against. I switched it over to using the mystwarr sprite callback and blending does seem to work! It was using the gaiapols callback which is slightly different, so I will play through the rest of the game and see if anything looks out of place. Will check sexyparo soonish, whenever I have time. |
|
I pushed the There is some other problem with blending, maybe overlapping blended sprites? This can be seen in |
-move back "dst_" vars under appropriate comment - further reduce reuse of "eax" variable
|
(accidentally closed request somehow, don't mind that) |
|
While you're at it, could you rename "eax" to something more contextual like "delta"? I mean for the ones you already modified, such as: |
-use cliprect directly inside function -rename various "eax" variables
|
Yes, you're right, that should be done. It seems like most of the "eax" vars are either a distance between a clipping rect and a sprite coord, or a sprite tile byte used as a palette index. I've tried to at least make the names a little more descriptive. |
|
I took a look at The first boss in the boss rush is some blue slime with a coin inside. That thing appears to use blending and seems to work as intended in mame. |
|
iirc the very same path is used by Castlevania stage for the glass background meshes (which should blend, around the boss).
|
| #define FP 19 | ||
| #define FPONE (1<<FP) | ||
| #define FPHALF (1<<(FP-1)) | ||
| #define FPENT 0 |
There was a problem hiding this comment.
Can you turn these into constexpr int or constexpr unsigned while you’re at it? Preprocessor macros have a habit of causing difficult to diagnose errors.
There was a problem hiding this comment.
I changed these to constexpr int and realized that FPONE and FPHALF go unused. Should I comment them out or outright remove them?
There was a problem hiding this comment.
Just remove them, the definitions contain no unique information as they’re just derived from FP.
src/mame/konami/mystwarr_v.cpp
Outdated
| *color = (*color & 0x1f) | m_sprite_colorbase | effect_attributes; | ||
|
|
||
| // Bit8 & 9 are effect attributes. It is not known whether the effects are generated by external logic. | ||
| if ((attr & 0x300) != 0x300) | ||
| { | ||
| *color = c; | ||
| *priority_mask = (attr & 0xe0) >> 2; | ||
| } | ||
| else | ||
| { | ||
| *color = c | 3<<K055555_MIXSHIFT | K055555_SKIPSHADOW; // reflection? | ||
| *priority_mask = 0x1c; | ||
| } | ||
| if (shadow == 0b11) { *color |= K055555_SKIPSHADOW; } |
There was a problem hiding this comment.
You haven’t used braces on your other single line if statements (e.g. the if (foo) return; in k053246_k053247_k055673.cpp – why have them here? You could avoid the if anyway if you wanted, like:
*color = (*color & 0x1f) | m_sprite_colorbase | effect_attributes | ((shadow == 0b11) ? K055555_SKIPSHADOW : 0);There was a problem hiding this comment.
I didn't pay enough attention there, I've changed it to use your suggestion, thanks!
I see!! Yeah, I can tell there's still things to improve on. |
|
someone trying to fix video emulation for konami stuff??? its a mircale! |
|
also i have noticed one regression, if you look at eagle's teeth, they seem to be the wrong color |
|
Yep, noticed the teeth as well! Will have to investigate |
|
so the water is a blend issue? |
|
also here is some pcb footage if you want it https://youtu.be/iuLQnJeBKZo?feature=shared&t=6197 |
|
The teeth regression appears to have been a priority mask issue. I updated the As a bonus, it also fixes the final boss's monitors in mystic warriors! Thanks for pointing it out.
It's possible! This PR is only focusing on sprites, though. I think this is all I will get done for this PR (unless there are other suggestions / objections). |
|
So happy someone is working on fixing the issues these games have had, thank you again |
|
Alright, so I went ahead and made one last addition. I'm sure I'm being annoying now 😅. In my testing all current changes come out ~7% faster than current mame, so it seems the compiler can work with drawmode checks inside the loop. |
This pull request is split into three commits:
d0faecd
The sprite effect attribute bits in mystwarr, metamrph and viostorm are used for sprite blending purposes. I changed the sprite callbacks to attach the attribute bits.
e7f911e
Blending was being done in reverse. There are probably several reasons why this was never noticed, but two reasons would be that 0% and 100% blending worked as intended and almost no games use sprite blending (at least in current MAME). Simple solution: reverse the operands to alpha_blend_r32().
8184130
Optional commit: minor cleanup of zdrawgfxzoom32GP(), the function that contains the alpha_blend_r32() calls. Initially I had planned to make bigger changes, like condensing the switch-cases, but I wasn't satisfied with my attempts at doing so. Instead I opted for a more conservative cleanup pass. It appears to give a marginal speedup as a bonus.
This is the least important commit of the three, so if it's deemed unnecessary it can be dropped.
This fixes transparent sprites in mystwarr, metamrph and viostorm. Mystwarr & viostorm make little use of these; it's a bit more apparent in metamrph.
I assume this affects other systems too - like konami GX? I ran all GX games with -bench 400 and didn't run into any sprites that use any transparency. Either GX games do not have transparent sprites or they're not tagged as such in current MAME. This is hardly the perfect testing method, so please let me know if I should be testing any other games and systems.