The two ports' findings, and the one arm in the header check that would close them
This commit is contained in:
parent
4c184e68ce
commit
69e5a777e7
@ -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 <example>` 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.
|
||||
|
||||
252
PORTING.md
252
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.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user