Files
libakgl/TODO.md
Andrew Kesterson 4e32328681
Some checks failed
libakgl CI Build / cmake_build (push) Failing after 21s
libakgl CI Build / performance (push) Failing after 21s
libakgl CI Build / memory_check (push) Failing after 17s
libakgl CI Build / mutation_test (push) Failing after 19s
Remove the corner helpers akgl_collide_rectangles no longer uses
akgl_rectangle_points, akgl_collide_point_rectangle, akgl_Point and
akgl_RectanglePoints go. They were the intermediate form of an implementation
that changed: akgl_collide_rectangles was eight corner-containment tests built on
them, and it has been four span comparisons since the cross-case fix. Nothing
outside tests/ called either function, and a point-in-rectangle test is four
comparisons a caller can write without a struct conversion in front of them.

akgl_collide_rectangles stays. It has two correct callers in the sidescroller
asking a game-level overlap question -- a coin, a hazard, from an updatefunc --
where a bool is the whole answer and a proxy plus a narrowphase call would be
computing a normal nothing reads. TODO.md records the split rather than leaving
it to be rediscovered.

Public API removal, so 194 exported akgl_ symbols against 196, and the manual's
counts move with them. The perf suite loses its rectangle_points row; the
all-pairs sweep stays as the control it is now labelled, and PERFORMANCE.md says
what 0.8.0 measured against it -- 188.5 us for 32,640 pairs at 256 actors, where
a whole step with collision attached is 54.1 us doing strictly more.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
2026-08-02 08:09:42 -04:00

151 KiB

TODO

Internal consistency

Findings from a sweep of src/ and include/. These are consistency and convention problems, not new functional defects — where one has a functional consequence it is called out. Ordered roughly by blast radius. Items that overlap the existing Defects list are cross-referenced rather than repeated.

1. Public naming conventions

Items 1 through 6 are resolved in 0.5.0, which is an ABI break and carries the soname to libakgl.so.0.5. What changed, and what a consumer has to rename:

Was Is
_ASSETS_H_, _CONTROLLER_H_, _DRAW_H_, _ERROR_H_, _JSON_HELPERS_H_, _PHYSICS_H_, _REGISTRY_H_, _RENDERER_H_, _TEXT_H_, _TILEMAP_H_, _UTIL_H_, _STRING_H_ _AKGL_<FILE>_H_
akgl_Actor_cmhf_* (8 functions) akgl_actor_cmhf_*
akgl_game_updateFPS akgl_game_update_fps
akgl_render_init2d, akgl_render_bind2d akgl_render_2d_init, akgl_render_2d_bind
akgl_sprite_sheet_coords_for_frame akgl_spritesheet_coords_for_frame
point, RectanglePoints akgl_Point, akgl_RectanglePoints
window, bgm, game, gamemap, renderer, physics, camera the same, akgl_-prefixed
_akgl_renderer, _akgl_physics, _akgl_camera, _akgl_gamemap akgl_default_renderer, akgl_default_physics, akgl_default_camera, akgl_default_gamemap
HEAP_ACTOR, HEAP_SPRITE, HEAP_SPRITESHEET, HEAP_CHARACTER, HEAP_STRING akgl_heap_actors, akgl_heap_sprites, akgl_heap_spritesheets, akgl_heap_characters, akgl_heap_strings
GAME_ControlMaps akgl_controlmaps
AKGL_SPRITE_MAX_CHARACTER_NAME_LENGTH AKGL_CHARACTER_MAX_NAME_LENGTH
AKGL_TIME_ONESEC_MS AKGL_TIME_ONEMS_NS

include/akgl/staticstring.h also stopped guarding with _STRING_H_ — a name several libc implementations use for their own <string.h> — and its #include "string.h" is now #include <string.h>.

akgl_render_bind2d was not on the original list. It is the same defect as akgl_render_init2d, 2d trailing where the six functions it installs carry it in the middle, and renaming one without the other would have been worse than renaming neither.

The renames were done by renaming each declaration and letting the compiler find every use, rather than by pattern substitution. That distinction matters for renderer, physics and camera, which are also parameter and struct-member names: a sed would have rewritten map->physics and every akgl_RenderBackend *renderer parameter, and nothing would have complained.

Item 6 had a live bug behind the name, now fixed. akgl_game_state_lock counted its retry loop against a constant named "one second in milliseconds" that held 1000000, so it retried 10,000 times at 100 ms and blocked for roughly sixteen minutes before reporting failure. The budget is now AKGL_GAME_STATE_LOCK_BUDGET_MS (1000), with the cadence named separately as AKGL_GAME_STATE_LOCK_RETRY_MS. tests/game.c carries test_game_state_lock_budget, which holds the mutex from a second thread and asserts the call gives up between half a second and five. The old code fails it by timing the suite out; a build that gave up without waiting at all fails the lower bound. Nothing exercised the contended path before — the existing lock test only ever took an uncontended mutex, which never reaches the retry loop.

Item 4 was not cosmetic, and the proof was sitting in the test suite. renderer was exported from the shared library, and tests/character.c defined an SDL_Renderer *renderer of its own. Both had external linkage and the same spelling, so the executable's definition preempted the library's: akgl_sprite_load_json read a SDL_Renderer * through an akgl_RenderBackend *, every texture load in that suite failed with Parameter 'renderer' is invalid, and the suite still reported success — see "Test suites that could not fail" below. It now binds a real backend with akgl_render_2d_bind, the way tests/sprite.c and tests/text.c do. A consuming game with a variable called renderer would have hit the same thing with no test to notice.

2. Header/implementation surface drift

Items 7, 8, 9, 11, 12, 13, 14 and 15 are resolved in 0.5.0. Item 10 is resolved for every pair it listed.

  1. Nineteen non-static functions were defined in src/ but declared in no header. Each is now declared or static:

    • akgl_controller_handle_button_down, _button_up, _added, _removed were defined as gamepad_handle_* while controller.h declared the akgl_controller_handle_* names it never defined. The definitions now carry the declared names, which closes this and Defects → Known and still open item 10 in one change, and their documentation moved from the definitions to the header where the convention puts it. tests/controller.c no longer needs its local re-declarations.
    • akgl_game_save_actors and akgl_game_load_versioncmp are declared in game.h under a new "part of the internal API" block; tests/game.c reaches them through the header now instead of declaring them itself.
    • The four save-table iterators and akgl_game_load_objectnamemap are static and have dropped the prefix -- save_actorname_iterator, load_objectnamemap and so on. They are SDL enumeration callbacks and a file-local reader; nothing outside game.c has any business calling them.
    • akgl_get_json_properties_number, _float, _double, akgl_tilemap_load_layer_image, akgl_tilemap_load_layer_object_actor and akgl_tilemap_load_physics are declared in tilemap.h's internal-API block, again with their documentation moved to the header. That is where the "does not need a renderer" work under Remaining work wanted them.
    • akgl_path_relative_root is static path_relative_root. akgl_path_relative_from is deleted; see below.

    This is enforced now. scripts/check_api_surface.sh reads the built library's dynamic symbol table, strips comments out of every public header, and fails when an exported akgl_* symbol is declared nowhere. It runs as the api_surface CTest test. Stripping comments is the point: four of these symbols were mentioned in controller.h prose, which is not the same as being declared there and is exactly how they went unnoticed.

  2. akgl_game_init_screen was declared and never defined. The declaration is gone. Screen setup is akgl_render_2d_init.

  3. Static helpers used three naming styles. They drop the akgl_ prefix now, which is what it is for: character_load_json_inner, character_load_json_state_int_from_strings, sprite_load_json_spritesheet, alongside the actor_visible, write_exact and write_name_field that already did.

  4. Parameter names disagreed between declaration and definition. All six pairs agree now. akgl_character_initialize takes obj, akgl_character_state_sprites_iterate takes props, akgl_heap_release_character takes ptr, akgl_set_property takes value, and the two akgl_tilemap_draw* functions take map -- the header called them dest and documented them as "Output destination populated by the function", which they are not.

    akgl_get_json_with_default was the interesting one: it took the incoming context as err and named its own context e, which is the name the convention reserves for an incoming one. It is e and errctx now, and renaming it was what surfaced that the two had been swapped rather than merely misspelled.

  5. Object-pool size macros were defined twice and the override hook was dead. AKGL_MAX_HEAP_ACTOR, _SPRITE, _SPRITESHEET and _CHARACTER are defined once, in heap.h, inside the #ifndef guards that were always supposed to make them overridable. actor.h, sprite.h and character.h no longer define them.

    tests/header_pool_override.c is the regression test: it defines its own ceilings, includes heap.h, and #errors if the guard did not fire or if AKGL_MAX_HEAP_SPRITE stopped deriving from AKGL_MAX_HEAP_ACTOR. The assertion is the compile -- it deliberately disagrees with the built library's ceilings and never touches the pools, which is fine because overriding a ceiling means the library and everything linking it have to be rebuilt together anyway.

  6. Headers relied on their includers for types. iterator.h used uint32_t without <stdint.h>; json_helpers.h used json_t without <jansson.h>; util.h used SDL_FRect and bool without any SDL include. Each compiled only because of .c-file include ordering.

    Resolved, and enforced rather than merely fixed. AKGL_PUBLIC_HEADERS in CMakeLists.txt is now the single list behind both install() and a generated translation unit per header -- each including exactly that header and nothing before it -- linked into the headers suite. A header that ships is a header that is checked.

    Writing that check found a case this item had missed: registry.h uses SDL_PropertiesID in eight declarations and included no SDL header at all. That is the argument for generating the check off the install list rather than hand-listing the headers somebody thought were at risk.

  7. Include spelling was split between quoted and angled forms. Resolved. Every in-project include in a public header uses #include <akgl/sibling.h>, and staticstring.h's #include "string.h" -- a relative-first lookup that reached the system header by accident -- is #include <string.h>. tests/*.c still use #include "testutil.h", correctly: that one is test-local rather than installed.

  8. Empty parameter lists. Resolved. akgl_game_init, akgl_game_update_fps, akgl_heap_init, akgl_heap_init_actor and the eight akgl_registry_init* functions declare and define (void). Before C23 () means "unspecified arguments" and suppresses argument checking, so these were the entry points a caller could pass anything to.

  9. AKERR_NOIGNORE was applied inconsistently at definition sites. Resolved. It is on the declarations, where it does its work, and on no definition in src/.

3. Error-handling pattern

  1. *_RETURN macros are used inside ATTEMPT blocks, which skips CLEANUP. Fixed in 0.5.0. Ten sites, found by scanning rather than from this list -- it named six and missed the four in akgl_collide_rectangles.

    The one that mattered was the success path of akgl_get_json_tilemap_property, which leaked two of the string pool's 256 entries on every lookup that found what it was asked for. A map load does that several times per layer.

    Two needed more than swapping the macro. In akgl_get_json_tilemap_property a plain break would have fallen through to the "property not found" FAIL_RETURN after FINISH, reporting a miss for something found, so the success path sets a flag. In akgl_collide_rectangles the eight early exits were followed by *collide = false;, which would have overwritten the hit that broke out of the block; each corner test writes the flag itself, so that line is gone rather than moved.

    akgl_controller_default was the other behavioural one: its SUCCEED_RETURN was the last statement in the ATTEMPT block, so the path that falls out of FINISH reached the closing brace of a non-void function.

    scripts/check_error_protocol.py keeps this closed, as the error_protocol test. It also enforces the other rule with a silent failure mode -- no return out of a HANDLE block.

  2. NULL-check discipline varies by function. Fixed in 0.5.0. The eight typed JSON accessors that validated their container and then wrote through dest unconditionally now check key and dest as the two string accessors always did; the null physics backend checks its actors like the arcade one; and akgl_render_2d_frame_start, _frame_end and _shutdown check self, which the first two read straight through.

    tests/renderer.c calls all three with NULL, which segfaulted before.

  3. Error-context variable naming is split between errctx and e. Fixed in 0.5.0, in its own commit because it is a rename and nothing else. All 45 remaining sites are errctx; applying \be\b -> errctx to each removed line reproduces the added line exactly, and the counts match at 333 either way.

    e keeps its meaning where the convention wants it -- an incoming context being inspected, as in akgl_get_json_with_default(e, ...).

4. Types and macros

Items 19 through 23 are resolved in 0.5.0.

  1. float/double used raw where types.h defines aliases. The four signatures and twelve struct fields that spelled them out now use float32_t and float64_t like the actor and character structs. They are plain typedefs, so this is a spelling change and not an ABI one. float64_t's doc no longer says "unused so far".

  2. AKGL_COLLIDE_RECTANGLES had unbalanced parentheses. Deleted. Three opens against two closes meant any expansion was a syntax error, it had no callers, and it duplicated akgl_collide_rectangles.

  3. Bitmask macros were unparenthesized. All five are fully parenthesized, and AKGL_BITMASK_CLEAR no longer carries a semicolon inside its body.

    tests/bitmasks.c covers the composition cases, and writing them turned up something worth recording: the obvious test does not catch this. For a bit that is set, !AKGL_BITMASK_HAS(mask, bit) misparses to !(mask & bit) == bit, which is 0 == bit -- false, the same answer the correct parse gives. It only diverges for a bit that is not set and whose value is not 1: !(0) is 1, and 1 == 64 is false where the answer should be true. The suite now uses that shape, and fails against the old macros.

  4. The state and iterator bit macros mixed forms. AKGL_ITERATOR_OP_UPDATE is (1 << 0) like its 31 siblings, and every 1 << n in iterator.h and actor.h is parenthesized, with the hand-aligned value columns preserved.

    The bit-pattern comments in actor.h are not wrong so much as unlabelled: each shows the pattern within its own 16-bit half, which is why bit 16 looks like it restarts at bit 0. The section headings say so now. Rendering the full 32-bit value instead would push those lines past the 100-column fill.

    Newly recorded, and still open: 1 << 31 is undefined behaviour on a signed int. It is (1 << 31) in both tables and wants to be an unsigned shift, but akgl_Actor::state is int32_t and akgl_Iterator::flags is uint32_t, so the two tables do not want the same answer. Worth deciding deliberately rather than sneaking a u in.

  5. akgl_Frame was defined and never used. Deleted.

5. AKGL_ACTOR_STATE_STRING_NAMES disagrees with actor.h

All three resolved in 0.5.0.

  1. The array bound differed between declaration and definition. The header declared [AKGL_ACTOR_MAX_STATES+1] (33) and the definition was a literal [32], so a consumer trusting the declared bound read past the object. Both are [AKGL_ACTOR_MAX_STATES] now, and the definition is sized by the macro rather than by a literal.

  2. Two entries named the wrong bit. Indices 11 and 12 said AKGL_ACTOR_STATE_UNDEFINED_11 and _12 where actor.h has MOVING_IN and MOVING_OUT, so no character JSON could ever bind a sprite to either state -- the name it would have to write was not in the registry.

    tests/registry.c now walks the whole table: every entry non-NULL, every entry resolving to its own bit through AKGL_REGISTRY_ACTOR_STATE_STRINGS, no two entries sharing a name (a duplicate silently overwrites and makes one bit unreachable), and MOVING_IN/MOVING_OUT named explicitly so a regression reads as what it is.

  3. The generation comment was stale. There is no generator, no Makefile and no lib_src/. The comment is gone and the file's own header now says it is maintained by hand and states the two invariants that keep breaking.

6. Doxygen drift

  1. Three struct doc comments in tilemap.h were rotated by one. Already correct -- the doxygen rewrite fixed this before it was checked here. akgl_TilemapObject, akgl_TilemapLayer and akgl_Tileset each describe themselves.

  2. point documented as two-dimensional with an x, y and z. Already correct, and the type is akgl_Point now.

  3. Doc comments on the definition rather than the header. Resolved with item 7: the four controller handlers and the six tilemap loader helpers had their documentation moved to the header when they were declared there.

7. Formatting and hygiene

