Own every pixel each frame, and bring the cursor back as a blinking block

The blinking backspace was not the backspace. The sink skipped rows it
believed were already clear, tracked in a drawn[] array, which assumes a frame
inherits the frame before it. SDL_RenderPresent swaps buffers, so a frame
inherits the one two back: a row erased once is clean in one buffer and dirty
in the other, and presenting alternates between them. The first character of a
backspaced wrap flickered forever.

It reproduces on X11 and never under the dummy driver or a software renderer,
which is why the suite stayed green through two rounds of fixing it. The sink
now repaints every row it owns every frame -- a frame either owns every pixel
it presents or inherits pixels it cannot reason about.

Confirmed on real hardware with the reported input: the wrapped tail's row
reads zero across five successive captures.

The cursor returns as a blinking block, half a second per cycle, off SDL's
clock. It sits in the cell after the text rather than under it, which is what
made the underscore unreadable, and it wraps to the next row when a line
exactly fills one -- where the next character actually lands.

The new regression test paints stale pixels by hand, which is what a swapped-in
buffer hands back, so the case is covered without real hardware. Every new
assertion was checked by reverting the fix and watching it fail.

Recorded honestly in TODO.md section 5: the same buffer swap means the
graphics verbs were never reliable across frames either. A one-shot DRAW lands
in one buffer and the next present shows the other. Making that work needs a
persistent surface, which is its own commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

TODO.md

@@ -844,38 +844,53 @@ deviations from the reference's *program*: `main.go` and the SDL half of
     exactly, and the whole golden corpus is driven through this loop to prove it. What it
     changes for a *user* is that the window closes when asked.
 
-26. **The frame is not cleared, but the text layer erases its own rows.** The reference blits a
+26. **The text layer repaints every row it owns, every frame.** Not just the rows that changed.
-    text surface onto the window surface and calls `UpdateSurface`. Here the graphics verbs draw
-    straight to the renderer rather than into a display list, so a `SDL_RenderClear` at the top
-    of each frame would erase every `DRAW` at the end of the frame it was issued in. The host
-    therefore does not clear.
 
-    **The sink erases row by row instead**, filling each row it is about to draw — and each row
+    That was tried — a `drawn[]` array marking which rows had carried glyphs, so an untouched
-    that has emptied since it last drew — with its background colour first. That makes the text
+    row could be skipped — and it is **wrong on real hardware**. `SDL_RenderPresent` swaps
-    layer authoritative over the rows it occupies and leaves every other pixel alone, which is
+    buffers, so a frame does not inherit the frame before it; it inherits the one *two* back,
-    how a C128's text plane sits over its bitmap.
+    and on some backends something undefined. A row erased once is clean in one buffer and still
+    dirty in the other, and presenting alternates between them.
 
-    **This was originally "the text layer is drawn over whatever is already there", and that was
+    That is what a backspace across a line wrap looked like: the first character of the wrapped
-    a bug rather than a deviation.** The grid was always right and the screen kept the previous
+    tail flickering in and out forever after the rest had gone. It reproduces on X11 and never
-    frame, which surfaced as three separate-looking faults reported together: a backspaced
+    under the dummy driver or a software renderer, which is exactly why the suite was green
-    character that stayed on screen, a scroll that appeared to overwrite the display with
+    through two rounds of fixing it. `tests/akgl_frontend.c` now paints stale pixels by hand —
-    garbage, and the editing cursor smearing an underscore along every column it had passed
+    which is what a swapped-in buffer hands back — so the case is covered without needing real
-    through. One cause, three symptoms. `tests/akgl_frontend.c` asserts the fix at pixel level —
+    hardware.
-    grid assertions could never have caught it, because the grid was never wrong.
 
-    **What it still costs:** a program that draws a picture and then prints loses the strip of
+    There is no cheaper correct answer available: a frame either owns every pixel it presents or
-    picture behind each text row it uses. That is the deliberate half of the trade — the
+    it inherits pixels it cannot reason about.
-    alternative was clearing the whole frame, which loses the picture entirely.
 
-27. **There is no cursor.** The reference draws one as a literal underscore glyph (`drawCursor`,
+    **What this costs, and it is more than it used to.** The host still does not call
-    `basicruntime_graphics.go:33`) and this did too. Removed on request: it sat under the text
+    `SDL_RenderClear`, but the text area is now repainted in full every frame, so anything a
-    being typed and made it hard to read.
+    graphics verb drew underneath it is erased every frame rather than only where text lands. In
+    the standalone driver the text area is the whole window, so `DRAW` and `PRINT` no longer
+    coexist there.
 
