Draw actors at their sprite's height, and update each one once a frame
Closes Defects item 26 and Performance item 32. akgl_actor_render set dest.h from curSprite->width, so every actor was drawn square and a non-square sprite was stretched or squashed. Invisible in the fixtures because they are all square, so tests/actor.c gets a 48x24 sprite and a render backend whose draw_texture records the rectangle it is handed instead of drawing it. That recording backend is the first coverage akgl_actor_render has had at all -- every other test in the file stubs renderfunc out. akgl_game_update looped over AKGL_TILEMAP_MAX_LAYERS with the actor sweep nested inside it and never compared an actor's layer to the layer it was on, so every live actor's updatefunc ran sixteen times a frame. The sweep is hoisted out: updating an actor is not a per-layer operation, and akgl_render_2d_draw_world already walks the layers for the half that is. AKGL_ITERATOR_OP_LAYERMASK is honoured rather than ignored now, so a caller who wants one layer can still ask, and gets each of those actors once. Counting is the assertion, deliberately. The defect was invisible in the frame total because the tilemap blits are three orders of magnitude larger, so a timing test would have measured the rasterizer. tests/game.c counts calls into a stub updatefunc and reports 16 against the old code. PERFORMANCE.md records the timing side as what it honestly is: the gap between the akgl_game_update and draw_world rows of the same run, 92 us before and noise in both directions after. The absolute table is not re-taken -- a later run on this machine read every row about 15% high, including rows nothing has touched. 25/25 pass, reindent --check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
134
tests/actor.c
134
tests/actor.c
@@ -17,6 +17,7 @@
|
||||
#include <akgl/sprite.h>
|
||||
#include <akgl/game.h>
|
||||
#include <akgl/heap.h>
|
||||
#include <akgl/renderer.h>
|
||||
|
||||
#include "testutil.h"
|
||||
|
||||
@@ -892,6 +893,122 @@ akerr_ErrorContext *test_actor_sprite_sheet_coords(void)
|
||||
SUCCEED_RETURN(e);
|
||||
}
|
||||
|
||||
/*
|
||||
* A render backend whose draw_texture records what it was asked to draw instead
|
||||
* of drawing it. akgl_actor_render's whole output is the rectangle it hands to
|
||||
* draw_texture, so that rectangle is the only thing there is to assert on.
|
||||
*/
|
||||
static akgl_RenderBackend testbackend;
|
||||
static SDL_FRect lastdrawdest;
|
||||
static SDL_FRect lastdrawsrc;
|
||||
static int drawcalls = 0;
|
||||
|
||||
static akerr_ErrorContext *record_draw_texture(
|
||||
akgl_RenderBackend *self,
|
||||
SDL_Texture *texture,
|
||||
SDL_FRect *src,
|
||||
SDL_FRect *dest,
|
||||
double angle,
|
||||
SDL_FPoint *center,
|
||||
SDL_FlipMode flip)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
|
||||
if ( src != NULL ) {
|
||||
lastdrawsrc = *src;
|
||||
}
|
||||
if ( dest != NULL ) {
|
||||
lastdrawdest = *dest;
|
||||
}
|
||||
drawcalls += 1;
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief akgl_actor_render must take the drawn height from the sprite's height.
|
||||
*
|
||||
* It read `curSprite->width` for both, so every actor was drawn square and a
|
||||
* non-square sprite was stretched or squashed. Invisible in the fixtures
|
||||
* because they are square, which is exactly why this test uses a sprite that
|
||||
* is not: 48 wide by 24 high, so a regression puts 48 in both.
|
||||
*
|
||||
* This is also the first test of akgl_actor_render at all -- everything else in
|
||||
* this file stubs renderfunc out.
|
||||
*/
|
||||
akerr_ErrorContext *test_actor_render_uses_sprite_height(void)
|
||||
{
|
||||
PREPARE_ERROR(e);
|
||||
akgl_Actor *actor = NULL;
|
||||
akgl_Character *basechar = NULL;
|
||||
akgl_Sprite *sprite = NULL;
|
||||
SDL_Texture *sheettexture = NULL;
|
||||
|
||||
ATTEMPT {
|
||||
CATCH(e, make_bound_actor(&actor, &basechar, &sprite, "renderactor", AKGL_ACTOR_STATE_ALIVE));
|
||||
|
||||
// akgl_spritesheet_coords_for_frame reads sheet->texture->w, so the
|
||||
// sheet needs a real one even though nothing is rasterized.
|
||||
sheettexture = SDL_CreateTexture(
|
||||
testbackend.sdl_renderer,
|
||||
SDL_PIXELFORMAT_RGBA8888,
|
||||
SDL_TEXTUREACCESS_TARGET,
|
||||
96, 48);
|
||||
FAIL_ZERO_BREAK(e, sheettexture, AKGL_ERR_SDL, "%s", SDL_GetError());
|
||||
sprite->sheet->texture = sheettexture;
|
||||
|
||||
sprite->width = 48;
|
||||
sprite->height = 24;
|
||||
sprite->frames = 2;
|
||||
actor->curSpriteFrameId = 0;
|
||||
actor->visible = true;
|
||||
actor->scale = 1.0f;
|
||||
actor->x = 0;
|
||||
actor->y = 0;
|
||||
|
||||
akgl_camera->x = 0;
|
||||
akgl_camera->y = 0;
|
||||
akgl_camera->w = 640;
|
||||
akgl_camera->h = 480;
|
||||
|
||||
drawcalls = 0;
|
||||
TEST_EXPECT_OK(e, akgl_actor_render(actor), "rendering a non-square actor");
|
||||
TEST_ASSERT(e, drawcalls == 1, "actor render made %d draw calls, expected 1", drawcalls);
|
||||
|
||||
TEST_ASSERT_FEQ(e, lastdrawdest.w, 48.0f,
|
||||
"drawn width is %f, expected the sprite's 48", lastdrawdest.w);
|
||||
TEST_ASSERT_FEQ(e, lastdrawdest.h, 24.0f,
|
||||
"drawn height is %f, expected the sprite's 24 -- it used to take the width",
|
||||
lastdrawdest.h);
|
||||
|
||||
// The source rectangle has always come from the sprite properly; assert
|
||||
// it too, so a regression cannot move the bug one line up.
|
||||
TEST_ASSERT_FEQ(e, lastdrawsrc.w, 48.0f, "source width is %f, expected 48", lastdrawsrc.w);
|
||||
TEST_ASSERT_FEQ(e, lastdrawsrc.h, 24.0f, "source height is %f, expected 24", lastdrawsrc.h);
|
||||
|
||||
// Scale multiplies both, and must not collapse them back together.
|
||||
actor->scale = 2.0f;
|
||||
drawcalls = 0;
|
||||
TEST_EXPECT_OK(e, akgl_actor_render(actor), "rendering a scaled non-square actor");
|
||||
TEST_ASSERT_FEQ(e, lastdrawdest.w, 96.0f, "scaled width is %f, expected 96", lastdrawdest.w);
|
||||
TEST_ASSERT_FEQ(e, lastdrawdest.h, 48.0f, "scaled height is %f, expected 48", lastdrawdest.h);
|
||||
} CLEANUP {
|
||||
if ( sprite != NULL && sprite->sheet != NULL ) {
|
||||
sprite->sheet->texture = NULL;
|
||||
}
|
||||
if ( sheettexture != NULL ) {
|
||||
SDL_DestroyTexture(sheettexture);
|
||||
}
|
||||
if ( actor != NULL ) {
|
||||
IGNORE(akgl_heap_release_actor(actor));
|
||||
}
|
||||
if ( basechar != NULL ) {
|
||||
IGNORE(akgl_heap_release_character(basechar));
|
||||
}
|
||||
} PROCESS(e) {
|
||||
} FINISH(e, true);
|
||||
SUCCEED_RETURN(e);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
akgl_actor_updated = 0;
|
||||
@@ -910,6 +1027,22 @@ int main(void)
|
||||
CATCH(errctx, akgl_registry_init_spritesheet());
|
||||
CATCH(errctx, akgl_registry_init_character());
|
||||
|
||||
// A renderer, so akgl_actor_render can be driven at all. The backend is
|
||||
// bound and then has its draw_texture replaced, which is the point:
|
||||
// what the function computes is only visible in what it hands over.
|
||||
if ( !SDL_Init(SDL_INIT_VIDEO) ) {
|
||||
FAIL_BREAK(errctx, AKGL_ERR_SDL, "Couldn't initialize SDL: %s", SDL_GetError());
|
||||
}
|
||||
memset(&testbackend, 0x00, sizeof(akgl_RenderBackend));
|
||||
if ( !SDL_CreateWindowAndRenderer("net/aklabs/libakgl/test_actor", 640, 480,
|
||||
SDL_WINDOW_HIDDEN, &akgl_window, &testbackend.sdl_renderer) ) {
|
||||
FAIL_BREAK(errctx, AKGL_ERR_SDL, "Couldn't create window/renderer: %s", SDL_GetError());
|
||||
}
|
||||
CATCH(errctx, akgl_render_2d_bind(&testbackend));
|
||||
testbackend.draw_texture = &record_draw_texture;
|
||||
akgl_renderer = &testbackend;
|
||||
akgl_camera = &akgl_default_camera;
|
||||
|
||||
CATCH(errctx, test_registry_actor_iterator_nullpointers());
|
||||
CATCH(errctx, test_registry_actor_iterator_missingactor());
|
||||
CATCH(errctx, test_registry_actor_iterator_updaterender());
|
||||
@@ -925,6 +1058,7 @@ int main(void)
|
||||
CATCH(errctx, test_actor_update());
|
||||
CATCH(errctx, test_actor_character_sprite_binding());
|
||||
CATCH(errctx, test_actor_sprite_sheet_coords());
|
||||
CATCH(errctx, test_actor_render_uses_sprite_height());
|
||||
} CLEANUP {
|
||||
} PROCESS(errctx) {
|
||||
} FINISH_NORETURN(errctx);
|
||||
|
||||
Reference in New Issue
Block a user