diff --git a/PERFORMANCE.md b/PERFORMANCE.md index 38c06a8..b281d91 100644 --- a/PERFORMANCE.md +++ b/PERFORMANCE.md @@ -167,6 +167,14 @@ rows are raw `SDL_RenderTexture` loops with no libakgl in the path. | `draw_world`, 1200 tiles + 64 actors | frame | 16,484,493.6 | 61 | | `akgl_game_update`, full frame | frame | 16,576,902.2 | 60 | +The `akgl_game_update` row is from before the sixteen-times-per-actor defect was +fixed in 0.5.0; see below. It sat about 92 µs above `draw_world` in the same +run, and now sits level with it. The absolute figures in this table have not +been re-taken -- a later run on this machine read every row about 15% high, +including rows nothing has touched, so the table is a record of one machine +state and the honest measurement of that fix is the *gap* between two rows of +the same run, not a new absolute. + ## The frame budget At 60 fps a frame is 16.67 ms. Here is where it goes for a 640x480 game with a @@ -247,16 +255,30 @@ The design conclusion is not "make errors cheaper". It is that a *routine* condition should not be reported as an error. A character that has no sprite for a state should answer that question with a boolean. -### `akgl_game_update` updates every actor sixteen times +### `akgl_game_update` updated every actor sixteen times — fixed in 0.5.0 -`src/game.c:617` loops over `AKGL_TILEMAP_MAX_LAYERS`, and the actor sweep -nested inside it does not filter by layer. Every live actor's `updatefunc` runs -**16 times per frame**. At 68.4 ns per update and 64 actors that is 70 µs of -work to do 4.4 µs of work. +`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. +Every live actor's `updatefunc` ran **16 times per frame**: at 68.4 ns per +update and 64 actors, 70 µs of work to do 4.4 µs of it — and, more to the +point, every piece of a game's own per-frame actor logic running sixteen times. -It is invisible in the frame total here because the tilemap blits are three -orders of magnitude larger. On a GPU backend, where the frame might be 2 ms, it -is 3.5% of the frame doing nothing. Filed in `TODO.md`. +The sweep is hoisted out of the layer loop. Updating an actor is not a per-layer +operation; drawing is, and `akgl_render_2d_draw_world` walks the layers itself. +`AKGL_ITERATOR_OP_LAYERMASK` is honoured now rather than ignored, so a caller +that genuinely wants one layer can still ask for it and gets each of those +actors once. + +**How it was measured.** `tests/game.c` counts calls into a stub `updatefunc` +and asserts exactly one per live actor per `akgl_game_update`; against the old +code it reports 16. The timing side is the gap between the `akgl_game_update` +and `draw_world` rows of the *same* benchmark run: 92 µs before, and across +three runs afterwards -414 µs, -376 µs and +19 µs — noise in both directions. + +It was invisible in the frame total because the tilemap blits are three orders +of magnitude larger, which is exactly why it needed a counting test rather than +a stopwatch. On a GPU backend, where a frame might be 2 ms, it was 3.5% of the +frame doing nothing. ### Text has no cache at all diff --git a/TODO.md b/TODO.md index d9c896e..3c9f57d 100644 --- a/TODO.md +++ b/TODO.md @@ -975,13 +975,15 @@ without coming here first. Ordered by blast radius. `src/character.c:124`. 26. **`akgl_actor_render` computes a sprite's drawn height from its width.** - `src/actor.c:276` sets `dest.h = curSprite->width * obj->scale` where it - should read `curSprite->height`. Every actor is therefore drawn square, and - a non-square sprite is stretched or squashed. Invisible in the current - assets because they are square; it will not stay that way. + **Fixed in 0.5.0**: `dest.h` takes `curSprite->height`. Every actor was + drawn square, so a non-square sprite was stretched or squashed -- invisible + in the fixtures because they are all square. - Fix: one word. Touches `src/actor.c:276`. Worth a test that renders a - deliberately non-square sprite and asserts on the destination rectangle. + `tests/actor.c` renders a 48x24 sprite through a backend whose + `draw_texture` records the rectangle it is handed rather than drawing it, + and asserts the destination is 48x24 and 96x48 at scale 2. That recording + backend is also the first coverage `akgl_actor_render` has had at all -- + every other test in the file stubs `renderfunc` out. ### Found while closing the akbasic API gaps @@ -1095,18 +1097,25 @@ Ordered by blast radius. Numbering continues the **Defects** list above. workaround and no longer has to. 32. **`akgl_game_update` runs the actor update sweep once per tilemap layer.** - `src/game.c:617` loops `i` over `AKGL_TILEMAP_MAX_LAYERS` and the actor - sweep nested inside it never compares `actor->layer` to `i`, so every live - actor's `updatefunc` runs **16 times per frame**. Measured: 68.4 ns per - update, 64 actors, so 70 µs of work to do 4.4 µs of work. + **Fixed in 0.5.0.** The sweep is hoisted out of the layer loop -- updating + an actor is not a per-layer operation, and `akgl_render_2d_draw_world` + already walks the layers for the half that is. - It hides behind the rasterizer today (a 640x480 software frame is 16 ms) and - it will not hide behind a GPU backend, where a frame is nearer 2 ms and this - is 3.5% of it. Fix: either filter by layer like `akgl_render_2d_draw_world` - does, or hoist the update sweep out of the layer loop entirely — updating an - actor is not a per-layer operation. The second is almost certainly right, - and it is the one that needs a test asserting `updatefunc` runs exactly once - per actor per `akgl_game_update`. + `AKGL_ITERATOR_OP_LAYERMASK` is honoured rather than ignored now, so a + caller that genuinely wants one layer can ask for it and gets each of those + actors once. It is no longer in the default flag set: every live actor once + is the job, and restricting the sweep is the caller asking for less. + + `tests/game.c` counts calls into a stub `updatefunc` and asserts exactly one + per live actor per frame, two over two frames, and the layer mask selecting + only its own layer. Against the old code it reports 16. + + Counting is the assertion on purpose. 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. The timing evidence is the + gap between the `akgl_game_update` and `draw_world` rows of the same + benchmark run: 92 us before, and noise in both directions after. See + `PERFORMANCE.md`. 33. **`akgl_heap_release_character` leaks its `state_sprites` property set.** **Fixed in 0.5.0**; see item 21, which is the same defect. The diff --git a/include/akgl/game.h b/include/akgl/game.h index c0aa210..ea199a2 100644 --- a/include/akgl/game.h +++ b/include/akgl/game.h @@ -282,17 +282,24 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_game_state_unlock(void); /** * @brief One frame: update every actor layer by layer, step the physics, draw the world. * - * Takes the state lock, counts the frame, then walks layers 0 through - * #AKGL_TILEMAP_MAX_LAYERS calling each live actor's `updatefunc`, optionally - * rescaling it to the tilemap first. Then it steps `akgl_physics` and draws through - * `akgl_renderer`, and releases the lock. + * Takes the state lock, counts the frame, then calls each live actor's + * `updatefunc` **exactly once**, optionally rescaling it to the tilemap first. + * Then it steps `akgl_physics` and draws through `akgl_renderer`, and releases + * the lock. * - * @param opflags Iterator flags. Optional -- `NULL` selects a default set that - * sweeps one layer at a time, in which case `layerid` is advanced - * by the loop. Only #AKGL_ITERATOR_OP_TILEMAPSCALE is read here; - * with it clear every actor's `scale` is forced to 1.0. Note that - * the flags are *not* forwarded to the physics or render calls, - * both of which are passed `NULL`. + * Until 0.5.0 the sweep sat inside a walk over #AKGL_TILEMAP_MAX_LAYERS that + * never compared an actor's `layer` to the layer it was on, so every actor + * updated sixteen times a frame. Updating an actor is not a per-layer + * operation; drawing is, and akgl_render_2d_draw_world walks the layers itself. + * + * @param opflags Iterator flags. Optional -- `NULL` selects + * #AKGL_ITERATOR_OP_UPDATE alone, which is every live actor + * once. Two bits are read here: + * #AKGL_ITERATOR_OP_TILEMAPSCALE, without which every actor's + * `scale` is forced to 1.0; and #AKGL_ITERATOR_OP_LAYERMASK, + * which restricts the sweep to the actors whose `layer` matches + * `layerid`. Note that the flags are *not* forwarded to the + * physics or render calls, both of which are passed `NULL`. * @return `NULL` on success, otherwise an error context owned by the caller. * @throws AKGL_ERR_SDL If the state lock cannot be taken. * @throws AKERR_* Whatever an actor's `updatefunc`, akgl_tilemap_scale_actor, diff --git a/src/actor.c b/src/actor.c index 09cdd95..4463800 100644 --- a/src/actor.c +++ b/src/actor.c @@ -273,7 +273,7 @@ akerr_ErrorContext *akgl_actor_render(akgl_Actor *obj) dest.y = (obj->y - akgl_camera->y); } dest.w = curSprite->width * obj->scale; - dest.h = curSprite->width * obj->scale; + dest.h = curSprite->height * obj->scale; PASS(errctx, akgl_renderer->draw_texture(akgl_renderer, curSprite->sheet->texture, &src, &dest, 0, NULL, SDL_FLIP_NONE)); SUCCEED_RETURN(errctx); diff --git a/src/game.c b/src/game.c index b47803d..1347154 100644 --- a/src/game.c +++ b/src/game.c @@ -643,12 +643,11 @@ akerr_ErrorContext *akgl_game_load(char *fpath) akerr_ErrorContext *akgl_game_update(akgl_Iterator *opflags) { PREPARE_ERROR(errctx); + // No AKGL_ITERATOR_OP_LAYERMASK by default: updating every actor once is + // the whole job, and restricting the sweep to one layer is the caller + // deliberately asking for less. akgl_Iterator defflags = { - // Was (LAYERMASK | LAYERMASK). Nothing in the loop below reads either - // bit today, so this is a statement of intent rather than a behaviour - // change -- and the reason the loop ignores LAYERMASK is TODO.md - // Performance item 32, which is still open. - .flags = (AKGL_ITERATOR_OP_UPDATE | AKGL_ITERATOR_OP_LAYERMASK), + .flags = AKGL_ITERATOR_OP_UPDATE, .layerid = 0 }; akgl_Actor *actor = NULL; @@ -661,22 +660,29 @@ akerr_ErrorContext *akgl_game_update(akgl_Iterator *opflags) akgl_game_update_fps(); - for ( int i = 0; i < AKGL_TILEMAP_MAX_LAYERS; i++ ) { - if ( opflags == &defflags ) { - opflags->layerid = i; + // One sweep, not one per layer. This loop used to sit inside a walk over + // AKGL_TILEMAP_MAX_LAYERS that never compared actor->layer to the layer it + // was on, so every live actor's updatefunc ran sixteen times a frame -- + // measured at 68.4 ns per update and 64 actors, 70 us of work to do 4.4 us + // of it. Updating an actor is not a per-layer operation. Drawing is, and + // akgl_render_2d_draw_world walks the layers itself. + for ( int j = 0; j < AKGL_MAX_HEAP_ACTOR; j++ ) { + actor = &akgl_heap_actors[j]; + if ( actor->refcount == 0 ) { + continue; } - for ( int j = 0; j < AKGL_MAX_HEAP_ACTOR; j++ ) { - actor = &akgl_heap_actors[j]; - if ( actor->refcount == 0 ) { - continue; - } - if ( AKGL_BITMASK_HAS(opflags->flags, AKGL_ITERATOR_OP_TILEMAPSCALE) ) { - PASS(errctx, akgl_tilemap_scale_actor(akgl_gamemap, actor)); - } else { - actor->scale = 1.0; - } - PASS(errctx, actor->updatefunc(actor)); + // The flag is honoured now rather than ignored. A caller that sets it + // gets exactly the actors on opflags->layerid, once each. + if ( AKGL_BITMASK_HAS(opflags->flags, AKGL_ITERATOR_OP_LAYERMASK) && + (actor->layer != opflags->layerid) ) { + continue; } + if ( AKGL_BITMASK_HAS(opflags->flags, AKGL_ITERATOR_OP_TILEMAPSCALE) ) { + PASS(errctx, akgl_tilemap_scale_actor(akgl_gamemap, actor)); + } else { + actor->scale = 1.0; + } + PASS(errctx, actor->updatefunc(actor)); } PASS(errctx, akgl_physics->simulate(akgl_physics, NULL)); PASS(errctx, akgl_renderer->draw_world(akgl_renderer, NULL)); diff --git a/tests/actor.c b/tests/actor.c index 51a0435..8df9b14 100644 --- a/tests/actor.c +++ b/tests/actor.c @@ -17,6 +17,7 @@ #include #include #include +#include #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); diff --git a/tests/game.c b/tests/game.c index 60a6003..fed01ab 100644 --- a/tests/game.c +++ b/tests/game.c @@ -21,6 +21,9 @@ #include #include #include +#include +#include +#include #include "testutil.h" @@ -510,6 +513,141 @@ akerr_ErrorContext *test_game_updateFPS(void) SUCCEED_RETURN(e); } +/** @brief Counts akgl_game_update calls into each actor's updatefunc, by actor index. */ +static int updatecounts[AKGL_MAX_HEAP_ACTOR]; + +/** @brief updatefunc stub: record that this actor was updated. */ +static akerr_ErrorContext *counting_updatefunc(akgl_Actor *obj) +{ + PREPARE_ERROR(e); + int i = 0; + + for ( i = 0; i < AKGL_MAX_HEAP_ACTOR; i++ ) { + if ( &akgl_heap_actors[i] == obj ) { + updatecounts[i] += 1; + break; + } + } + SUCCEED_RETURN(e); +} + +/** @brief Physics backend stub: akgl_game_update calls simulate, and it must not matter here. */ +static akerr_ErrorContext *stub_simulate(akgl_PhysicsBackend *self, akgl_Iterator *opflags) +{ + PREPARE_ERROR(e); + SUCCEED_RETURN(e); +} + +/** @brief Render backend stub: akgl_game_update calls draw_world; drawing is not under test. */ +static akerr_ErrorContext *stub_draw_world(akgl_RenderBackend *self, akgl_Iterator *opflags) +{ + PREPARE_ERROR(e); + SUCCEED_RETURN(e); +} + +/** + * @brief akgl_game_update must call each live actor's updatefunc exactly once. + * + * The sweep used to sit inside a walk over AKGL_TILEMAP_MAX_LAYERS and never + * compared actor->layer to the layer it was on, so every live actor updated + * sixteen times a frame -- 70 microseconds of work to do 4.4 of it, and every + * bit of per-frame actor logic running sixteen times over. It hid behind a + * software rasterizer and would not have hidden behind a GPU backend. + * + * Counting is the assertion. A timing test would measure the machine. + */ +akerr_ErrorContext *test_game_update_visits_each_actor_once(void) +{ + PREPARE_ERROR(e); + akgl_PhysicsBackend stubphysics; + akgl_RenderBackend stubrenderer; + akgl_Iterator opflags; + akgl_Actor *actors[3] = { NULL, NULL, NULL }; + int layers[3] = { 0, 1, 1 }; + int i = 0; + int live = 0; + + ATTEMPT { + set_game_identity(); + akgl_game.statelock = SDL_CreateMutex(); + FAIL_ZERO_BREAK(e, akgl_game.statelock, AKGL_ERR_SDL, "unable to create the state mutex"); + akgl_game.lowfpsfunc = &akgl_game_lowfps; + + CATCH(e, akgl_heap_init()); + CATCH(e, akgl_registry_init_actor()); + + memset(&stubphysics, 0x00, sizeof(akgl_PhysicsBackend)); + memset(&stubrenderer, 0x00, sizeof(akgl_RenderBackend)); + stubphysics.simulate = &stub_simulate; + stubrenderer.draw_world = &stub_draw_world; + akgl_physics = &stubphysics; + akgl_renderer = &stubrenderer; + akgl_gamemap = &akgl_default_gamemap; + + for ( i = 0; i < 3; i++ ) { + char name[32]; + snprintf((char *)&name, sizeof(name), "sweepactor%d", i); + CATCH(e, akgl_heap_next_actor(&actors[i])); + CATCH(e, akgl_actor_initialize(actors[i], (char *)&name)); + actors[i]->updatefunc = &counting_updatefunc; + actors[i]->layer = layers[i]; + } + + // The default sweep: every live actor, once. + memset(&updatecounts, 0x00, sizeof(updatecounts)); + TEST_EXPECT_OK(e, akgl_game_update(NULL), "one default game update"); + live = 0; + for ( i = 0; i < AKGL_MAX_HEAP_ACTOR; i++ ) { + if ( akgl_heap_actors[i].refcount == 0 ) { + TEST_ASSERT(e, updatecounts[i] == 0, + "a free actor slot %d was updated %d times", i, updatecounts[i]); + continue; + } + live += 1; + TEST_ASSERT(e, updatecounts[i] == 1, + "actor %d updated %d times in one frame, expected 1", + i, updatecounts[i]); + } + TEST_ASSERT(e, live == 3, "%d live actors, expected 3", live); + + // Two frames means two updates, not thirty-two. + memset(&updatecounts, 0x00, sizeof(updatecounts)); + TEST_EXPECT_OK(e, akgl_game_update(NULL), "the second game update"); + TEST_EXPECT_OK(e, akgl_game_update(NULL), "the third game update"); + for ( i = 0; i < 3; i++ ) { + TEST_ASSERT(e, updatecounts[i] == 2, + "actor %d updated %d times over two frames, expected 2", + i, updatecounts[i]); + } + + // AKGL_ITERATOR_OP_LAYERMASK now means what it says: only layer 1. + memset(&updatecounts, 0x00, sizeof(updatecounts)); + AKGL_BITMASK_CLEAR(opflags.flags); + AKGL_BITMASK_ADD(opflags.flags, AKGL_ITERATOR_OP_UPDATE); + AKGL_BITMASK_ADD(opflags.flags, AKGL_ITERATOR_OP_LAYERMASK); + opflags.layerid = 1; + TEST_EXPECT_OK(e, akgl_game_update(&opflags), "a layer-masked game update"); + TEST_ASSERT(e, updatecounts[0] == 0, + "the layer 0 actor updated %d times under a layer 1 mask", updatecounts[0]); + TEST_ASSERT(e, updatecounts[1] == 1, + "the first layer 1 actor updated %d times, expected 1", updatecounts[1]); + TEST_ASSERT(e, updatecounts[2] == 1, + "the second layer 1 actor updated %d times, expected 1", updatecounts[2]); + } CLEANUP { + for ( i = 0; i < 3; i++ ) { + if ( actors[i] != NULL ) { + IGNORE(akgl_heap_release_actor(actors[i])); + } + } + if ( akgl_game.statelock != NULL ) { + SDL_DestroyMutex(akgl_game.statelock); + akgl_game.statelock = NULL; + } + } PROCESS(e) { + } FINISH(e, true); + SUCCEED_RETURN(e); +} + int main(void) { PREPARE_ERROR(errctx); @@ -535,6 +673,7 @@ int main(void) CATCH(errctx, test_game_state_lock_budget()); CATCH(errctx, test_game_updateFPS()); CATCH(errctx, test_game_updateFPS_without_a_lowfps_handler()); + CATCH(errctx, test_game_update_visits_each_actor_once()); } CLEANUP { } PROCESS(errctx) { } FINISH_NORETURN(errctx); diff --git a/tests/perf_render.c b/tests/perf_render.c index cc2cb2a..6d6271c 100644 --- a/tests/perf_render.c +++ b/tests/perf_render.c @@ -712,11 +712,13 @@ static akerr_ErrorContext *bench_scene(void) * @brief Time a whole frame the way akgl_game_update runs one. * * This is the number a host actually gets, and it is not the sum of the parts. - * akgl_game_update's update sweep is nested inside a loop over - * #AKGL_TILEMAP_MAX_LAYERS and does not filter by layer, so every live actor is - * updated sixteen times per frame rather than once. The gap between this - * benchmark and the scene benchmark plus the logic frame in tests/perf.c is that - * multiplier, and TODO.md carries it as a defect. + * + * It used to be measurably worse than that: the update sweep was nested inside + * a loop over #AKGL_TILEMAP_MAX_LAYERS that never filtered by layer, so every + * live actor updated sixteen times a frame, and this benchmark sat about 92 us + * above the scene benchmark below it. That gap *was* the multiplier. Since + * 0.5.0 the two are equal within run-to-run noise, which is the shape of the + * fix rather than a number worth quoting on its own. */ static akerr_ErrorContext *bench_game_update(void) {