diff --git a/HANDOFF-raylib-ports.md b/HANDOFF-raylib-ports.md index 19abffa..f6d3e9b 100644 --- a/HANDOFF-raylib-ports.md +++ b/HANDOFF-raylib-ports.md @@ -1,39 +1,89 @@ # Handoff — the last two raylib ports Two examples, `text_codepoints_loading` and `textures_image_processing`, ported into -`examples/`. This file is the running record: what is done, what was decided without -being able to ask, and what is still open. `PORTING.md` gets the findings that outlive -the port; this file gets the state of the work. +`examples/`. This file is the running record: what is done, what was decided without being +able to ask, and what is still open. `PORTING.md` has the findings that outlive the port — +its last section, "Porting the raylib examples — the last two" — and this file has the +state of the work. -## Status +## Status — done -- [ ] `examples/text-codepoints-loading.flan` -- [ ] `examples/textures-image-processing.flan` -- [ ] Headless acceptance cases for the halves that are arithmetic -- [ ] `PORTING.md` section for this round +- [x] `examples/text-codepoints-loading.flan` — builds +- [x] `examples/textures-image-processing.flan` — builds +- [x] `test/programs/raylib-codepoints.flan` and `test/programs/raylib-image-processing.flan`, + both in the acceptance table, both gated on libraylib, both run plain and at `-O0` +- [x] `PORTING.md` section for this round +- [x] `dune test --root .` green -## The plan, and why it is this one +Four commits, each one a working state: -Both examples in the upstream tree load a file out of `resources/`, and neither file is -in this repo. That is the first decision and it is made the same way the three textures -examples already here made it: **nothing is vendored, the pixels are generated.** -`parrots.png` is `❔` in raylib's own `examples/textures/resources/LICENSE.md` — no -author, no licence — so it is not a file to copy into somebody else's repository on a -sleeping author's behalf. The image-processing example builds its source image out of -`gen-image-*` calls instead. - -The font is the harder half and is the one open question below. +1. the plan and the open question (this file's first version) +2. `PixelFormat`, the one binding this round added +3. the image-processing example and its headless case +4. the codepoints example and its headless case ## Open questions for the author 1. **Vendor `DotGothic16-Regular.ttf`?** `text_codepoints_loading` draws Japanese, and there is no CJK font in this tree. The font is SIL OFL 1.1 and therefore - redistributable, but it is 2 MB of binary in a repository that has one 1 KB PNG in - it, and that is a call about the repository rather than about the port. The example - is written to work either way: it looks for the file, and if the font did not load it - says so on screen and falls back to the default font. Drop the TTF in and the glyphs - appear. + redistributable, but it is 2 MB of binary in a repository whose only binary asset is a + 1 KB PNG, and that is a call about the repository rather than about the port. The + example works either way: it looks for + `examples/resources/DotGothic16-Regular.ttf`, and if it is not there raylib hands back + the default font, the kana draw as boxes, and the program says on screen why. Drop the + TTF in and the glyphs appear with nothing else to change. `~/Repositories/raylib/examples/text/resources/` + has the file and its OFL notice. + +2. **`lib/cimport.ml`'s `agrees` promises an arm it does not have.** The comment at + `lib/cimport.ml:1291` lists, among the differences that are "expected and are not + reported", *"a `(Ptr T)` where the header says `T *` and the hand-written line chose + something more specific"*. The function implements the enum-against-i32 arm and nothing + else. That missing arm is the only thing standing between this round's two refused + bindings and ordinary binding work — see `PORTING.md` §A.1 and §A.2, which have the + exact declarations and the exact error. **Not built: it is a compiler change and the + freeze is on.** It is small and it is well specified, and it is the thing to do first if + these two ports are ever revisited. + +3. The image-processing port keeps the C's `LoadImageColors` → `UpdateTexture` round trip + even though `(.data im-copy)` is the same bytes and already the right type. Kept + because it is what the C does and because nothing else in the corpus exercises that + pair. If you would rather the example were the short version, the comment on + `reload-texture` says exactly what to delete. + +## Deviations from the upstream C, all deliberate, all commented in place + +- **Neither asset is vendored.** `parrots.png` has no licence at all in raylib's own + LICENSE.md, so the image-processing example generates its picture. The generated image is + asymmetric in both axes and has hard edges on purpose — a symmetric one makes the two + flips indistinguishable and the blur invisible. +- **The duplicate removal is a build-up, not a compaction.** The C's shift-the-tail-down + loop reads one element past the end of its allocation, which C tolerates and Flan's + bounds check does not. +- **The codepoint cursor clamps at both ends.** The C's runs off both. +- **`GetCodepointPrevious` is not called**, because it cannot be: a Flan string reaches C + as a NUL-terminated copy and that function reads backwards out of the pointer. Four + lines of continuation-byte walking replace it, and the headless case pins them against + `LoadCodepoints`. +- **The UP key in image processing wraps to 7 and not to 8**, which is the C's own + off-by-one, left as it is so that the port and the example it claims to be do not + disagree. + +## Things a next lane should know + +- `examples/` is **not** compiled by `dune test`. `test/dune` globs it so that imports + resolve, nothing more. `flan build ` is the only proof an example compiles, and + it is worth running on all of them after any change to `vendor/raylib`. +- The header check in `vendor/raylib/headers` runs on **every ordinary build**, not only on + `flan generate-c`. A hand-written `declare-c` that disagrees with `raylib-5.5.h` therefore + breaks every build in the tree, not just regeneration. That is what makes open question 2 + a blocker rather than an inconvenience. +- `(string (slice b off (len b)))` is the idiom for a C `char *` cursor into the middle of a + string, and it costs nothing. It is correct for every entry point that reads forwards and + wrong for every one that reads backwards. +- `slice-from-ptr` is how a raylib pointer-plus-count becomes something with a length. Both + new test programs lean on it. ## Log -- Started from `957ba07` on `dev-loop`. +- Started from `957ba07` on `dev-loop`. The worktree came up on a much older commit + (`2c232dd`, no `examples/` directory at all) and was reset onto the branch tip first. diff --git a/PORTING.md b/PORTING.md index 5ee7c6d..a65a4af 100644 --- a/PORTING.md +++ b/PORTING.md @@ -635,3 +635,255 @@ site, fixed by one parameter), `Handle` and pools (nothing to pool), `Result`/`t (neither host uses that discipline), `handler-case` (one site, `handler-bind` covers it), `loop`/`recur` and tail calls (nothing recurses), user-written allocators, structural typing. + +--- + +# Porting the raylib examples — the last two + +`text_codepoints_loading` and `textures_image_processing`, the two left on the list, both +ported and both building: `examples/text-codepoints-loading.flan` and +`examples/textures-image-processing.flan`, each with a headless acceptance case beside it +in `test/programs/`. They were picked because they were expected to stress two corners the +earlier rounds had not — Unicode and codepoint arrays on one side, CPU-side pixel buffers +and in-place mutation on the other — and both expectations were right, though not in the +places anyone guessed. + +**Coverage.** Read in full: both upstream C sources in `~/Repositories/raylib`, every +`.flan` in `examples/`, `vendor/raylib/raylib.flan`, `generated.flan`, `bindings` and +`headers`, the `PixelFormat` and `Font`/`GlyphInfo` halves of `raylib-5.5.h`, `test/dune`, +the raylib blocks of `test/test_acceptance.ml`, `test/programs/raylib-image.flan` and +`test/programs/virtual-controls-headless.flan`, and the string-crossing half of +`lib/shim.ml` and `lib/cimport.ml`. Everything below was run rather than reasoned about; +each finding in §A names the command that produced the failure. + +## A. The two gaps + +### A.1 `GetCodepointPrevious` cannot be called, and closing it is a checker change + +This is the real find of the round, and it is general: **a C function that reads +*backwards* from the pointer it is handed cannot be given a Flan `string`.** + +`lib/shim.ml` is explicit about why — `Pstr` is "ptr+len in, a NUL-terminated copy out", +and the wrapper builds that copy for the duration of the call. Forwards, that is invisible +and free. Backwards it is not: the bytes in front of the copy belong to the allocator, so a +function that steps back from the pointer reads memory that has nothing to do with the +text. + +It does not crash, which is the bad part. Run headless: + +```flan +(let [text "いろはに" + b (bytes text) + sz 0] + (println (rl/get-codepoint-previous (string (slice b 3 (len b))) (addr sz))) + (println sz)) +``` + +That prints `0` and `0`, where the answer is 12356 (い) and 3. Zero is also what +`GetCodepointPrevious` returns for a genuinely malformed sequence, so there is nothing in +the result to distinguish "Flan handed you a copy" from "your text is broken". The same +expression with `get-codepoint-next` is correct, because that one only reads forwards. +That is the whole of the difference. + +**The code I wanted to write, and what stops it.** The obvious repair is the one +`raylib.flan` already uses twice — a hand-written `-raw` declaration that says `(Ptr u8)` +and means it, as `load-image-from-memory-raw` and `load-font-ex-raw` do: + +```flan +(declare-c get-codepoint-previous-raw + [text (Ptr u8) codepoint-size (Ptr i32)] i32 + "GetCodepointPrevious") +``` + +with `(addr (at b i))` for the interior pointer. It does not work, and the reason is not +the FFI. `lib/cimport.ml` maps `const char *` to `string`, and `agrees` — the function that +decides whether a hand-written declaration and the header say the same thing — accepts +exactly one difference, an enum against a 32-bit integer. `(Ptr u8)` against `string` is not +it. Adding the line above and building *anything*, not only regenerating, gives: + +``` +vendor/raylib/generated.flan:233:12: the declare-c of get-codepoint-previous-raw + disagrees with vendor/raylib/raylib-5.5.h: parameter text is (Ptr u8) and the + header says string (const char *) +``` + +`headers` is read on every build, so the binding cannot be added without relaxing the +header check, and relaxing the header check is a change to the compiler. **Feature freeze; +not built.** + +**Worth flagging beside it: `cimport.ml`'s own comment promises this and the code does not +deliver it.** The list of "three differences that are expected and are not reported" at +`lib/cimport.ml:1291` includes + +> - a `(Ptr T)` where the header says `T *` and the hand-written line chose something more +> specific for a reason it recorded. + +`agrees` implements the enum arm and nothing else. Either that bullet describes an +intention never written, or the rule was lost in a refactor. Whichever it is, the paragraph +is already the design note for the fix: a third arm in `agrees` accepting a hand-written +`(Ptr T)` against a header pointer, which would let this binding and the one in A.2 both be +written honestly. + +**The workaround, and it is a good one.** UTF-8 is walkable backwards without asking +anybody: a continuation byte is `10xxxxxx`, so stepping back over continuation bytes lands +on the lead byte of the previous codepoint, and `get-codepoint-next` from *there* says what +it is — forwards, where the copy costs nothing. That is `step-back` in the example, four +lines. `test/programs/raylib-codepoints.flan` pins it by walking the poem forwards, walking +it backwards, and checking that the two are reverses of each other and that the forward one +is what `LoadCodepoints` says the text contains. + +It is fewer instructions than the call would have been. **But the finding is not "this one +function"** — it is that the direction a C function reads in is invisible in its signature, +and Flan's string crossing makes half of those directions silently wrong. Every future +binding over a `const char *` cursor meets it. + +### A.2 There is no cast between pointer types + +`textures_image_processing` hands the pixels `LoadImageColors` returned straight to +`UpdateTexture`. In C both are pointers and nothing has to be said. Here +`load-image-colors` answers `(Ptr Color)` and `update-texture` takes `(Ptr u8)` — the +header spells that parameter `const void *`, and `cimport.ml` has to render a `void *` as +something — so the call is refused: + +``` +expected (Ptr u8), found (Ptr rl/Color) +``` + +A hand-written `(declare-c update-texture-colors [texture Texture2D pixels (Ptr Color)] … +"UpdateTexture")` is the natural binding fix and is refused by the same header check as +A.1, with the same message. **Not built.** + +**Workaround, and it is legitimate rather than a trick:** the address of the first field of +the first element is the address of the buffer. + +```flan +(rl/update-texture texture (addr (.r (at (slice-from-ptr pixels n) 0)))) +``` + +`Color`'s first field is `r`, a `u8`, at offset 0. It compiles, it is the right address, +and it says out loud what C's implicit conversion was doing quietly. It is also ugly, and +the ugliness is the report: a `(Ptr u8)` view of a typed buffer is something an FFI wants +often — `void *` appears thirty-odd times in raylib.h alone. + +Two smaller notes on the same call. `(.data im-copy)` is already a `(Ptr u8)` over the same +bytes, so the whole round trip is avoidable; the example keeps it because it is what the C +does and because nothing else in the corpus exercises +`LoadImageColors`/`UnloadImageColors`. And `slice-from-ptr` is what makes any of this +readable — it is the one form that turns a raylib pointer plus a raylib count into +something with a length, and it was reached for three times across the two files. + +## B. What was expected to be a gap and was not + +**UTF-8 in a string literal works, end to end, with nothing added.** This was the round's +open question and the answer is clean. The reader takes the bytes, the object file carries +them, the shim hands them over, and `LoadCodepoints` decodes the 54 codepoints of the Iroha +into the 49 distinct ones the atlas is built from. Nothing in Flan claims to know what a +character is — a `string` is bytes and `(bytes s)` / `(string b)` say so in both directions +at no cost — and for this job that is exactly the right amount of opinion. There is a +`valid-utf8?` in the prelude and this example never needs it. + +**An interior pointer into a string has an idiom already.** `(string (slice b off (len b)))` +is the C's `char *ptr` and compiles to nothing: a `string` and a `[u8]` are the same two +words. Every forward-reading `const char *` entry point is reachable that way. + +**`load-font-ex` needed nothing.** It takes a `[i32]` of codepoints, takes the +pointer-and-count apart itself, and uses a zeroed `defvar` as the null pointer that means +"the default ASCII set". It was written for this call before anything called it, and the +call fit it exactly. + +**Fixed arrays were the right shape for both.** A codepoint table with a count, a +`[9 Rectangle]` of toggle buttons, a `[9 string]` of labels. The §3 finding from the +siam-farmer report — a global cannot hold a `Vec`, and fixed arrays with counts are usually +the better answer anyway — held again, in two more programs, with no friction at all. + +## C. Where Flan was better than the C + +**The upstream deduplication reads out of bounds, and Flan will not perform it.** +`CodepointRemoveDuplicates` compacts its array by shifting the tail down over each +duplicate: + +```c +for (int k = j; k < codepointsNoDupsCount; k++) codepointsNoDups[k] = codepointsNoDups[k + 1]; +``` + +On the first duplicate `codepointsNoDupsCount` is still the full count, so at `k = N-1` +that reads element `N` of an `N`-element allocation. C does not notice: `RL_CALLOC` has +slack and the value is overwritten immediately. A literal transcription signals +`BoundsError` and stops the frame. The port is a build-up instead — scan the output, append +if absent — which is shorter, has no shifting in it, and keeps first-seen order exactly as +the C's version does. This is Tier 1 item 4 of the report above catching a real upstream bug +in the first program that met it. + +**The C's cursor walks off both ends of its string,** and the port clamps. `ptr += size` +with RIGHT held runs past the terminator; `ptr -= size` with LEFT held runs in front of the +literal. Same class of thing, same outcome: what C lets through is what Flan makes you +decide about. + +## D. What was added, and what was deliberately not + +**Added — `PixelFormat`, a 24-member `defenum` in `vendor/raylib/raylib.flan`.** +`ImageFormat` moved from the generated half to the hand-written one (`exclude ImageFormat` +in `bindings`), and `enum PixelFormat PIXELFORMAT_` maps the members so all 24 values are +compared against `raylib-5.5.h` on every build. Two of them need a `constant` line, because +raylib spells the ASTC block sizes `ASTC_4x4` with a lowercase `x` where every other letter +in that enum is upper — the same narrow exception `GESTURE_DOUBLETAP` already had. This is +the trade `TextureFilter` and `MouseCursor` already made, for the same reason: the C is +`int newFormat` and exactly one of twenty-four conversions is the one a program meant. + +It paid for itself in the test rather than at the call site. `ImageColorGrayscale` +reallocates into a one-byte-per-pixel buffer, so the working image's `format` goes from 7 to +1 mid-run, and `raylib-image-processing.flan` asserts that it does. Before the enum, that +row was two integers with nothing to say about them. + +**Corrected — a wrong comment.** `raylib.flan`'s `image-from-image` said "There is no +ImageCopy in 5.5". There is: `raylib-5.5.h:1348`, and `generated.flan` has had it bound all +along. The note now says which of the two is the better call for duplicating a whole image. + +**Not added — the two assets, and this is the one open decision.** Both upstream examples +load a file out of `resources/` and neither file is in this tree. + +- `parrots.png` is listed in raylib's own `examples/textures/resources/LICENSE.md` with no + author and no licence — a `❔` in both columns. It is not a file to copy into somebody + else's repository on a sleeping author's behalf. The image-processing example generates + its source picture instead, as the three textures examples already here do. What the + generated image has to be is decided by the filters rather than by taste: asymmetric in + **both** axes, or the vertical and the horizontal flip are indistinguishable and the port + looks broken, and carrying sharp edges, or the Gaussian blur has nothing to work on. +- `DotGothic16-Regular.ttf` is SIL OFL 1.1 and therefore redistributable, but it is 2 MB, + and whether a compiler repository whose only binary asset is a 1 KB PNG should grow a 2 MB + font is a call about the repository rather than about this port. Left to the author; + `HANDOFF-raylib-ports.md` carries it as the open question. The example looks for it under + `examples/resources/`, says on screen when it is not there, and runs either way — raylib + answers a missing path with the default font, whose glyphs are ASCII, so the kana draw as + boxes and the program explains itself rather than looking broken. + +**Neither example needed a language feature.** Nothing below the two refused bindings in §A +was blocked at all, and both of those are one arm in `lib/cimport.ml`'s `agrees` away from +being ordinary binding work. + +## E. Testing + +Both examples build. `flan build` on each is the only direct proof there is: `test/dune` +globs `examples/*` so that imports resolve, and does not compile them. + +Both now also have a headless half in the acceptance table, gated on `ldconfig` finding +libraylib exactly as the existing raylib cases are, each run twice — plain and `-O0`: + +- `test/programs/raylib-image-processing.flan` imports the example and runs its nine filters + over its pixels. It is the first case in the corpus to exercise the **in-place** half of + the Image surface: every filter takes a `(Ptr Image)` and rewrites the buffer under it, + and two of them free the old buffer and install a new one. `raylib-image.flan` is entirely + by value and cannot reach that. Grayscale, invert, tint, contrast, brightness and the two + flips are pinned to the byte; the blur is pinned structurally — size and format survive, a + pixel outside a red rectangle reddens, the inside stays red-dominant — because the exact + kernel is raylib's business and pinning a blurred byte buys a test that goes red when + raylib improves. +- `test/programs/raylib-codepoints.flan` imports the other example and pins three separate + things: that the literal survived (49 distinct of 54, a fact about the Iroha and nothing + else), that the forward and backward walks are reverses of each other (which is what + `step-back` is for), and that the forward walk agrees with `LoadCodepoints` element for + element — the outside opinion, because the second claim alone would pass if both walks + were wrong in the same way. + +Both rows were confirmed to go red when deliberately perturbed, which is the only way to +know an acceptance case is wired in at all. `dune test --root .` is green.