Add detail texture support to rdpq_sprite_upload #380
Conversation
include/rdpq.h
Outdated
| * @brief Set the RDP MIN LOD combiner register (RDP command: SET_PRIM_COLOR (partial)) | ||
| * | ||
| * This function sets the internal RDP PRIM MIN LOD register, that is used for | ||
| * determining the interpolation blend factor of a detail texture. |
There was a problem hiding this comment.
Should we maybe rename it in another way? I don't know the story behind "MIN LOD", but if we use it as blend factor for detail texture, maybe it should be rdpq_set_blend_detail_factor() or something like that.
include/sprite.h
Outdated
| * @param sprite The sprite to access | ||
| * @return Blend factor if sprite has detail texture, 0 otherwise | ||
| */ | ||
| float sprite_detail_get_factor(sprite_t *sprite); |
There was a problem hiding this comment.
this is a bit wide as an API surface for a single feature. My suggestion is to add a sprite_detail_t structure, and then use a single API like this:
surface_t sprite_get_detail_pixels(sprite_t *sprite, sprite_detail_t *info);so that you return the surface and the info (maybe the info can even be optional, if NULL).
src/rdpq/rdpq_sprite.c
Outdated
| if (!parms && sprite_get_texparms(sprite, &parms_builtin)) | ||
| parms = &parms_builtin; | ||
|
|
||
| // Check for detail texture | ||
| sprite_detail_t detailinfo; | ||
| sprite_get_detail_texparms(sprite, &detailtexparms); |
There was a problem hiding this comment.
I'd move this call in the if(use_detail) below. Moreover, you can use the boolean return value: if the value is false, you can call rdpq_tex_upload passing NULL instead of &detailtexparms. Passing NULL to rdpq_tex_upload is always faster as it skips the whole tile parms calculation.
src/rdpq/rdpq_tex.c
Outdated
| int bytes; | ||
| int limit; | ||
|
|
||
| tex_loader_t last_tload; |
There was a problem hiding this comment.
since we're using this also outside of the multi sequence, maybe we should also move this outside of rdpq_multi_upload_t, for clarity. I'm not super excited about always using a global instance of texloader rather than a stack instance like before, but I guess we can live with this for a while.
src/rdpq/rdpq_tex.c
Outdated
| assertf(tmem_offset % 8 == 0, "Offset %i must be in multiples of 8", tmem_offset); | ||
| tex_format_t fmt = surface_get_format(tload->tex); | ||
|
|
||
| rdpq_set_tile(tload->tile, fmt, tmem_offset, tload->rect.tmem_pitch, &(tload->tileparms)); |
There was a problem hiding this comment.
shouldn't this be tload->tmem_addr + tmem_offset? Also how does this work if tmem_addr is already precomputed as part of the autotmem allocator?
src/rdpq/rdpq_tex.c
Outdated
| multi_upload_bytes = 0; | ||
| multi_upload_limit = 4096; | ||
| if (multi_upload.used++ == 0) { | ||
| multi_upload.used = true; |
There was a problem hiding this comment.
I think this line is spurious? used is a counter
Co-authored-by: Giovanni Bajo <rasky@develer.com>
Basically the title.
The detail texture can now be created using mksprite and uploaded alongside the main texture and/or its mipmaps.
This change also introduces a few new modes and functions
(not yet ready in PR, hopefully fixed soon)