diff --git a/CLAUDE.md b/CLAUDE.md index 01bb9c5d..cbb8df75 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ A high-performance multi-platform system driving large LED installations and DMX ## The Process -Every change follows the same timeline: **main → branch → build → test → document → commit → merge → release**. The **product owner** (PO) is the person initiating a branch — any contributor can be one. The PO initiates every event and every gate list — never start one unprompted; if unsure, ask ("Feature work is done; run pre-commit, or do you want to look first?"). A conditional check runs only when its objective trigger matches; an applicable-but-skipped check needs a one-line reason in the commit/PR/release notes. Each cycle produces visible output, and each cycle subtracts: remove code and docs that no longer earn their place, or know why nothing can go — `backlog/` and `history/` shrink too. External contributors follow the same timeline: fork, branch, PR into main — the same checks and review apply. +Every change follows the same timeline: **main → branch → build → test → document → commit → merge → release**. The **product owner** (PO) is the person initiating a branch — any contributor can be one. The PO initiates every event and every gate list — never start one unprompted; if unsure, ask ("Feature work is done; run pre-commit, or do you want to look first?"). This holds even when a gate script would only be *checking* work in progress: running `precommit.py`/`premerge.py` to see where things stand is still starting a gate list, and it writes the logs the PO's own run reports from. Verify work in progress with the individual tools instead (a build, `ctest`, one check script); the event scripts are the PO's to fire. A conditional check runs only when its objective trigger matches; an applicable-but-skipped check needs a one-line reason in the commit/PR/release notes. Each cycle produces visible output, and each cycle subtracts: remove code and docs that no longer earn their place, or know why nothing can go — `backlog/` and `history/` shrink too. External contributors follow the same timeline: fork, branch, PR into main — the same checks and review apply. ### Main @@ -64,7 +64,7 @@ Docs land with the code, not at merge time: the module's spec and catalog card d ### Commit -Git only with the PO in the loop: staging, committing, and pushing happen only when the PO explicitly triggers them. What and when to commit or merge is 100% the product owner's call — never ask or propose commit timing. One combined commit per cycle (no partial commits; hygiene changes fold into the next one). Branches and commits may bundle multiple topics: not every small change gets its own commit — the pre-commit and pre-merge checks would be too much overhead. +Git only with the PO in the loop: staging, committing, and pushing happen only when the PO explicitly triggers them. **The PO verifies EVERY changed file before it is committed.** That is the rule the others serve: nothing reaches history unseen. Two things follow, and both have been broken. **The trigger is the words "commit now", never a task instruction** — "fix it", "do step 4", "the build is broken", even "hotfix it on main" say what to change and nothing about recording it; finishing the work is not a prompt to commit it. And **a "commit now" covers only the files the PO has actually looked at** — touch one more, anything at all, and the tree again holds something unverified, so the go-ahead is void until they see it. Stop at a clean tree, say exactly which files changed, and wait. On main exactly as on a branch; a one-line fix exactly as a feature. What and when to commit or merge is 100% the product owner's call — never ask or propose commit timing. One combined commit per cycle (no partial commits; hygiene changes fold into the next one). Branches and commits may bundle multiple topics: not every small change gets its own commit — the pre-commit and pre-merge checks would be too much overhead. On "run pre-commit": `uv run moondeck/event/precommit.py`. It runs every gate whose trigger the change matches and reports PASS / FAIL / SKIP / MANUAL. Then wait for an explicit "commit now". diff --git a/docs/MIGRATING.md b/docs/MIGRATING.md index 9e34049e..b5605a67 100644 --- a/docs/MIGRATING.md +++ b/docs/MIGRATING.md @@ -6,6 +6,8 @@ projectMM ships **no migration code**: the persistence layer is robust by defaul **Read this when upgrading a device that already holds persisted state.** Entries are newest first. Each says what changed and what to do; most need nothing at all, because the lost value re-populates on next use. +**MoonLive is exempt until it launches.** Nobody is running scripts on a device yet, so a break in the script language or its storage cannot strand anyone, and an entry here would describe an upgrade path no user can take. Its breaking changes are recorded in the commit and PR record instead. This exemption ends at the first release that ships MoonLive as a supported feature; from then it follows the same rule as everything else. + **Action legend** — how much work an entry costs you: | Action | Meaning | @@ -20,40 +22,6 @@ projectMM ships **no migration code**: the persistence layer is robust by defaul ## Unreleased (`next-iteration`) -### MoonLive scripts move to the filesystem (2026-08-11) - -A scripted module used to carry its script as a `source` textarea — a fixed 1 KB array per module, plus a second 1 KB copy to notice edits, **resident whether or not a script was loaded**. Six modules cost 13 KB of a classic ESP32's 320 KB for text that was mostly empty. The script now lives in a file under `/moonlive/`, and the module holds only its **name** (~32 bytes): it is read into a right-sized buffer to compile and freed immediately, so nothing script-sized stays in RAM. A script is bounded by the filesystem instead of by a 1 KB array. - -**Action: *re-add a module* — or, to keep your scripts, *update a file* first.** - -The `source` control no longer exists, so a persisted `"source"` value is an unknown key and is ignored (the robust-reader rule). A MoonLive module therefore boots with **no script**, reporting `no script — set the script name`, and renders nothing until one is named. - -| What | Why | What to do | -|---|---|---| -| Your script text | It was persisted under `source`, a control that is gone | **Copy it out before updating** — it is in `/.config/Layouts.json` (or `Effects.json`) as `"N.source"`. Save it as `/moonlive/.mlv` via the File Manager, then set the module's `script` control to `.mlv` | -| The module's own controls | A script's `@control` sliders exist only once it has compiled, so they are absent until a script is named | Nothing — they reappear with the script, keeping their persisted values | - -`/moonlive/` is created on demand: naming a script is enough to make the folder appear, so a fresh device needs no setup. - -**Editing today** goes through the File Manager rather than the module's own card. Wiring the card's editor to the same file is a separate change. - -### MoonLive: a script can no longer declare a name the engine supplies (2026-08-10) - -`t` (elapsed milliseconds), `width`/`height`/`depth` (the logical grid) and `x`/`y`/`z` (the light a modifier is transforming) are now **system variables** the engine supplies, so a script cannot declare one. Previously each binding faked them by prepending hidden declarations to the script, which meant an effect could declare its own `width` and quietly disagree with the layer it was drawing into. - -Each module supplies only the names it writes, so what is reserved depends on the module: a layout gets `t` alone, an effect adds the grid, a modifier adds the coordinate. **`x` and `y` remain usable as loop counters in a layout or an effect.** - -**Action: *update a file*, for scripted layouts only.** - -A **layout** is the one script that legitimately used those names for its own controls: it *defines* where lights are, so it has no grid to be handed. A persisted layout script declaring `uint8_t width = 16;` now fails to compile with `name is a system variable`, and the layout places no lights — the fixture is **dark** until the script is edited. - -| What | Why | What to do | -|---|---|---| -| A scripted layout declaring `width`/`height` | The name is what the layout is defining, so the declaration is a compile error and no lights are placed | Edit the `.mlv` file in the File Manager, renaming its own controls (the shipped `grid.mlv` uses `cols`/`rows`), then set the module's `script` control to that file | -| A scripted **modifier** using `x`, `y` or `z` as a loop variable | A modifier IS handed a coordinate under those names, so they cannot also be counters there | Rename the loop variable to something the modifier is not handed (`i`, `n`) | - -Effects and modifiers need no change: they were already being handed these values, just through a preamble instead of by name. The error names the clash, and the module shows it on its card, so a broken script says why rather than failing silently. - ### The `Layers` container is renamed to `Effects` (2026-08-08) The three top-level light containers are now **Layouts, Effects, Drivers** — L.E.D. The old name sat one character from its own child (`Layers` holding `Layer`s) and read as a near-twin of `Layouts`, which is the pair a newcomer actually has to tell apart. The tree is unchanged in shape: `Effects` → `Layer`s → effects and modifiers. diff --git a/docs/architecture.md b/docs/architecture.md index 69b5d031..c3599d0b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -470,7 +470,7 @@ The engine is a **domain-neutral core** with one narrow seam, structured as thre A recompile is the normal cold-path rebuild: editing the `source` control routes through the same `prepare()` sweep every control change uses, so a new script swaps in live (no reboot), and a parse error surfaces in the module status while the layer renders dark — robust to any input. The module contract is [MoonLiveEffect](moonmodules/light/MoonLiveEffect.md). -**A scripted module differs from a compiled one in one thing only: where its behaviour comes from.** Everything else is the same mechanism — the same base class, the same `prepare()`/`release()` lifecycle, the same controls, the same status and memory reporting, the same container contract. A `MoonLiveLayout` is a `LayoutBase` that answers `lightCount()` and `forEachCoord()` like any other; it just answers them by running compiled machine code instead of arithmetic over its members. When a scripted binding needs a mechanism its compiled sibling does not, that is a finding: either the mechanism belongs in the base for everyone, or the divergence needs its reason stated where it is introduced. A binding that drifts into its own lifecycle stops being a module and becomes a second system to maintain. +**A scripted module differs from a compiled one in one thing only: where its behaviour comes from.** Everything else is the same mechanism — the same base class, the same `prepare()`/`release()` lifecycle, the same controls, the same status and memory reporting, the same container contract. A `MoonLiveLayout` is a `LayoutBase` that answers `lightCount()` and `placeLights()` like any other; it just answers them by running compiled machine code instead of arithmetic over its members. When a scripted binding needs a mechanism its compiled sibling does not, that is a finding: either the mechanism belongs in the base for everyone, or the divergence needs its reason stated where it is introduced. A binding that drifts into its own lifecycle stops being a module and becomes a second system to maintain. The one place this is not yet clean: `applyState()` prepares parent-before-child, so a container asks its children for their extent before those children have prepared. A compiled layout computes its count from its members and does not notice; a scripted one has nothing to answer with until it compiles, so it compiles on demand from a `const` method — the `const_cast` and `mutable` members in `MoonLiveLayout` exist for that and for nothing else. Removing them means giving core a way for children to prepare before a container aggregates them, which is a lifecycle change for every module. diff --git a/docs/backlog/backlog-core.md b/docs/backlog/backlog-core.md index ae67d754..766237d5 100644 --- a/docs/backlog/backlog-core.md +++ b/docs/backlog/backlog-core.md @@ -248,9 +248,9 @@ Sequencing rule (unchanged): each functionality lands a device-side control firs ### Per-layout coordinate offset for independent placement (backlog) -`Layouts` stitches multiple child layouts into one physical light space, but only their *indices* are stitched (offset sequentially in `forEachCoord`) — their *coordinates* are not translated. Two layouts therefore overlap in the same coordinate box: two 64×64 grids both occupy x,y ∈ 0..63, so the Layer's dense bounding-box buffer is 64×64 (4096 voxels) even though the container reports 8192 lights, and the second layout's lights land on the first's positions. `scenario_Layouts_mutation` documents this (its steps assert pipeline liveness, not buffer-size arithmetic). +`Layouts` stitches multiple child layouts into one physical light space, but only their *indices* are stitched (offset sequentially in `placeLights`) — their *coordinates* are not translated. Two layouts therefore overlap in the same coordinate box: two 64×64 grids both occupy x,y ∈ 0..63, so the Layer's dense bounding-box buffer is 64×64 (4096 voxels) even though the container reports 8192 lights, and the second layout's lights land on the first's positions. `scenario_Layouts_mutation` documents this (its steps assert pipeline liveness, not buffer-size arithmetic). -When picked up: add `offsetX/Y/Z` (lengthType) controls to `LayoutBase`; `Layouts::forEachCoord` translates each child's emitted coords by its offset so layouts occupy disjoint regions of the physical extent (a 64-wide grid at offsetX=64 sits beside another at offsetX=0 → a 128×64 combined extent). `Layer::onBuildState` already derives physical dims from the max emitted coordinate, so it would pick up the wider extent automatically. Until then, "multiple layouts" means "multiple layouts sharing a coordinate box", which is only useful when they genuinely overlap (e.g. a sphere inscribed in a grid). +When picked up: add `offsetX/Y/Z` (lengthType) controls to `LayoutBase`; `Layouts::placeLights` translates each child's emitted coords by its offset so layouts occupy disjoint regions of the physical extent (a 64-wide grid at offsetX=64 sits beside another at offsetX=0 → a 128×64 combined extent). `Layer::onBuildState` already derives physical dims from the max emitted coordinate, so it would pick up the wider extent automatically. Until then, "multiple layouts" means "multiple layouts sharing a coordinate box", which is only useful when they genuinely overlap (e.g. a sphere inscribed in a grid). ### Improv as a child of NetworkModule (deferred — needs scheduler work first) @@ -279,7 +279,36 @@ Run user-authored scripts on a running device — a scripted effect, layout, mod The **bottom-up landscape survey** is done — [livescripts-analysis-bottom-up.md](livescripts-analysis-bottom-up.md): deep-reads the [ESPLiveScript fork](https://github.com/ewowi/ESPLiveScript/tree/fix-warnings) (a from-scratch C-like JIT that emits **native Xtensa** machine code — blazingly fast but **Xtensa-only**, so it covers classic+S3 and *not* P4/Teensy/desktop), surveys the field (PixelBlaze bytecode VM + web editor, WLED ARTI-FX AST-walking interpreter, embedded VMs / WASM / lightweight multi-ISA JITs), and extracts the load-bearing decisions (execution strategy, the IR seam ESPLiveScript lacks, the MoonModule binding, the per-pixel contract, memory placement, sync, sandboxing). Its thesis to validate: a **portable bytecode-VM baseline that runs on every target on day one + an optional native back-end for the hot ISAs behind a shared IR**. **Next: the top-down redesign** — the prompt that generates `livescripts-analysis-top-down.md` is at the bottom of the bottom-up doc; it produces the reference architecture + staged spike plan. Implementation is multi-commit, spike-ordered, after the top-down lands. Credits: [history/hpwit-ESPLiveScript.md](../history/hpwit-ESPLiveScript.md). -## HTTP and OTA +### Duplicate module names are reachable, and silent (backlog) + +Two modules in the tree may hold the SAME name. Found on the bench: a classic ESP32 had a +`MoonLiveLayout` and a `MoonLiveEffect` both called `MoonLive`, one under `Layouts` and one under a +`Layer`. Nothing reported it. The UI keys a card's controls by module name, so both cards resolved to +the same entry and the effect's `bpm`/`zoom` sliders rendered under the LAYOUT's heading, where its +own `petals`/`radius` should have been. The server data was correct throughout; only the display was +wrong, which is what makes it hard to recognise. + +`Scheduler::ensureUniqueName` exists and is called on `/api/modules` creation and after a persistence +load, so the tree normally cannot reach this state. The bench pair predates that pass or arrived +through a path that skipped it, which is exactly the case a check would catch. **The gap is that +nothing NOTICES:** a name collision is tolerated silently rather than reported, and the first symptom +is a UI showing another module's controls. + +Fix: assert uniqueness after the persistence load and report a collision in the module status, so a +device that reaches this state says so instead of rendering the wrong card. Renaming a module from +the UI would also give a user a way out; there is no `name` control today. + +### Deleting a module by name removes the FIRST match (backlog) + +`DELETE /api/modules/` resolves through `findModuleByName`, which returns the first match in +tree order. With a duplicate name (above) that is not necessarily the module the caller meant: on the +bench, deleting the effect by name would have removed the layout, because the layout came first. + +Noticed while repairing that device, and avoided only by reading the handler before running the +request. It is latent rather than dangerous today, because duplicates are supposed to be impossible, +but the two issues compound: the state that makes a delete ambiguous is the same state nothing warns +about. Fix alongside the check above, either by refusing an ambiguous delete or by addressing a +module by a path rather than a bare name. ### HTTP file serving blocks the render tick (backlog) diff --git a/docs/history/lessons.md b/docs/history/lessons.md index 286d7e8f..59d70682 100644 --- a/docs/history/lessons.md +++ b/docs/history/lessons.md @@ -110,7 +110,7 @@ The harder lesson is verification: the agent's hardware test asserted the Layer The consolidation (three CRTP driver classes → one `ParallelLedDriver` selecting a `LedPeripheral` backend at runtime) surfaced four robustness bugs — most on the bench, where a desktop test could not reach them — each about a *cross-cutting rule reaching the wrong object*, and all found only because a driver's state lives on a swappable backend now. -- **A per-parent `quiesce()` misses a worker that walks the WHOLE tree.** Replacing a **layout** on a split-render device (the core-1 encode worker running) was a LoadProhibited use-after-free. Core already had a "stop the worker before a structural mutation" rule — `MoonModule::removeChild`/`replaceChildAt` call `this->quiesce()` — but that only stops a worker the *mutated node's parent* owns. The encode worker is owned by `Drivers`, yet it ticks the drivers and a driver walks the entire layout/layer tree (`PreviewDriver::sendFrame → Layouts::forEachCoord`), so freeing a node in a *sibling* subtree (a layout) never quiesced it. The lesson: when a worker reads **across** subtrees, quiescing the mutated node's own parent is not enough — the guard must reach the worker wherever it lives. Fix: a core `quiesceForMutation()` that also fires a `quiesceRenderHook_` (a function-pointer seam mirroring `setSchemaChangedHook`), wired once in `main.cpp` to `Drivers::quiesceRenderSplit()`, so core stays domain-neutral. And the completeness trap: the first fix covered `add`/`remove`/`replace` but missed the **fourth** mutator, `moveChildTo` (the drag-reorder UI) — a cross-cutting rule lifted into core must cover *every* path, not three of four. (`unit_Drivers_rendersplit` pins both a layout mutation and a reorder through the real worker thread; each is verified green→red.) +- **A per-parent `quiesce()` misses a worker that walks the WHOLE tree.** Replacing a **layout** on a split-render device (the core-1 encode worker running) was a LoadProhibited use-after-free. Core already had a "stop the worker before a structural mutation" rule — `MoonModule::removeChild`/`replaceChildAt` call `this->quiesce()` — but that only stops a worker the *mutated node's parent* owns. The encode worker is owned by `Drivers`, yet it ticks the drivers and a driver walks the entire layout/layer tree (`PreviewDriver::sendFrame → Layouts::placeLights`), so freeing a node in a *sibling* subtree (a layout) never quiesced it. The lesson: when a worker reads **across** subtrees, quiescing the mutated node's own parent is not enough — the guard must reach the worker wherever it lives. Fix: a core `quiesceForMutation()` that also fires a `quiesceRenderHook_` (a function-pointer seam mirroring `setSchemaChangedHook`), wired once in `main.cpp` to `Drivers::quiesceRenderSplit()`, so core stays domain-neutral. And the completeness trap: the first fix covered `add`/`remove`/`replace` but missed the **fourth** mutator, `moveChildTo` (the drag-reorder UI) — a cross-cutting rule lifted into core must cover *every* path, not three of four. (`unit_Drivers_rendersplit` pins both a layout mutation and a reorder through the real worker thread; each is verified green→red.) - **A control whose backing variable moves between objects is lost on reload unless persistence re-binds first.** After a watchdog reboot the giant wall's `clockPin` (and the whole MoonI80 ring cluster) reverted to their defaults — a reboot silently changing a control, which should never happen. Root cause: those controls live on the *peripheral backend* object, and which backend is live depends on the `peripheral` control's value. On reload `FilesystemModule::applyNode` overlaid the saved values in list order: `peripheral` got written but did **not** swap the live backend, so `clockPin` was written to the *default* backend's member — then the later swap to the saved peripheral discarded that backend, reverting clockPin to its constructor default. The lesson: when a module's **control set depends on one of its own control values**, a single overlay pass writes the value-dependent controls onto the wrong (about-to-be-replaced) objects. Fix: overlay → `rebuildControls()` (which re-runs `defineControls`, swapping the live backend to match the just-applied `peripheral` and re-binding the list to the *right* members) → overlay again. General (any value-dependent control set), gated by `rebuildControls`'s schema-hash so it no-ops for ordinary modules, and the second overlay is idempotent. Invisible on desktop (no real backends link, so no swap); found only on a MoonI80 board whose persisted peripheral differs from the constructor default. (`unit_FilesystemModule_persistence` pins it with a value-dependent mock, verified green→red.) @@ -501,3 +501,49 @@ instructions and the defect was in the stack layout around them. start at at least 32") and phrases it as a minimum for exactly this reason. The reserve is therefore derived from the emitted call opcode rather than written down, so widening the call moves the reserve with it or fails the build. + +## Lessons from the script-functions branch: a tidier disassembly can be the broken one + +Local calls and recursion in MoonLive cost four stacked defects, and every one of them was +invisible to a green test suite. The theme is narrower than "test on hardware": it is that the +evidence which *looks* most authoritative here is the evidence that lies. + +- **A cleaner-looking emitted block can be the broken one.** Removing the spill pass's function + boundary remap makes `crosshair.mlv` disassemble BETTER on Xtensa (three tidy `entry`/`retw.n` + pairs instead of two and a stray) and keeps all 1282 host tests green. It also boot-loops an S3 + with `StoreProhibited` and the light-buffer pointer holding `0xff`: a store through a register + that a mid-statement frame boundary left holding a colour byte. Two things conspire — the host + backend never executes that path, and the disassembler decodes the zero padding between + functions as instructions. **A change to function-boundary or frame code gets a flash and a + soak, never a listing review.** + +- **The host backend cannot pin an argument-passing contract, because its register map hides + one.** A script-to-script call emitted no argument setup at all, so each callee parked whatever + the caller had left in the argument registers and its first control read faulted at + `EXCVADDR 0x9`, offset 9 into a null arena. On arm64 the same omission fails nothing: R0..R4 map + onto the ABI argument registers and `bl` leaves them alone, so the values survive by luck of the + mapping. **Where a backend's register map coincides with the ABI, a test on that backend proves + the contract holds by accident rather than by construction — say so in the test.** + +- **An index into an array a later pass rewrites is a bug with a delay.** `CallScript` first + carried the callee's IR index. The spill pass inserts a Reload before a read, so every index past + its first insertion shifts and the call named an op that no longer started a function. A function + NUMBER survives any rewrite. **When one pass hands a position to another, hand it a name.** + +- **`swap()` must swap the whole object.** `IrProgram::swap` exchanged the ops, counts and slots + but not the function table, so the spill pass's carefully remapped boundaries went into the + discarded half and the lowering read the stale ones. The remap was correct; the transfer was not. + +- **An empty function is a real case in a guard that brackets a body.** The recursion depth guard + is emitted at a function's first real op, because it must follow the host arguments being parked. + A function whose entire body is that parking (`nop() {}` parses) never reached the emission point + while its epilogue still decremented, so every call to it drove the counter DOWN, two calls + wrapped the byte past zero, and the next legal call was refused as too deep. **Any guard with an + entry and an exit needs the zero-length body as a test, not just the deep one.** + +- **The toolchain answers ISA questions faster than reasoning does.** "Must an Xtensa `entry` be + 4-byte aligned?" took one `xtensa-esp32-elf-as` invocation to settle: it refuses with + `Error: unaligned entry instruction`. The same question had already cost an afternoon of + hypotheses. hpwit's `new-parser` hits the identical wall and leaves it unhandled, which is + confirmation the question is real rather than self-inflicted. **Assemble the case before + theorising about it.** diff --git "a/docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow.md" "b/docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow (shipped, steps 4-5 superseded by 20260813).md" similarity index 99% rename from "docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow.md" rename to "docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow (shipped, steps 4-5 superseded by 20260813).md" index edd42650..1da63433 100644 --- "a/docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow.md" +++ "b/docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow (shipped, steps 4-5 superseded by 20260813).md" @@ -1,7 +1,7 @@ # Plan: MoonLive scales — right-sized IR, and the stack as the register overflow > **Steps 1–3 shipped. Steps 4–5 (the register allocator) are SUPERSEDED by -> [Plan-20260813 — MoonLive on a stack machine](Plan-20260813%20-%20MoonLive%20on%20a%20stack%20machine%20%E2%80%94%20the%20frame%20is%20where%20values%20live.md).** +> [Plan-20260813 — MoonLive on a stack machine](Plan-20260813%20-%20MoonLive%20on%20a%20stack%20machine%20%E2%80%94%20the%20frame%20is%20where%20values%20live%20(shipped).md).** > The allocator was built and works on the host at every budget, but on Xtensa it leaves ZERO > allocatable registers (10 − 1 scratch − 5 ABI vregs − 4 reload temps), so every looped script is > refused there. Bench-measured on an S3. The successor plan puts every variable in the frame diff --git "a/docs/history/plans/Plan-20260813 - MoonLive on a stack machine \342\200\224 the frame is where values live.md" "b/docs/history/plans/Plan-20260813 - MoonLive on a stack machine \342\200\224 the frame is where values live (shipped).md" similarity index 79% rename from "docs/history/plans/Plan-20260813 - MoonLive on a stack machine \342\200\224 the frame is where values live.md" rename to "docs/history/plans/Plan-20260813 - MoonLive on a stack machine \342\200\224 the frame is where values live (shipped).md" index aa7e9619..f14bcb22 100644 --- "a/docs/history/plans/Plan-20260813 - MoonLive on a stack machine \342\200\224 the frame is where values live.md" +++ "b/docs/history/plans/Plan-20260813 - MoonLive on a stack machine \342\200\224 the frame is where values live (shipped).md" @@ -1,6 +1,6 @@ # Plan: MoonLive on a stack machine — the frame is where values live -Supersedes [Plan-20260809 — MoonLive scales](Plan-20260809%20-%20MoonLive%20scales%20%E2%80%94%20right-sized%20IR,%20and%20the%20stack%20as%20the%20register%20overflow.md), +Supersedes [Plan-20260809 — MoonLive scales](Plan-20260809%20-%20MoonLive%20scales%20%E2%80%94%20right-sized%20IR,%20and%20the%20stack%20as%20the%20register%20overflow%20(shipped,%20steps%204-5%20superseded%20by%2020260813).md), whose steps 1–3 shipped and stand. This replaces its steps 4–5 (the register allocator) with a different answer to the same goal. @@ -286,28 +286,76 @@ Each step is independently verifiable, and the branch stays green throughout: `draw::line` be ordinary calls rather than a special case. Script-local functions will pass their arguments the same way when they arrive. -4. ⬜ **Collapse the three lowerers into one.** With storage no longer per-target, the IR walk is one - algorithm; what remains per-backend is encodings, the frame/ABI constants and the branch forms. - Verify by adding nothing to the platform layer that is not genuinely platform-specific. -5. ⬜ **Delete the allocator** (`MoonLiveSpill.{h,cpp}`, `RegBudget`, the per-backend budget plumbing) - once nothing calls it. The `prologue(slots)` / `spillStore` / `spillLoad` surface on the three - assemblers is KEPT — it is already frame-addressed through a frame pointer for exactly this - reason, and the stack machine uses it directly. -6. ⬜ **One system-variable table for every role**, with a modifier's coordinate renamed to - `xPos`/`yPos`/`zPos`. Independent of the codegen work — different files, no shared risk — so it - can land whenever, but it comes before step 7 because it is what makes the bindings differ by - almost nothing. Needs a MIGRATING entry and a sweep of the shipped scripts. -7. ⬜ **Factor the three bindings onto one shared base**, so the script name, hash, engine and the - compile-and-report path exist once. The scripted driver that follows should then be a small - subclass, and that is the test of whether this step actually worked. +4. ✅ **Collapse the three lowerers into one.** The IR walk is now `core/moonlive/moonlive_lower.h`, + a template over the assembler; each backend is a ~20-line adapter naming its assembler and its + register count. 537 lines of triplicated algorithm became 190 shared plus 62 of adapter. The + two device lowerings had differed by two identifier tokens; the host one by `Mov`, the branch + spelling, and a `FillElems` that used a third scratch register, all of which turned out to be + free choices rather than ISA facts. Host gained `movReg`/`branchGeU`/`branchNe` (its `cmp` and + `branchIf` are now private, since a flags pair cannot be shared with a backend that has none) + and adopted the devices' `FillElems`, which is what let the scratch reservation become uniform. + Verified: nothing ISA-specific left the platform layer, and all four boards emit byte-identical + exec blocks to the three-file version. +5. ⛔ **DROPPED: delete the allocator.** Its precondition never came true. The step said "once + nothing calls it", on the assumption that a stack machine makes spilling unreachable; it does not. + Registers still hold expression temporaries, so a complex enough expression on Xtensa's ten still + spills, and all three lowerings call `spillToBudget` today. The allocator is 380 lines with its own + test suite built on the squeezed-budget technique, which is the ONLY way the register algorithm is + tested at all, since only the host backend executes in tests. Deleting it would remove a working + safety net and its coverage to save nothing. It earns its place; the step was written before that + was knowable. +6. ✅ **One system-variable table for every role**, with a modifier's coordinate renamed to + `xPos`/`yPos`/`zPos`. `lightSysVars()` is the one table; the three role accessors remain as + aliases so every call site reads unchanged. `x`/`y`/`z` are now ordinary loop counters in every + role, which is what removes the trap that made `disasm.py` refuse the shipped `grid.mlv`. The + three shipped modifier scripts and the tests that encoded the old per-role rule moved with it; + the test that specified the split now specifies the single vocabulary. No MIGRATING entry: that + file is exempt for MoonLive until it launches, since nobody is running scripts on a device yet + and an entry would describe an upgrade path no user can take. Verified on the S3: a scripted + modifier compiles and folds with the new names. +7. ⛔ **SUPERSEDED: factor the three bindings onto one shared base.** Not implemented, and it should + not be: the step asked for the wrong shape, and both the code and the product direction say so. + + **The code's objection.** The three bindings derive from three SIBLING bases (`EffectBase`, + `LayoutBase`, `ModifierBase`), each deriving from `MoonModule`. A shared `MoonLiveBase : MoonModule` + therefore gives every binding TWO `MoonModule` subobjects (two control lists, two status fields) + unless `MoonModule` becomes a virtual base, which changes object layout and cost for every module + in the system to serve three of them. A CRTP mixin avoids that but cannot reach `MoonModule`'s + protected members without friend declarations in all three, trading duplication for access + plumbing. + + **The payload is also smaller than it looked.** Excluding comments, the duplication is ~25 lines + appearing three times, of which only `defineControls()` (8 lines) is identical. The compile trunk + has three genuinely different tails, and `compiledHash_` MEANS two different things: the layout + tests only whether it is zero (a presence flag), the modifier compares its value (a change + detector). Sharing it naively breaks one or the other. + + **The product's objection, which is the decisive one.** A MoonLive script should look like the + compiled module it stands in for: `defineControls()` and `tick()` for an effect, + `forEachCoord()`/`lightCount()` for a layout, `modifyLogical()`/`modifyLogicalSize()` for a + modifier. Once a script DEFINES named entry points, a binding's job is to compile, discover which + ones it defined, and call the right one at the right time. The three bindings stop being three + kinds and become one kind with different entry points present, which is also what makes "an effect + that also modifies" expressible. A class hierarchy is the wrong structure for that; a dispatch + table is the right one, and it cannot be designed before the entry points exist. + + Steps 10 to 13 replace this step. + +Steps 10 to 13, which replaced step 7, moved to their own plan once they grew into a language +change rather than a refactor: [Plan-20260817 — MoonLive scripts are +classes](Plan-20260817%20-%20MoonLive%20scripts%20are%20classes.md). + 8. ✅ **Bench: S3 and P4**, a scripted layout and a scripted effect, both with nested loops. Done on FOUR boards (S3, classic ESP32, P4, S31), scripted layout + effect, plasma and the heavier ripples, after the Xtensa frame fix below. 9. ✅ **Measure** with `collect_kpi.py` and record the cost honestly in performance.md, so the later decision about register promotion is made against numbers rather than intuition. -Steps 4, 6 and 7 are the deduplication, and they come AFTER the mechanism works rather than during it — -collapsing three copies while the design underneath is still moving would mean doing it twice. +Steps 4 and 6 are the deduplication, and they come AFTER the mechanism works rather than during it: +collapsing three copies while the design underneath is still moving would mean doing it twice. Step 7 +was meant to be the third, and turned out to be the point where deduplication stops being the right +question. Steps 10 to 13 replace it, and they are additive rather than subtractive: they change what +a script LOOKS LIKE, and the shared structure falls out at the end instead of being designed up front. ## Xtensa: the one target still failing, and why @@ -390,13 +438,28 @@ invisible to every encoding check because each instruction was correct. See [lessons § the register-window frame bug](../lessons.md#lessons-from-the-moonlive-on-xtensa-branch-the-register-window-frame-bug). All four boards (S3, classic, P4, S31) now run scripted layouts and effects. +## Status: CLOSED + +Every step is resolved. 1, 2, 3, 3b, 3c, 4, 6, 8 and 9 shipped and are verified on four boards +(S3, classic ESP32, P4, S31). Step 5 is dropped and step 7 is superseded, each with its reason +recorded above. The machine this plan set out to build is done: values live in frame slots, one +lowering serves every backend, one system-variable vocabulary serves every role, and the Xtensa +frame contract that blocked the whole thing is fixed and pinned. + +What a script LOOKS LIKE is the next question, and it continues in +[Plan-20260817 — MoonLive scripts are classes](Plan-20260817%20-%20MoonLive%20scripts%20are%20classes.md). + ## Then, separately Only after the above is on main and measured: - Whether to promote anything into registers, and which — including whether any of the fixed ABI vregs earn a register at all. -- Script-local functions with arguments, `if`, and recursion — the features this design exists to - make cheap. -- A scripted DRIVER as the fourth role, which is the honest test of step 6: if it needs more than a - small subclass, the factoring did not go far enough. +- A scripted DRIVER as the fourth role, which is the honest test of the successor plan's dispatch + step: if it needs more than its own entry point and a row in the table, the structure did not go + far enough. + +Everything else this list used to hold moved INTO the successor plan rather than being deferred: +script functions and recursion (recursion is a stated payoff of the stack machine above, "each +activation gets its own frame", so listing it as a later nicety contradicted this plan's own table), +`if`, and an effect that also defines `modifyLogical`. They are launch requirements, not follow-ups. diff --git a/docs/history/plans/Plan-20260817 - MoonLive scripts are classes.md b/docs/history/plans/Plan-20260817 - MoonLive scripts are classes.md new file mode 100644 index 00000000..c946b2f2 --- /dev/null +++ b/docs/history/plans/Plan-20260817 - MoonLive scripts are classes.md @@ -0,0 +1,517 @@ +# Plan: MoonLive scripts are classes + +Takes over from [Plan-20260813 — MoonLive on a stack machine](Plan-20260813%20-%20MoonLive%20on%20a%20stack%20machine%20%E2%80%94%20the%20frame%20is%20where%20values%20live%20(shipped).md), +whose step 7 (factor the three bindings onto one shared base) this replaces. That plan finished the +MACHINE: values live in frame slots, one lowering serves every backend, one system-variable +vocabulary serves every role. This plan changes what a script IS: how it is written (steps 1 to 5) +and what it can say (steps 6 to 10). + +**MoonLive launches when all of it is there**, so the order below is the one that is best to BUILD +in, not the one that shows best soonest. The consequence worth stating out loud is that the most +visible work (`if`, reading a light back, particles, the editor) comes last, which is a deliberate +trade rather than an oversight. + +## What is missing before it can launch + +The engine is not the gap. Live-editing a script on a running device and watching 12,288 lights +change on the next tick, at native speed, is already true, and it is the thing nobody expects from a +microcontroller. The gap is what a script can EXPRESS: today that is smooth arithmetic over a grid, +which gives plasma, ripples and gradients and stops there. Steps 6 to 9 are that list. + +Recursion is in scope and ships in step 1, but it is not on that list, because it is not what an +effect author is missing: they do not write recursive functions. It earns its place for a different +reason. It is what makes this a real language rather than a macro expander, which is a credibility +floor rather than a feature anyone points at, and the stack machine already bought it, so the cost +now is that a script function gets a real frame instead of being special-cased. + +## Why the change + +A MoonLive script today is a bag of declarations and statements, and a control is declared by a +COMMENT that changes behaviour: + +``` +uint8_t bpm = 30; // @control 1..240 +for (y = 0; y < height; y = y + 1) { ... } +``` + +That is not C, and it does not resemble the compiled module it stands in for. A compiled effect is a +class with `defineControls()` and `tick()`; a scripted one should read the same way, so that what a +contributor learns from one transfers to the other. The end state: + +``` +class PlasmaEffect { + uint8_t bpm; + uint8_t zoom; + + defineControls() { + addUint8("bpm", 30, 1, 240); + addUint8("zoom", 24, 1, 64); + } + + tick() { + for (y = 0; y < height; y = y + 1) { ... } + } +} +``` + +Step 7 of the previous plan tried to reach the same goal from the other end, by factoring the three +BINDINGS onto a shared base. It was superseded for two reasons, and both are what this plan is built +on. The code objected: the three bindings derive from three sibling bases under `MoonModule`, so a +shared base needs virtual inheritance and changes the layout of every module in the system to serve +three of them. And the direction objected: once a script defines NAMED ENTRY POINTS, the three +bindings stop being three kinds and become one kind with different entry points present, which is a +dispatch question rather than an inheritance one. The structure falls out at the end (step 5) instead +of being designed up front. + +## The enclosing class declaration + +`class PlasmaEffect { ... }` is the only top-level form. It makes the class semantics VISIBLE rather +than implied: without it a script merely behaves like a class and a reader has to be told, while with +it `defineControls` and `tick` stop looking like magic top-level names and read as what they are, +members the host calls. It also gives the engine a NAME that is not the filename, for the UI, the +status line and error messages, which matters the first time somebody renames a file. + +Two constraints on it: + +- **The filename loads it; the class name identifies it.** The `script` control still holds the file + name, because that is what the engine reads from the filesystem, and the class name is what the + status line and compile errors report. This is how a C translation unit works: `plasma.c` is what + you compile, and the diagnostics name the function inside it. Both can be renamed independently + without breaking the other. +- **The name is just a name.** Role does NOT come from the suffix. Inferring "this is an effect" from + `...Effect` is the kind of magic that surprises people the first time a rename changes behaviour; + the role comes from which entry points the class defines, which is step 5's dispatch model, and it + is also what lets one class define both `tick` and `modifyLogical`. +- **The declaration is MANDATORY.** One top-level form, not two. Optional was considered, on the + argument that `onered.mlv` is a single `setRGB` and a class around it is ceremony, and rejected: + it would keep a bare statement list in the grammar forever, which is a second parse path, a second + set of rules to document, and a second thing to test, permanently, so that a handful of two-line + scripts can stay two lines. That is more code to support less clarity, and the whole point of this + plan is that a reader can tell what a script is by looking at it. Nothing is released yet, so the + only cost is rewriting the shipped scripts once, which is our own work. + +## Decisions taken + +**The standard: a script IS a class.** Not "like" one loosely: it has members (script-level +variables) and functions, some of which the host calls from outside at known times (`tick`, +`defineControls`, `placeLights`). Syntax may be simplified, and semantics may be simplified where +that buys something, but where a reader has an expectation from any class-based language, the +behaviour meets it. Most of what follows is settled by asking "what would a class do". + +**Where script-level state lives.** The moment a script has more than +one function, a variable shared between them cannot live in a frame slot, because the frame belongs +to one call and dies with it. That is this plan's own premise ("the frame is where values live") +meeting the one case it does not cover. The same mechanism is what a variable persisting ACROSS +`tick()` calls needs, which is the stateful-effect family (fire, trails, decay) the language cannot +express at all today, so it is worth solving once rather than twice. + +The storage already exists: the CONTROL ARENA outlives every call, keeps a stable address across a +recompile, and is reachable from emitted code through `kArg4`. A script-level variable is close to +"a control the UI does not show", which is a good sign about the shape. What is missing is the rules, +and each is a real decision rather than a detail: + +- **Scope: SETTLED by the class model.** A variable declared outside any function is a member: + visible in every function, one per script instance. +- **Initialisation: SETTLED by the class model.** A member is initialised once when the object is + constructed, which here is compile time, seeded exactly as a declared control already is. Persisting + across `tick()` calls follows from that rather than needing a `setup()` entry point to explain it. +- **Width and TYPE: the open one, and the real work.** Arena slots are BYTES today, which is why a + coordinate clamps at 255 and a shift modifier cannot walk a light off a large grid. A member must be + able to be a scalar, a STRUCT (`Coord3D`), an ARRAY, or an array of structs. Not on day one, but the + storage has to be designed for it, which makes this a typed addressable region rather than a wider + row of bytes. Everything else here is downstream of this decision. +- **Cost.** Every access becomes an arena load rather than a frame slot read. Cheap at today's script + sizes, worth measuring before it is the default for every variable. + +The type question is settled before step 2 writes any of it, because `defineControls()` setting a +value that `tick()` reads is exactly this case, and finding the rules wrong late means rebuilding +whatever was laid on top of them. + +**Argument passing: by value for scalars, by reference for aggregates.** The ABI already does both +with one mechanism, so neither is a special case: arguments are staged in consecutive frame slots and +the callee receives a POINTER to that block, so a value argument is "copy the value into the slot" +and a reference argument is "put the address in the slot". + +Passing everything by reference was considered and rejected. It is not a simplification of the class +model but a departure from it: a script could then not have an ordinary scalar parameter, since +assigning to it would write through to the caller's variable, surprising in precisely the place this +design promises no surprises. It also collides with the host built-ins, which are compiled C +functions taking values (`sin`, `beat`, `scale`) and are not ours to change. The value/reference split +is what a contributor already predicts, and an explicit `&` can be added later without redesign. + +**One exec block, an offset per entry point.** A script compiles to a single allocation holding +every function it defines, and the engine records each named entry's offset within it; the binding +gets a function pointer to that offset. This is what a symbol table is, and what every compiler and +JIT does: one code section, a name-to-address map over it. + +The alternatives lose on specifics rather than on taste. A block per entry makes every +script-to-script call cross-allocation, so an ordinary relative call becomes an absolute address +fixed up at load, and it multiplies `allocExec` calls, which on ESP32 is scarce fragmenting IRAM. A +selector argument on one entry point puts a branch on every call, including `tick()` at 60 fps +forever, paying at run time for something known at compile time. With one block, a call between +script functions is just a call, and step 5's dispatch question ("which entry points does this +script define") is answered by which names are in the table. + +The offset recorded must be the address in the FINAL PLACED block, not in the staging buffer: +`writeExec` copies to a different address, and the single-entry path already accounts for this, so +this is the same rule applied per name. + +**Functions are REAL CALLS, and recursion works.** Not inlining, and not a later nicety. The +predecessor plan's own table lists recursion as something the stack machine BUYS ("a fixed slot file +cannot hold two activations" becomes "each activation gets its own frame"), which is one of the +reasons the rework happened at all. Inlining would satisfy `tick()` and nothing else: a recursive +function cannot be inlined, so choosing it would quietly drop the payoff. What real calls require: + +- **A frame per activation, at run time.** Today the emitted routine has ONE frame from one + `entry`/prologue, sized at compile time. A script function needs its own, so the prologue and the + frame-slot addressing become per function rather than per program. +- **On Xtensa, a nested `call8`.** Every activation therefore owes the 32-byte window-save reserve + the frame contract demands, and the structural checker has to see a script function's frame the + same way it sees the entry routine's. This is the one place where the ISA makes recursion cost + more than bookkeeping, and it is exactly the defect class that cost three days, so the checker + extension belongs to this step rather than to a follow-up. +- **A depth bound with a clean diagnostic.** An ESP32 render task has a fixed stack, so unbounded + recursion is a reset, which the robustness rule forbids. A general compile-time depth limit is not + possible, so this is a runtime guard that degrades visibly. + +**Inlining is NOT part of this.** It was proposed twice while writing this plan, first to keep the +hot path flat and then to get both answers at once, and it does not survive its own cost/benefit: + +- **What it saves is not the cost.** Inlining removes one call and one frame setup per call site, on + the order of a microsecond. MoonLive's time goes elsewhere: `ripples.mlv` ticks at 1695 us, nearly + all of it in ~15 HOST calls per cell into libm. Script-to-script calls are not the bottleneck and + are not on a path to becoming one. +- **What it costs is a pass.** A call graph, cycle detection over it, a size heuristic, and the + substitution itself: rewriting a callee's IR into the caller with members remapped, labels + renamed against collision, and arguments bound to caller expressions. Hundreds of lines in the + compile path, on every device, in a compiler where only one backend executes in tests. +- **Its failure mode is worse than its win.** Getting the analysis wrong inlines a mutually + recursive pair forever ("does it call itself" does not catch `a` calls `b` calls `a`), which hangs + the compiler on a device rather than reporting an error. + +So: real calls, always. It is the simplest thing that fully works, and it is what delivers recursion. +Inlining stays available as a pure optimisation if a measurement ever shows script-to-script calls +mattering, and it can be added then without changing any semantics, which is exactly why it does not +need to be decided now. + +Note this is orthogonal to argument passing. Value-for-scalars and reference-for-aggregates holds +whether or not a call is inlined: references are about how a callee REACHES its caller's data, while +recursion is about each activation owning its OWN locals. A recursive function still passes scalars +by value, and still needs a frame per activation; using references to avoid frames would make every +activation share one set of locals and corrupt itself. + +## Sequence + +Steps 1 to 5 are the SHAPE: how a script is written. Steps 6 to 10 are the VOCABULARY: what it can +say. The shape comes first so that every feature in the back half lands on finished ground rather +than being retrofitted into a language still moving underneath it. + +1. ✅ **The `class` declaration and script functions, together.** Done: the class form ships, every + script and test uses it, and a script now calls its own functions and itself. `crosshair.mlv` is + the shipped example, verified on all four boards. + + Originally: They are one change: making the + declaration mandatory means there is no bare-statement-list form left, so the grammar's new top + level is a class body, and a class body holds functions. `tick()` is the first named entry point + (an effect is the simplest case), and the shipped scripts convert in the same commit, because + there is nothing to fall back on. First rather than last: with one top-level form, everything + after it is written inside a class, and converting `moonlive/` twice would be the alternative. + Calls are real from the start, per the section above: a script calling its own function, and then + calling it recursively, is the acceptance test. + + **Recursion is not a feature beside local calls; it IS local calls.** A recursive call is a local + call whose target happens to be the running function, and the machine cannot tell the difference: + it allocates a frame, jumps, returns. The proviso is that every value lives in the callee's own + frame rather than a fixed location, which is exactly what the stack machine bought (the + predecessor plan's table: "a fixed slot file cannot hold two activations" becomes "each activation + gets its own frame"). So recursion is a TEST CASE for local calls, not separate work. + + Two things are not free, and both are robustness rather than mechanism. A runaway recursion costs + 176 bytes of stack per activation on Xtensa (48 for the host-call area + 84 for 21 slots + 32 for + the window reserve + alignment) against a 12 KB main-task stack, so it resets the device at + roughly 64 deep: that needs a counter, and 32 is a generous limit at 46% of the budget. And on + Xtensa each call8 rotates the register window, so past ~8 nested frames the hardware spills to the + stack; that is correct and automatic, and the 32-byte reserve already accounts for where the + spills land, so it costs memory traffic rather than correctness. + + **What local calls took, as built.** All four pieces landed: + + - **A script-call IR op.** `IrOp::CallScript`, carrying the callee's FUNCTION NUMBER. It first + carried the callee's IR index, which the spill pass invalidates: every index past its first + inserted Reload shifts, so the call named a position that no longer started a function. A + function number survives any rewrite of the ops. + - **A relative call in each assembler.** `callLabel(Label)` on all three, reusing the branch + fixup machinery with a discriminator (`FixKind::Call` on Xtensa, `Jal` on RISC-V, kind 2 on + arm64), because a call's displacement is encoded differently from a branch's. + - **Function-entry alignment, which was not foreseen.** Xtensa requires a 4-byte-aligned `entry` + : the toolchain rejects anything else outright ("unaligned entry instruction") and CALLn + encodes its target in 4-byte units, so an unaligned callee is not expressible. Instructions are + 2 or 3 bytes, so a function following another lands anywhere. `alignForEntry()` pads before + every prologue, which is what `.align 4` does in hand-written assembly. hpwit's `new-parser` + hits the same wall and leaves it unhandled, so this is the missing piece rather than a + workaround. + - **The depth guard**, in the CALLEE's prologue rather than at each call site: one copy per + function instead of one per call, emitted only when `hasScriptCall()`, so every shipped script + carries none of it. A refusing callee returns, so there is no branch-around at the call site + and no counter to restore across a call; the decrement lives in the one epilogue both paths + take. Measured: 9 instructions ≈ 25 bytes per function, one arena byte, and ~5-10% on a script + that calls (215-233µs vs 204µs for `crosshair.mlv` on the classic). + + The counter is a byte in the CONTROL ARENA (`kDepthSlot`, above the system variables), not a + C++ member: recursion happens entirely inside the emitted block with no C++ frame between + activations, and every function already holds the arena pointer, so this costs one byte and no + new argument. The host zeroes it before each run rather than trusting the block to unwind: a + script that hit the limit would otherwise leak a level and shrink every later frame's budget. + A stack-limit check (comparing `sp` against a bound) is the more canonical form and is cheaper + still, but needs a per-platform stack-bound source; worth revisiting if the guard ever shows up + in a profile. + + - **A bigger label and fixup table.** `kMaxLabels`/`kMaxFixups` were 16/32, sized when a script + was one routine, and each backend held its own private copy of both. A class allocates a label + per function on top of its loop and store labels, so `crosshair.mlv` exhausted the table and + failed with the generic "too large". Now `kAsmLabels`/`kAsmFixups` (48/96) in core, so the + three backends cannot drift into disagreeing about which scripts compile. + + **Cost: 640 bytes of STACK, and no flash.** Both tables are members of the assembler, which is + a local in `lowerWith`, which runs on the render task. Measured on the classic ESP32 image, + that frame went 480 -> 1120 bytes (4 per label, 8 per fixup), making it the largest on the + compile chain: 144 + 288 + 576 + 1120 = 2128 nested, 17% of the 12 KB main task. Flash is + unchanged, since these are stack arrays. Pinned by `the assembler stays small enough to + build on a render task`, a tripwire in entry counts rather than host bytes (the host's 64-bit + size_t makes its Fixup 16 bytes against the device's 8, so sizeof here overstates the device). + If a script ever needs more, the tables move to the heap beside the code buffer: which was + moved off the stack for exactly this reason: rather than the constants going up again. + + **Three traps, all found on hardware and none visible to the host suite:** + + - `IrProgram::swap()` did not swap the function table, so the spill pass's remapped boundaries + were discarded and the lowering opened a frame two ops early, mid-statement. + - A local call passed NO arguments. Each function's prologue parks buf/nLights/cpl/t/ctrls out of + the argument registers into its own frame, so a bare call left the callee parking garbage and + its first control read faulted at `EXCVADDR 0x9`. On Xtensa the arguments go in a10..a14, + because `call8` rotates the window by 8. + - The depth guard must be emitted AFTER the host arguments are parked. Both it and the epilogue + address the arena through the parked frame copy, and a refusing activation jumps straight to + the epilogue: a guard placed first makes the refusal read a slot nothing wrote (SIGBUS). + + The host backend cannot pin the argument-passing contract: its R0..R4 map onto the ABI argument + registers and `bl` leaves them alone, so removing the fix fails no test there while crashing an + S3. The boards are the only check for that class. + +2. ⬜ **Typed script-level members**, per *Where script-level state lives* above: a variable declared inside the + class but outside any function lives in the arena, is visible in every function, is initialised + once and survives every call. Scalars first, with the storage designed so a struct and an array + can follow without moving anything. This is what makes a stateful effect (fire, trails, decay) + expressible at all, so it is worth landing on its own and measuring before anything is built on + it. + +3. ⬜ **`defineControls()`, replacing the `// @control` comment.** A control is declared by calling + `addUint8("bpm", 30, 1, 240)` inside a `defineControls()` the script defines, the same call a + compiled module makes. Today's form is a COMMENT that changes behaviour, which is not C and does + not resemble the thing it imitates; the lexer's `ControlAnno` token and its capture path go away + with it. Comes after step 1 because it IS a function, and after step 2 because the control it + declares is a member. The shipped scripts and the three docs move with it. + +3b. ✅ **A frame per FUNCTION, not per program.** Done: each function emits its own prologue and + epilogue, the host arguments are parked per function (they were spilling into a frame that did + not exist yet, which was half the segfault), and the structural checker re-reads the frame at + every prologue rather than judging the block by its first. Verified by control: shrinking the + Xtensa reserve to 16 makes the checker fire, restoring it passes. + + Originally: The lowering emits one prologue before the first + op; each function needs its own, with the epilogue to match, so that its recorded offset is an + address a caller can actually jump to. Three parts, and the second is the one this project has + already paid for once: + + - **Prologue and epilogue per function**, sized from that function's own slots rather than the + program's total, which is also what makes each activation independent. + - **On Xtensa, every activation owes the 32-byte window-save reserve.** A script function calling + a built-in is a nested `call8`, so the frame contract applies to it exactly as to the entry + routine, and the structural checker has to see a script function's frame the same way it sees + the entry routine's. Extending the checker belongs to this step: it is the defect class that + cost three days, and it is silent when wrong. + - **The block start stops being the program.** With several functions in one block, falling off + the end of one into the next is a real hazard, so each function returns rather than running on. + +4. ✅ **The remaining entry points per role.** Done, and simplified by the moment model above: + layouts declare `placeLights`, modifiers `modifyLogical`, effects `tick`, and each binding runs + its moment IF the script defined it. `modifyLogicalTick` is not built; it is a new moment the + Layer would have to own, so it belongs with whatever needs it. + + Originally: once the mechanism holds: `forEachCoord`/`lightCount` + for a layout, `modifyLogical`/`modifyLogicalSize` for a modifier, plus `modifyLogicalTick` (a + per-drawn-light hook we never implemented; MoonLight has it, and it is what a dynamic rotation + modifier needs). + + **Every shipped script uses `tick()` until this step**, including the layouts and modifiers, which + is a way-station rather than the shape: a layout does not tick, it is ASKED how many lights it has + and where they are, and a modifier is asked to fold one coordinate. Naming both `tick` hides what + the host actually does with them. It is what step 1 could deliver while `tick` was the only entry + point in existence. + + **The byte-offset map is DONE** (landed with step 1). The parser records the IR index each + function starts at, the shared lowering converts it to a byte as it emits, and `CompileResult` + reports name plus offset: a symbol table over one code section. Pinned by a two-function test + whose second entry must start after the first, verified to FAIL when the map is stubbed back to + zero. `MoonLive::entry(name)` turns a name into a callable address. + + **But the map is necessary and not sufficient, which the code taught us by segfaulting.** Wiring + a binding to CALL its entry point crashes, because the lowering emits ONE prologue for the whole + program, before any function. An entry's recorded offset therefore points PAST the frame setup, + and calling it directly runs a routine whose frame was never established; the first frame access + faults. Per-function frames were sequenced after this step and belong before it: a named entry + point is not callable until each function owns its frame. That is now step 3b, and this step is + the wiring that follows it. + +**A NAME IS A MOMENT, NOT A ROLE** (PO, during step 4). The binding does not pick which entry point +belongs to its kind. The HOST owns moments and calls whatever the script defined for each: `tick` +when a frame renders, `placeLights` when lights are placed, `modifyLogical` when one coordinate is +folded. An entry a class did not define is simply not called. + +This is simpler than a per-role name in every direction. There is no selection, no fallback and no +"which name is mine" question; a binding checks whether the moment it owns is defined and runs it. +Nothing validates which names a class may use, which is what leaves the author in control and +responsible: a script that defines a name no moment calls has a function that does not run, and that +is visible immediately rather than silent. It is also what makes the stretch goal free rather than a +feature: an effect that also defines `modifyLogical` gets both, because it defined both. + +It settles step 5 before step 5 starts. The three bindings already differ only by which moments they +own, so there is no inheritance question left to answer, and `tick` stays available to mean something +in a layout or a modifier later without a grammar change. + +5. ⬜ **Consolidate the three bindings onto a HELD HELPER.** The design question this step existed + to answer is settled: the moment model above means the bindings no longer differ in behaviour, + only in which base they extend and which moment they own. What is left is measurable duplication, + and the shape it should take is now concrete rather than anticipated. + + **A `MoonLiveScript` MEMBER, not a shared base.** It owns `engine_`, `script_`, the compile-and- + report path and `defineControls`, and each binding holds one and forwards. A base class was + re-checked against the code and is still wrong for the same structural reason: the three derive + from three SIBLING bases under `MoonModule`, so a shared base needs virtual inheritance and would + change the object layout of every module in the system to serve three of them. A held member + needs no inheritance change at all. + + **What it removes, measured:** `defineControls` is already byte-identical in the layout and the + modifier, and the compile trunk is the same in all three; roughly 75 lines of ~537. What stays + per binding is ~20 lines of genuinely its own: the role virtuals (`placeLights`/`lightCount`, + `modifyLogical`/`modifyLogicalSize`, `tick`/`dimensions`) and the base-class call in `release`. + + **Its own change, with its own bench pass.** Not folded into the language work: these three files + currently work and are verified on hardware, and mixing a restructure into a grammar change means + a reviewer cannot tell which broke what. The test of whether it worked is the scripted DRIVER as + a fourth binding: if it needs more than the member plus its own moment, the factoring did not go + far enough. + +The shape is finished at that point. What follows decides whether MoonLive is an impressive +mechanism or a language people build with. + +6. ⬜ **`if` / `else`.** The single largest gap between what MoonLive can express and what an effect + IS. Today the language does smooth arithmetic over a grid (plasma, ripples, a gradient) and + nothing that branches, so fire, sparkles, particles, a boundary test, "respawn this one if it + died" are all unreachable. The stack machine already made this cheap (the predecessor plan's + table: "a branch over a region; storage is untouched"), and the emitter already has the + conditional branches the loops use. This is where the language stops being a demo. + +7. ⬜ **Reading a light back: `get(x, y)`.** One builtin, and an entire family of effects becomes + expressible: fire, decay, trails, blur feedback all work by reading what was drawn and modifying + it. The buffer already persists between frames, which is why every script begins with `fill` to + clear it, so the data is there and only the read is missing. Needs a decision on how a colour + comes back: three builtins (`red`/`green`/`blue`) or bit operators, which is the same question + the seven-argument `line()` answered for arguments and would answer once for both. + +8. ⬜ **Arrays, and arrays of structs.** Step 2 designs the storage for it; this is where it works. + A particle array is the difference between an effect that draws a formula and one that simulates + something, and it is what most of the effects people ask for are built on. Includes the arena + ceiling and its diagnostic: an array lets a script ask for more memory than a classic ESP32 has, + and the answer must be a clear compile error rather than a failed allocation at run time. + +9. ⬜ **Wider values than a byte.** Coordinates, members and arena slots are 8-bit, so a script + cannot address a 256-wide wall correctly, and a modifier cannot walk a light off a large grid. + This is a correctness wall on exactly the installations worth demonstrating on, and it touches + the same typed-storage decision as steps 2 and 8, so those three want to agree with each other. + +10. ⬜ **The editing loop, which is the thing people will actually see.** Editing a script means the + File Manager today: find the file, edit it, save it, then re-name it on the module. The demo is + live authoring, and that wants an editor on the module's own card, saving to the same file the + engine compiles. Tooling rather than language, and the last step because it is worth building + against the finished shape rather than twice. + +## Files + +Per step, the surface each touches. The pattern is that the FRONT END grows and the backends do not: +the shared lowering and the three assemblers are finished work, and a step that needs to change them +is a step whose design is wrong. + +- `src/core/moonlive/MoonLiveCompiler.cpp`: every step from 1 to 9 lands here first: the class body, + function definitions, the member symbol table, `if`, types wider than a byte. +- `src/core/moonlive/MoonLiveIr.h`: new ops as the language grows (a call to a SCRIPT function, a + conditional branch, a typed member load/store). +- `src/core/moonlive/moonlive_lower.h`: one arm per new IR op, and nothing else. Touching more than + that means an ISA fact leaked into the language. +- `src/core/moonlive/MoonLive.{h,cpp}`: the arena becomes typed storage (step 2) and gains its + ceiling (step 8); entry-point discovery lives here (step 1) for the bindings to consume. +- `src/light/moonlive/MoonLive{Effect,Layout,Modifier}.h`: call an entry point instead of running + the whole program (step 1), then collapse onto dispatch (step 5). +- `src/light/moonlive/MoonLiveBuiltins_light.h`: `addUint8` for step 3, `get`/`red`/`green`/`blue` + for step 7. +- `src/platform/esp32/moonlive_asm_xtensa.cpp` + `test/unit/core/moonlive_structural.inc`: step 1 + only: a per-function frame means a nested `call8`, so the frame contract and the checker that + enforces it both extend to script functions. +- `moonlive/**.mlv`: converted in step 1 (mandatory class) and again in step 3 (`defineControls`). +- `docs/moonmodules/light/MoonLive*.md`, `moonlive/README.md`: the language reference, which is what + a user reads; it moves with each step rather than at the end. +- `src/ui/`: step 10 only. + +## Verification + +The governing risk is unchanged from the predecessor plan and is what shapes all of this: **only the +host backend is EXECUTED by tests**, while the constraints that bite hardest are Xtensa's. Every step +therefore needs a host test that proves the semantics and a bench run that proves the encoding. + +1. ✅ **A script calling its own function, and then calling it recursively** (step 1). The recursion case + is the one that proves a frame per activation, and it is the acceptance test for the step. + Done: `a function the script calls can light pixels and read the script's controls`, `arguments + reach a function two calls deep`, and `a script function can call itself, each call keeping its + own values` in unit_moonlive_compiler.cpp, plus `every function in a class starts where a call + can reach it` per device backend. Each was control-checked by reverting its fix. +2. ✅ **The frame contract, extended to script functions** (step 1). The structural checker must refuse a + script function whose frame intrudes into the window-save reserve, and it must be shown FAILING on + a deliberately wrong frame before it is trusted: the same control that caught the original bug. + Done: the checker's case list gained a calling class and a recursive one, so every prologue it + walks now carries the argument reload and the depth guard. Control-checked by dropping + kWindowSaveReserve from the frame calculation, which fires the offset check as it should. The + derived reserve resisted the first attempt to break it, which is the anti-drift design working: + editing the static_assert alone changes nothing, because the value comes from the callx opcode. +3. **A member written by one function and read by another**, and a member that survives across + `tick()` calls (step 2). The second is what a stateful effect depends on and is not provable by + inspection. +4. **The same script at the host's real budget and a squeezed one renders identical pixels.** The + predecessor plan's technique, still the only way the register work is testable off hardware, and + every new construct has to keep passing it. +5. ✅ **Recursion depth degrades visibly** (step 1): a script that recurses without bound keeps the + device rendering rather than resetting it. Pinned by `a script that recurses without end keeps + rendering instead of resetting`, which also re-runs the script to prove the counter unwinds: a + leaked level per frame would silently shrink every later frame's budget. Verified on the classic + ESP32 (the tightest stack): `forever.mlv` ran 110 seconds continuously at 109 fps, no reset. + + NOT done as specified: the script does not REPORT an error. The refusal is silent, and what a + user sees is the picture being wrong where the recursion bottomed out. Reporting it needs a + channel from the emitted block back to the binding, which does not exist yet: worth having, and + left for the step that gives scripts a diagnostic path. +6. **An arena ceiling reports a compile error** (step 8), not a failed allocation at run time. +7. **The bench, on all four boards**, after each step: S3 and classic (Xtensa), P4 and S31 (RISC-V), + a scripted layout and a scripted effect. Exec-block sizes compared against the previous step, since + an unexplained jump is the cheapest signal that codegen went wrong. +8. **`collect_kpi.py` after step 2**, because members change how EVERY variable is accessed. That is + the one step where a hot-path regression is plausible, so it is measured rather than assumed. + +## Deliberately not in this plan + +- **Inlining**, per the decision above: it optimises what is not the cost, and its failure mode hangs + the compiler. +- **`while`, `break`, `continue`.** `for` and `if` cover what an effect does; the rest is language + completeness rather than expressiveness, and each one costs a grammar rule and a test surface. +- **Floating point.** The render path is integer by rule ([coding-standards](../../coding-standards.md)), + and the Xtensa classic has no FPU, so a float in a script would be a silent softfloat call per light. +- **A scripted DRIVER as the fourth role.** It is the honest test of step 5's dispatch, but it needs + the driver surface to be as settled as the other three are, and that is its own question. diff --git a/docs/metrics/repo-health.json b/docs/metrics/repo-health.json index 88b746db..ff4de606 100644 --- a/docs/metrics/repo-health.json +++ b/docs/metrics/repo-health.json @@ -1,13 +1,13 @@ { - "commit": "e9a7cca8", + "commit": "2f4c292f", "flash": { - "esp32": 1722656, - "esp32p4-eth": 1611632, + "esp32": 1726144, + "esp32p4-eth": 1615872, "esp32p4-eth-wifi": 1793760, - "esp32s3-n16r8": 1761424, + "esp32s3-n16r8": 1764928, "esp32s3-n8r8": 1753232, - "esp32s31": 2033152, - "desktop": 1156376, + "esp32s31": 2037728, + "desktop": 1157176, "esp32-16mb": 1714608, "esp32-eth": 1324816, "esp32-wrover": 1765504, @@ -15,8 +15,8 @@ }, "perf": { "desktop": { - "tick_us": 179, - "fps": 5586 + "tick_us": 133, + "fps": 7518 }, "esp32": { "tick_us": 2151, @@ -24,54 +24,54 @@ } }, "loc": { - "core": 17763, - "light": 24625, - "platform": 13583, + "core": 18392, + "light": 24653, + "platform": 13309, "ui": 6468, - "test": 42195, - "moondeck": 20830 + "test": 42534, + "moondeck": 20835 }, "comments": { "core": { - "lines": 6766, - "ratio": 0.414 + "lines": 7099, + "ratio": 0.42 }, "light": { - "lines": 9581, - "ratio": 0.43 + "lines": 9613, + "ratio": 0.431 }, "platform": { - "lines": 4768, - "ratio": 0.387 + "lines": 4727, + "ratio": 0.392 }, "ui": { "lines": 1670, "ratio": 0.274 }, "test": { - "lines": 7451, - "ratio": 0.204 + "lines": 7569, + "ratio": 0.205 }, "moondeck": { - "lines": 3353, + "lines": 3357, "ratio": 0.184 } }, "tests": { - "cases": 1345, + "cases": 1357, "scenarios": 23 }, "docs": { - "md_files": 178, - "md_lines": 24507, - "plans_files": 91, - "backlog_lines": 3654, + "md_files": 179, + "md_lines": 25049, + "plans_files": 92, + "backlog_lines": 3685, "lessons_lines": 503, "claude_md_lines": 135 }, "complexity": { - "functions": 2524, + "functions": 2538, "over_threshold": 158, - "worst_ccn": 105 + "worst_ccn": 108 } } diff --git a/docs/metrics/repo-health.md b/docs/metrics/repo-health.md index 610d36be..f62cb1fb 100644 --- a/docs/metrics/repo-health.md +++ b/docs/metrics/repo-health.md @@ -1,6 +1,6 @@ # Repo health -Measured at `e9a7cca8`. Generated by [`moondeck/check/repo_health.py`](../../moondeck/check/repo_health.py) on every KPI-gate run. **Do not edit by hand.** +Measured at `2f4c292f`. Generated by [`moondeck/check/repo_health.py`](../../moondeck/check/repo_health.py) on every KPI-gate run. **Do not edit by hand.** Current state only; the trend is this file's git history (`git log -p docs/metrics/repo-health.md`). Nothing here fails a build: the numbers make growth visible, the judgment stays human. @@ -8,59 +8,59 @@ Current state only; the trend is this file's git history (`git log -p docs/metri | Target | Flash | |---|---:| -| desktop | 1,129 KB | -| esp32 | 1,682 KB | +| desktop | 1,130 KB (+0 KB) ⚠ | +| esp32 | 1,686 KB (+3 KB) ⚠ | | esp32-16mb | 1,674 KB | | esp32-eth | 1,294 KB | | esp32-wrover | 1,724 KB | -| esp32p4-eth | 1,574 KB | +| esp32p4-eth | 1,578 KB (+2 KB) ⚠ | | esp32p4-eth-wifi | 1,752 KB | -| esp32s3-n16r8 | 1,720 KB (+0 KB) ⚠ | +| esp32s3-n16r8 | 1,724 KB (+3 KB) ⚠ | | esp32s3-n8r8 | 1,712 KB | -| esp32s31 | 1,986 KB | +| esp32s31 | 1,990 KB (+2 KB) ⚠ | | qemu | 1,287 KB | ## Render performance | Target | Tick | FPS | |---|---:|---:| -| desktop | 179 µs (+1 µs) ⚠ | 5,586 (−31) ⚠ | +| desktop | 133 µs (+1 µs) ⚠ | 7,518 (−57) ⚠ | | esp32 | 2,151 µs | 464 | ## Code | Area | Lines | Comments | Comment share | |---|---:|---:|---:| -| core | 17,763 (+6) ⚠ | 6,766 | 41.4 % | -| light | 24,625 (+11) ⚠ | 9,581 | 43.0 % | -| platform | 13,583 | 4,768 | 38.7 % | +| core | 18,392 (+247) ⚠ | 7,099 | 42.0 % (+0.4 %) ⚠ | +| light | 24,653 | 9,613 | 43.1 % | +| platform | 13,309 (+173) ⚠ | 4,727 | 39.2 % (+0.4 %) ⚠ | | ui | 6,468 | 1,670 | 27.4 % | -| test | 42,195 (+56) ⚠ | 7,451 | 20.4 % | -| moondeck | 20,830 (+22) ⚠ | 3,353 | 18.4 % | +| test | 42,534 (+113) ⚠ | 7,569 | 20.5 % | +| moondeck | 20,835 (+5) ⚠ | 3,357 | 18.4 % | ## Tests | Kind | Count | |---|---:| -| unit cases | 1,345 (+2) ✓ | +| unit cases | 1,357 (+5) ✓ | | scenarios | 23 | ## Complexity | Metric | Value | |---|---:| -| functions | 2,524 | +| functions | 2,538 (+7) ✓ | | over threshold | 158 | -| worst CCN | 105 | +| worst CCN | 108 (+3) ⚠ | ## Documentation | Metric | Value | |---|---:| -| markdown files | 178 | -| markdown lines | 24,507 (+10) ⚠ | -| plan files | 91 | -| backlog lines | 3,654 | +| markdown files | 179 | +| markdown lines | 25,049 (+77) ⚠ | +| plan files | 92 | +| backlog lines | 3,685 | | lessons lines | 503 | | CLAUDE.md lines | 135 | diff --git a/docs/moonmodules/light/MoonLiveEffect.md b/docs/moonmodules/light/MoonLiveEffect.md index d4d624a9..42e43a2f 100644 --- a/docs/moonmodules/light/MoonLiveEffect.md +++ b/docs/moonmodules/light/MoonLiveEffect.md @@ -4,14 +4,23 @@ MoonLive is projectMM's **live-script engine** — author an effect as text and Scripts call the same [power functions](power-functions.md) compiled effects use, reached through the builtin table — so the vocabulary is shared, in its flat scalar form. -A scripted effect names a **script file** under `/moonlive/`; the UI loads, edits and saves that file, and the module holds only the name (~32 bytes) — the text is read into a right-sized buffer to compile and freed immediately, so nothing script-sized stays resident. A front-end (lexer → parser → IR → per-ISA assembler) compiles it to native code on the next tick. The grammar is a sequence of **statements** — a function call, or a `for` loop over them — with **expression arguments**, so any argument may be a literal or a nested call: +A scripted effect names a **script file** under `/moonlive/`; the UI loads, edits and saves that file, and the module holds only the name (~32 bytes) — the text is read into a right-sized buffer to compile and freed immediately, so nothing script-sized stays resident. A front-end (lexer → parser → IR → per-ISA assembler) compiles it to native code on the next tick. + +**A script is a class.** It declares one, and the host calls its functions: an effect's `tick()` runs once per frame. That is the same shape a compiled effect has, so what a contributor learns from one transfers to the other. ``` -setRGB(random16(256), 0, 0, 255); // a random pixel, blue -setRGB(5, random16(256), 0, 0); // pixel 5, a random red -fill(0, 0, 255); // every light blue +class RandomPixelEffect { + tick() { + setRGB(random16(256), 0, 0, 255); // a random pixel, blue + setRGB(5, random16(256), 0, 0); // pixel 5, a random red + } +} ``` +Inside a function the grammar is a sequence of **statements** — a function call, or a `for` loop over them — with **expression arguments**, so any argument may be a literal or a nested call. The class declaration is required: one top-level form rather than two means one set of rules to learn and one parse path to maintain. + +The **class name is not the file name**. `plasma.mlv` may declare `class PlasmaEffect`; the file is what the engine loads, the class is what diagnostics and the module status report. Renaming either leaves the other alone, the same way a C translation unit and the functions inside it are independent. + The functions are **not built into the compiler** — `setRGB`, `fill`, `random16` are registered by the *host* (the light domain) in a builtin table; the core compiler owns only the grammar and a generic call/inline mechanism (the ESPLiveScript / ARTI bound-function model). The compiler emits machine code for whichever ISA the device runs (Xtensa on the classic/S3) or the host ISA on desktop, places it in executable memory, and the engine calls it each render tick. ## Controls @@ -20,28 +29,34 @@ The functions are **not built into the compiler** — `setRGB`, `fill`, `random1 - **Scripted controls** — a script declares a tunable variable with a range annotation, and the engine surfaces it as a real `uint8` MoonModule control (slider + UI + persistence), bound to a live value the running native code reads each tick: ```c - uint8_t speed = 50; // @control 0..99 → a "speed" slider, default 50, range 0..99 - uint8_t hue = 128; // @control 0..255 - setRGB(speed, hue, 0, 255); + class SpeedyEffect { + uint8_t speed = 50; // @control 0..99 → a "speed" slider, default 50, range 0..99 + uint8_t hue = 128; // @control 0..255 + + tick() { setRGB(speed, hue, 0, 255); } + } ``` + A declared variable sits in the class body, not inside a function: it is a member, which is what + lets the UI bind to it and what will let one function set a value another reads. + Declaring the variable is what **creates** the control: `uint8_t = ;` becomes a `` slider (default ``, range `0..255`). The trailing `// @control ..` only **adjusts that control's range**; it's optional. A declared name used in a statement reads the control's **current** value. Editing a control's slider does **not** recompile — the value lands in the engine's control-values arena and the next render tick reads it (the live-edit guarantee, the *no-reboot* principle). Saving the script file and re-naming it recompiles and re-derives the control set; a control kept across the edit keeps its slider value, a removed control's saved value drops. Stage 1 is `uint8` only. ### System variables — what the engine hands a script -Some names are **reserved**: the engine defines them, the script only reads them, and a declaration that reuses one is a compile error (`name is a system variable`). Each module supplies the names it actually writes, so a name a script cannot be given is simply unknown there rather than silently reading 0. +Some names are **reserved**: the engine defines them, the script only reads them, and a declaration that reuses one is a compile error (`name is a system variable`). **One vocabulary serves every role** — a name means the same thing in a layout, an effect and a modifier — so what you learn from one script transfers to the next. -| name | what it is | layout | effect | modifier | -|---|---|:-:|:-:|:-:| -| `t` | elapsed milliseconds — the clock an animation is written against | ✓ | ✓ | ✓ | -| `width`, `height`, `depth` | the **logical grid** the script renders into, `0..255` | | ✓ | ✓ | -| `x`, `y`, `z` | the light being transformed, `0..255` | | | ✓ | +| name | what it is | +|---|---| +| `t` | elapsed milliseconds — the clock an animation is written against | +| `width`, `height`, `depth` | the **logical grid**, `0..255` | +| `xPos`, `yPos`, `zPos` | the light being transformed, `0..255` (a [modifier](MoonLiveModifier.md) is the one handed these; elsewhere they read 0) | Every one but `t` is a byte, because it lives in the controls arena. A grid extent past 255 reports 255 rather than wrapping to a small number, and a modifier handed a coordinate outside `0..255` passes it through untransformed instead of folding a wrong position — so a script never silently sees a value that means something else. -Supplying a name is also what reserves it, so the tight lists are what leave `x` and `y` usable as ordinary loop counters in a layout or an effect — neither is handed a coordinate. +The coordinate is `xPos`/`yPos`/`zPos` rather than `x`/`y`/`z` so that **`x` and `y` stay free as loop counters in every script**, which is what an author reaches for and what the shipped `grid.mlv` uses. Reserving them globally would break the most ordinary code there is; a per-role reservation was the alternative and was worse, because a name then meant one thing in one role and was refused in another — which is how `disasm.py`, compiling against the widest vocabulary, came to refuse the shipped default layout. -`width`/`height`/`depth` are the Layer's own dimensions, derived from the layouts and the modifier chain. An effect is *told* its canvas rather than declaring it: a size restated as a control is a second answer that can disagree with the first, and a script that sets `width` to 16 on an 8×8 panel draws off the edge. A [layout](MoonLiveLayout.md) is upstream of that grid — it is what the dimensions are derived *from* — so it is not given them at all, and names its own controls instead (`cols`, `rows`). +`width`/`height`/`depth` are the Layer's own dimensions, derived from the layouts and the modifier chain. An effect is *told* its canvas rather than declaring it: a size restated as a control is a second answer that can disagree with the first, and a script that sets `width` to 16 on an 8×8 panel draws off the edge. A [layout](MoonLiveLayout.md) is upstream of that grid — it is what the dimensions are derived *from* — so it names its own controls instead (`cols`, `rows`) and reads the grid only if it has a use for it. Reserving is what makes the guarantee hold: without it a declaration would silently shadow the value the engine handed in, and the script would disagree with its layer with no error anywhere. @@ -69,6 +84,17 @@ Registered by the light domain, not built into the compiler (the core owns only `turn(n)` exists because a full revolution is 65536 — one past the largest number a script can write — and the grammar has no division. Without it, placing `n` points evenly on a circle is not expressible. +### The script's own functions + +A class may define functions beside its entry point and call them, including calling itself. `effects/crosshair.mlv` is the worked example: a `column()` and a `row()`, both called from `tick()`. + +These are real calls, not text pasted in by the compiler: the callee allocates its own frame when it runs, which is what lets one helper call another and what makes recursion work. A function takes no arguments and returns nothing yet, so a helper does a whole job rather than computing a value. + +Two rules a script author meets: + +- **Declare a helper above the function that calls it.** Only functions already parsed are visible, so a call to one declared further down reports `unknown function`. A function can always call itself. +- **Recursion is bounded.** About 30 calls deep a further call does nothing and returns, because a render task has a fixed stack and the alternative to a limit is a device that resets mid-frame. It is not reported: what you see is the picture being wrong where the recursion stopped, on a device that keeps running. + ### Wire contract — control declaration The controls are **derived from the script** (one per declared `uint8` control; the optional `@control` annotation only refines a control's range), then **surfaced in `/api/state`** — the device JSON view the integrator consumes — as regular `uint8` controls alongside `script`. So an integrator sees and writes them exactly like any other control — e.g. `POST /api/control` with `{"module": "ML", "control": "speed", "value": 80}`; they're fully present in the device JSON, just authored in the script rather than fixed in the module. The script's `\n` line breaks are standard JSON string escapes the device decodes, so a multi-line script round-trips through `/api/file`. diff --git a/docs/moonmodules/light/MoonLiveLayout.md b/docs/moonmodules/light/MoonLiveLayout.md index 9b85dd90..23bd6b93 100644 --- a/docs/moonmodules/light/MoonLiveLayout.md +++ b/docs/moonmodules/light/MoonLiveLayout.md @@ -11,17 +11,21 @@ A [layout](layouts.md) is the one part of the pipeline that differs for every ph The script places every light itself, with a loop. That is the difference from a scripted modifier: the Layer calls a modifier once per light, so its script transforms a single coordinate — a layout has no such per-light call to ride on. ```c -uint8_t cols = 16; // @control 1..64 -uint8_t rows = 16; // @control 1..64 - -for (y = 0; y < rows; y = y + 1) { - for (x = 0; x < cols; x = x + 1) { - addLight(x, y, 0); +class GridLayout { + uint8_t cols = 16; // @control 1..64 + uint8_t rows = 16; // @control 1..64 + + placeLights() { + for (y = 0; y < rows; y = y + 1) { + for (x = 0; x < cols; x = x + 1) { + addLight(x, y, 0); + } + } } } ``` -That is the default: a plain grid, one light per cell. `addLight(x, y, z)` places the next light along the strand — no index, because the order the script calls it in *is* the strand order. +That is the default: a plain grid, one light per cell. The function is named `placeLights` because that is the moment a layout is asked about: the module calls it when the fixture is being built, and a script that does not define it places nothing. An effect's moment is `tick`, a modifier's is `modifyLogical`, and a class may define any of them. `addLight(x, y, z)` places the next light along the strand — no index, because the order the script calls it in *is* the strand order. The `cols` and `rows` lines are the script's own controls, not something the module hands it. A layout is never told how big it is: the pipeline works out the bounding box from the coordinates the layouts actually place, so a size passed in from outside would be a second answer that could disagree with the first. diff --git a/docs/moonmodules/light/MoonLiveModifier.md b/docs/moonmodules/light/MoonLiveModifier.md index 014a5384..3540e764 100644 --- a/docs/moonmodules/light/MoonLiveModifier.md +++ b/docs/moonmodules/light/MoonLiveModifier.md @@ -11,10 +11,19 @@ A [modifier](modifiers.md) reshapes how a Layer's output maps onto the physical The script transforms **one coordinate**. It needs no loop over the lights, because the Layer already does that: it calls the script once per physical light while it builds its mapping. (A `for` is available if the arithmetic wants one — it just is not how the script reaches the next light.) ```c -setXYZ(0, width - 1 - x, y, z); // mirror along x -setXYZ(0, y, x, z); // swap the axes -setXYZ(0, x + 4, y, z); // shift by four -setXYZ(0, (width - 1 - x) * 2, y, z); // mirror, then stretch +class MirrorModifier { + modifyLogical() { setXYZ(0, width - 1 - xPos, yPos, zPos); } // mirror along x +} +``` + +The function is named `modifyLogical` because that is the moment a modifier is asked about: the Layer calls it once per light while building its mapping, and a script that does not define it passes every light through unchanged. An effect's moment is `tick`, a layout's is `placeLights`. + +The body is one expression per axis. Other shapes, in the same place: + +```c +setXYZ(0, yPos, xPos, zPos); // swap the axes +setXYZ(0, xPos + 4, yPos, zPos); // shift by four +setXYZ(0, (width - 1 - xPos) * 2, yPos, zPos); // mirror, then stretch ``` `setXYZ(index, x, y, z)` writes the transformed position, mirroring `setRGB(index, r, g, b)`. The index is the destination slot: today the script is handed a single coordinate, so it is always `0`. @@ -27,7 +36,7 @@ setXYZ(0, (width - 1 - x) * 2, y, z); // mirror, then stretch ### Seeing inside a script -`print(v)` logs a value and returns it, so it wraps any part of an expression: `setXYZ(0, print(width - 1 - x), y, z)`. +`print(v)` logs a value and returns it, so it wraps any part of an expression: `setXYZ(0, print(width - 1 - xPos), yPos, zPos)`. It is for debugging and comes back out again — [what print costs](../../../moonlive/README.md#debugging-print). ## Limits diff --git a/docs/performance.md b/docs/performance.md index 953230d0..e78f9b07 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -244,7 +244,15 @@ The rows above are a dated S3 bench record; the numbers below them are what a de The exec block is the emitted machine code, so it varies by ISA (the RISC-V rows above are the larger encoding); the tick is the native loop. `lines.mlv` is the cheapest of the three because `line()` moves the per-cell loop out of emitted code and into the shared `draw::line`, which is the argument for adding power functions as builtins rather than writing them in script. -**Desktop tick across this cycle:** 150 → 122 µs (6666 → 8196 fps), measured by `collect_kpi.py --commit` at each commit. The gain is not from MoonLive — it tracks the two heap-overrun fixes and the register-reuse work landing earlier in the branch. No scenario `contract` was renegotiated on this branch: all 20 scenarios pass inside their existing budgets, which is the assertion surface this page defers to. +**A script calling its own functions** costs what the call costs, and nothing when a script makes none. `crosshair.mlv` (three functions, two calls per frame) ticks at 219 µs on the classic against 204 µs for the same drawing without the recursion guard, so roughly 5-10% on a script that calls. A script with no local call emits no guard code at all, so every shipped script is byte-identical to before: the guard is nine instructions in a function's prologue, emitted only when the program contains a `CallScript`. + +The **depth guard** is one arena byte, incremented on entry and decremented in the epilogue. A refused call returns rather than the caller branching around it, which is why the cost sits in the callee and not at every call site. Unbounded recursion therefore degrades instead of resetting: the classic ran a deliberately non-terminating script for 110 s at 109 fps, with the deepest calls doing nothing. + +**Desktop tick across this cycle:** 150 → 133 µs (6666 → 7518 fps), measured by `collect_kpi.py --commit` at each commit. The gain is not from MoonLive — it tracks the two heap-overrun fixes and the register-reuse work landing earlier in the branch. No scenario `contract` was renegotiated on this branch: all 20 scenarios pass inside their existing budgets, which is the assertion surface this page defers to. + +**Flash**, measured by building the classic at the branch point and again with the local-call work: 1723295 → 1726027 bytes, +2732 (+0.16%). High per line of source (about 20 bytes for ~137 net lines of code) because nearly all of it is emitter code instantiated once per backend, so one line of the shared lowering becomes three copies of emitted-instruction sequences in the image. + +**The compile path's stack grew 640 bytes.** `kAsmLabels`/`kAsmFixups` went from 16/32 to 48/96 because a class allocates a label per function, so `lowerWith`'s frame went 480 → 1120 bytes on the classic — the largest on the chain (144 + 288 + 576 + 1120 = 2128 nested, 17% of the 12 KB main task). It is a compile-path local, not a per-tick cost, but the compile runs on the render task. --- diff --git a/docs/usecases/build-your-own-moonmodules.md b/docs/usecases/build-your-own-moonmodules.md index f38478ae..4337caea 100644 --- a/docs/usecases/build-your-own-moonmodules.md +++ b/docs/usecases/build-your-own-moonmodules.md @@ -265,7 +265,7 @@ Effects are the on-ramp. The same "fill in the hooks, let the core orchestrate" ### Layouts — where the pixels are -A **layout** answers one question: *for light number N, where is it in 3D space?* You override `forEachCoord`, which walks every light and reports its `(x, y, z)`. A grid is the classic example: +A **layout** answers one question: *for light number N, where is it in 3D space?* You override `placeLights`, which walks every light and reports its `(x, y, z)`. A grid is the classic example: ```cpp class GridLayout : public LayoutBase { @@ -280,7 +280,7 @@ public: nrOfLightsType lightCount() const override { return width * height; } - void forEachCoord(CoordCallback cb, void* ctx) const override { + void placeLights(CoordCallback cb, void* ctx) const override { nrOfLightsType idx = 0; for (lengthType y = 0; y < height; y++) for (lengthType x = 0; x < width; x++) @@ -321,6 +321,6 @@ You get all of that "release the pin on disable" behaviour by implementing the s - **The effects catalog:** [docs/moonmodules/light/effects.md](../moonmodules/light/effects.md) — every shipped effect, with screenshots and controls. The best source of copy-and-tweak starting points. - **The architecture doc:** [docs/architecture.md](../architecture.md) — the render pipeline (Layouts → Effects → Layer → Effect/Modifier → Drivers) and the hot-path rules (why we avoid heap and floats inside `tick()`). - **Coding standards:** [docs/coding-standards.md](../coding-standards.md) — the house style (header-only light modules, `constexpr`, naming) so your module reads like the rest. -- **The real modules:** the smallest ones make the best teachers — `RainbowEffect` (a clean loop), `GameOfLifeEffect` (the memory lifecycle), `GridLayout` (`forEachCoord`). +- **The real modules:** the smallest ones make the best teachers — `RainbowEffect` (a clean loop), `GameOfLifeEffect` (the memory lifecycle), `GridLayout` (`placeLights`). The recurring lesson across all of them: **keep your module about what it does.** Declare your controls, draw or transform in the hook, allocate-in-`prepare`/free-in-`release` if you hold memory — and let the core decide when any of it runs. That discipline is what keeps a large, multi-platform light engine understandable one small module at a time. diff --git a/moondeck/MoonDeck.md b/moondeck/MoonDeck.md index 4222c024..dcb8f21f 100644 --- a/moondeck/MoonDeck.md +++ b/moondeck/MoonDeck.md @@ -96,6 +96,18 @@ uv run moondeck/check/check_specs.py Scans `src/` for MoonModule `.h` files and checks each has a `docs/moonmodules/*.md` page whose control names / source facts still agree with the header. The always-run commit gate (fast, <1s) — catches `.h` ↔ doc drift even on doc-only commits. +### check_prose + +Verify that prose a change ADDS follows the coding standards: no em-dashes, US spelling. + +```bash +uv run moondeck/check/check_prose.py +``` + +Reads the added lines of the branch diff and the working tree, so pre-existing prose a rename +merely touched is out of scope. Run by hand: the tree still holds instances that predate the +check, so it is not in the gate table until those are swept. + ### check_platform_boundary Verify that platform-specific code stays inside `src/platform/`. diff --git a/moondeck/check/check_prose.py b/moondeck/check/check_prose.py new file mode 100755 index 00000000..aa1b9dae --- /dev/null +++ b/moondeck/check/check_prose.py @@ -0,0 +1,105 @@ +#!/usr/bin/env -S uv run --script +"""Prose rules the coding standards state and nothing enforced: no em-dashes, US spelling. + +Both rules were written down and then broken repeatedly, in the same commits that swept them +out of other files, because they are habits rather than decisions. A habit is not fixed by +intending to do better; it is fixed by a check that fails. + +ADDED LINES ONLY. Pre-existing prose is not this check's business: rewriting a sentence a +change merely touched is churn that buries the actual diff, and the standards apply to new +prose. A line that only moved, or whose only edit was a rename, keeps whatever it had. + +RUN BY HAND, deliberately not in the gate table. The tree still holds pre-existing instances in +CLAUDE.md, docs/ and files a rename touched, so registering this today would fail every commit +until a sweep that has nothing to do with the change being committed. Register it once that +sweep has landed. + + uv run moondeck/check/check_prose.py +""" + +import re +import subprocess +import sys + +# Files whose prose the standards govern. Not .json or .txt: generated or data. +SUFFIXES = (".h", ".hpp", ".c", ".cpp", ".inc", ".md", ".py", ".js", ".css", ".html") + +# Paths exempt, with the reason each earns it. +EXEMPT = ( + "docs/history/", # the record of what was written then; rewriting it falsifies it + "docs/backlog/", # same: prior-project digests quoted from their sources + "docs/metrics/", # generated + "docs/tests/", # generated from test comments (fix the test, not the page) + "docs/moonmodules/", # partly generated technical pages +) + +# The banned character, by CODEPOINT rather than as a literal. Written literally, a sweep that +# rewrites em-dashes in this repo edits the detector itself: one such pass turned this into a +# comma and the check then flagged every comma in the tree. The en-dash (U+2013) and the arrow +# (U+2192) are NOT banned, so the test is this one codepoint and nothing else. +EM_DASH = "\u2014" + +# British to American. Substring matches, so a stem covers its inflections. +SPELLING = { + "behaviour": "behavior", "colour": "color", "initialis": "initializ", + "optimis": "optimiz", "recognis": "recogniz", "analys": "analyz", + "materialis": "materializ", "normalis": "normaliz", "serialis": "serializ", + "cancelled": "canceled", "modelling": "modeling", "labelled": "labeled", + "centre": "center", "licence": "license", "defence": "defense", +} + + +def added_lines(base): + """Every line this branch or working tree ADDS, as (path, line text).""" + diff = subprocess.run( + ["git", "diff", base, "--unified=0"], capture_output=True, text=True + ).stdout + path, out = None, [] + for line in diff.split("\n"): + if line.startswith("+++ b/"): + path = line[6:] + elif line.startswith("+") and not line.startswith("+++") and path: + out.append((path, line[1:])) + return out + + +def main(): + # Against main when on a branch, else the working tree: the check means "what am I adding". + base = "main...HEAD" if len(sys.argv) < 2 else sys.argv[1] + if subprocess.run(["git", "rev-parse", "--verify", "main"], + capture_output=True).returncode != 0: + base = "HEAD" + + findings = [] + for path, text in added_lines(base) + added_lines("HEAD"): + if not path.endswith(SUFFIXES) or path.startswith(EXEMPT): + continue + if EM_DASH in text: + # Quote AROUND the offending character, not the head of the line. A long line + # truncated at 96 characters hides it and shows an arrow or a hyphen instead, which + # reads as a false positive and teaches the reader to distrust the check. + at = text.index(EM_DASH) + findings.append((path, "em-dash", text[max(0, at - 40):at + 40].strip())) + low = text.lower() + for brit, amer in SPELLING.items(): + if brit in low: + findings.append((path, f"{brit} -> {amer}", text.strip()[:96])) + break + + # The same line can arrive from both diffs; report each once. + findings = sorted(set(findings)) + if not findings: + print("Prose check: no em-dashes or British spellings in added lines.") + return 0 + + print(f"Prose check: {len(findings)} issue(s) in ADDED lines.\n") + for path, what, text in findings: + print(f" {path}: {what}") + print(f" {text}") + print("\nAn em-dash reads as a habit rather than a choice: use a colon for an explanation,") + print("commas or parentheses for an aside, or a full stop for two independent clauses.") + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/moondeck/moonlive/check_encodings.py b/moondeck/moonlive/check_encodings.py index c0cc09ed..2f3546b0 100644 --- a/moondeck/moonlive/check_encodings.py +++ b/moondeck/moonlive/check_encodings.py @@ -60,6 +60,11 @@ # Control flow. `bltu`'s displacement is a single SIGNED byte (+/-127), the field that # truncated silently on a long loop body before branch relaxation. ("callx8 a8", "e00800", "call: the windowed call"), + # The script-to-script call. The offset field is patched later, so what is pinned here + # is the OPCODE: a wrong low-six-bits would decode as an unrelated instruction, and the + # first version of this used 0x25 in the wrong byte position. Assembled at a known pc + # with a known target so the displacement is reproducible. + ("call8 . - 4", "a5ffff", "call: a function in this block, by label"), ("retw.n", "1df0", "epilogue"), ("l8ui a2, a11, 8", "220b08", "LoadCtrl: read a control byte from the arena"), ("s8i a3, a12, 0", "324c00", "StoreElem: write one channel"), diff --git a/moonlive/README.md b/moonlive/README.md index 5f608f23..e7deb56b 100644 --- a/moonlive/README.md +++ b/moonlive/README.md @@ -1,14 +1,43 @@ # MoonLive scripts Scripts for the [MoonLive](../docs/moonmodules/light/MoonLiveEffect.md) engine, one file per script, -grouped by the module that runs it. Paste one into a module's `source` control on a running device -and it compiles to native code on the next tick. +grouped by the module that runs it. Name one in a module's `script` control on a running device and +it compiles to native code on the next tick. + +Each script declares a **class**, and the host calls its functions: `tick()` for an effect, +`placeLights()` for a layout, `modifyLogical()` for a modifier. A function is called when it is +present and its moment arrives, so which entry points a class defines is what decides what it does. +The class name is independent of the file name, the way a C file and the functions in it are. + +A class may also define functions of its own and **call them**, including calling itself: + +``` +class CrosshairEffect { + uint8_t bpm = 30; // @control 1..240 + + column() { for (y = 0; y < height; y = y + 1) { setRGB(y * width + scale(beat(bpm, t), width), 255, 40, 0); } } + tick() { fill(0, 0, 0); column(); } +} +``` + +These are real calls, not pasted-in text: the callee gets its own frame when it runs, which is what +lets one helper call another and lets a function recurse. A function takes no arguments and returns +nothing yet, so a helper does a whole job rather than computing a value. `effects/crosshair.mlv` is +the worked example. + +**Declare a helper above the function that calls it.** Only functions already parsed are visible, so +a call to one declared further down reports `unknown function`. A function can always call itself. + +**Recursion is bounded.** About 30 calls deep, a further call does nothing and returns. A render +task has a fixed stack, so the alternative to a limit is a device that resets mid-frame. What you +see if you hit it is the picture being wrong where the recursion stopped, on a device that keeps +running. Nothing is reported; the exact depth is `kMaxCallDepth`. | folder | run by | a script writes | |---|---|---| | `layouts/` | [MoonLiveLayout](../docs/moonmodules/light/MoonLiveLayout.md) | where the lights physically are — `addLight(x, y, z)` | | `effects/` | [MoonLiveEffect](../docs/moonmodules/light/MoonLiveEffect.md) | a colour per light: `setRGB(index, r, g, b)`, or a whole shape at once with `line(x1, y1, x2, y2, r, g, b)` | -| `modifiers/` | [MoonLiveModifier](../docs/moonmodules/light/MoonLiveModifier.md) | where one light lands — `setXYZ(0, x, y, z)` | +| `modifiers/` | [MoonLiveModifier](../docs/moonmodules/light/MoonLiveModifier.md) | where one light lands: `setXYZ(0, xPos, yPos, zPos)` | Each module ships one of these as its default, so the folder doubles as the reference for what a working script looks like. diff --git a/moonlive/effects/crosshair.mlv b/moonlive/effects/crosshair.mlv new file mode 100644 index 00000000..2c85f271 --- /dev/null +++ b/moonlive/effects/crosshair.mlv @@ -0,0 +1,31 @@ +// Crosshair: a red column and a blue row sweeping the grid, each drawn by a function the script +// defines for itself. +// +// The point of this script is the helpers. `column()` and `row()` are the script's own functions, +// called from `tick()`. Each is a REAL call: the callee allocates its own frame when it runs, which +// is what lets one helper call another, and eventually itself. Nothing is pasted in by the compiler. +// +// A script function takes no arguments and returns nothing yet, so a helper here does a whole job +// rather than computing a value. Parameters and members that a caller can set are the next steps; +// when they arrive, the shape of this script does not change, the helpers just get shorter. +class CrosshairEffect { + uint8_t bpm = 30; // @control 1..240 + + column() { + for (y = 0; y < height; y = y + 1) { + setRGB(y * width + scale(beat(bpm, t), width), 255, 40, 0); + } + } + + row() { + for (x = 0; x < width; x = x + 1) { + setRGB(scale(beat(bpm + 7, t), height) * width + x, 0, 120, 255); + } + } + + tick() { + fill(0, 0, 0); + column(); + row(); + } +} diff --git a/moonlive/effects/gradient.mlv b/moonlive/effects/gradient.mlv index 8cb47dd2..67809eb7 100644 --- a/moonlive/effects/gradient.mlv +++ b/moonlive/effects/gradient.mlv @@ -1,5 +1,10 @@ // A gradient painted by a loop: red rises across the strand while blue falls. // The script that first proved `for` reaches the emitter with a distinct value each pass. -for (i = 0; i < 256; i = i + 1) { - setRGB(i, i, 255 - i, 60); + +class GradientEffect { + tick() { + for (i = 0; i < 256; i = i + 1) { + setRGB(i, i, 255 - i, 60); + } + } } diff --git a/moonlive/effects/lines.mlv b/moonlive/effects/lines.mlv index e2193370..cb1caaf4 100644 --- a/moonlive/effects/lines.mlv +++ b/moonlive/effects/lines.mlv @@ -4,14 +4,19 @@ // line(x1, y1, x2, y2, r, g, b) is the seven-argument draw builtin: the script names the // endpoints and the shared draw::line walks the pixels, replacing the per-cell loop this // script used to spell out. -uint8_t bpm = 30; // @control 1..240 -fill(0, 0, 0); +class LinesEffect { + uint8_t bpm = 30; // @control 1..240 -line(scale(beat(bpm, t), width), 0, - scale(beat(bpm, t), width), height - 1, - 255, 0, 0); + tick() { + fill(0, 0, 0); -line(0, scale(beat(bpm, t), height), - width - 1, scale(beat(bpm, t), height), - 0, 255, 0); + line(scale(beat(bpm, t), width), 0, + scale(beat(bpm, t), width), height - 1, + 255, 0, 0); + + line(0, scale(beat(bpm, t), height), + width - 1, scale(beat(bpm, t), height), + 0, 255, 0); + } +} diff --git a/moonlive/effects/plasma.mlv b/moonlive/effects/plasma.mlv index 10be8a2e..85e11cd0 100644 --- a/moonlive/effects/plasma.mlv +++ b/moonlive/effects/plasma.mlv @@ -5,15 +5,20 @@ // The heaviest script that ships: a nested loop over the whole grid with nine host calls per // cell (3 beat, 2 sin, 1 cos, 3 scale) feeding one setRGB. `beat(bpm, t)` reads the clock // through the same path an effect always does. -uint8_t bpm = 12; // @control 1..120 -uint8_t zoom = 24; // @control 1..64 -for (y = 0; y < height; y = y + 1) { - for (x = 0; x < width; x = x + 1) { - // Each axis gets its own wave, offset by the beat so the pattern travels diagonally. - setRGB(y * width + x, - scale(sin(x * zoom * 8 + beat(bpm, t)), 256), - scale(sin(y * zoom * 8 + beat(bpm, t)), 256), - scale(cos((x + y) * zoom * 4 + beat(bpm, t)), 256)); +class PlasmaEffect { + uint8_t bpm = 12; // @control 1..120 + uint8_t zoom = 24; // @control 1..64 + + tick() { + for (y = 0; y < height; y = y + 1) { + for (x = 0; x < width; x = x + 1) { + // Each axis gets its own wave, offset by the beat so the pattern travels diagonally. + setRGB(y * width + x, + scale(sin(x * zoom * 8 + beat(bpm, t)), 256), + scale(sin(y * zoom * 8 + beat(bpm, t)), 256), + scale(cos((x + y) * zoom * 4 + beat(bpm, t)), 256)); + } + } } } diff --git a/moonlive/effects/random-pixel.mlv b/moonlive/effects/random-pixel.mlv index 4a546814..48a1ab10 100644 --- a/moonlive/effects/random-pixel.mlv +++ b/moonlive/effects/random-pixel.mlv @@ -1,3 +1,8 @@ // One random light in a random colour per frame. The shipped default: always visibly alive, // and the smallest script that shows the engine running. -setRGB(random16(256), random16(256), random16(256), random16(256)); + +class RandomPixelEffect { + tick() { + setRGB(random16(256), random16(256), random16(256), random16(256)); + } +} diff --git a/moonlive/effects/ripples.mlv b/moonlive/effects/ripples.mlv index 14a1f97d..71c87b8c 100644 --- a/moonlive/effects/ripples.mlv +++ b/moonlive/effects/ripples.mlv @@ -6,18 +6,23 @@ // // The heaviest shipped script: ~15 host calls per cell against plasma's 9, so it is also // the working stress test for the call path. -uint8_t bpm = 10; // @control 1..120 -uint8_t rings = 8; // @control 1..32 -for (y = 0; y < height; y = y + 1) { - for (x = 0; x < width; x = x + 1) { - setRGB(y * width + x, - scale(sin(((x - beatsin(bpm, t, width - 1)) * (x - beatsin(bpm, t, width - 1)) - + (y - beatsin(bpm + 4, t, height - 1)) * (y - beatsin(bpm + 4, t, height - 1))) - * rings * 64), 256), - scale(sin(((x - beatsin(bpm + 7, t, width - 1)) * (x - beatsin(bpm + 7, t, width - 1)) - + (y - beatsin(bpm + 3, t, height - 1)) * (y - beatsin(bpm + 3, t, height - 1))) - * rings * 64), 256), - scale(sin((x + y) * rings * 32 + beat(bpm, t)), 256)); +class RipplesEffect { + uint8_t bpm = 10; // @control 1..120 + uint8_t rings = 8; // @control 1..32 + + tick() { + for (y = 0; y < height; y = y + 1) { + for (x = 0; x < width; x = x + 1) { + setRGB(y * width + x, + scale(sin(((x - beatsin(bpm, t, width - 1)) * (x - beatsin(bpm, t, width - 1)) + + (y - beatsin(bpm + 4, t, height - 1)) * (y - beatsin(bpm + 4, t, height - 1))) + * rings * 64), 256), + scale(sin(((x - beatsin(bpm + 7, t, width - 1)) * (x - beatsin(bpm + 7, t, width - 1)) + + (y - beatsin(bpm + 3, t, height - 1)) * (y - beatsin(bpm + 3, t, height - 1))) + * rings * 64), 256), + scale(sin((x + y) * rings * 32 + beat(bpm, t)), 256)); + } + } } } diff --git a/moonlive/layouts/diagonal.mlv b/moonlive/layouts/diagonal.mlv index 335da1a6..42fba6a6 100644 --- a/moonlive/layouts/diagonal.mlv +++ b/moonlive/layouts/diagonal.mlv @@ -1,6 +1,11 @@ // A diagonal run — light i at (i, i). The kind of fixture that otherwise needs its own class. -uint8_t count = 16; // @control 1..64 -for (i = 0; i < count; i = i + 1) { - addLight(i, i, 0); +class DiagonalLayout { + uint8_t count = 16; // @control 1..64 + + placeLights() { + for (i = 0; i < count; i = i + 1) { + addLight(i, i, 0); + } + } } diff --git a/moonlive/layouts/grid.mlv b/moonlive/layouts/grid.mlv index 37977c85..1b97b395 100644 --- a/moonlive/layouts/grid.mlv +++ b/moonlive/layouts/grid.mlv @@ -1,10 +1,15 @@ // A grid, the layout almost every panel is. // `cols`/`rows` are this layout's own controls; the logical grid comes from what it places. -uint8_t cols = 16; // @control 1..64 -uint8_t rows = 16; // @control 1..64 -for (y = 0; y < rows; y = y + 1) { - for (x = 0; x < cols; x = x + 1) { - addLight(x, y, 0); +class GridLayout { + uint8_t cols = 16; // @control 1..64 + uint8_t rows = 16; // @control 1..64 + + placeLights() { + for (y = 0; y < rows; y = y + 1) { + for (x = 0; x < cols; x = x + 1) { + addLight(x, y, 0); + } + } } } diff --git a/moonlive/layouts/lattice.mlv b/moonlive/layouts/lattice.mlv index 5032e0e6..8356396e 100644 --- a/moonlive/layouts/lattice.mlv +++ b/moonlive/layouts/lattice.mlv @@ -2,14 +2,19 @@ // `z` is an ordinary axis to a layout -- the shipped 2D layouts simply pass 0 for it. // Three nested loops need more registers than Xtensa has, so this runs on P4/S31/desktop // but not the S3; two loops (grid.mlv) fit everywhere. -uint8_t cols = 4; // @control 1..32 -uint8_t rows = 3; // @control 1..32 -uint8_t layers = 5; // @control 1..32 -for (z = 0; z < layers; z = z + 1) { - for (y = 0; y < rows; y = y + 1) { - for (x = 0; x < cols; x = x + 1) { - addLight(x, y, z); +class LatticeLayout { + uint8_t cols = 4; // @control 1..32 + uint8_t rows = 3; // @control 1..32 + uint8_t layers = 5; // @control 1..32 + + placeLights() { + for (z = 0; z < layers; z = z + 1) { + for (y = 0; y < rows; y = y + 1) { + for (x = 0; x < cols; x = x + 1) { + addLight(x, y, z); + } + } } } } diff --git a/moonlive/layouts/reversed-row.mlv b/moonlive/layouts/reversed-row.mlv index f6d998b2..3b451f64 100644 --- a/moonlive/layouts/reversed-row.mlv +++ b/moonlive/layouts/reversed-row.mlv @@ -1,6 +1,11 @@ // A strand wired right to left: light 0 sits at the far end. -uint8_t cols = 16; // @control 1..64 -for (i = 0; i < cols; i = i + 1) { - addLight(cols - 1 - i, 0, 0); +class ReversedRowLayout { + uint8_t cols = 16; // @control 1..64 + + placeLights() { + for (i = 0; i < cols; i = i + 1) { + addLight(cols - 1 - i, 0, 0); + } + } } diff --git a/moonlive/layouts/ring.mlv b/moonlive/layouts/ring.mlv index c2f04d48..73c63390 100644 --- a/moonlive/layouts/ring.mlv +++ b/moonlive/layouts/ring.mlv @@ -1,10 +1,15 @@ // A circle: `count` lights evenly around a centre, spanning 2*radius+1 cells. // Lights and grid cells differ -- 24 lights in an 11x11 box. // `cos`/`sin` run 0..65535 centred at 32768, so scaling by the DIAMETER lands the whole circle. -uint8_t count = 24; // @control 3..255 -uint8_t radius = 5; // @control 1..127 -for (i = 0; i < count; i = i + 1) { - addLight(scale(cos(i * turn(count)), radius * 2 + 1), - scale(sin(i * turn(count)), radius * 2 + 1), 0); +class RingLayout { + uint8_t count = 24; // @control 3..255 + uint8_t radius = 5; // @control 1..127 + + placeLights() { + for (i = 0; i < count; i = i + 1) { + addLight(scale(cos(i * turn(count)), radius * 2 + 1), + scale(sin(i * turn(count)), radius * 2 + 1), 0); + } + } } diff --git a/moonlive/layouts/rose.mlv b/moonlive/layouts/rose.mlv index 5b1710bd..00532d94 100644 --- a/moonlive/layouts/rose.mlv +++ b/moonlive/layouts/rose.mlv @@ -6,15 +6,20 @@ // // The envelope is recomputed where it is used: the grammar has no locals, and a layout // walk runs once per edit, so clarity beats the repeated call. -uint8_t petals = 2; // @control 1..8 -uint8_t radius = 15; // @control 4..30 -for (i = 0; i < 256; i = i + 1) { - addLight(radius - scale(sin(i * turn(256) * petals), radius + 1) - + scale(cos(i * turn(256)), - 2 * scale(sin(i * turn(256) * petals), radius + 1) + 1), - radius - scale(sin(i * turn(256) * petals), radius + 1) - + scale(sin(i * turn(256)), - 2 * scale(sin(i * turn(256) * petals), radius + 1) + 1), - 0); +class RoseLayout { + uint8_t petals = 2; // @control 1..8 + uint8_t radius = 15; // @control 4..30 + + placeLights() { + for (i = 0; i < 256; i = i + 1) { + addLight(radius - scale(sin(i * turn(256) * petals), radius + 1) + + scale(cos(i * turn(256)), + 2 * scale(sin(i * turn(256) * petals), radius + 1) + 1), + radius - scale(sin(i * turn(256) * petals), radius + 1) + + scale(sin(i * turn(256)), + 2 * scale(sin(i * turn(256) * petals), radius + 1) + 1), + 0); + } + } } diff --git a/moonlive/layouts/two-rows.mlv b/moonlive/layouts/two-rows.mlv index 3c0b9c6e..7dadd22d 100644 --- a/moonlive/layouts/two-rows.mlv +++ b/moonlive/layouts/two-rows.mlv @@ -1,10 +1,15 @@ // Two rows from one strand: out along y=0, back along y=1. // The return row counts x DOWN -- the strand turns around at the far end. -uint8_t cols = 16; // @control 1..64 -for (i = 0; i < cols; i = i + 1) { - addLight(i, 0, 0); -} -for (i = 0; i < cols; i = i + 1) { - addLight(cols - 1 - i, 1, 0); +class TwoRowsLayout { + uint8_t cols = 16; // @control 1..64 + + placeLights() { + for (i = 0; i < cols; i = i + 1) { + addLight(i, 0, 0); + } + for (i = 0; i < cols; i = i + 1) { + addLight(cols - 1 - i, 1, 0); + } + } } diff --git a/moonlive/modifiers/mirror.mlv b/moonlive/modifiers/mirror.mlv index a206564a..28aa38cf 100644 --- a/moonlive/modifiers/mirror.mlv +++ b/moonlive/modifiers/mirror.mlv @@ -1,2 +1,7 @@ // Mirror along x. Reflecting around `width` (not a fixed 255) keeps every light in the grid. -setXYZ(0, width - 1 - x, y, z); + +class MirrorModifier { + modifyLogical() { + setXYZ(0, width - 1 - xPos, yPos, zPos); + } +} diff --git a/moonlive/modifiers/shift.mlv b/moonlive/modifiers/shift.mlv index 0542817a..e4f85276 100644 --- a/moonlive/modifiers/shift.mlv +++ b/moonlive/modifiers/shift.mlv @@ -1,5 +1,10 @@ -// Slide along x. A coordinate is a byte, so keep amount small enough that x + amount stays under -// 256 -- past that it wraps and the light reappears at the left edge. -uint8_t amount = 4; // @control 0..64 +// Slide along x. A coordinate is a byte, so keep amount small enough that xPos + amount stays under +// 256: past that it wraps and the light reappears at the left edge. -setXYZ(0, x + amount, y, z); +class ShiftModifier { + uint8_t amount = 4; // @control 0..64 + + modifyLogical() { + setXYZ(0, xPos + amount, yPos, zPos); + } +} diff --git a/moonlive/modifiers/transpose.mlv b/moonlive/modifiers/transpose.mlv index 4116e54e..53102c43 100644 --- a/moonlive/modifiers/transpose.mlv +++ b/moonlive/modifiers/transpose.mlv @@ -1,2 +1,7 @@ // Swap the axes: rows become columns. -setXYZ(0, y, x, z); + +class TransposeModifier { + modifyLogical() { + setXYZ(0, yPos, xPos, zPos); + } +} diff --git a/src/core/BinaryBroadcaster.h b/src/core/BinaryBroadcaster.h index 838ca634..29c1269c 100644 --- a/src/core/BinaryBroadcaster.h +++ b/src/core/BinaryBroadcaster.h @@ -13,7 +13,7 @@ namespace mm { struct BinaryBroadcaster { // Stream ONE binary WS frame whose payload is PUSHED incrementally, so the caller never // holds the whole frame in a buffer. Begin/push/end trio, fitting a forward-only producer - // like Layouts::forEachCoord (push from inside its callback): + // like Layouts::placeLights (push from inside its callback): // beginBinaryFrame(totalLen) — build + send the WS header (totalLen = exact payload size) // pushBinaryFrame(data, len) — send the next payload slice (call as many times as needed) // endBinaryFrame() — finish; returns true if every client got the whole frame diff --git a/src/core/HttpServerModule.cpp b/src/core/HttpServerModule.cpp index 8ab213e8..29ef0398 100644 --- a/src/core/HttpServerModule.cpp +++ b/src/core/HttpServerModule.cpp @@ -2484,7 +2484,7 @@ bool HttpServerModule::sendAllOrClose(platform::TcpConnection& ws, const uint8_t // Streamed frame: header now, payload pushed in slices, no frame-sized staging buffer — so a // large frame (PreviewDriver's coordinate table or color frame) goes out on a memory-tight -// board where a contiguous block won't fit. The producer (forEachCoord) pushes forward-only; +// board where a contiguous block won't fit. The producer (placeLights) pushes forward-only; // each slice fans to every client before the next push. A client that can't keep up is closed // (its WS message ends incomplete → it reconnects), so this never blocks the tick indefinitely. void HttpServerModule::beginBinaryFrame(size_t totalLen) { diff --git a/src/core/HttpServerModule.h b/src/core/HttpServerModule.h index 3f326930..08e7adbe 100644 --- a/src/core/HttpServerModule.h +++ b/src/core/HttpServerModule.h @@ -254,7 +254,7 @@ class HttpServerModule : public MoonModule, public BinaryBroadcaster { // begin/push/endBinaryFrame stream a binary WS frame straight to every client with NO // frame-sized buffer: the header goes out on begin, each pushed slice is fanned to all // clients, and end reports whether every client got the whole frame. A producer (PreviewDriver - // streaming the producer buffer / forEachCoord) holds no copy. wsFrameAllSent_ tracks the + // streaming the producer buffer / placeLights) holds no copy. wsFrameAllSent_ tracks the // current frame's all-sent result across the push calls. bool wsFrameAllSent_ = true; // Max TOTAL WouldBlock spins for one span in sendAllOrClose before a stuck client is closed. diff --git a/src/core/moonlive/MoonLive.cpp b/src/core/moonlive/MoonLive.cpp index 574181c6..57f2209f 100644 --- a/src/core/moonlive/MoonLive.cpp +++ b/src/core/moonlive/MoonLive.cpp @@ -17,6 +17,9 @@ void MoonLive::freeCode() { fn_ = nullptr; anim_ = nullptr; ctrl_ = nullptr; + // The entry table describes code that no longer exists. Left behind, entry() would hand a + // binding an address into a freed block: the same stale-state trap the control arena has. + entryCount_ = 0; } // Copy `len` already-emitted bytes into a fresh exec block. writeExec hides the ISA quirks @@ -104,6 +107,15 @@ bool MoonLive::compile(const char* source, const BuiltinTable& table, const SysV ctrlNames_[i][len] = '\0'; controls_[i].name = ctrlNames_[i]; } + // Copy the entry table, names included: a CompileResult's `name` points into the source text, + // which the caller frees as soon as this returns. + entryCount_ = cr.entryCount < kMaxEntryPoints ? cr.entryCount : kMaxEntryPoints; + for (uint8_t i = 0; i < entryCount_; i++) { + const uint8_t n = cr.entries[i].nameLen < kMaxEntryName ? cr.entries[i].nameLen : kMaxEntryName; + for (uint8_t j = 0; j < n; j++) entryNames_[i][j] = cr.entries[i].name[j]; + entryNames_[i][n] = '\0'; + entries_[i] = {entryNames_[i], n, cr.entries[i].offset}; + } ctrl_ = reinterpret_cast(block); return true; } diff --git a/src/core/moonlive/MoonLive.h b/src/core/moonlive/MoonLive.h index 45fb5b36..484ebde0 100644 --- a/src/core/moonlive/MoonLive.h +++ b/src/core/moonlive/MoonLive.h @@ -5,6 +5,7 @@ #include "core/moonlive/moonlive_emit.h" #include "core/moonlive/MoonLiveBuiltins.h" #include "core/moonlive/MoonLiveCompiler.h" // CompileResult (carries the declared controls) +#include // std::strcmp: entry lookup by name // MoonLive — the live-script engine core (domain-neutral, §3.1/§3.9 of // livescripts-analysis-top-down.md). compile() turns a program into native code — either a @@ -43,6 +44,28 @@ class MoonLive { bool compileAnimated(); bool ok() const { return fn_ != nullptr || anim_ != nullptr || ctrl_ != nullptr; } + + /// The compiled function named `name`, or nullptr when the script did not define one. + /// + /// This is what makes a script's ROLE a question of what it defined rather than what type it is: + /// a layout asks for `placeLights`, an effect for `tick`, and a script that defines both is + /// served by both. The address is inside the ONE emitted block, at the offset the lowering + /// recorded, which is why a script may define as many functions as it likes for the cost of one + /// allocation. + CtrlFn entry(const char* name) const { + if (!code_ || !name) return nullptr; + for (uint8_t i = 0; i < entryCount_; i++) { + if (std::strcmp(entryNames_[i], name) != 0) continue; + if (entries_[i].offset >= codeLen_) return nullptr; // a corrupt map is not callable + return reinterpret_cast(static_cast(code_) + entries_[i].offset); + } + return nullptr; + } + + /// Names of the functions the script defined, for a binding that wants to report them and for + /// the dispatch question "which entry points does this script have". + const char* entryName(uint8_t i) const { return i < entryCount_ ? entryNames_[i] : nullptr; } + uint8_t entryCount() const { return entryCount_; } const char* error() const { return error_; } // The hot path: run the compiled routine over the host's buffer. `t` is the host's @@ -51,17 +74,36 @@ class MoonLive { // channels +0/+1/+2 per light, so a buffer that can't hold RGB — null, zero lights, or // fewer than 3 channels per light — is left untouched rather than overrun (robust to any // grid size / layout, the hard rule). - void run(uint8_t* buf, uint32_t nLights, uint8_t cpl, uint32_t t) const { + /// Run the script's ENTRY POINT called `name`, or the whole program when `name` is null. + /// + /// A binding names the function its role calls for: an effect wants `tick`, a layout + /// `placeLights`. Passing a name a script did not define runs NOTHING, which is the honest + /// answer: the module reports it rather than silently running some other function. + void run(uint8_t* buf, uint32_t nLights, uint8_t cpl, uint32_t t, + const char* name = nullptr) const { if (!buf || nLights == 0 || cpl < 3) return; // The arena is the fifth argument, and a front-end-compiled program reads its controls and // system variables straight through it — a null there is dereferenced by the EMITTED code, // which faults as a LoadProhibited at a nonsense address with no C++ frame to blame. Checked // with the other preconditions rather than trusted: every other operand of the call is. - if (ctrl_ && ctrlArena_) ctrl_(buf, nLights, cpl, t, ctrlArena_); // front-end-compiled + if (ctrl_ && ctrlArena_) { + // Start every frame at depth zero. The emitted code restores the counter as it unwinds, + // so this is normally already 0. A script that HIT the limit stopped calling + // rather than returning through the restore, and a leaked level would shrink the next + // frame's budget, and the next, until a legal recursion no longer ran. One byte. + ctrlArena_[kDepthSlot] = 0; + // A named entry when asked for one; otherwise the block start, which is what the + // hand-encoded programs and a single-function script both want. + CtrlFn f = name ? entry(name) : ctrl_; + if (f) f(buf, nLights, cpl, t, ctrlArena_); + } else if (fn_) fn_(buf, nLights, cpl); // hand-encoded fixed fill else if (anim_) anim_(buf, nLights, cpl, t); // hand-encoded animated fill } + /// Does the script define this entry point? A binding asks before reporting "no tick() to run". + bool hasEntry(const char* name) const { return entry(name) != nullptr; } + // Release the exec block + the control arena (the "destructor" role — release returns the // memory). void free(); @@ -120,6 +162,13 @@ class MoonLive { CtrlFn ctrl_ = nullptr; // front-end-compiled routine (5-arg, reads the controls arena) const char* error_ = ""; + // The functions the script defined, with their offsets into `code_`, and their names owned here + // for the same reason the control names are: a CompileResult's `name` points into source text + // the caller frees the moment compile() returns. + EntryPoint entries_[kMaxEntryPoints] = {}; + char entryNames_[kMaxEntryPoints][kMaxEntryName + 1] = {}; + uint8_t entryCount_ = 0; + uint8_t* ctrlArena_ = nullptr; // live control + system-variable bytes (platform::alloc, kArenaBytes, fixed) uint8_t controlCount_ = 0; // controls the current program declared DeclaredControl controls_[kMaxCtrls] = {}; // the declared-control metadata for the binding diff --git a/src/core/moonlive/MoonLiveBuiltins.h b/src/core/moonlive/MoonLiveBuiltins.h index 03aa770b..899abe13 100644 --- a/src/core/moonlive/MoonLiveBuiltins.h +++ b/src/core/moonlive/MoonLiveBuiltins.h @@ -91,10 +91,11 @@ struct BuiltinTable { static constexpr uint8_t kMaxCtrls = 8; // a script declares a handful of controls; fixed, no heap -// The controls arena holds two kinds of byte, in one allocation with a fixed split: +// The controls arena holds three kinds of byte, in one allocation with a fixed split: // [0 .. kMaxCtrls) script-declared controls, offset == declaration index -// [kMaxCtrls .. kArenaBytes) host system variables (width/height/…), offset assigned by -// the host and CONSTANT for the program's life +// [kMaxCtrls .. kMaxCtrls+kMaxSysVars) host system variables (width/height/…), offset assigned +// by the host and CONSTANT for the program's life +// [kDepthSlot] the recursion depth counter, owned by the emitted code // System variables sit ABOVE the script's range so that adding or removing a control — which // renumbers every control offset — cannot move them. The binding caches their slot pointers, so a // moving offset would silently write the wrong byte. @@ -127,7 +128,31 @@ constexpr size_t codeCapFor(uint32_t tokens) { } static constexpr uint8_t kMaxSysVars = 8; -static constexpr uint8_t kArenaBytes = kMaxCtrls + kMaxSysVars; + +/// Where the emitted code keeps its RECURSION DEPTH, one byte in the arena above the system +/// variables. In the arena rather than in a C++ member because the counter is read and written by +/// the emitted block itself: a recursive call happens entirely inside the exec block, with no C++ +/// frame between the activations for a host-side counter to sit in. Every function already holds +/// the arena pointer (kArg4), so the guard costs a byte and no new argument. +/// +/// The host zeroes it before each run rather than trusting the block to unwind cleanly: a script +/// that hits the limit leaves the counter wherever the skipped call left it, and a stale value +/// would shrink the budget of every later frame until nothing ran at all. +static constexpr uint8_t kDepthSlot = kMaxCtrls + kMaxSysVars; + +/// The depth at which a call is REFUSED: an activation that would make the counter reach this +/// number returns without running, so 31 activations execute, the entry function included. +/// +/// A fixed render-task stack makes unbounded recursion a device reset, which the robustness rule +/// forbids, so the depth is bounded at run time rather than at compile time: whether a recursion +/// terminates is not decidable from the source. The number is measured rather than chosen. An +/// activation costs 176 bytes of stack on Xtensa (48 host-call area + 84 slots + 32 window +/// reserve + alignment) against a 12 KB main task, so the device resets at roughly 64 deep. This +/// leaves the deepest legal recursion at under half the budget, which is the margin the interrupt +/// stack and the rest of the render path need. +static constexpr uint8_t kMaxCallDepth = 32; + +static constexpr uint8_t kArenaBytes = kMaxCtrls + kMaxSysVars + 1; // +1: kDepthSlot /// A name the HOST defines and the script only reads: `width`, `height`, `depth`. Reserved — a /// script cannot declare one, so the name means the same thing in every script (the `t` rule, one diff --git a/src/core/moonlive/MoonLiveCompiler.cpp b/src/core/moonlive/MoonLiveCompiler.cpp index 4cb43298..6bda1e3a 100644 --- a/src/core/moonlive/MoonLiveCompiler.cpp +++ b/src/core/moonlive/MoonLiveCompiler.cpp @@ -112,6 +112,13 @@ struct Parser { const BuiltinTable& table; const SysVarTable& sysvars; IrProgram& ir; + char* classNameOut = nullptr; // the caller's buffer; the parser fills it + // Each function the class defined, with the IR index its body starts at. An IR index, not a byte + // offset: the parser runs before lowering, so the byte an entry lands on is not known yet. The + // emitter converts one to the other, which is the same seam a linker crosses. + struct FnMark { const char* name; uint8_t nameLen; uint16_t irStart; }; + FnMark fns[kMaxEntryPoints] = {}; + uint8_t fnCount = 0; VReg nextTemp = kFirstTemp; // high-water mark — also IrProgram.vregsUsed VReg freeStack[kMaxVRegs] = {}; // recycled temps (LIFO), so a dead vreg is reused uint8_t freeCount = 0; @@ -303,7 +310,35 @@ struct Parser { void parseCall(VReg* resultOut) { if (lex.kind != Tok::Ident) { fail("expected a function name"); return; } const Builtin* fn = table.find(lex.identBeg, lex.identLen); - if (!fn) { fail("unknown function"); return; } + if (!fn) { + // Not a built-in: the script's own function, if it declared one by this name. Resolved + // against the class's function list rather than the builtin table, which is what makes + // a helper callable and, when the name is the running function's own, what makes + // recursion work: nothing here treats the two cases differently. + // + // Only functions ALREADY PARSED are visible. A forward call (to a helper declared + // further down) is refused rather than half-supported, because resolving it needs a + // second pass over the class body. The cost is that "unknown function" is what a + // forward call reports too, with the column but not the name, so a helper has to be + // declared above its caller. Recursion is unaffected: a function is added to the list + // before its body is parsed, so it can see itself. + for (uint8_t i = 0; i < fnCount; i++) { + if (fns[i].nameLen != lex.identLen) continue; + if (std::strncmp(fns[i].name, lex.identBeg, lex.identLen) != 0) continue; + lex.advance(); + if (!expect(Tok::LParen, "expected '(' after the function name")) return; + if (!expect(Tok::RParen, "a script function takes no arguments yet")) return; + if (resultOut) { fail("a script function returns nothing yet"); return; } + // `imm` is the callee's FUNCTION NUMBER, not its position in the op array. An IR + // index would be the more obvious choice and was the first one, but the spill pass + // rewrites the array and every index past its first insertion shifts: the call then + // named a position that no longer started a function, and the lowering opened the + // next frame mid-statement. A function number survives any rewrite of the ops. + emit({IrOp::CallScript, 0, 0,0,0,0, static_cast(i), nullptr, {}}); + return; + } + fail("unknown function"); return; + } lex.advance(); if (!expect(Tok::LParen, "expected '(' after the function name")) return; @@ -423,10 +458,14 @@ struct Parser { controlCount++; } - // Is the current Ident the `uint8_t` type keyword (the only declared type in Stage 1)? - bool atTypeKeyword() const { - return lex.kind == Tok::Ident && lex.identLen == 7 && std::strncmp(lex.identBeg, "uint8_t", 7) == 0; + // Is the current Ident this exact keyword? Keywords are matched by text rather than lexed as + // their own token kind: the set is tiny, and a script may still use `class` or `for` as part of + // a longer identifier, which a length-checked compare gets right for free. + bool atKeyword(const char* kw, size_t len) const { + return lex.kind == Tok::Ident && lex.identLen == len && std::strncmp(lex.identBeg, kw, len) == 0; } + // Is the current Ident the `uint8_t` type keyword (the only declared type in Stage 1)? + bool atTypeKeyword() const { return atKeyword("uint8_t", 7); } // program := { decl } { stmt }. Declarations (control vars) come first, then one-or-more // call statements. (Multi-statement now: a script has decl lines AND a statement line.) @@ -594,33 +633,78 @@ struct Parser { /// One statement: a call, or a for. bool parseStatement() { - if (lex.kind == Tok::Ident && lex.identLen == 3 && - std::strncmp(lex.identBeg, "for", 3) == 0) { - return parseFor(); - } + if (atKeyword("for", 3)) return parseFor(); if (lex.kind != Tok::Ident) { fail("expected a function call"); return false; } parseCall(nullptr); if (failed) return false; return expect(Tok::Semicolon, "expected ';'"); } + /// The body of one named function: `name() { statements }`, with the name already consumed. + /// Stage 1 emits it INLINE at the point the class body reaches it, which is what makes `tick()` + /// the whole program while it is the only entry point. Real per-function frames arrive with the + /// call support in this same step; this is the parse shape they will attach to. + bool parseFunctionBody() { + if (!expect(Tok::LParen, "expected '(' after the function name")) return false; + if (!expect(Tok::RParen, "expected ')': parameters arrive with typed members")) return false; + if (!expect(Tok::LBrace, "expected '{' to open the function body")) return false; + while (!failed && lex.kind != Tok::RBrace && lex.kind != Tok::End) + if (!parseStatement()) return false; + if (failed) return false; + return expect(Tok::RBrace, "expected '}' to close the function body"); + } + + /// program := "class" NAME "{" { decl | function } "}" + /// + /// ONE top-level form. A bare statement list is no longer accepted: keeping it would mean two + /// parse paths, two sets of rules to document and two things to test, permanently, so that the + /// shortest scripts could stay one line shorter. The class declaration is what makes a script + /// read as the module it stands in for, which is the whole point of the shape. bool parseProgram() { - // Park the host arguments in their fixed slots at the top of the frame, once, before any - // script code runs. They are read-only, so one store each is all it takes — and every later - // read becomes a Reload, which frees five registers for the whole program. - for (VReg v = 0; v < kFirstTemp; v++) - emit({IrOp::Spill, 0, v, 0,0,0, hostArgSlot(v), nullptr, {}}); + if (!atKeyword("class", 5)) { fail("a script is a class: expected `class { … }`"); return false; } + lex.advance(); + if (lex.kind != Tok::Ident) { fail("expected a name after `class`"); return false; } + // Copied, not borrowed: the source buffer is freed as soon as the compile returns, and the + // name outlives it in the status line. + const size_t n = lex.identLen < kMaxClassName ? lex.identLen : kMaxClassName; + std::memcpy(classNameOut, lex.identBeg, n); + classNameOut[n] = '\0'; + lex.advance(); + if (!expect(Tok::LBrace, "expected '{' to open the class body")) return false; + // Declarations first (the controls), then the functions. Both live inside the braces now. while (!failed && atTypeKeyword()) { lex.advance(); parseDecl(); } if (failed) return false; - if (lex.kind == Tok::End) { fail("empty program (no statement)"); return false; } + bool any = false; - while (!failed && lex.kind != Tok::End) { - if (!parseStatement()) return false; + while (!failed && lex.kind != Tok::RBrace && lex.kind != Tok::End) { + if (lex.kind != Tok::Ident) { fail("expected a function, or '}' to close the class"); return false; } + if (fnCount >= kMaxEntryPoints) { fail("too many functions in one class"); return false; } + // The engine copies entry names into a fixed buffer, so a longer one would be + // TRUNCATED there. Two functions sharing a 23-character prefix would then land under + // the same name and `entry()` would return whichever came first: a call dispatched to + // the wrong function, silently. Refused here, where a control name already is, so the + // script author is told rather than the engine guessing. + if (lex.identLen > kMaxEntryName) { fail("function name too long"); return false; } + fns[fnCount] = {lex.identBeg, static_cast(lex.identLen), + static_cast(ir.count)}; + // The IR carries the start INDEX; the lowering turns it into a byte offset. + ir.fnIrStart[fnCount] = static_cast(ir.count); + ir.fnCount = static_cast(fnCount + 1); + fnCount++; + lex.advance(); // the function name + // Park the host arguments in this FUNCTION's frame. Read-only, so one store each, and + // every later read is a Reload, which frees five registers for the body. Per function + // rather than per program because each function owns its own frame now: a spill emitted + // before the first prologue would write to a frame that does not exist yet. + for (VReg v = 0; v < kFirstTemp; v++) + emit({IrOp::Spill, 0, v, 0,0,0, hostArgSlot(v), nullptr, {}}); + if (!parseFunctionBody()) return false; any = true; } - if (!any) { fail("expected a statement"); return false; } - return true; + if (failed) return false; + if (!any) { fail("a class with no function does nothing"); return false; } + return expect(Tok::RBrace, "expected '}' to close the class"); } }; @@ -659,7 +743,7 @@ CompileResult compileSource(const char* source, const BuiltinTable& table, return r; } Lexer lex(source); - Parser parser{lex, table, sysvars, ir}; + Parser parser{lex, table, sysvars, ir, r.className}; if (!parser.parseProgram()) { r.error = parser.error; r.errorCol = parser.errorCol; return r; } // Hand the backend the frame the script's variables need. The register allocator numbers any // further slot from here up, so the two never overlap in the one frame they share. @@ -675,6 +759,12 @@ CompileResult compileSource(const char* source, const BuiltinTable& table, // Surface the declared controls so the binding can create real MoonModule controls. r.controlCount = parser.controlCount; for (uint8_t i = 0; i < parser.controlCount; i++) r.controls[i] = parser.controls[i]; + // The functions the class defined, each with the byte its code starts at. The parser recorded + // an IR index and the lowering converted it while emitting, so this is a real symbol table: a + // binding asks for an entry by name and gets an address inside the one emitted block. + r.entryCount = parser.fnCount; + for (uint8_t i = 0; i < parser.fnCount; i++) + r.entries[i] = {parser.fns[i].name, parser.fns[i].nameLen, ir.fnOffset[i]}; return r; } diff --git a/src/core/moonlive/MoonLiveCompiler.h b/src/core/moonlive/MoonLiveCompiler.h index b31e72cf..b8309c2d 100644 --- a/src/core/moonlive/MoonLiveCompiler.h +++ b/src/core/moonlive/MoonLiveCompiler.h @@ -29,6 +29,29 @@ inline constexpr const char* kCodegenFailed = "codegen failed (unsupported on th // Result of compiling source: on success, ok==true and the bytes are in out[0..len). On // failure, ok==false and error points at a static diagnostic (1-based column, 0 if n/a). +/// A function the script defined, and where its code starts within the emitted block. +/// +/// This is a symbol table, which is what every compiler and linker keeps: one code section, and a +/// name-to-offset map over it. The binding asks for an entry by name and gets a callable address, +/// so which ROLE a script plays is decided by which entries it defined rather than by its type. +struct EntryPoint { + const char* name = nullptr; ///< into the source, or the engine's own copy after compile + uint8_t nameLen = 0; + uint16_t offset = 0; ///< byte offset of its first instruction within the block +}; + +/// How many named functions one script may define. A handful of entry points plus the helpers a +/// script writes for itself; past that is a script that wants to be a module. +inline constexpr uint8_t kMaxEntryPoints = 8; + +/// Longest function name the engine keeps. The host's own entry names (`placeLights`, +/// `modifyLogicalSize`) are the long ones; a script's helpers are usually short. +inline constexpr uint8_t kMaxEntryName = 23; + +/// Longest class name kept. A diagnostic quotes it, so it is bounded like every other name the +/// engine holds rather than pointing into source that is freed as soon as the compile returns. +inline constexpr size_t kMaxClassName = 31; + struct CompileResult { bool ok = false; const char* error = ""; @@ -38,6 +61,14 @@ struct CompileResult { // this list and creates a real MoonModule control per entry, bound to the run-time arena slot. DeclaredControl controls[kMaxCtrls]; uint8_t controlCount = 0; + // The name the script gave its class. What diagnostics and the module status report, so a + // renamed FILE does not change what a user is told: the filename is what the engine loads, the + // class name is what it is. Copied out of the source, which is freed after the compile. + char className[kMaxClassName + 1] = ""; + // The functions this script defined, in source order. `tick` is the one the light bindings look + // for today; the rest arrive with the per-role entry points. + EntryPoint entries[kMaxEntryPoints]; + uint8_t entryCount = 0; }; // Compile `source` to machine code in `out` (capacity `cap`), resolving calls against `table`. diff --git a/src/core/moonlive/MoonLiveIr.h b/src/core/moonlive/MoonLiveIr.h index b3f25359..8259d9c4 100644 --- a/src/core/moonlive/MoonLiveIr.h +++ b/src/core/moonlive/MoonLiveIr.h @@ -62,6 +62,26 @@ enum class IrOp : uint8_t { // the parser stages every argument into consecutive slots, so a call carries a // POSITION and a COUNT rather than the values, and arity is bounded by frame slots // instead of by operand fields. Backends materialise the address themselves. + CallScript,// call the script's OWN function, the one numbered `imm` in declaration order. + // Distinct from Call because the target is a position in THIS program rather than a + // host address: the backend emits a relative call to a label, so there is nothing to + // materialize and nothing to stage. A function NUMBER rather than a position: this + // pass's own rewrite shifts every index past its first insertion, so an IR index + // named an op that no longer started a function. It is also what makes recursion + // work, since the + // callee's own prologue allocates a fresh frame per activation. + // + // THE CALLER PASSES THE HOST ARGUMENTS ON. Every function's prologue parks + // buf/nLights/cpl/t/ctrls out of the argument registers into its own frame, because + // that is how the host enters the block, so a call that passed nothing left the + // callee parking whatever those registers held and its first control read faulted on + // a null arena. Each backend reloads them from the frame before the call; where they + // go is the per-ISA part. + // + // NOTHING MAY BE LIVE IN A REGISTER ACROSS ONE. Script variables are slot-resident + // and temporaries die within their statement, so this holds today and is why no + // save-set is emitted. Xtensa's window rotation would hide a violation that + // corrupts RISC-V, so it is stated here rather than left to be discovered. Inline, // a host-registered inline op (inlineOp tag); operands a/b/c/d (op-specific) LoadCtrl, // dst = ((const uint8_t*)kArg4)[imm] — read a control value byte at offset imm Mov, // dst = a — the assignment a loop variable needs (vregs are otherwise write-once) @@ -109,11 +129,35 @@ struct DeclaredControl { /// Branch targets one IR program may use. Two per `for` (entry guard + back edge), and the counter /// runs for the whole program rather than per scope — a label is never reused once a loop closes — -/// so this bounds the TOTAL number of loops in a script (8), not how deeply they nest. The -/// assemblers carry the same ceiling in their own label tables, and the compiler fails loudly -/// rather than silently miscompiling past it. Nesting depth is bounded separately, by `locals`. +/// so this bounds the TOTAL number of loops in a script (8), not how deeply they nest. Nesting +/// depth is bounded separately, by `locals`. static constexpr uint8_t kIrLabels = 16; +/// Labels and fixups an ASSEMBLER's tables hold. Larger than kIrLabels because a backend allocates +/// labels the IR never names: one per named function (a call may precede its definition, so these +/// cannot be lazy), plus one per StoreElem for its bounds guard and two per FillElems. A class of +/// three functions each holding a loop and a store needs more than the sixteen that sized these +/// when a script was a single routine: crosshair.mlv is exactly that script, and it failed to +/// compile with the generic "too large" rather than naming the table it exhausted. +/// +/// Held in core so the three backends cannot drift: they carried an identical private copy each, +/// and a script that fit one would have been refused by another. +/// +/// THE COST IS STACK, and it is the number to watch when raising these. Both tables are members of +/// the assembler, which is a local in lowerWith: 4 bytes per label, 8 per fixup on a 32-bit +/// target. Measured on the classic ESP32 image, `lowerWith` went from 480 to 1120 bytes, so 48/96 +/// costs 640 bytes more than 16/32. That frame is the largest on the compile chain +/// (compileScriptFile 144 + MoonLive::compile 288 + compileSource 576 + lowerWith 1120 = 2128 +/// nested), against CONFIG_ESP_MAIN_TASK_STACK_SIZE = 12288: 17% of the task at the deepest point. +/// Flash is unchanged, since these are stack arrays rather than data. +/// +/// There is headroom, but not unlimited headroom, and a compile runs on the RENDER task. Doubling +/// these again would put lowerWith past 2 KB. If a future script needs more, move the tables to the +/// heap alongside the code buffer (which was moved for exactly this reason) rather than raising the +/// constants: this project has already bootlooped a P4 on an oversized stack frame. +static constexpr uint8_t kAsmLabels = 48; +static constexpr uint8_t kAsmFixups = 96; + /// Script variables live in FRAME SLOTS, and this bounds how many one program may hold at once. /// Sixteen matches what every backend's frame can address (`kMaxSpillSlots`), so a program that /// parses is a program the assembler can encode. Raising it means widening the frame on all three @@ -142,6 +186,11 @@ constexpr uint8_t hostArgSlot(VReg v) { /// host arguments above it. static constexpr uint8_t kTotalSlots = kMaxLocals + kHostArgSlots; +/// Named functions one program may define. Mirrors kMaxEntryPoints in the compiler header: +/// the IR carries the offsets, the CompileResult carries the names, and the two are filled +/// from the same parse, so they are bounded together. +static constexpr uint8_t kMaxIrEntries = 8; + static constexpr uint8_t kMaxControlName = 24; // max control-name length (incl. NUL); the compiler // rejects longer names so the binding's name pool @@ -170,6 +219,19 @@ struct IrProgram { /// at zero without a variable and a spilled temp landing on the same bytes. uint8_t localSlots = 0; + /// Where each named function's code STARTS, filled in as the lowering walks the ops. + /// + /// The front end knows which IR index a function begins at, but not which byte: that is decided + /// by the encoding, which is the backend's business. So the parser records the index in + /// `fnIrStart` and the lowering fills `fnOffset` as it passes it. This is the same crossing a + /// linker makes between a symbol and its address, done in one pass because there is one section. + /// + /// `fnCount` is 0 for a program with no named functions (the hand-encoded fills), and the + /// binding then calls the block start, which is what it has always done. + uint16_t fnIrStart[kMaxIrEntries] = {}; + uint16_t fnOffset[kMaxIrEntries] = {}; + uint8_t fnCount = 0; + IrProgram() = default; ~IrProgram() { platform::free(ops); } IrProgram(const IrProgram&) = delete; // owns a buffer; a copy would double-free @@ -217,6 +279,16 @@ struct IrProgram { t = count; count = o.count; o.count = t; VReg v = vregsUsed; vregsUsed = o.vregsUsed; o.vregsUsed = v; uint8_t ls = localSlots; localSlots = o.localSlots; o.localSlots = ls; + // The FUNCTION TABLE is part of the program, so it moves with the ops. Omitting it left the + // spill pass's remapped boundaries in the discarded half while the lowering read the stale + // pre-spill ones: it then opened a function's frame two ops early, in the middle of the + // previous function's pixel write, and the emitted block was structurally plausible enough + // (one entry and one retw per function) that only a disassembly showed it. + uint8_t fc = fnCount; fnCount = o.fnCount; o.fnCount = fc; + for (uint8_t f = 0; f < kMaxIrEntries; f++) { + uint16_t s = fnIrStart[f]; fnIrStart[f] = o.fnIrStart[f]; o.fnIrStart[f] = s; + uint16_t b = fnOffset[f]; fnOffset[f] = o.fnOffset[f]; o.fnOffset[f] = b; + } } /// Which inline ops this program contains, so a backend reserves scratch only for what is there. @@ -232,6 +304,15 @@ struct IrProgram { if (ops[i].op == IrOp::Inline && ops[i].inlineOp == which) return true; return false; } + + /// Does any function call another function of this script? The recursion depth guard is emitted + /// only when one does, so a script that just defines `tick()` (every shipped script today) + /// carries none of it. + bool hasScriptCall() const { + for (uint16_t i = 0; i < count; i++) + if (ops[i].op == IrOp::CallScript) return true; + return false; + } }; } // namespace mm::moonlive diff --git a/src/core/moonlive/MoonLiveSpill.cpp b/src/core/moonlive/MoonLiveSpill.cpp index 9c37117c..c5843976 100644 --- a/src/core/moonlive/MoonLiveSpill.cpp +++ b/src/core/moonlive/MoonLiveSpill.cpp @@ -66,6 +66,10 @@ uint8_t sourcesOf(const IrInst& in, VReg* out) { // vreg. Reporting a/b/c as sources gave the count a live interval and let the rewrite below // remap it into a register number; it survived only because a fixed ABI vreg maps to itself. case IrOp::Call: return 0; + // Nor does a call to the script's OWN function: `imm` is the callee's function NUMBER, not + // a value, and the callee reads its arguments from frame slots exactly as a host built-in + // does. + case IrOp::CallScript: return 0; case IrOp::Inline: // The inline ops read every operand field the host filled in. Both of today's ops also // read kArg0..kArg2 (buf, nLights, cpl), but those are fixed ABI vregs this pass never @@ -82,6 +86,9 @@ uint8_t sourcesOf(const IrInst& in, VReg* out) { bool writesDst(IrOp op) { switch (op) { case IrOp::Label: case IrOp::BranchGe: case IrOp::BranchNe: + // CallScript writes no dst either: a script function returns nothing today, so the call is + // a statement rather than an expression. When it gains a return value this moves. + case IrOp::CallScript: case IrOp::Spill: case IrOp::Inline: return false; default: return true; } @@ -317,7 +324,27 @@ bool spillToBudget(IrProgram& ir, const RegBudget& budget, uint8_t& slotsUsed) { return true; }; + // The function boundaries move with the ops. `fnIrStart` indexes the INPUT array, and this + // rewrite inserts a Reload before a read and a Spill after a define, so every index past the + // first insertion shifts. Left unmapped, the lowering closes a function at the wrong op: it + // emitted a `retw` in the middle of an expanding StoreElem, splitting the pixel write across + // two frames. Found by disassembling, because the emitted stream was structurally plausible + // (two entries, two retws, one call8) and only the POSITION of the boundary was wrong. + // + // REQUIRED, and the disassembly says otherwise. Removing this makes crosshair.mlv emit a + // TIDIER-looking block (one entry/retw pair per function, at plausible offsets) and every + // host test still passes, because the host backend cannot reach this path. On an S3 that block + // boot-loops with StoreProhibited and the buffer pointer holding 0xff: a store through a + // register the split left holding a color byte. Verify a change here on a board, not on a + // listing and not on the suite. + uint16_t newFnStart[kMaxIrEntries] = {}; + for (uint16_t i = 0; i < ir.count; i++) { + // Record where this function begins in the OUTPUT array, before anything is emitted for + // the op that starts it. + for (uint8_t f = 0; f < ir.fnCount; f++) + if (ir.fnIrStart[f] == i) { newFnStart[f] = out.count; } + IrInst in = ir.ops[i]; VReg src[4]; const uint8_t n = sourcesOf(in, src); @@ -372,6 +399,9 @@ bool spillToBudget(IrProgram& ir, const RegBudget& budget, uint8_t& slotsUsed) { } out.vregsUsed = newHighWater; + // Carry the function table across the swap, with the boundaries remapped to the output array. + out.fnCount = ir.fnCount; + for (uint8_t f = 0; f < ir.fnCount; f++) out.fnIrStart[f] = newFnStart[f]; ir.swap(out); slotsUsed = nSpilled; return true; diff --git a/src/core/moonlive/moonlive_lower.h b/src/core/moonlive/moonlive_lower.h new file mode 100644 index 00000000..4fbdf8de --- /dev/null +++ b/src/core/moonlive/moonlive_lower.h @@ -0,0 +1,324 @@ +#pragma once + +#include "core/moonlive/moonlive_emit.h" +#include "core/moonlive/MoonLiveIr.h" +#include "core/moonlive/MoonLiveSpill.h" // the register allocator, run before lowering + +#include + +// MoonLive IR -> machine bytes: the ONE lowering, written once for every backend. +// +// Walking the IR is not a per-target algorithm. Which register holds a value, which frame slot it +// spills to, when a host argument is reloaded, how an inline op is expanded: all of that is decided +// by core, identically for arm64, RISC-V and Xtensa. What differs per target is how an instruction +// is ENCODED, and that already lives behind the assembler. So the walk lives here and each backend +// supplies its assembler. +// +// This was three near-identical files. The two device lowerings differed by two identifier tokens, +// and the host one by a handful of lines that turned out to be free choices rather than ISA facts. +// The risk that shape carries is not the duplication itself but the silence: a rule changed in one +// copy and not the others miscompiles on one target while the tests, which execute only the host +// backend, stay green. +// +// The assembler contract, which all three satisfy: +// ctor(size_t cap), newLabel, bind, prologue(uint8_t), epilogue, alignForEntry, finalize, +// bytes, size, overflowed, spillStore, spillLoad, slotAddr, +// movImm, movReg, addImm, addReg, mulReg, store8, load8, +// branchIfZero, branchGeU, branchNe, call, callLabel, and kMaxSpillSlots. +// The branches are the FUSED forms (compare-and-branch as one call). arm64 has no such +// instruction and spells each as cmp + b.cond inside its assembler, which is exactly where a +// difference of encoding belongs. +// +// INCLUDE ORDER: a backend includes its own assembler header BEFORE this one. `Reg` and `Label` +// are declared per assembler (each ISA's register file is a different size, which is the whole +// point of kRegCount), so this header names them without declaring them: it is the algorithm, not +// the register set. That is also why it is a template rather than a compiled unit; there is no +// single `Reg` for it to compile against. + +namespace mm::moonlive { + +/// Lower `ir` into `out` using assembler `A`. Returns the byte count, or 0 when the program cannot +/// be encoded (degrade, never miscompile). +template +size_t lowerWith(IrProgram& ir, uint8_t* out, size_t cap, const RegBudget* squeeze, + uint8_t regCount) { + // Taken FROM the assembler, not named globally: each backend declares its own `Reg` (the + // register files differ in size, which is what kRegCount is) and its own `Label`. Deducing + // them from the assembler keeps this header free of any one ISA's types, so it compiles as + // core code rather than needing a backend included first. RegId/LabelId rather than Reg/Label: + // the backend's own names are already in scope wherever this is instantiated, and reusing them + // shadows those declarations (-Wshadow is an error here). + using RegId = typename A::RegType; + // LabelId, not Label: every assembler declares `using Label = uint8_t` in its own namespace, + // so reusing that name here would shadow it. Deduced from the ASSEMBLER INSTANCE below rather + // than through std::declval, because the codegen tests compile a backend inside a wrapper + // namespace, and a include here would land in it and nest a second `std`. + auto reg = [](VReg v) { return static_cast(v); }; + + // THREE scratch registers, for every program. Two are shared by everything that needs a + // temporary for the length of one instruction, and the third is the host-argument reload. + // + // The two shared ones (sAddr, sCtr below) serve, in turn: FillElems (loop counter + + // per-channel address), StoreElem (the address, which must NOT be folded into the caller's + // index vreg, since that destroys a `for` counter), a host Call (its argument block address + // and count), and the recursion depth guard (the counter and its comparand). None of those + // is live at the same time as another, because each dies inside the one instruction that + // uses it, so they share rather than reserve separately: reserving per user cost registers + // permanently, which on Xtensa's ten is the difference between compiling and not. + // + // NOT conditional on which inline ops the program contains, and the floor of two is what + // makes that safe. It reads like a missed optimization, and was one until the depth guard + // arrived: the guard is emitted in a function's prologue and uses both shared registers, in + // a script that may contain no inline op at all. A count derived from `hasInline` alone + // would hand the guard a register the program does not own. + // + // The +1 is the host-argument reload: the host arguments live in frame slots (core parks + // them at entry), so an op that reads buf/nLights/cpl/ctrls brings one back for its own + // instruction. + constexpr uint8_t kSharedScratch = 2; + const uint8_t scratchTotal = kSharedScratch + 1; + if (!out || cap == 0) return 0; + + // Run the register allocator before lowering. It leaves a program that already fits untouched, + // and rewrites one that does not into Spill/Reload against this backend's frame, replacing the + // hand-rolled bail that used to REFUSE such a script outright. False here means even the + // spilled form does not fit, which is a diagnostic, never a miscompile. + uint8_t slots = 0; + // `squeeze` overrides the REGISTER COUNT and slot count a test wants to constrain, but never + // `reserved`: the scratch is what this lowering is about to use for its inline ops and call + // argument block, so a test-supplied value would let the allocator hand out a register the + // lowering then overwrites, miscompiling exactly the squeezed programs the seam exists to prove. + const RegBudget budget = squeeze ? RegBudget{squeeze->regs, scratchTotal, squeeze->slots} + : RegBudget{regCount, scratchTotal, A::kMaxSpillSlots}; + if (!spillToBudget(ir, budget, slots)) return 0; + // sAddr FIRST: it is the one StoreElem also uses, and a store-only program reserves a single + // scratch, so the shared one has to be the lowest index or it would name an unreserved register. + const RegId sAddr = static_cast(ir.vregsUsed); // per-channel address (both ops) + const RegId sCtr = static_cast(ir.vregsUsed + 1); // FillElems loop counter + + // Size the assembler's buffer to the CALLER's: `cap` is what the staging buffer holds, so the + // two can never disagree about how much a script may emit (they were separately constant, and + // a script that fit one overflowed the other). + A a(cap); + using LabelId = decltype(a.newLabel()); + // The LAST reserved scratch index, derived from scratchTotal rather than hard-coded: the `+1` + // in scratchTotal above IS this register, so the reservation and the use cannot drift apart. + // A fixed offset sat OUTSIDE the reservation and only worked because the register maps happen + // to have spare entries above the high-water mark. + const RegId sHost = static_cast(ir.vregsUsed + scratchTotal - 1); + auto host = [&](VReg v) -> RegId { a.spillLoad(sHost, hostArgSlot(v)); return sHost; }; + // The frame must cover the parked HOST ARGUMENTS at the top as well as whatever the parser and + // the allocator claimed at the bottom: they are stored before any script code runs, so a frame + // sized only from `slots` would put them past its end. + const uint8_t frameSlots = slots > kTotalSlots ? slots : kTotalSlots; + // A program with NO named functions is one routine, so its prologue opens the block. A class + // gets a prologue PER FUNCTION instead, emitted at each function's first op below, because an + // entry point's recorded offset has to be an address a caller can jump to: pointing it past a + // single program-wide prologue would enter a routine whose frame was never established. + if (ir.fnCount == 0) a.prologue(frameSlots); + + // An IR label id becomes an assembler label ON FIRST USE. Allocating the whole range up front + // exhausts the assembler's fixed label table, and the inline ops (StoreElem's bounds guard, + // FillElems' loop) then get nothing when they ask for their own, which broke every program that + // contains no loop at all. Lazy allocation costs one lookup and leaves the table for the labels + // a program actually has. + // One label per named function, allocated up front: a CallScript may appear BEFORE the + // function it targets (and does, for recursion), so the label has to exist before it is bound. + LabelId fnLabel[kMaxIrEntries]; + for (uint8_t f = 0; f < ir.fnCount; f++) fnLabel[f] = a.newLabel(); + + // The depth guard is emitted only for a script whose functions call each other: it is dead + // weight in every script that just defines `tick()`, which is all of the shipped ones. + const bool guardDepth = ir.hasScriptCall(); + // Where a function jumps when it is too deep: its own epilogue, so the refusal unwinds through + // the same decrement and return as a normal exit and there is no second way out of a frame. + LabelId tooDeep[kMaxIrEntries]; + if (guardDepth) + for (uint8_t f = 0; f < ir.fnCount; f++) tooDeep[f] = a.newLabel(); + // Function number + 1 while a guard is owed, 0 when none is: the guard is emitted a few ops + // after the prologue, once the host arguments have been parked. + int guardPending = 0; + + LabelId labels[kIrLabels]; + bool labelMade[kIrLabels] = {}; + auto labelFor = [&](int32_t id) -> LabelId { + if (!labelMade[id]) { labels[id] = a.newLabel(); labelMade[id] = true; } + return labels[id]; + }; + + // Emit this function's depth increment, if one is still owed. + // + // It goes AFTER the function's host-argument Spills, not before them. Both the guard and the + // epilogue address the arena through the PARKED copy in the frame, and a refusing activation + // jumps straight to the epilogue, so a guard placed first would make the refusal read a frame + // slot nothing had written yet. Emitting it once the parking is done means the two agree by + // construction rather than by argument. + // + // Called both at the first non-Spill op and from closeFn, because a function may have NO other + // op: `nop() {}` parses, and its whole body is the five parking Spills. Flushed only at the + // first site, that function got no increment while its epilogue still decremented, so every + // call to it drove the counter DOWN. Two such calls wrapped the byte to 255 and the next real + // call was refused as too deep, which reads as a legal call silently doing nothing. + auto flushGuard = [&]() { + if (!guardPending) return; + const uint8_t f = static_cast(guardPending - 1); + guardPending = 0; + const RegId d = host(kArg4); // one reload; nothing below clobbers it + a.load8(sAddr, d, kDepthSlot); + a.movImm(sCtr, 1); + a.addReg(sAddr, sAddr, sCtr); // depth + 1 + a.movImm(sCtr, kDepthSlot); + a.store8(d, sCtr, sAddr); // arena[kDepthSlot] = depth + 1 + a.movImm(sCtr, kMaxCallDepth); + a.branchGeU(sAddr, sCtr, tooDeep[f]); // at or past the limit: unwind immediately + }; + + // Close function `f`: the one exit every activation takes, whether it ran to the end or refused + // at the guard. Written once so the two cannot drift: a refusal that skipped the decrement + // would leak a level per attempt and shrink the budget until nothing recursed at all. + auto closeFn = [&](uint8_t f) { + flushGuard(); // an empty function still balances: increment, then decrement + if (guardDepth) { + a.bind(tooDeep[f]); // the too-deep path joins here + const RegId d = host(kArg4); + a.load8(sAddr, d, kDepthSlot); + a.movImm(sCtr, -1); + a.addReg(sAddr, sAddr, sCtr); // depth - 1 + a.movImm(sCtr, kDepthSlot); + a.store8(d, sCtr, sAddr); + } + a.epilogue(); + }; + + // uint16_t, matching IrProgram::count: the op array is sized to the script now, so a uint8_t + // counter wrapped at 256 ops and looped forever instead of emitting. + for (uint16_t i = 0; i < ir.count; i++) { + // A named function starts here. Close the previous one and open this one, so every function + // is independently callable: its recorded offset is its PROLOGUE, and it returns rather than + // running on into whatever was emitted next. + for (uint8_t f = 0; f < ir.fnCount; f++) { + if (ir.fnIrStart[f] != i) continue; + if (f > 0) closeFn(static_cast(f - 1)); // the previous function returns + // Align BEFORE recording the offset, so the recorded address is the one a caller + // actually jumps to. On Xtensa a function entry must be 4-byte aligned or the call + // cannot be encoded and the instruction itself is illegal; the other backends are + // fixed-width and this costs them nothing. + a.alignForEntry(); + // Recorded AFTER the epilogue and BEFORE the prologue: the offset is the first byte a + // caller executes, which is the frame setup, not the first statement. + ir.fnOffset[f] = static_cast(a.size()); + a.bind(fnLabel[f]); // where a CallScript to this function lands + a.prologue(frameSlots); + // THE RECURSION DEPTH GUARD, in the CALLEE's prologue: the textbook placement, one copy + // per function rather than one per call site, and none at all for a script that never + // calls (which is every shipped script today). + // + // if (++arena[kDepthSlot] >= kMaxCallDepth) return; + // + // A refusing callee RETURNS rather than the caller skipping the call. That is what + // makes the guard cheap: no branch-around at every call site, no counter to restore on + // the way back, and one exit path. The counter is decremented in the epilogue, so it + // unwinds with the frames that incremented it. + // + // Degradation is visible and the device keeps rendering: the deepest calls do nothing, + // so the picture is wrong where the recursion bottomed out, rather than the whole + // device resetting. That is what the robustness rule asks for. + guardPending = guardDepth ? int(f) + 1 : 0; // emit it after the args are parked + } + + if (ir.ops[i].op != IrOp::Spill) flushGuard(); + + const IrInst& op = ir.ops[i]; + switch (op.op) { + case IrOp::Const: a.movImm(reg(op.dst), op.imm); break; + case IrOp::Add: a.addReg(reg(op.dst), reg(op.a), reg(op.b)); break; + case IrOp::AddImm: a.addImm(reg(op.dst), reg(op.a), op.imm); break; + case IrOp::Mul: a.mulReg(reg(op.dst), reg(op.a), reg(op.b)); break; + // A real register move, NOT add-immediate-zero: Xtensa's addi.n cannot encode 0, since + // the ISA reuses that slot for -1, so `dst = a + 0` silently computed a - 1. A loop + // counter initialized through Mov therefore started at -1, the unsigned loop guard saw + // 0xffffffff >= limit, and the body never ran. It compiled, reported no error, and + // placed no lights. + case IrOp::Mov: a.movReg(reg(op.dst), reg(op.a)); break; + case IrOp::Label: + if (op.imm >= 0 && op.imm < kIrLabels) a.bind(labelFor(op.imm)); + break; + case IrOp::BranchGe: + if (op.imm >= 0 && op.imm < kIrLabels) + a.branchGeU(reg(op.a), reg(op.b), labelFor(op.imm)); + break; + case IrOp::BranchNe: + if (op.imm >= 0 && op.imm < kIrLabels) + a.branchNe(reg(op.a), reg(op.b), labelFor(op.imm)); + break; + case IrOp::LoadCtrl: a.load8(reg(op.dst), host(kArg4), op.imm); break; // dst = ctrls[imm] + // The allocator's two ops. `imm` is a slot INDEX; the assembler owns the frame layout. + case IrOp::CallScript: { + // `imm` is the callee's function NUMBER, so the label is a direct index. The + // callee's address is not known when the call is emitted (it may be defined further + // down, and for recursion it is the enclosing function), so this binds a label and + // the assembler patches the displacement once every function is placed. + if (op.imm < 0 || op.imm >= ir.fnCount) break; + a.callLabel(fnLabel[op.imm]); + break; + } + case IrOp::Spill: a.spillStore(reg(op.a), static_cast(op.imm)); break; + case IrOp::Reload: a.spillLoad(reg(op.dst), static_cast(op.imm)); break; + case IrOp::Call: { + // The IR carries the host's function pointer (valid in the single flashed image) and + // the FRAME SLOT its arguments start at. Hand the host their address and their + // count: nothing is held in a register across the call, and how many arguments a + // builtin takes stops being a property of this instruction. + if (!op.callFn) return 0; + const RegId argPtr = static_cast(ir.vregsUsed); + a.slotAddr(argPtr, static_cast(op.imm)); + const RegId argN = static_cast(ir.vregsUsed + 1); + a.movImm(argN, static_cast(op.b)); + a.call(reg(op.dst), argPtr, argN, + host(kArg4), reinterpret_cast(op.callFn)); + break; + } + case IrOp::Inline: + switch (op.inlineOp) { + case InlineOp::StoreElem: { + LabelId skip = a.newLabel(); + // The address goes in SCRATCH, not the index vreg: folding it in destroyed + // a `for` counter, which the loop's step and test read again after the store. + a.branchGeU(reg(op.a), host(kArg1), skip); + a.mulReg(sAddr, reg(op.a), host(kArg2)); // addr = index * cpl + a.store8(host(kArg0), sAddr, reg(op.b)); + a.addImm(sAddr, sAddr, 1); a.store8(host(kArg0), sAddr, reg(op.c)); + a.addImm(sAddr, sAddr, 1); a.store8(host(kArg0), sAddr, reg(op.d)); + a.bind(skip); + break; + } + case InlineOp::FillElems: { + LabelId done = a.newLabel(), top = a.newLabel(); + a.movImm(sCtr, 0); + a.branchIfZero(host(kArg1), done); + a.bind(top); + a.mulReg(sAddr, sCtr, host(kArg2)); + a.store8(host(kArg0), sAddr, reg(op.a)); + a.addImm(sAddr, sAddr, 1); a.store8(host(kArg0), sAddr, reg(op.b)); + a.addImm(sAddr, sAddr, 1); a.store8(host(kArg0), sAddr, reg(op.c)); + a.addImm(sCtr, sCtr, 1); + a.branchNe(sCtr, host(kArg1), top); + a.bind(done); + break; + } + } + break; + default: break; + } + } + // Close the LAST function (or the single routine of a function-less program, which has no + // guard because it cannot call). + if (ir.fnCount > 0) closeFn(static_cast(ir.fnCount - 1)); + else a.epilogue(); + a.finalize(); + if (a.overflowed() || a.size() > cap) return 0; + std::memcpy(out, a.bytes(), a.size()); + return a.size(); +} + +} // namespace mm::moonlive diff --git a/src/light/drivers/Drivers.h b/src/light/drivers/Drivers.h index 8af36c73..770f19c0 100644 --- a/src/light/drivers/Drivers.h +++ b/src/light/drivers/Drivers.h @@ -112,7 +112,7 @@ class Drivers : public MoonModule { /// Stop the core-1 encode worker so a STRUCTURAL TREE MUTATION (a module replace / delete / add) can /// free tree nodes without the worker dereferencing them mid-tick. The worker ticks the driver /// children, and a driver walks the whole Layouts/Layer tree (PreviewDriver::sendFrame → - /// Layouts::forEachCoord), so freeing ANY layout/layer/driver node while core 1 runs is a + /// Layouts::placeLights), so freeing ANY layout/layer/driver node while core 1 runs is a /// use-after-free — a LoadProhibited fault (e.g. replacing a layout on a running split device). The /// mutation path (HttpServerModule) calls this before the free; the trailing prepareTree() re-engages /// the split. Idempotent + safe when the split is off (stopEncodeTask guards on the task handle). diff --git a/src/light/drivers/PreviewDriver.h b/src/light/drivers/PreviewDriver.h index e9255df8..a816bea6 100644 --- a/src/light/drivers/PreviewDriver.h +++ b/src/light/drivers/PreviewDriver.h @@ -232,7 +232,7 @@ class PreviewDriver : public DriverBase { nrOfLightsType n = layouts->totalLightCount(); if (n == 0) return; - // Box EXTENT = the maximum coordinate the positions reach, which is (size − 1): forEachCoord + // Box EXTENT = the maximum coordinate the positions reach, which is (size − 1): placeLights // emits x in [0, width−1], so an 8-wide grid spans 0..7 and its extent is 7, NOT 8. The // header carries these extents and the browser centres the cloud by dividing by the largest, // so they must match the packed coordinates' span exactly — using the size (8) instead drew @@ -276,7 +276,7 @@ class PreviewDriver : public DriverBase { // Count the lights the lattice keeps. A dense grid in natural order (no LUT) is a regular // box, so the kept count is closed-form: ceil(size/s) per axis — no walk. A sparse/mapped - // layout (LUT) has an arbitrary index↔position map, so it's counted by one forEachCoord + // layout (LUT) has an arbitrary index↔position map, so it's counted by one placeLights // pass applying the same lattice predicate the color/coord passes use (color[k] ↔ coord[k] // line up by shared order, no stored index map). if (denseGrid()) { @@ -287,7 +287,7 @@ class PreviewDriver : public DriverBase { CountCtx cc{s, 0}; // A gap is a real preview position (drawn dark at its (x,y,z)), so count/emit it like any // light — blackCb null → blackPixel falls back to the same handler. - layouts->forEachCoord(CoordSink{[](void* c, nrOfLightsType, lengthType x, lengthType y, lengthType z) { + layouts->placeLights(CoordSink{[](void* c, nrOfLightsType, lengthType x, lengthType y, lengthType z) { auto* p = static_cast(c); if (x % p->s == 0 && y % p->s == 0 && z % p->s == 0) p->out++; }, nullptr, &cc}); @@ -307,7 +307,7 @@ class PreviewDriver : public DriverBase { keptIdxAllocFailed_ = false; publishHeapBytes(); // the index cache grew — refresh the memory readout } else { - keptIdxAllocFailed_ = true; // degraded — the gather walks forEachCoord per frame + keptIdxAllocFailed_ = true; // degraded: the gather walks placeLights per frame } } } @@ -329,7 +329,7 @@ class PreviewDriver : public DriverBase { broadcaster_->pushBinaryFrame(h, sizeof(h)); // Push the kept lights' scaled positions in small slices through a stack scratch. A dense // grid strides its box directly (closed-form, no walk over skipped cells); a sparse/mapped - // layout walks forEachCoord with the lattice predicate. BOTH visit the kept lights in the + // layout walks placeLights with the lattice predicate. BOTH visit the kept lights in the // SAME order the color pass uses, so color[k] ↔ coord[k] line up. The C callback can't // capture, so it shares PosCtx (used by both the dense loop and the sparse callback). struct PosCtx { @@ -350,10 +350,10 @@ class PreviewDriver : public DriverBase { for (lengthType x = 0; x < ax; x += s) pc.emit(x, y, z); } else { // While emitting coords, CACHE the kept lights' buffer indices — the per-frame color - // gather then loops this index map instead of re-walking forEachCoord over every light + // gather then loops this index map instead of re-walking placeLights over every light // (an O(total-lights) callback walk per firing, measured ~8 ms at 12K lights on the // encode worker). The map's lifecycle IS the coord table's: same pass, same invalidation. - layouts->forEachCoord(CoordSink{[](void* c, nrOfLightsType idx, lengthType x, lengthType y, lengthType z) { + layouts->placeLights(CoordSink{[](void* c, nrOfLightsType idx, lengthType x, lengthType y, lengthType z) { auto* p = static_cast(c); if (x % p->s != 0 || y % p->s != 0 || z % p->s != 0) return; PreviewDriver* self = p->self; @@ -408,7 +408,7 @@ class PreviewDriver : public DriverBase { // kept subset + order MUST match the coord table's, so color[k] ↔ coord[k] line up (the // browser drops a count/stride-mismatched frame). A dense grid strides its box directly — // light (x,y,z) is at buffer index z·H·W + y·W + x, closed-form, no walk over skipped cells. A - // sparse/mapped layout walks forEachCoord with the same lattice predicate (its index↔position + // sparse/mapped layout walks placeLights with the same lattice predicate (its index↔position // map is arbitrary — no formula). tick()'s idle gate means no drain holds stage_ right now. // TRANSPORT A/B (resumableFrames): ON gathers into the staging buffer and hands it to the // RESUMABLE sender (drains on tick20ms, off the render thread — the sub-hot-path fix). OFF is @@ -454,7 +454,7 @@ class PreviewDriver : public DriverBase { // Fallback (index-map alloc miss): the full lattice walk, s as the FULL stride (not // clamped) — must match buildAndSendCoordTable's. struct Skip { ColCtx* col; nrOfLightsType s; } sk{&col, s}; - layer_->layouts()->forEachCoord(CoordSink{[](void* c, nrOfLightsType idx, lengthType x, lengthType y, lengthType z) { + layer_->layouts()->placeLights(CoordSink{[](void* c, nrOfLightsType idx, lengthType x, lengthType y, lengthType z) { auto* p = static_cast(c); if (x % p->s != 0 || y % p->s != 0 || z % p->s != 0) return; p->col->emit(idx); @@ -496,7 +496,7 @@ class PreviewDriver : public DriverBase { /// Publish the preview's operating status: PLAIN "previewing N points" normally, or a WARNING naming /// the degradation when a resumable-path buffer could not allocate (RAM-tight board) so the tab shows /// WHY it fell back — the synchronous send returns (blocking socket writes on the encode thread, the - /// LED-hitch this optimization removed) or the sparse gather walks forEachCoord per frame. Called from + /// LED-hitch this optimization removed) or the sparse gather walks placeLights per frame. Called from /// the cold path (prepare) and refreshed on the coord rebuild, never the render loop. void refreshStatus() { if (resumableFrames && stageAllocFailed_) { @@ -549,9 +549,9 @@ class PreviewDriver : public DriverBase { // the graceful fallback above the cap. // True when the source is a dense grid in natural box order (no mapping LUT): driver index i is // exactly box cell i, so the kept-light set + each light's buffer index are CLOSED-FORM from the - // box dimensions and the stride — no forEachCoord walk needed (the count, the coord positions, + // box dimensions and the stride: no placeLights walk needed (the count, the coord positions, // and the downsampled colors all stride the box directly). A LUT means a sparse / serpentine / - // modified layout whose index↔position map is arbitrary, so those paths must walk forEachCoord. + // modified layout whose index↔position map is arbitrary, so those paths must walk placeLights. // Mirrors the Layer's own dense-vs-LUT decision (Layer::isNaturalOrder gates lut_.setIdentity), // so the two agree: no LUT ⇔ Drivers passed the dense box buffer ⇔ closed-form is valid here. bool denseGrid() const { return layer_ && !layer_->lut().hasLUT(); } diff --git a/src/light/layers/Layer.h b/src/light/layers/Layer.h index 70034a47..b4e8c064 100644 --- a/src/light/layers/Layer.h +++ b/src/light/layers/Layer.h @@ -73,7 +73,7 @@ class Layer : public MoonModule { void setLayouts(Layouts* lg) { layouts_ = lg; } // The active Layouts, for consumers that need per-light coordinates (e.g. - // PreviewDriver builds its coordinate table from layouts()->forEachCoord). + // PreviewDriver builds its coordinate table from layouts()->placeLights). Layouts* layouts() const { return layouts_; } /// Channels per light (3 = RGB, 4 = RGBW, more for fixture profiles). Zero is not a valid /// light: it would allocate a zero-byte buffer and make every effect's per-light stride 0, so @@ -114,7 +114,7 @@ class Layer : public MoonModule { // real position), so one callback handles both kinds — blackCb null → blackPixel falls back. struct DimCtx { lengthType maxX, maxY, maxZ; }; DimCtx dctx{0, 0, 0}; - layouts_->forEachCoord(CoordSink{[](void* ctx, nrOfLightsType, lengthType x, lengthType y, lengthType z) { + layouts_->placeLights(CoordSink{[](void* ctx, nrOfLightsType, lengthType x, lengthType y, lengthType z) { auto* d = static_cast(ctx); if (x > d->maxX) d->maxX = x; if (y > d->maxY) d->maxY = y; @@ -417,7 +417,7 @@ class Layer : public MoonModule { static constexpr nrOfLightsType kNoDriver = static_cast(-1); // Does the layout emit lights in natural box order — driver index i == box cell i (x fastest, - // then y, then z)? Measured, not declared: one allocation-free forEachCoord pass over the same + // then y, then z)? Measured, not declared: one allocation-free placeLights pass over the same // coords the build would walk, so there's a single source of truth (the coords) and no // per-layout hint to keep in sync. True → the dense memcpy fast path is valid; false → a // reordered grid (serpentine) needs the folded LUT. Only meaningful for a dense layout @@ -427,7 +427,7 @@ class Layer : public MoonModule { Ctx ctx{physicalWidth_, physicalHeight_, true}; // Only reached for a gap-free layout (a gap routes to the folded build before this is asked), // so blackCb is null and gaps, were there any, would fall back to the same order check. - layouts_->forEachCoord(CoordSink{[](void* c, nrOfLightsType driverIdx, lengthType x, lengthType y, lengthType z) { + layouts_->placeLights(CoordSink{[](void* c, nrOfLightsType driverIdx, lengthType x, lengthType y, lengthType z) { auto* k = static_cast(c); if (!k->ok) return; // once a mismatch is found the answer is settled; skip the rest nrOfLightsType box = static_cast(z) * k->w * k->h @@ -442,7 +442,7 @@ class Layer : public MoonModule { // in-order writes — but folding scatters onto arbitrary, repeated logical indices. // So this is the textbook counting-sort CSR build: pass A counts destinations per // logical cell, prefix-sum to offsets, pass B scatters, then replay through - // setMapping in logical order. Two forEachCoord passes + a counts/dests scratch, + // setMapping in logical order. Two placeLights passes + a counts/dests scratch, // all on the cold rebuild path; the hot-path read (forEachDestination) is unchanged. // Returns false on OOM (caller degrades to identity). bool buildFoldedLUT(const Coord3D& logical, @@ -467,7 +467,7 @@ class Layer : public MoonModule { // (the Layer's own children — enabled static modifiers, in order, no array) to a // logical index (or skips it if a modifier rejects it or it lands out of box — // guarded, never trusted), then either counts it (pass A) or writes the driver - // index at the cell's cursor (pass B). Everything travels through the forEachCoord + // index at the cell's cursor (pass B). Everything travels through the placeLights // void* ctx, so the lambda captures nothing (it's a function ptr). struct FoldCtx { Layer* self; // for the dynamic child list (the modifier chain) @@ -496,7 +496,7 @@ class Layer : public MoonModule { static_cast(pos.x); if (li >= f->logicalCount) return; // defensive // Pass B writes where pass A counted — safe only while both passes see the SAME - // coordinates. A scripted layout compiles lazily inside forEachCoord, so a control + // coordinates. A scripted layout compiles lazily inside placeLights, so a control // edited between the two passes makes pass B emit more lights than pass A counted and // the scatter runs past dests. That corrupts the heap; the failure then surfaces in an // unrelated allocation, which is what made resizing a scripted layout crash at random. @@ -520,7 +520,7 @@ class Layer : public MoonModule { [](void*, nrOfLightsType, lengthType, lengthType, lengthType) {}; // Pass A — count. - layouts_->forEachCoord(CoordSink{onCoord, kDropGap, &fctx}); + layouts_->placeLights(CoordSink{onCoord, kDropGap, &fctx}); // Prefix-sum counts → offsets (counts[li] becomes the start of cell li's run). nrOfLightsType running = 0; @@ -533,7 +533,7 @@ class Layer : public MoonModule { // Pass B — scatter. counts[] is now the per-cell write cursor (offsets advance). fctx.scatter = true; - layouts_->forEachCoord(CoordSink{onCoord, kDropGap, &fctx}); + layouts_->placeLights(CoordSink{onCoord, kDropGap, &fctx}); // Pass B advanced each cell's cursor to the END of its run, so counts[i] now // holds the end offset of cell i — which equals the START offset of cell i+1. diff --git a/src/light/layouts/CarLightsLayout.h b/src/light/layouts/CarLightsLayout.h index 9d93ad5f..6da71494 100644 --- a/src/light/layouts/CarLightsLayout.h +++ b/src/light/layouts/CarLightsLayout.h @@ -26,7 +26,7 @@ namespace mm { // (int)x / (int)y truncation). The external-ringCenter form is the one CarLights // needs, which is why we don't delegate to RingLayout.h. // -// Float trig runs on the cold build path (forEachCoord / lightCount, called from +// Float trig runs on the cold build path (placeLights / lightCount, called from // a rebuild), never the hot render loop, so it's allowed here. MoonLight's // pin/wiring plumbing (nextPin / doNextPin) is dropped — a projectMM layout // emits coordinates only; the driver owns pins. @@ -53,7 +53,7 @@ class CarLightsLayout : public LayoutBase { return n; } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { walk(sink.cb, sink.ctx, nullptr); } diff --git a/src/light/layouts/CubeLayout.h b/src/light/layouts/CubeLayout.h index e66e20b6..00af79a7 100644 --- a/src/light/layouts/CubeLayout.h +++ b/src/light/layouts/CubeLayout.h @@ -72,7 +72,7 @@ class CubeLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // Choose the axis order (which loop slot drives which axis). axes[0] is // the OUTERMOST loop's axis, axes[2] the innermost (fastest). Verbatim // from MoonLight: index 0=X,1=Y,2=Z. Guard an out-of-range select. @@ -142,7 +142,7 @@ class CubeLayout : public LayoutBase { // Resolve one serpentine pass's value for step `step` (0..count-1). Base // direction from `inc`; `snake` flips it when the enclosing counter `prev` - // is odd — the boustrophedon toggle. RECONSTRUCTED (see forEachCoord). + // is odd: the boustrophedon toggle. RECONSTRUCTED (see placeLights). static lengthType axisValue(lengthType step, lengthType count, bool inc, bool snake, lengthType prev) { bool ascending = inc; diff --git a/src/light/layouts/GridBlacksLayout.h b/src/light/layouts/GridBlacksLayout.h index 93635e15..e2cec5b1 100644 --- a/src/light/layouts/GridBlacksLayout.h +++ b/src/light/layouts/GridBlacksLayout.h @@ -48,7 +48,7 @@ class GridBlacksLayout : public LayoutBase { // rather than take the identity fast path (which would light them). No run → renders like a Grid. bool hasBlackPixels() const override { return blackCount != 0; } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // uint32_t idx so it never wraps on uint16_t nrOfLightsType (no-PSRAM 512×512 > 65535); stop at // the clamped lightCount() so emitted indices stay within the allocated buffer. const uint32_t limit = lightCount(); diff --git a/src/light/layouts/GridLayout.h b/src/light/layouts/GridLayout.h index e9082d90..4df64d9a 100644 --- a/src/light/layouts/GridLayout.h +++ b/src/light/layouts/GridLayout.h @@ -35,7 +35,7 @@ class GridLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // Use uint32_t for idx so it never wraps on uint16_t nrOfLightsType // (e.g. no-PSRAM ESP32 where 512×512 > 65535). Stop at the clamped // lightCount() so emitted indices stay within the allocated buffer. diff --git a/src/light/layouts/HumanSizedCubeLayout.h b/src/light/layouts/HumanSizedCubeLayout.h index 23d66f60..96157d0d 100644 --- a/src/light/layouts/HumanSizedCubeLayout.h +++ b/src/light/layouts/HumanSizedCubeLayout.h @@ -38,7 +38,7 @@ class HumanSizedCubeLayout : public LayoutBase { const char* tags() const override { return "💫"; } nrOfLightsType lightCount() const override { - // Sum of the five face areas, matching the loop bounds in forEachCoord: + // Sum of the five face areas, matching the loop bounds in placeLights: // front + back : width*height each (2 * w*h) // above : width*depth (w*d) // left + right : depth*height each (2 * d*h) @@ -49,7 +49,7 @@ class HumanSizedCubeLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { const uint32_t limit = lightCount(); uint32_t idx = 0; diff --git a/src/light/layouts/LayoutBase.h b/src/light/layouts/LayoutBase.h index 13af97dd..d86d107d 100644 --- a/src/light/layouts/LayoutBase.h +++ b/src/light/layouts/LayoutBase.h @@ -9,7 +9,7 @@ // class MyLayout : public LayoutBase { ... }; // } // -// A layout overrides lightCount() and forEachCoord() (reporting each light's (x,y,z)). The helper set +// A layout overrides lightCount() and placeLights() (reporting each light's (x,y,z)). The helper set // below is the whole surface a layout commonly reaches for — the integer + float trig and the small // standard helpers coordinate placement uses. Unused declarations cost zero firmware bytes; a layout // needing something outside this surface adds that one extra include. @@ -60,14 +60,14 @@ struct CoordSink { }; /// Base for one layout child of the `Layouts` container. A concrete layout -/// (grid, sphere shell, …) implements `lightCount` and `forEachCoord` directly — +/// (grid, sphere shell, …) implements `lightCount` and `placeLights` directly , /// no wrapper. Every layout control changes the physical light count, so any /// control change triggers the pipeline-wide rebuild. class LayoutBase : public MoonModule { public: ModuleRole role() const MM_NONBLOCKING override { return ModuleRole::Layout; } virtual nrOfLightsType lightCount() const = 0; - virtual void forEachCoord(const CoordSink& sink) const = 0; + virtual void placeLights(const CoordSink& sink) const = 0; /// Whether this layout emits any GAP (black) pixels — physical wire slots held dark. Default /// false: a layout with no dark regions never overrides this and stays unaware gaps exist. Gates diff --git a/src/light/layouts/Layouts.h b/src/light/layouts/Layouts.h index 103790c5..5d458fc4 100644 --- a/src/light/layouts/Layouts.h +++ b/src/light/layouts/Layouts.h @@ -11,7 +11,7 @@ namespace mm { /// Top-level container for one or more `LayoutBase` children — it defines the physical light topology of the installation and is shared by every Layer in the `Effects` container (one Layouts describing the physical setup, multiple Effects render into it). /// -/// **Coordinate iteration is owned by the container, not the layer:** `forEachCoord` walks every enabled child layout's coordinates in registration order, offsetting physical indices so multiple layouts (for example 16 strips making one panel) stitch into a single flat physical address space without overlap. A Layer *uses* those coordinates to build its LUT. `totalLightCount` (the sum across enabled children) sizes both the layer buffer and the driver output buffer. +/// **Coordinate iteration is owned by the container, not the layer:** `placeLights` walks every enabled child layout's coordinates in registration order, offsetting physical indices so multiple layouts (for example 16 strips making one panel) stitch into a single flat physical address space without overlap. A Layer *uses* those coordinates to build its LUT. `totalLightCount` (the sum across enabled children) sizes both the layer buffer and the driver output buffer. /// /// **Disabling a layout:** disabling a layout child (the `enabled` toggle) removes its lights from the LUT entirely, and the indices of any layouts after it shift down to close the gap — with two grids of 4 and 2 lights, disabling the first leaves the second at indices 0–1 and `totalLightCount` drops from 6 to 2. A `Scheduler::prepareTree` fires so the LUT, layer buffer, and driver output buffer reallocate. Side effect: ArtNet universe assignments shift with the indices — to keep driver-to-fixture mapping stable across enable changes, disable the driver instead of the layout. Disabling the container itself reports zero lights and an empty iteration, the same effect as disabling every child. /// @@ -46,7 +46,7 @@ class Layouts : public MoonModule { return total; } - void forEachCoord(const CoordSink& sink) const { + void placeLights(const CoordSink& sink) const { if (!enabled()) return; nrOfLightsType offset = 0; for (uint8_t i = 0; i < childCount(); i++) { @@ -60,7 +60,7 @@ class Layouts : public MoonModule { nrOfLightsType offset; }; WrapCtx wctx{&sink, offset}; - layout->forEachCoord(CoordSink{ + layout->placeLights(CoordSink{ [](void* wc, nrOfLightsType idx, lengthType x, lengthType y, lengthType z) { auto* w = static_cast(wc); w->sink->pixel(idx + w->offset, x, y, z); @@ -93,11 +93,11 @@ class Layouts : public MoonModule { /// setup (no lights / zero box) flags Warning so the UI shows it's empty. void prepare() override { const nrOfLightsType lights = totalLightCount(); - // One forEachCoord pass for the bounding box: max coordinate + 1 per axis. + // One placeLights pass for the bounding box: max coordinate + 1 per axis. struct Extent { lengthType x, y, z; bool any; } e{0, 0, 0, false}; // Gaps count toward the physical box (a black pixel is a real position at (x,y,z)), so the // extent walk uses one callback for both kinds — blackCb null → blackPixel falls back to it. - forEachCoord(CoordSink{[](void* ctx, nrOfLightsType, lengthType x, lengthType y, lengthType z) { + placeLights(CoordSink{[](void* ctx, nrOfLightsType, lengthType x, lengthType y, lengthType z) { auto* ex = static_cast(ctx); if (x > ex->x) ex->x = x; if (y > ex->y) ex->y = y; diff --git a/src/light/layouts/PanelLayout.h b/src/light/layouts/PanelLayout.h index 71beab68..1e2d3619 100644 --- a/src/light/layouts/PanelLayout.h +++ b/src/light/layouts/PanelLayout.h @@ -62,7 +62,7 @@ class PanelLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // MoonLight: axes = axisOrders[wiringOrder]; XY(0) = {1,0} (Y outer, X inner), // YX(1) = {0,1} (X outer, Y inner). axes[0] is the outer axis, axes[1] the inner. const uint8_t axisOrders[2][2] = { diff --git a/src/light/layouts/PanelsLayout.h b/src/light/layouts/PanelsLayout.h index c4c7fe7a..75e9a7d7 100644 --- a/src/light/layouts/PanelsLayout.h +++ b/src/light/layouts/PanelsLayout.h @@ -78,7 +78,7 @@ class PanelsLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // MoonLight: axes = axisOrders[wiringOrder]; XY(0) = {1,0} (Y outer, X inner), // YX(1) = {0,1} (X outer, Y inner). axes[0] is the outer axis, axes[1] the inner. // The SAME table drives both the panel-grid walk and the per-panel walk. diff --git a/src/light/layouts/RingLayout.h b/src/light/layouts/RingLayout.h index df17fd9c..4328bfc4 100644 --- a/src/light/layouts/RingLayout.h +++ b/src/light/layouts/RingLayout.h @@ -17,7 +17,7 @@ namespace mm { // pin/wiring plumbing (doNextPin/nextPin) is dropped — a projectMM layout emits // coordinates only; the driver owns pins. // -// Float trig runs on the cold build path (forEachCoord / lightCount, called from a +// Float trig runs on the cold build path (placeLights / lightCount, called from a // rebuild), never the hot render loop, so it's allowed here. // Author: MoonLight — https://github.com/MoonModules/MoonLight/blob/main/src/MoonLight/Nodes/Layouts/L_MoonLight.h /// Layout of a single ring of evenly-spaced LEDs. @@ -39,14 +39,14 @@ class RingLayout : public LayoutBase { } nrOfLightsType lightCount() const override { - // Reuse the exact inclusion predicate as forEachCoord so count and emit + // Reuse the exact inclusion predicate as placeLights so count and emit // never disagree (a partial arc emits fewer than nrOfLEDs lights). nrOfLightsType n = 0; walk([](void*, nrOfLightsType, lengthType, lengthType, lengthType) {}, nullptr, &n); return n; } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { walk(sink.cb, sink.ctx, nullptr); } diff --git a/src/light/layouts/Rings241Layout.h b/src/light/layouts/Rings241Layout.h index 77b401e7..e13dfc80 100644 --- a/src/light/layouts/Rings241Layout.h +++ b/src/light/layouts/Rings241Layout.h @@ -42,13 +42,13 @@ class Rings241Layout : public LayoutBase { nrOfLightsType lightCount() const override { // Fixed by construction: the nine ring sizes always sum to 241, and every // ring is a full circle so no LED is culled. Kept in lockstep with - // forEachCoord below (same kRingSizes sum). + // placeLights below (same kRingSizes sum). nrOfLightsType total = 0; for (uint8_t n : kRingSizes) total += n; return total; // 1+8+12+16+24+32+40+48+60 = 241 } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // Shared centre — MoonLight: leftMargin = 1.1 * getRadius(60), assigned to // a uint8_t (implicit truncation), stored as ringCenter's integer x/y, then // scaled per LED: x = scale * ringCenter.x. diff --git a/src/light/layouts/SingleColumnLayout.h b/src/light/layouts/SingleColumnLayout.h index d5499f55..1f6084b6 100644 --- a/src/light/layouts/SingleColumnLayout.h +++ b/src/light/layouts/SingleColumnLayout.h @@ -35,7 +35,7 @@ class SingleColumnLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // Emit the column in wiring order. The COORDINATE is (xposition, y, 0); // reversed_order walks y from the high end down, matching MoonLight's // addLight order. Stop at the clamped lightCount() so emitted indices diff --git a/src/light/layouts/SingleRowLayout.h b/src/light/layouts/SingleRowLayout.h index ed4ee192..4fd38e3d 100644 --- a/src/light/layouts/SingleRowLayout.h +++ b/src/light/layouts/SingleRowLayout.h @@ -43,7 +43,7 @@ class SingleRowLayout : public LayoutBase { return static_cast(width); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // The coordinate at index i is fixed at row yPosition, z=0; only the x walk // direction depends on reversedOrder — the two branches mirror MoonLight's // onLayout() forward/reverse loops exactly. diff --git a/src/light/layouts/SphereLayout.h b/src/light/layouts/SphereLayout.h index e3dc14f6..e7e66858 100644 --- a/src/light/layouts/SphereLayout.h +++ b/src/light/layouts/SphereLayout.h @@ -9,7 +9,7 @@ namespace mm { // (2r+1)^3 bounding box, centred at (r,r,r). A lattice point is on the shell // when its distance from the centre rounds to `radius`, i.e. it falls in the // half-open band [radius-0.5, radius+0.5). The same band predicate drives both -// lightCount() (count) and forEachCoord() (emit), so they never disagree. +// lightCount() (count) and placeLights() (emit), so they never disagree. // // Distances are compared squared (integer math, no sqrt/float per light) — the // hot-path discipline (integer math, no float per light) applies here even @@ -37,7 +37,7 @@ class SphereLayout : public LayoutBase { return n; } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { forEachShellPoint(sink.cb, sink.ctx, nullptr); } diff --git a/src/light/layouts/SpiralLayout.h b/src/light/layouts/SpiralLayout.h index 603214eb..4a65ae8a 100644 --- a/src/light/layouts/SpiralLayout.h +++ b/src/light/layouts/SpiralLayout.h @@ -16,7 +16,7 @@ namespace mm { // // Prior art: MoonLight SpiralLayout (MoonModules/MoonLight, src light nodes). // Geometry reproduced exactly — the float trig runs on the cold build path -// (forEachCoord is called from a rebuild, not the render loop), so MoonLight's +// (placeLights is called from a rebuild, not the render loop), so MoonLight's // sinf/cosf and the float→integer truncation on each coordinate are kept as-is. // MoonLight's per-strip pin plumbing (nextPin) is dropped: a projectMM layout // emits coordinates only; the driver owns wiring. @@ -43,7 +43,7 @@ class SpiralLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { const uint32_t limit = lightCount(); if (limit == 0) return; diff --git a/src/light/layouts/TorontoBarGourdsLayout.h b/src/light/layouts/TorontoBarGourdsLayout.h index b1842852..8ed2e15e 100644 --- a/src/light/layouts/TorontoBarGourdsLayout.h +++ b/src/light/layouts/TorontoBarGourdsLayout.h @@ -25,7 +25,7 @@ namespace mm { // the MoonLight lineage. // // A single walk() is the one source of truth for the geometry: lightCount() -// runs it with a no-op callback to tally, forEachCoord() runs it to emit, so the +// runs it with a no-op callback to tally, placeLights() runs it to emit, so the // count and the emitted set can never disagree (the RingLayout/SphereLayout // pattern). Integer math throughout; this is the cold build path. // Author: troyhacks — custom Toronto bar decorative-gourd installation, reconstructed for projectMM — https://github.com/troyhacks/WLED @@ -53,7 +53,7 @@ class TorontoBarGourdsLayout : public LayoutBase { return n; } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { walk(sink.cb, sink.ctx, nullptr); } diff --git a/src/light/layouts/TubesLayout.h b/src/light/layouts/TubesLayout.h index 7bb5e15e..0fd46a19 100644 --- a/src/light/layouts/TubesLayout.h +++ b/src/light/layouts/TubesLayout.h @@ -38,7 +38,7 @@ class TubesLayout : public LayoutBase { nrOfLightsType lightCount() const override { // A restored/persisted value can be negative (the controls are signed int16); a negative - // dimension emits no coordinates in forEachCoord(), so report 0 here to match rather than + // dimension emits no coordinates in placeLights(), so report 0 here to match rather than // casting to uint32_t and wrapping to a huge count. Multiply in uint32_t to detect overflow. if (nrOfTubes <= 0 || ledsPerTube <= 0) return 0; uint32_t n = static_cast(nrOfTubes) * static_cast(ledsPerTube); @@ -46,7 +46,7 @@ class TubesLayout : public LayoutBase { return static_cast(n > kMax ? kMax : n); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { // uint32_t idx so it never wraps on a uint16_t nrOfLightsType; stop at the // clamped lightCount() so emitted indices stay within the buffer. const uint32_t limit = lightCount(); diff --git a/src/light/layouts/WheelLayout.h b/src/light/layouts/WheelLayout.h index ee2bfc66..aacf8f62 100644 --- a/src/light/layouts/WheelLayout.h +++ b/src/light/layouts/WheelLayout.h @@ -34,7 +34,7 @@ class WheelLayout : public LayoutBase { return static_cast(spokes) * static_cast(ledsPerSpoke); } - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { const int32_t maxR = ledsPerSpoke; // outermost radius (centre shift) nrOfLightsType idx = 0; for (uint16_t s = 0; s < spokes; s++) { diff --git a/src/light/moonlive/MoonLiveBuiltins_light.h b/src/light/moonlive/MoonLiveBuiltins_light.h index f9ed89c5..0a0a854c 100644 --- a/src/light/moonlive/MoonLiveBuiltins_light.h +++ b/src/light/moonlive/MoonLiveBuiltins_light.h @@ -345,43 +345,60 @@ enum : uint8_t { /// /// Adding one is a single line here plus the binding writing its slot. -/// `t` alone — every script animates, so every list starts here. -inline void addClock(SysVarTable& t) { +/// The entry points the light domain calls, by role. A script defines the ones its role needs; the +/// engine looks each up by name in the one emitted block. +/// +/// A name is a MOMENT, not a role. The host owns the moments and calls whatever the script defined +/// for each one: `tick` when a frame is rendered, `placeLights` when lights are being placed, +/// `modifyLogical` when one coordinate is folded. An entry a script did not define is simply not +/// called, which is why nothing validates which names belong to which module. +/// +/// This is what makes the bindings differ by which moments they OWN rather than by kind, and it is +/// what lets one class serve more than one: an effect that also defines `modifyLogical` gets both, +/// with no feature to add. It also leaves `tick` free to mean something in a layout later without a +/// grammar change. Guarding any of it would be code spent forbidding what a script author is +/// entitled to do, and the cost of a name nothing calls is a function that does not run, which is +/// visible immediately rather than silent. +inline constexpr const char* kEntryTick = "tick"; // an effect, per frame +inline constexpr const char* kEntryPlaceLights = "placeLights"; // a layout, placing lights +inline constexpr const char* kEntryModify = "modifyLogical"; // a modifier, folding one light + +/// The system variables EVERY light script can read. One vocabulary for all three roles, rather +/// than a table per role. +/// +/// The split that preceded this bought less than it cost. It prevented no mistake (a layout reading +/// `width` got a compile error, which is the same outcome as reading a variable that is always +/// zero) and it created a trap: the tables were different vocabularies rather than nested ones, so +/// a name meant one thing in one role and was RESERVED in another. `disasm.py` compiled against the +/// widest table and therefore refused `grid.mlv`, the shipped default layout, with "name is a +/// system variable" -- the tool was blind to the one script most worth inspecting. +/// +/// `width`/`height`/`depth` mean the same thing everywhere: the dimensions of the grid. A layout +/// DEFINES them by the coordinates it places; an effect and a modifier READ them. What a binding +/// still decides is which slots it WRITES each frame; reading is uniform. +/// +/// The per-light coordinate is `xPos`/`yPos`/`zPos`, not `x`/`y`/`z`. Those are the names an author +/// reaches for as loop counters (`grid.mlv` uses both), so reserving them globally would break the +/// most ordinary code there is. Only a modifier is handed a coordinate; elsewhere the slots read 0. +inline SysVarTable lightSysVars() { + SysVarTable t; // Elapsed milliseconds, passed in kArg3 on every run. An argument register, so it costs no // instruction and no arena byte. t.add({"t", SysVarKind::Arg, kArg3}); -} - -/// A LAYOUT: the clock, and nothing else. It is upstream of the logical grid — it contributes the -/// physical coordinates that several layouts together bound (architecture.md § Layouts) — so there -/// is no size to hand it, and it names its own controls (`cols`, `radius`). -inline SysVarTable layoutSysVars() { - SysVarTable t; - addClock(t); - return t; -} - -/// An EFFECT: the logical grid it renders into. The Layer derives width/height/depth from the -/// layouts and its modifier chain and writes them each tick; an effect is TOLD its canvas rather -/// than declaring it, because a size restated as a control is a second answer that can disagree. -inline SysVarTable effectSysVars() { - SysVarTable t; - addClock(t); t.add({"width", SysVarKind::Arena, kSysWidth}); t.add({"height", SysVarKind::Arena, kSysHeight}); t.add({"depth", SysVarKind::Arena, kSysDepth}); + t.add({"xPos", SysVarKind::Arena, kSysX}); + t.add({"yPos", SysVarKind::Arena, kSysY}); + t.add({"zPos", SysVarKind::Arena, kSysZ}); return t; } -/// A MODIFIER: the grid, plus the coordinate of the light being folded, which the binding writes -/// per call. This is the only binding that supplies x/y/z. -inline SysVarTable modifierSysVars() { - SysVarTable t = effectSysVars(); - t.add({"x", SysVarKind::Arena, kSysX}); - t.add({"y", SysVarKind::Arena, kSysY}); - t.add({"z", SysVarKind::Arena, kSysZ}); - return t; -} +/// The three roles keep their names as aliases of the one table: a binding says which role it is +/// playing, and every call site reads the same way it always did. +inline SysVarTable layoutSysVars() { return lightSysVars(); } +inline SysVarTable effectSysVars() { return lightSysVars(); } +inline SysVarTable modifierSysVars() { return lightSysVars(); } // The light-domain built-in table the binding injects into the compiler. setRGB and fill are // Inline (they lower to stores — the hot-path writers, no per-call cost); random16 is a Call. diff --git a/src/light/moonlive/MoonLiveEffect.h b/src/light/moonlive/MoonLiveEffect.h index af471562..0f3794a1 100644 --- a/src/light/moonlive/MoonLiveEffect.h +++ b/src/light/moonlive/MoonLiveEffect.h @@ -101,7 +101,11 @@ class MoonLiveEffect : public EffectBase { // installed for exactly one run and detached after, so a script can only ever draw into // the layer it is ticking in. moonlive::setDrawCanvas(canvas()); - engine_.run(buffer(), nrOfLights(), cpl, elapsed()); + // The frame moment: run `tick` if the script defined one. A script that defines only + // `modifyLogical` renders nothing here and folds coordinates instead, which is the author's + // choice rather than an error. + if (engine_.hasEntry(moonlive::kEntryTick)) + engine_.run(buffer(), nrOfLights(), cpl, elapsed(), moonlive::kEntryTick); moonlive::setDrawCanvas({}); } diff --git a/src/light/moonlive/MoonLiveLayout.h b/src/light/moonlive/MoonLiveLayout.h index fbc50d2b..884560e5 100644 --- a/src/light/moonlive/MoonLiveLayout.h +++ b/src/light/moonlive/MoonLiveLayout.h @@ -29,7 +29,7 @@ // container and have to behave identically through this interface. // // **The count and the coordinates come from the same code.** `lightCount()` runs the script with a -// counting sink; `forEachCoord` runs it again into the caller's. Same script, same arithmetic, so +// counting sink; `placeLights` runs it again into the caller's. Same script, same arithmetic, so // the two answers cannot drift apart — the property SphereLayout's comment names. namespace mm { @@ -77,7 +77,7 @@ class MoonLiveLayout : public LayoutBase { } /// Run the script again, emitting each light into the consumer's sink. - void forEachCoord(const CoordSink& sink) const override { + void placeLights(const CoordSink& sink) const override { compile(); if (!engine_.ok()) return; Emitter e{&sink, 0}; @@ -108,7 +108,7 @@ class MoonLiveLayout : public LayoutBase { private: /// Compile if the source has changed since the program that is loaded. /// - /// Called from prepare(), and also from lightCount()/forEachCoord — because applyState() runs + /// Called from prepare(), and also from lightCount()/placeLights: because applyState() runs /// PARENT-FIRST (MoonModule.h): the container computes its bounding box by walking its children /// before those children have prepared. A layout whose count is arithmetic (GridLayout) does not /// notice; one that needs a compiled program would report an empty fixture to whoever asked @@ -172,7 +172,10 @@ class MoonLiveLayout : public LayoutBase { void runScript(moonlive::AddLightFn fn, void* ctx) const { uint8_t scratch[3] = {0, 0, 0}; moonlive::setAddLightSink(fn, ctx); - const_cast(engine_).run(scratch, 1, 3, 0); + // The placement moment: run `placeLights` if the script defined one. A script without it + // places no lights, which the module reports as an empty fixture rather than a failure. + if (!engine_.hasEntry(moonlive::kEntryPlaceLights)) return; + const_cast(engine_).run(scratch, 1, 3, 0, moonlive::kEntryPlaceLights); moonlive::setAddLightSink(nullptr, nullptr); } @@ -190,7 +193,7 @@ class MoonLiveLayout : public LayoutBase { // Has this script name already been tried and failed? A FAILED compile leaves compiledHash_ at 0 // and the engine not ok(), which is indistinguishable from "not compiled yet" — so without this - // flag every lightCount()/forEachCoord() re-reads and re-compiles the file. Each attempt is two + // flag every lightCount()/placeLights() re-reads and re-compiles the file. Each attempt is two // LittleFS operations (~5 ms on an S3), the pipeline asks repeatedly while sizing and walking the // fixture, and the retries starve the task until the 12 s watchdog resets the device. One attempt // per script name is all that can ever help: nothing about the file changes between two calls in diff --git a/src/light/moonlive/MoonLiveModifier.h b/src/light/moonlive/MoonLiveModifier.h index dd35ad22..43e055dc 100644 --- a/src/light/moonlive/MoonLiveModifier.h +++ b/src/light/moonlive/MoonLiveModifier.h @@ -132,7 +132,11 @@ class MoonLiveModifier : public ModifierBase { // script written against a future `for` loop uses the identical call. uint8_t out[3] = {*sx, *sy, *sz}; // seeded with the input, so a script that writes // nothing leaves the coordinate untouched - self->engine_.run(out, 1, 3, 0); + // The fold moment: run `modifyLogical` if the script defined one, and leave the coordinate + // untouched otherwise. The cold path (once per light at mapping build, not per frame), so + // the lookup costs nothing measurable. + if (!engine_.hasEntry(moonlive::kEntryModify)) return true; + self->engine_.run(out, 1, 3, 0, moonlive::kEntryModify); pos.x = static_cast(out[0]); pos.y = static_cast(out[1]); diff --git a/src/platform/desktop/moonlive_asm_host.cpp b/src/platform/desktop/moonlive_asm_host.cpp index 0181ced1..d59db4c5 100644 --- a/src/platform/desktop/moonlive_asm_host.cpp +++ b/src/platform/desktop/moonlive_asm_host.cpp @@ -46,7 +46,7 @@ void HostAssembler::bind(Label l) { if (l < kMaxLabels) labelPos_[l] = static_ca // Record a pending branch fixup, guarding the fixed table — a script with too many branches sets // overflow_ rather than writing past fixups_ (the same failure path as a full code buffer). -void HostAssembler::addFixup(size_t at, Label label, uint8_t kind) { +void HostAssembler::addFixup(size_t at, Label label, FixKind kind) { if (fixupCount_ >= kMaxFixups) { overflow_ = true; return; } fixups_[fixupCount_++] = {at, label, kind}; } @@ -156,15 +156,22 @@ void HostAssembler::cmp(Reg a, Reg b) { // cmp wA, wB (subs emit32(0x6b00001fu | (mr(b) << 16) | (mr(a) << 5)); } void HostAssembler::branchIfZero(Reg a, Label l) { // cbz wA, l (offset patched) - addFixup(len_, l, 0); + addFixup(len_, l, FixKind::Branch); emit32(0x34000000u | mr(a)); } void HostAssembler::branchIf(Cond c, Label l) { // b.cond l (offset patched) // arm64 condition codes: NE=1, HS/CS=2, LO/CC=3. const uint8_t cond = (c == Cond::Lo) ? 0x3 : (c == Cond::Ne ? 0x1 : 0x2); - addFixup(len_, l, static_cast(1u | (cond << 4))); + addFixup(len_, l, FixKind::Branch); // the condition is already in the instruction emit32(0x54000000u | cond); } +// The fused forms the shared lowering calls. arm64 has no compare-and-branch pair, so each is +// cmp + b.cond here, and one instruction on RISC-V and Xtensa. Both spellings live behind the +// same name, which is what lets the IR walk be written once. +void HostAssembler::movReg(Reg d, Reg a) { addImm(d, a, 0); } // mov wD, wA (add wD, wA, #0) +void HostAssembler::branchGeU(Reg a, Reg b, Label l) { cmp(a, b); branchIf(Cond::Hs, l); } +void HostAssembler::branchNe(Reg a, Reg b, Label l) { cmp(a, b); branchIf(Cond::Ne, l); } + void HostAssembler::call(Reg d, Reg a, Reg b, Reg c, const void* fn) { // Preserve EVERY register that may hold a live value across the call: the host args // (x0/x1/x2/x3), the link register x30 (blr overwrites it; our function is a leaf), and the @@ -218,6 +225,22 @@ void HostAssembler::call(Reg d, Reg a, Reg b, Reg c, const void* fn) { } void HostAssembler::ret() { emit32(0xd65f03c0u); } +// bl