Items 31 through 36, 38 and 41 are resolved in 0.5.0. Item 37 is not; see below.

  1. Leftover debug code. The four SDL_Log lines in src/controller.c guarded by event->type == 768 && event->key.which == 11 && event->key.key == 13 -- decimal literals for one keyboard on one developer's machine, inside the per-event inner loop -- are gone.

  2. Large commented-out blocks. The five abandoned SDL_GetBasePath() path-prefixing lines across assets.c, sprite.c, character.c and tilemap.c are gone. They were superseded by akgl_path_relative.

  3. Unused locals. curTime in akgl_game_update and in akgl_render_2d_draw_world, j in akgl_render_2d_draw_world (shadowed by its own inner loop), target in akgl_character_sprite_get, both result declarations in util.c, and screenwidth/screenheight in akgl_game_init. opflags in akgl_heap_release_character is no longer unused -- it is what drives the state-sprite walk added for Defects item 21.

  4. akgl_game_update's default flags OR-ed the same bit twice. It reads AKGL_ITERATOR_OP_UPDATE | AKGL_ITERATOR_OP_LAYERMASK now. This is a statement of intent rather than a behaviour change, and the reason is worth knowing: nothing in that loop reads either bit. It never compares actor->layer to the layer it is sweeping, which is exactly Performance item 32 -- every actor updated sixteen times a frame -- and that is still open.

  5. akgl_draw_background was the only public function outside the error protocol. It now takes an akgl_RenderBackend * like every other entry point in draw.h, returns an error context, checks the backend and its sdl_renderer, and restores the draw colour it found instead of leaving it changed.

    It was listed under Remaining work as needing the offscreen renderer harness. It does not, and never did -- what it needed was to stop reading the global. tests/draw.c covers the checkerboard pattern, the restored draw colour, zero and negative sizes, and a NULL backend.

  6. akgl_registry_init_actor was the only initializer that destroyed the registry it was replacing. All eight go through one registry_create() helper now, so the other seven stop leaking an SDL_PropertiesID per call after the first -- which a game that resets between levels makes on every level.

    Fixed alongside it, because it is the same function: akgl_registry_init() never called akgl_registry_init_properties(), which is Defects → Known and still open item 3. AKGL_REGISTRY_PROPERTIES stayed 0 for any caller that did not also go through akgl_game_init, making akgl_set_property a silent no-op and akgl_get_property always return the caller's default -- so akgl_physics_init_arcade and akgl_render_2d_init quietly ignored their configuration. tests/registry.c sets a property and reads it back.

  7. Redundant casts obscure the code. Still open -- but the precondition it named is now met. Roughly 180 pointer casts across src/, a large majority no-ops, densest in src/tilemap.c and src/character.c.

    This item used to say the benefit only arrives once the build turns on the warnings those casts suppress, and that doing it first buys nothing but churn. -Wall is on now (see AGENTS.md, "Compiler warnings"), so the sweep can proceed as its own commit.

    The argument turned out to be right, with evidence. Turning -Wall on found three genuine signedness mismatches in src/sprite.c: obj->width, obj->height and obj->speed are uint32_t and were being passed straight to akgl_get_json_integer_value(..., int *). A cast would have silenced all three. They are fixed by reading into an int, range-checking, and assigning -- which also turned up that speed is scaled by 1,000,000 into a 32-bit field, so anything past 4294 ms overflowed rather than being held.

    When doing the sweep: remove a cast, rebuild, and read what the compiler says. A cast that was load-bearing will say so.

  8. struct-qualified parameters in two definitions. akgl_physics_null_move and akgl_physics_arcade_move use the typedef like the other eight.

  9. text.c validated the wrong argument. Resolved earlier, alongside the text measurement work.

  10. akgl_path_relative_from disagreed on the output parameter. Moot: the function is deleted. See Defects → Known and still open item 4.

  11. dst vs dest for output parameters. akgl_string_copy and akgl_path_relative take dest now, like everything else.

Test suites that could not fail

Found while renaming the exported globals, and the reason item 4 above is filed as a real defect rather than a style complaint. Both are fixed.

Every suite reported success on any status whose low byte is zero. libakerror's default unhandled-error handler ends in exit(errctx->status). exit keeps only the low byte of what it is given, and libakgl's status band starts at AKERR_FIRST_CONSUMER_STATUS, which is 256. AKGL_ERR_SDL is therefore exactly 256, exit(256) is a wait status of 0, and CTest recorded a pass. The most common failure status in a library built on SDL was the one status that could not fail a test.

tests/character.c was green while running one of its four tests. It aborted in test_character_sprite_mgmt with Failed loading asset .../spritesheet.png : Parameter 'renderer' is invalid — the symbol collision described in item 4 — and exited 0 because that status was AKGL_ERR_SDL. Two independent defects had to line up, and they did, for long enough that TODO.md recorded the suite as passing and wondered which change had fixed it. Nothing had; it had stopped running.

Fixed twice. 0.5.0 worked around it here, with a TEST_TRAP_UNHANDLED_ERRORS() in tests/testutil.h that installed a handler collapsing any status a byte cannot carry onto 1. Once installed, no other suite changed colour, so this was not masking anything beyond character — but it could have been at any time, and nothing would have said so.

That entry ended "any consumer's test suites have this problem; that is worth raising upstream", and it was. libakerror 2.0.1 fixes it at the source: akerr_exit() owns the status-to-exit-code mapping and the default handler calls it, so 0 exits 0, 1 through 255 exit themselves, and anything else exits AKERR_EXIT_STATUS_UNREPRESENTABLE (125) rather than a low byte that is either a lie or a claim of success. There is no wider exit() to reach for — _exit(), _Exit(), quick_exit() and the raw exit_group syscall all truncate the same way, and even waitid() reports the truncated value.

The workaround is gone as of 0.6.0, along with its 21 call sites. Verified by putting the original failure back: a FAIL_BREAK(AKGL_ERR_SDL) in tests/character.c's main now exits 125 and CTest records a failure, where it exited 0 and passed before.

Coverage status

Generated with:

cmake -S . -B build-coverage -DAKGL_COVERAGE=ON -DCMAKE_BUILD_TYPE=Debug
cmake --build build-coverage --parallel
ctest --test-dir build-coverage --output-on-failure

Reports land in build-coverage/coverage/ (index.html, coverage.xml).

Line coverage 83.9%, function coverage 91.2% (2433/2901 lines, 176/193 functions), up from 79.6% / 87.2% before the 0.5.0 defect work and from a 39.6% / 44.3% baseline. All 25 registered tests pass, and that count now includes three non-C ones: api_surface, error_protocol and the generated per-header compile checks inside headers.

The character suite deserves its own line. It was recorded here as passing; it was not running. See "Test suites that could not fail" below.

Note the trap described in "Known and still open" item 13 while regenerating these numbers: a coverage tree rebuilt after a source edit fails coverage_reset with GCOV returncode was 5. find build-coverage -name '*.gcda' -delete clears it.

File Lines Functions
src/actor.c 205/258 (80%) 16/18
src/assets.c 0/21 (0%) 0/1
src/audio.c 229/248 (92%) 22/23
src/character.c 122/126 (97%) 7/7
src/controller.c 314/349 (90%) 17/17
src/draw.c 281/286 (98%) 12/12
src/error.c 9/9 (100%) 1/1
src/game.c 142/243 (58%) 13/18
src/heap.c 116/116 (100%) 12/12
src/json_helpers.c 123/123 (100%) 11/11
src/physics.c 144/144 (100%) 10/10
src/registry.c 83/111 (75%) 12/13
src/renderer.c 61/83 (74%) 8/8
src/sprite.c 102/110 (93%) 5/5
src/staticstring.c 17/18 (94%) 2/2
src/text.c 79/80 (99%) 7/7
src/tilemap.c 276/444 (62%) 13/20
src/util.c 128/130 (98%) 7/7
src/version.c 2/2 (100%) 1/1

src/tilemap.c moved the most, 47% to 62%, because the bounds and leak work needed the loaders driven rather than merely called. src/assets.c is still the one file with no coverage at all.

Branch coverage reads 24.7% and should not be used as a target. The akerror control-flow macros (ATTEMPT/CATCH/PROCESS/FINISH, FAIL_*_RETURN) expand into large branch trees per call site, most of them unreachable in normal operation — src/game.c reports over 1700 branches across 230 lines. Track line and function coverage; treat branch coverage as a relative signal within a file.

Mutation testing

scripts/mutation_test.py was run over the three new files as a smoke check (--max-mutants 8 to 10 each, so these are samples rather than exhaustive scores):

File Score Surviving mutants
src/draw.c 90% Deleting the FAIL_ZERO_BREAK on SDL_CreateTextureFromSurface in akgl_draw_flood_fill
src/audio.c 75% Deleting SUCCEED_RETURN from the static check_voice; deleting spec.freq before opening a device
src/text.c 50% Three in akgl_text_rendertextat, which had no test yet, plus one SUCCEED_RETURN deletion

The first pass over src/audio.c scored 50% and named two real gaps, both of which are now tested: nothing asserted that an unconfigured voice is audible (the whole reason the table defaults to a square wave at full level rather than a zeroed struct), and no envelope test used a non-zero attack and decay together, so the decay measuring from the wrong origin survived. src/draw.c scored 70% first and named one: the circle was only checked at its four axis points, which a mis-signed octant reflection survives, so it now checks that every plotted pixel has a mirror in the other three quadrants.

Re-run as a smoke check after the akbasic API work, again at 10 sampled mutants each:

File Score Surviving mutants
src/controller.c 40% Three SDL_Log deletions; keybuffer_head's initialiser; count > 0 in keybuffer_attach_text; the mod a text-only entry is given
src/audio.c 60% The break on the last switch case; errctx->handled in the device callback; ensure_voices() in the mixer; the mixer's voice loop bound

Only one of those was a real gap and it is now asserted: nothing checked that a text-only ring entry reports no modifiers. Of the rest, three are equivalent mutants rather than misses — a ring buffer whose head starts at 1 behaves identically, count > 0 cannot be false where it is checked, and deleting the break on a switch's last case changes nothing — and the mixer's v <= bound reads one voice past a zeroed table, which is undefined rather than observable. SDL_Log deletions are unobservable by construction.

What is left is honestly untestable from here. Deleting a SUCCEED_RETURN leaves a non-void function falling off its end, which is undefined rather than observably wrong, and the surviving SDL branches are allocation failures the suite has no way to provoke. The three src/text.c survivors in akgl_text_rendertextat are gone: it is tested now, against a software renderer, and deleting any of its three backend checks fails the suite. That was not free — the first draft of the test used a made-up SDL_Renderer pointer, which SDL refuses on its own, so the deleted-check mutant survived it. A live renderer was what made the check observable.

Suites

Every suite is registered through the AKGL_TEST_SUITES list in CMakeLists.txt, which drives target creation, CTest registration, the WORKING_DIRECTORY/TIMEOUT properties, the link line, and the FIXTURES_REQUIRED akgl_coverage list together. Adding tests/<name>.c and the name to that list is all a new suite needs; it can no longer be accidentally left out of the coverage fixture.

Shared assertion helpers are in tests/testutil.h: TEST_ASSERT, TEST_ASSERT_FEQ, TEST_EXPECT_STATUS, TEST_EXPECT_OK, TEST_EXPECT_ANY_ERROR, and TEST_ASSERT_FLAG. All except the last expand to a break on failure, so they belong directly inside an ATTEMPT block, not inside a loop nested in one.

Done:

  • tests/physics.c — both backends, the factory, and the full simulation loop including thrust clamping, drag, layer masking, parent/child positioning, and logic-interrupt handling. 100%.
  • tests/heap.c — pool exhaustion for all five pools, refcount clamping, recursive child release, registry cleanup on release. 100%.
  • tests/json_helpers.c — every typed accessor, both string accessors' allocate-vs-reuse paths, array bounds, and akgl_get_json_with_default. 100%.
  • tests/controller.c — control map push and capacity, the default binding set, keyboard and gamepad dispatch including cross-device rejection, the dpad handlers, and device add/remove against the dummy drivers. 90%.
  • tests/game.c — version gating, the save/load roundtrip, foreign-save rejection, truncated-table detection, the state lock, and FPS accounting. 54%.
  • tests/actor.c — extended with the eight control-map handlers, automatic facing, movement logic, the animation frame state machine, akgl_actor_update, and character/sprite binding lookups. 80%.
  • tests/audio.c — every waveform, the ADSR envelope stage by stage, gate expiry and release, the frequency sweep frame by frame, voice summing and clamping, the master level, and device open/shutdown under the dummy driver. 92%.
  • tests/draw.c — every primitive against a 64x64 software renderer with the pixels read back, including flood-fill containment, save/paste roundtrip, and that drawing restores the renderer's draw color. 95%.
  • tests/text.c — font loading into the registry, both measurement entry points against a monospaced fixture font, and drawing through a bound backend over a software renderer, including every way the backend can be unusable. 100%.
  • tests/renderer.c — the 2D vtable binding, frame start and end, both draw_texture paths, the refusals a bound-but-unrendered backend gives, and the draw_mesh stub. 56%; akgl_render_init2d and akgl_render_2d_draw_world are what is left, and both want the harness.
  • tests/headers.c — that akgl/controller.h compiles as the first include in a translation unit. The assertion is the compile; main() only has to return zero.

Remaining work

Needs the offscreen renderer harness

akgl_render_init2d and akgl_render_2d_draw_world in src/renderer.c (33 lines), akgl_draw_background in src/draw.c (13), src/assets.c (21), akgl_actor_render/actor_visible in src/actor.c (53), and the drawing half of src/tilemap.c all need a live renderer global, a window, or the world globals.

akgl_text_rendertextat was on this list and is not any more: tests/text.c builds a software renderer and binds a backend to it with akgl_render_bind2d in nine lines, which is enough for anything that only needs a renderer rather than the whole world. tests/renderer.c and tests/draw.c do the same. The harness is still wanted, but for what is left it is a convenience rather than the blocker it was.

Build tests/harness.c / tests/harness.h with akgl_test_init_headless() and akgl_test_shutdown_headless(): set the dummy video and audio drivers, SDL_Init(), akgl_heap_init(), akgl_registry_init(), create a software SDL_CreateWindowAndRenderer, and point the global renderer at it. Seven existing tests hand-roll this today (tests/sprite.c:194, tests/character.c:200, tests/tilemap.c:421, tests/charviewer.c:42, plus tests/draw.c, tests/renderer.c and tests/text.c); collapse them onto the shared harness in the same change.

Then:

  • tests/renderer.c extensionsakgl_render_init2d populating the camera from the property registry, and draw_world layer ordering. tests/renderer.c exists and covers everything that does not need a world or the registry: the vtable binding, frame start/end against a NULL sdl_renderer, both draw_texture paths including angle != 0 with a NULL center, and the draw_mesh stub. Note defflags at src/renderer.c:113 is uninitialized until the if body runs.
  • tests/assets.c — BGM loading into AKGL_REGISTRY_MUSIC under the dummy audio driver.
  • tests/draw.c extensionsakgl_draw_background at zero, negative and oversized dimensions. tests/draw.c exists and covers every other primitive against a software renderer; akgl_draw_background is the one function in the file that still reads the global renderer rather than taking a backend.
  • tests/tilemap.c extensionsakgl_tilemap_draw, _draw_tileset, and akgl_tilemap_load_layer_image.

Does not need a renderer

  • src/tilemap.cakgl_tilemap_scale_actor is pure math over three branches (src/tilemap.c:824-830); akgl_get_json_properties_number, _float, and _double need only a JSON snippet; akgl_tilemap_load_physics needs a fixture with the physics property block present, absent, and malformed. Together roughly 100 of the 225 uncovered lines.
  • src/game.cakgl_game_init, akgl_game_update, akgl_game_lowfps, and akgl_game_updateFPS's frame loop need a window; revisit after the harness lands.
  • src/registry.cakgl_registry_load_properties needs a fixture with a properties object, plus the missing-file, missing-key, and wrong-value-type cases. Assert the loop at src/registry.c:148-158 does not leak the string heap.

