Pool collision proxies, and tie one to the life of its actor

A proxy is what the broad phase will index: an actor's shape, where it is, and
the bounds that follow from those. It is a sixth heap layer, two per actor --
one for the actor itself and headroom for static geometry a game registers that
is not tile-aligned. Solid *tiles* deliberately get none: the broad phase will
read those out of the tilemap's own array, because one proxy per tile is tens of
megabytes to index data that is already a grid.

The pool follows the existing convention rather than improving on it.
akgl_heap_next_collision_proxy finds a free slot and does not claim it;
akgl_collision_proxy_initialize claims it. That is TODO.md item 8's asymmetry,
and it is kept on purpose: an acquire abandoned before initialization leaks
nothing, because the slot still reads as free, and a sixth pool with its own
rule would be worse than one defect with five instances. Fixing item 8 means
fixing all five together with every call site audited, and that is its own
change.

The proxy holds a *copy* of the shape rather than a pointer. An actor's own
logic may rewrite its shape mid-step -- a character crouching, a projectile
arming -- and a broad phase whose bounds came from a shape that has since
changed is a bug that only appears when two things happen in the same frame.

akgl_heap_release_actor now releases the actor's proxy. Without it the proxy
holds a borrowed `owner` into a slot that has just been zeroed, so the broad
phase keeps a registration whose owner reads as a free actor, and the next
contact against it is a collision with nothing. Verified by removing the release
and watching test_proxy_dies_with_its_actor go red.

The tests pin the convention as well as the behaviour: that two acquires with no
initialize between them return the same slot is asserted, not merely tolerated,
so that changing the convention has to change a test that says why it exists.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-01 23:56:21 -04:00
parent 9056ff06d6
commit d0b057f47a
6 changed files with 331 additions and 0 deletions

View File

