diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 14fe25e..6a7a628 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -165,16 +165,16 @@ jobs: echo "commit it as tests/golden/smoke_test_software.ppm, then re-run." >&2 exit 0 fi - # `test-render-capture` runs ONLY tests/render/capture.zig — it - # bypasses the generic `zig build test` which pulls in every - # test in the repo (some have unrelated ReleaseSafe issues - # tracked as M0.7 housekeeping debt). Scope: PSNR regression - # gate, nothing else. The test itself raises a typed error - # when PSNR < 40 dB, so a non-zero exit propagates through - # pipefail — no separate grep needed (the previous grep on - # "PSNR" matched the test's success log line that does not - # exist, tripping exit 1 under pipefail on every passing run). - zig build test-render-capture -Doptimize=ReleaseSafe --summary all 2>&1 | tee test-output.txt + # M0.5 item 1: direct PSNR comparison of the PPM already produced + # by the previous step against the golden. `test-ppm-psnr` runs + # ONLY tests/render/ppm_psnr_compare.zig, which imports just `std` + # (no `weld_render`): it compiles in seconds and does NOT rebuild + # the render stack or re-spawn the triangle (the prior step already + # wrote out/smoke_test.ppm). Replaces the rebuild-heavy + # `test-render-capture` invocation (~3-5 min/run). The test raises a + # typed error when PSNR < 40 dB, so a non-zero exit propagates + # through pipefail. + zig build test-ppm-psnr -Doptimize=ReleaseSafe --summary all 2>&1 | tee test-output.txt - name: Upload capture artifact if: always() diff --git a/briefs/M0.5-housekeeping.md b/briefs/M0.5-housekeeping.md new file mode 100644 index 0000000..3824a0b --- /dev/null +++ b/briefs/M0.5-housekeeping.md @@ -0,0 +1,230 @@ +# M0.5 — Post-M0.4 Housekeeping + Renderer Debt + +> **Status:** IN PROGRESS +> **Phase:** 0.5 +> **Branch:** `phase-0/housekeeping/post-m0.4` +> **Planned tag:** `v0.5.0-M0.5-housekeeping` +> **Dependencies:** M0.4 (all absorbed debt originates there) +> **Opened:** 2026-06-01 +> **Closed:** — + +--- + +# FROZEN SECTION + +*Produced by Claude.ai. Not modifiable by Claude Code outside a Claude.ai round-trip (cf. § Acted deviations).* + +## Context + +Housekeeping milestone opened after M0.4 (Vulkan forward renderer + GAL). It absorbs the tooling/CI/legacy debt accumulated during the M0.4 final sprint, fixes latent bugs and robustness debt surfaced by the post-M0.4 code review, and applies for the first time the structural conventions ratified at the M0.4 post-mortem (empirical per-consumer surface coverage, `engine-zig-conventions.md §13`; source preservation until parity, `engine-development-workflow.md §2.5`; risk-domain scoping, `engine-development-workflow.md §2.2`). Deliberately bounded — **no new functional surface**. It advances no new C0.x criterion; it consolidates C0.3. + +## Scope + +Eleven items, sourced verbatim from `engine-phase-0-plan.md § M0.5`. Item 6 is transient hardware debt, closed by repair, **not by PR** — tracked for memory, out of code scope. + +- **Item 1** — CI `runtime-smoke-test` PSNR redundancy: the `Verify PSNR vs golden` step rebuilds all tests in ReleaseSafe although the PPM is already produced by the previous step. Replace the `zig build test-render-capture` invocation with a direct PSNR comparison (light script or minimal test reading `out/smoke_test.ppm` with no rebuild). Measured cost ~3-5 min/run. +- **Item 2** — Extract the inline frame-capture/PPM logic from `examples/triangle/src/main.zig` into a `Device.captureFrameToPPM(path)` helper on the public GAL surface, exercised by ≥ 1 `§13` consumer test; the example consumes it. The post-extraction line count is recorded as an **observation** (~449 lines measured), **not** a gate — the original ≤ 200 target was Null-backend-calibrated and does not survive the example's legitimate offscreen-Vulkan setup (cf. B1 / § Notes recalibration). +- **Item 3** — `vk_gen` `s_type` defaults audit: every Vulkan struct emitted by `vk_gen` must carry a comptime `s_type` default. Confirm whether the bindgen pre-fills it (the M0.4 sprint added an explicit `s_type` on `PipelineShaderStageCreateInfo` as defense in depth without confirming this). Documented audit. +- **Item 4** — `src/editor/vk_blit.zig` (legacy S6) must hold **no `device_dispatch` access outside the GAL**: replace its `vk.device_dispatch.*` accesses with the sanctioned `*Raw` GAL wrappers, remove the `WELD_LEGACY_VK_DISPATCH` marker, and harden the `no_device_dispatch_outside_gal` linter rule to forbid the marker entirely. (Full consolidation of vk_blit onto `gal.Device` is out of scope — see § Out-of-scope.) +- **Item 5** — `plugin_loader` and `events` tests failing in ReleaseSafe on Linux CI: investigate root cause (real event-queue bug under ReleaseSafe assumptions vs ill-designed test), document it, make both green in debug AND ReleaseSafe. +- **Item 6** — Win11 + RTX 4080 coverage: transient hardware debt (machine down, defective CPU). Manual Vulkan hardware validation to resume on repair. **Closed by repair, not by PR.** Tracked for memory only — no step. +- **Item 7** — Latent bug, render-graph false WAW cycle (`src/modules/render/render_graph/graph.zig`, `passDependsOn`): WAW is treated symmetrically, so two passes writing the same resource yield edges in both directions → `error.RenderGraphCycle`. Contradicts the file's documented semantics (WAW → writer ordered by insertion). Fix: in the WAW case, emit the edge only from the lower-index writer to the higher-index writer (insertion-order tiebreak) — passing pass indices, or handling WAW in `compile` rather than in `passDependsOn`. +- **Item 8** — Latent bug, unescaped Etch identifiers at codegen (`src/etch/zig_codegen/lower.zig`): user identifiers are emitted verbatim; a **value-identifier** valid in Etch but a Zig keyword (`align`, `noalias`, `comptime`, `error`, `var`…) produces uncompilable Zig. The collision surface is **value-identifiers only** — component/resource/type names are PascalCase `TYPE_IDENT` and can never collide with a (lowercase) Zig keyword; the colliding sites are component **fields**, `let` bindings, and parameters. Fix: emit all user value-identifiers via Zig's `@"…"` syntax, OR reject at type-check (`src/etch/types.zig`) Etch value-idents that are Zig keywords, with a dedicated diagnostic. +- **Item 9** — Robustness debt, unhandled EINTR in POSIX IPC transport (`src/core/ipc/transport_posix.zig`): every I/O entry point sharing the `n < 0` → `error.BrokenPipe` pattern — `send` (write loop), `recv`, **and** the `*WithHandles` variants (`sendmsg`/`recvmsg`) — treats `n < 0` as a broken connection without distinguishing EINTR, so a signal-interrupted syscall is mistaken for a broken pipe. Fix: retry on EINTR in **all four** functions before returning `BrokenPipe`. +- **Item 10** — Debt, `writeValueAsBytes` `@panic` instead of typed error (`src/etch/ecs_bridge.zig`, debt i from M0.2): type-mismatch branches `@panic` instead of returning a typed error → runtime crash on type incoherence. Fix: return `error.TypeMismatch`. Linked dead code (`RuntimeQuery`, `RuntimeReport.last_error`) removed or wired in the same pass. +- **Item 11** — Consistency debt, module-root file named `main.zig` (`src/modules/render/main.zig`, `src/modules/audio/main.zig`, `src/modules/render/gal/main.zig`): these are module roots (no `pub fn main`) but named `main.zig`, contradicting the `root.zig` convention followed by the core (`engine-zig-conventions.md §2`: `root.zig` = module, `main.zig` = executable). Fix: rename the three to `root.zig`; update `build.zig` (`root_source_file` entries for `weld_audio` and `weld_render`) and internal relative imports (e.g. `render/root.zig` importing `gal/root.zig`). External `@import("weld_render")`/`@import("weld_audio")` are unaffected (they resolve `root_source_file`, independent of the file name). + +## Step decomposition (E1–E6) + +Fixed by Guy. Scoped by risk domain, not by task type — each step is an independently reviewable unit with a localized rollback boundary. One branch, one PR opened early (draft), **Claude Code stops at the end of each step for review before continuing** (M0.1 model). Commits are granular within a step. Step order is fixed; do not reorder. + +- **E1 — GAL capture + CI PSNR de-duplication** (items 2, 1). Functionally coupled: the extracted `Device.captureFrameToPPM` helper *produces* the PPM, the CI PSNR step *consumes* it without rebuild. **End-of-step gate:** the helper is present in the public GAL surface and exercised by ≥ 1 test (per-consumer surface coverage, `engine-zig-conventions.md §13`); the example consumes it; the post-extraction line count is recorded as an observation (~449 lines), **not** a gate; Linux CI job wall-time down ~3-5 min vs M0.4. Stop for GO. +- **E2 — GAL dispatch discipline** (items 3, 4). `s_type` audit across all `vk_gen`-emitted structs + `vk_blit` `device_dispatch` removal (`*Raw` wrappers) + `WELD_LEGACY_VK_DISPATCH` marker removal + linter rule hardened to forbid the marker. **End-of-step gate:** every Vulkan struct has a comptime `s_type` default (documented audit); `vk_blit.zig` has no `device_dispatch` access outside the GAL (`*Raw` variants), marker gone; linter rejects the marker. Stop for GO. +- **E3 — Render-graph WAW fix** (item 7) + repro test. Isolated: algorithmic bug in `passDependsOn`, distinct risk domain from GPU dispatch. **End-of-step gate:** repro test "two passes writing the same resource, no RAW relation" → `compile()` succeeds and orders by insertion (not `RenderGraphCycle`). Stop for GO. +- **E4 — Etch backend** (items 8, 10). Item 8: keyword-ident escaping at codegen + repro test (a value-ident colliding with a Zig keyword — a component **field** named `var`, a `let comptime = …` binding, or a keyword param → compilable Zig, OR clean type-check rejection with a dedicated diagnostic). Item 10: `writeValueAsBytes` → `error.TypeMismatch`, dead code removed or wired. **End-of-step gate:** both repro/behavior criteria met. Stop for GO. +- **E5 — Runtime robustness** (items 5, 9). Item 5: investigate the ReleaseSafe failures, document root cause, make `plugin_loader`/`events` green in debug AND ReleaseSafe. Item 9: EINTR retried in `send`, `recv`, and the `*WithHandles` variants (`sendmsg`/`recvmsg`). Placed late: item 5 has an uncertain outcome and blocks nothing else. **End-of-step gate:** both test suites green in both modes (root cause documented); EINTR handled (test or documented review). Stop for GO. +- **E6 — `root.zig` convention** (item 11) — **last**. Cross-cutting rename over roots already touched by E1/E2 (GAL), done after content stabilizes to avoid intra-branch conflicts. **End-of-step gate:** three files renamed; `build.zig` and internal imports updated; `bindgen-verify` green; `zig build test` green; zero `main.zig` without `pub fn main` under `src/modules/`. Stop for GO. + +## Out-of-scope + +- Any new functional surface. +- V-Buffer and other Phase 1 renderer deliverables. +- GAL redesign beyond the capture-helper extraction (item 2). +- GAL ergonomics improvements beyond the `captureFrameToPPM` extraction — a higher-level capture/readback API that would let the example shrink further is **Phase 1 GAL-ergonomics debt** (deferred; cf. B1). +- Consolidation of `src/editor/vk_blit.zig`'s standalone Vulkan renderer onto `gal.Device` — deferred to a dedicated milestone whose closure criterion is a **lavapipe blit test** (vk_blit has no test/CI coverage today and the editor is Linux-only; cf. § Acted deviations AD1). +- Cleanup of the existing ~640 lines of French comments (dedicated milestone; the source is already cut off upstream by the process fix — `engine-development-workflow.md` language doctrine). +- Item 6 hardware coverage as a code deliverable (tracked for memory only; closed by repair). + +## Specs to read first + +1. `engine-phase-0-plan.md` — § M0.5 (canonical source of the 11 items, their fixes, and acceptance criteria). Primary reference; all item contracts are stated there. +2. `engine-render.md` — Render Graph section (topological-sort semantics, WAW → insertion order) for item 7. +3. `engine-zig-conventions.md` — §2 (`root.zig` vs `main.zig` convention) for item 11; §13 (per-consumer surface coverage) for the item 2 helper test and the general acceptance bar. +4. `engine-ipc.md` — POSIX transport / socket framing context for item 9. +5. `engine-development-workflow.md` — §3 (brief format), §4.3 (Conventional Commits), §4.6 (squash-and-merge), and the language doctrine (artefacts EN strict) for the closing criteria. + +## Files to create or modify + +- `.github/workflows/` (CI runtime-smoke-test) — edit — replace the rebuild-based PSNR step with a no-rebuild comparison (item 1). +- `scripts/` or `tests/render/` PSNR comparator — create — light script or minimal test reading `out/smoke_test.ppm` without rebuild (item 1). +- `src/modules/render/gal/` GAL surface — edit — add `Device.captureFrameToPPM(path)` to the public surface (item 2). +- `examples/triangle/src/main.zig` — edit — consume the new helper; line count recorded as an observation, not a gate (item 2). +- `tests/render/` — create — test exercising `Device.captureFrameToPPM` (item 2, per §13). +- `tools/` `vk_gen` emitter — edit — ensure comptime `s_type` default on every emitted Vulkan struct (item 3). +- `src/editor/vk_blit.zig` — edit — replace `device_dispatch` accesses with `*Raw` GAL wrappers, remove `WELD_LEGACY_VK_DISPATCH` marker (item 4). +- `tools/weld_lint/` `no_device_dispatch_outside_gal` rule — edit — forbid the marker (item 4). +- `tests/` `plugin_loader` and `events` suites — edit — fix root cause, green in debug + ReleaseSafe (item 5). +- `src/modules/render/render_graph/graph.zig` — edit — WAW insertion-order tiebreak in `passDependsOn` or `compile` (item 7). +- `tests/render/` — create — render-graph WAW repro test (item 7). +- `src/etch/zig_codegen/lower.zig` and/or `src/etch/types.zig` — edit — escape user idents via `@"…"` or reject Zig-keyword idents with a dedicated diagnostic (item 8). +- `tests/etch/` — create — keyword-ident repro test (item 8). +- `src/etch/ecs_bridge.zig` — edit — `@panic` → `error.TypeMismatch`; remove or wire `RuntimeQuery` / `RuntimeReport.last_error` (item 10). +- `src/modules/render/main.zig` → `root.zig`, `src/modules/audio/main.zig` → `root.zig`, `src/modules/render/gal/main.zig` → `root.zig` — rename — module roots (item 11). +- `build.zig` — edit — `root_source_file` entries for `weld_audio` and `weld_render`; internal relative imports (item 11). + +*Files outside this list must not be touched by Claude Code without validation.* + +## Acceptance criteria + +### Tests + +- `tests/render/` — `test "captureFrameToPPM exercised"` — the GAL capture helper produces a PPM and is exercised by ≥ 1 test (item 2, §13). +- `tests/render/` — `test "render graph WAW two writers no RAW"` — two passes writing the same resource with no RAW relation → `compile()` succeeds and orders by insertion, not `RenderGraphCycle` (item 7). +- `tests/etch/` — `test "etch ident equals zig keyword"` — an Etch program with a **value-ident** colliding with a Zig keyword (a component field named `var`, or `let comptime = …`) → emitted Zig compiles, OR type-check rejects with the dedicated diagnostic (item 8). +- `tests/` `plugin_loader` and `events` suites — green in debug AND ReleaseSafe on Linux CI; root cause documented in the brief living section (item 5). +- IPC POSIX EINTR — retried in `send`, `recv`, and the `*WithHandles` variants (`sendmsg`/`recvmsg`); covered by a test or a documented review (item 9). +- `writeValueAsBytes` returns `error.TypeMismatch` (no `@panic`); linked dead code removed or wired (item 10). + +### Observable behavior + +- CI `runtime-smoke-test` PSNR step no longer rebuilds: Linux job wall-time down ~3-5 min vs M0.4 (item 1). +- `examples/triangle/src/main.zig` consumes the `captureFrameToPPM` helper; post-extraction line count (~449) recorded as an observation, **not** a gate (item 2). +- Documented `s_type` audit: every `vk_gen`-emitted Vulkan struct has a comptime `s_type` default (item 3). +- `src/editor/vk_blit.zig` has no `device_dispatch` access outside the GAL (`*Raw` variants); `WELD_LEGACY_VK_DISPATCH` marker removed; linter rule hardened to forbid it (item 4). +- After E6: zero `main.zig` without `pub fn main` under `src/modules/`; `bindgen-verify` green (item 11). + +### CI + +- `zig build` clean, zero warning, on the configured matrix. +- `zig build test` green (debug + ReleaseSafe). +- `zig fmt --check` green. +- `zig build lint` green. +- `commit-msg` hook green on every commit of the branch. +- CI green on Linux + Windows. +- Manual GPU validation: best-effort only for M0.5 (not mandatory — `engine-development-workflow.md §4.5.1`). Win11 + RTX 4080 marked `unavailable` (item 6). + +## Conventions + +- **Branch:** `phase-0/housekeeping/post-m0.4` +- **Final tag:** `v0.5.0-M0.5-housekeeping` +- **PR title:** `Phase 0 / Housekeeping / Post-M0.4 + Renderer Debt` +- **Commit convention:** Conventional Commits (`engine-development-workflow.md §4.3`) +- **Merge strategy:** squash-and-merge (`engine-development-workflow.md §4.6`) +- **PR opened early** as draft after E1 is green; one PR for the whole branch. +- **Squash commit (set by Guy at merge):** title `chore(housekeeping): post-m0.4 debt and renderer fixes (M0.5)` style, mandatory structured body per `engine-development-workflow.md §4.x` (intro / per-step labels / CI / Notable items for review / Measures / `Closes M0.5. Brief: briefs/M0.5-housekeeping.md.`). + +## Notes + +- Risk-domain scoping rationale: E3 (render-graph logic) is deliberately separated from E1/E2 (GAL/Vulkan dispatch) — distinct failure surfaces, dedicated review point for a latent bug plus its repro test. +- Items 7 and 8 are latent bugs not caught by the existing differential/golden corpora (no adversarial pass writers, no adversarial idents). Each MUST ship with a repro test that fails before the fix. +- Item 5 outcome is uncertain (real bug vs ill-designed test). If the investigation reveals a real event-queue bug requiring surface changes, that is a design blocker → stop and return to Claude.ai (do not silently expand scope). +- Item 9 EINTR may be hard to exercise deterministically without signal injection; a documented code review is an acceptable substitute for a unit test per the plan ("test ou revue"). +- E6 last: the rename touches roots edited by E1/E2; doing it after content stabilizes avoids intra-branch rebase friction. +- **Decision 12 recalibration:** the `examples/triangle/src/main.zig` ≤ 200-line target (M0.4 brief, decision 12) was calibrated against a Null-backend example and does not survive the example's legitimate offscreen-Vulkan render setup. M0.5 carries the recalibration (line count → observation, not a gate; cf. B1); the already-merged M0.4 brief is **not** re-patched. + +--- + +# LIVING SECTION + +*Maintained by Claude Code during the milestone. The log serves review and post-mortem debugging, not marketing.* + +## Specs read + +- [x] `engine-phase-0-plan.md` (§ M0.5) — read 2026-06-01 10:02 +- [x] `engine-render.md` (Render Graph) — read 2026-06-01 10:02 +- [x] `engine-zig-conventions.md` (§2, §13) — read 2026-06-01 10:02 +- [x] `engine-ipc.md` (POSIX transport) — read 2026-06-01 10:02 +- [x] `engine-development-workflow.md` (§3, §4.3, §4.6, language doctrine) — read 2026-06-01 10:02 + +## Execution log + +- **2026-06-01 — Setup.** Branch `phase-0/housekeeping/post-m0.4` cut from `main` (HEAD `8434b6e`). Brief committed verbatim as the first commit. All 5 specs read in full and ticked off above. Execution proceeds E1→E6 with a mandatory stop at each end-of-step gate (GO required from Guy before the next step). Draft PR to be opened once E1 is green. +- **2026-06-01 — E1 started; stopped on blocker B1.** Read `examples/triangle/src/main.zig` (measured **501 lines**, not the ~250 stated by item 2) and the GAL surface (`src/modules/render/gal/main.zig`). The item 2 `≤ 200` line gate is unreachable via the prescribed capture-helper extraction alone (~390 lines remain after extraction). Logged as B1; stopping E1 implementation pending a Claude.ai round-trip. Item 1 (CI PSNR de-duplication) and the `captureFrameToPPM` extraction + its test remain feasible and unblocked under any B1 resolution. +- **2026-06-01 — B1 routed to Cas 2; interim GO from Guy.** B1 (≤ 200 infeasible in-scope, false premise) → Cas 2, returned to Claude.ai for the FROZEN amendment (gate removal + measured post-extraction baseline + Phase 1 GAL-ergonomics debt). On Guy's interim GO, advancing the unblocked subset: item 1 (CI PSNR de-duplication) + `captureFrameToPPM` extraction into the public GAL surface + its §13 consumer test + wiring the example to consume it. The ≤ 200 target is NOT chased (no higher-level GAL surface, no sibling-file split to game the caller-side metric); the post-extraction line count is recorded as an observation, not a gate. The FROZEN amendment itself is applied by Guy via the updated brief and traced under Acted deviations at that commit. +- **2026-06-01 — E1 implemented (unblocked subset); stopping for GO.** + - **Item 2** — extracted the readback + PPM-encode boilerplate into a new public GAL helper `src/modules/render/gal/capture.zig` (generic `captureFrameToPPM(device, …)` over the backend `Device`, plus the pure, unit-testable `encodePpm`/`writePpm`). Added a thin delegating `captureFrameToPPM` method to the Vulkan and Null `Device`s, registered it in the interface contract (`interface.required_methods` + `TestShape`), and exported `gal.capture` from the GAL root. Wired `examples/triangle/src/main.zig` to consume it; the offscreen render stays in the example (the canonical GAL-rendering consumer, §13 pattern 3) — only readback+encode moved. + - **Item 2 §13 coverage** — `tests/render/capture_helper.zig`: `encodePpm` RGBA→P6 (pure, every platform) + `captureFrameToPPM` on the Null backend asserting clean `error.Unsupported` readback propagation (every platform). The full Vulkan path is exercised end-to-end on lavapipe CI by the example (`--capture-frame`) + the PSNR gate. + - **Item 1** — new std-only `tests/render/ppm_psnr_compare.zig` + `zig build test-ppm-psnr` step: reads the already-produced `out/smoke_test.ppm` and compares PSNR vs golden with **no render-stack rebuild and no triangle re-spawn**. CI `runtime-smoke-test` "Verify PSNR" switched from `test-render-capture` to `test-ppm-psnr` (saves ~3-5 min/run). `tests/render/capture.zig` (spawn-based) is **retained untouched per source-preservation §2.5** — removal/dedup deferred until the new gate is proven green on Linux CI. + - **Measure (observation, not a gate)** — `examples/triangle/src/main.zig`: **501 → 449 lines**. The ≤ 200 target is removed pending the FROZEN amendment (B1) and is NOT chased: no higher-level GAL surface, no sibling-file split. The clean low-level readback extraction reclaims 52 lines; the offscreen render setup correctly stays in the example. + - **Local gates (macOS dev, ReleaseSafe-aware)** — `zig build test` (debug + ReleaseSafe) green; `zig fmt --check` green; `zig build lint` green; `zig build run-example-triangle -- --smoke-test` exit 0; `zig build test-ppm-psnr` 2 pass / 1 skip (gate skips without a local capture). The pre-existing `plugin_loader`/`events` stderr noise is unchanged and out of scope here (item 5 / E5). + - Stopping at the E1 end-of-step gate for Guy's GO before E2; draft PR to be opened. +- **2026-06-01 — E1 CI green (recorded with the E2 commits, per Guy — avoids re-triggering ~20 min CI on a doc-only push).** PR #19: all 7 checks pass, incl. `runtime-smoke-test` (lavapipe) in 3m33s — validating the Vulkan `captureFrameToPPM` readback (PSNR ≥ 40 dB vs golden) and the no-rebuild `test-ppm-psnr` gate end-to-end. +- **2026-06-01 — E2 implemented (GAL dispatch discipline); stopping for GO.** + - **Item 3 (s_type audit — documented, zero code change).** The bindgen `tools/bindgen/adapters/vk_xml/` emits a comptime `s_type` default for every Vulkan struct carrying a fixed `sType` discriminant (``). `vk.zig` coverage: **229/231** `s_type` fields have a default; the 2 without (`BaseInStructure`, `BaseOutStructure`) are the polymorphic pNext-chain base headers — correctly defaultless (no fixed type). The M0.4 "explicit s_type" is in fact array-literal init in `pipeline.zig` so the default applies (the prior `undefined` init left `sType` indeterminate → Linux validation SIGSEGV); there is **no redundant explicit set to remove**. Conclusion: bindgen covers universally, no gap, no fix; the `pipeline.zig` comment is accurate and retained. + - **Item 4 (vk_blit dispatch discipline — Option A, per Guy's E2 decision).** `src/editor/vk_blit.zig`: its only 2 `vk.device_dispatch.*` accesses (`vkAcquireNextImageKHR`, `vkQueuePresentKHR`) now use the idiomatic `*Raw` wrappers (`acquireNextImageKHRRaw`, `presentKHRRaw`) — the sanctioned D-S2-dispatch-bypass path, returning raw `Result` so the manual `suboptimal_khr`/`error_out_of_date_khr` resize handling is preserved. The `WELD_LEGACY_VK_DISPATCH` marker is removed. The `no_device_dispatch_outside_gal` rule is hardened: the marker is now forbidden outright (`legacyMarkerOffset`, header-only scan so the rule file does not flag itself; a diagnostic replaces the grandfather skip). New fixtures `tests/lint/bad/device_dispatch/{has_marker,raw_dispatch}.zig` + a runner test assert both the forbidden marker and a raw-dispatch access are flagged. + - **Deferred debt (logged, not touched).** The editor still owns a **1133-line standalone Vulkan renderer** (`vk_blit.zig`) duplicating the GAL. Full consolidation onto `gal.vulkan_backend.Device` (Option B) was assessed and **refused for M0.5**: vk_blit has zero test/CI coverage and the editor is Linux-only (`error.Unimplemented` on the macOS dev box), so a 1133-line rewrite would be unvalidatable — contradicting §2.5 (parity proof) + §13 (consumer-empirical coverage) + the M0.4 lesson this milestone applies. Consolidation is a dedicated future milestone whose exit gate must be a lavapipe blit test. + - **Gates (macOS dev)** — `zig build` (incl. editor/vk_blit compile), `zig build test` (debug + ReleaseSafe), `zig fmt --check`, `zig build lint` (production tree clean): all green. Stopping at the E2 end-of-step gate for Guy's GO before E3. +- **2026-06-01 — E3 implemented (render-graph WAW fix, item 7); stopping for GO.** TDD per the brief: the repro test was written and committed **RED first** (`93c315b` — `compile()` returned `error.RenderGraphCycle`), then fixed. + - **Repro** (`tests/render/render_graph_topo.zig`): two passes writing the same texture with no RAW relation → `compile()` must succeed and order by insertion. Verified red before the fix (1 fail), green after. + - **Fix** (`src/modules/render/render_graph/graph.zig`): `passDependsOn` is now a pure RAW data-hazard query — the symmetric WAW block (which emitted edges in both directions → false cycle) is removed. A new `writesSameResource` helper detects the WAW hazard, and `compile` applies the **insertion-order tiebreak**: for two writers i, j it emits only the `i < j` edge (lower-index writer first), so WAW is acyclic by construction. Doc updated to match. Approach = Guy's light preference (tiebreak in `compile`; `passDependsOn` stays a pure data-hazard query) — the insertion index is naturally available in `compile`, and ordering is not a dependency-detection concern. + - **No regression**: RAW topo order, real-cycle detection (mutual RAW), and barrier tracking still pass — 365/382 (+1 vs the red run's 364, 0 fail). `runtime-smoke-test` untouched (E3 is CPU-only graph logic). + - **Gates (macOS dev)**: `zig build test` (debug + ReleaseSafe), `zig fmt --check`, `zig build lint` green. Stopping at the E3 end-of-step gate for Guy's GO before E4. +- **2026-06-02 — E4 implemented (Etch backend, items 8 + 10); stopping for GO.** + - **Item 8 (keyword idents — voie (a) escaping, test-first).** Voie (a) chosen per Guy's criteria: voie (b) (type-check rejection) requires an Etch-semantic reason to forbid the ident, and there is none — the lexer reserves only Etch keywords (`const`/`fn`/`struct`); `align`/`var`/`comptime`/`error` are valid Etch idents. **Anti-hallucination correction:** the brief's "component `align`" example is invalid Etch — component/resource/type names must be capitalized `TYPE_IDENT`, so they can never collide with (lowercase) Zig keywords. The real collision surface is lowercase value-idents: field names, `let` bindings, params. **Repro** (`tests/etch/keyword_ident_test.zig`, committed RED first `73f756c`): a component with a keyword **field** (`var`) used in a when-filter and a field access is codegen'd, then the *generated Zig is parsed with `std.zig.Ast.parse`* — bare keywords in identifier position are parse errors, so this enforces escaping across every site the program emits. RED (4 parse errors) → green. **Fix**: new `Writer.ident` (`emit.zig`) emits user idents as `@"name"` when they collide with a Zig keyword or primitive (via `std.zig.Token.keywords` + `std.zig.primitives` — the authoritative sets; the bindgen's manual keyword list is incomplete, missing `align`). **Exhaustivity audit — 7 sites in `lower.zig`**: field decl (×2), when-filter reads (×2), `let` decl, ident/var-ref (×2), field access. Left unescaped by design (safe): all type/component names (TYPE_IDENT), RTTI field-name *string* literals, `rule_`/`_arr`/`_id`-suffixed names, literal values. + - **Item 10 (writeValueAsBytes + dead code).** `writeValueAsBytes` (`ecs_bridge.zig`) now returns `BridgeError!void`: the 5 `@panic` type-mismatch branches **and** the latent `bool_` mis-tag (an unchecked `v.bool_` that panicked in safe builds) return `error.TypeMismatch`. Three callers updated with `try` — `writeComponentField`, `writeResourceField`, and `interp.compileTypeDecl`'s default-bytes loop (the third was caught by the compiler, not the original 2-caller plan). New test asserts `error.TypeMismatch` on incompatible tags. **Linked dead code removed** (purely dead — never assigned/read, confirmed by grep incl. tests): the unused `RuntimeQuery` alias in `ecs_bridge.zig`, and `RuntimeReport.last_error` + its now-orphaned `RuntimeError`/`RuntimeErrorKind` aliases in `interp.zig`. The full wiring (assign `last_error` in `execBody`; consume-or-remove `world.query_dynamic`/`RuntimeQuery`) is the M0.8 D-S4-runtime-report / D-S4-runtime-query debt — out of M0.5 scope; removing the dead remnants here satisfies item 10's "not left dangling" without pulling that interpreter work forward. + - **Gates (macOS dev)**: `zig build test` (debug + ReleaseSafe), `zig fmt --check`, `zig build lint` green. Stopping at the E4 end-of-step gate for Guy's GO before E5. +- **2026-06-02 — E5 implemented (runtime robustness, items 5 + 9); stopping for GO.** + - **Item 5 (plugin_loader/events "ReleaseSafe failures" — root cause: a misread benign build-runner line; not Cas 2).** Investigated both FROZEN hypotheses; **both refuted**. The two suites **pass** in debug AND ReleaseSafe — `zig build test` and `zig build test -Doptimize=ReleaseSafe` both exit 0 (`177/177 steps; 366/383 tests passed, 17 skipped, 0 failed`), and the E4 CI Linux ReleaseSafe job is green. *Root cause* (not the symptom): the events `drop saturation` test and the plugin_loader error-path tests **deliberately** drive `std.log.warn` paths (queue drop-threshold; bad-plugin rejection). Those warns reach stderr; the Zig 0.16 build runner, seeing a test child write to stderr, prints a `failed command: …--listen=-` line to *surface* that output — but the step succeeds and the build exits 0. That benign line (its "failed" wording) is what was misread as a test failure. "Real event-queue bug under ReleaseSafe" is refuted — saturation/drop/threshold/drain are exercised and green in ReleaseSafe, the very mode suspected. "Test mal conçu" holds only insofar as the tests did not flag their warn output as intentional — fixed by anti-misread comments at both sites (`saturation_test.zig`, `load_unload_test.zig`). A test-scoped `std_options.logFn` to suppress the warn was tried and **does not work**: in Zig 0.16 the test *runner* is the compilation root, so `std_options` declared in a test file is ignored (and `zig build lint` scans tests/, so the attempt also tripped a rule). Cleanly removing the benign surfacing would need a custom test runner (build-system scope) — out of M0.5 scope, logged as an option. **Not Cas 2**: no public API / ordering contract / queue semantics changed. + - **Item 9 (EINTR retry — documented review, no fragile test).** `transport_posix.zig` `send` (write loop) and `recv` now retry on `std.c.errno(n) == .INTR` (mirroring std.posix's own `.INTR => continue`) before returning `error.BrokenPipe`: a signal-interrupted syscall (n<0, errno EINTR, no bytes moved) is retried, not mis-reported as a broken pipe. No unit test by design — a deterministic EINTR needs a signal injected mid-syscall, which is inherently racy (the GO forbade a fragile timing test); the fix is a mechanical retry reviewed by inspection, and `test-ipc` passes in both modes (no regression). **Scope correction (Guy's E5 review):** the item-9 intent is that the POSIX transport never mis-reports EINTR, so the `*WithHandles` variants are in scope too. Source-checked: `sendWithHandles` (`sendmsg`) and `recvWithHandles` (`recvmsg`) carried the **identical** `if (n < 0) return error.BrokenPipe` motif with no EINTR distinction — same interruptible syscalls, same socket, same signal exposure. The same `.INTR => continue` retry is now applied to both (sendmsg retries the whole message — EINTR means nothing left the socket, so passed fds are not duplicated; recvmsg retries with nothing received yet). All four POSIX I/O entry points (`send`/`recv`/`sendmsg`/`recvmsg`) are now EINTR-safe. + - **Gates (macOS dev)**: `zig build`, `zig build lint`, `zig fmt --check`, `zig build test` (debug + ReleaseSafe), `zig build test-ipc` (debug + ReleaseSafe) — all green; ReleaseSafe `177/177 steps, 0 failed`. Stopping at the E5 end-of-step gate for Guy's GO before E6. +- **2026-06-02 — E6 implemented (root.zig convention, item 11); stopping for GO.** Cross-cutting rename done last (touches roots edited by E1/E2), per the brief. + - **Rename**: `git mv` (history preserved) of the three **module roots** — all confirmed `pub fn main`-free (libraries, not exes): `src/modules/render/main.zig`, `src/modules/audio/main.zig`, `src/modules/render/gal/main.zig` → `root.zig`. + - **Wiring**: `build.zig` two `root_source_file` paths (audio l.57, render l.75) → `root.zig`; six internal relative imports of the GAL submodule (`gal/main.zig` → `gal/root.zig`): `render/root.zig`, `render_graph/{graph,pass}.zig`, `render_graph/passes/{capture,depth_prepass,forward}.zig`; plus two stale comments (`tests/render/gal_null_smoke.zig`, `gal/vulkan/device.zig`). External `@import("weld_render")` / `@import("weld_audio")` resolve `root_source_file` (filename-independent) → unchanged, as expected. + - **Positive verification (Guy's gate addition)**: `grep --include="*.zig"` for the three old paths (`gal/main.zig`, `modules/render/main.zig`, `modules/audio/main.zig`) across the tree → **0 residual references in code** (not merely relying on the compiler's broken-import red). And **0 `main.zig` remain under `src/modules/`** (the three were the only ones) → "zero `main.zig` without `pub fn main`" holds trivially. + - **Gate (macOS dev)**: `bindgen-verify` green (regen + `git diff --quiet` on bindings/generated + src/core/platform — the rename touches neither), `zig build test` (debug + ReleaseSafe, `177/177 steps, 0 failed`), `zig build`, `zig fmt --check`, `zig build lint` — all green. Stopping at the E6 end-of-step gate; PR stays draft (ready-for-review / merge / tag are Guy's call). +- **2026-06-02 — Post-E6 PR-review micro-fix (item 9, in-branch before squash).** `recvWithHandles` now rearms `msg.msg_controllen` (to the ancillary-buffer capacity `ctrl_size`) and `msg.msg_flags = 0` at the top of each EINTR retry: `recvmsg` treats both as in/out, and POSIX leaves the msghdr unspecified on error, so the retry no longer leans on the (de-facto-safe but unspecified) Linux/macOS behavior of not mutating them on EINTR. `sendWithHandles` (`sendmsg`, input-only) is correct as-is and untouched. No test (deterministic EINTR is racy — documented review, consistent with E5). Gates green: `zig build`, `zig build test` + `test-ipc` (debug + ReleaseSafe), `zig fmt --check`, `zig build lint`. Completes item 9. + +## Acted deviations + +### AD1 — item 4 delivered as Option A (targeted); full GAL migration deferred (E2) + +The FROZEN item-4 gate reads "vk_blit.zig passe par la GAL". On E2 recon the file proved to be a **1133-line standalone Vulkan renderer** with only 2 `device_dispatch` accesses and **zero test/CI coverage** (the editor is Linux-only, `error.Unimplemented` on the macOS dev box). A full rewrite onto `gal.vulkan_backend.Device` (Option B) would be unvalidatable here — contradicting §2.5 / §13 and the M0.4 lesson M0.5 applies. Per Guy's E2 decision, item 4 is delivered as **Option A**: drop the `device_dispatch` accesses (idiomatic `*Raw` wrappers), remove the marker, and harden the linter to forbid the marker — satisfying the rule the marker escaped (`no_device_dispatch_outside_gal`) and all of item 4's enforceable outcomes. The literal "passe par la GAL" is reinterpreted as this enforceable criterion; the full `gal.Device` consolidation is logged as deferred editor debt (Execution log). The FROZEN reformulation (enforceable criterion + deferred debt) will be applied by Guy in the amended brief; this entry records the deviation at the commit that introduces it. + +### AD2 — FROZEN amendment v2 applied by Claude Code (Claude.ai round-trip product) + +On Guy's explicit instruction, Claude Code applied the M0.5 FROZEN-SECTION amendment v2 — sourced from `engine-phase-0-plan.md § M0.5 amendé` (the Claude.ai round-trip product Guy brought back). This is the sanctioned channel for FROZEN edits (cf. the FROZEN-section notice); the LIVING SECTION / journal is otherwise unchanged. Status `PLANNED → IN PROGRESS`. Five corrections: + +1. **B1 resolution** — the `examples/triangle/src/main.zig` ≤ 200-line gate is removed from item 2, gate E1, the Files note, and Observable behavior; replaced by "helper on the public GAL surface + exercised by ≥ 1 §13 test + the example consumes it", with the line count recorded as an observation (~449 lines) and **no count gate**. Phase 1 GAL-ergonomics debt added to Out-of-scope. (Lands the resolution Blocker B1 was waiting on.) +2. **Item 4 / AD1 reformulation** — "passe par la GAL" → "no `device_dispatch` outside the GAL (`*Raw` variants); `WELD_LEGACY_VK_DISPATCH` marker removed; linter hardened to forbid it" (item 4, gate E2, Files, Observable behavior). vk_blit → `gal.Device` consolidation added to Out-of-scope (closure criterion = lavapipe blit test, dedicated milestone). Realizes the FROZEN reformulation AD1 deferred to Guy. +3. **Item 8** — the invalid "component named `align`" repro is corrected: component/resource/type names are PascalCase `TYPE_IDENT` and can never collide with a (lowercase) Zig keyword; the collision surface is value-idents only (fields, `let` bindings, params). Updated in item 8, gate E4, and the Tests criterion. +4. **Item 9** — EINTR scope extended to all four POSIX I/O entry points sharing the `n < 0` → `BrokenPipe` motif: `send`, `recv`, **and** `sendmsg`/`recvmsg` (`*WithHandles`). Updated in item 9, gate E5, and the Tests criterion (reflects what E5 already delivered). +5. **Notes** — added the decision-12 recalibration note (the ≤ 200 target was Null-backend-calibrated; M0.5 carries the recalibration; the merged M0.4 brief is not re-patched). + +## Blockers encountered + +### B1 — item 2 `≤ 200 lines` gate infeasible via the prescribed capture-helper extraction (wrong FROZEN premise) + +**Status:** RESOLVED (2026-06-02) — the Claude.ai round-trip landed as **FROZEN amendment v2**: the ≤ 200-line gate is removed (line count → observation, ~449 lines) and the Phase 1 GAL-ergonomics debt is logged in Out-of-scope. Applied and traced in § Acted deviations **AD2**. The Finding / Why / History / Scope-conflict notes below are retained as the historical record that motivated the round-trip. + +**Finding.** Item 2 states `examples/triangle/src/main.zig` is "~250 lines" (sourced from M0.4 brief §Notes decision 12). The file is in fact **501 lines**, and has been since the M0.4 squash (`fcb7f3d`); the language-normalization chore (`8434b6e`) left it unchanged. The "~250" premise is a mis-measurement, not a recent regression. + +**Why ≤ 200 is unreachable by the prescribed mechanism.** Measured breakdown (501 lines): header+imports ~22, `Args.parse` ~27, `main`+`supportsVulkanWindow` ~25, constants+vertex types ~20, `TrianglePipeline` ~91, `frameClearColor` ~10, `captureFrame` ~73, `writePPM` ~37, `runVulkan` ~114, `runNullBackend` ~70. The only "capture helper" extractable into the GAL is `captureFrame` + `writePPM` ≈ 110 lines. Replacing them with a single `Device.captureFrameToPPM(...)` call leaves main.zig at **≈ 390 lines** — still ~2× over the gate. The remaining bulk (Vulkan interactive loop, dual `TrianglePipeline`, Null path) is not a capture helper. + +**History.** At commit `ef8f21f` (M0.4) the example was 155 lines (Null backend only), under the ≤ 200 architectural-test threshold. The M0.4 post-review continuation wired the full Vulkan path + capture path into the same file → 501. M0.5 item 2 is the debt to claw it back; the prescribed single helper is insufficient. + +**Scope conflict.** M0.4's own doctrine (§Notes decision 12) treats a >200-line example as a signal that the public GAL surface is too low-level and must be revised via a Claude.ai round-trip. Reaching ≤ 200 from ~390 needs either (a) additional higher-level GAL helpers (a frame-loop / swapchain-render convenience) — explicitly OUT of M0.5 scope ("GAL redesign beyond the capture-helper extraction"), or (b) splitting main.zig into sibling files — which games the "≤ 200 on the caller side" architectural-test metric without raising the GAL's level (a silent workaround the workflow forbids). No honest in-scope path reaches ≤ 200. + +**Options proposed for the Claude.ai round-trip:** +1. Amend the item 2 gate to a realistic post-extraction figure (e.g. main.zig ≤ ~400), keeping `captureFrameToPPM` + its §13 surface-coverage test as the E1 deliverable. +2. Deliberately widen scope to add the higher-level GAL helper(s) that bring the example genuinely below 200 (the M0.4-doctrine GAL-surface revision) — larger, may warrant its own step or milestone. +3. Decouple item 2: keep `captureFrameToPPM` + test in M0.5, move the ≤ 200 line target to a dedicated example-ergonomics milestone. + +**Unblocked regardless of resolution:** the `captureFrameToPPM` extraction + its test, and item 1 (CI PSNR de-duplication), are feasible and in scope under all three options. + +## Closing notes + +- **What worked:** +- **What deviated from the original spec:** +- **What to flag explicitly in review:** +- **Final measures:** +- **Residual risk / debt left intentionally:** diff --git a/build.zig b/build.zig index 1e5358d..bad2b2e 100644 --- a/build.zig +++ b/build.zig @@ -54,7 +54,7 @@ pub fn build(b: *std.Build) void { // audio tests and, later, by the runtime once the audio strategy // selection wires in. const audio_module = b.createModule(.{ - .root_source_file = b.path("src/modules/audio/main.zig"), + .root_source_file = b.path("src/modules/audio/root.zig"), .target = target, .optimize = optimize, }); @@ -72,7 +72,7 @@ pub fn build(b: *std.Build) void { // `b.dependency("weld", ...).module("weld_render")` — a prerequisite of // the `examples/triangle/` sub-project (brief §Scope). const render_module = b.addModule("weld_render", .{ - .root_source_file = b.path("src/modules/render/main.zig"), + .root_source_file = b.path("src/modules/render/root.zig"), .target = target, .optimize = optimize, }); @@ -327,6 +327,9 @@ pub fn build(b: *std.Build) void { .{ .path = "tests/bindings/vk_abi_test.zig" }, .{ .path = "tests/bindings/wayland_abi_test.zig", .wl_protocols = true }, .{ .path = "tests/etch/corpus_test.zig", .etch = true }, + // M0.5 item 8 — Etch idents that collide with Zig keywords must + // codegen to parseable (escaped) Zig. RED before the lower.zig fix. + .{ .path = "tests/etch/keyword_ident_test.zig", .etch = true }, .{ .path = "tests/etch_interp/corpus_test.zig", .etch_interp = true }, // M0.3 — common platform layer tests. .{ .path = "tests/platform/fs_vfs_test.zig" }, @@ -370,6 +373,14 @@ pub fn build(b: *std.Build) void { // unrelated tests with current ReleaseSafe issues — tracked as // M0.7 housekeeping debt). .{ .path = "tests/render/capture.zig", .render = true, .dedicated_step = "test-render-capture" }, + // M0.5 item 2 — GAL capture helper surface coverage (encodePpm + + // Device.captureFrameToPPM); §13 consumer test, runs on every platform. + .{ .path = "tests/render/capture_helper.zig", .render = true }, + // M0.5 item 1 — direct PSNR gate reading the pre-produced + // out/smoke_test.ppm with no rebuild and no triangle re-spawn. + // std-only (no .render), so `zig build test-ppm-psnr` compiles in + // seconds and replaces the rebuild-heavy `test-render-capture` in CI. + .{ .path = "tests/render/ppm_psnr_compare.zig", .dedicated_step = "test-ppm-psnr" }, // M0.4 § Scope Post-Review — hot-reload filewatch latency < 200 ms. // Skip if glslc absent from PATH. .{ .path = "tests/render/shader_hot_reload.zig", .render = true }, diff --git a/examples/triangle/src/main.zig b/examples/triangle/src/main.zig index ca98401..2d68dee 100644 --- a/examples/triangle/src/main.zig +++ b/examples/triangle/src/main.zig @@ -202,10 +202,10 @@ fn frameClearColor(frame: u32) gal.types.ColorClear { }; } -/// Render frame `frame_idx` into an offscreen R8G8B8A8_UNORM texture, -/// copy it through a staging buffer, and write the result as a binary -/// PPM (P6) to `path`. Used by the smoke-test capture path consumed by -/// `tests/render/capture.zig`. The `pipeline` argument is the same +/// Render frame `frame_idx` into an offscreen R8G8B8A8_UNORM texture, then +/// delegate the GPU readback + PPM write to the public GAL helper +/// `Device.captureFrameToPPM` (M0.5 item 2; cf. `gal/capture.zig`). The +/// render leaves the texture in `transfer_src` layout. The `pipeline` argument is the same /// triangle pipeline used in the interactive loop — drawn over the /// clear-color background so the captured PPM exercises the full /// forward path (vertex → rasterizer → fragment → blend), not just @@ -229,19 +229,10 @@ fn captureFrame( const offscreen_view = try device.createTextureView(offscreen, .{ .label = "capture.view" }); defer device.destroyTextureView(offscreen_view); - const staging_bytes: u64 = @as(u64, FRAME_WIDTH) * FRAME_HEIGHT * 4; - const staging = try device.createBuffer(.{ - .label = "capture.staging", - .size = staging_bytes, - .usage = .{ .copy_dst = true }, - .host_visible = true, - }); - defer device.destroyBuffer(staging); - const fence = try device.createFence(false); defer device.destroyFence(fence); - const enc = try device.createCommandEncoder("capture"); + const enc = try device.createCommandEncoder("capture.render"); defer device.destroyCommandEncoder(enc); var pass = try enc.beginRenderPass(.{ @@ -258,62 +249,19 @@ fn captureFrame( pass.setScissor(0, 0, FRAME_WIDTH, FRAME_HEIGHT); pipeline.draw(&pass); pass.end(); - - enc.copyTextureToBuffer( - .{ .texture = offscreen, .aspect = .color }, - .{ .buffer = staging, .bytes_per_row = FRAME_WIDTH * 4 }, - .{ .width = FRAME_WIDTH, .height = FRAME_HEIGHT }, - ); enc.finish(); try device.submit(enc, .{ .fence = fence }); try device.waitFence(fence, std.math.maxInt(u64)); - const rgba = try device.mapBuffer(staging); - defer device.unmapBuffer(staging); - - try writePPM(allocator, io, path, rgba); + // Readback + PPM encode/write now live on the public GAL surface + // (`Device.captureFrameToPPM`, M0.5 item 2). The render above left the + // offscreen texture in `transfer_src` layout, the contract the helper + // expects (cf. `gal/capture.zig`). + try device.captureFrameToPPM(allocator, io, offscreen, FRAME_WIDTH, FRAME_HEIGHT, path); log.info("captured frame {d} -> {s}", .{ frame_idx, path }); } -/// PPM P6 writer. Strips alpha from the RGBA8 source (drops every fourth -/// byte). `path`'s parent directory is created if missing. Binary P6 format: -/// "P6\n \n255\n" followed by W*H*3 bytes of RGB. -fn writePPM( - allocator: std.mem.Allocator, - io: std.Io, - path: []const u8, - rgba: []const u8, -) !void { - if (std.fs.path.dirname(path)) |dir| { - std.Io.Dir.cwd().createDirPath(io, dir) catch |e| switch (e) { - error.PathAlreadyExists => {}, - else => return e, - }; - } - - var file = try std.Io.Dir.cwd().createFile(io, path, .{ .truncate = true }); - defer file.close(io); - - var buf: [4096]u8 = undefined; - var writer = file.writer(io, &buf); - const w = &writer.interface; - - try w.print("P6\n{d} {d}\n255\n", .{ FRAME_WIDTH, FRAME_HEIGHT }); - - const pixel_count = @as(usize, FRAME_WIDTH) * FRAME_HEIGHT; - var rgb = try allocator.alloc(u8, pixel_count * 3); - defer allocator.free(rgb); - var i: usize = 0; - while (i < pixel_count) : (i += 1) { - rgb[i * 3 + 0] = rgba[i * 4 + 0]; - rgb[i * 3 + 1] = rgba[i * 4 + 1]; - rgb[i * 3 + 2] = rgba[i * 4 + 2]; - } - try w.writeAll(rgb); - try w.flush(); -} - fn runVulkan(allocator: std.mem.Allocator, io: std.Io, args: Args) !void { var window = try window_mod.Window.create(allocator, .{ .title = "Weld Triangle (M0.4)", diff --git a/src/core/ipc/transport_posix.zig b/src/core/ipc/transport_posix.zig index 855fb03..66a6f74 100644 --- a/src/core/ipc/transport_posix.zig +++ b/src/core/ipc/transport_posix.zig @@ -224,16 +224,28 @@ pub const Backend = struct { var offset: usize = 0; while (offset < bytes.len) { const n = sys.write(self.fd, bytes.ptr + offset, bytes.len - offset); - if (n < 0) return error.BrokenPipe; + if (n < 0) switch (std.c.errno(n)) { + // A signal interrupted the write before any byte moved — + // retry the same chunk rather than mis-report a broken + // pipe (mirrors std.posix's own `.INTR => continue`). + .INTR => continue, + else => return error.BrokenPipe, + }; if (n == 0) return error.BrokenPipe; offset += @intCast(n); } } pub fn recv(self: *Backend, buffer: []u8) Error!usize { - const n = sys.read(self.fd, buffer.ptr, buffer.len); - if (n < 0) return error.BrokenPipe; - return @intCast(n); + while (true) { + const n = sys.read(self.fd, buffer.ptr, buffer.len); + if (n < 0) switch (std.c.errno(n)) { + // Interrupted by a signal with nothing read yet — retry. + .INTR => continue, + else => return error.BrokenPipe, + }; + return @intCast(n); + } } pub fn sendWithHandles( @@ -273,8 +285,17 @@ pub const Backend = struct { .msg_flags = 0, }; - const n = sys.sendmsg(self.fd, &msg, MSG_NOSIGNAL); - if (n < 0) return error.BrokenPipe; + while (true) { + const n = sys.sendmsg(self.fd, &msg, MSG_NOSIGNAL); + if (n < 0) switch (std.c.errno(n)) { + // Interrupted before any byte/ancillary left the socket — + // retry the whole message. Nothing was sent, so the passed + // fds are not duplicated. (Same EINTR contract as `send`.) + .INTR => continue, + else => return error.BrokenPipe, + }; + break; + } } pub fn recvWithHandles( @@ -301,8 +322,22 @@ pub const Backend = struct { .msg_flags = 0, }; - const n = sys.recvmsg(self.fd, &msg, 0); - if (n < 0) return error.BrokenPipe; + var n: isize = undefined; + while (true) { + // `msg_controllen` and `msg_flags` are in/out for recvmsg; rearm + // them to the ancillary-buffer capacity each attempt so an EINTR + // retry never relies on msghdr state POSIX leaves unspecified on + // error. The ancillary buffer itself is not rebuilt. + msg.msg_controllen = ctrl_size; + msg.msg_flags = 0; + n = sys.recvmsg(self.fd, &msg, 0); + if (n < 0) switch (std.c.errno(n)) { + // Interrupted with nothing received yet — retry. + .INTR => continue, + else => return error.BrokenPipe, + }; + break; + } var handle_count: usize = 0; if (msg.msg_controllen >= @sizeOf(CmsgHdr) and handles_out.len > 0) { diff --git a/src/editor/vk_blit.zig b/src/editor/vk_blit.zig index 5300926..1eb9c1e 100644 --- a/src/editor/vk_blit.zig +++ b/src/editor/vk_blit.zig @@ -1,9 +1,10 @@ //! S6 editor Vulkan blit renderer. //! -//! WELD_LEGACY_VK_DISPATCH — pre-M0.4 code, migration to the Vulkan GAL -//! (cf. `src/modules/render/gal/vulkan/`) tracked as Phase 1+ debt. -//! The `no_device_dispatch_outside_gal` linter rule is suspended for -//! this file via the marker above. +//! Uses the idiomatic `vk.Device`/`vk.Queue` wrappers throughout, including +//! the `*Raw` acquire/present variants (`acquireNextImageKHRRaw`, +//! `presentKHRRaw`) where the raw `Result` is needed for swapchain-resize +//! handling — no `vk.device_dispatch.*` access remains, so the +//! `no_device_dispatch_outside_gal` rule holds with no grandfather marker. //! //! Opens a window-sized swapchain and a fullscreen-quad pipeline that //! samples the runtime-written viewport shm framebuffer (1280×720 @@ -952,12 +953,11 @@ pub fn drawFrame(r: *Renderer) vk.Error!bool { const cur = r.current_frame; try r.device.waitForFences(&.{r.in_flight[cur]}, 1, std.math.maxInt(u64)); - // Use the raw dispatch for `vkAcquireNextImageKHR` so we can + // Use the `*Raw` acquire wrapper (`acquireNextImageKHRRaw`) so we can // see `suboptimal_khr` and `error_out_of_date_khr` directly // — the wrapped Device method folds suboptimal into success. var img_index: u32 = 0; - const acquire_result = vk.device_dispatch.vkAcquireNextImageKHR( - r.device, + const acquire_result = r.device.acquireNextImageKHRRaw( r.swapchain, std.math.maxInt(u64), r.image_available[cur], @@ -1117,7 +1117,7 @@ pub fn drawFrame(r: *Renderer) vk.Error!bool { .p_image_indices = @ptrCast(&img_index), .p_results = @ptrCast(&per_swapchain_result), }; - const present_call = vk.device_dispatch.vkQueuePresentKHR(r.queue, &present); + const present_call = r.queue.presentKHRRaw(&present); switch (present_call) { .success => {}, .suboptimal_khr => r.swapchain_dirty = true, diff --git a/src/etch/ecs_bridge.zig b/src/etch/ecs_bridge.zig index 4fca399..fcdad8a 100644 --- a/src/etch/ecs_bridge.zig +++ b/src/etch/ecs_bridge.zig @@ -1,7 +1,7 @@ //! S4 Etch ↔ ECS adapter — translates the interpreter's name-based view of //! the world (`entity.get(Health).current`, `when resource Score changed`) //! onto the Tier 0 byte-oriented surface (`Registry`, `DynamicArchetype`, -//! `ResourceStore`, `RuntimeQuery`). +//! `ResourceStore`). //! //! The bridge owns the mapping `Etch component / resource name → ComponentId` //! built once at program load. It does not own the registry or the world — @@ -19,7 +19,6 @@ const FieldDesc = RegistryNS.FieldDesc; const World = weld_core.ecs.world.World; const DynamicArchetype = weld_core.ecs.archetype_dynamic.DynamicArchetype; const Chunk = weld_core.ecs.archetype_dynamic.Chunk; -const RuntimeQuery = weld_core.ecs.query_runtime.RuntimeQuery; const ResourceStore = weld_core.ecs.resources.ResourceStore; const CoreEntityId = weld_core.ecs.entity.EntityId; @@ -42,6 +41,7 @@ pub const BridgeError = error{ UnknownComponent, UnknownResource, UnknownField, + TypeMismatch, OutOfMemory, }; @@ -151,7 +151,7 @@ pub const Bridge = struct { const idx = arch.componentIndex(ref.component_id) orelse return BridgeError.UnknownComponent; const slot_bytes = arch.componentSlot(chunk, idx, ref.slot); const field_bytes = slot_bytes[field.offset .. field.offset + @as(u16, @intCast(field.kind.sizeBytes()))]; - writeValueAsBytes(field.kind, field_bytes, v); + try writeValueAsBytes(field.kind, field_bytes, v); } // ─── Resource access ───────────────────────────────────────────────── @@ -178,7 +178,7 @@ pub const Bridge = struct { const field = registry.findField(resource_id, field_name) orelse return BridgeError.UnknownField; const bytes = store.getMutResource(resource_id) orelse return BridgeError.UnknownResource; const slice = bytes[field.offset .. field.offset + @as(u16, @intCast(field.kind.sizeBytes()))]; - writeValueAsBytes(field.kind, slice, v); + try writeValueAsBytes(field.kind, slice, v); } }; @@ -224,17 +224,19 @@ pub fn readBytesAsValue(kind: FieldKind, bytes: []const u8) Value { }; } -/// Encode an interpreter `Value` into the on-storage byte -/// representation of a field. `bytes` must already be sized to the -/// field's column stride; the function panics on type mismatch (the -/// S4 closing-debt `D-S4-ecs-bridge-panic` will swap this for a -/// typed `TypeMismatch` variant in M0.7). -pub fn writeValueAsBytes(kind: FieldKind, bytes: []u8, v: Value) void { +/// Encode an interpreter `Value` into the on-storage byte representation of a +/// field. `bytes` must already be sized to the field's column stride. +/// +/// Returns `error.TypeMismatch` when `v`'s tag is incompatible with the +/// field's `kind` (M0.5 item 10 — resolves the S4 closing-debt +/// `D-S4-ecs-bridge-panic`: a type incoherence is now a recoverable typed +/// error propagated to the caller instead of a runtime `@panic`). +pub fn writeValueAsBytes(kind: FieldKind, bytes: []u8, v: Value) BridgeError!void { switch (kind) { .int_ => { const x: i64 = switch (v) { .int_ => |a| a, - else => @panic("type mismatch on writeValueAsBytes (int_)"), + else => return error.TypeMismatch, }; @memcpy(bytes[0..@sizeOf(i64)], std.mem.asBytes(&x)); }, @@ -242,25 +244,28 @@ pub fn writeValueAsBytes(kind: FieldKind, bytes: []u8, v: Value) void { const x: f64 = switch (v) { .float_ => |a| a, .int_ => |a| @floatFromInt(a), - else => @panic("type mismatch on writeValueAsBytes (float_)"), + else => return error.TypeMismatch, }; @memcpy(bytes[0..@sizeOf(f64)], std.mem.asBytes(&x)); }, .bool_ => { - const x: u8 = if (v.bool_) 1 else 0; - bytes[0] = x; + const b: bool = switch (v) { + .bool_ => |a| a, + else => return error.TypeMismatch, + }; + bytes[0] = if (b) 1 else 0; }, .i32_ => { const x: i32 = switch (v) { .int_ => |a| @intCast(a), - else => @panic("type mismatch on writeValueAsBytes (i32_)"), + else => return error.TypeMismatch, }; @memcpy(bytes[0..@sizeOf(i32)], std.mem.asBytes(&x)); }, .u32_ => { const x: u32 = switch (v) { .int_ => |a| @intCast(a), - else => @panic("type mismatch on writeValueAsBytes (u32_)"), + else => return error.TypeMismatch, }; @memcpy(bytes[0..@sizeOf(u32)], std.mem.asBytes(&x)); }, @@ -268,7 +273,7 @@ pub fn writeValueAsBytes(kind: FieldKind, bytes: []u8, v: Value) void { const x: f32 = switch (v) { .float_ => |a| @floatCast(a), .int_ => |a| @floatFromInt(a), - else => @panic("type mismatch on writeValueAsBytes (f32_)"), + else => return error.TypeMismatch, }; @memcpy(bytes[0..@sizeOf(f32)], std.mem.asBytes(&x)); }, @@ -279,21 +284,28 @@ pub fn writeValueAsBytes(kind: FieldKind, bytes: []u8, v: Value) void { test "readBytesAsValue / writeValueAsBytes roundtrip on int" { var buf: [8]u8 = undefined; - writeValueAsBytes(.int_, &buf, .{ .int_ = -42 }); + try writeValueAsBytes(.int_, &buf, .{ .int_ = -42 }); const v = readBytesAsValue(.int_, &buf); try std.testing.expectEqual(@as(i64, -42), v.int_); } test "readBytesAsValue / writeValueAsBytes roundtrip on float" { var buf: [8]u8 = undefined; - writeValueAsBytes(.float_, &buf, .{ .float_ = 3.14 }); + try writeValueAsBytes(.float_, &buf, .{ .float_ = 3.14 }); const v = readBytesAsValue(.float_, &buf); try std.testing.expectEqual(@as(f64, 3.14), v.float_); } test "readBytesAsValue / writeValueAsBytes roundtrip on bool" { var buf: [1]u8 = undefined; - writeValueAsBytes(.bool_, &buf, .{ .bool_ = true }); + try writeValueAsBytes(.bool_, &buf, .{ .bool_ = true }); const v = readBytesAsValue(.bool_, &buf); try std.testing.expect(v.bool_); } + +test "writeValueAsBytes returns TypeMismatch on an incompatible value tag" { + var buf: [8]u8 = undefined; + try std.testing.expectError(error.TypeMismatch, writeValueAsBytes(.int_, &buf, .{ .bool_ = true })); + try std.testing.expectError(error.TypeMismatch, writeValueAsBytes(.bool_, &buf, .{ .int_ = 1 })); + try std.testing.expectError(error.TypeMismatch, writeValueAsBytes(.f32_, &buf, .{ .bool_ = false })); +} diff --git a/src/etch/interp.zig b/src/etch/interp.zig index 519134c..d90a05b 100644 --- a/src/etch/interp.zig +++ b/src/etch/interp.zig @@ -34,8 +34,6 @@ const NodeId = ast_mod.NodeId; const StringId = ast_mod.StringId; const Diagnostic = diag_mod.Diagnostic; const Value = value_mod.Value; -const RuntimeError = value_mod.RuntimeError; -const RuntimeErrorKind = value_mod.RuntimeErrorKind; const EntityId = value_mod.EntityId; const Bridge = bridge_mod.Bridge; @@ -46,7 +44,6 @@ pub const RuntimeReport = struct { rules_evaluated: u64 = 0, rules_matched: u64 = 0, runtime_errors: u64 = 0, - last_error: ?RuntimeError = null, }; const ResourceDep = struct { @@ -664,7 +661,7 @@ fn compileTypeDecl( const v = evalConst(ast, f.default_value) catch continue; const fd = fields.items[f_i]; const slot = default_buf[fd.offset .. fd.offset + @as(u16, @intCast(fd.kind.sizeBytes()))]; - bridge_mod.writeValueAsBytes(fd.kind, slot, v); + try bridge_mod.writeValueAsBytes(fd.kind, slot, v); } const id = try world.registry.registerComponentRaw(gpa, .{ diff --git a/src/etch/zig_codegen/emit.zig b/src/etch/zig_codegen/emit.zig index 05b8706..e3bf8ba 100644 --- a/src/etch/zig_codegen/emit.zig +++ b/src/etch/zig_codegen/emit.zig @@ -28,6 +28,23 @@ pub const Writer = struct { try self.buffer.appendSlice(self.gpa, s); } + /// Write a user-chosen identifier, escaping it as `@"name"` when it + /// collides with a Zig keyword or primitive type so the generated source + /// stays compilable. Etch identifiers are otherwise valid Zig identifiers + /// (ASCII, no leading digit) — escaping is needed only for that collision, + /// which can arise for lowercase value-idents (field, binding, param + /// names); Etch type/component names are capitalized and never collide. + /// Cf. M0.5 item 8. + pub fn ident(self: *Writer, name: []const u8) !void { + if (std.zig.Token.keywords.has(name) or std.zig.primitives.isPrimitive(name)) { + try self.write("@\""); + try self.write(name); + try self.write("\""); + } else { + try self.write(name); + } + } + pub fn writeIndent(self: *Writer) !void { var i: u32 = 0; while (i < self.indent) : (i += 1) { diff --git a/src/etch/zig_codegen/lower.zig b/src/etch/zig_codegen/lower.zig index 2d2bef6..8994e90 100644 --- a/src/etch/zig_codegen/lower.zig +++ b/src/etch/zig_codegen/lower.zig @@ -176,12 +176,15 @@ fn emitComponentLikeStruct(w: *Writer, ast: *const AstArena, data: u32, kind: De const zig_type = type_map.mapBuiltin(etch_type) orelse return CodegenError.NonPodComponent; const fname = ast.strings.slice(f.name); if (f.default_value.isNone()) { - try w.printLine("{s}: {s} = {s},", .{ fname, zig_type, zeroDefault(zig_type) }); + try w.writeIndent(); + try w.ident(fname); + try w.print(": {s} = {s},\n", .{ zig_type, zeroDefault(zig_type) }); } else { // Emit the default expression. The S3 type-checker has already // verified it is const-evaluable on the field type. try w.writeIndent(); - try w.print("{s}: {s} = ", .{ fname, zig_type }); + try w.ident(fname); + try w.print(": {s} = ", .{zig_type}); try emitConstExpr(w, ast, f.default_value, zig_type); try w.write(",\n"); } @@ -426,7 +429,9 @@ fn emitRuleAsComptimeQuery(w: *Writer, ast: *const AstArena, rule: ast_mod.RuleD if (info.field_filter) |ff| { const idx = indexOfComponent(info.components, ff.component_name) orelse return CodegenError.InternalCodegenBug; try w.writeIndent(); - try w.print("if (__row[{d}].{s} != ", .{ idx, ff.field_name }); + try w.print("if (__row[{d}].", .{idx}); + try w.ident(ff.field_name); + try w.write(" != "); try emitConstExpr(w, ast, ff.value, ff.value_zig_type); try w.write(") continue;\n"); } @@ -471,7 +476,9 @@ fn emitRuleAsArchWalk(w: *Writer, ast: *const AstArena, rule: ast_mod.RuleDecl, w.indentBy(1); if (info.field_filter) |ff| { try w.writeIndent(); - try w.print("if ({s}_arr[slot].{s} != ", .{ ff.component_name, ff.field_name }); + try w.print("if ({s}_arr[slot].", .{ff.component_name}); + try w.ident(ff.field_name); + try w.write(" != "); try emitConstExpr(w, ast, ff.value, ff.value_zig_type); try w.write(") continue;\n"); } @@ -822,7 +829,9 @@ fn emitLet(w: *Writer, ast: *const AstArena, ctx: *LocalCtx, let: ast_mod.LetStm const keyword = if (let.is_mut) "var" else "const"; try w.writeIndent(); - try w.print("{s} {s}: {s} = ", .{ keyword, ast.strings.slice(let.name), zig_t }); + try w.print("{s} ", .{keyword}); + try w.ident(ast.strings.slice(let.name)); + try w.print(": {s} = ", .{zig_t}); try emitExpr(w, ast, ctx, let.value); try w.write(";\n"); @@ -871,13 +880,13 @@ fn emitExpr(w: *Writer, ast: *const AstArena, ctx: *LocalCtx, id: NodeId) Codege const name_id: StringId = data; if (ctx.lookup(name_id)) |local| { switch (local.kind) { - .value => try w.write(ast.strings.slice(name_id)), + .value => try w.ident(ast.strings.slice(name_id)), .component_alias => try emitComponentSlot(w, ctx, local.component_name), } } else { // Unknown ident — type-checker should have caught this. Emit // the raw name and let `zig build` complain. - try w.write(ast.strings.slice(name_id)); + try w.ident(ast.strings.slice(name_id)); } }, .field_access => { @@ -936,7 +945,8 @@ fn binaryOpText(op: ast_mod.BinaryOp) []const u8 { fn emitFieldAccessExpr(w: *Writer, ast: *const AstArena, ctx: *LocalCtx, fa: ast_mod.FieldAccessExpr) CodegenError!void { try emitExpr(w, ast, ctx, fa.receiver); - try w.print(".{s}", .{ast.strings.slice(fa.field_name)}); + try w.write("."); + try w.ident(ast.strings.slice(fa.field_name)); } fn inferZigType(ast: *const AstArena, ctx: *LocalCtx, expr: NodeId, annotation: NodeId) []const u8 { diff --git a/src/modules/audio/main.zig b/src/modules/audio/root.zig similarity index 100% rename from src/modules/audio/main.zig rename to src/modules/audio/root.zig diff --git a/src/modules/render/gal/capture.zig b/src/modules/render/gal/capture.zig new file mode 100644 index 0000000..9dd8d48 --- /dev/null +++ b/src/modules/render/gal/capture.zig @@ -0,0 +1,139 @@ +//! GAL frame-capture helper — Phase 0 / M0.5 (item 2). +//! +//! Reads back an already-rendered color texture into host memory and writes +//! it as a binary PPM (P6) file. Extracted from the M0.4 triangle example so +//! the staging-buffer readback + PPM-encoding boilerplate lives once on the +//! public GAL surface instead of being hand-rolled by every consumer +//! (cf. `engine-zig-conventions.md` §13 surface coverage — the triangle +//! example is the canonical empirical consumer of this path). +//! +//! The helper composes existing GAL primitives only (`createBuffer`, +//! `createCommandEncoder`, `copyTextureToBuffer`, `submit`, `waitFence`, +//! `mapBuffer`); it adds no backend-specific code, so a single generic +//! implementation serves every backend. Each backend `Device` exposes it as +//! the method `captureFrameToPPM` (a one-line delegation) and the interface +//! contract (`interface.required_methods`) lists it so every backend must +//! provide it. +//! +//! Caller contract: `texture` must already be rendered and left in a +//! transfer-source layout (e.g. a render pass with +//! `final_layout = .transfer_src`), and the rendering work must be complete +//! before the call (the caller waits its own render fence). The helper +//! performs its own copy submit + fence wait, then a CPU readback. + +const std = @import("std"); +const types = @import("types.zig"); + +/// Capture an already-rendered RGBA8 color `texture` of `width`×`height` +/// to a binary PPM (P6) file at `path`. Generic over the backend `Device` +/// (duck-typed on the GAL primitives it calls). The texture must be in a +/// transfer-source layout with its rendering already complete (cf. the +/// file-header caller contract). +/// +/// Thread-safety: not thread-safe — issues a one-shot copy submit + fence +/// wait on the device. Intended for the debug / smoke-test capture path, +/// not the hot render loop. +/// +/// Errors: +/// - `error.Unsupported` if the backend cannot map a host-visible buffer +/// (e.g. the Null backend), so portable callers can skip capture. +/// - any GAL error from the buffer / encoder / submit / fence operations. +/// - any I/O error from encoding or writing `path`. +pub fn captureFrameToPPM( + device: anytype, + gpa: std.mem.Allocator, + io: std.Io, + texture: types.TextureHandle, + width: u32, + height: u32, + path: []const u8, +) !void { + const staging_bytes: u64 = @as(u64, width) * height * 4; + const staging = try device.createBuffer(.{ + .label = "gal.capture.staging", + .size = staging_bytes, + .usage = .{ .copy_dst = true }, + .host_visible = true, + }); + defer device.destroyBuffer(staging); + + const fence = try device.createFence(false); + defer device.destroyFence(fence); + + const enc = try device.createCommandEncoder("gal.capture"); + defer device.destroyCommandEncoder(enc); + + enc.copyTextureToBuffer( + .{ .texture = texture, .aspect = .color }, + .{ .buffer = staging, .bytes_per_row = width * 4 }, + .{ .width = width, .height = height }, + ); + enc.finish(); + + try device.submit(enc, .{ .fence = fence }); + try device.waitFence(fence, std.math.maxInt(u64)); + + const rgba = try device.mapBuffer(staging); + defer device.unmapBuffer(staging); + + try writePpm(gpa, io, path, rgba, width, height); +} + +/// Write an RGBA8 pixel buffer as a binary PPM (P6) file at `path`, dropping +/// the alpha channel. The parent directory of `path` is created if missing. +/// `rgba.len` must be at least `width*height*4`. +/// +/// Errors: any I/O error from creating the directory, the file, or writing. +pub fn writePpm( + gpa: std.mem.Allocator, + io: std.Io, + path: []const u8, + rgba: []const u8, + width: u32, + height: u32, +) !void { + const ppm = try encodePpm(gpa, rgba, width, height); + defer gpa.free(ppm); + + if (std.fs.path.dirname(path)) |dir| { + std.Io.Dir.cwd().createDirPath(io, dir) catch |e| switch (e) { + error.PathAlreadyExists => {}, + else => return e, + }; + } + + var file = try std.Io.Dir.cwd().createFile(io, path, .{ .truncate = true }); + defer file.close(io); + + var buf: [4096]u8 = undefined; + var writer = file.writer(io, &buf); + const w = &writer.interface; + try w.writeAll(ppm); + try w.flush(); +} + +/// Encode an RGBA8 pixel buffer as binary PPM (P6) bytes: the ASCII header +/// `"P6\n \n255\n"` followed by `width*height` RGB triplets (the alpha +/// byte of each source pixel is dropped). Caller owns the returned slice. +/// Pure function (no I/O, no device) — the unit-testable core of the capture +/// path. `rgba.len` must be at least `width*height*4`. +/// +/// Errors: `error.OutOfMemory`. +pub fn encodePpm(gpa: std.mem.Allocator, rgba: []const u8, width: u32, height: u32) ![]u8 { + const pixel_count = @as(usize, width) * height; + std.debug.assert(rgba.len >= pixel_count * 4); + + var header_buf: [48]u8 = undefined; + const header = std.fmt.bufPrint(&header_buf, "P6\n{d} {d}\n255\n", .{ width, height }) catch unreachable; + + const out = try gpa.alloc(u8, header.len + pixel_count * 3); + @memcpy(out[0..header.len], header); + + var i: usize = 0; + while (i < pixel_count) : (i += 1) { + out[header.len + i * 3 + 0] = rgba[i * 4 + 0]; + out[header.len + i * 3 + 1] = rgba[i * 4 + 1]; + out[header.len + i * 3 + 2] = rgba[i * 4 + 2]; + } + return out; +} diff --git a/src/modules/render/gal/interface.zig b/src/modules/render/gal/interface.zig index b092bd6..f1dd216 100644 --- a/src/modules/render/gal/interface.zig +++ b/src/modules/render/gal/interface.zig @@ -79,6 +79,9 @@ pub const required_methods = [_]RequiredMethod{ .{ .name = "getQueue", .purpose = "Gets a Queue (graphics/compute/transfer)" }, .{ .name = "createCommandEncoder", .purpose = "Starts recording a command buffer" }, .{ .name = "submit", .purpose = "Submits a finished CommandEncoder to the graphics queue (cf. SubmitDescriptor)" }, + + // Capture (M0.5 item 2) + .{ .name = "captureFrameToPPM", .purpose = "Reads back a rendered texture and writes it as a PPM file (cf. gal/capture.zig)" }, }; /// Comptime check that `Backend` declares all the required methods. @@ -244,6 +247,9 @@ const TestShape = struct { pub fn submit(self: *TestShape, encoder: *CommandEncoderStub, descriptor: types.SubmitDescriptor) types.Error!void { _ = .{ self, encoder, descriptor }; } + pub fn captureFrameToPPM(self: *TestShape, gpa: std.mem.Allocator, io: std.Io, texture: types.TextureHandle, width: u32, height: u32, path: []const u8) types.Error!void { + _ = .{ self, gpa, io, texture, width, height, path }; + } }; const CommandEncoderStub = struct {}; diff --git a/src/modules/render/gal/null/device.zig b/src/modules/render/gal/null/device.zig index d42deee..3dba9bd 100644 --- a/src/modules/render/gal/null/device.zig +++ b/src/modules/render/gal/null/device.zig @@ -15,6 +15,7 @@ const std = @import("std"); const types = @import("../types.zig"); const escape = @import("../escape_hatches.zig"); const stubs = @import("stubs.zig"); +const capture = @import("../capture.zig"); /// Null Device. Stores only the allocator, a handle counter, and /// a single queue (graphics+compute+transfer fused). Makes no system @@ -255,6 +256,23 @@ pub const Device = struct { pub fn destroyCommandEncoder(self: *Device, encoder: *stubs.CommandEncoder) void { self.allocator.destroy(encoder); } + + /// Read back a rendered color texture and write it as a binary PPM + /// file. Thin delegation to the backend-agnostic `gal.capture` helper + /// (M0.5 item 2). The Null backend has no real buffer storage, so the + /// readback surfaces `error.Unsupported` at `mapBuffer` — portable + /// callers can skip capture on the Null backend. + pub fn captureFrameToPPM( + self: *Device, + gpa: std.mem.Allocator, + io: std.Io, + texture: types.TextureHandle, + width: u32, + height: u32, + path: []const u8, + ) !void { + return capture.captureFrameToPPM(self, gpa, io, texture, width, height, path); + } }; // ============================================================================ diff --git a/src/modules/render/gal/main.zig b/src/modules/render/gal/root.zig similarity index 95% rename from src/modules/render/gal/main.zig rename to src/modules/render/gal/root.zig index 2687049..fd8369e 100644 --- a/src/modules/render/gal/main.zig +++ b/src/modules/render/gal/root.zig @@ -35,6 +35,10 @@ pub const barriers = @import("barriers.zig"); pub const null_backend = @import("null/device.zig"); /// Vulkan backend — Phase 0+ implementation (cf. brief §Scope). pub const vulkan_backend = @import("vulkan/device.zig"); +/// Frame-capture helper (texture → PPM readback), M0.5 item 2. Backend- +/// agnostic; each backend `Device` also exposes it as a `captureFrameToPPM` +/// method (cf. `gal/capture.zig`). +pub const capture = @import("capture.zig"); // Caller-friendly re-exports (avoids the `gal.types.*` nesting). diff --git a/src/modules/render/gal/vulkan/device.zig b/src/modules/render/gal/vulkan/device.zig index eab06f7..2d3fc1c 100644 --- a/src/modules/render/gal/vulkan/device.zig +++ b/src/modules/render/gal/vulkan/device.zig @@ -41,6 +41,7 @@ const cmd_mod = @import("command_encoder.zig"); const sync_mod = @import("sync.zig"); const queue_mod = @import("queue.zig"); const frame_mod = @import("frame.zig"); +const capture = @import("../capture.zig"); const log = std.log.scoped(.gal_vk); @@ -525,6 +526,22 @@ pub const Device = struct { .fence = descriptor.fence, }); } + + /// Read back a rendered color texture and write it as a binary PPM + /// file. Thin delegation to the backend-agnostic `gal.capture` helper + /// (M0.5 item 2); see `gal/capture.zig` for the caller contract + /// (texture in transfer-source layout, rendering already complete). + pub fn captureFrameToPPM( + self: *Device, + gpa: std.mem.Allocator, + io: std.Io, + texture: types.TextureHandle, + width: u32, + height: u32, + path: []const u8, + ) !void { + return capture.captureFrameToPPM(self, gpa, io, texture, width, height, path); + } }; // ============================================================================ @@ -744,6 +761,6 @@ fn createLogicalDevice(device: *Device) !void { test "device: struct shape compiles with all GAL methods" { // The test is compile-only; checks that Device has all the methods // required by interface.checkBackend. The real check is triggered - // by gal/main.zig. + // by gal/root.zig. _ = Device; } diff --git a/src/modules/render/render_graph/graph.zig b/src/modules/render/render_graph/graph.zig index 1328bba..ac8b375 100644 --- a/src/modules/render/render_graph/graph.zig +++ b/src/modules/render/render_graph/graph.zig @@ -13,7 +13,7 @@ //! §Notes decision 2). const std = @import("std"); -const gal = @import("../gal/main.zig"); +const gal = @import("../gal/root.zig"); const pass_mod = @import("pass.zig"); /// Error set of the render graph. @@ -94,7 +94,17 @@ pub const Graph = struct { var j: usize = 0; while (j < n) : (j += 1) { if (i == j) continue; - if (passDependsOn(&self.passes.items[j], &self.passes.items[i])) { + // RAW (data-directional): pass i writes a resource pass j reads, + // so j must run after i. Checked in both orderings, so a mutual + // RAW (a true data cycle) still surfaces as error.RenderGraphCycle. + var has_edge = passDependsOn(&self.passes.items[j], &self.passes.items[i]); + // WAW insertion-order tiebreak: two passes writing the same + // resource are serialized lower-index-first. Only the i < j edge + // is emitted, so WAW is acyclic by construction (no reverse edge). + if (i < j and writesSameResource(&self.passes.items[i], &self.passes.items[j])) { + has_edge = true; + } + if (has_edge) { try adj[i].append(self.allocator, @intCast(j)); in_degree[j] += 1; } @@ -168,30 +178,37 @@ pub const Graph = struct { } }; -/// Determines whether `b` topologically depends on `a` (i.e. whether `a` -/// must execute before `b` in the DAG). -/// -/// Phase 0: only RAW and WAW create a topological dependency for -/// the execution order. -/// - **RAW** (Read-After-Write): `a` produces a resource that `b` consumes -/// → producer/consumer, `b` must wait for `a`. -/// - **WAW** (Write-After-Write): both write the same resource → -/// serialization required for coherence. +/// Determines whether `b` has a RAW (Read-After-Write) data dependency on +/// `a`: `a` writes a resource that `b` reads, so `a` must execute before `b`. +/// This is a pure, direction-by-data query — it does not consult insertion +/// order, and it deliberately does NOT cover WAW. /// -/// **WAR is NOT a topological dependency**: it is a pure memory -/// hazard (the writer must not clobber while the reader reads), -/// handled by the `BarrierTracker` (cf. `gal/barriers.zig`) which inserts the -/// barrier without imposing a topological order. Consistent with WebGPU and -/// the Frostbite/Bevy/Mach render graphs. +/// - **RAW** (Read-After-Write): `a` produces a resource that `b` consumes, +/// so `b` must wait for `a`. The direction is fixed by the data flow. +/// - **WAW** (Write-After-Write) is handled in `compile`, not here: two passes +/// writing the same resource are serialized by insertion order (lower index +/// first) via `writesSameResource` + the `i < j` tiebreak. Keeping it out of +/// this query is what makes WAW acyclic (M0.5 item 7 — the old symmetric WAW +/// check here emitted edges both ways and a false `RenderGraphCycle`). +/// - **WAR is NOT a topological dependency**: it is a pure memory hazard, +/// handled by the `BarrierTracker` (cf. `gal/barriers.zig`) which inserts the +/// barrier without imposing a topological order. Consistent with WebGPU and +/// the Frostbite/Bevy/Mach render graphs. /// -/// Cycle = two passes that mutually produce each other's inputs -/// (a RAW in both directions, or a circular WAW). +/// A real cycle is therefore only a mutual RAW — two passes each producing the +/// other's input — surfaced by `compile` as `error.RenderGraphCycle`. fn passDependsOn(b: *const pass_mod.Pass, a: *const pass_mod.Pass) bool { // RAW: a.writes ∩ b.reads for (a.writes) |aw| { for (b.reads) |br| if (sameResource(aw.resource, br.resource)) return true; } - // WAW: a.writes ∩ b.writes + return false; +} + +/// True if `a` and `b` write at least one resource in common (a WAW hazard). +/// The ordering of the two writers is the `compile` insertion-order tiebreak, +/// decided there rather than here so WAW never forms a cycle. +fn writesSameResource(a: *const pass_mod.Pass, b: *const pass_mod.Pass) bool { for (a.writes) |aw| { for (b.writes) |bw| if (sameResource(aw.resource, bw.resource)) return true; } diff --git a/src/modules/render/render_graph/pass.zig b/src/modules/render/render_graph/pass.zig index 8065083..086d3d3 100644 --- a/src/modules/render/render_graph/pass.zig +++ b/src/modules/render/render_graph/pass.zig @@ -11,7 +11,7 @@ //! (cf. `gal/barriers.zig`), unless `barrier_mode = .explicit` const std = @import("std"); -const gal = @import("../gal/main.zig"); +const gal = @import("../gal/root.zig"); const escape = gal.escape_hatches; /// Reference to a resource read or written by a pass. diff --git a/src/modules/render/render_graph/passes/capture.zig b/src/modules/render/render_graph/passes/capture.zig index 724b157..51f93c4 100644 --- a/src/modules/render/render_graph/passes/capture.zig +++ b/src/modules/render/render_graph/passes/capture.zig @@ -14,7 +14,7 @@ //! `transient.captured` flag on the resource declaration side. const std = @import("std"); -const gal = @import("../../gal/main.zig"); +const gal = @import("../../gal/root.zig"); const pass_mod = @import("../pass.zig"); /// Configuration of the capture pass. diff --git a/src/modules/render/render_graph/passes/depth_prepass.zig b/src/modules/render/render_graph/passes/depth_prepass.zig index 215ae5f..2d3b3a9 100644 --- a/src/modules/render/render_graph/passes/depth_prepass.zig +++ b/src/modules/render/render_graph/passes/depth_prepass.zig @@ -9,7 +9,7 @@ //! The slot remains for tests / legacy scenes compatibility. const std = @import("std"); -const gal = @import("../../gal/main.zig"); +const gal = @import("../../gal/root.zig"); const pass_mod = @import("../pass.zig"); /// Configuration of the depth prepass. diff --git a/src/modules/render/render_graph/passes/forward.zig b/src/modules/render/render_graph/passes/forward.zig index 3e9c7bc..a3ea7c7 100644 --- a/src/modules/render/render_graph/passes/forward.zig +++ b/src/modules/render/render_graph/passes/forward.zig @@ -11,7 +11,7 @@ //! `--smoke-test` mode). const std = @import("std"); -const gal = @import("../../gal/main.zig"); +const gal = @import("../../gal/root.zig"); const pass_mod = @import("../pass.zig"); /// Configuration of the forward opaque pass. diff --git a/src/modules/render/main.zig b/src/modules/render/root.zig similarity index 98% rename from src/modules/render/main.zig rename to src/modules/render/root.zig index c2d5aee..cb7390b 100644 --- a/src/modules/render/main.zig +++ b/src/modules/render/root.zig @@ -13,7 +13,7 @@ /// GAL namespace — public types, comptime interface, Null + Vulkan backends, /// barriers tracker, escape hatches. -pub const gal = @import("gal/main.zig"); +pub const gal = @import("gal/root.zig"); // Re-export of the render_graph submodule. diff --git a/tests/core/events/saturation_test.zig b/tests/core/events/saturation_test.zig index 73e7ad6..1ffa6e7 100644 --- a/tests/core/events/saturation_test.zig +++ b/tests/core/events/saturation_test.zig @@ -58,24 +58,16 @@ test "drops counter is reset after drainAtBoundary" { } test "drops above warning threshold emits a warn log on drain" { - // Capture the test scope's log output via a thread-local - // override is brittle; instead, this test exercises the - // threshold semantics by checking that: - // - Below threshold, no log is emitted (proxied by the - // fact that the threshold constant is preserved across - // drain cycles and the drops counter resets cleanly). - // - Above threshold, the bus drains without crashing - // (the log macro is `std.log.scoped(.events).warn`, - // a runtime no-op when stripped in release; correctness - // is "does not crash + drops still reset"). - // - // A capture-based check would require a custom std.log - // sink installed in test main(), which the project's test - // target does not currently configure. The threshold - // constant `DROPS_WARN_THRESHOLD = 10` is part of the - // public surface; tests assert the value so changes are - // deliberate. - + // This test deliberately drives the queue past DROPS_WARN_THRESHOLD, so + // the drain emits `std.log.scoped(.events).warn`. That warn is EXPECTED + // output, not a failure: `zig build test` surfaces it as a benign + // "failed command: …--listen=-" line while the build still exits 0 and + // the test passes. A capture-based assertion would need a custom std.log + // sink on the test runner — `std_options` declared in a test file is + // ignored because the runner, not the test file, is the compilation root + // — which is out of scope here. Correctness is therefore checked by + // "drops exceed threshold, drain does not crash, drops reset". The + // threshold is public surface, so the test pins its value. try std.testing.expectEqual(@as(u64, 10), events.DROPS_WARN_THRESHOLD); const gpa = std.testing.allocator; diff --git a/tests/core/plugin_loader/load_unload_test.zig b/tests/core/plugin_loader/load_unload_test.zig index 1b47355..6e25ec7 100644 --- a/tests/core/plugin_loader/load_unload_test.zig +++ b/tests/core/plugin_loader/load_unload_test.zig @@ -21,10 +21,12 @@ const Loader = weld_core.plugin_loader.Loader; // Note: the loader emits `log.warn` (not `log.err`) on its // error paths (`MissingEntryPoint`, `ApiVersionTooNew`, // `LibraryLoadFailed`). The error is carried by the -// `LoaderError` return, the log is purely diagnostic. This -// choice avoids the Zig 0.16 test runner counting the logs as -// failures when the tests are precisely exercising the error -// paths. +// `LoaderError` return, the log is purely diagnostic. Using +// warn (not err) keeps the Zig 0.16 test runner from counting +// these intentional logs as failures. The warn text still +// reaches stderr, which `zig build test` surfaces as a benign +// "failed command: …--listen=-" line — the build exits 0 and +// the tests pass; that line is not a failure. const lib_prefix = switch (builtin.os.tag) { .windows => "", diff --git a/tests/etch/keyword_ident_test.zig b/tests/etch/keyword_ident_test.zig new file mode 100644 index 0000000..3eceb7c --- /dev/null +++ b/tests/etch/keyword_ident_test.zig @@ -0,0 +1,68 @@ +//! Item 8 (M0.5): Etch identifiers that collide with Zig keywords. +//! +//! An Etch program may legitimately name a component / field / binding with a +//! word that is a Zig keyword (`align`, `var`, `error`, `comptime`, …) — these +//! are NOT Etch keywords, so the parser and interpreter accept them. The Zig +//! codegen must therefore not emit such names as bare Zig identifiers (which +//! would be uncompilable); it escapes them via `@"…"`. +//! +//! This test codegens such a program and parses the *generated Zig* with Zig's +//! own parser: a bare keyword in identifier position (e.g. `pub const align = +//! …`) is a parse error, so asserting zero parse errors enforces escaping +//! across every emission site the program exercises (component name, field +//! name, type reference, field access). RED before the fix, green after. + +const std = @import("std"); +const etch = @import("weld_etch"); + +fn codegenZig(gpa: std.mem.Allocator, src: []const u8, out: *std.ArrayListUnmanaged(u8)) !void { + var pr = try etch.parseSource(gpa, src); + defer pr.ast.deinit(gpa); + defer if (pr.diagnostic) |*d| d.deinit(gpa); + try std.testing.expect(pr.diagnostic == null); + + var diags: std.ArrayListUnmanaged(etch.Diagnostic) = .empty; + defer { + for (diags.items) |*d| d.deinit(gpa); + diags.deinit(gpa); + } + try etch.typeCheck(gpa, &pr.ast, &diags); + try std.testing.expectEqual(@as(usize, 0), diags.items.len); + + _ = try etch.codegen_zig.generateToBuffer(gpa, &pr.ast, "keyword_ident", out); +} + +test "etch idents that are zig keywords codegen to parseable zig" { + const gpa = std.testing.allocator; + // `var` is a Zig keyword but a valid Etch field name. (Component / type + // names must be capitalized TYPE_IDENTs, so they can never be Zig + // keywords; only lowercase value-idents — fields, bindings, params — can + // collide.) The codegen must escape the field name at every site it + // emits as a Zig identifier: the struct field declaration, the when-clause + // field filter, and the field-access expression. + const src = + \\component Counter { var: int = 0 } + \\rule bump(entity: Entity) + \\ when entity has Counter { var == 0 } + \\{ + \\ entity.get_mut(Counter).var += 1 + \\} + ; + var buf: std.ArrayListUnmanaged(u8) = .empty; + defer buf.deinit(gpa); + try codegenZig(gpa, src, &buf); + + // The generated Zig must be syntactically valid. Zig's parser rejects a + // bare keyword in identifier position, so a missed escape surfaces here. + const z = try gpa.dupeZ(u8, buf.items); + defer gpa.free(z); + var tree = try std.zig.Ast.parse(gpa, z, .zig); + defer tree.deinit(gpa); + if (tree.errors.len != 0) { + std.debug.print( + "generated zig has {d} parse error(s):\n{s}\n", + .{ tree.errors.len, buf.items }, + ); + } + try std.testing.expectEqual(@as(usize, 0), tree.errors.len); +} diff --git a/tests/lint/bad/device_dispatch/has_marker.zig b/tests/lint/bad/device_dispatch/has_marker.zig new file mode 100644 index 0000000..bd6394c --- /dev/null +++ b/tests/lint/bad/device_dispatch/has_marker.zig @@ -0,0 +1,7 @@ +//! Fixture: a file carrying the forbidden `WELD_LEGACY_VK_DISPATCH` marker. +//! +//! WELD_LEGACY_VK_DISPATCH — the grandfather escape is banned (M0.5 item 4). +//! `weld_lint lint` must flag this file's header marker with a non-zero exit, +//! even though the file itself uses no `vk.device_dispatch` access. + +const placeholder = 0; diff --git a/tests/lint/bad/device_dispatch/raw_dispatch.zig b/tests/lint/bad/device_dispatch/raw_dispatch.zig new file mode 100644 index 0000000..39f56b8 --- /dev/null +++ b/tests/lint/bad/device_dispatch/raw_dispatch.zig @@ -0,0 +1,10 @@ +//! Fixture: raw `vk.device_dispatch` access outside the GAL Vulkan backend. +//! +//! `weld_lint lint` must flag this with a non-zero exit. With the M0.5 +//! hardening, no `WELD_LEGACY_VK_DISPATCH` grandfather marker can suppress it. + +const vk = @import("vk"); + +fn present() void { + _ = vk.device_dispatch.vkQueuePresentKHR; +} diff --git a/tests/lint/runner_test.zig b/tests/lint/runner_test.zig index 6f4b36c..ff21a64 100644 --- a/tests/lint/runner_test.zig +++ b/tests/lint/runner_test.zig @@ -135,6 +135,11 @@ test "rule c_module_isolation flags bad fixtures" { try forEachZigFile(ctx.gpa, ctx.io, "tests/lint/bad/c_module_isolation", &ctx, &assertBadFixture); } +test "rule no_device_dispatch_outside_gal flags raw dispatch and forbidden marker" { + var ctx: Context = .{ .gpa = std.testing.allocator, .io = std.testing.io }; + try forEachZigFile(ctx.gpa, ctx.io, "tests/lint/bad/device_dispatch", &ctx, &assertBadFixture); +} + test "good fixtures pass clean" { var ctx: Context = .{ .gpa = std.testing.allocator, .io = std.testing.io }; try forEachZigFile(ctx.gpa, ctx.io, "tests/lint/good", &ctx, &assertGoodFile); diff --git a/tests/render/capture_helper.zig b/tests/render/capture_helper.zig new file mode 100644 index 0000000..fc3bc69 --- /dev/null +++ b/tests/render/capture_helper.zig @@ -0,0 +1,55 @@ +//! GAL capture-helper surface coverage — Phase 0 / M0.5 (item 2, §13). +//! +//! Exercises the public `gal.capture` surface (`encodePpm` + the +//! `Device.captureFrameToPPM` method) so its bodies are analyzed by a real +//! consumer (cf. `engine-zig-conventions.md` §13 — a public GAL symbol must +//! be exercised by a test that calls it with realistic data and asserts the +//! result, not merely compiled). The full Vulkan readback path is covered +//! end-to-end on lavapipe CI by the triangle example + the PSNR gate +//! (`tests/render/ppm_psnr_compare.zig`); here we cover the backend-agnostic +//! core (PPM encoding) and the Null backend's clean `Unsupported` +//! propagation, both of which run on every platform incl. the macOS dev box. + +const std = @import("std"); +const gal = @import("weld_render").gal; + +test "encodePpm: RGBA8 encodes to P6 with alpha stripped" { + const gpa = std.testing.allocator; + // 2x1 image: opaque red, half-alpha green. The encoder drops alpha. + const rgba = [_]u8{ 255, 0, 0, 255, 0, 255, 0, 128 }; + const ppm = try gal.capture.encodePpm(gpa, &rgba, 2, 1); + defer gpa.free(ppm); + + const header = "P6\n2 1\n255\n"; + try std.testing.expect(std.mem.startsWith(u8, ppm, header)); + try std.testing.expectEqual(header.len + 2 * 3, ppm.len); + try std.testing.expectEqualSlices(u8, &.{ 255, 0, 0, 0, 255, 0 }, ppm[header.len..]); +} + +test "encodePpm: header carries the requested dimensions" { + const gpa = std.testing.allocator; + const rgba = [_]u8{0} ** (4 * 4 * 4); + const ppm = try gal.capture.encodePpm(gpa, &rgba, 4, 4); + defer gpa.free(ppm); + try std.testing.expect(std.mem.startsWith(u8, ppm, "P6\n4 4\n255\n")); +} + +test "captureFrameToPPM: Null backend surfaces Unsupported readback" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var device = try gal.null_backend.Device.init(gpa, .{ .label = "capture-helper-test" }); + defer device.deinit(); + + const tex = try device.createTexture(.{ + .format = .rgba8_unorm, + .width = 4, + .height = 4, + .usage = .{ .color_attachment = true, .copy_src = true }, + }); + defer device.destroyTexture(tex); + + // The Null backend cannot map a host-visible buffer, so the readback + // path must surface error.Unsupported cleanly — no leak, no file written. + const result = device.captureFrameToPPM(gpa, io, tex, 4, 4, "out/__null_capture_unused.ppm"); + try std.testing.expectError(error.Unsupported, result); +} diff --git a/tests/render/gal_null_smoke.zig b/tests/render/gal_null_smoke.zig index 3e19f8c..0ac9178 100644 --- a/tests/render/gal_null_smoke.zig +++ b/tests/render/gal_null_smoke.zig @@ -18,7 +18,7 @@ const gal = @import("weld_render"); // ---------------------------------------------------------------------- Pins -- // // Guarantee the analysis of the inline tests under `src/modules/render/gal/`. -// The `gal/main.zig` module already re-exports its sub-files via `pub const`, +// The `gal/root.zig` module already re-exports its sub-files via `pub const`, // but we also pin explicitly to withstand a future refactor that would // turn some re-exports private. diff --git a/tests/render/ppm_psnr_compare.zig b/tests/render/ppm_psnr_compare.zig new file mode 100644 index 0000000..d5c5f7b --- /dev/null +++ b/tests/render/ppm_psnr_compare.zig @@ -0,0 +1,111 @@ +//! Direct PPM PSNR gate — Phase 0 / M0.5 (item 1). +//! +//! Reads the smoke-test capture at `out/smoke_test.ppm` (produced by a prior +//! `run-example-triangle --smoke-test --capture-frame=N` step) and compares +//! it against the committed golden via PSNR — WITHOUT rebuilding the render +//! stack and WITHOUT re-running the triangle. This replaces the CI +//! `runtime-smoke-test` "Verify PSNR" step's `zig build test-render-capture` +//! invocation, which rebuilt the test target in ReleaseSafe AND re-spawned +//! the triangle even though the PPM was already produced by the prior step. +//! Cost saved: ~3-5 min/run (cf. brief item 1). +//! +//! This module imports only `std` (no `weld_render`), so `zig build +//! test-ppm-psnr` compiles in seconds. The gate skips when either PPM is +//! absent (e.g. run locally without first producing the capture). + +const std = @import("std"); + +const FRAME_WIDTH: u32 = 1280; +const FRAME_HEIGHT: u32 = 720; +const PSNR_GATE_DB: f64 = 40.0; +const GOLDEN_PATH: []const u8 = "tests/golden/smoke_test_software.ppm"; +const CAPTURED_PATH: []const u8 = "out/smoke_test.ppm"; + +fn fileExists(io: std.Io, path: []const u8) bool { + var f = std.Io.Dir.cwd().openFile(io, path, .{}) catch return false; + f.close(io); + return true; +} + +/// Read a binary P6 PPM and return its `FRAME_WIDTH*FRAME_HEIGHT*3` RGB +/// payload (caller owns it). Validates the magic and that the payload size +/// matches the expected frame dimensions. +fn readPpm(allocator: std.mem.Allocator, io: std.Io, path: []const u8) ![]u8 { + var file = try std.Io.Dir.cwd().openFile(io, path, .{}); + defer file.close(io); + + const stat = try file.stat(io); + const bytes = try allocator.alloc(u8, stat.size); + errdefer allocator.free(bytes); + var read_buf: [4096]u8 = undefined; + var reader = file.reader(io, &read_buf); + try reader.interface.readSliceAll(bytes); + + // Parse the P6 header: "P6\n \n255\n". + if (bytes.len < 11) return error.InvalidHeader; + if (!std.mem.startsWith(u8, bytes, "P6\n")) return error.UnsupportedFormat; + + var cursor: usize = 3; + while (cursor < bytes.len and bytes[cursor] != '\n') cursor += 1; + if (cursor >= bytes.len) return error.InvalidHeader; + cursor += 1; + while (cursor < bytes.len and bytes[cursor] != '\n') cursor += 1; + if (cursor >= bytes.len) return error.InvalidHeader; + cursor += 1; + + const pixel_bytes = bytes.len - cursor; + const expected: usize = @as(usize, FRAME_WIDTH) * FRAME_HEIGHT * 3; + if (pixel_bytes != expected) return error.SizeMismatch; + + const out = try allocator.alloc(u8, expected); + @memcpy(out, bytes[cursor..]); + allocator.free(bytes); + return out; +} + +fn psnrDb(a: []const u8, b: []const u8) f64 { + std.debug.assert(a.len == b.len); + var sse: u128 = 0; + for (a, b) |x, y| { + const dx: i32 = @as(i32, x) - @as(i32, y); + sse += @intCast(dx * dx); + } + if (sse == 0) return std.math.inf(f64); + const mse: f64 = @as(f64, @floatFromInt(sse)) / @as(f64, @floatFromInt(a.len)); + return 20.0 * std.math.log10(255.0 / @sqrt(mse)); +} + +test "ppm psnr gate: captured smoke frame matches golden within 40 dB" { + const io = std.testing.io; + if (!fileExists(io, CAPTURED_PATH)) return error.SkipZigTest; + if (!fileExists(io, GOLDEN_PATH)) return error.SkipZigTest; + + const allocator = std.testing.allocator; + const captured = readPpm(allocator, io, CAPTURED_PATH) catch |e| { + std.log.warn("ppm-psnr: failed to read {s}: {t}", .{ CAPTURED_PATH, e }); + return error.SkipZigTest; + }; + defer allocator.free(captured); + + const golden = try readPpm(allocator, io, GOLDEN_PATH); + defer allocator.free(golden); + + const psnr = psnrDb(captured, golden); + std.log.info("ppm-psnr vs golden: {d:.2} dB (gate {d:.2})", .{ psnr, PSNR_GATE_DB }); + try std.testing.expect(psnr >= PSNR_GATE_DB); +} + +test "psnrDb: identical inputs report infinity" { + const a = [_]u8{ 1, 2, 3 }; + try std.testing.expect(std.math.isInf(psnrDb(&a, &a))); +} + +test "psnrDb: 1-unit average error sits around 48 dB" { + var a: [3072]u8 = undefined; + var b: [3072]u8 = undefined; + for (&a) |*v| v.* = 128; + for (&b) |*v| v.* = 129; + const psnr = psnrDb(&a, &b); + try std.testing.expect(psnr > 46.0); + try std.testing.expect(psnr < 50.0); +} diff --git a/tests/render/render_graph_topo.zig b/tests/render/render_graph_topo.zig index fe43400..110c18a 100644 --- a/tests/render/render_graph_topo.zig +++ b/tests/render/render_graph_topo.zig @@ -114,3 +114,49 @@ test "graph detects cycle and returns error" { }); try std.testing.expectError(error.RenderGraphCycle, g.compile()); } + +test "graph WAW two writers same resource: ordered by insertion, not a cycle" { + // M0.5 item 7 (latent bug repro): two passes writing the SAME resource + // with no RAW relation between them must be serialized by insertion order + // (lower index first), NOT reported as error.RenderGraphCycle. Guards the + // WAW-symmetry bug in `passDependsOn` (edges in both directions → false + // cycle). RED before the fix, green after. + var g = Graph.init(std.testing.allocator); + defer g.deinit(); + + const t = TextureHandle{ .inner = 42 }; + const writer_a = try g.addPass(.{ + .name = "writer_a", + .body = noopBody, + .writes = &.{.{ + .resource = .{ .texture = t }, + .stage = .{ .fragment = true }, + .access = .{ .write = true, .color_attachment = true }, + .layout = .color_attachment, + }}, + }); + const writer_b = try g.addPass(.{ + .name = "writer_b", + .body = noopBody, + .writes = &.{.{ + .resource = .{ .texture = t }, + .stage = .{ .fragment = true }, + .access = .{ .write = true, .color_attachment = true }, + .layout = .color_attachment, + }}, + }); + + // Must compile without a false cycle. + try g.compile(); + try std.testing.expectEqual(@as(usize, 2), g.execution_order.items.len); + + // Insertion-order tiebreak: writer_a (added first) precedes writer_b. + var pos_a: ?usize = null; + var pos_b: ?usize = null; + for (g.execution_order.items, 0..) |pi, i| { + if (pi == writer_a) pos_a = i; + if (pi == writer_b) pos_b = i; + } + try std.testing.expect(pos_a != null and pos_b != null); + try std.testing.expect(pos_a.? < pos_b.?); +} diff --git a/tools/weld_lint/rules/no_device_dispatch_outside_gal.zig b/tools/weld_lint/rules/no_device_dispatch_outside_gal.zig index 2cf22a2..b894f89 100644 --- a/tools/weld_lint/rules/no_device_dispatch_outside_gal.zig +++ b/tools/weld_lint/rules/no_device_dispatch_outside_gal.zig @@ -23,17 +23,19 @@ const name = "no_device_dispatch_outside_gal"; const allowed_prefix_posix = "src/modules/render/gal/vulkan/"; const allowed_prefix_win = "src\\modules\\render\\gal\\vulkan\\"; -/// Opt-in marker for the legacy S6 files that call `device_dispatch` -/// before the GAL Phase 1+ migration. Present at the head of the file in -/// the form: -/// //! WELD_LEGACY_VK_DISPATCH — pre-M0.4 code, migration tracked Phase 1+ +/// The legacy `WELD_LEGACY_VK_DISPATCH` grandfather marker. M0.5 (item 4) +/// migrated the last `device_dispatch` site (`src/editor/vk_blit.zig`) onto +/// the idiomatic `vk.*` wrappers, so the marker is now FORBIDDEN: its presence +/// in a file header is itself a lint error — no file may opt out of the +/// dispatch-discipline rule any more. const legacy_marker = "WELD_LEGACY_VK_DISPATCH"; -/// Hook called by `main.runLint` once per `.zig` file. Skip immediately -/// if the file lives under `gal/vulkan/` (legitimate case) or carries the -/// `WELD_LEGACY_VK_DISPATCH` marker (S6 grandfather case). Otherwise, scan the -/// source for the `vk.device_dispatch` pattern and emit one diagnostic per -/// occurrence. +/// Hook called by `main.runLint` once per `.zig` file. Files under +/// `gal/vulkan/` are the legitimate dispatch site and are skipped entirely. +/// For every other file: emit a diagnostic if the now-forbidden +/// `WELD_LEGACY_VK_DISPATCH` grandfather marker is present (M0.5 — no opt-out), +/// then scan the source for the `vk.device_dispatch` pattern and emit one +/// diagnostic per occurrence. pub fn check( arena: std.mem.Allocator, file: []const u8, @@ -45,7 +47,20 @@ pub fn check( // on both separators (POSIX `/`, Win32 `\`). if (std.mem.indexOf(u8, file, allowed_prefix_posix) != null) return; if (std.mem.indexOf(u8, file, allowed_prefix_win) != null) return; - if (hasLegacyMarker(source)) return; + + // M0.5 (item 4): the `WELD_LEGACY_VK_DISPATCH` grandfather escape is + // removed. The marker is forbidden outright — flag its presence, then keep + // scanning so any `device_dispatch` access in the same file is also caught. + if (legacyMarkerOffset(source)) |off| { + const pos = diag.lineColFromOffset(source, off); + try out.append(arena, .{ + .file = file, + .line = pos.line, + .col = pos.col, + .rule = name, + .message = "`WELD_LEGACY_VK_DISPATCH` marker is forbidden — the device-dispatch grandfather escape was removed in M0.5; route through the idiomatic `vk.*` wrappers (or the GAL) instead", + }); + } var tokenizer = std.zig.Tokenizer.init(source); var last_ident_was_vk: bool = false; @@ -80,17 +95,20 @@ pub fn check( } } -/// Checks whether the source carries the legacy marker at the head (scans the -/// first 8 lines to tolerate an introductory block comment). -fn hasLegacyMarker(source: []const u8) bool { +/// Returns the byte offset of the legacy marker if it appears in the file +/// header (scans the first 8 lines to tolerate an introductory block comment), +/// else null. Header-only by design: this matches how the marker was always +/// written (a file-top `//!` line) and keeps this rule file — which names the +/// marker in its own body below line 8 — from flagging itself. +fn legacyMarkerOffset(source: []const u8) ?usize { var line_count: u32 = 0; var start: usize = 0; while (start < source.len and line_count < 8) { const eol = std.mem.indexOfScalarPos(u8, source, start, '\n') orelse source.len; const line = source[start..eol]; - if (std.mem.indexOf(u8, line, legacy_marker) != null) return true; + if (std.mem.indexOf(u8, line, legacy_marker)) |rel| return start + rel; start = eol + 1; line_count += 1; } - return false; + return null; }