-
Notifications
You must be signed in to change notification settings - Fork 167
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
Introduce batching engine and shadow batching #847
base: develop/3.0.0
Are you sure you want to change the base?
Conversation
include/sm64.h
Outdated
enum BatchAlpha { | ||
// coin frames batches, it goes 0, 1, 2, 3, 4, 3, 2, 1 | ||
BATCH_ALPHA_COINS_FIRST, | ||
BATCH_ALPHA_COINS_LAST = BATCH_ALPHA_COINS_FIRST + 4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk how I feel about this, what does the 4 actually represent here, number of coin textures? If so, that should be defined somewhere, ideally automatically computed or enforced with a static assert if possible.
Ditto for the flames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the easiest solution is to add a static assert. sm64.h
is a glue between subsystems geo->layer->batches->dls. It would be hard to deduce amount of coin textures without including actors in sm64.h which is not something I want to do.
For now I will remove those enums because they are unused anyways and reland them later with batching for specific objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have provided the static asserts for constant batches, it actually appeared to be pretty easy to implement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could these be moved to batch_list.h
? Idk if they make the most sense in sm64.h
anyway, though to be fair the layer definitions are also here...
src/game/rendering_graph_node.c
Outdated
|
||
static void main_render(Gfx **ptempGfxHead, struct DisplayListNode* currList, u32 mode1, u32 mode2, int phaseIndex) | ||
{ | ||
(void)phaseIndex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a leftover compiler warning bypass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I prefer to be explicit here when silhouette is off. I have moved it under SILHOUETTE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you leave a short comment here too just to clarify it's a compiler warning bypass? You could also use the UNUSED keyword (though this is still messy)
This PR is introducing changes from #720 but in a more robust way by introducing generic batching engine. LayerBatches has unused values that are proposed for future if this PR will be accepted.