@@ -15,8 +15,11 @@
#include <akerror.h>
#include <akgl/actor.h>
#include <akgl/collision.h>
#include <akgl/error.h>
#include <akgl/heap.h>
#include <akgl/registry.h>
#include "testutil.h"
@@ -305,6 +308,145 @@ akerr_ErrorContext *test_shape_interacts(void)
SUCCEED_RETURN(errctx);
}
/**
* @brief The pool convention, and the window it deliberately leaves open.
*/
akerr_ErrorContext *test_proxy_pool(void)
{
akgl_CollisionProxy *first = NULL;
akgl_CollisionProxy *second = NULL;
akgl_CollisionShape shape;
SDL_FRect body = { .x = 0.0f, .y = 0.0f, .w = 16.0f, .h = 16.0f };
akerr_ErrorContext *inner = NULL;
int claimed = 0;
int i = 0;
PREPARE_ERROR(errctx);
ATTEMPT {
CATCH(errctx, akgl_heap_init());
CATCH(errctx, akgl_collision_shape_box(&shape, &body, 0.0f));
/*
* Acquiring does not claim the slot, and that is the convention rather
* than an oversight: an acquire abandoned before initialization leaks
* nothing, because the slot still reads as free. The cost is this window,
* where two acquires in a row return the same pointer -- which is why the
* library writes the acquire and the initialize adjacent.
*/
CATCH(errctx, akgl_heap_next_collision_proxy(&first));
TEST_ASSERT(errctx, (first->refcount == 0),
"acquiring a proxy claimed the slot; that is not this pool's convention");
CATCH(errctx, akgl_heap_next_collision_proxy(&second));
TEST_ASSERT(errctx, (first == second),
"two acquires with no initialize between them returned different slots");
// Initializing is what closes it.
CATCH(errctx, akgl_collision_proxy_initialize(first, NULL, &shape, 10.0f, 20.0f, 0.0f));
TEST_ASSERT(errctx, (first->refcount == 1), "initializing did not take the reference");
CATCH(errctx, akgl_heap_next_collision_proxy(&second));
TEST_ASSERT(errctx, (first != second), "an initialized slot was handed out again");
// The proxy carries a copy of the shape, not a pointer to it, so a later
// edit to the caller's shape cannot change what the broad phase indexed.
shape.hx = 999.0f;
TEST_ASSERT_FEQ(errctx, first->shape.hx, 8.0f,
"the proxy tracked an edit to the caller's shape; it holds %f", first->shape.hx);
// Body {0,0,16,16} centres at (8,8) with half-extents 8, so at owner x=10
// the bounds start at 10 and are 16 wide.
TEST_ASSERT_FEQ(errctx, first->bounds.x, 10.0f,
"proxy bounds x is %f, expected 10", first->bounds.x);
TEST_ASSERT_FEQ(errctx, first->bounds.y, 20.0f,
"proxy bounds y is %f, expected 20", first->bounds.y);
TEST_ASSERT_FEQ(errctx, first->bounds.w, 16.0f,
"proxy bounds w is %f, expected 16", first->bounds.w);
CATCH(errctx, akgl_heap_release_collision_proxy(first));
TEST_ASSERT(errctx, (first->refcount == 0), "releasing did not free the slot");
// Exhaustion is an ordinary reported error, not a crash and not a silent
// hand-out of a slot somebody else is using.
CATCH(errctx, akgl_heap_init());
for ( i = 0; i < (AKGL_MAX_HEAP_COLLISION_PROXY + 4); i++ ) {
inner = akgl_heap_next_collision_proxy(&first);
if ( inner != NULL ) {
break;
}
inner = akgl_collision_proxy_initialize(first, NULL, &shape, 0.0f, 0.0f, 0.0f);
if ( inner != NULL ) {
break;
}
claimed += 1;
}
TEST_ASSERT(errctx, (inner != NULL), "the proxy pool never ran out");
TEST_EXPECT_STATUS(errctx, AKGL_ERR_HEAP, inner, "exhausting the proxy pool");
inner = NULL;
TEST_ASSERT(errctx, (claimed == AKGL_MAX_HEAP_COLLISION_PROXY),
"the pool handed out %d of %d slots before refusing",
claimed, AKGL_MAX_HEAP_COLLISION_PROXY);
TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, akgl_heap_next_collision_proxy(NULL),
"acquiring into no destination");
TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, akgl_heap_release_collision_proxy(NULL),
"releasing no proxy");
TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER,
akgl_collision_proxy_initialize(NULL, NULL, &shape, 0.0f, 0.0f, 0.0f),
"initializing no proxy");
} CLEANUP {
if ( inner != NULL ) {
inner->handled = true;
inner = akerr_release_error(inner);
}
} PROCESS(errctx) {
} FINISH(errctx, true);
SUCCEED_RETURN(errctx);
}
/**
* @brief A proxy must not outlive the actor it points at.
*
* akgl_heap_release_actor zeroes the actor slot. A proxy left registered would
* hold a borrowed `owner` into a slot that now reads as free, and the next
* contact against it would be a collision with nothing -- so the release has to
* reach through and take the proxy with it.
*/
akerr_ErrorContext *test_proxy_dies_with_its_actor(void)
{
akgl_Actor *actor = NULL;
akgl_CollisionProxy *proxy = NULL;
akgl_CollisionShape shape;
SDL_FRect body = { .x = 0.0f, .y = 0.0f, .w = 16.0f, .h = 16.0f };
PREPARE_ERROR(errctx);
ATTEMPT {
CATCH(errctx, akgl_heap_init());
CATCH(errctx, akgl_registry_init_actor());
CATCH(errctx, akgl_collision_shape_box(&shape, &body, 0.0f));
CATCH(errctx, akgl_heap_next_actor(&actor));
CATCH(errctx, akgl_actor_initialize(actor, "collider"));
CATCH(errctx, akgl_heap_next_collision_proxy(&proxy));
CATCH(errctx, akgl_collision_proxy_initialize(proxy, actor, &shape, 0.0f, 0.0f, 0.0f));
actor->proxy = proxy;
TEST_ASSERT(errctx, (proxy->refcount == 1), "the proxy was not claimed");
CATCH(errctx, akgl_heap_release_actor(actor));
TEST_ASSERT(errctx, (proxy->refcount == 0),
"releasing the actor left its proxy registered, pointing at a freed slot");
TEST_ASSERT(errctx, (proxy->owner == NULL), "the released proxy still names an owner");
// An actor with no proxy releases without complaint, which is every
// actor a game written before this existed.
CATCH(errctx, akgl_heap_next_actor(&actor));
CATCH(errctx, akgl_actor_initialize(actor, "plain"));
CATCH(errctx, akgl_heap_release_actor(actor));
} CLEANUP {
} PROCESS(errctx) {
} FINISH(errctx, true);
SUCCEED_RETURN(errctx);
}
int main(void)
{
PREPARE_ERROR(errctx);
@@ -316,6 +458,8 @@ int main(void)
CATCH(errctx, test_shape_extrusion_beats_planar_penetration());
CATCH(errctx, test_shape_bounds());
CATCH(errctx, test_shape_interacts());
CATCH(errctx, test_proxy_pool());
CATCH(errctx, test_proxy_dies_with_its_actor());
} CLEANUP {
} PROCESS(errctx) {
} FINISH_NORETURN(errctx);