Defects

Fixed while building the suites

Each was found by a test written to assert correct behavior.

  1. akgl_physics_simulate dereferenced self before its NULL check. src/physics.c:132 read self->gravity_time at declaration time, three lines above FAIL_ZERO_RETURN(e, self, ...). A NULL backend segfaulted.
  2. akgl_game_save never flushed or closed its stream. CLEANUP and PROCESS were transposed, which put the fclose inside the PROCESS switch, where it only ran if an error context existed and reported success. An ordinary save produced an empty file.
  3. akgl_game_save_actors wrote name-table terminators from a single char. aksl_fwrite((void *)&nullval, 1, AKGL_ACTOR_MAX_NAME_LENGTH, fp) emitted 127 bytes of adjacent stack memory into the save file and produced a sentinel the loader could not recognize.
  4. akgl_game_load_objectnamemap swallowed read failures. CATCH used directly inside while (1) breaks the loop, not the function, so a truncated or corrupt name table loaded as a successful game.
  5. akgl_Actor_cmhf_up_on and _down_on dereferenced basechar unguarded, unlike their left and right counterparts.
  6. akgl_actor_logic_movement checked actor twice instead of checking actor->basechar before dereferencing it.
  7. The gamepad handlers checked appstate three times each, so a NULL event or a missing player actor was never caught and player->state was dereferenced regardless.

Known and still open

  1. akgl_render_and_compare compares a texture against itself. Fixed in 0.5.0: the second pass draws t2. Both passes drew t1, so it always reported a match and every image assertion built on it -- including the ones in tests/sprite.c -- asserted nothing.

  2. akgl_tilemap_release double-frees tileset textures. Fixed in 0.5.0. The layers loop tested layers[i].texture and destroyed tilesets[i].texture, so every tileset texture was freed twice on a single release and no image layer's texture was freed at all. Each pointer is cleared as it goes now, which also makes a second release safe rather than a use-after-free.

    tests/tilemap.c loads the fixture map, releases it three times, and asserts every texture pointer is NULL.

  3. akgl_registry_init never initializes the properties registry. Fixed in 0.5.0, alongside internal-consistency item 36, which is the same function. akgl_registry_init() calls akgl_registry_init_properties() now, so AKGL_REGISTRY_PROPERTIES is live for callers that do not also go through akgl_game_init -- which is what made akgl_set_property a silent no-op and akgl_get_property always hand back the caller's default, and so what made akgl_physics_init_arcade and akgl_render_2d_init quietly ignore their configuration. tests/registry.c sets a property and reads it back.

  4. akgl_path_relative_from is a stub that leaks. Deleted in 0.5.0. It claimed a heap string, never wrote its output, and never released it, so 256 calls exhausted the string pool. It was declared in no header, called from nowhere, and duplicated what akgl_path_relative already does. Fixing an unfinished function nobody can reach is worse than deleting it; this also closes item 40, which was about the output-parameter shape it disagreed with the rest of the family on.

  5. akgl_compare_sdl_surfaces memcmps without checking geometry. Fixed in 0.5.0: dimensions, pitch and pixel format are compared first, and any difference is a mismatch. tests/util.c compares a 32x32 surface against an 8x8 one in both directions, which used to read 4 KiB past the end of the smaller.

  6. akgl_string_initialize overflows by four bytes when init is NULL. Fixed in 0.5.0: it zeroes sizeof(obj->data). The four bytes it used to run past the end of the object landed on the next pool slot's refcount, which is the field the allocator reads to decide whether a slot is free -- so the overrun could hand a live string out twice.

    tests/staticstring.c claims two adjacent slots, stamps a sentinel into the second's refcount, and initializes the first.

    Fixed alongside it, because it is the same file and the same class: akgl_string_copy accepted a count above AKGL_MAX_STRING_LENGTH, which read past the end of one pool slot and wrote past the end of another. The header documented that as behaviour. It is AKERR_OUTOFBOUNDS now, and a negative count is refused too.

  7. Savegame name lengths disagree between writer and reader. Fixed in 0.5.0. The reader names the same constant the writer does, per table -- AKGL_SPRITE_SHEET_MAX_FILENAME_LENGTH for spritesheets and so on. It used AKGL_ACTOR_MAX_NAME_LENGTH for all four; the other three are 128 as well, so only the spritesheet table was wrong, and that was enough.

    The failure was worse than "cannot be read back", which is what made the test interesting. The tables carry no length prefix and end at a zeroed sentinel, so a reader stepping the wrong width does not run off anything -- it finds a run of zeros somewhere inside an entry, stops early, and reports success with silently wrong maps. A test that only asserted the load succeeded passed against the broken reader.

    So akgl_game_load checks the stream is at EOF once the four tables are read. That is what turns a width disagreement into AKERR_IO instead of a corruption, and it is the assertion tests/game.c hangs on. The new roundtrip test registers a name in each of the four registries, with a full-length one in the spritesheet registry, and fails with AKERR_IO against a mismatched reader.

    That EOF check has to move when the objects themselves start being written; there is a comment at the site saying so.

    A first pass at this introduced four AKGL_GAME_SAVE_*_NAME_WIDTH aliases, one per table, on the reasoning that the on-disk format's widths are a separate concern from the object model's. They were removed. Each expanded to exactly one existing constant and had exactly one use per side, so they were indirection with no second consumer -- and the divergence they anticipated cannot happen quietly anyway: raising one of those lengths is an ABI change, which bumps the version, and akgl_game_load refuses a save whose libversion does not match before it reads a single table. What actually guards the widths is the EOF check, which works however they are spelled.

  8. Heap acquire functions are asymmetric. akgl_heap_next_string increments refcount; next_actor, next_sprite, next_spritesheet, and next_character do not. tests/heap.c pins the current behavior and says so; decide whether to make them symmetric or document the split.

  9. tests/util.c defines test_akgl_collide_point_rectangle_logic but main() never calls it. Fixed in 0.5.0; it is called, and passes.

  10. controller.h declares functions that do not exist. Fixed in 0.5.0; the definitions carry the declared akgl_controller_handle_* names now. See internal-consistency item 7, and scripts/check_api_surface.sh, which is what stops this class of drift coming back.

  11. akgl_controller_pushmap and akgl_controller_default accept negative map ids. Fixed in 0.5.0: both check the lower bound as well. tests/controller.c passes -1 and -4096 to each.

  12. A failed controller-DB fetch silently destroys the tracked fallback. Fixed in 0.5.0, both halves.

    mkcontrollermappings.sh now runs under set -euo pipefail, fetches into a temporary directory, and moves the result into place only after checking three things: curl's exit status (with --fail, so an HTTP error is a status rather than an error page in the body), a plausible minimum mapping count, and that no mapping carries a quote or backslash that would break the C string literal it becomes. Any of those failing leaves the tracked header exactly as it was and exits non-zero. AKGL_CONTROLLERDB_URL and AKGL_CONTROLLERDB_MIN_LINES are environment-overridable, which is how the failure paths were exercised.

    Verified against all five: an unresolvable host, a 404, a truncated response, an empty-but-successful response -- the original failure exactly -- and a response containing a quote. Each refuses, and the header's checksum is unchanged after every one.

    The build no longer runs it. The add_custom_command declared OUTPUT include/akgl/SDL_GameControllerDB.h as a relative path, which CMake resolves against the binary directory while the script writes to the source directory, so the declared output never appeared, the command was permanently out of date, and every build re-ran it -- needing network access and leaving the tree dirty. It is an explicit controllerdb target now, and the header is no longer listed as a library source, which is all it was there for.

    The empty-initializer problem goes with it: const char *SDL_GAMECONTROLLER_DB[] = {}; is a constraint violation in ISO C that compiles only as a GCC extension, and the minimum-count check is what guarantees at least one entry.

    Regenerating against the real upstream produced byte-identical mappings -- 2255 entries, no content change -- so the rewrite is faithful. The only diff is the $(date) stamp and the include guard, which is _AKGL_SDL_GAMECONTROLLERDB_H_ now to match every other header. That change was made in the generator rather than by hand, and the tracked copy carries it so a future regeneration shows no spurious diff.

  13. A stale build tree in the source directory breaks the coverage run. Fixed in 0.5.0. Both gcovr invocations take the build tree as an explicit positional search path and neither passes --object-directory.

    gcovr searches for .gcda/.gcno under its search paths, and with none given it searches --root -- the source directory, which is where developers keep their build trees. --object-directory does not narrow that; per gcovr's own help it only identifies "the path between gcda files and the directory where the compiler was originally run".

    Verified by reproducing it. Two instrumented trees were built inside the source directory with a source edit between them, so they described different line numbers for the same functions. The old invocation fails with Got function write_exact on multiple lines: 46, 48 and exits 64; the new one exits 0 and the whole 25-test coverage run passes with the stale tree still sitting there.

    The second, smaller version of the same thing -- rebuilding a coverage tree after editing a test leaves .gcda files describing the old object layout, and coverage_reset fails with GCOV returncode was 5 before it can delete them -- is unchanged. find build-coverage -name '*.gcda' -delete clears it. That one is gcov's, not gcovr's search path.

  14. 22 public symbols shipped without a version or soname bump. 42b60f7 added akgl_draw_point, _line, _rect, _filled_rect, _circle, _flood_fill, _copy_region and _paste_region; akgl_audio_init, _shutdown, _tone, _stop, _waveform, _envelope, _volume, _voice_active and _mix; akgl_controller_poll_key and _flush_keys; and akgl_text_measure and _measure_wrapped. project(akgl VERSION 0.1.0) and the libakgl.so.0.1 soname were both left alone.

    That contradicts this repository's own stated rule, which akbasic's CLAUDE.md quotes back: "for both 0.x libraries the soname carries MAJOR.MINOR deliberately: 0.1 and 0.2 are different ABIs." Adding exported symbols under an unchanged soname has two consequences, and the first is the one that bites:

    • A binary compiled against the new headers links happily against a libakgl.so.0.1 built from the old tree, because the soname says they are the same ABI. It fails at symbol resolution rather than at configure time.
    • AKGL_VERSION_AT_LEAST(0, 1, 0) is true for both trees, so a consumer cannot feature-test for the new API at all. akbasic pins the requirement by submodule commit instead, and says so in its README, which is not a thing a released library should make anybody do.

    Fix: bump project() to 0.2.0, which carries the soname to libakgl.so.0.2 through the existing logic at CMakeLists.txt:138. akstdlibConfigVersion.cmake's SameMinorVersion equivalent then refuses the mismatch at configure time as well.

    Resolved by 1066ac7, which did exactly that while this was being written — the two crossed, rather than one following the other. Kept rather than deleted because the reason is still the useful part: akbasic could not feature-test for the new API and had to pin libakgl by submodule commit in its README until this landed, which is the concrete cost of an additive release under an unchanged soname. Worth remembering the next time a handful of symbols looks too small to bump for.

Found while rewriting the Doxygen comments

Each of these came out of reading an implementation against the contract its header claimed. They are recorded inline as @note or @warning on the function concerned, so a reader of the generated documentation finds them without coming here first. Ordered by blast radius.

  1. akgl_path_relative leaks one error-context slot per call on its fallback path. src/util.c:120 returns akgl_path_relative_root(...) from inside the HANDLE(e, ENOENT) block. FINISH is what carries RELEASE_ERROR, so returning before it never gives the context back. libakerror hands these out of a fixed AKERR_ARRAY_ERROR[128], and this is not a rare path — it is the ordinary one, taken every time an asset names a neighbour relative to its own file rather than to the working directory. Every tileset image, layer image and spritesheet in a map costs a slot, permanently. Once the array is exhausted every subsequent failure anywhere in the process has nowhere to report from.

    Fix: assign the result to a local, break, and return after FINISH; or hoist the fallback out of the handler entirely and let the HANDLE block only record that a retry is wanted. Touches src/util.c:117-121 only.

  2. akgl_sprite_load_json does not bound the frames array. Fixed in 0.5.0. The count is bounded against AKGL_SPRITE_MAX_FRAMES before anything is written, and each element is read into an int and narrowed deliberately rather than written through a uint32_t * cast of a uint8_t *. A frame number that does not fit a uint8_t is refused too, rather than truncated into an index that names a different tile.

    tests/sprite.c covers exactly the maximum (must load, and every id must arrive), one past it, and the wide frame number, against three new fixtures. Against the old code the middle case loads happily.

  3. Two more unbounded array loads in the tilemap loader. Fixed in 0.5.0, both with the bound at the top of the loop body, the shape akgl_tilemap_load_layers in the same file already used.

    tests/tilemap.c covers both against generated fixtures: an object layer of exactly 128 (must load) and of 129, and a map with 17 tilesets. The object one is the reachable half -- 128 objects is not a large object layer and akgl_TilemapObject is not small.

  4. akgl_get_json_with_default defaults on a status the array accessors never raise. Fixed in 0.5.0: a third HANDLE_GROUP(e, AKERR_OUTOFBOUNDS), placed above the arm holding the memcpy, since HANDLE_GROUP emits no break and every arm falls into that body.

    tests/json_helpers.c covers it through the integer and object index accessors, so the fix is pinned to the status rather than to one call site.

    Worth knowing for the next test written against this function: it returns the context it was given when it does not handle the status, so TEST_EXPECT_OK -- which releases whatever the statement returns -- will double-release it against a CLEANUP block that also releases it, and a double-released context corrupts the failure instead of reporting it. The first draft of this test did exactly that and passed against the unfixed library. It takes the result into a local and hands ownership over explicitly now.

  5. The background music never loops. src/assets.c:20 initialises bgmprops to 0, src/assets.c:44 sets MIX_PROP_PLAY_LOOPS_NUMBER on it, and src/assets.c:46 plays the track with it. 0 is SDL's "no property set" sentinel, not a set this function owns, so the write is rejected — unchecked — and the play call is given no options. The music plays once and stops. akgl_load_start_bgm reports success either way.

    Fix: SDL_CreateProperties() into bgmprops, check it, and destroy it in CLEANUP. Touches src/assets.c:20-47.

  6. akgl_character_sprite_add leaks a sprite reference when a state is remapped. Fixed in 0.5.0: it reads the existing entry first and releases it once the new binding is recorded, and the write is checked.

    The new reference is taken before the write and given back if the write fails, so there is no window where a sprite is bound with nothing behind it. Rebinding a state to the sprite already there is not treated as a displacement.

    tests/character.c binds, rebinds, and then runs 200 alternating rebinds, asserting the displaced sprite's count comes back each time. This is the half Carried over item 1 calls replacement; there is still no API to remove a binding without replacing it.

  7. akgl_heap_release_character abandons the whole state-to-sprite map. src/heap.c:150 zeroed the character without walking state_sprites, so every sprite reference the character took in akgl_character_sprite_add was lost and the SDL_PropertiesID holding the map was never destroyed. Loading and releasing characters in a loop — level to level — exhausted the sprite pool and leaked an SDL property set each time.

    Fixed in 0.5.0. At a refcount of zero it enumerates state_sprites with akgl_character_state_sprites_iterate and AKGL_ITERATOR_OP_RELEASE, then SDL_DestroyProperties, then zeroes the slot. The iterator existed for exactly this walk and simply was never called from here.

    The test was already written. tests/character.c has asserted this contract all along — "character did not reduce reference count of its child sprites when released" — and had never once run, for the two reasons under "Test suites that could not fail". It runs now, and it fails against the old code.

    Still open and separate: item 20, akgl_character_sprite_add leaking the displaced sprite when a state is remapped. Releasing at character teardown does not cover a binding replaced while the character is alive.

  8. akgl_path_relative_root uses FAIL_RETURN inside its ATTEMPT block. Fixed in 0.5.0 with internal-consistency item 16, which swept every such site. It is FAIL_BREAK, and path_relative_root is static now. scripts/check_error_protocol.py fails the build if one comes back.

  9. Three smaller leaks on failure paths. All three fixed in 0.5.0, each by moving the release into a CLEANUP block.

    • akgl_render_2d_init released its two pooled strings only after both aksl_atoi calls succeeded, so a non-numeric game.screenwidth leaked two of the pool's 256 entries. tests/renderer.c runs 512 failing initializations against each of the two properties and asserts the pool is unchanged; against the old code the first loop claims every slot.
    • akgl_controller_open_gamepads freed the enumeration array only after the loop completed, so a gamepad that failed to open took the array with it. The open failure is recorded and reported after the loop, because a FAIL_ZERO_BREAK inside it would have broken the loop rather than the block. It also frees the array on the "no gamepads enumerated" path, which SDL still allocates for.
    • akgl_text_rendertextat destroyed the surface and texture only on the success path, so a failed upload leaked the surface and a failed draw leaked both -- once per frame on a HUD line.
  10. akgl_get_property reads past the end of the property value. Already fixed, as Defects the memory checker found item 35 -- the two entries are the same defect found twice, from reading the code and from running valgrind. Recorded here only so the duplicate does not read as outstanding.

  11. character_load_json_state_int_from_strings checks the same argument twice. Fixed in 0.5.0: the second guard's subject is dest, which is what it was always meant to be.

    Not asserted, deliberately. The function is static and has one call site, which passes &stateval -- so a NULL dest is unreachable from the public API and the guard cannot fire. Testing it would mean giving the function external linkage purely to reach a defensive check, which undoes internal-consistency item 9. The fix is a one-word correction to a guard that is there for the next call site, not for this one.

  12. akgl_actor_render computes a sprite's drawn height from its width. 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.

    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

  1. akgl_text_rendertextat refuses the empty string, and akgl_text_measure accepts it. Fixed in 0.5.0: it returns success without rasterizing when text[0] == '\0', matching the measure side.

    The check sits after the font, text and backend guards rather than before them, so drawing nothing still refuses the things drawing something refuses -- a caller does not get a different contract for an empty string.

    tests/text.c had the case written and deliberately unasserted, waiting for the two halves of the header to agree. It is a TEST_EXPECT_OK now, on both the wrapped and unwrapped paths, and against the old code it reports AKERR_NULLPOINTER.