-    Worth separating the two reasons it was unbearable, because only one of them is gone. It
+    **And the same buffer swap means the graphics verbs were never reliable across frames
-    *smeared* — an underscore left at every column the cursor passed through — and that was
+    anyway**, which is worth stating plainly rather than leaving as a surprise: a one-shot `DRAW`
-    deviation 26's missing erase, now fixed. It also simply sits under the text, which is what
+    lands in one buffer and the next present shows the other. It looked fine in a single
-    an underscore cursor does in a font whose glyphs reach the baseline. A cursor could come back
+    screenshot and would have flickered exactly as the text did. Making graphics survive needs a
-    and behave now; a block or a different colour would read better than an underscore.
+    persistent surface the frame is composited from, which is real work and its own commit —
+    filed here rather than half-done.
+
+27. **The cursor is a blinking block.** The reference draws an underscore glyph (`drawCursor`,
+    `basicruntime_graphics.go:33`), and so did this until the underscore turned out to sit
+    *under* the text being typed and make it hard to read. A block occupies the cell after the
+    text instead of the space beneath it.
+
+    Half a second per blink (`AKBASIC_SINK_CURSOR_BLINK_MS`), which is about a Commodore's and
+    slow enough to read under. `akbasic_AkglSink.cursorperiodms` set to zero holds it solid,
+    which is what makes a frame deterministic for a test.
+
+    Two details that are decisions rather than accidents. The clock is **SDL's**, not the
+    host's: everywhere else the host owns the clock because the library owns no loop and must
+    not block, but this is an adaptor that already links SDL and threading a timestamp through
+    the sink interface to animate a cursor would be a poor trade. And a line that exactly fills
+    a row leaves the cursor one column past the end, because `putchar_at` wraps only when the
+    next character arrives — the cursor is drawn at the start of the next row instead, which is
+    where the next character will actually land and where a C128 puts it.
 
 28. **The window is closed by the host, and that is not `QUIT`.** Closing the window stops the
     frontend and leaves `obj->mode` alone; it does not set `AKBASIC_MODE_QUIT`. The distinction

include/akbasic/akgl.h

@@ -52,6 +52,14 @@
 #include <akbasic/input.h>
 #include <akbasic/sink.h>
 
+/**
+ * @brief One full cursor blink in milliseconds, half on and half off.
+ *
+ * Half a second, which is about what a Commodore does and is slow enough to read
+ * under.
+ */
+#define AKBASIC_SINK_CURSOR_BLINK_MS	500
+
 /** @brief How many saved SSHAPE regions the graphics backend will hold at once. */
 #define AKBASIC_AKGL_MAX_SHAPES		16
 
@@ -113,12 +121,10 @@ typedef struct
     char text[64][256];
 
     /**
-     * Which rows carried glyphs when this sink last rendered. A row that has
+     * Milliseconds for one full cursor blink, half on and half off. Zero holds
-     * emptied since -- scrolled away, cleared, or shortened -- still has to be
+     * the cursor solid, which is what a test wanting a deterministic frame sets.
-     * erased, and after it is erased there is no longer anything in the grid to
-     * say it once had something in it.
      */
-    bool drawn[64];
+    uint64_t cursorperiodms;
 
     /*
      * The line editor. Nothing here is live except while readline is running,

src/sink_akgl.c

@@ -417,6 +417,7 @@ akerr_ErrorContext *akbasic_sink_init_akgl(akbasic_TextSink *obj, akbasic_AkglSi
     state->color.b = 0xff;
     state->color.a = 0xff;
     /* Opaque, not transparent: the text layer owns the rows it draws. */
+    state->cursorperiodms = AKBASIC_SINK_CURSOR_BLINK_MS;
     state->background.r = 0x00;
     state->background.g = 0x00;
     state->background.b = 0x00;
