From fca9ad4a89df2910ab68fd4d8972deb36a175f5d Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Fri, 31 Jul 2026 13:25:14 -0400 Subject: [PATCH] Consume libakgl 0.3.0: every workaround deleted, two capabilities gained 0.3.0 closed all ten API gaps this port had filed. The four workarounds go with them: the CMake block that declared libakgl's vendored dependencies by hand, the akgl/actor.h include in three files, and the six vtable pointers assigned by hand in two more, now akgl_render_bind2d(). Two gaps were capabilities rather than inconveniences, and both are now real: The line editor takes the composed UTF-8 text the ring carries in preference to the keycode, so shifted characters, keyboard layouts, compose keys and dead keys all work. A double quote can be typed, which means a BASIC string literal can be typed -- the sharp end of the old limitation. Letters are no longer folded to upper case. SOUND's dir/min/step reach akgl_audio_sweep instead of being refused. dir 3 sweeps once rather than oscillating and TODO.md section 5 says so. A backend with no sweep still refuses the swept note and plays the held one. The adaptors now carry an AKGL_VERSION_AT_LEAST(0, 3, 0) floor, verified by temporarily demanding 0.4.0 and watching it fire. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- CMakeLists.txt | 35 ++++---- README.md | 2 +- TODO.md | 191 +++++++++++++++++++--------------------- deps/libakgl | 2 +- include/akbasic/akgl.h | 37 ++++++-- include/akbasic/audio.h | 10 +++ src/audio_akgl.c | 21 +++++ src/frontend_akgl.c | 39 ++------ src/input_akgl.c | 9 -- src/runtime_audio.c | 81 +++++++++++++++-- src/sink_akgl.c | 102 ++++++++++++++------- tests/akgl_backends.c | 25 ++---- tests/akgl_frontend.c | 132 +++++++++++++++++++++------ tests/audio_verbs.c | 52 +++++++++-- tests/mockdevice.h | 20 +++++ 16 files changed, 495 insertions(+), 265 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 980dfb1..4026fce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -88,7 +88,7 @@ that repo is next touched. See the error-code section below for a live collision |---|---|---|---|---| | `deps/libakerror` | 1.0.0 | `libakerror.so.1` | major only | **none** — no version macro, no `ConfigVersion` file | | `deps/libakstdlib` | 0.2.0 | `libakstdlib.so.0.2` | **`MAJOR.MINOR` while major is 0** | `AKSL_VERSION_*`, `aksl_version()`, `AKSL_VERSION_CHECK()` | -| `deps/libakgl` | 0.2.0 | `libakgl.so.0.2` | **`MAJOR.MINOR` while major is 0** | `AKGL_VERSION*`, `akgl_version()`, `AKGL_VERSION_AT_LEAST()` | +| `deps/libakgl` | 0.3.0 | `libakgl.so.0.3` | **`MAJOR.MINOR` while major is 0** | `AKGL_VERSION*`, `akgl_version()`, `AKGL_VERSION_AT_LEAST()` | For both 0.x libraries the soname carries `MAJOR.MINOR` deliberately: 0.1 and 0.2 are *different* ABIs, and both become major-only at 1.0. Do not read `0.1 → 0.2` as a compatible bump — and both diff --git a/CMakeLists.txt b/CMakeLists.txt index c070082..541b308 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,6 +46,14 @@ option(AKBASIC_SANITIZE "Build with ASan + UBSan" OFF # Rename the dependency's on the way past. Remove this once libakerror applies # the same CMAKE_SOURCE_DIR test to `coverage` that it already applies to # `mutation` -- filed in deps/libakstdlib/TODO.md section 2.3. +# +# **Only one project in a tree may shadow add_test(), and this is that project.** +# CMake exposes an overridden command as `_name` and chains exactly one level: a +# second override rebinds `_add_test` to the first override and the builtin +# becomes unreachable to everyone, so our own registrations recurse until CMake +# stops at "Maximum recursion depth of 1000 exceeded". libakgl 0.3.0 briefly +# shadowed it unconditionally and this is exactly what happened; the top-level +# guard it used to have is back, and libakstdlib has always had one. # --------------------------------------------------------------------------- set(AKBASIC_SUPPRESS_ADD_TEST TRUE) @@ -72,26 +80,15 @@ endfunction() add_subdirectory(deps/libakerror EXCLUDE_FROM_ALL) add_subdirectory(deps/libakstdlib EXCLUDE_FROM_ALL) if(AKBASIC_WITH_AKGL) - # libakgl builds its own vendored SDL, SDL_image, SDL_mixer, SDL_ttf and - # jansson only when it is *top-level*. Embedded, it goes down a find_package - # path instead and requires all five installed on the system -- which is a - # surprise, because they are sitting right there in deps/libakgl/deps as - # submodules. Every one of those lookups is guarded with - # if(NOT TARGET ...), so declaring the targets here first satisfies them - # and the vendored copies get used after all. Same trick, and the same - # ordering requirement, that akerror::akerror and akstdlib::akstdlib already - # need above. + # libakgl 0.3.0 adds its own vendored SDL, SDL_image, SDL_mixer, SDL_ttf and + # jansson whether or not it is top-level, so there is nothing to do here but + # add it. Before that it went down a find_package path when embedded and + # required all five installed on the system, and this block declared them by + # hand first -- filed as libakgl API-gap item 5 and resolved there. # - # Filed upstream: an embedded libakgl should add its own vendored - # dependencies rather than requiring them installed. - set(JANSSON_WITHOUT_TESTS ON CACHE BOOL "Do not build vendored Jansson tests" FORCE) - set(JANSSON_EXAMPLES OFF CACHE BOOL "Do not build vendored Jansson examples" FORCE) - set(JANSSON_BUILD_DOCS OFF CACHE BOOL "Do not build vendored Jansson docs" FORCE) - add_subdirectory(deps/libakgl/deps/jansson EXCLUDE_FROM_ALL) - add_subdirectory(deps/libakgl/deps/SDL EXCLUDE_FROM_ALL) - add_subdirectory(deps/libakgl/deps/SDL_image EXCLUDE_FROM_ALL) - add_subdirectory(deps/libakgl/deps/SDL_mixer EXCLUDE_FROM_ALL) - add_subdirectory(deps/libakgl/deps/SDL_ttf EXCLUDE_FROM_ALL) + # akerror::akerror and akstdlib::akstdlib still have to exist first: libakgl + # guards every dependency with if(NOT TARGET ...), which is what stops its + # vendored copies of those two being declared a second time. add_subdirectory(deps/libakgl EXCLUDE_FROM_ALL) endif() diff --git a/README.md b/README.md index d18b73d..ddb4295 100644 --- a/README.md +++ b/README.md @@ -464,7 +464,7 @@ The exception is `FILTER`, which sets the SID's filter cutoff, band switches and * [libakerror](https://source.starfort.tech/andrew/libakerror) 1.0.0 — TRY/CATCH-style error contexts. Every function that can fail returns one. * [libakstdlib](https://source.starfort.tech/andrew/libakstdlib) 0.2.0 — libc wrappers that report through `libakerror`. -* [libakgl](https://source.starfort.tech/andrew/libakgl) 0.2.0 — **optional**, only for `-DAKBASIC_WITH_AKGL=ON`. Pulls in SDL3, and requires `libakstdlib` 0.2, which is why the two move together. +* [libakgl](https://source.starfort.tech/andrew/libakgl) 0.3.0 — **optional**, only for `-DAKBASIC_WITH_AKGL=ON`. Pulls in SDL3. 0.3.0 closed every API gap this project had filed against it, so there are no libakgl workarounds left here. * [basicinterpret](https://source.starfort.tech/andrew/basicinterpret) — the Go original. Vendored as the behavioural spec for questions about semantics, and for nothing else: its acceptance corpus and its Commodore font are checked in here now, so **the build and the test suite do not need it**. Not linked, not built, and safe to omit. Everything is a submodule; `git submodule update --init --recursive` gets all of it. There is nothing to install first. diff --git a/TODO.md b/TODO.md index 727c196..ad86cc9 100644 --- a/TODO.md +++ b/TODO.md @@ -535,50 +535,49 @@ path is proved only through SDL's own event queue in `tests/akgl_frontend.c` — same queue a real keyboard delivers into, but not the same as a window manager giving the window focus. Somebody should type at it once. -### Five things that had to be worked around to get there +### The five things that had to be worked around — **all gone** -All five are `libakgl`'s and all five are filed in `deps/libakgl/TODO.md`. Each workaround is -commented at its site with the words "filed upstream" so it can be found and deleted later. +Every one was filed against `libakgl` and every one is resolved in its 0.3.0. What is left here +is the note that they existed, because the pattern is the point: file it upstream, comment the +workaround at its site with the words "filed upstream", delete it when it lands. -1. **An embedded `libakgl` demands its dependencies be *installed*.** It builds its vendored - SDL, SDL_image, SDL_mixer, SDL_ttf and jansson only when it is top-level; embedded, it goes - down a `find_package` path and fails on a machine that has none of them — while the - submodules sit right there in `deps/libakgl/deps`. Every lookup is guarded with - `if(NOT TARGET ...)`, so our `CMakeLists.txt` adds those five subdirectories *before* - `add_subdirectory(deps/libakgl)` and the vendored copies get used after all. Same trick and - same ordering requirement `akerror::akerror` and `akstdlib::akstdlib` already need. +| Was | Resolved by | +|---|---| +| An embedded `libakgl` required its vendored SDL, SDL_image, SDL_mixer, SDL_ttf and jansson to be *installed*, so our `CMakeLists.txt` declared all five by hand first | It adds them whether or not it is top-level | +| `akgl/controller.h` did not compile on its own — it used `akgl_Actor *` and included nothing that declared it | It includes `` | +| There was no way to bind a 2D backend to a renderer you already had, so the six vtable pointers were assigned by hand in two files | `akgl_render_bind2d()` | +| `akgl_text_rendertextat()` dereferenced an unset `draw_texture`, so the first `PRINT` through an unbound backend segfaulted | It checks, and reports `AKERR_NULLPOINTER` like the draw entry points | +| `SOUND`'s frequency sweep had no equivalent and was refused | `akgl_audio_sweep()` | -2. **`akgl/controller.h` does not compile on its own.** It declares two handler function - pointers taking an `akgl_Actor *` and includes nothing that declares the type. - `src/input_akgl.c` includes `akgl/actor.h` ahead of it. - -3. **There is no way to attach a 2D backend to a renderer you already have.** - `akgl_render_init2d()` installs the six vtable pointers, but it also creates its own window - from the game properties and writes to the `camera` global, so it belongs to the - `akgl_game_init()` path — which is exactly the path an embedding host is not on. - `tests/akgl_backends.c` assigns the six pointers by hand. What is wanted upstream is the - vtable half of `init2d` on its own. - -4. **`akgl_text_rendertextat()` segfaults on a backend whose vtable is empty.** It reaches - through `renderer->draw_texture` without checking it, so the first `PRINT` through the sink - dereferences NULL. That is the same class of defect 42b60f7's own commit added a draw test - for — "a backend that exists but was never given an `SDL_Renderer`" — and the text path - still has it. +**0.3.0 also arrived with a regression that this repository is the one that found**, because it +is the only consumer that embeds `libakgl` alongside other projects: the commit that made the +vendored dependencies unconditional also made its `add_test()` shadow unconditional, and CMake +chains command overrides exactly one level deep. Two projects in one tree cannot both shadow +`add_test` — the second rebinds `_add_test` to the first and the builtin becomes unreachable to +everybody, so our own registrations recursed to CMake's depth limit. Fixed upstream by putting +the top-level guard back; filed as `libakgl` defect 27, with the two-line CMake program that +demonstrates the one-level limit. ### Still missing from the sink -Nothing that blocks a verb. The line editor exists (see above), so `INPUT` and the REPL work in -the window. Two limits are worth stating because a program can reach both: +Nothing that blocks a verb, and as of `libakgl` 0.3.0 nothing that blocks a *character* either. +The ring now carries the composed UTF-8 text SDL worked out from each keystroke, so the editor +takes that in preference to the keycode and shifted characters, keyboard layouts, compose keys +and dead keys all work. **A double quote can be typed, so a BASIC string literal can be typed**, +which was the sharp end of the old limitation — it used to mean a program with a string in it +had to be passed on the command line. Letters are no longer folded to upper case, because there +is no longer any reason to. + +One limit remains, and it is a choice rather than a gap: -- **No shifted character can be typed.** The keystroke ring carries a keycode and no modifier - state, so `"`, `!`, `(`, `)`, `:` and `;` are unreachable and a string literal cannot be typed - at the window at all. Letters are folded to upper case, which is a C128 doing what a C128 does - rather than a workaround, but punctuation has no such excuse. Filed upstream as `libakgl` - item 10; until it lands, the way to run a program with a string literal in it is to pass the - file on the command line, which is unaffected. - **No cursor movement within a line.** Backspace and escape only. The arrow keys stay in the ring for a script's own `GET` loop, which is the more valuable use of them. +A non-ASCII character is accepted by the keyboard and then dropped rather than stored, which is +§1.2 rather than the editor: the grid is a byte per cell and a value's string is a fixed 256 +bytes, so a multi-byte character has nowhere to go. Dropping it is honest where storing half of +it is not. + `WINDOW` and `KEY` in group E want a text-window rectangle and a function-key table on top of this, which is ordinary work here rather than anything blocked. @@ -757,13 +756,28 @@ behaviour to port. They matter to somebody typing in a listing out of a C128 man `step()` is called. Audible, deterministic, and never a hang, which is the right way for that to fail. -21. **`PLAY`'s `M` (measure) is a no-op and `SOUND`'s frequency sweep is refused.** `M` - synchronises the three voices on a C128; with one sequential queue there is nothing to - synchronise, and a listing full of them should still play, so it is accepted and ignored. - The sweep is the opposite call: `SOUND`'s `dir`/`min`/`step` arguments have no - `akgl_audio_*` equivalent, and the only way to fake one is to re-issue tones from - `step()` — which would tie audible pitch to how often the host happens to call us, a tune - that changes key with the frame rate. Filed upstream as `akgl_audio_sweep`; see §7. +21. **`PLAY`'s `M` (measure) is a no-op, and `SOUND`'s sweep runs once rather than + oscillating.** `M` synchronises the three voices on a C128; with one sequential queue there + is nothing to synchronise, and a listing full of them should still play, so it is accepted + and ignored. + + The sweep *used* to be refused outright — there was no `akgl_audio_*` equivalent and faking + one would have tied audible pitch to how often the host calls us. `libakgl` 0.3.0 added + `akgl_audio_sweep()` and `SOUND voice, freq, dur, dir, min, step` now reaches it. What does + not survive is `dir` 3, "oscillate": `akgl_audio_sweep` runs one pass between two endpoints, + and a real oscillation needs the mixer to turn around at them. **What this changes for a + program:** a `dir` 3 siren rises or falls once instead of warbling. `dir` 1 and 2 are exact, + and so is the direction — both the SID and `akgl_audio_sweep` take it from which endpoint is + higher, so a `min` above the starting frequency rises whatever `dir` says. + + `step` is converted as a *delta* rather than a position: it is a register increment, and the + register-to-hertz table maps positions, so it goes across as the distance between register 0 + and register `step`. A step that converts to zero would never arrive and is raised to one + hertz, which is the smallest move that still gets there. + + A backend whose record has no `sweep` — a host written against `libakgl` 0.2.0 — still + refuses a swept note with `AKBASIC_ERR_DEVICE` and plays a held one normally. + `tests/audio_verbs.c` asserts both halves. 22. **`TEMPO`'s constant is a calibration choice, not a transcription.** BASIC 7.0 documents `TEMPO` as a relative duration from 1 to 255 defaulting to 8, and does not publish what a @@ -1031,8 +1045,9 @@ When phase 7 or phase 8 needs something `libakgl` does not have, **stop and file the `akgl_*` entry point should look like, and what tests would cover it. Growing `libakgl` to serve the interpreter is a wanted outcome, not a detour. -**All four gaps filed so far are now closed upstream**, at `libakgl` 42b60f7. Nothing here is -blocked on `libakgl` any more; what remains is akbasic-side work. +**All ten gaps filed so far are closed upstream**, as of `libakgl` 0.3.0. Nothing here is +blocked on `libakgl`, and this repository carries no `libakgl` workarounds at all — the four +§3 used to list are deleted. | Was blocking | Closed by | |---|---| @@ -1040,70 +1055,42 @@ blocked on `libakgl` any more; what remains is akbasic-side work. | Immediate-mode drawing (§4 group G) | `akgl_draw_point`/`_line`/`_rect`/`_filled_rect`/`_circle`/`_flood_fill`/`_copy_region`/`_paste_region` | | Audio (§4 group I) | `akgl_audio_init`/`_tone`/`_envelope`/`_waveform`/`_volume`/`_stop`/`_voice_active`/`_mix` | | Console input (§4 group E) | `akgl_controller_poll_key`, `akgl_controller_flush_keys` | +| Vendored dependencies when embedded | added unconditionally, 0.3.0 | +| `controller.h` not self-contained | includes ``, 0.3.0 | +| Binding a 2D backend to an existing renderer | `akgl_render_bind2d()`, 0.3.0 | +| `akgl_text_rendertextat()` on an unbound backend | checked, 0.3.0 | +| `SOUND`'s frequency sweep (§5 item 21) | `akgl_audio_sweep()`, 0.3.0 | +| The line editor could not see Shift (§3) | `akgl_Keystroke` + `akgl_controller_poll_keystroke()`, 0.3.0 | -Two notes for whoever picks up §3 next. The draw calls take an `akgl_RenderBackend *` as their -first argument rather than reaching for a global renderer, which fits the rule that the -interpreter draws through whatever renderer the host already initialized. And the audio API is -a synthesised-voice one — `akgl_audio_tone(voice, hz, ms)` with a separate ADSR envelope — -which is the shape `PLAY` and `ENVELOPE` need, rather than the sample playback SDL3_mixer -would have given. +Three notes for whoever files the next one. The draw calls take an `akgl_RenderBackend *` as +their first argument rather than reaching for a global renderer, which fits the rule that the +interpreter draws through whatever renderer the host already initialized. The audio API is a +synthesised-voice one — `akgl_audio_tone(voice, hz, ms)` with a separate ADSR envelope — which +is the shape `PLAY` and `ENVELOPE` need, rather than the sample playback SDL3_mixer would have +given. And `akgl_controller_poll_key()` survives alongside the keystroke form on purpose: `GET` +and `GETKEY` still use it, because a script asking "was the up arrow pressed" wants a keycode +and would get the empty string from the other one. + +**`FILTER` is the one verb still refused**, and it is not filed as a gap because upstream said +plainly what it would take: `akgl_audio_*` synthesises raw waveforms and mixes them, there is no +filter stage to configure, and SDL3 supplies no primitive to build one from. Until an +`akgl_audio_filter()` exists, `FILTER` is accepted by the parser and refused at execution with +`AKBASIC_ERR_DEVICE` rather than silently ignored — a program that asks for a low-pass and gets +an unfiltered square wave has been lied to. + +**One thing went the other way**, and it is worth recording because it is the only defect this +project has ever sent upstream in a release it also consumed. 0.3.0's vendored-dependency fix +also made `libakgl`'s `add_test()` shadow unconditional. CMake chains command overrides exactly +one level deep, so two projects in one tree cannot both shadow it: the second rebinds +`_add_test` to the first and the builtin becomes unreachable to everyone, and this repository's +own test registrations recursed until CMake stopped at depth 1000. `akbasic` is the only +consumer that embeds `libakgl` next to other projects, so it is the only place this could show +up. Fixed upstream by restoring the top-level guard and filed as `libakgl` defect 27. Building against any of it needs `-DAKBASIC_WITH_AKGL=ON`, which pulls in `libakgl`'s own submodules (SDL and friends). Those are not initialized in a default clone; `git submodule update --init --recursive` gets them. -**Seven items are filed upstream and outstanding**, at `deps/libakgl/TODO.md` items 5-10 of "API -gaps blocking akbasic" and item 14 of "Defects". Four are workarounds this repository carries -today, each commented at its site with the words "filed upstream" so it can be found and -deleted: the embedded-dependency CMake trick in our `CMakeLists.txt`, the `akgl/actor.h` -include in `src/input_akgl.c` and `src/sink_akgl.c`, and the hand-populated vtable and its -NULL-deref hazard in `tests/akgl_backends.c` and `src/frontend_akgl.c`. §3 describes all four. -The fifth is `SOUND`'s frequency sweep, the sixth is the missing version bump recorded in §8, -and the seventh is new: - -- **The keystroke ring carries a keycode and no modifier state**, so the line editor cannot see - Shift. Every shifted character — `"`, `!`, `(`, `)`, `:`, `;` — is unreachable, which means a - string literal cannot be typed at the window at all, and lower case is unreachable too. - Letters are folded to upper case, which is what a C128 does and is therefore the right answer - for letters rather than merely the available one; punctuation has no such excuse. It is not - fixable from here: by the time a caller could ask `SDL_GetModState()` the key is long - released, and decoupling reads from events is the entire point of a ring. The wanted API is - an `akgl_controller_poll_keystroke()` returning keycode, modifiers and the composed UTF-8 - text SDL already computes from `SDL_EVENT_TEXT_INPUT` — which is also the only way a keyboard - layout, a compose key or a dead key can ever work. Filed as item 10, alongside the existing - `akgl_controller_poll_key()`, which stays exactly as it is: a game asking "was the up arrow - pressed" wants precisely what it already gets. - -**One further gap is outstanding, and upstream named it itself.** The commit that added the audio API -says plainly what it does not cover: *"Still missing for a complete BASIC sound vocabulary: -`FILTER` (SDL3 has no filter primitive; this would need writing), `TEMPO` and the `PLAY` -note-string parser, both of which belong in the interpreter rather than here."* - -- **`TEMPO` and the `PLAY` parser are ours**, not a gap. They are group I work in this - repository and nothing upstream is waiting on. -- **`FILTER` is a real gap.** BASIC 7.0's `FILTER freq, lp, bp, hp, res` sets the SID's - filter cutoff, the three band-pass switches and the resonance. `akgl_audio_*` synthesises - raw waveforms and mixes them; there is no filter stage to configure and SDL3 supplies no - primitive to build one from. Until an `akgl_audio_filter()` exists, `FILTER` is accepted by - the parser and refused at execution with `AKBASIC_ERR_DEVICE` rather than silently ignored — - a program that asks for a low-pass and gets an unfiltered square wave has been lied to. - -A second gap turned up while implementing group I, and is filed alongside it: - -- **`SOUND` has no frequency sweep.** BASIC 7.0's `SOUND voice, freq, dur, dir, min, step` - ramps the pitch from `freq` toward `min` in `step` increments per tick, in the direction - `dir` selects. `akgl_audio_tone(voice, hz, ms)` holds one pitch for one duration and there - is no entry point that changes pitch over the life of a note. The wanted API is roughly - `akgl_audio_sweep(int voice, float32_t from_hz, float32_t to_hz, float32_t step_hz, uint32_t ms)`, - advanced on the mixer's own frame counter — the same place the phase is already derived - from, and the reason it belongs there rather than here. Faking it in the interpreter means - re-issuing tones from `akbasic_runtime_step()`, which ties audible pitch to how often a host - calls us: a tune that changes key with the frame rate. Refused with `AKBASIC_ERR_DEVICE` - until it lands. Tests would drive `akgl_audio_mix` by hand and assert the frame at which the - pitch has moved, exactly as `tests/audio.c` already does for the envelope. - -When the next gap turns up, file it there rather than working around it here. - --- ## 8. Status @@ -1150,7 +1137,7 @@ Dependency baseline: |---|---|---| | `deps/libakerror` | 1.0.0 | Private ownership-enforced status registry. akbasic reserves 512–767 in `akbasic_error_register()`. Does not namespace its `coverage` target when embedded — worked around in our `CMakeLists.txt`. | | `deps/libakstdlib` | 0.2.0 | soname `libakstdlib.so.0.2`. `AKSL_VERSION_CHECK()` asserted in `tests/version_check.c`. This release fixed all six confirmed defects the port was working around — see §1.9, where the bans are now lifted. | -| `deps/libakgl` | 0.2.0 | soname `libakgl.so.0.2`. Owns status codes 256–260. Linked and tested under `-DAKBASIC_WITH_AKGL=ON`, which still defaults OFF so the core library and its whole suite build on a machine with no SDL. It requires `libakstdlib` 0.2, which is why the two moved together. | +| `deps/libakgl` | 0.3.0 | soname `libakgl.so.0.3`. Owns status codes 256–260. Linked and tested under `-DAKBASIC_WITH_AKGL=ON`, which still defaults OFF so the core library and its whole suite build on a machine with no SDL. **0.3.0 closed every API gap this port had filed** — see §7 — so the four workarounds §3 used to list are gone. | **The `libakgl` requirement used to be pinned by commit, and no longer is.** 42b60f7 added 22 public symbols across four headers and left `project(akgl VERSION 0.1.0)` and the diff --git a/deps/libakgl b/deps/libakgl index 54df954..58f9b70 160000 --- a/deps/libakgl +++ b/deps/libakgl @@ -1 +1 @@ -Subproject commit 54df954ed6c4f45b3a2b27bcb19dc01445589b3e +Subproject commit 58f9b70a49a2e2d1861248e877dc87be44bcc0d1 diff --git a/include/akbasic/akgl.h b/include/akbasic/akgl.h index b3526b0..7187340 100644 --- a/include/akbasic/akgl.h +++ b/include/akbasic/akgl.h @@ -30,6 +30,22 @@ #include #include +#include + +/* + * Everything in this target now depends on APIs that arrived in libakgl 0.3.0: + * akgl_render_bind2d(), akgl_audio_sweep(), and akgl_Keystroke with + * akgl_controller_poll_keystroke(). Refused here rather than at link time, + * because a missing symbol names a function and this names the release. + * + * The soname carries MAJOR.MINOR while the major is 0, so 0.2 and 0.3 are + * different ABIs and a mismatch is normally caught at load. This catches the + * case the soname cannot: a build against 0.2 headers that happens to find a + * 0.3 library, or the reverse. + */ +#if !AKGL_VERSION_AT_LEAST(0, 3, 0) +#error "akbasic's libakgl adaptors require libakgl 0.3.0 or later" +#endif #include #include @@ -164,16 +180,21 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_sink_akgl_render(akbasic_TextSink *ob * collects keystrokes from libakgl's ring -- the one the host's own event pump * fills -- echoes them into the grid, and returns the line on Return. * - * **What the editor cannot do, and why.** akgl_controller_poll_key() reports a - * keycode and nothing else: no modifier state and no composed text. So the - * editor cannot see Shift, which puts every shifted character out of reach and - * makes lowercase unreachable. Letters are folded to upper case, which is what a - * C128 does anyway and what the C64 font is drawn for. Filed upstream against - * libakgl; when the ring carries modifiers this becomes a real keyboard. + * **It is a real keyboard as of libakgl 0.3.0.** The ring carries the composed + * UTF-8 text SDL worked out from the keystroke, so shifted characters, a + * keyboard layout, a compose key and a dead key all do what the person typing + * expects -- including the double quote, without which a BASIC string literal + * cannot be typed at all. Until 0.3.0 the ring reported a keycode and nothing + * else, so none of that was reachable and letters were folded to upper case; + * that was libakgl API-gap item 10, filed from here. + * + * Composed text is taken over the keycode wherever there is any, which is what + * makes the layout work: on an AZERTY keyboard the key SDL calls `SDLK_Q` + * composes to "a", and "a" is what the program should see. * * Editing is Backspace to erase one character and Escape to abandon the line. - * There is no cursor movement within the line, for the same reason: the ring - * reports the arrow keys, but a script's own GET loop wants them. + * There is no cursor movement within the line: the ring reports the arrow keys, + * but a script's own GET loop wants them. * * @param obj The sink to give an editor to. * @param pump One frame of the host's loop, or NULL to take the editor away. diff --git a/include/akbasic/audio.h b/include/akbasic/audio.h index e8e46e2..9218244 100644 --- a/include/akbasic/audio.h +++ b/include/akbasic/audio.h @@ -67,6 +67,16 @@ typedef struct akbasic_AudioBackend /** Start a note on a voice, for a bounded duration. */ akerr_ErrorContext AKERR_NOIGNORE *(*tone)(struct akbasic_AudioBackend *self, int voice, double hz, int ms); + /** + * Start a note whose pitch moves, which is what SOUND's `dir`, `min` and + * `step` arguments ask for -- a siren, a laser, a falling bomb. + * + * May be NULL, and a host that leaves it so gets `SOUND` refused with + * AKBASIC_ERR_DEVICE for a swept note and served normally for a held one. + * That is deliberate: this arrived in libakgl 0.3.0, and a backend written + * against an older one is still a valid backend for everything else. + */ + akerr_ErrorContext AKERR_NOIGNORE *(*sweep)(struct akbasic_AudioBackend *self, int voice, double from_hz, double to_hz, double step_hz, int ms); /** Silence a voice immediately. */ akerr_ErrorContext AKERR_NOIGNORE *(*stop)(struct akbasic_AudioBackend *self, int voice); /** Select the waveform a voice synthesises. */ diff --git a/src/audio_akgl.c b/src/audio_akgl.c index 2e7eb9d..438353b 100644 --- a/src/audio_akgl.c +++ b/src/audio_akgl.c @@ -60,6 +60,26 @@ static akerr_ErrorContext *snd_tone(akbasic_AudioBackend *self, int voice, doubl SUCCEED_RETURN(errctx); } +/** + * @brief A note whose pitch moves, for SOUND's dir/min/step arguments. + * + * New in libakgl 0.3.0, which is why the record's entry may be NULL and why + * SOUND checks before using it. The sweep is advanced on the mixer's own frame + * counter -- doing it from akbasic_runtime_step() instead would tie audible + * pitch to how often a host happens to call us, which is the reason this was + * filed upstream rather than faked here. + */ +static akerr_ErrorContext *snd_sweep(akbasic_AudioBackend *self, int voice, double from_hz, double to_hz, double step_hz, int ms) +{ + PREPARE_ERROR(errctx); + + (void)self; + FAIL_ZERO_RETURN(errctx, (ms >= 0), AKERR_VALUE, "Negative note duration %d", ms); + PASS(errctx, akgl_audio_sweep(voice, (float32_t)from_hz, (float32_t)to_hz, + (float32_t)step_hz, (uint32_t)ms)); + SUCCEED_RETURN(errctx); +} + static akerr_ErrorContext *snd_stop(akbasic_AudioBackend *self, int voice) { PREPARE_ERROR(errctx); @@ -128,6 +148,7 @@ akerr_ErrorContext *akbasic_audio_init_akgl(akbasic_AudioBackend *obj) obj->self = NULL; /* the voice table is libakgl's, not ours */ obj->tone = snd_tone; + obj->sweep = snd_sweep; obj->stop = snd_stop; obj->waveform = snd_waveform; obj->envelope = snd_envelope; diff --git a/src/frontend_akgl.c b/src/frontend_akgl.c index 65356a4..ac83d94 100644 --- a/src/frontend_akgl.c +++ b/src/frontend_akgl.c @@ -27,13 +27,6 @@ #include -/* - * akgl/actor.h before akgl/controller.h: controller.h declares two handler - * function pointers taking an akgl_Actor * and includes nothing that declares - * the type, so on its own it does not compile. Filed upstream against libakgl; - * see the same note in src/input_akgl.c, which is where it turned up first. - */ -#include #include #include /* @@ -49,28 +42,6 @@ #include #include -/** @brief Fill in the 2D backend's six vtable entries against an existing renderer. - * - * akgl_render_init2d() installs exactly these, but it also creates its own - * window from the game properties and writes to the `camera` global, which makes - * it part of the akgl_game_init() path -- and a host that already has a renderer - * is not on that path. Without them akgl_text_rendertextat() dereferences a NULL - * draw_texture and the first PRINT segfaults. - * - * Filed upstream: what is wanted is the vtable half of init2d on its own. - * tests/akgl_backends.c carries the identical six lines for the identical - * reason; delete both when it lands. - */ -static void install_2d_vtable(akgl_RenderBackend *backend) -{ - backend->shutdown = &akgl_render_2d_shutdown; - backend->frame_start = &akgl_render_2d_frame_start; - backend->frame_end = &akgl_render_2d_frame_end; - backend->draw_texture = &akgl_render_2d_draw_texture; - backend->draw_mesh = &akgl_render_2d_draw_mesh; - backend->draw_world = &akgl_render_2d_draw_world; -} - akerr_ErrorContext *akbasic_frontend_akgl_init(akbasic_AkglFrontend *obj, const char *title, int w, int h, const char *fontpath, int fontsize, FILE *mirror, FILE *in) { PREPARE_ERROR(errctx); @@ -106,7 +77,15 @@ akerr_ErrorContext *akbasic_frontend_akgl_init(akbasic_AkglFrontend *obj, const &obj->window, &obj->renderer->sdl_renderer), AKGL_ERR_SDL, "Couldn't create the window: %s", SDL_GetError()); window = obj->window; - install_2d_vtable(obj->renderer); + /* + * Bind the 2D methods onto the renderer we just made. akgl_render_init2d() + * would do this too, but it creates its own window from the game properties + * first, which is the akgl_game_init() path a host with its own window is + * not on. libakgl 0.3.0 split the vtable half out for exactly this caller -- + * it was API-gap item 7, and until it landed these six pointers were + * assigned by hand here and in tests/akgl_backends.c. + */ + PASS(errctx, akgl_render_bind2d(obj->renderer)); obj->font = TTF_OpenFont(fontpath, (float)fontsize); FAIL_ZERO_RETURN(errctx, (obj->font != NULL), AKGL_ERR_SDL, diff --git a/src/input_akgl.c b/src/input_akgl.c index 207cc4e..b064163 100644 --- a/src/input_akgl.c +++ b/src/input_akgl.c @@ -16,15 +16,6 @@ #include -/* - * akgl/actor.h before akgl/controller.h, and it is not optional: controller.h - * declares two handler function pointers taking an akgl_Actor * and includes - * nothing that declares the type, so on its own it does not compile. Not a - * workaround anybody should copy -- filed upstream against libakgl, whose own - * style rules call for self-contained headers. Delete this note and the include - * when controller.h includes what it uses. - */ -#include #include #include diff --git a/src/runtime_audio.c b/src/runtime_audio.c index f07c98c..b15343a 100644 --- a/src/runtime_audio.c +++ b/src/runtime_audio.c @@ -73,6 +73,61 @@ static akerr_ErrorContext *collect_numbers(akbasic_Runtime *obj, akbasic_ASTLeaf SUCCEED_RETURN(errctx); } +/** + * @brief Issue SOUND's swept note, translating its arguments out of SID space. + * + * BASIC 7.0 spells the sweep as `dir`, `min` and `step`, all in the SID's + * 16-bit register units rather than in hertz, and `dir` picks the shape: + * + * 0 no sweep -- handled by the caller, which issues a held note instead + * 1 up, from the SOUND frequency toward min + * 2 down, from the SOUND frequency toward min + * 3 oscillate between the two + * + * **Direction 3 sweeps once rather than oscillating**, and that is a deviation + * worth knowing about: akgl_audio_sweep runs one pass from a start to an end, + * and a genuine oscillation needs the mixer to turn around at the endpoints. + * Sweeping once in the direction the endpoints imply is the closest honest + * approximation; TODO.md section 5 records it. + * + * Directions 1 and 2 both name `min` as the far end, so the *value* of min is + * what decides which way the pitch actually travels -- a `min` above the + * starting frequency rises whatever `dir` says. That is the SID's behaviour and + * akgl_audio_sweep works the same way, comparing its two endpoints, so the two + * agree without this having to second-guess either. + */ +static akerr_ErrorContext AKERR_NOIGNORE *sweep_note(akbasic_Runtime *obj, int voice, double hz, double *args, int ms) +{ + PREPARE_ERROR(errctx); + double tohz = 0.0; + double stephz = 0.0; + double stepbase = 0.0; + + PASS(errctx, akbasic_audio_register_to_hz((int)args[4], &tohz)); + + /* + * `step` is a register *delta*, not a register value, so it cannot go + * through register_to_hz on its own -- that maps a position, and a delta has + * no position. Convert it as the distance between register 0 and register + * `step`, which is the same linear scale the table applies to everything + * else. + */ + PASS(errctx, akbasic_audio_register_to_hz(0, &stepbase)); + PASS(errctx, akbasic_audio_register_to_hz((int)args[5], &stephz)); + stephz -= stepbase; + + /* + * A zero step would never arrive and libakgl refuses it outright. One hertz + * is the smallest move that still gets there, and a program that asked for + * no movement asked for a held note -- which is what it gets. + */ + if ( stephz <= 0.0 ) { + stephz = 1.0; + } + PASS(errctx, obj->audio->sweep(obj->audio, voice, hz, tohz, stephz, ms)); + SUCCEED_RETURN(errctx); +} + /* --------------------------------------------------------------- SOUND --- */ akerr_ErrorContext *akbasic_cmd_sound(akbasic_Runtime *obj, akbasic_ASTLeaf *expr, akbasic_Value *lval, akbasic_Value *rval, akbasic_Value **dest) @@ -110,13 +165,27 @@ akerr_ErrorContext *akbasic_cmd_sound(akbasic_Runtime *obj, akbasic_ASTLeaf *exp /* * Arguments 4 through 6 are a frequency sweep: a direction, a floor and a - * step. There is no akgl_audio_* equivalent, and faking one by re-issuing - * tones from akbasic_runtime_step() would tie audible pitch to how often the - * host happens to call us -- a tune that changes key with the frame rate. - * Refused, and filed upstream as akgl_audio_sweep. TODO.md section 7. + * step. `dir` is 0 to hold, 1 to sweep up, 2 to sweep down and 3 to + * oscillate; `min` is the far end of the sweep as a register value, and + * `step` is how far the pitch moves per tick. + * + * This was refused outright until libakgl 0.3.0 grew akgl_audio_sweep -- + * faking it by re-issuing tones from akbasic_runtime_step() would have tied + * audible pitch to how often the host happens to call us, a tune that + * changes key with the frame rate. It is still refused when the host's + * backend has no sweep, which is what an older one looks like. */ - FAIL_NONZERO_RETURN(errctx, (count >= 4 && args[3] != 0.0), AKBASIC_ERR_DEVICE, - "SOUND's frequency sweep needs a device capability that does not exist yet"); + if ( count >= 4 && args[3] != 0.0 ) { + FAIL_ZERO_RETURN(errctx, (args[3] >= 0.0 && args[3] <= 3.0), AKBASIC_ERR_BOUNDS, + "SOUND direction %d out of range (0 to 3)", (int)args[3]); + FAIL_ZERO_RETURN(errctx, (obj->audio->sweep != NULL), AKBASIC_ERR_DEVICE, + "SOUND's frequency sweep needs an audio device that can sweep"); + FAIL_ZERO_RETURN(errctx, (count >= 6), AKBASIC_ERR_SYNTAX, + "SOUND's frequency sweep expected a direction, a minimum and a step"); + PASS(errctx, sweep_note(obj, voice, hz, args, ms)); + SUCCEED_TRUE(obj, dest); + SUCCEED_RETURN(errctx); + } if ( count >= 7 ) { /* The seventh argument selects a waveform, 0 through 3. */ diff --git a/src/sink_akgl.c b/src/sink_akgl.c index 4214052..cdebc90 100644 --- a/src/sink_akgl.c +++ b/src/sink_akgl.c @@ -13,19 +13,11 @@ * equivalent until then. */ -#include #include #include #include -/* - * akgl/actor.h before akgl/controller.h, and it is not optional: controller.h - * declares two handler function pointers taking an akgl_Actor * and includes - * nothing that declares the type. Filed upstream; see the same note in - * src/input_akgl.c, which is where it was found first. - */ -#include #include #include #include @@ -152,47 +144,83 @@ static void echo_line(akbasic_AkglSink *state) state->echolen = state->editlen; } -/** - * @brief Fold one keycode into the line being typed. - * - * The keycodes are SDL's, and an unshifted printable key carries its own ASCII - * value, which is why the range test below is all the translation there is. - * Shifted characters are unreachable: the ring carries no modifier state. See - * akbasic_sink_akgl_set_pump() in akgl.h. - */ -static void edit_key(akbasic_AkglSink *state, int keycode, bool *submitted) +/** @brief Append one byte to the line being typed, if there is room for it. */ +static void edit_append(akbasic_AkglSink *state, char c) { - if ( keycode == '\r' || keycode == '\n' ) { + if ( state->editlen >= (int)sizeof(state->editline) - 1 ) { + /* Full. Dropped silently, exactly as a C128's 80-character limit does. */ + return; + } + state->editline[state->editlen] = c; + state->editlen += 1; + state->editline[state->editlen] = '\0'; +} + +/** + * @brief Fold one keystroke into the line being typed. + * + * **The composed text is preferred over the keycode wherever there is any**, and + * that is the whole reason libakgl 0.3.0's akgl_Keystroke exists. A keycode + * cannot express a shifted character, a keyboard layout, a compose key or a dead + * key; SDL has already worked all of that out by the time the ring sees it, and + * `text` is the answer. Before 0.3.0 this took a bare keycode, folded letters to + * upper case, and could not type a double quote -- which meant a BASIC string + * literal could not be typed at the window at all. + * + * The keycode is still what identifies the editing keys, because Return, + * Backspace and Escape are keys rather than characters and several of them + * compose to text SDL would otherwise hand straight through. + */ +static void edit_key(akbasic_AkglSink *state, const akgl_Keystroke *key, bool *submitted) +{ + size_t i = 0; + + if ( key->key == SDLK_RETURN || key->key == SDLK_KP_ENTER || + key->key == '\r' || key->key == '\n' ) { *submitted = true; return; } - if ( keycode == '\b' || keycode == 0x7f ) { + if ( key->key == SDLK_BACKSPACE || key->key == '\b' || key->key == 0x7f ) { if ( state->editlen > 0 ) { + /* + * One *byte* at a time, which is wrong for a multi-byte character and + * is left that way deliberately: every character a BASIC program can + * hold is one byte (section 1.2's inline string), so a multi-byte + * character cannot survive being submitted anyway. Erasing what was + * accepted is consistent with that. + */ state->editlen -= 1; state->editline[state->editlen] = '\0'; echo_line(state); } return; } - if ( keycode == 0x1b ) { + if ( key->key == SDLK_ESCAPE || key->key == 0x1b ) { state->editlen = 0; state->editline[0] = '\0'; echo_line(state); return; } - if ( keycode < 0x20 || keycode > 0x7e ) { - /* A cursor or function key. Not an editing command here; a script's own - * GET loop is what wants those. */ + + if ( key->text[0] != '\0' ) { + for ( i = 0; key->text[i] != '\0' && i < sizeof(key->text); i++ ) { + /* + * Printable ASCII only. The grid is a byte per cell and a value's + * string is a fixed 256 bytes, so a multi-byte character has nowhere + * to go -- dropping it is honest where storing half of it is not. + */ + if ( (unsigned char)key->text[i] >= 0x20 && (unsigned char)key->text[i] < 0x7f ) { + edit_append(state, key->text[i]); + } + } + echo_line(state); return; } - if ( state->editlen >= (int)sizeof(state->editline) - 1 ) { - /* Full. Dropped silently, exactly as a C128's 80-character limit does. */ - return; - } - state->editline[state->editlen] = (char)toupper(keycode); - state->editlen += 1; - state->editline[state->editlen] = '\0'; - echo_line(state); + + /* + * No composed text: a cursor key, a function key or a bare modifier. Not an + * editing command here -- a script's own GET loop is what wants those. + */ } /** @@ -206,15 +234,21 @@ static void edit_key(akbasic_AkglSink *state, int keycode, bool *submitted) static akerr_ErrorContext AKERR_NOIGNORE *edit_loop(akbasic_AkglSink *state, bool *eof) { PREPARE_ERROR(errctx); + akgl_Keystroke key; bool submitted = false; bool available = false; bool running = true; - int keycode = 0; while ( !submitted ) { - PASS(errctx, akgl_controller_poll_key(&keycode, &available)); + /* + * poll_keystroke rather than poll_key: the same ring, with the modifier + * state and the composed text still attached. poll_key is the reduced + * view, and it is what GET and GETKEY still use -- a script asking "was + * the up arrow pressed" wants a keycode, not the empty string. + */ + PASS(errctx, akgl_controller_poll_keystroke(&key, &available)); if ( available ) { - edit_key(state, keycode, &submitted); + edit_key(state, &key, &submitted); continue; } /* diff --git a/tests/akgl_backends.c b/tests/akgl_backends.c index 1888af0..d5010c2 100644 --- a/tests/akgl_backends.c +++ b/tests/akgl_backends.c @@ -22,7 +22,6 @@ #include -#include #include #include /* @@ -358,26 +357,14 @@ int main(void) &window, &renderer->sdl_renderer), AKGL_ERR_SDL, "Couldn't create window/renderer: %s", SDL_GetError()); /* - * Populate the backend's vtable by hand, which is not where anybody would - * expect to have to do it. - * - * akgl_text_rendertextat() reaches through renderer->draw_texture, and an - * SDL_Renderer alone does not fill that in -- + * Bind the 2D methods. akgl_text_rendertextat() reaches through + * renderer->draw_texture and an SDL_Renderer alone does not fill that in; * deps/libakgl/tests/draw.c gets away without it because the draw - * primitives take the SDL_Renderer directly. The obvious call, - * akgl_render_init2d(), does install these six pointers, but it also - * creates its own window from the game properties and writes to the - * `camera` global, so it is part of the akgl_game_init() path rather than - * something a caller who already has a renderer can use. A host embedding - * this interpreter is in exactly that position. Filed upstream: what is - * wanted is the vtable half of init2d on its own. + * primitives take the SDL_Renderer directly. These six pointers were + * assigned by hand here until libakgl 0.3.0 split akgl_render_bind2d out + * of akgl_render_init2d -- API-gap item 7, filed from this file. */ - renderer->shutdown = &akgl_render_2d_shutdown; - renderer->frame_start = &akgl_render_2d_frame_start; - renderer->frame_end = &akgl_render_2d_frame_end; - renderer->draw_texture = &akgl_render_2d_draw_texture; - renderer->draw_mesh = &akgl_render_2d_draw_mesh; - renderer->draw_world = &akgl_render_2d_draw_world; + CATCH(errctx, akgl_render_bind2d(renderer)); /* * libakgl's own monospaced fixture, borrowed rather than copied. Being diff --git a/tests/akgl_frontend.c b/tests/akgl_frontend.c index 0cec385..66f73cf 100644 --- a/tests/akgl_frontend.c +++ b/tests/akgl_frontend.c @@ -28,7 +28,6 @@ #include -#include #include #include @@ -82,6 +81,9 @@ static bool anything_drawn(SDL_Surface *shot, int x0, int y0, int w, int h) * Straight into SDL's own queue rather than into libakgl's ring, so the * frontend's event pump is the thing under test: nothing reaches the * interpreter unless pump_events() drained this and handed it over. + * + * Key-down only, with no composed text -- which is what a cursor key, a function + * key or a bare modifier produces. */ static void push_key(int keycode) { @@ -93,13 +95,62 @@ static void push_key(int keycode) SDL_PushEvent(&event); } +/** + * @brief Queue a printable keystroke: the key-down *and* the text it composed to. + * + * Two events, because that is what a real keyboard produces and what libakgl + * assembles a keystroke from -- SDL_EVENT_KEY_DOWN records the press and + * SDL_EVENT_TEXT_INPUT delivers the character the layout, the shift state, a + * compose key and a dead key between them worked out. A keycode alone cannot + * express any of that, which is why the ring carries both. + * + * `text` is deliberately allowed to differ from `keycode`: that is how a shifted + * character and a non-US layout are tested at all. + */ +static void push_text_key(int keycode, const char *text) +{ + SDL_Event event; + + push_key(keycode); + memset(&event, 0, sizeof(event)); + event.type = SDL_EVENT_TEXT_INPUT; + event.text.text = text; + SDL_PushEvent(&event); +} + +/** + * @brief A one-character string with static storage, for one printable byte. + * + * SDL_PushEvent copies the SDL_Event but *not* the string an + * SDL_EVENT_TEXT_INPUT points at, so the text has to outlive the call that + * queued it and must not be reused before the queue is drained. A local buffer + * is neither: the first attempt at push_line() used one and every event in the + * line ended up pointing at the same overwritten two bytes, which showed up as + * a REPL that never received a line and a test that hung rather than failed. + */ +static const char *one_char(unsigned char c) +{ + static char table[128][2]; + static bool built = false; + int i = 0; + + if ( !built ) { + for ( i = 0; i < 128; i++ ) { + table[i][0] = (char)i; + table[i][1] = '\0'; + } + built = true; + } + return table[c & 0x7f]; +} + /** @brief Queue a whole typed line, terminator included. */ static void push_line(const char *text) { size_t i = 0; for ( i = 0; text[i] != '\0'; i++ ) { - push_key((int)(unsigned char)text[i]); + push_text_key((int)(unsigned char)text[i], one_char((unsigned char)text[i])); } push_key('\r'); } @@ -269,9 +320,9 @@ static akerr_ErrorContext AKERR_NOIGNORE *test_window_close_stops(void) * @brief The line editor turns keystrokes into a line, echoed as it is typed. * * Read through the sink directly rather than through the REPL, because what is - * being asserted here is the editing itself: the fold to upper case that the - * ring's missing modifier state forces, the backspace, and the echo landing in - * the grid where the cursor says it is. + * being asserted here is the editing itself: that the *composed text* is what + * gets typed rather than the keycode, the backspace, the escape, and the echo + * landing in the grid where the cursor says it is. */ static akerr_ErrorContext AKERR_NOIGNORE *test_line_editor(void) { @@ -282,40 +333,65 @@ static akerr_ErrorContext AKERR_NOIGNORE *test_line_editor(void) PASS(errctx, start_frontend(NULL)); PASS(errctx, FRONTEND.input.flush_keys(&FRONTEND.input)); - /* "prXnt" with the X backspaced away, typed in lower case. */ - push_key('p'); - push_key('r'); - push_key('x'); - push_key('\b'); - push_key('i'); - push_key('n'); - push_key('t'); + /* "prXnt" with the X backspaced away. Lower case survives, which it did not + * before libakgl 0.3.0 -- the editor folded everything to upper case because + * a keycode was all it had. */ + push_text_key('p', "p"); + push_text_key('r', "r"); + push_text_key('x', "x"); + push_key(SDLK_BACKSPACE); + push_text_key('i', "i"); + push_text_key('n', "n"); + push_text_key('t', "t"); push_key('\r'); PASS(errctx, FRONTEND.akglsink.readline(&FRONTEND.akglsink, line, sizeof(line), &eof)); TEST_REQUIRE(!eof, "a submitted line is not end of input"); - /* - * Upper case because akgl_controller_poll_key() reports a keycode and no - * modifier state, so Shift is invisible and lower case is unreachable. A - * C128 is upper case too, which is why this is the right resolution rather - * than merely the available one. Filed upstream. - */ - TEST_REQUIRE_STR(line, "PRINT"); + TEST_REQUIRE_STR(line, "print"); /* The echo is in the grid, and the backspace really removed a character. */ - TEST_REQUIRE_STR(FRONTEND.akglstate.text[0], "PRINT"); + TEST_REQUIRE_STR(FRONTEND.akglstate.text[0], "print"); TEST_REQUIRE_INT(FRONTEND.akglstate.cursorrow, 1); TEST_REQUIRE_INT(FRONTEND.akglstate.cursorcol, 0); TEST_REQUIRE(!FRONTEND.akglstate.editing, "the editor should be idle once the line is in"); - /* Escape abandons a line rather than submitting it. */ - push_key('z'); - push_key(0x1b); - push_key('o'); - push_key('k'); + /* + * A shifted character, which is the whole point of the 0.3.0 keystroke API: + * the key is still SDLK_2 and the character is '"'. Without the composed + * text there is no way to tell those apart, and a BASIC string literal could + * not be typed at the window at all. + */ + push_text_key(SDLK_2, "\""); + push_text_key('h', "H"); + push_text_key(SDLK_2, "\""); push_key('\r'); PASS(errctx, FRONTEND.akglsink.readline(&FRONTEND.akglsink, line, sizeof(line), &eof)); - TEST_REQUIRE_STR(line, "OK"); - TEST_REQUIRE_STR(FRONTEND.akglstate.text[1], "OK"); + TEST_REQUIRE_STR(line, "\"H\""); + + /* + * And a key whose composed character has nothing to do with its keycode, + * which is what a non-US layout looks like: AZERTY's SDLK_Q types "a". + */ + push_text_key(SDLK_Q, "a"); + push_key('\r'); + PASS(errctx, FRONTEND.akglsink.readline(&FRONTEND.akglsink, line, sizeof(line), &eof)); + TEST_REQUIRE_STR(line, "a"); + + /* A key that composes to nothing is not typed: an arrow is not a character. */ + push_key(SDLK_UP); + push_key(SDLK_LEFT); + push_text_key('z', "z"); + push_key('\r'); + PASS(errctx, FRONTEND.akglsink.readline(&FRONTEND.akglsink, line, sizeof(line), &eof)); + TEST_REQUIRE_STR(line, "z"); + + /* Escape abandons a line rather than submitting it. */ + push_text_key('z', "z"); + push_key(SDLK_ESCAPE); + push_text_key('o', "o"); + push_text_key('k', "k"); + push_key('\r'); + PASS(errctx, FRONTEND.akglsink.readline(&FRONTEND.akglsink, line, sizeof(line), &eof)); + TEST_REQUIRE_STR(line, "ok"); /* A closed window ends the wait, reported as end of input rather than raised. */ push_quit(); diff --git a/tests/audio_verbs.c b/tests/audio_verbs.c index 8ff09d1..81f51cb 100644 --- a/tests/audio_verbs.c +++ b/tests/audio_verbs.c @@ -131,20 +131,58 @@ static void test_sound(void) harness_stop(); /* - * The frequency sweep is refused rather than faked. Faking it would mean - * re-issuing tones from step(), which would tie audible pitch to how often - * the host calls us -- a tune that changes key with the frame rate. + * The frequency sweep reaches the device, which it could not until libakgl + * 0.3.0 grew akgl_audio_sweep -- it was refused outright before, because + * faking it would have meant re-issuing tones from step() and tying audible + * pitch to how often the host calls us. + * + * Sweeping *down*: register 1000 is below register 16777, so the endpoints + * decide the direction and both the SID and akgl_audio_sweep read them that + * way. */ - TEST_REQUIRE_OK(run_program("10 SOUND 1, 1000, 60, 1, 500, 10\n")); - TEST_REQUIRE(strstr(HARNESS_OUTPUT, "frequency sweep") != NULL, - "a sweep should be refused, got \"%s\"", HARNESS_OUTPUT); + TEST_REQUIRE_OK(run_program("10 SOUND 1, 16777, 60, 2, 1000, 10\n")); + TEST_REQUIRE(strstr(MOCK.log, "sweep v0 ") != NULL, + "a sweep should reach the device, got \"%s\"", MOCK.log); + TEST_REQUIRE(strstr(MOCK.log, "1000ms") != NULL, + "60 jiffies should still be 1000 ms on a sweep, got \"%s\"", MOCK.log); + TEST_REQUIRE(strstr(HARNESS_OUTPUT, "") != NULL && HARNESS_OUTPUT[0] == '\0', + "a sweep should not be refused, got \"%s\"", HARNESS_OUTPUT); harness_stop(); - /* A sweep direction of zero is "no sweep" and must still play. */ + /* A direction outside 0..3 is a typo worth catching. */ + TEST_REQUIRE_OK(run_program("10 SOUND 1, 1000, 60, 9, 500, 10\n")); + TEST_REQUIRE(strstr(HARNESS_OUTPUT, "SOUND direction 9") != NULL, + "direction 9 should be refused, got \"%s\"", HARNESS_OUTPUT); + harness_stop(); + + /* A sweep needs all three of its arguments. */ + TEST_REQUIRE_OK(run_program("10 SOUND 1, 1000, 60, 1\n")); + TEST_REQUIRE(strstr(HARNESS_OUTPUT, "direction, a minimum and a step") != NULL, + "a partial sweep should be refused, got \"%s\"", HARNESS_OUTPUT); + harness_stop(); + + /* A sweep direction of zero is "no sweep" and must still play a held note. */ TEST_REQUIRE_OK(run_program("10 SOUND 1, 1000, 60, 0\n")); TEST_REQUIRE(strstr(MOCK.log, "tone v0 ") != NULL, "direction 0 is not a sweep and should play, got \"%s\"", MOCK.log); harness_stop(); + + /* + * A backend with no sweep is still a valid backend -- that is what a host + * written against libakgl 0.2.0 looks like -- and SOUND refuses the swept + * note rather than dereferencing a NULL entry point. + */ + TEST_REQUIRE_OK(harness_start(NULL)); + mock_devices_init(); + MOCK_AUDIO.sweep = NULL; + TEST_REQUIRE_OK(akbasic_runtime_set_devices(&HARNESS_RUNTIME, &MOCK_GRAPHICS, + &MOCK_AUDIO, &MOCK_INPUT)); + TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME, "10 SOUND 1, 1000, 60, 1, 500, 10\n")); + TEST_REQUIRE_OK(akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN)); + TEST_REQUIRE_OK(akbasic_runtime_run(&HARNESS_RUNTIME, 0)); + TEST_REQUIRE(strstr(HARNESS_OUTPUT, "audio device that can sweep") != NULL, + "a backend without sweep should refuse, got \"%s\"", HARNESS_OUTPUT); + harness_stop(); } /** @brief ENVELOPE maps the SID rate numbers, and sustain is a level not a time. */ diff --git a/tests/mockdevice.h b/tests/mockdevice.h index 9506ce3..7293296 100644 --- a/tests/mockdevice.h +++ b/tests/mockdevice.h @@ -191,6 +191,19 @@ static akerr_ErrorContext *mock_tone(akbasic_AudioBackend *self, int voice, doub SUCCEED_RETURN(errctx); } +static akerr_ErrorContext *mock_sweep(akbasic_AudioBackend *self, int voice, double from_hz, double to_hz, double step_hz, int ms) +{ + PREPARE_ERROR(errctx); + (void)self; + + FAIL_ZERO_RETURN(errctx, (voice >= 0 && voice < AKBASIC_AUDIO_VOICES), AKBASIC_ERR_BOUNDS, + "mock voice %d out of range", voice); + MOCK.voice_active[voice] = true; + mock_log("sweep v%d %.1fhz->%.1fhz step %.1fhz %dms\n", + voice, from_hz, to_hz, step_hz, ms); + SUCCEED_RETURN(errctx); +} + static akerr_ErrorContext *mock_audio_stop(akbasic_AudioBackend *self, int voice) { PREPARE_ERROR(errctx); @@ -296,6 +309,13 @@ static void mock_devices_init(void) MOCK_AUDIO.self = &MOCK; MOCK_AUDIO.tone = mock_tone; + /* + * Set here, and deliberately cleared again by the one test that needs a + * backend without it: `sweep` may be NULL, which is what a host written + * against libakgl 0.2.0 looks like, and SOUND has to refuse rather than + * crash on one. + */ + MOCK_AUDIO.sweep = mock_sweep; MOCK_AUDIO.stop = mock_audio_stop; MOCK_AUDIO.waveform = mock_waveform; MOCK_AUDIO.envelope = mock_envelope;