Performance

The first measured baseline is in PERFORMANCE.md, produced by tests/perf.c and tests/perf_render.c (ctest --test-dir build -L perf). Both suites hold every measurement to a budget set at roughly ten times the recorded baseline, so an algorithmic regression fails the suite rather than being discovered by a player. Read PERFORMANCE.md before arguing with anything below — every claim here has a number behind it, and several of the things I expected to be slow are not.

Defects the perf suites found

Ordered by blast radius. Numbering continues the Defects list above.

  1. akgl_path_relative leaked an error context on every root-fallback resolution. src/util.c:118 took its ENOENT branch by returning from inside the HANDLE block, which skips the RELEASE_ERROR that FINISH ends with. One entry of AKERR_ARRAY_ERROR was lost per call, and the 129th call hit "Unable to pull an error context from the array!" and exited the process. Every tilemap load resolves several paths this way, so a game that loaded fifty levels died in the loader.

    Fixed. The branch now records a flag and calls akgl_path_relative_root after FINISH. tests/util.c carries test_akgl_path_relative_releases_contexts, which resolves AKERR_MAX_ARRAY_ERROR * 2 paths through that branch and asserts the pool is where it started; against the old code that test does not fail, it terminates the suite.

    This is the only return from inside a HANDLE block in src/ — the other candidates use SUCCEED_RETURN, which releases correctly. Worth a grep before anyone writes a new one, and worth a line in AGENTS.md's error-handling protocol, which warns about *_RETURN inside ATTEMPT but not about returning out of HANDLE.

  2. akgl_tilemap_load leaks five pooled strings per load. Fixed in 0.5.0, in two steps, and the split changes what the number means.

    Three of the five were akgl_get_json_tilemap_property leaking two scratch strings on every successful property lookup, through a SUCCEED_RETURN inside its ATTEMPT block -- internal-consistency item 16. That took the measured leak from five per load to two.

    The remaining two were each a claim with no matching release: akgl_tilemap_load_layers never gave back the string it read every layer's type into, and akgl_tilemap_load never gave back the dirname its relative paths resolve against. Both release in CLEANUP now.

    tests/tilemap.c asserts the pool is exactly where it started after one load/release cycle and after 64 -- enough that a leak of one string per load could not finish. Finding the last two meant dumping the contents of every still-claimed slot after a cycle rather than reading the code again; 'tilelayer' and an assets directory named themselves immediately.

    Fixed alongside, same file and same class: akgl_tilemap_load_layer_objects released its scratch string after reading each object's name and then kept using the slot -- and akgl_get_json_string_value reuses a non-NULL destination without taking another reference, so the slot was free while still live and any other claim could have been handed it.

    The tilemap-load benchmark in tests/perf_render.c no longer needs to reclaim the pool by hand between iterations.

  3. Two JSON accessors turn string-pool exhaustion into a segfault. Fixed in 0.5.0: FINISH(errctx, true) in both, so a failed akgl_heap_next_string reaches the caller instead of being swallowed and then strncpyd through.

    tests/json_helpers.c claims every slot in the pool and asserts AKGL_ERR_HEAP comes back out of both accessors with the destination left untouched. Against the old code that test segfaults rather than failing -- and so did the new tilemap property-lookup test, which is how this was confirmed rather than merely believed.

  4. akgl_game_update segfaults if akgl_game_init did not run. Fixed in 0.5.0: akgl_game_update_fps installs akgl_game_lowfps when it finds the hook NULL.

    Worth keeping the reasoning. game.fps is 0 for the first second of the process, which is under the threshold, so the unguarded call fired on frame one. Only akgl_game_init installed the default -- and include/akgl/renderer.h documents the other path on purpose: a host that owns its own window calls akgl_render_2d_bind instead of akgl_render_2d_init. An embedder following that documentation crashed on its first frame, and akbasic is exactly that embedder.

    tests/game.c clears the hook and calls the function, which is precisely that state. tests/perf_render.c installed the default by hand as a workaround and no longer has to.

  5. akgl_game_update runs the actor update sweep once per tilemap layer. 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.

    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.

  6. akgl_heap_release_character leaks its state_sprites property set. Fixed in 0.5.0; see item 21, which is the same defect. The character-load benchmark in tests/perf_render.c was written around it and no longer has to be.

Targets

What a library like this should hit. These are not predictions of what the current code does — several are missed today, and each says which. The frame budget throughout is 16.67 ms (60 fps); where a target is stated per-operation it is because that is the number that survives a change of renderer.

The one that governs the rest: libakgl's own bookkeeping should never be the reason a frame is late. Everything the library decides — pool scans, registry lookups, state-to-sprite mapping, physics, visibility, error contexts — should fit in 5% of a frame, leaving 95% for the pixels and the game's own logic. At 64 actors and a screenful of tiles that is a ceiling of about 800 µs.

# Target Today Verdict
1 Library bookkeeping under 5% of a 60 fps frame at 64 actors + 1200 tiles ~0.3% (excluding pixel work) met
2 One actor's logic update under 200 ns 68.4 ns met
3 One actor's render bookkeeping, excluding the blit, under 500 ns ~250 ns, by subtraction rather than direct measurement met, weakly measured
4 Physics sweep under 25 ns per live actor, and proportional to live actors rather than pool size 19 ns per live actor, but 63.9 ns for an empty pool partly
5 Tilemap draw bookkeeping under 100 ns per tile, excluding the blit ~25 ns met
6 Pool acquire under 100 ns regardless of how full the pool is 3.9 ns empty, 250.9 ns on the last free string slot missed
7 Pool release proportional to the bytes actually used, not to the slot's capacity 47.2 ns, a fixed 4 KiB wipe missed
8 Re-drawing an unchanged line of text under 1 µs 12.6 µs, every frame, no cache missed
9 Zero texture creation or destruction per frame in steady state one create + one destroy per line of text per frame missed
10 A handled, routine condition costs no more than twice the path that succeeds 616.5 ns vs 68.4 ns — 9x missed
11 256 actors simulated, updated and made ready to draw in under 1 ms ~22 µs at 64 actors (5.8 µs measured logic + estimated render bookkeeping); linear, so ~90 µs extrapolated met, untested at that size
12 Collision for 256 actors under 2 ms without the caller writing a broad phase 54.1 µs for a whole physics step with collision at -DAKGL_MAX_HEAP_ACTOR=256 met
13 Level load under 100 ms for a map with 8 tilesets, 4 layers and 64 actors 11.9 ms for a 2x2 map with one tileset unknown at that size
14 Fixed per-load overhead under 1% of a level load 11.5% — zeroing 26 MB of akgl_Tilemap missed
15 Static footprint under 4 MB in the default configuration 28 MB, 94% of it one tilemap missed
16 No pool leaks across a load/release cycle of any asset type tilemap is clean, asserted over 64 cycles; sprite, spritesheet and character cycles are not asserted met where tested
17 Pool exhaustion reports AKGL_ERR_HEAP and never crashes all five pools report it, and the two JSON accessors that used to strncpy through a NULL now report it too met
18 Every benchmark held to 10x its recorded baseline, enforced in CI done, ctest -L perf met

Targets 16 and 17 moved in 0.5.0, with Defects items 29 and 30. Target 16 is qualified rather than met outright on purpose: the tilemap cycle is the one that was leaking and the one that is now asserted, and nothing yet asserts the same of a sprite, spritesheet or character load/release cycle. That is a gap in the tests, not a known leak.

Notes on the ones worth arguing about:

  • 6 and 7 are the same fix. The acquire scan is 64x slower on a full string pool than an empty one purely because the pool is a megabyte and the scan touches one refcount per 4 KiB. A free-list index — one int per layer, remembering where the last free slot was — takes both to constant time without changing the "no malloc" rule at all. That is the change I would make first, and it is worth doing before anyone raises AKGL_MAX_HEAP_*, because the cost of the current design grows with the ceiling rather than with the usage.

  • 8 and 9 are one cache. A single entry keyed on (font, string, colour, wrap) would cover the common case — a HUD field that changes once a second — and a four- or eight-entry ring would cover the rest. This is the clearest optimisation in the library and it is maybe forty lines.

  • 10 is a design target, not a speed target. The answer is not a faster error context. It is that "this character has no sprite for this state" is a question with a boolean answer, and reporting it through AKERR_KEY costs nine times the update it replaces. akgl_character_sprite_get wants a companion that returns NULL without raising.

  • 12 is a scope decision, not a defect. The library deliberately does not own a broad phase, and at 64 actors the naive all-pairs loop is 0.7% of a frame — genuinely fine. At 256 it is 11%. Either the ceiling stays where it is and this target is dropped, or a uniform grid keyed on tile size goes in. I do not think a spatial index belongs here yet; I think the target belongs on record so that raising AKGL_MAX_HEAP_ACTOR is a decision made with the number in front of it.

  • 14 and 15 are the same fix too. akgl_Tilemap is 26 MB because every layer carries a 512x512 int grid and every tileset a 65,536-entry offset table, sized for the worst case at compile time. The pool rule does not require this: a layer could carry an index into one shared cell arena sized by AKGL_TILEMAP_MAX_WIDTH * AKGL_TILEMAP_MAX_HEIGHT once rather than sixteen times, and the offset table could be sized by tilecount rather than by the maximum. That is a real refactor with a real ABI break, so it belongs to 0.4 rather than to a patch release — but 28 MB of BSS on a handheld or an ESP32-class target is the difference between fitting and not.

  • 13 is untested and should not stay that way. The only map fixture in the tree is 2x2 with one tileset, which is why the load benchmark measures a PNG decode and a memset rather than a map. A realistic fixture — 128x128, four layers, several tilesets — would make target 13 measurable and would probably find something.

The plan

The order below is the order the work should happen in, set by blast radius per frame and per byte, not by how interesting the change is. Items 1 through 6 are the next changes; 7 through 9 are on record so the decision that unblocks each is made with the numbers in front of it. The engine comparison that informs several of these is in PERFORMANCE.md under How other engines spend the same frame.

  1. String pool free-list index — targets 6 and 7, one change. A next-free hint per pool in src/heap.c (the five scans are src/heap.c:52-116) takes the last-slot claim from 250.9 ns to constant time, and it lands before anyone raises AKGL_MAX_HEAP_*, because today's cost grows with the ceiling rather than the usage. Fold in item 8 under Known and still openakgl_heap_next_string is the only acquire that takes a reference, and a free-list change touches every acquire anyway. Make the release wipe (src/heap.c:211) proportional to bytes used: either memset through strnlen(data, AKGL_MAX_STRING_LENGTH) + 1 or a tracked length field — and the length-field option must respect the layout history in item 6, which put two real bugs on the bytes after data. Budgets to move: tests/perf.c:317, :333, :347; re-record the three rows in PERFORMANCE.md in the same commit.

  2. Text texture ring cache — targets 8 and 9, one cache. A static four-to-eight entry ring in src/text.c keyed on (TTF_Font *, text bytes, SDL_Color, wraplength) — x/y stay out of the key, position only affects dest. Invalidate in akgl_text_unloadfont (src/text.c:40-55) and akgl_text_unloadallfonts (:76-87): a cached texture keyed on a closed font whose address was reused is a stale hit, so unload must sweep the ring. Verification is a counting test, not a stopwatch — target 9 is zero texture create/destroy per steady-state frame, so count them through a stub; plus a new perf row ("rendertextat, cached") with the raw-SDL control row the perf rules require. Budget to move: tests/perf_render.c:391.

  3. Non-raising sprite lookup — target 10, a design change. Give akgl_character_sprite_get (src/character.c:84-94) the companion that returns NULL without raising, then convert the three sites that raise and handle AKERR_KEY for a routine condition: akgl_actor_update (src/actor.c:165, handled :172), actor_visible (:209, handled :212-216) and akgl_actor_render (:241, handled :245-249). While there, stop akgl_actor_render looking the sprite up twice on the success path (:241 and again inside actor_visible at :242). Takes the sprite-less update from 616.5 ns toward the twice-success ceiling. Budget to move: tests/perf.c:588.

  4. Demote the hot-path log lines. akgl_actor_initialize logs every spawn (src/actor.c:51) and akgl_character_sprite_add every binding (src/character.c:80); 20% of a spawn is formatting a line nobody reads, and that is with the output thrown away. Demote them (and src/character.c:285, src/renderer.c:34, src/tilemap.c:660) from SDL_Log to SDL_LogDebug — SDL checks priority before formatting, so the cost disappears at default priority and the messages survive for anyone who turns debug on. The spawn benchmark pair (tests/perf.c:410, :429) already measures exactly this gap.

  5. Fix the tileset scan, and the two bugs hiding in it. The per-tile scan in akgl_tilemap_draw (src/tilemap.c:756-798) is the known O(tiles x tilesets) FIXME, worth 0.8% of the frame at eight tilesets — but it is also wrong twice. The range test uses >= on the upper bound (firstgid + tilecount >= tilenum, src/tilemap.c:760-761), so the tile one past the end of set A also matches set B's firstgid; and there is no break after a match, so a tile matching two ranges is blitted twice. Correct the test to tilenum < firstgid + tilecount, break on match, and skip tilenum == 0 before scanning at all. Verify with a counting test — a stub draw_texture backend asserting exactly one blit per non-empty visible cell, same pattern as the tests/game.c update counter — because a boundary double-blit is invisible to a stopwatch and mostly invisible on screen. File alongside, same entry: start_x/start_y are never clamped to zero (src/tilemap.c:719-728), and the first-column src.x += accumulates across rows (:766). Mutation testing is mandatory here; this is control flow. Do not reorganize the loader for this — 0.8% does not justify it.

  6. Bound the draw_world layer loop. akgl_render_2d_draw_world (src/renderer.c:150-167) walks all AKGL_TILEMAP_MAX_LAYERS and rescans all 64 actor slots per layer — 1024 refcount checks a frame for a one-layer map. Bound the walk by akgl_gamemap->numlayers and build the per-layer actor lists in one pool pass into static index arrays. Invisible at 60 fps under the software renderer; measurable on a 2 ms GPU frame. Counting test, same as item 5.

  7. Target 12: done in 0.8.0, and the prediction held. The structure is the incremental uniform grid, not the tree — cells keyed on tile size, static arrays, insert/remove as actors move, wired into the collide slot of akgl_PhysicsBackend that the simulation loop now calls. Construct measured that design at a 96% check reduction over brute force and rejected quadtrees as costlier to maintain; Phaser's rebuild-the-RTree-every-frame approach carries a documented ~5,000-body ceiling and is the counterexample. Citations stay in PERFORMANCE.md.

    Measured here rather than cited: the grid beats the BSP by 2.6x on the same 64-proxy population (44.3 ns against 116.1 ns per query), before counting that the tree rebuilds whenever a proxy moves and the grid's move is 11.9 ns when the cell rectangle has not changed. A whole physics step with collision is 15.4 µs at 64 actors and 54.1 µs at 256, against a 2 ms target. Over that 4x range the step grew 3.5x and the all-pairs control grew 15.6x, which is the the index exists to avoid.

    The BSP ships anyway. A vtable with one implementation behind it has never been asked to be a vtable; tests/partition.c runs its whole contract against a table of both, so the seam is proven rather than asserted. It would earn its place in a world with wildly non-uniform object sizes or one larger than the grid's 128x128 cell array covers.

  8. Target 13 needs the fixture before it needs anything else. A 128x128, four-layer, multi-tileset JSON map fixture, so akgl_tilemap_load is measured on a map rather than on a PNG decode and a memset. Today the loader benchmark uses the 2x2 fixture while perf_render synthesizes its 128x128 map in-process (tests/perf_render.c:141-175) — the draw path is measured at realistic size and the load path is not. It would probably find something.

  9. Targets 14 and 15: the footprint refactor, sketched for when it is scheduled. One shared cell arena sized AKGL_TILEMAP_MAX_WIDTH * AKGL_TILEMAP_MAX_HEIGHT once instead of per-layer, and tileset offset tables sized by tilecount instead of by AKGL_TILEMAP_MAX_TILES_PER_IMAGE. One correction to the record: the FIXME at include/akgl/tilemap.h:113-128 worries about wasted leading entries per tileset, but akgl_tilemap_compute_tileset_offsets (src/tilemap.c:179-231) indexes by local id from zero — the table is merely oversized, not sparse. The refactor kills most of the 1.37 ms memset in akgl_tilemap_load (src/tilemap.c:615) and takes BSS from 28 MB toward the 4 MB target. Real ABI break; belongs to a minor release, as the note above already says.