@@ -457,23 +458,26 @@ akerr_ErrorContext *akbasic_sink_akgl_render(akbasic_TextSink *obj)
 		     "akgl sink has no state");
 
     /*
-     * **Erase each row before drawing it.** Without this the sink paints glyphs
+     * **Repaint every row of the text area, every frame.** Not just the rows
-     * on top of whatever the previous frame left, and the host deliberately does
+     * that changed -- that was tried, with a `drawn[]` array marking which rows
-     * not clear the frame either -- §5 deviation 26, so that a DRAW survives
+     * had carried glyphs, and it is wrong on real hardware.
-     * past the frame it was issued in. The grid was always right and the screen
-     * kept the last frame, which showed up as three separate-looking bugs: a
-     * backspaced character that would not go away, a scroll that appeared to
-     * overwrite the screen with garbage, and the editing cursor smearing an
-     * underscore along every column it passed through.
      *
-     * Row by row rather than one clear over the whole text area, and that is the
+     * SDL_RenderPresent swaps buffers. What a frame inherits is not the frame
-     * deliberate part: the text layer is authoritative over the rows it occupies
+     * before it, it is the frame *two* before it, and on some backends it is
-     * and leaves every other pixel alone. A program that draws a picture and
+     * undefined. So a row erased once is erased in one buffer and still dirty in
-     * prints one line loses one line's strip of it, the way a C128's text plane
+     * the other, and presenting alternates between them: the row blinks in and
-     * sits over its bitmap -- rather than losing the picture.
+     * out forever. That is exactly what a backspace across a line wrap looked
+     * like -- the first character of the wrapped tail flickering after the rest
+     * had gone. It reproduces on X11 and never under the dummy driver or a
+     * software renderer, which is why the suite was green.
      *
-     * `drawn` is what makes a row that has *emptied* get erased. Once it is
+     * There is no cheaper correct answer available here. A frame either owns
-     * cleared there is nothing left in the grid to say it ever held anything.
+     * every pixel it presents or it inherits pixels it cannot reason about.
+     *
+     * Row by row rather than one fill over the whole area, still: the two are
+     * equivalent in cost here and the row loop keeps the text layer's ownership
+     * of exactly `rows * cellh` pixels obvious, which matters when the area is
+     * smaller than the window.
      *
      * A loop, so no CATCH and no _BREAK macros: they expand to a C break, which
      * would escape the loop with an error still pending. PASS only.
@@ -483,20 +487,15 @@ akerr_ErrorContext *akbasic_sink_akgl_render(akbasic_TextSink *obj)
      * put characters somewhere the cursor does not think they are.
      */
     for ( row = 0; row < state->rows; row++ ) {
-	bool hastext = (state->text[row][0] != '\0');
 	SDL_FRect cells;
 
-	if ( !hastext && !state->drawn[row] ) {
-	    continue;
-	}
 	cells.x = (float)state->x;
 	cells.y = (float)(state->y + (row * state->cellh));
 	cells.w = (float)state->width;
 	cells.h = (float)state->cellh;
 	PASS(errctx, akgl_draw_filled_rect(state->renderer, &cells, state->background));
 
-	state->drawn[row] = hastext;
+	if ( state->text[row][0] == '\0' ) {
-	if ( !hastext ) {
 	    continue;
 	}
 	PASS(errctx, akgl_text_rendertextat(state->font, state->text[row],
@@ -506,13 +505,52 @@ akerr_ErrorContext *akbasic_sink_akgl_render(akbasic_TextSink *obj)
     }
 
     /*
-     * No cursor glyph. The reference draws one as a literal underscore
+     * The cursor: a filled block at the cursor cell, blinking.
-     * (drawCursor, basicruntime_graphics.go:33) and this did too, until it turned
+     *
-     * out to sit *under* the text being typed and make it hard to read. Removed
+     * A block rather than the reference's underscore glyph (drawCursor,
-     * on request; §5 records it, and the smearing that made it unbearable is
+     * basicruntime_graphics.go:33). The underscore sat *under* the text being
-     * fixed by the erase above rather than by the removal -- so a cursor could
+     * typed and made it hard to read, which is what got it removed; a block
-     * come back and behave, if one is wanted.
+     * occupies the cell after the text instead of the space beneath it.
+     *
+     * The clock is SDL's rather than the host's. Everywhere else in this
+     * interpreter the host owns the clock, because the library owns no loop and
+     * must not block -- but this is an adaptor that already links SDL, the
+     * blink is cosmetic, and threading a timestamp through the sink interface to
+     * animate a cursor would be a poor trade. `cursorperiodms` of zero holds it
+     * solid, which is what makes a frame deterministic for a test.
      */
+    if ( state->editing ) {
+	SDL_FRect cell;
+	bool visible = true;
+
+	if ( state->cursorperiodms > 0 ) {
+	    visible = ((SDL_GetTicks() % state->cursorperiodms) <
+		       (state->cursorperiodms / 2));
+	}
+	if ( visible ) {
+	    int curcol = state->cursorcol;
+	    int currow = state->cursorrow;
+
+	    /*
+	     * A line that exactly fills a row leaves the cursor one past the last
+	     * column, because putchar_at only wraps when the *next* character
+	     * arrives. Drawn there it would be off the right edge and invisible.
+	     * Show it where the next character will actually land instead, which
+	     * is also where a C128 puts it.
+	     */
+	    if ( curcol >= state->columns ) {
+		curcol = 0;
+		currow += 1;
+	    }
+	    if ( currow < state->rows ) {
+		cell.x = (float)(state->x + (curcol * state->cellw));
+		cell.y = (float)(state->y + (currow * state->cellh));
+		cell.w = (float)state->cellw;
+		cell.h = (float)state->cellh;
+		PASS(errctx, akgl_draw_filled_rect(state->renderer, &cell, state->color));
+	    }
+	}
+    }
     SUCCEED_RETURN(errctx);
 }
 

tests/akgl_frontend.c

@@ -29,6 +29,7 @@
 #include <akerror.h>
 
 #include <akgl/controller.h>
+#include <akgl/draw.h>
 #include <akgl/error.h>
 
 #include <akbasic/error.h>
@@ -666,7 +667,7 @@ static akerr_ErrorContext AKERR_NOIGNORE *test_backspace_across_a_wrap(void)
  * cursor had passed through, which read as an underline beneath the whole line
  * and made typed text hard to read. Removed outright; §5 records it.
  */
-static akerr_ErrorContext AKERR_NOIGNORE *test_no_cursor_underline(void)
+static akerr_ErrorContext AKERR_NOIGNORE *test_block_cursor_blinks(void)
 {
     PREPARE_ERROR(errctx);
     SDL_Surface *shot = NULL;
@@ -678,32 +679,119 @@ static akerr_ErrorContext AKERR_NOIGNORE *test_no_cursor_underline(void)
     PASS(errctx, start_frontend(NULL));
     sink = &FRONTEND.akglstate;
 
-    /*
-     * Rendered *while editing*, which is the only state the cursor was ever
-     * drawn in -- an earlier version of this test rendered after readline had
-     * returned, by which time `editing` is false and a re-added cursor would
-     * sail straight through it.
-     */
     PASS(errctx, FRONTEND.akglsink.write(&FRONTEND.akglsink, "ab"));
     sink->editing = true;
-    PASS(errctx, akbasic_sink_akgl_render(&FRONTEND.akglsink));
 
-    /*
+    /* Period zero holds it solid, so a frame is deterministic. */
-     * The bottom scanline of each cell the text occupies and the one after it.
+    sink->cursorperiodms = 0;
-     * An underscore lands there; "a" and "b" in this font do not, so anything
+    PASS(errctx, akbasic_sink_akgl_render(&FRONTEND.akglsink));
-     * lit on that line is the cursor and nothing else.
-     */
     shot = SDL_RenderReadPixels(FRONTEND.renderer->sdl_renderer, NULL);
     TEST_REQUIRE(shot != NULL, "could not read the render target back");
+
+    /*
+     * A *block*: every scanline of the cursor cell is lit, including the top
+     * one. An underscore lights only the bottom, and it was the underscore
+     * sitting under the text that made typed text hard to read.
+     */
+    TEST_REQUIRE(anything_drawn(shot, sink->x + (2 * sink->cellw), sink->y, sink->cellw, 1),
+		 "the cursor cell's top scanline should be lit: a block, not an underscore");
+    TEST_REQUIRE(anything_drawn(shot, sink->x + (2 * sink->cellw),
+				sink->y + sink->cellh - 1, sink->cellw, 1),
+		 "the cursor cell's bottom scanline should be lit too");
+
+    /* And nothing under the text itself, which is what was reported. */
     y = sink->y + sink->cellh - 1;
-    for ( col = 0; col < 4; col++ ) {
+    for ( col = 0; col < 2; col++ ) {
 	if ( anything_drawn(shot, sink->x + (col * sink->cellw), y, sink->cellw, 1) ) {
 	    lit += 1;
 	}
     }
     TEST_REQUIRE_INT(lit, 0);
     SDL_DestroySurface(shot);
+
+    /*
+     * It blinks. A short period and a wait past half of it: the same cell must
+     * come out differently, which is the whole of what blinking is.
+     */
+    sink->cursorperiodms = 200;
+    while ( (SDL_GetTicks() % 200) >= 100 ) {
+	SDL_Delay(5);
+    }
+    PASS(errctx, akbasic_sink_akgl_render(&FRONTEND.akglsink));
+    shot = SDL_RenderReadPixels(FRONTEND.renderer->sdl_renderer, NULL);
+    TEST_REQUIRE(anything_drawn(shot, sink->x + (2 * sink->cellw), sink->y,
+				sink->cellw, sink->cellh),
+		 "the cursor should be visible in the first half of its period");
+    SDL_DestroySurface(shot);
+
+    while ( (SDL_GetTicks() % 200) < 100 ) {
+	SDL_Delay(5);
+    }
+    PASS(errctx, akbasic_sink_akgl_render(&FRONTEND.akglsink));
+    shot = SDL_RenderReadPixels(FRONTEND.renderer->sdl_renderer, NULL);
+    TEST_REQUIRE(!anything_drawn(shot, sink->x + (2 * sink->cellw), sink->y,
+				 sink->cellw, sink->cellh),
+		 "the cursor should be hidden in the second half of its period");
+    SDL_DestroySurface(shot);
+
     sink->editing = false;
+    sink->cursorperiodms = 0;
+    stop_frontend();
+    SUCCEED_RETURN(errctx);
+}
+
+/**
+ * @brief A frame owns every pixel it presents, including ones it did not write.
+ *
+ * **This is the regression test for the blinking backspace, and it is the only
+ * one that could have caught it without real hardware.** The sink used to skip
+ * rows it believed were already clear, tracked in a `drawn[]` array. That
+ * assumes a frame inherits the frame before it. SDL_RenderPresent swaps
+ * buffers, so a frame inherits the one *two* back -- a row erased once is clean
+ * in one buffer and dirty in the other, and presenting alternates between them.
+ * The first character of a backspaced wrap flickered forever.
+ *
+ * It reproduced on X11 and never under the dummy driver, because a software
+ * renderer with a single target does preserve. Rather than require real
+ * hardware, this paints the stale pixels by hand -- which is exactly what the
+ * other buffer hands back -- and asserts the sink erases them anyway.
+ */
+static akerr_ErrorContext AKERR_NOIGNORE *test_frame_owns_every_pixel(void)
+{
+    PREPARE_ERROR(errctx);
+    SDL_Surface *shot = NULL;
+    akbasic_AkglSink *sink = NULL;
+    SDL_FRect stale;
+
+    PASS(errctx, start_frontend(NULL));
+    sink = &FRONTEND.akglstate;
+
+    /* One line of text, and the grid otherwise empty. */
+    PASS(errctx, FRONTEND.akglsink.writeln(&FRONTEND.akglsink, "KEPT"));
+    PASS(errctx, akbasic_sink_akgl_render(&FRONTEND.akglsink));
+
+    /*
+     * Now dirty row 1 behind the sink's back, the way a swapped-in buffer
+     * arrives already dirty. The sink has never drawn there and its grid says
+     * the row is empty, so nothing in its own bookkeeping suggests any work.
+     */
+    stale.x = (float)sink->x;
+    stale.y = (float)(sink->y + sink->cellh);
+    stale.w = (float)sink->cellw;
+    stale.h = (float)sink->cellh;
+    PASS(errctx, akgl_draw_filled_rect(FRONTEND.renderer, &stale, sink->color));
+
+    shot = SDL_RenderReadPixels(FRONTEND.renderer->sdl_renderer, NULL);
+    TEST_REQUIRE(!cell_is_blank(shot, sink, 0, 1), "the stale pixels should be there to start");
+    SDL_DestroySurface(shot);
+
+    PASS(errctx, akbasic_sink_akgl_render(&FRONTEND.akglsink));
+    shot = SDL_RenderReadPixels(FRONTEND.renderer->sdl_renderer, NULL);
+    TEST_REQUIRE(shot != NULL, "could not read the render target back");
+    TEST_REQUIRE(cell_is_blank(shot, sink, 0, 1),
+		 "a row the grid says is empty must be erased even if this sink never drew it");
+    TEST_REQUIRE(!cell_is_blank(shot, sink, 0, 0), "and the row that has text is still drawn");
+    SDL_DestroySurface(shot);
 
     stop_frontend();
     SUCCEED_RETURN(errctx);
@@ -789,7 +877,8 @@ int main(void)
 	CATCH(errctx, test_render_erases_what_it_drew());
 	CATCH(errctx, test_scroll_reaches_the_screen());
 	CATCH(errctx, test_backspace_across_a_wrap());
-	CATCH(errctx, test_no_cursor_underline());
+	CATCH(errctx, test_block_cursor_blinks());
+	CATCH(errctx, test_frame_owns_every_pixel());
 	CATCH(errctx, test_repl_session());
     } CLEANUP {
 	stop_frontend();