From 42b53dcb20205afd013ffbfce08a47bbc01f0ff3 Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Fri, 31 Jul 2026 12:52:20 -0400 Subject: [PATCH] Check the renderer backend before drawing text akgl_text_rendertextat() called renderer->draw_texture() with no check on the global renderer, its sdl_renderer or the function pointer itself, so a backend that has an SDL_Renderer but was never bound segfaults on the first line of text -- the state akgl_render_bind2d() exists to get a host out of. All three are now checked, with AKERR_NULLPOINTER as the draw entry points report, and checked before anything is rasterized rather than after. tests/text.c grows a software renderer with a bound backend and covers all three refusals plus the successful draw, wrapped and unwrapped: src/text.c goes from 58% to 100% line coverage. A made-up SDL_Renderer pointer is not enough to test this -- SDL refuses the bogus handle on its own and the call fails for the wrong reason, which a deleted check survives. Also documents what the tests found: SDL_ttf refuses the empty string, so drawing an empty line is an error while measuring one is not. Filed in TODO.md rather than pinned in the suite. Co-Authored-By: Claude Opus 5 (1M context) --- include/akgl/text.h | 10 +++- src/text.c | 7 +++ tests/text.c | 111 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 123 insertions(+), 5 deletions(-) diff --git a/include/akgl/text.h b/include/akgl/text.h index ea4d574..1242924 100644 --- a/include/akgl/text.h +++ b/include/akgl/text.h @@ -64,7 +64,10 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_text_loadfont(char *name, char *filepath * @param x Left edge of the text, in screen pixels. * @param y Top edge of the text, in screen pixels. * @return `NULL` on success, otherwise an error context owned by the caller. - * @throws AKERR_NULLPOINTER If @p font or @p text is `NULL`; if SDL_ttf cannot + * @throws AKERR_NULLPOINTER If @p font or @p text is `NULL`; if the global + * `renderer`, its `sdl_renderer`, or its `draw_texture` is `NULL` -- + * that last one is the state a backend is in between being allocated + * and being run through akgl_render_bind2d(); if SDL_ttf cannot * rasterize the string; or if the surface cannot be uploaded as a * texture. The last two carry `SDL_GetError()` and are a reused status * rather than a pointer problem. @@ -73,6 +76,11 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_text_loadfont(char *name, char *filepath * @note On a failure after rasterizing -- the texture upload, or the draw -- the * surface and texture are not destroyed, because the error returns before * the cleanup. Repeated failures leak. + * @note The empty string is **refused**, not drawn as nothing: SDL_ttf reports + * "Text has zero width" and this passes that on as `AKERR_NULLPOINTER`. + * akgl_text_measure() accepts it, so the two disagree. A caller drawing a + * line of text that may be empty has to check for it. TODO.md, "Known and + * still open". */ akerr_ErrorContext AKERR_NOIGNORE *akgl_text_rendertextat(TTF_Font *font, char *text, SDL_Color color, int wraplength, int x, int y); /** diff --git a/src/text.c b/src/text.c index 275dadb..4e97441 100644 --- a/src/text.c +++ b/src/text.c @@ -39,6 +39,13 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_text_rendertextat(TTF_Font *font, char * PREPARE_ERROR(errctx); FAIL_ZERO_RETURN(errctx, font, AKERR_NULLPOINTER, "NULL font"); FAIL_ZERO_RETURN(errctx, text, AKERR_NULLPOINTER, "NULL text string"); + // Checked before anything is rasterized, and checked at all because a + // backend that has an SDL_Renderer but was never run through + // akgl_render_bind2d has a NULL draw_texture -- which is exactly the state + // akgl_render_init2d used to be the only escape from. + FAIL_ZERO_RETURN(errctx, renderer, AKERR_NULLPOINTER, "No renderer backend"); + FAIL_ZERO_RETURN(errctx, renderer->sdl_renderer, AKERR_NULLPOINTER, "No valid SDL rendering backend"); + FAIL_ZERO_RETURN(errctx, renderer->draw_texture, AKERR_NULLPOINTER, "Renderer backend has no draw_texture"); if ( wraplength > 0 ) { textsurf = TTF_RenderText_Blended_Wrapped( font, diff --git a/tests/text.c b/tests/text.c index 6095b96..e6c6852 100644 --- a/tests/text.c +++ b/tests/text.c @@ -2,9 +2,11 @@ * @file text.c * @brief Unit tests for font loading and text measurement. * - * Measurement needs a font but no renderer, so this suite runs without a window - * and without the offscreen harness the drawing half of src/text.c is waiting - * on. akgl_text_rendertextat() is therefore not covered here. + * Measurement needs a font but no renderer at all. Drawing needs one, but not a + * display: a software renderer over a window under the dummy video driver takes + * akgl_text_rendertextat() all the way from rasterizing to blitting, so this + * suite covers it without waiting for the offscreen harness. What is still + * missing is any assertion about the *pixels* -- that is the harness's job. * * The fixture font is monospaced on purpose: the width of an N-character string * is exactly N times the width of one character, so every assertion below is a @@ -18,7 +20,9 @@ #include #include +#include #include +#include #include #include "testutil.h" @@ -27,9 +31,13 @@ #define TEST_FONT_PATH "assets/akgl_test_mono.ttf" /** @brief Point size every test in this file opens the fixture font at. */ #define TEST_FONT_SIZE 16 +/** @brief Width and height of the offscreen target the drawing tests draw into. */ +#define TEST_TARGET_SIZE 128 /** @brief The fixture font, opened once by main() and shared by every test. */ static TTF_Font *testfont = NULL; +/** @brief A 2D backend bound to a software renderer, built by main(). */ +static akgl_RenderBackend testbackend; akerr_ErrorContext *test_text_loadfont(void) { @@ -214,18 +222,94 @@ akerr_ErrorContext *test_text_measure_wrapped(void) SUCCEED_RETURN(errctx); } +akerr_ErrorContext *test_text_render_backend_guards(void) +{ + PREPARE_ERROR(errctx); + akgl_RenderBackend backend; + SDL_Color white = { 0xff, 0xff, 0xff, 0xff }; + + ATTEMPT { + // Nothing initialized the renderer at all, which is where the global + // starts and where a host that has not called akgl_game_init leaves it. + renderer = NULL; + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, + akgl_text_rendertextat(testfont, "hello", white, 0, 0, 0), + "drawing text with no renderer backend"); + + // A backend that exists but was never given an SDL_Renderer -- the + // state a host is in between allocating one and initializing it. + memset(&backend, 0x00, sizeof(akgl_RenderBackend)); + renderer = &backend; + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, + akgl_text_rendertextat(testfont, "hello", white, 0, 0, 0), + "drawing text through a backend with no SDL renderer"); + + // A live SDL_Renderer but no methods: a host that created its own + // renderer and has not called akgl_render_bind2d() yet. This is the one + // that used to be a segfault rather than an error -- with a real + // renderer behind it the call gets all the way to a NULL function + // pointer, which is why the check has to be here and not left to SDL. + backend.sdl_renderer = testbackend.sdl_renderer; + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, + akgl_text_rendertextat(testfont, "hello", white, 0, 0, 0), + "drawing text through a backend that was never bound"); + + renderer = &testbackend; + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, + akgl_text_rendertextat(NULL, "hello", white, 0, 0, 0), + "drawing text with a NULL font"); + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, + akgl_text_rendertextat(testfont, NULL, white, 0, 0, 0), + "drawing a NULL string"); + } CLEANUP { + renderer = &testbackend; + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + +akerr_ErrorContext *test_text_rendertextat(void) +{ + PREPARE_ERROR(errctx); + SDL_Color white = { 0xff, 0xff, 0xff, 0xff }; + + ATTEMPT { + // A bound backend over a software renderer is enough to draw through: + // no display, no window shown, and the whole path from rasterizing to + // blitting runs. + renderer = &testbackend; + TEST_EXPECT_OK(errctx, renderer->frame_start(renderer), "starting a frame"); + TEST_EXPECT_OK(errctx, akgl_text_rendertextat(testfont, "hello", white, 0, 4, 4), + "drawing a line of text"); + // Wrapping takes the other rasterizing path. + TEST_EXPECT_OK(errctx, + akgl_text_rendertextat(testfont, "one two three", white, TEST_TARGET_SIZE / 2, 0, 0), + "drawing wrapped text"); + // Not asserted here: the empty string, which SDL_ttf refuses with "Text + // has zero width" on both paths, so drawing an empty line reports a + // failure rather than drawing nothing. That is filed rather than pinned + // -- see TODO.md, "Known and still open". + TEST_EXPECT_OK(errctx, renderer->frame_end(renderer), "ending the frame"); + } CLEANUP { + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + int main(void) { PREPARE_ERROR(errctx); SDL_SetHint(SDL_HINT_VIDEO_DRIVER, "dummy"); SDL_SetHint(SDL_HINT_AUDIO_DRIVER, "dummy"); + SDL_SetHint(SDL_HINT_RENDER_DRIVER, "software"); ATTEMPT { CATCH(errctx, akgl_error_init()); + memset(&testbackend, 0x00, sizeof(akgl_RenderBackend)); FAIL_ZERO_BREAK( errctx, - SDL_Init(0), + SDL_Init(SDL_INIT_VIDEO), AKGL_ERR_SDL, "Couldn't initialize SDL: %s", SDL_GetError()); @@ -245,9 +329,28 @@ int main(void) TEST_FONT_PATH, SDL_GetError()); + // The host owns the window and libakgl binds to it, which is the + // arrangement akgl_render_bind2d exists for and the one an embedded + // interpreter drawing text is in. + FAIL_ZERO_BREAK( + errctx, + SDL_CreateWindowAndRenderer( + "net/aklabs/libakgl/test_text", + TEST_TARGET_SIZE, + TEST_TARGET_SIZE, + 0, + &window, + &testbackend.sdl_renderer), + AKGL_ERR_SDL, + "Couldn't create window/renderer: %s", + SDL_GetError()); + CATCH(errctx, akgl_render_bind2d(&testbackend)); + CATCH(errctx, test_text_loadfont()); CATCH(errctx, test_text_measure()); CATCH(errctx, test_text_measure_wrapped()); + CATCH(errctx, test_text_render_backend_guards()); + CATCH(errctx, test_text_rendertextat()); } CLEANUP { if ( testfont != NULL ) { TTF_CloseFont(testfont);