From 842ef75ddf4211b11c8e58620567c50fc45b5fd5 Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Sat, 1 Aug 2026 23:12:01 -0400 Subject: [PATCH] Report the overlap akgl_collide_rectangles could not see It asked whether either rectangle enclosed one of the other's four corners -- eight akgl_collide_point_rectangle calls, stopping at the first hit. That is a different question from "do these overlap", and it has the wrong answer for one arrangement: a tall thin rectangle crossing a short wide one overlaps in a plus sign with no corner of either inside the other, and all eight tests said no. A long thin platform crossing a tall thin character is exactly that shape, so this is a shape a 2D game produces, not a curiosity. util.h carried an @note describing it and docs/18-utilities.md had a diagram of it, both under the heading of a limitation rather than a defect, and there was no test for it at all -- nor for full containment, nor for a shared edge. It is four comparisons now, on both axes. `<=` rather than `<` because akgl_collide_point_rectangle is inclusive on all four edges and these two have always agreed that touching counts; a span test written with `<` would have silently changed a contract both the header and the manual state. The test was written first and failed on the cross before the fix went in. Two answers change for a caller upgrading, and both are in the header note and the chapter: - The cross reports `true`, which is the point. - The comparison is in float rather than through akgl_Point's int members, so a sub-pixel overlap is no longer truncated away. A pickup test that was accidentally forgiving by up to a pixel is no longer forgiving. Both tutorials use this for coins and hazards; both still pass. Faster as a side effect rather than a goal, and worth recording because the numbers move a documented budget: 24.9 ns -> 6.1 overlapping, 57.9 -> 6.1 disjoint, and the all-pairs sweep over 64 actors 115 us -> 12.2. The disjoint case gained most because it was the one that ran all eight tests before answering. The three moved rows are re-recorded in PERFORMANCE.md and nothing else is. akgl_rectangle_points is untouched by this change and reads 6.1 in the same run against the 4.0 recorded, so 6 ns is this run's floor and the new figure means "too cheap to measure" rather than "exactly 6.1" -- said in the prose so the next reader does not re-baseline the table around it. Co-Authored-By: Claude Code Co-Authored-By: Claude Opus 5 (1M context) --- PERFORMANCE.md | 32 ++++++++++------ docs/01-introduction.md | 2 +- docs/18-utilities.md | 38 +++++++++++-------- include/akgl/util.h | 12 ++++-- src/util.c | 65 +++++++++++-------------------- tests/util.c | 84 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 158 insertions(+), 75 deletions(-) diff --git a/PERFORMANCE.md b/PERFORMANCE.md index a6cf340..8276634 100644 --- a/PERFORMANCE.md +++ b/PERFORMANCE.md @@ -126,9 +126,9 @@ reciprocal. | `akgl_physics_simulate`, empty pool | frame | 63.9 | 15,650,101 | | logic frame: 64 updates + simulate | frame | 5,763.1 | 173,517 | | `akgl_rectangle_points` | call | 4.0 | 248,412,026 | -| `akgl_collide_rectangles`, overlapping | call | 24.9 | 40,181,108 | -| `akgl_collide_rectangles`, disjoint | call | 57.9 | 17,261,906 | -| all-pairs collision sweep, 64 actors (2016 pairs) | sweep | 115,023.6 | 8,694 | +| `akgl_collide_rectangles`, overlapping | call | 6.1 | 164,345,776 | +| `akgl_collide_rectangles`, disjoint | call | 6.1 | 164,423,708 | +| all-pairs collision sweep, 64 actors (2016 pairs) | sweep | 12,241.6 | 81,688 | | `akgl_string_initialize` | call | 32.1 | 31,197,432 | | `akgl_string_copy`, full length | call | 32.2 | 31,075,004 | | `json_load_file`, small document | load | 11,338.8 | 88,193 | @@ -183,7 +183,7 @@ full screen of 16-pixel tiles and 64 actors: | Part of the frame | Cost | Share of 16.67 ms | |---|---:|---:| | Logic: 64 actor updates + one physics sweep | 0.006 ms | 0.03% | -| All-pairs collision over 64 actors, if you do it | 0.115 ms | 0.7% | +| All-pairs collision over 64 actors, if you do it | 0.012 ms | 0.07% | | Clear + present | 0.023 ms | 0.1% | | 64 actor renders (48x48 blits) | 0.191 ms | 1.1% | | 1200 tile blits | 16.26 ms | 97.6% | @@ -306,15 +306,25 @@ and not a string operation. ### The collision helpers are fine; the missing broad phase is the problem -`akgl_collide_rectangles` is 24.9 ns when the rectangles overlap (it returns at -the first corner that hits) and 57.9 ns when they do not (all eight corner tests -run). Both are fine. +`akgl_collide_rectangles` is 6.1 ns whether the rectangles overlap or not. It +was 24.9 ns overlapping and 57.9 ns disjoint until 0.8.0, when it stopped being +eight `akgl_collide_point_rectangle` calls and became four comparisons -- a +change made to fix the cross case it could not see, with the speed as a side +effect rather than the goal. The disjoint case improved most because it was the +one that ran all eight tests before answering. + +**Read that 6.1 as a floor, not a measurement.** `akgl_rectangle_points` is +untouched by that change and reads 6.1 in the same run against the 4.0 recorded +above, so this run's resolution is around 6 ns and the collision test is now too +cheap to distinguish from a function that does almost nothing. The rows above are +not re-baselined for it; only the three rows the change actually moved are. What the library does not provide is a broad phase, so a caller that wants -collision writes the all-pairs loop: 2016 pairs for 64 actors, 115 µs a frame, -0.7% of a 60 fps budget. That is affordable. It is also O(n²): raise -`AKGL_MAX_HEAP_ACTOR` to 256 and the same loop is 32,640 pairs and 1.9 ms — over -10% of the frame, for a game that has done nothing yet. +collision writes the all-pairs loop: 2016 pairs for 64 actors, **12 µs** a frame, +0.07% of a 60 fps budget. That is affordable. It is still O(n²): raise +`AKGL_MAX_HEAP_ACTOR` to 256 and the same loop is 32,640 pairs, which at this +per-pair cost is around 200 µs — 1.2% of the frame, for a game that has done +nothing yet. ### Spawning is cheap, and a third of it is a log line diff --git a/docs/01-introduction.md b/docs/01-introduction.md index 6e8e451..9e13a54 100644 --- a/docs/01-introduction.md +++ b/docs/01-introduction.md @@ -39,7 +39,7 @@ software-rasterized. At 60 fps a frame is 16.67 ms: | Part of the frame | Cost | Share of 16.67 ms | |---|---:|---:| | Logic: 64 actor updates + one physics sweep | 0.006 ms | 0.03% | -| All-pairs collision over 64 actors, if you do it | 0.115 ms | 0.7% | +| All-pairs collision over 64 actors, if you do it | 0.012 ms | 0.07% | | Clear + present | 0.023 ms | 0.1% | | 64 actor renders (48x48 blits) | 0.191 ms | 1.1% | | 1200 tile blits | 16.26 ms | 97.6% | diff --git a/docs/18-utilities.md b/docs/18-utilities.md index 1186cce..bcd7ad2 100644 --- a/docs/18-utilities.md +++ b/docs/18-utilities.md @@ -53,36 +53,44 @@ drawn with a non-zero `angle` still collides as its unrotated box. **Edges count as touching.** A point exactly on a boundary is inside; two rectangles sharing an edge and no area overlap. Both comparisons are `>=` and `<=`. -**Coordinates are truncated from `float` to `int` on the way in.** A rectangle at -`x = 10.9` has its corners at 10. That is right for tile-grid work and wrong for sub-pixel -work; if you need the latter, do not round-trip through `akgl_rectangle_points`. +**`akgl_rectangle_points` truncates from `float` to `int`.** A rectangle at `x = 10.9` has +its corners at 10. That is right for tile-grid work and wrong for sub-pixel work; if you +need the latter, do not round-trip through it. **`akgl_collide_rectangles` no longer does** +— it compares the spans in `float` directly, so a sub-pixel overlap registers. **`z` is carried and never used.** `akgl_Point` has one, `akgl_rectangle_points` never writes it, and neither collision test reads it. These are 2D tests. -### The arrangement it misses +### The arrangement it used to miss -`akgl_collide_rectangles` works corner by corner: it tests each rectangle's four corners -against the other, eight tests, stopping at the first hit. Testing both directions is what -catches one rectangle wholly inside the other, which has no corner in its neighbour. - -**It still misses the cross.** A tall thin rectangle crossing a short wide one overlaps -without either enclosing a corner of the other, and both are reported as not colliding: +`akgl_collide_rectangles` compares the two rectangles' spans on both axes. Until 0.8.0 it +asked a different question — whether either rectangle enclosed one of the other's four +corners, eight tests, stopping at the first hit — and that question has the wrong answer for +one arrangement: ```text +---+ +---+ | | | | +---|---|---+ +---+---+---+ <- overlaps, but - | | | | is | | | | no corner of + | | | | was | | | | no corner of +---|---|---+ reported +---+---+---+ either is inside | | as | | the other +---+ "no hit" +---+ ``` -The fix is an edge-span comparison instead of a corner-containment one — `r1.x < r2.x + r2.w -&& r2.x < r1.x + r1.w` on both axes — and it is not made here. If your game can produce that -shape, and a long thin platform crossing a tall thin character is exactly that shape, test -for it yourself. Recorded as an `@note` on the function. +A tall thin rectangle crossing a short wide one overlaps in a plus sign with no corner of +either inside the other, so all eight tests said no — and a long thin platform crossing a +tall thin character is exactly that shape. The header carried a `@note` describing it rather +than a fix. + +It is four comparisons now, `r1.x <= r2.x + r2.w && r2.x <= r1.x + r1.w` on both axes. +`<=` rather than `<` because [`akgl_collide_point_rectangle`](#collision-helpers) is +inclusive on all four edges and these two have always agreed: touching counts. + +**If you are upgrading from 0.7.x, two answers change.** The cross now reports `true`, which +is the point. And because the comparison no longer round-trips through `akgl_Point`'s `int` +members, overlaps and gaps smaller than a pixel are now seen rather than truncated away — so +a pickup test that was accidentally forgiving by up to a pixel is no longer forgiving. This is separate from the fact that **nothing in the library calls these during a frame.** `akgl_physics_simulate` never calls `collide`, and `akgl_physics_arcade_collide` raises diff --git a/include/akgl/util.h b/include/akgl/util.h index 22e85d8..19ef1cb 100644 --- a/include/akgl/util.h +++ b/include/akgl/util.h @@ -89,10 +89,14 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_collide_point_rectangle(akgl_Point *p, a * @return `NULL` on success, otherwise an error context owned by the caller. * @throws AKERR_NULLPOINTER If @p r1, @p r2, or @p collide is `NULL`. * - * @note A corner-containment test misses the one arrangement where two - * rectangles overlap in a cross without either enclosing a corner of the - * other -- a tall thin rectangle crossing a short wide one. Both are - * reported as not colliding. + * @note Fixed in 0.8.0. This was eight corner-containment tests, which answer a + * different question and get the cross wrong: a tall thin rectangle + * crossing a short wide one overlaps without either enclosing a corner of + * the other, and every corner test said no. It is a span comparison on + * both axes now. Two consequences for a caller upgrading: that + * arrangement starts reporting `true`, and the comparison is in `float` + * rather than through akgl_Point's `int` members, so an overlap smaller + * than one pixel is no longer truncated away. */ akerr_ErrorContext AKERR_NOIGNORE *akgl_collide_rectangles(SDL_FRect *r1, SDL_FRect *r2, bool *collide); diff --git a/src/util.c b/src/util.c index 548c9bc..c045ef2 100644 --- a/src/util.c +++ b/src/util.c @@ -163,56 +163,33 @@ akerr_ErrorContext *akgl_collide_point_rectangle(akgl_Point *p, akgl_RectanglePo akerr_ErrorContext *akgl_collide_rectangles(SDL_FRect *r1, SDL_FRect *r2, bool *collide) { - akgl_RectanglePoints r1p; - akgl_RectanglePoints r2p; PREPARE_ERROR(errctx); FAIL_ZERO_RETURN(errctx, r1, AKERR_NULLPOINTER, "NULL rectangle reference"); FAIL_ZERO_RETURN(errctx, r2, AKERR_NULLPOINTER, "NULL rectangle reference"); FAIL_ZERO_RETURN(errctx, collide, AKERR_NULLPOINTER, "NULL collision flag reference"); - ATTEMPT { - CATCH(errctx, akgl_rectangle_points(&r1p, r1)); - CATCH(errctx, akgl_rectangle_points(&r2p, r2)); + /* + * Two rectangles overlap when their spans overlap on both axes. This used to + * ask a different question -- whether either rectangle enclosed a corner of + * the other, eight akgl_collide_point_rectangle calls -- and that question + * has a different answer for a tall thin rectangle crossing a short wide + * one. They overlap in a plus sign, neither holds a corner of the other, and + * all eight tests said no. util.h carried a @note describing the arrangement + * rather than a fix. + * + * `<=` and not `<`, because akgl_collide_point_rectangle is inclusive on all + * four edges and this function has always agreed with it: two rectangles + * sharing exactly one edge collide. + * + * The comparison is in float now. The corner form went through akgl_Point, + * whose members are int, so a rectangle at x = 10.9 had its corner at 10 and + * a sub-pixel overlap was invisible. + */ + *collide = ( (r1->x <= (r2->x + r2->w)) && + (r2->x <= (r1->x + r1->w)) && + (r1->y <= (r2->y + r2->h)) && + (r2->y <= (r1->y + r1->h)) ); - // is the upper left corner of r1 contacting r2? - CATCH(errctx, akgl_collide_point_rectangle(&r1p.topleft, &r2p, collide)); - if ( *collide == true ) { break; } - - // is the upper left corner of r2 contacting r1? - CATCH(errctx, akgl_collide_point_rectangle(&r2p.topleft, &r1p, collide)); - if ( *collide == true ) { break; } - - // is the top right corner of r1 contacting r2? - CATCH(errctx, akgl_collide_point_rectangle(&r1p.topright, &r2p, collide)); - if ( *collide == true ) { break; } - - // is the top right corner of r2 contacting r1? - CATCH(errctx, akgl_collide_point_rectangle(&r2p.topright, &r1p, collide)); - if ( *collide == true ) { break; } - - // is the bottom left corner of r1 contacting r2? - CATCH(errctx, akgl_collide_point_rectangle(&r1p.bottomleft, &r2p, collide)); - if ( *collide == true ) { break; } - - // is the bottom left corner of r2 contacting r1? - CATCH(errctx, akgl_collide_point_rectangle(&r2p.bottomleft, &r1p, collide)); - if ( *collide == true ) { break; } - - // is the bottom right corner of r1 contacting r2? - CATCH(errctx, akgl_collide_point_rectangle(&r1p.bottomright, &r2p, collide)); - if ( *collide == true ) { break; } - - // is the bottom right corner of r2 contacting r1? - CATCH(errctx, akgl_collide_point_rectangle(&r2p.bottomright, &r1p, collide)); - if ( *collide == true ) { break; } - - } CLEANUP { - } PROCESS(errctx) { - } FINISH(errctx, true); - - // No trailing `*collide = false;` here. Each corner test writes the flag - // itself, so the eighth one leaves it false when nothing hit; assigning - // after FINISH would overwrite the hit that broke out of the block early. SUCCEED_RETURN(errctx); } diff --git a/tests/util.c b/tests/util.c index d39281e..515b4a7 100644 --- a/tests/util.c +++ b/tests/util.c @@ -247,6 +247,89 @@ akerr_ErrorContext *test_akgl_collide_rectangles_nullpointers(void) SUCCEED_RETURN(errctx); } +/** + * @brief The arrangements corner containment cannot see, and the ones it must keep. + * + * Eight corner-in-rectangle tests answer "do these overlap" correctly only when + * one rectangle encloses a corner of the other. A tall thin rectangle crossing a + * short wide one overlaps in a plus sign with no corner inside either, and eight + * corner tests all report false. That was documented on akgl_collide_rectangles + * as a known limitation rather than fixed, and there was no test for it. + * + * The three cases below the cross are the ones a rewrite can break while fixing + * it: touching edges must keep counting as a collision, because + * akgl_collide_point_rectangle is inclusive on all four edges and both headers + * promise it; full containment must keep working; and a separation of less than + * one pixel must be seen, which the old implementation could not do because it + * routed through akgl_Point and truncated float to int. + */ +akerr_ErrorContext *test_akgl_collide_rectangles_arrangements(void) +{ + SDL_FRect tall = { .x = 10.0f, .y = 0.0f, .w = 4.0f, .h = 40.0f }; + SDL_FRect wide = { .x = 0.0f, .y = 10.0f, .w = 40.0f, .h = 4.0f }; + SDL_FRect outer = { .x = 0.0f, .y = 0.0f, .w = 64.0f, .h = 64.0f }; + SDL_FRect inner = { .x = 16.0f, .y = 16.0f, .w = 8.0f, .h = 8.0f }; + SDL_FRect left = { .x = 0.0f, .y = 0.0f, .w = 10.0f, .h = 10.0f }; + SDL_FRect right = { .x = 10.0f, .y = 0.0f, .w = 10.0f, .h = 10.0f }; + SDL_FRect near1 = { .x = 0.0f, .y = 0.0f, .w = 10.9f, .h = 10.0f }; + SDL_FRect near2 = { .x = 10.5f, .y = 0.0f, .w = 10.0f, .h = 10.0f }; + bool collide = false; + + PREPARE_ERROR(errctx); + + ATTEMPT { + // The cross. Neither rectangle encloses a corner of the other, and they + // plainly overlap in the middle. + CATCH(errctx, akgl_collide_rectangles(&tall, &wide, &collide)); + TEST_ASSERT(errctx, (collide == true), + "a tall rectangle crossing a wide one was reported as not colliding"); + + // The same pair the other way round. The test is symmetric and has to be. + CATCH(errctx, akgl_collide_rectangles(&wide, &tall, &collide)); + TEST_ASSERT(errctx, (collide == true), + "the crossing pair was reported as not colliding with the arguments swapped"); + + // Full containment, both orders. + CATCH(errctx, akgl_collide_rectangles(&outer, &inner, &collide)); + TEST_ASSERT(errctx, (collide == true), "a contained rectangle was missed"); + CATCH(errctx, akgl_collide_rectangles(&inner, &outer, &collide)); + TEST_ASSERT(errctx, (collide == true), "a containing rectangle was missed"); + + // A shared edge and nothing more. akgl_collide_point_rectangle is + // inclusive on all four edges, so touching counts; util.h and chapter 18 + // both say so, and a span test written with < rather than <= silently + // changes that. + CATCH(errctx, akgl_collide_rectangles(&left, &right, &collide)); + TEST_ASSERT(errctx, (collide == true), + "two rectangles sharing exactly one edge were reported as not colliding"); + + // Overlapping by four tenths of a pixel. The corner tests truncated + // float to int on the way in, so this read as a shared edge at 10. + CATCH(errctx, akgl_collide_rectangles(&near1, &near2, &collide)); + TEST_ASSERT(errctx, (collide == true), "a sub-pixel overlap was missed"); + + // Separated by four tenths of a pixel. Truncation read this as touching. + near2.x = 11.3f; + CATCH(errctx, akgl_collide_rectangles(&near1, &near2, &collide)); + TEST_ASSERT(errctx, (collide == false), "a sub-pixel gap was reported as a collision"); + + // Disjoint on one axis only, which is the case a span test gets wrong + // when it forgets to check both. + tall.x = 100.0f; + CATCH(errctx, akgl_collide_rectangles(&tall, &wide, &collide)); + TEST_ASSERT(errctx, (collide == false), + "rectangles separated on x were reported as colliding"); + tall.x = 10.0f; + tall.y = 100.0f; + CATCH(errctx, akgl_collide_rectangles(&tall, &wide, &collide)); + TEST_ASSERT(errctx, (collide == false), + "rectangles separated on y were reported as colliding"); + } CLEANUP { + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + akerr_ErrorContext *test_akgl_collide_rectangles_logic(void) { SDL_FRect testrect1 = { .x = 0, .y = 0, .w = 32, .h = 32}; @@ -465,6 +548,7 @@ int main(void) CATCH(errctx, test_akgl_collide_point_rectangle_logic()); CATCH(errctx, test_akgl_collide_rectangles_nullpointers()); CATCH(errctx, test_akgl_collide_rectangles_logic()); + CATCH(errctx, test_akgl_collide_rectangles_arrangements()); CATCH(errctx, test_akgl_compare_sdl_surfaces_checks_geometry()); CATCH(errctx, test_akgl_path_relative_releases_contexts()); } CLEANUP {