diff --git a/MAINTENANCE.md b/MAINTENANCE.md index 418e576..cf2925d 100644 --- a/MAINTENANCE.md +++ b/MAINTENANCE.md @@ -411,24 +411,32 @@ that have not moved. Is that worth changing to a per-frame cadence? Measured at `RelWithDebInfo`, scale 10, best of 5: -| Row | ns/op | -|---|---| -| `spr_collisions`, 0 sprites | 19.4 | -| `spr_collisions`, 2 sprites | 26.1 | -| `spr_collisions`, 4 sprites | 40.3 | -| `spr_collisions`, 8 sprites, all overlapping | 96.3 | -| one rendered frame, 8 sprites + full text grid | 1,174,081 | +| Row | ns/op | as 256 scans of a 1.20 ms frame | +|---|---|---| +| `spr_collisions`, 0 sprites | 16.6 | 0.35% | +| `spr_collisions`, 2 sprites, spread out | 29.6 | 0.63% | +| `spr_collisions`, 4 sprites, spread out | 37.4 | 0.80% | +| `spr_collisions`, 8 sprites, spread out | 54.9 | 1.2% | +| **`spr_collisions`, breakout's own layout** | **211.8** | **4.5%** | +| `spr_collisions`, 8 sprites, all overlapping | 800.4 | 17% | +| one rendered frame, 8 sprites + full text grid | 1,203,923 | — | -**The answer is no.** 256 scans at 96.3 ns is 24.7 µs against a 1.17 ms frame — **2.1%**, and -that is the pathological case: eight sprites all overlapping, in a program that never blocks and -therefore burns all 256 steps. The cost a program with no sprites pays is 0.42% of a frame. The -frame is dominated by the text layer repainting every row it owns, which `TODO.md` already -records as the real rendering cost. +**The answer is no.** Even the row that matters most — the arrangement +`examples/breakout/sprites/breakout.bas` actually has, which is the most demanding program +here — is 4.5% of a frame, and it only reaches that because two of its eight sprites *are the +screen*: a captured HUD strip and a captured play field, so the field's box covers everything +and the bounding-box reject can never throw those pairs out. The frame is dominated by the text +layer repainting every row it owns, which `TODO.md` already records as the real rendering cost. So the per-step cadence stays. It is what makes a collision report describe where the sprites have just been moved to rather than where they were (`src/runtime.c`, above the service call), -and buying 2% of a frame is not worth changing when a handler fires for every program that -already works. Recorded here rather than argued again. +and buying a few percent of a frame is not worth changing when a handler fires for every program +that already works. Recorded here rather than argued again. + +**Read the two synthetic extremes as a bracket, not as an answer.** "Spread out" is every pair +rejected on four comparisons; "all overlapping" is every pair going through to the narrowphase, +which is a state no real program sits in. Real programs land between them, and the breakout row +is there so the question "between them *where*" has an answer. **Read a benchmark as a gap between two rows of the same run, not as an absolute.** libakgl's `PERFORMANCE.md` records a whole laptop reading 15% high on a later run, including rows nothing diff --git a/include/akbasic/akgl.h b/include/akbasic/akgl.h index 03eb68a..1c7b922 100644 --- a/include/akbasic/akgl.h +++ b/include/akbasic/akgl.h @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -235,6 +236,42 @@ typedef struct /** Per-axis expansion, which an actor's single `scale` cannot carry. */ bool xexpand[AKBASIC_MAX_SPRITES]; bool yexpand[AKBASIC_MAX_SPRITES]; + + /** + * One collision proxy per slot, claimed once at init and held for the life + * of the backend. + * + * Claimed up front rather than per scan for two reasons. The pool is shared + * with whatever host this interpreter is embedded in -- a game with its own + * shaped actors draws from the same #AKGL_MAX_HEAP_COLLISION_PROXY -- so + * running out is a real possibility, and it should be an init-time failure + * naming the pool rather than a collision scan that starts refusing halfway + * through a game. And a proxy carries a *copy* of its shape, so there is + * nothing to rebuild per scan beyond the position. + * + * These are not registered with any partitioner. akgl_collision_test() + * takes two positioned proxies and needs no world, which is what lets eight + * sprites be tested pairwise without paying for a broad phase that eight + * objects would not repay. + */ + akgl_CollisionProxy *proxies[AKBASIC_MAX_SPRITES]; + /** + * The shape each proxy is synced from. + * + * Kept here rather than read back off the proxy because + * akgl_collision_proxy_sync() wants a live shape to copy from, and because + * the shape is derived from the sprite's frame and expansion bits every scan + * -- a sprite whose picture or expansion changed has a different box, and + * the cheapest correct thing is to rebuild it rather than track what + * invalidates it. + */ + akgl_CollisionShape shapes[AKBASIC_MAX_SPRITES]; + /** + * The rectangle each proxy was last synced from, so a scan can tell whether + * there is anything to sync. See sync_proxy() in src/sprite_akgl.c for why + * this is compared rather than flagged. + */ + SDL_FRect syncedbox[AKBASIC_MAX_SPRITES]; } akbasic_AkglSprites; /** diff --git a/src/sprite_akgl.c b/src/sprite_akgl.c index caeac4f..be980c8 100644 --- a/src/sprite_akgl.c +++ b/src/sprite_akgl.c @@ -479,10 +479,90 @@ static akerr_ErrorContext *spr_shared_colors(akbasic_SpriteBackend *self, akbasi SUCCEED_RETURN(errctx); } +/** + * @brief Is slot @p i a thing that can be collided with at all? + * + * The three guards the pair loop used to repeat inline. A slot with no actor was + * never defined; a slot with no sprite has no frame and therefore no size; and an + * invisible sprite is out of the world entirely, which is the cheapest way a + * program has of switching one off without forgetting where it was. + */ +static bool slot_collidable(akbasic_AkglSprites *state, int i) +{ + return (state->actors[i] != NULL + && state->sprites[i] != NULL + && state->actors[i]->visible); +} + +/** + * @brief Point slot @p i's proxy at where the sprite is and what size it is now. + * + * The shape is rebuilt every scan rather than cached against a dirty flag. It is + * derived from the sprite's frame and the two expansion bits, both of which a + * program may change at any statement, and building a box is a handful of + * arithmetic -- cheaper than being wrong about what invalidates it. + * + * **The mask override is the load-bearing line.** akgl_collision_shape_box() + * leaves a shape on the ACTOR layer responding to STATIC only, so that giving a + * town full of NPCs hitboxes does not make them shove each other around. That is + * the right default for a tile game and exactly wrong here: BASIC's collision is + * sprite-against-sprite first, and taking the library's default would make + * BUMP(1) report nothing at all, silently. + */ +static akerr_ErrorContext AKERR_NOIGNORE *sync_proxy(akbasic_AkglSprites *state, int i, + SDL_FRect *box) +{ + PREPARE_ERROR(errctx); + SDL_FRect body; + + /* + * **Nothing to do if nothing moved.** The scan runs at the top of every + * interpreter step -- up to 256 times a rendered frame -- and a sprite moves + * at most once in that time, so the overwhelming majority of syncs would + * rewrite a proxy with what it already holds. Four float comparisons to find + * that out are far cheaper than the shape build and the sync they skip. + * + * Compared against the last synced rectangle rather than tracked with a dirty + * flag a verb sets. A flag would be smaller and would be wrong: this file's + * own header says a host game sees BASIC's sprites in libakgl's actor + * registry "and can do anything to them it can do to an actor", which + * includes moving one behind the interpreter's back. Comparing the answer + * cannot miss that; flagging the question can. + */ + if ( state->syncedbox[i].x == box->x && state->syncedbox[i].y == box->y + && state->syncedbox[i].w == box->w && state->syncedbox[i].h == box->h ) { + SUCCEED_RETURN(errctx); + } + + body.x = 0.0f; + body.y = 0.0f; + body.w = box->w; + body.h = box->h; + /* + * A zero-sized frame would be refused by the shape builder, and a sprite can + * legitimately have one before its sheet is loaded. Treat it as not + * collidable rather than raising: the program has not done anything wrong. + */ + if ( body.w <= 0.0f || body.h <= 0.0f ) { + SUCCEED_RETURN(errctx); + } + PASS(errctx, akgl_collision_shape_box(&state->shapes[i], &body, 0.0f)); + state->shapes[i].collidemask = AKGL_COLLISION_LAYER_ACTOR | AKGL_COLLISION_LAYER_STATIC; + PASS(errctx, akgl_collision_proxy_sync(state->proxies[i], &state->shapes[i], + box->x, box->y, 0.0f)); + state->syncedbox[i] = *box; + SUCCEED_RETURN(errctx); +} + static akerr_ErrorContext *spr_collisions(akbasic_SpriteBackend *self, uint16_t *mask) { PREPARE_ERROR(errctx); akbasic_AkglSprites *state = NULL; + akgl_Contact contact; + SDL_FRect box[AKBASIC_MAX_SPRITES]; + bool live[AKBASIC_MAX_SPRITES]; + bool synced[AKBASIC_MAX_SPRITES]; + bool hit = false; int i = 0; int j = 0; @@ -490,41 +570,81 @@ static akerr_ErrorContext *spr_collisions(akbasic_SpriteBackend *self, uint16_t FAIL_ZERO_RETURN(errctx, (mask != NULL), AKERR_NULLPOINTER, "NULL mask in collisions"); *mask = 0; + for ( i = 0; i < AKBASIC_MAX_SPRITES; i++ ) { + live[i] = slot_collidable(state, i); + synced[i] = false; + if ( !live[i] ) { + continue; + } + box[i].x = state->actors[i]->x; + box[i].y = state->actors[i]->y; + box[i].w = (float32_t)state->sprites[i]->width * (state->xexpand[i] ? 2.0f : 1.0f); + box[i].h = (float32_t)state->sprites[i]->height * (state->yexpand[i] ? 2.0f : 1.0f); + } + /* - * Axis-aligned bounding boxes, compared here rather than through - * akgl_collide_rectangles(): that function has a documented - * corner-containment defect -- it misses a plus-shaped overlap where neither - * rectangle contains a corner of the other -- and nothing in libakgl calls - * it. Four comparisons are both correct and smaller. + * **Two phases, and the split is the whole performance story.** * - * Box against box, not pixel against pixel. A C128 collides on set pixels, - * so two sprites whose boxes touch but whose art does not are reported as - * colliding here and would not be there. Recorded in TODO.md section 5. + * The obvious implementation -- sync all eight proxies, then run + * akgl_collision_test() on all twenty-eight pairs -- was measured at 984 ns a + * scan against 96 ns for the four-comparison loop it replaced. At 256 scans a + * frame that is 21% of a frame, and it bought nothing: the mask came back + * bit-identical and the contact was thrown away. + * + * So: reject on the bounding boxes first, in the four comparisons that were + * always here, and pay for the narrowphase only on a pair that survives. + * Nearly every scan of a real game rejects all twenty-eight, and a pair that + * does overlap is worth an exact answer. This is not a workaround for a slow + * library -- it is what a broad phase *is*, and akgl_collision_test() does the + * same thing internally with the proxies' own bounds before it commits to a + * solver. + * + * **The narrowphase still decides.** The box test only says "maybe"; the bit + * is set by what the library answers. That matters for a shape that is not a + * box -- a circle inscribed in a frame overlaps a smaller region than the + * frame does -- so the prefilter can over-report and must never be the final + * word. Today every shape is the whole frame and the two always agree, which + * is exactly why this is the moment to get the ordering right rather than + * the moment it starts to matter. + * + * AKGL_COLLISION_TEST_PLANAR because a Commodore sprite has no z. Without it + * the extrusion the shape builder chose takes part in the test and the + * minimum axis can come back as a depth along z, which no BASIC program can + * act on. + * + * Still box against box, not pixel against pixel: a C128's VIC-II collides on + * set pixels, so two sprites whose boxes overlap but whose art does not are + * reported as colliding here and would not be there. TODO.md section 5. */ for ( i = 0; i < AKBASIC_MAX_SPRITES; i++ ) { + if ( !live[i] ) { + continue; + } for ( j = i + 1; j < AKBASIC_MAX_SPRITES; j++ ) { - float32_t ax = 0.0f, ay = 0.0f, aw = 0.0f, ah = 0.0f; - float32_t bx = 0.0f, by = 0.0f, bw = 0.0f, bh = 0.0f; - - if ( state->actors[i] == NULL || state->actors[j] == NULL ) { + if ( !live[j] ) { continue; } - if ( !state->actors[i]->visible || !state->actors[j]->visible ) { + if ( !(box[i].x < box[j].x + box[j].w && box[j].x < box[i].x + box[i].w + && box[i].y < box[j].y + box[j].h && box[j].y < box[i].y + box[i].h) ) { continue; } - if ( state->sprites[i] == NULL || state->sprites[j] == NULL ) { - continue; + /* + * PASS rather than CATCH throughout: this is a loop, and CATCH + * expands to a break, which would leave the remaining pairs untested + * and the mask half built. + */ + if ( !synced[i] ) { + PASS(errctx, sync_proxy(state, i, &box[i])); + synced[i] = true; } - ax = state->actors[i]->x; - ay = state->actors[i]->y; - aw = (float32_t)state->sprites[i]->width * (state->xexpand[i] ? 2.0f : 1.0f); - ah = (float32_t)state->sprites[i]->height * (state->yexpand[i] ? 2.0f : 1.0f); - bx = state->actors[j]->x; - by = state->actors[j]->y; - bw = (float32_t)state->sprites[j]->width * (state->xexpand[j] ? 2.0f : 1.0f); - bh = (float32_t)state->sprites[j]->height * (state->yexpand[j] ? 2.0f : 1.0f); - - if ( ax < bx + bw && bx < ax + aw && ay < by + bh && by < ay + ah ) { + if ( !synced[j] ) { + PASS(errctx, sync_proxy(state, j, &box[j])); + synced[j] = true; + } + hit = false; + PASS(errctx, akgl_collision_test(state->proxies[i], state->proxies[j], + AKGL_COLLISION_TEST_PLANAR, &contact, &hit)); + if ( hit ) { *mask |= (uint16_t)(1u << i); *mask |= (uint16_t)(1u << j); } @@ -536,6 +656,7 @@ static akerr_ErrorContext *spr_collisions(akbasic_SpriteBackend *self, uint16_t akerr_ErrorContext *akbasic_sprite_init_akgl(akbasic_SpriteBackend *obj, akbasic_AkglSprites *state, akgl_RenderBackend *renderer, akbasic_AkglGraphics *graphics) { PREPARE_ERROR(errctx); + int i = 0; FAIL_ZERO_RETURN(errctx, (obj != NULL && state != NULL && renderer != NULL), AKERR_NULLPOINTER, "NULL argument in sprite_init_akgl"); @@ -546,6 +667,37 @@ akerr_ErrorContext *akbasic_sprite_init_akgl(akbasic_SpriteBackend *obj, akbasic state->renderer = renderer; state->graphics = graphics; + /* + * The eight collision proxies, claimed here and held for the life of the + * backend. + * + * Up front rather than on demand because the pool is shared: a host game + * embedding this interpreter draws its own shaped actors from the same + * AKGL_MAX_HEAP_COLLISION_PROXY. Claiming late would mean a collision scan + * discovering mid-game that a host had taken the last slot, and reporting it + * against whichever BASIC line was unlucky. Claiming here makes it an + * initialization failure that names the pool, before a program has run. + * + * A default box shape so a proxy is never initialized from an uninitialized + * shape -- libakgl fixed exactly that defect in 3a6569e, where filing a proxy + * under half-extents that had not been written yet was invisible to its + * tests and visible only to a memory checker. Real geometry arrives on the + * first sync_proxy(); this is only ever a placeholder. + */ + for ( i = 0; i < AKBASIC_MAX_SPRITES; i++ ) { + SDL_FRect placeholder = { 0.0f, 0.0f, 1.0f, 1.0f }; + + PASS(errctx, akgl_collision_shape_box(&state->shapes[i], &placeholder, 0.0f)); + /* + * Acquire and initialize adjacent, with nothing between them: the slot + * is free until initialize takes the reference, and libakgl's heap says + * so where next_collision_proxy is declared. + */ + PASS(errctx, akgl_heap_next_collision_proxy(&state->proxies[i])); + PASS(errctx, akgl_collision_proxy_initialize(state->proxies[i], NULL, + &state->shapes[i], 0.0f, 0.0f, 0.0f)); + } + memset(obj, 0, sizeof(*obj)); obj->self = state; obj->define = spr_define; @@ -591,5 +743,16 @@ void akbasic_sprite_akgl_shutdown(akbasic_SpriteBackend *obj) state = (akbasic_AkglSprites *)obj->self; for ( i = 0; i < AKBASIC_MAX_SPRITES; i++ ) { IGNORE(release_slot(state, i)); + /* + * The proxies go back too. They are not registered with any partitioner, + * so there is nothing to unlink first -- which is the one ordering trap + * here, and the reason worth writing down: releasing a proxy that *is* + * registered leaves stale cell entries holding a pool index, and the + * damage only shows when the slot is handed out again. + */ + if ( state->proxies[i] != NULL ) { + IGNORE(akgl_heap_release_collision_proxy(state->proxies[i])); + state->proxies[i] = NULL; + } } } diff --git a/tests/akgl_backends.c b/tests/akgl_backends.c index 30c7ad2..96b711c 100644 --- a/tests/akgl_backends.c +++ b/tests/akgl_backends.c @@ -115,6 +115,20 @@ static akerr_ErrorContext AKERR_NOIGNORE *start_runtime(const char *source) static void stop_runtime(void) { + /* + * The sprite backend has to be torn down, not just dropped. + * + * This used to close the output file and nothing else, and got away with it + * because akbasic_sprite_init_akgl() claimed no pooled objects -- the actors + * and textures a program had created leaked, but at two or three per case + * and 64 actor slots nothing ever ran out. Claiming eight collision proxies + * per backend changed the arithmetic: twenty cases at eight apiece is a + * hundred and sixty against a pool of a hundred and twenty-eight, and the + * suite started failing in whichever case happened to be twelfth. + * + * A host releases what it took. So does this. + */ + akbasic_sprite_akgl_shutdown(&SPRITES); if ( OUT != NULL ) { fclose(OUT); OUT = NULL; diff --git a/tests/collision_perf.c b/tests/collision_perf.c index 319cd47..4b1b2d8 100644 --- a/tests/collision_perf.c +++ b/tests/collision_perf.c @@ -88,14 +88,17 @@ static akerr_ErrorContext AKERR_NOIGNORE *load(const char *source) } /** - * @brief A program defining @p n sprites, all overlapping at the origin. + * @brief A program defining @p n sprites, stacked or spread out. * - * Overlapping rather than scattered on purpose: it is the worst case for the - * pair loop, since every pair that passes the visibility guards goes on to the - * four comparisons and sets two bits. A scattered arrangement measures the - * early-out instead, and the early-out is not the thing anybody is worried about. + * **Both arrangements are worth a row, and they measure different things.** + * Stacked, every pair survives the bounding-box reject and pays for the + * narrowphase -- the worst case the scan can be put in, and not a state a real + * game sits in. Spread out, every pair is rejected on four comparisons, which is + * what a game looks like on almost every frame. The gap between the two rows is + * the prefilter earning its place, and libakgl's own table splits its overlap + * and disjoint rows for the same reason. */ -static void sprite_program(char *dest, size_t size, int n) +static void sprite_program(char *dest, size_t size, int n, bool stacked) { char line[128]; int i = 0; @@ -106,17 +109,68 @@ static void sprite_program(char *dest, size_t size, int n) "30 P#(I#) = 255\n" "40 NEXT I#\n"); for ( i = 1; i <= n; i++ ) { + /* 40 apart is wider than a 24x21 sprite, so no two boxes can touch. */ + int x = (stacked ? 20 : 20 + ((i - 1) * 40)); + snprintf(line, sizeof(line), - "%d SPRSAV P#, %d\n%d SPRITE %d, 1\n%d MOVSPR %d, 20, 20\n", + "%d SPRSAV P#, %d\n%d SPRITE %d, 1\n%d MOVSPR %d, %d, 20\n", 100 + (i * 10), i, 101 + (i * 10), i, - 102 + (i * 10), i); + 102 + (i * 10), i, x); strncat(dest, line, size - strlen(dest) - 1); } } -/** @brief Time the collision scan with @p n sprites live. */ -static akerr_ErrorContext AKERR_NOIGNORE *bench_scan(int n) +/** + * @brief The arrangement examples/breakout/sprites/breakout.bas actually has. + * + * Neither synthetic row is what that game looks like, and it is the most + * demanding program in the repository. Two of its eight sprites are the *screen* + * -- a captured HUD strip and a captured play field, installed as sprites because + * a sprite is the one thing this interpreter draws for nothing -- so the field's + * box covers everything and every moving sprite overlaps it permanently. The + * bounding-box reject can never throw those pairs out, which puts this game + * between the two synthetic rows rather than at the cheap end of them. + */ +static akerr_ErrorContext AKERR_NOIGNORE *bench_breakout_shaped(void) +{ + PREPARE_ERROR(errctx); + akerr_ErrorContext *inner = NULL; + uint16_t mask = 0; + int iterations = bench_iterations(200000); + int i = 0; + + PASS(errctx, load("10 GRAPHIC 1, 1\n" + "20 SSHAPE H$, 0, 0, 799, 59\n" + "30 SSHAPE F$, 0, 60, 799, 599\n" + "40 SSHAPE S$, 0, 0, 22, 22\n" + "50 SPRSAV H$, 1\n" + "60 SPRSAV F$, 2\n" + "70 SPRSAV S$, 3\n" + "80 SPRSAV S$, 5\n" + "90 SPRSAV S$, 6\n" + "100 SPRSAV S$, 7\n" + "110 SPRSAV S$, 8\n" + "120 FOR I# = 1 TO 8\n" + "130 SPRITE I#, 1\n" + "140 NEXT I#\n" + "150 MOVSPR 1, 0, 0\n" + "160 MOVSPR 2, 0, 60\n" + "170 MOVSPR 3, 300, 540\n" + "180 MOVSPR 5, 100, 300\n" + "190 MOVSPR 6, 400, 200\n" + "200 MOVSPR 7, 600, 400\n" + "210 MOVSPR 8, 200, 150\n")); + + bench_start("spr_collisions, breakout's own layout", "call", 0.0); + BENCH_LOOP(inner, i, iterations, SPRITES.collisions(&SPRITES, &mask)); + bench_stop((uint64_t)iterations); + PASS(errctx, inner); + SUCCEED_RETURN(errctx); +} + +/** @brief Time the collision scan with @p n sprites live, stacked or spread. */ +static akerr_ErrorContext AKERR_NOIGNORE *bench_scan(int n, bool stacked) { PREPARE_ERROR(errctx); akerr_ErrorContext *inner = NULL; @@ -127,10 +181,11 @@ static akerr_ErrorContext AKERR_NOIGNORE *bench_scan(int n) int i = 0; memset(source, 0, sizeof(source)); - sprite_program(source, sizeof(source), n); + sprite_program(source, sizeof(source), n, stacked); PASS(errctx, load(source)); - snprintf(name, sizeof(name), "spr_collisions, %d sprites", n); + snprintf(name, sizeof(name), "spr_collisions, %d sprites, %s", n, + (stacked ? "all overlapping" : "spread out")); bench_start(name, "call", 0.0); BENCH_LOOP(inner, i, iterations, SPRITES.collisions(&SPRITES, &mask)); bench_stop((uint64_t)iterations); @@ -154,7 +209,7 @@ static akerr_ErrorContext AKERR_NOIGNORE *bench_frame(void) int i = 0; memset(source, 0, sizeof(source)); - sprite_program(source, sizeof(source), 8); + sprite_program(source, sizeof(source), 8, false); /* A screenful of text, so the sink renders what a real program's sink renders. */ strncat(source, "900 FOR R# = 0 TO 30\n" @@ -215,10 +270,12 @@ int main(void) FAIL_ZERO_BREAK(errctx, font, AKGL_ERR_SDL, "Couldn't open %s: %s", AKBASIC_TEST_FONT, SDL_GetError()); - CATCH(errctx, bench_scan(0)); - CATCH(errctx, bench_scan(2)); - CATCH(errctx, bench_scan(4)); - CATCH(errctx, bench_scan(8)); + CATCH(errctx, bench_scan(0, false)); + CATCH(errctx, bench_scan(2, false)); + CATCH(errctx, bench_scan(4, false)); + CATCH(errctx, bench_scan(8, false)); + CATCH(errctx, bench_scan(8, true)); + CATCH(errctx, bench_breakout_shaped()); CATCH(errctx, bench_frame()); BENCH_REPORT_BREAK(errctx); } CLEANUP {