Memory checking

cmake --build build --target memcheck runs the suites that already exist under valgrind — ctest -T memcheck with the headless drivers forced, wrapped by scripts/memcheck.sh so that a finding is an exit status rather than a line in a log nobody reads. There are no memory-check test programs, and there should never be any: tests/benchutil.h notices that it is running under valgrind and divides every benchmark's iteration count by two thousand, which turns the perf suites into the broadest path coverage in the tree at a cost valgrind can survive. The whole run is about thirty seconds.

The two halves fit together on purpose. A benchmark is a program that walks one path a hundred thousand times; a leak check wants every path walked once. Same binaries, same registration, one flag apart.

Third-party findings are suppressed in scripts/valgrind.supp, and the run forces SDL_VIDEO_DRIVER=dummy / SDL_RENDER_DRIVER=software / SDL_AUDIO_DRIVER=dummy so the vendor GPU stack is never loaded — that removes thousands of unfixable findings inside amdgpu_dri.so without suppressing anything at all. Only definite losses and invalid accesses are counted; "still reachable" is what SDL and FreeType keep for the process lifetime and says nothing about this library.

Defects the memory checker found

All six are fixed. They are kept here rather than deleted because the sizes are measured and the reasoning is worth having next time somebody asks why a loader ends in a CLEANUP block, or why a name field is staged through a zeroed buffer. cmake --build build --target memcheck is clean, and the CI job that runs it gates: the next one of these fails the build on the push that introduces it.

Ordered by blast radius as they were found. Numbering continues the lists above. Every size below is measured, not estimated.

  1. Every JSON loader leaks its parsed document. There are four json_load_file calls in src/ and not one json_decref anywhere in the library, so the whole parsed tree — objects, hashtables, strings — is abandoned on both the success and failure paths:

    Loader Site Leaked per call
    akgl_sprite_load_json src/sprite.c:140 ~1,500 bytes
    akgl_character_load_json src/character.c:232 ~2,150 bytes
    akgl_tilemap_load src/tilemap.c:693 ~9,000 bytes (2x2 fixture map)
    akgl_registry_load_properties src/registry.c:134 not exercised by any test

    Blast radius: every asset load, forever. The map figure is for the 2x2 fixture; a real map's JSON is the size of its layer data, so a 128x128 map leaks on the order of a megabyte per load. A game that reloads a level on death leaks a level's worth of JSON each time, and this is the one item in this list that grows without bound.

    Fixed. json_decref in the CLEANUP block of each, on the success path as well as the failure one, with the handle nulled after so a second pass cannot double-release. akgl_registry_load_properties needed its loop moved inside the ATTEMPT block first: props is a borrowed reference into the document and was read after the block ended, so every exit from that loop leaked the document. The test is the memcheck run -- nothing in the public API can observe a jansson refcount, and inventing a hook to prove it would be testing the test.

  2. akgl_get_property reads up to 4 KiB past the end of the property value. src/registry.c:181 copies a fixed AKGL_MAX_STRING_LENGTH bytes out of whatever SDL_GetStringProperty returns, and what it returns is an SDL_strdup of the value — four bytes for "0.0". Valgrind reports an invalid read on every call, twelve of them in tests/physics.c alone, because akgl_physics_init_arcade reads six properties and akgl_render_init2d reads two more.

    This has been in registry.h as a @note about the copy being "a fixed #AKGL_MAX_STRING_LENGTH bytes rather than the length" — filed as waste. It is not waste, it is an out-of-bounds read: today it walks into the rest of SDL's heap and returns garbage past the terminator, and on a value that lands at the end of a page it is a segfault in a getter.

    Fixed. The copy is bounded by the value's own length plus its terminator, and a value too long for an akgl_String is now refused with AKERR_OUTOFBOUNDS instead of silently truncated. The documented AKERR_NULLPOINTER for "unset with no default" is unchanged -- it is raised deliberately now rather than arriving from inside aksl_memcpy. tests/registry.c fills the destination with a sentinel, reads a three-byte property back, and asserts every byte past the terminator still holds the sentinel; that assertion fails against the old code.

  3. The savegame name tables read past the end of every registry key, and write what they find into the file. akgl_game_save_actorname_iterator (src/game.c:219) writes AKGL_ACTOR_MAX_NAME_LENGTH — 128 — bytes starting at the key SDL handed it, and SDL allocated that key to fit the name. Valgrind catches it on a 40-byte allocation. The three sibling iterators do the same thing at src/game.c:248, src/game.c:280 and src/game.c:308, with 128, 512 and 128 byte fixed widths; only the actor one is reached by the current tests, because the other registries are empty in the save roundtrip.

    Two consequences, and the second is the interesting one. The read can fault if the key sits at the end of a page. And whatever it reads goes into the save file, so a savegame contains up to 500 bytes of this process's heap per registered object — anything that happened to be next to the key. That is a file a player might send someone.

    Fixed. All four iterators now write through write_name_field, which stages the key into a zeroed fixed-width buffer, so the padding is deterministic and nothing but the name leaves the process. A negative-array-size typedef fails the build if any of the four widths ever outgrows the staging buffer. Still open and unrelated to the overread: Defects -> Known and still open item 7, the same tables disagreeing about their widths between writer and reader.

  4. akgl_controller_list_keyboards leaks the array SDL gives it. src/controller.c:188 calls SDL_GetKeyboards, which allocates, and never calls SDL_free on the result. Four bytes per call in the test environment — one keyboard id — but it is per call, and the function is shaped like something a game calls when a device is hotplugged.

    Fixed. One SDL_free, in a CLEANUP block so a failure inside the loop cannot take the array with it. Still worth asking the same question of every SDL enumeration in src/controller.c: SDL_GetGamepads has the same contract, and the dummy driver reports no gamepads, so no test reaches it.

  5. A font, once loaded, is never freed and cannot be. akgl_text_loadfont (src/text.c:20) opens a TTF_Font, puts the pointer in AKGL_REGISTRY_FONT, and that is the last anyone can do about it: the header exposes no way to close a font, and SDL_Quit destroying the property registry drops the last reference. 10,523 bytes per font — 736 of them SDL_ttf's, the rest FreeType's.

    This is bounded by how many fonts a game loads, so it is not the runaway that item 34 is. It was a gap in the API rather than only a leak: a game that switches fonts between scenes, or a tool like charviewer that reloads one while the user picks a size, had no way to give the old one back.

    Fixed, and this one is a new public symbol rather than a repair: akgl_text_unloadfont(char *name) clears the registry entry and closes the font. akgl_text_loadfont calls it when it replaces a live name, which closes the second leak the header used to document as intended behaviour -- but only after the new font has opened, so a failed reload leaves the caller with the font they already had. The version goes to 0.4.0 with it: an 0.3 consumer cannot be handed this library and told it is the same ABI.

    Still open: nothing closes the registry's remaining fonts at shutdown. A game that exits without unloading leaks them exactly once, which valgrind reports against the process rather than against a loop, and which akgl_game_shutdown would be the natural home for if it existed.

  6. Vendored deps/semver's own unit test leaks 188 bytes across 16 blocks, from the callocs in test_strcut_first and test_strcut_second (deps/semver/semver_unit.c:8 and :21). Not libakgl's code and not libakgl's to fix; recorded so that nobody re-diagnoses it, and because semver_unit is registered as one of our CTest tests and so shows up in our memcheck run.

    Suppressed, which is what this list said the answer would be if it ever became noise, and gating the job made it noise. The two entries in scripts/valgrind.supp name the functions rather than the file, so a rewrite of those cases stops being suppressed and comes back as a finding. They are the only entries there that hide a real leak in a program this build runs; the alternative was a fork of a vendored dependency.

Not defects, and why

  • tests/util.c fixtures were reading uninitialised stack floats. Three null-pointer tests declared SDL_FRect and point fixtures without initializing them and then made one real call with them at the end, which is sixteen "conditional jump depends on uninitialised value" findings for a test that is not about coordinates. The fixtures are zeroed now. The library was never at fault; a memory checker that reports noise gets ignored, which is the only reason this was worth touching.
  • "Still reachable" at exit is not counted. SDL's global state, the hint table, the property registry and FreeType's library instance are one per process and are reclaimed by SDL_Quit / TTF_Quit. Counting them would bury the six findings above under a hundred that mean nothing.
  • The GPU driver's findings are avoided rather than suppressed. Running the suite against the real driver produces thousands of findings inside amdgpu_dri.so; running it headless produces none, and headless is what the suites are written for anyway.

Found while embedding libakgl in a consumer

  1. Shadowing add_test() unconditionally broke every embedding consumer. Fixed in the same release it was introduced in, and worth keeping because the CMake behaviour behind it is not obvious and will catch the next person.

    18399f2 moved the vendored-dependency block out from behind if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) — which was the point of that commit — and took the add_test()/set_tests_properties() override out with it. The override's own comment claimed "an embedding consumer's add_test() still reaches CTest". It does not, and cannot.

    CMake chains command overrides exactly one level deep. Overriding add_test makes the builtin available as _add_test; overriding it a second time rebinds _add_test to the first override and the builtin becomes unreachable to everybody, permanently — there is no __add_test. Verified directly rather than inferred from the documentation:

    function(add_test) _add_test(${ARGV}) endfunction()   # _add_test  -> builtin
    function(add_test) _add_test(${ARGV}) endfunction()   # _add_test  -> the first override
    if(COMMAND __add_test) ... endif()                    # absent
    

    So two projects in one tree cannot both shadow it. akbasic shadows it first — it has to, for libakerror and libakstdlib, which register their tests unconditionally — and its own registrations then recursed until CMake stopped at "Maximum recursion depth of 1000 exceeded". A consumer had no way to work around it: once the builtin is gone it is gone for that project too.

    Fix: the override is guarded on being top-level again, exactly as it was before 18399f2 and exactly as libakstdlib guards its own. The vendored-dependency block stays unconditional, which is what that commit was actually for. An embedded libakgl leaves add_test alone and lets its consumer suppress what it does not want — machinery the consumer has to have anyway.

Build notes

The vendored SDL satellite libraries are built into per-project subdirectories that are not on the loader's default search path, and LD_LIBRARY_PATH is searched ahead of RPATH — so a developer who has previously run rebuild.sh had their installed libakgl.so shadow the one under test, and a developer who had not saw every test abort before main() with "cannot open shared object file". CMakeLists.txt now sets BUILD_RPATH on the library, the utility, and every test target, and prepends the build tree to LD_LIBRARY_PATH for the CTest run.

API gaps blocking akbasic

akbasic (the C port of the BASIC interpreter, source.starfort.tech/andrew/akbasic) is being built to link into libakgl as a scripting engine for game authors.

All ten items are resolved. Items 1 through 4 landed first and akbasic has since consumed every one of them: its libakgl-backed text sink, its graphics backend, its sound backend and its input backend are written and tested, and the BASIC 7.0 graphics verbs (GRAPHIC, COLOR, DRAW, BOX, CIRCLE, PAINT, SCALE, SSHAPE, GSHAPE, LOCATE), the sound verbs (SOUND, ENVELOPE, VOL, PLAY, TEMPO) and the console input verbs (GET, GETKEY, SCNCLR) all work against them.

Doing that turned up items 5 through 9. Four were things akbasic had to work around to build at all; each workaround is commented at its site over there with the words "filed upstream". Those workarounds can now be deleted — including the five add_subdirectory lines for the vendored SDL projects and the hand-assigned render vtable in tests/akgl_backends.c.

Building the standalone SDL frontend on top of all that — a real window, a real event pump and a line editor — turned up item 10, and re-confirmed items 6 and 7: both workarounds had to be written a second time, in src/frontend_akgl.c, because a host that owns a window is exactly the caller they inconvenience.

These were filed here rather than worked around in akbasic because growing libakgl to serve a consumer is the wanted outcome. Each entry says what the BASIC verb needs, what the akgl_* entry point should look like, and what would cover it.

Items 7, 9 and 10 add public symbols and item 9 adds three fields to akgl_AudioVoice, so the project version goes to 0.3.0 and the soname with it — an 0.2 consumer cannot be handed this library and told it is compatible.

  1. No way to measure rendered text. include/akgl/text.h exposes akgl_text_loadfont() and akgl_text_rendertextat(), and nothing that reports how large a string will be in a given font. A terminal-style text surface cannot be built on that: a cursor needs the advance width of one cell, and wrapping needs to know where a string crosses the right margin. The reference interpreter got this from SDL2_ttf's font.SizeUTF8("A") and derived its whole character grid from it.

    Wants akerr_ErrorContext AKERR_NOIGNORE *akgl_text_measure(TTF_Font *font, char *text, int *w, int *h); over TTF_GetStringSize, and probably a companion akgl_text_measure_wrapped(font, text, wraplength, w, h) matching the wraplength argument akgl_text_rendertextat already takes. Tests: a known string in a known font at a known size, the empty string, and a wrapped string wide enough to force two lines.

    This is the only one of the four that blocks work already designed and waiting. akbasic cannot render any output through libakgl until it lands.

    Resolved. akgl_text_measure(font, text, w, h) and akgl_text_measure_wrapped(font, text, wraplength, w, h) are in include/akgl/text.h, over TTF_GetStringSize and TTF_GetStringSizeWrapped. Neither needs a renderer. A negative wraplength is refused with AKERR_OUTOFBOUNDS rather than passed through, because SDL_ttf reads it as a very large unsigned width and silently stops wrapping. tests/text.c covers both against tests/assets/akgl_test_mono.ttf, a 10 KB monospaced ASCII subset added for the purpose — being monospaced, it lets the suite assert width("AAAA") == 4 * width("A") instead of hardcoding glyph metrics that FreeType is free to round differently. Same change fixed item 39 below: akgl_text_loadfont checked name twice and never checked filepath.

  2. No immediate-mode drawing. include/akgl/draw.h declares exactly one function, akgl_draw_background(int w, int h), and src/draw.c is at 0% coverage. BASIC 7.0's graphics verbs are all immediate-mode plotting against the current screen: DRAW (line and point), BOX, CIRCLE, PAINT (flood fill), LOCATE (set the pixel cursor), COLOR, and SSHAPE/GSHAPE (save and restore a rectangle of pixels).

    Wants an akgl_draw_* family taking the renderer the host already initialized -- akgl_draw_line, _rect, _filled_rect, _circle, _point, _flood_fill, _copy_region -- in the shape of the existing akgl_render_2d_draw_texture. SDL3's SDL_RenderLine/SDL_RenderRect/SDL_RenderFillRect cover most of it; the circle and the flood fill do not exist in SDL3 and need writing. Tests belong with the offscreen renderer harness described under "Remaining work": render a known shape, read the target back, and compare against a reference surface with the existing akgl_compare_sdl_surfaces.

    Resolved. include/akgl/draw.h now declares akgl_draw_point, _line, _rect, _filled_rect, _circle, _flood_fill, _copy_region and _paste_region, all taking the akgl_RenderBackend * the host initialized, in the shape of akgl_render_2d_draw_texture. Decisions worth knowing:

    • Color is an argument, not state. There is no current-color global to get out of step with the caller's own. Each call saves and restores the renderer's draw color, so drawing a line does not change what the host's next SDL_RenderClear paints. tests/draw.c asserts that.
    • The circle is a midpoint circle, integer arithmetic with eight-way symmetry, plotted eight points per step through SDL_RenderPoints.
    • The flood fill reads the target back, fills on the CPU, and blits only the bounding box of what changed. It keeps a fixed AKGL_DRAW_MAX_FLOOD_SPANS (4096) stack of horizontal runs at file scope rather than recursing per pixel; running out reports AKERR_OUTOFBOUNDS and leaves the region partially filled, which is stated in the header. It is therefore not reentrant — neither is anything else that draws to a single SDL_Renderer.
    • _copy_region allocates when *dest is NULL and otherwise copies into the caller's surface, matching akgl_get_json_string_value and friends. A region that would be clipped by the target edge is refused rather than silently returning a smaller surface.

    tests/draw.c draws into a 64x64 software renderer under the dummy video driver and reads pixels back, so it did not need the offscreen harness. That harness is still wanted for src/renderer.c, src/text.c and src/assets.c. akgl_draw_background is untouched and still outside the error protocol (item 35).

  3. No audio API at all. SDL3_mixer is a vendored dependency and registry.h declares AKGL_REGISTRY_MUSIC, but there is no src/audio.c, no include/akgl/audio.h, and no akgl_* symbol that opens a mixer, loads a chunk, or plays a note. BASIC 7.0's sound verbs are SOUND (a tone on a voice, with a duration), PLAY (a string of notes in a Commodore-specific notation), ENVELOPE (ADSR per voice), FILTER, VOL and TEMPO.

    PLAY and ENVELOPE want a synthesised voice rather than a sample, which SDL3_mixer does not provide directly -- the honest first step is a small tone generator feeding SDL_AudioStream, with akgl_audio_init, akgl_audio_tone(voice, hz, ms), akgl_audio_envelope(voice, a, d, s, r) and akgl_audio_volume(level). This is the largest of the four and the one most worth designing before writing. Tests can run under the dummy audio driver and assert state transitions rather than sound.

    Resolved. include/akgl/audio.h and src/audio.c add a three-voice tone generator over SDL_AudioStream: akgl_audio_init, _shutdown, _tone, _stop, _waveform, _envelope, _volume, _voice_active and _mix. It is deliberately separate from the SDL3_mixer side of the library, which plays audio assets; nothing in audio.c reads a file. Decisions worth knowing:

    • The voice table works with no device open. akgl_audio_init connects it to one; without that a host can still pull samples itself through akgl_audio_mix. That is not only for embedding — it is what makes the suite deterministic. A device pulls samples on SDL's audio thread whenever it likes, so a test that opened one and then asserted on voice state would be racing the callback. tests/audio.c mixes by hand and opens a device only in its last test.
    • Phase is derived from the frame counter, not accumulated. A float increment of hz / 44100 is not exact, and adding it 44100 times a second walks a held note off pitch.
    • A voice that was never configured is audible. A zeroed voice has a sustain of 0.0, which is silence with no error to explain it, so the table defaults to a square wave held at full level.
    • Three voices summing past full scale are clamped, not scaled, so one voice plays at the level it was asked for rather than a third of it.
    • Everything that touches the table takes the stream lock when a device is open, since the callback reads it on another thread.

    Still missing for a complete BASIC sound vocabulary: FILTER (SDL3 has no filter primitive; this would need writing), TEMPO and the PLAY note-string parser, both of which belong in the interpreter rather than here.

  4. No non-blocking keystroke read. include/akgl/controller.h is built around SDL event handlers the host pumps (akgl_controller_handle_event and friends), which suits a game loop and does not suit GET and GETKEY -- those ask "is there a keystroke waiting, yes or no" and must not require the interpreter to own the event loop. Goal 3 of akbasic forbids it owning one.

    Wants akerr_ErrorContext AKERR_NOIGNORE *akgl_controller_poll_key(int *keycode, bool *available); reading a small ring buffer that akgl_controller_handle_event already fills, so the host keeps pumping events and the interpreter drains characters at its own pace. Tests: push synthetic SDL_EVENT_KEY_DOWN events through the existing handler and drain them, plus the empty-buffer and overflow cases.

    Resolved. akgl_controller_poll_key(int *keycode, bool *available) and akgl_controller_flush_keys(void) are in include/akgl/controller.h, over a fixed AKGL_CONTROLLER_KEY_BUFFER (32) ring in src/controller.c. akgl_controller_handle_event records every SDL_EVENT_KEY_DOWN before it scans the control maps, so a key bound to an actor still reaches a polling caller — the scan returns as soon as a binding claims the event, and doing it afterwards would have lost exactly the keys a game also acts on. An empty buffer is success with available false, not an error. A full buffer drops the newest key rather than overwriting the oldest, matching the Commodore keyboard buffer and keeping what was typed first. tests/controller.c covers drain order, the release-is-not-a-keystroke case, a key shared with a control map, flush, both NULL arguments, and overflow plus reuse afterwards.

  5. An embedded libakgl demands its dependencies be installed, while vendoring them. CMakeLists.txt:17 gates the whole vendored-dependency block on CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR, so deps/SDL, deps/SDL_image, deps/SDL_mixer, deps/SDL_ttf and deps/jansson are added only when this repository is the top-level project. Embedded with add_subdirectory(), the else() branch at :51 runs find_package(SDL3 REQUIRED) and friends instead, and configure fails on a machine that has none of them installed — with the submodules sitting right there in deps/, already checked out by the recursive clone the consumer just did.

    akbasic works around it by adding those five subdirectories itself, immediately before add_subdirectory(deps/libakgl). That works only because every lookup in the else() branch is guarded with if(NOT TARGET ...) — which is the same escape hatch akerror::akerror and akstdlib::akstdlib already rely on, and it should not be the documented answer. Fix: add the vendored dependencies on both paths, still guarded by if(NOT TARGET ...) so a consumer that has already declared them wins. The suppression of their CTest registration should move with them.

    Resolved. The vendored block no longer sits behind CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR. An akgl_add_vendored_dependency(<target> <dir>) macro adds each of the seven submodules when nothing has already declared that target and the submodule is actually checked out; the find_package lookups for anything left over run afterwards, unchanged, so a checkout without submodules still resolves against the system. A macro rather than a function because add_subdirectory() inside a function runs in that function's variable scope.

    The CTest suppression moved with them, and is lifted again before this project registers its own suites, so an embedding consumer's add_test() still reaches CTest. Two smaller consequences of the move: find_package(PkgConfig) now runs only when something has to come from the system (a fully vendored build needs no pkg-config of its own, though deps/SDL_mixer asks for it separately), and the build-tree RPATH block keys on whether anything was vendored rather than on being the top-level project, so an embedded build's tests can also find the satellite libraries.

    Verified by configuring and building a scratch consumer that embeds this repository with CMAKE_DISABLE_FIND_PACKAGE_SDL3, _SDL3_image, _SDL3_mixer, _SDL3_ttf, _akerror, _akstdlib and _jansson all set, so any reliance on an installed copy would have failed the configure. It configures, builds and links. The five add_subdirectory lines in akbasic's CMakeLists.txt can be deleted.

  6. include/akgl/controller.h does not compile on its own. Lines 35, 36 and 41 declare handler function pointers taking an akgl_Actor *, and the header includes only SDL3/SDL.h, akerror.h and types.h — none of which declares that type. Any translation unit that includes akgl/controller.h before akgl/actor.h fails with "unknown type name 'akgl_Actor'". src/controller.c never notices because it includes akgl/game.h first.

    This is the house rule in AGENTS.md — keep headers self-contained, include what you use. Fix: #include "actor.h" in controller.h, or forward-declare struct akgl_Actor if the include order makes that circular. A one-line test that includes only akgl/controller.h would have caught it and would keep catching it.

    Resolved. controller.h includes <akgl/actor.h>. Not a forward declaration: akgl_Actor is a typedef of a named struct, and repeating a typedef is C11, not C99. The include closes no cycle — actor.h reaches only types.h and character.h, neither of which knows about the controller.

    tests/headers.c is the test, and it is a whole suite for one #include on purpose: a second #include after the first proves nothing about the second, because by then the first has dragged its dependencies in. Covering another header means another file shaped like that one. It also stopped being true that tests/controller.c has to include akgl/actor.h first, and the comment there saying so is gone.

  7. There is no way to attach a 2D backend to a renderer the caller already has. akgl_render_init2d() (src/renderer.c:17) does two separable things: it creates a window and an SDL_Renderer from the game.screenwidth/game.screenheight properties and writes to the camera global, and it installs the six function pointers that make an akgl_RenderBackend usable. A caller who already owns an SDL_Renderer wants only the second half — and that caller is not hypothetical, it is precisely the embedding host the API-gap section above exists to serve, since an embedded interpreter must not create the window.

    Today the only options are to call akgl_game_init() and let libakgl own the window, or to assign the six pointers by hand, which is what akbasic's tests/akgl_backends.c does. Fix: split out akgl_render_bind2d(akgl_RenderBackend *self) that installs the vtable and nothing else, and have akgl_render_init2d() call it after it has made its window. tests/renderer.c could then cover the vtable half without a display at all.

    Resolved. akgl_render_bind2d(akgl_RenderBackend *self) installs the six pointers and returns; akgl_render_init2d() calls it once the window and the camera exist. It deliberately does not touch self->sdl_renderer, which is the whole point — a host that already owns one keeps it, and a host that does not gets a backend whose entry points all report AKERR_NULLPOINTER instead of crashing.

    tests/renderer.c is new and needs no display: it covers the binding (including that the caller's SDL_Renderer survives it, and that binding a backend without one is legal), frame start and end and both draw_texture paths against a software renderer under the dummy video driver, the refusals a bound-but-unrendered backend gives, and the draw_mesh "not implemented" stub. src/renderer.c goes from 10% line coverage to 56%; what is left is akgl_render_init2d itself and akgl_render_2d_draw_world, both of which want the offscreen harness and the world globals. akbasic's hand-assigned vtable in tests/akgl_backends.c can be deleted.

  8. akgl_text_rendertextat() dereferences an uninitialised backend vtable. src/text.c calls renderer->draw_texture(renderer, ...) with no NULL check on either renderer or the function pointer, so a backend that has an SDL_Renderer but has not been through akgl_render_init2d() segfaults on the first line of text. That is exactly the state item 7 leaves a host in, and it is the same class of defect the draw commit at 42b60f7 added a test for — "a backend that exists but was never given an SDL_Renderer, which is the state a host is in between allocating one and initializing it; every draw entry point has to report it rather than dereference it." The text path still has it.

    Fix: FAIL_ZERO_RETURN on renderer, on renderer->sdl_renderer and on renderer->draw_texture, reporting AKGL_ERR_SDL or AKERR_NULLPOINTER as the draw entry points do. Test alongside the existing tests/draw.c backend-without-a-renderer case.

    Resolved. All three are checked, with AKERR_NULLPOINTER to match the draw entry points, and they are checked before anything is rasterized rather than after: refusing early costs nothing and leaks nothing, where refusing after the rasterize would hit the leak the @note on this function already describes.

    tests/text.c grew a software renderer under the dummy video driver — bound with the new akgl_render_bind2d, which is what made this cheap — and covers all three refusals plus the successful draw, wrapped and unwrapped. src/text.c goes from 58% to 100% line coverage, and the three akgl_text_rendertextat mutants the mutation run left surviving are dead. The third case is the one that matters: with a live SDL_Renderer behind a backend that was never bound, the old code got all the way to a NULL function pointer. Checking it against a made-up renderer pointer, as the first draft of the test did, is worth nothing — SDL refuses the bogus handle and the call fails for the wrong reason, which a deleted-check mutant survives.

    Found while writing that test and filed below as item 27: SDL_ttf refuses the empty string on both rasterizing paths, so drawing an empty line is an error while measuring one is not.

  9. SOUND's frequency sweep has no akgl_audio_* equivalent. BASIC 7.0's SOUND voice, freq, dur, dir, min, step ramps the pitch from freq toward min in step increments per tick, in the direction dir selects — a siren, a laser, the whole reason SOUND has six arguments. akgl_audio_tone(voice, hz, ms) holds one pitch for one duration, and nothing changes pitch over the life of a note.

    akbasic refuses those three arguments rather than faking them, and the reasoning is worth recording because it is what makes this a libakgl gap rather than an interpreter one: the only way to fake a sweep from the interpreter is to re-issue tones from its step loop, which ties audible pitch to how often the host happens to call it — a tune that changes key with the frame rate. It has to be advanced on the mixer's own frame counter, which is where the phase is already derived from and which only this library can see.

    Fix: akgl_audio_sweep(int voice, float32_t from_hz, float32_t to_hz, float32_t step_hz, uint32_t ms), advanced in akgl_audio_mix() beside the envelope. Tests would drive akgl_audio_mix by hand and assert the frame at which the pitch has moved, exactly as tests/audio.c already does for the envelope stages.

    Resolved. akgl_audio_sweep(voice, from_hz, to_hz, step_hz, ms) is in include/akgl/audio.h, with the step taken in akgl_audio_mix() off the mixer's own frame counter. Decisions worth knowing:

    • One step every 1/60 second (AKGL_AUDIO_SWEEP_TICK_HZ), because that is the rate the machine this vocabulary comes from advanced its sweep at, and its tunes are written for it. It divides 44100 exactly, so a step boundary always lands on a whole frame — which is what lets the suite assert the exact frame the pitch moves on rather than a tolerance.
    • Direction comes from the two frequencies, not from the sign of the step. step_hz stays positive; to_hz below from_hz sweeps down. A negative step is refused rather than silently meaning something.
    • It arrives and holds. The last step is clamped to to_hz rather than overshooting, and equal frequencies are a legal held tone rather than an error, so a caller computing its own limits does not have to special-case them.
    • akgl_audio_tone is now the same start path with a step of 0, which is also what makes a voice reused for a plain tone stop sweeping. That is one function, start_note, rather than two copies of the same six assignments.
    • A swept voice accumulates its phase instead of deriving it from the frame counter. Deriving assumes a constant frequency; under a sweep it jumps the waveform at every step, which is an audible click. The drift the derived form exists to avoid is not something a note that is changing pitch anyway can be said to suffer from.

    tests/audio.c drives the mixer by hand: one tick's frames do not move the pitch, the next frame does, both directions clamp at their target, a gate shorter than the sweep cuts it off part way, and a plain tone on a swept voice stays put. src/audio.c holds at 92%.

    Still missing for a complete SOUND: the oscillating third direction (C128 dir 2), which is a re-issue on arrival rather than a third kind of sweep, and FILTER, TEMPO and the PLAY note-string parser as before.

  10. The keystroke ring carries a keycode and nothing else, so Shift is invisible. akgl_controller_handle_event() (src/controller.c:104-105) pushes event->key.key into the ring and drops the rest of the event, and akgl_controller_poll_key(int *keycode, bool *available) hands back only that. Item 4 asked for a non-blocking keystroke read and got one; what it did not ask for, and what a line editor turns out to need, is which character the keystroke actually produced.

    akbasic has now built one — an INPUT and a REPL prompt drawn in the window, in src/sink_akgl.c — and the consequence is concrete: no shifted character is reachable, so ", !, (, ), : and ; cannot be typed at all, and lower case cannot be typed either. A BASIC line editor that cannot type a double quote cannot enter a string literal. The interpreter folds letters to upper case, which is what a C128 does anyway and is the right resolution for letters; it is not a resolution for punctuation, and there is nothing the caller can do about it because the modifier state was discarded two layers down.

    Note this is not solvable by the caller polling SDL_GetModState() at read time: by then the key is long released, and the whole point of a ring is that reads are decoupled from events.

    Fix: widen the ring entry to carry the modifier state and the composed text SDL already computes. Either

    typedef struct akgl_Keystroke {
        SDL_Keycode key;
        SDL_Keymod  mod;
        char        text[8];    /* UTF-8, from SDL_EVENT_TEXT_INPUT; "" for a non-printing key */
    } akgl_Keystroke;
    
    akerr_ErrorContext AKERR_NOIGNORE *akgl_controller_poll_keystroke(akgl_Keystroke *dest, bool *available);
    

    alongside the existing akgl_controller_poll_key(), which stays as it is so nothing breaks — a game asking "was the up arrow pressed" wants exactly what it already gets. Filling text means handling SDL_EVENT_TEXT_INPUT as well as SDL_EVENT_KEY_DOWN, which is the only correct way to get a character out of SDL anyway: it is what makes a keyboard layout, a compose key and a dead key work, none of which a keycode can express.

    Tests would push a shifted key-down plus its text-input event through akgl_controller_handle_event() and assert both the keycode and the composed character come back, mirroring what tests/controller.c already does for the plain ring.

    Resolved. akgl_Keystroke (keycode, SDL_Keymod, and eight bytes of UTF-8) and akgl_controller_poll_keystroke(akgl_Keystroke *dest, bool *available) are in include/akgl/controller.h; the ring now holds those instead of bare keycodes. akgl_controller_poll_key() is untouched from its caller's side. Decisions worth knowing:

    • The text is attached to the press that is still waiting for it, tracked by a flag rather than by "whatever entry is newest". Without the flag, a press dropped by a full buffer hands its text to some older key that happens to be sitting at the end of the ring — which is wrong in exactly the case where things are already going badly.
    • Text with no press behind it is buffered on its own, with a keycode of 0. An input method commit and a character finished by a dead key have no key press of their own, and a line editor wants the character regardless. akgl_controller_poll_key() discards those on the way past rather than reporting a keystroke with no key.
    • Oversized text is cut on a code point boundary. An IME can commit several characters at once and an entry holds one; truncating on a byte boundary would leave a partial UTF-8 sequence that nothing downstream can render.
    • SDL only sends SDL_EVENT_TEXT_INPUT while text input is started, so a host that wants text populated calls SDL_StartTextInput() on its window. Without it key and mod still arrive. That is documented on the function rather than left to be discovered.

    tests/controller.c covers the shifted-key case end to end (the " that started this), a non-printing key composing to nothing, a text-only entry through both pollers, text that outlived its own press, the code-point-boundary truncation, and both NULL arguments. src/controller.c holds at 91%.

Truncated registry keys can collide

Found while turning -Wall on. akgl_actor_initialize and akgl_character_initialize document their name fields as "Truncated, not rejected, if the source name is longer", and that is what they do -- the copy is aksl_strncpy bounded to the field, so it always terminates now.

Termination was the overread half, and it is fixed. The other half is not: the truncated name is the registry key. Two distinct 200-character names truncate to the same 127-byte key, and the second SDL_SetPointerProperty silently replaces the first. The objects are different; the registry cannot tell.

The same applies to akgl_Sprite::name (128), akgl_SpriteSheet::name (512) and the tilemap's object and tileset names.

Whether that matters depends on whether long asset names are realistic, and 127 bytes is generous for a hand-written name in a JSON file. Recording it rather than fixing it, because the fix is a contract change -- refuse an over-long name with AKERR_OUTOFBOUNDS instead of truncating -- and every one of those headers currently promises the opposite. aksl_strncpy already reports exactly that status when the bytes do not fit, so the change is to stop capping n at size - 1 and let it raise.

akgl.pc names no dependencies

akgl.pc.in carries Libs: -L${libdir} -lakgl and no Requires: or Requires.private: line, so a consumer that finds libakgl through pkg-config is told nothing about libakerror, libakstdlib, SDL3, SDL3_image, SDL3_mixer, SDL3_ttf or jansson.

That has always been wrong, and libakerror 2.x makes it sharper: akerror.h is part of libakgl's public interface -- IGNORE and the FAIL_* macros expand in the consumer's own translation units and reference __akerr_last_ignored, which is thread-local as of 2.0.0. A consumer that builds against whatever akerror.h happens to be on its include path and links whatever libakerror.so the loader finds can get a mismatch that pkg-config had every opportunity to prevent and did not mention.

The fix is a Requires: line for the libraries whose headers libakgl's headers include (akerror, akstdlib, SDL3) and Requires.private: for the rest, plus a version floor of akerror >= 2.0.1. Not done here because it changes what pkg-config --libs akgl emits for every existing consumer, and that wants to be its own commit with an install-tree test behind it. The CMake path is not affected -- akglConfig.cmake re-finds the targets.

libakstdlib wrappers not yet adopted

libakgl links libakstdlib and uses ten of its wrappers. It calls the raw libc function at a good many more sites. Adopting the four that were carrying a real defect is done (see the 0.6.0 commit); this is what is left, and the reason each is left.

Adopted, for the record: aksl_fclose (an unchecked close in akgl_game_save lost buffered data silently), aksl_fgetc (fgetc(3) spells EOF and a read error the same way), aksl_snprintf at two sites that were truncating under a silenced -Wformat-truncation, and aksl_strncpy at the two src/game.c sites the fixed-width copy sweep missed.

The pure-arithmetic wrappers are deliberately not adopted

memset (38 sites), strlen (14), strcmp (9), strncmp (9), memcpy (6), memcmp (2). Every one of these can only fail on a NULL argument, and at all but a handful of those sites the argument is a stack object whose address cannot be NULL. aksl_memset and aksl_memcpy are used twice each, which is the inconsistency worth naming: there is no rule distinguishing those two sites from the other 44.

Converting them costs an errctx check per line and buys a NULL check the compiler already knows is redundant. Not converting them leaves the file mixing two spellings of the same operation. Pick one and write it down -- the recommendation is to convert only where the argument is a parameter or a heap pointer, and say so in AGENTS.md, rather than either extreme.

DISABLE_GCC_WARNING_FORMAT_TRUNCATION is now dead

include/akgl/error.h exports it and RESTORE_GCC_WARNINGS, and after the aksl_snprintf conversion nothing in the tree uses either. They are public macros, so removing them is an API change and is not done here. Both suppressed sites turned out to be real truncations, which is the argument for deleting rather than keeping them: the two times this library silenced that warning, the compiler was right.

No wrapper is used for the collections

aksl_HashMap, aksl_List and aksl_TreeNode exist and libakgl uses SDL property sets for every registry instead. That is not an oversight -- the registries hand SDL_PropertiesID values to callers and SDL owns the lifetime. Recorded so nobody re-derives it.

Arcade physics feel

Found by building tests/physics_sim.c, which runs the arcade backend as a game would -- a Mario-esque jump and fall, Zelda-style top-down movement, and a fast run reversed at speed -- and prints what the actor actually did. Three defects it found are fixed in 0.6.0 (an unbounded first step, release handlers zeroing the gravity accumulator, and per-axis thrust caps composing to a 141% diagonal). These are what it found and did not fix.

No terminal velocity

akgl_physics_arcade_gravity adds gravity_y * dt to ey every step and nothing bounds it. sy caps thrust, deliberately -- it is the character's own top speed, not a speed limit on the world -- so a fall accelerates without limit for as long as it lasts. The jump simulation reaches 560 px/s in 0.7 s and would keep going.

Drag is the mechanism that exists for this: physics.drag.y makes ey approach gravity_y / drag_y instead of diverging. So the behaviour is reachable, it just is not the default and is not documented as the way to get a terminal velocity. Either document it that way or add an explicit physics.terminal_velocity. Touches src/physics.c and include/akgl/physics.h; no ABI change if it is a property rather than a field.

Releasing a direction stops the actor dead

akgl_actor_cmhf_*_off sets tx (or ty) to zero, so a character running at full speed stops within one frame of the key coming up -- the top-down simulation measures 0.0 px of drift in the second after release. That is correct for Zelda and wrong for Mario, who slides. There is no deceleration or ground friction anywhere in the arcade backend; ax is the only rate, and it applies only while a direction is held.

The shape that fits the existing model is a per-character deceleration that decays tx toward zero over several frames instead of assigning zero, with the current behaviour as the value that means "instant". That is a new akgl_Character field and an ABI break, so it wants to land with other character changes rather than alone.

Integration is explicit Euler, so trajectories are frame-rate dependent

Velocity is applied to position using the velocity computed this step, and thrust is accumulated before the cap, so the same jump traces a slightly different arc at 30 Hz than at 144 Hz. The reversal simulation measures 0.500 s to turn around where the closed form predicts 0.489 s -- about 2% at 60 Hz, and it grows with the step.

max_timestep bounds how bad it gets (that is half of why it exists), and 2% is not a bug a player can see. It is recorded because the fix -- a fixed-step accumulator, integrating in whole max_timestep slices and carrying the remainder -- would also remove the slow-motion trade that bounding the step currently makes, and the two are the same piece of work.

A large drag coefficient can invert velocity

actor->ex -= actor->ex * self->drag_x * dt is a first-order decay, so it only decays for drag * dt < 1. Past that it overshoots zero, and past 2 it diverges. With dt bounded to the default 0.05 s that needs a drag coefficient above 20, which is not a plausible setting, so this is a sharp edge rather than a live defect -- but nothing rejects it, and max_timestep is caller-settable. src/physics.c, in the three drag blocks. The exact fix is expf(-drag * dt) instead of 1 - drag * dt, which is unconditionally stable.

Carried over

  1. Make character-to-sprite state bindings release their references symmetrically. akgl_character_sprite_add() increments the sprite refcount for every state-map binding, but no corresponding removal API exists. Replacing a state binding also leaves the previous sprite's refcount incremented. When akgl_heap_release_character() drops the character refcount to zero, it clears the character registry entry and zeroes the structure without enumerating state_sprites, decrementing the bound sprites, or destroying the SDL property map. Implement binding removal/replacement so each removed binding releases exactly one sprite reference. On final character release, enumerate every remaining binding (the existing akgl_character_state_sprites_iterate() release path may be reusable), release each reference, destroy state_sprites, and then clear the character. Add tests for removal, replacement, duplicate sprite bindings across multiple states, and final character release.

    The final-release half is done in 0.5.0 — see Defects item 21. akgl_heap_release_character now enumerates every remaining binding, releases each reference, destroys state_sprites, and then clears the character, and akgl_character_state_sprites_iterate is covered by that path.

    Replacement is done too, as Defects item 20: akgl_character_sprite_add releases the sprite it displaces, and tests/character.c covers binding, rebinding, and 200 alternating rebinds.

    Still open: removal. There is no API to unbind one state without binding something else over it -- akgl_character_sprite_del -- and no test for duplicate sprite bindings across several states. Neither leaks anything today; they are a gap in the surface rather than a defect.

  2. An actor cannot be scaled per axis. akgl_Actor::scale is one float32_t applied to both dest.w and dest.h, so there is no way to express "twice as wide, the same height". The VIC-II has had separate x- and y-expand bits since 1982 and Commodore BASIC 7.0's SPRITE verb exposes both, so a BASIC interpreter drawing through libakgl cannot implement its own documented verb.

    Two shapes are plausible and the choice is a design decision rather than an obvious fix: add scale_x / scale_y and keep scale as a convenience that writes both, or replace scale outright and take the ABI break while the major version is still 0. The second is cleaner and the soname already carries MAJOR.MINOR.

    Related to defect 26, and reached the same way: akbasic's sprites are Commodore sprites, 24 wide and 21 high, and SPRITE n,,,,1 expands one axis. It works around both by installing its own renderfunc on each actor rather than patching around the library.

    Half of that workaround is no longer needed. Defect 26 -- akgl_actor_render taking the drawn height from the sprite's width -- is fixed in 0.5.0, so a non-square sprite draws at its own proportions through the library's own renderfunc. This item is the half that remains: one uniform scale, with no way to expand a single axis.

Found while writing the manual

Twenty-one chapters and two tutorial games were written against src/ rather than against the header comments, and the exercise turned up two distinct classes of problem. Both are recorded here because publishing a problem you cannot fix yet is still a contribution, and because the second class is the more dangerous one: every item in it was documented, in a header, incorrectly.

The manual documents each of these where a reader would hit it, and points here.

Defects with no prior entry

  1. akgl_game_update calls simulate through a NULL pointer. akgl_game_update invokes akgl_physics->simulate(akgl_physics, NULL) with no NULL check, and akgl_default_physics is zeroed BSS — all four method pointers are NULL. A program that does not call akgl_physics_init_arcade/_null itself therefore segfaults on its first frame, measured as exit 139, rather than raising AKERR_NULLPOINTER. physics.h:8-9,195 tells the reader akgl_game_init selects a backend from a physics.engine property, so a caller who believes the header writes exactly the program that crashes. This is the worst first-contact experience in the library and the fix is a NULL check. Closing it touches src/game.c only.

  2. Parent/child offset is double-counted at draw time. src/physics.c:192-197 writes a child's x as parent->x + vx — an absolute world coordinate. src/actor.c:273-279 then draws it at parent->x + obj->x, while actor_visible three lines above tests the camera against the raw obj->x as absolute. Two readings of one field inside one function. Confirmed by measurement: with the player at (280,146) and a (-14,+10) offset, the guarded draw lands at (130,114) and the unguarded one at (410,260), off a 320x240 screen; rendering both and hashing SDL_RenderReadPixels gives different images. Invisible only while the parent sits at the origin. actor.h documents both readings, in two places. examples/jrpg works around it with a renderfunc that nulls obj->parent for the duration of the draw.

  3. Actors and characters unregister under a different key than they register. akgl_actor_initialize (src/actor.c:46) and akgl_character_initialize (src/character.c:39) register under the caller's untruncated name, while akgl_heap_release_actor (src/heap.c:132) and _release_character (src/heap.c:165) clear using the object's truncated 128-byte field. Past 127 bytes those are different keys, so releasing leaves a live registry entry pointing at a zeroed pool slot. Distinct from "Truncated registry keys can collide" above, which describes sprites and spritesheets, where the truncated name genuinely is the key.

  4. Every actor spawned from a map is invisible on frame one. akgl_actor_initialize sets movement_controls_face, and the default facefunc clears every facing bit and re-sets one only from a movement bit. An NPC has no movement bit, so state 17 falls to 16, which maps to no sprite, and the actor is silently skipped by the draw. The same thing makes a player character vanish the moment they stop walking. Both tutorials clear the field on every live actor after loading a map; that workaround should not be necessary.

  5. use_own_physics is set and never read. akgl_tilemap_load_physics builds a complete akgl_PhysicsBackend on the map when it declares a physics.model property, and sets map->use_own_physics — and nothing in the library ever consults it. A map's declared physics is silently ignored unless the application writes the switch itself. README.md used to show that if, which made a caller-side workaround read like a library feature.

  6. akgl_TilemapLayer does not record a layer's name. akgl_tilemap_load_layers reads id, opacity, visible, x, y and type and drops the name Tiled wrote; the only name retained anywhere is per-object. A game therefore cannot ask for "the terrain layer" and must hard-code a numeric layer id, which changes whenever somebody reorders layers in the editor. Both tutorials hard-code one.

  7. akgl_character_sprite_add leaks a reference when the same sprite is re-added. ref->refcount += 1 is unconditional but the matching release is guarded by displaced != ref, so re-adding a sprite to a state it already occupies leaks a pool slot per call.

  8. akgl_tilemap_compute_tileset_offsets silently requires spacing == 0 and margin == 0. It adds spacing to the tile pitch but sets row 0's y offset to spacing rather than 0, and ignores margin entirely. A tileset with a gutter — which is most published tileset packs — renders misaligned with no diagnostic. This materially constrains what art the library can consume; it ruled out several otherwise suitable CC0 packs while sourcing the tutorial assets.

  9. akgl_actor_render hard-codes SDL_FLIP_NONE (src/actor.c:283). There is no mirrored blit, so a side-on character needs both facings drawn in the sheet. That is why docs/tutorials/assets/sidescroller/player.png carries six frames rather than three, and it doubles the art cost of every such character.

  10. akgl_Actor::layer is unbounded but draw_world stops at AKGL_TILEMAP_MAX_LAYERS. An actor assigned layer 16 or above is accepted, updated, simulated — and never drawn.

  11. akgl_registry_load_properties leaks one string-pool slot per failed property. The per-property CLEANUP block is empty. The string pool is 256 slots, so a sufficiently malformed properties file drains it.

  12. speedtime is dead. It is loaded from character JSON (src/character.c:263), written through (int *)&obj->speedtime on a uint64_t field — correct only because the struct was zeroed and the host is little-endian — and then read by nothing. Frame timing comes from sprite->speed.

  13. AKGL_SPRITE_MAX_REGISTRY_SIZE is dead. Defined in sprite.h, referenced nowhere in src/, include/, tests/ or util/.

  14. AKGL_TILEMAP_MAX_TILES_PER_IMAGE is checked nowhere, and costs 512 KiB per tileset regardless of the image's real tile count.

  15. There is no sound-effect API. akgl_audio_* is a synthesizer that reads no files; akgl_load_start_bgm is the only file-audio entry point, and its infinite-loop request is set on property set 0, so background music plays once. A game cannot load and play a sound effect through this library at all.

  16. A literal 512x512 tilemap is rejected. Both bounds are >=, so the documented AKGL_TILEMAP_MAX_WIDTH/_HEIGHT of 512 is off by one and 512x511 is the largest map that loads.

  17. Unverified asset provenance. tests/assets/World_A1.png and util/assets/Actor1.png carry the default filenames of RPG Maker's bundled art and ship with no license file, while tests/assets/akgl_test_mono.ttf sits beside akgl_test_mono.LICENSE.txt. RPG Maker's bundled assets are licensed to users of that product; redistributing them inside a C library is not something that license covers. The tutorial assets under docs/tutorials/assets/ deliberately do not depend on either file, and tests/docs_setups/tilemap.sh says why it stages a different image. Closing this means replacing two fixtures and the maps that reference them.

  18. util/assets/littleguy.json does not load. The sample data for charviewer, the one demo program the library ships, uses the pre-0.5.0 unprefixed state names (ACTOR_STATE_ALIVE) and velocity_x/velocity_y instead of speed_x/speed_y. The current loader accepts neither.

  19. A control map could not say "any keyboard", so binding one was a coin flip. akgl_controller_handle_event matched event->key.which == curmap->kbid exactly, and there was no way to express "whatever keyboard the player types on". That is not a theoretical gap, because the id a key event carries is chosen by the video backend and does not have to be an id SDL_GetKeyboards() reports: on X11 without XInput2 every key event carries SDL_GLOBAL_KEYBOARD_ID (0) while SDL registers the keyboard as SDL_DEFAULT_KEYBOARD_ID (1), and with XInput2 the events carry the physical slave device's sourceid while SDL_GetKeyboards()[0] may be the master. A game that bound SDL_GetKeyboards()[0] -- the obvious thing -- got a map that matched nothing and controls that silently did nothing. examples/sidescroller shipped exactly that.

    Fixed: kbid/jsid of 0 now match any device of that kind. 0 is safe to spend because SDL documents which as 0 when the source is unknown or virtual and joystick ids start at 1, so no real device is 0. A non-zero id still matches only that device, which is what keeps two local players apart. util/charviewer.c was already passing 0 and depending on the old accidental behaviour, so it works under XInput2 now too.

    The reason this reached a release is worth keeping: every test in tests/controller.c dispatched an event whose id equalled the id the map was bound with, so none of them could see it, and the sidescroller's smoke test called the handlers directly rather than dispatching events, so it passed against a control map that matched nothing. Both are fixed -- test_controller_wildcard_device_ids asserts the contract, and the smoke test now drives akgl_controller_handle_event from a non-zero device id and fails if the press does not arrive.

Header comments that describe code that has changed

Twenty-seven claims across the public headers were false when checked against src/. AGENTS.md already warns that this file "carried eleven entries describing code that had already changed"; this is the same failure in the headers, and Doxygen publishes it.

Nothing catches these. WARN_IF_UNDOCUMENTED proves a symbol has a comment, not that the comment is true, and api_surface strips comments precisely because prose is not a declaration. The full list is in the manual, each noted in the chapter that covers the subsystem, but the ones that would actively mislead a caller are:

  • physics.h:8-9,195 — the physics.engine property and akgl_game_init calling the factory. Neither exists. See defect 1 above for what this costs.
  • README.md (now corrected) — "ONLY supports TilED TMJ tilemaps with tileset external references". Backwards: "source" appears nowhere in src/tilemap.c, and akgl_tilemap_load_tilesets_each reads columns/firstgid/tilecount/image inline. Only embedded tilesets load.
  • renderer.hframe_start/frame_end/draw_texture "dereference self before it is checked". Each function's first statement is FAIL_ZERO_RETURN(errctx, self, ...).
  • sprite.hspeed is "seconds, scaled to milliseconds". It is milliseconds scaled to nanoseconds. Also claims frames is unbounded (bounded at src/sprite.c:207) and that akgl_sprite_initialize overreads via memcpy (it uses aksl_strncpy).
  • character.hspeedtime "in seconds" (milliseconds), and sprite_add never releasing a displaced sprite (src/character.c:60,77-79 releases it).
  • actor.h — the cmhf block comment says the _off handlers zero acceleration, thrust, environmental and velocity. They zero only ax/tx or ay/ty; zeroing ey was the gravity-cancel defect fixed in 0.6.0, and two @notes still describe it.
  • json_helpers.h — the conventions block says dest is not NULL-checked and that only akgl_get_json_string_value checks its key. All eleven accessors check dest, and all seven key-taking accessors check key.
  • assets.h:17-18 — "akgl_game_init (or a bare akgl_audio_init) has to have run first". akgl_mixer is created only in akgl_game_init; akgl_audio_init opens the synthesizer's stream and never touches it, so akgl_load_start_bgm after only that hands NULL to MIX_LoadAudio.
  • registry.h:54akgl_registry_init creating seven registries and not properties. It creates eight including properties; the genuinely false part is that akgl_game_init never calls it at all, calling the eight individually in a different order.
  • tilemap.h:59-60,354-357,447-450,462-468 — object and tileset counts unbounded, and akgl_tilemap_release double-freeing. All fixed; the surviving half of the last one is that release does not release the map's actors.
  • controller.h:232-234,262-263 — a negative controlmapid not rejected. Both call sites check.

Entries in this file that are themselves stale

  • "Known and still open" item 15akgl_path_relative leaking an error-context slot per ENOENT call — is fixed. src/util.c:115-129 sets a flag in the HANDLE block and calls path_relative_root after FINISH, which is exactly the fix the item proposes.
  • TODO.md:2027, under "akgl.pc names no dependencies", says "akglConfig.cmake re-finds the targets". No such file is generated or installed — install() ships the library, the headers and akgl.pc only, so find_package(akgl) cannot work against an install tree at all. That makes the section's problem worse than it states.
  • AGENTS.md:636 says sim_step() sets gravity_time to now - dt. tests/physics_sim.c sets it to 0 and drives the step through max_timestep; the now - dt form was replaced because it went red under parallel ctest.

tg is vendored and has no consumer

deps/tg is pinned at v0.7.9 and nothing in the tree uses it. It was evaluated alongside deps/libccd for the collision work and lost, for two reasons that are properties of the library rather than preferences:

  • It is 2D permanently. Its own documentation says the z coordinate is "only carried along and serve[s] no other purpose than to be available for when it's desired to export to an output representation"; no predicate reads it. libccd is natively 3D, so the 2D case is a restriction that lifts later.
  • Its predicates return bool. Resolving a collision needs a minimum translation vector, and tg_geom_intersects cannot supply one.

Secondary: every coordinate in its API is double against libakgl's float32_t; tg.c is a single 16,000-line amalgamation that embeds a second JSON parser next to the jansson already vendored; and it ships no build system, so akgl_add_vendored_dependency would silently do nothing for it.

It is kept rather than dropped because there is one job it does that libccd structurally cannot: indexed point-in-polygon against concave polygons with holes. GJK and MPR are convex-only, always. A trigger volume or walkable region authored as a concave Tiled polygon is the case, and tg answers it at 10M+ ops/sec with its own segment index.

This entry is the deadline. Either that consumer appears, or deps/tg comes out. A vendored dependency with no caller is a question every future reader has to answer, and 6 MB of it is cloned by every recursive checkout in CI. Closing it means either writing the region-query API or git rm -- not leaving it here.

Actor rotation, and collision under it

No actor carries an angle. akgl_actor_render hard-codes SDL_FLIP_NONE, every collision shape is axis-aligned, and both example games depend on that being true. Adding rotation is a small change in five named places and a large change in one, and this entry exists so the large one is not a surprise.

What it touches, in the order it would have to be done:

  1. akgl_Actor grows an angle, in radians, about z. Render consumes it: SDL_RenderTextureRotated takes an angle in degrees and a centre, so akgl_actor_render converts and picks a centre. The centre is a decision, not a detail — a sprite rotating about its frame centre and one rotating about its feet look nothing alike.
  2. collision_support rotates dir into local space at the top and rotates the answer back. That one function is written so this is the whole narrowphase change: every arm of its switch already answers in the shape's own frame, and the world centre is added by collision_ccdobj rather than inside the switch. A ccd_quat_t on collision_ccdobj, one ccdQuatRotVec in and one ccdQuatRotVec out, and MPR and GJK both work on rotated shapes with nothing else touched. collision_center is unaffected — the centre of a shape rotated about its own centre is the same point.
  3. The box fast path stops being valid for a rotated pair. collision_box_box is a closed form that assumes both boxes are axis-aligned; a rotated pair has to fall through to MPR. That is a branch on (a->angle != 0 || b->angle != 0) and a measurable cost — the fast path is 2-7x cheaper (PERFORMANCE.md, "Collision, measured"), so a game that rotates everything pays for it.
  4. The broad-phase bound becomes the rotated bound. akgl_collision_shape_bounds returns the shape's own AABB; a rotated shape needs the AABB of the rotated shape, which is larger. The function grows an angle parameter, and every caller — akgl_collision_settle, the grid's insert/move, ss_grounded in the sidescroller — passes one. A broad phase that keeps returning the unrotated bound under-reports, which is the one failure mode the partitioner contract forbids.
  5. akgl_collision_proxy_sync copies the angle alongside the shape and the position, since that is the one place the proxy's copy is refreshed.

The z-extrusion invariant is unaffected. Rotation is about z, so a shape's half-extent along z does not change and AKGL_COLLISION_DEPTH_RATIO still makes z overlap exceed any achievable planar penetration. See Chapter 15, "Why a 2D shape has a depth".

What it does not touch: the response. akgl_actor_collide_block works on a normal and a velocity and does not care how the normal was found. Angular velocity, torque and rotational response are a different piece of work and are not implied by any of the above — a rotated shape that resolves linearly is a perfectly coherent intermediate state, and it is the one a top-down shooter actually wants.

Swept narrowphase for fast movers

AKGL_COLLISION_FLAG_BULLET is defined, documented as reserved, and does nothing. Sub-stepping bounds how far an actor travels between tests and it is capped at AKGL_COLLISION_MAX_SUBSTEPS (8) sub-steps of at most AKGL_COLLISION_SUBSTEP_FRACTION (0.5) of a cell, so the speed above which tunnelling returns is:

    v_max = substeps * fraction * cellsize / max_timestep
          = 8 * 0.5 * 16 / 0.05
          = 1280 px/s        on 16-pixel tiles at the default max_timestep

Check your own numbers against that: a game with 8-pixel tiles halves it, and a game that raises physics.max_timestep lowers it proportionally. A walking character is nowhere near it; a projectile is a projectile.

The fix is a swept test rather than more sub-steps — sweeping the shape's AABB over the sub-step's motion, testing that, and resolving at the time of first contact rather than at the end position. It belongs behind the flag so it is paid for only by the shapes that need it.

The ccd arena is single-threaded and sized by measurement

src/collision_arena.c is one static arena of AKGL_CCD_ARENA_BYTES (64 KB) with a bump pointer, reset at the top of every akgl_collision_test. Two things follow that are true today and would not survive a change:

  • It is not thread-safe. libakgl is single-threaded behind akgl_game.statelock, and two threads in the narrowphase at once would interleave allocations in the same arena. Threading libakgl means an arena per thread, or a lock, before anything else.
  • The size is a measurement, not a guess. One GJK/EPA box pair measures 7,264 bytes. Exhaustion is reported, not crashed: ccdGJKPenetration returns -2 and akgl_collision_test raises AKGL_ERR_COLLISION with the high-water mark in the message, so the failure names its own fix. MPR — the default on the frame path — allocates nothing at all, so the arena is only under pressure from a caller who has explicitly asked for EPA's manifold.

akgl_ccd_arena_highwater() is public so a game can assert its own ceiling.

akgl_collide_rectangles stays; the corner helpers did not

The collision plan's last step was to delete akgl_rectangle_points, akgl_collide_point_rectangle and akgl_collide_rectangles once the shape API landed, on the grounds that collision now has a real narrowphase. Two of the three went, in 0.8.0, along with akgl_Point and akgl_RectanglePoints. The third stays, and this is the reasoning rather than an oversight.

Why the two went. They were the intermediate form of an implementation that changed. akgl_collide_rectangles was eight corner-containment tests built on them; it is four span comparisons now and does not go through either. Nothing outside tests/ called them, and a point-in-rectangle test is four comparisons a caller can write without a struct conversion in front of it.

Why the third stays.

  • It has callers that are correct. examples/sidescroller/player.c uses it twice, for coins and for hazards. Those are overlap questions asked from an updatefunc -- "am I touching this" -- and the right answer is a bool. Routing them through a collision shape would mean a proxy, a broad-phase insert and a narrowphase call to compute a normal and a depth that nothing reads.
  • It was fixed in this same series, not left broken. It missed the cross case and truncated through akgl_Point's int members; it is exact in float32_t now. Deleting it two commits after fixing it is a strange thing to do to a caller.
  • A game-level overlap question is not always a physics question. A minimap marker, a UI hit test, "is the cursor over this". None of those wants to be pushed, and none of them belongs in a collision world.

One thing found while doing this and fixed rather than recorded: akgl_RectanglePoints' Doxygen comment still said akgl_collide_rectangles "works corner by corner rather than by comparing edge spans", which had been false since the fix. It went with the type.