Five places where one fact lived twice now state it once (maintainability sweep) #415

Merged

enricobuehler merged 5 commits from worktree-refactor-sweep into main 2026-08-27 16:42:57 +00:00

Conversation 0 Commits 5 Files Changed 17

+673 -211

enricobuehler commented 2026-08-27 16:32:01 +00:00

Owner

Copy Link

Copy Source

A maintainability sweep over the survey's top findings. Each commit is one finding, ordered by pain × ease; every diff either deletes a fork of the same fact or names a growth path that stops minting duplicates.

1. enc/policy.rs — the IR/LTR env knobs parse once (559b1852)
QSV's copies had already diverged twice in ways the file itself confesses (the dropped trim() that left LTR enabled on Intel, the IR-period knob that silently did nothing), and PUNKTFUNK_LTR_FORCE_AT=0 still behaved differently per vendor. rfi.rs's sibling; tuned per-vendor defaults (QSV ¼ s vs AMF ½ s LTR cadence) and API-bound clamps deliberately stay with their backend.

2. ABI v26 — the connect ladder gets a last rung that never mints another symbol (9c164aa6)
Eleven generations of punktfunk_connect_ex* each added a field by minting a new exported symbol plus a 20-something-parameter forwarding shim (ex11 over ex10: two fields). punktfunk_connect_opts takes every option in one size-prefixed PunktfunkConnectOpts — PunktfunkConfig's struct_size guard with the growth direction added: an older, shorter caller gets its tail defaulted to unspecified/auto. Layout locked padding-free on both pointer widths (96/68 B) by const asserts and by the C harness compiling the same sizes. Every ex keeps its symbol and behaviour; the embedding guide's ladder table (stalled at ex7 — the maintenance tax on display) now closes the chain.
Also root-caused in passing: the cargo-test C harness never linked on an Apple-Silicon dev box (-L/opt/homebrew/lib was missing) and silently reused a stale featureless .a (ensure_staticlib now builds --features quic unconditionally).

3. control::Task — the control task's 31 positional arguments become named fields (c53dd153)
Five mpsc senders and six receivers in a row, two both carrying bare u32s (retarget_rx / gap_rx) — one silent transposition at the spawn site away from a runtime puzzle. Destructured at the top into same-named locals, so the 500-line body is byte-for-byte unchanged. Drops one of the workspace's 127 too_many_arguments allows.

4. composite_plan() — bring-up and the compositor retarget derive the cursor-composite pair once (92f80bdf)
The two sites had drifted — the defect class virtual_stream's size invites, found live: bring-up keyed gamescope_composite on the compositor alone, the capture-loss retarget on plan.gamescope_cursor, which also folds in the gamescope_composites_cursor() capability. On a gamescope that paints its own pointer, bring-up planned a host composite the XFixes reader never feeds. One derivation now serves both — settle_portal_cursor's discipline applied to the pair session_plan.rs already documents as must-agree.

5. pf_gpu::render_node_env() — the house DRM-node knob parses once (6092f80d)
Three independent readers disagreed on trim and empty-string handling; the remote-worker handshake forwarded a padded/empty value across the process boundary verbatim. The PyroWave sites keep the env-only form deliberately — their device-selection oracle rules forbid the manual-preference layer.

Verified: cargo fmt --all --check; mac cargo test -p punktfunk-core --features quic (502 lib tests + C harness + loopback, all green — the harness now actually runs on a Mac); punktfunk-rust-ci docker clippy -D warnings, green and non-vacuous for punktfunk-host (default features), pf-encode (with and without pyrowave), pf-gpu.

Caveats for review

• amf.rs/qsv.rs are Windows-target-only and .133 was unreachable; those two diffs are re-read-verified — this PR's CI is their compile gate.
• Deliberately not touched: PUNKTFUNK_FEC_PCT still diverges with teeth — the native plane clamps ≤ 90 and trims, gamestream/stream.rs does neither (default 20, no clamp). Whether 90 is the right cap for the fresh WP2.1 wire-budget code is a domain call for a human.
• The virtual_stream decomposition (2,912 lines; a 2,000-line main loop over ~60 shared mutable locals) and the cross-client settings-option schema remain programs, not commits — seams mapped for both.

A maintainability sweep over the survey's top findings. Each commit is one finding, ordered by pain × ease; every diff either deletes a fork of the same fact or names a growth path that stops minting duplicates. **1. `enc/policy.rs` — the IR/LTR env knobs parse once** (`559b1852`) QSV's copies had already diverged twice in ways the file itself confesses (the dropped `trim()` that left LTR enabled on Intel, the IR-period knob that silently did nothing), and `PUNKTFUNK_LTR_FORCE_AT=0` still behaved differently per vendor. `rfi.rs`'s sibling; tuned per-vendor defaults (QSV ¼ s vs AMF ½ s LTR cadence) and API-bound clamps deliberately stay with their backend. **2. ABI v26 — the connect ladder gets a last rung that never mints another symbol** (`9c164aa6`) Eleven generations of `punktfunk_connect_ex*` each added a field by minting a new exported symbol plus a 20-something-parameter forwarding shim (ex11 over ex10: two fields). `punktfunk_connect_opts` takes every option in one size-prefixed `PunktfunkConnectOpts` — `PunktfunkConfig`'s `struct_size` guard with the growth direction added: an older, shorter caller gets its tail defaulted to unspecified/auto. Layout locked padding-free on both pointer widths (96/68 B) by const asserts **and** by the C harness compiling the same sizes. Every `ex` keeps its symbol and behaviour; the embedding guide's ladder table (stalled at ex7 — the maintenance tax on display) now closes the chain. Also root-caused in passing: the cargo-test C harness never *linked* on an Apple-Silicon dev box (`-L/opt/homebrew/lib` was missing) and silently reused a stale featureless `.a` (`ensure_staticlib` now builds `--features quic` unconditionally). **3. `control::Task` — the control task's 31 positional arguments become named fields** (`c53dd153`) Five mpsc senders and six receivers in a row, two both carrying bare `u32`s (`retarget_rx` / `gap_rx`) — one silent transposition at the spawn site away from a runtime puzzle. Destructured at the top into same-named locals, so the 500-line body is byte-for-byte unchanged. Drops one of the workspace's 127 `too_many_arguments` allows. **4. `composite_plan()` — bring-up and the compositor retarget derive the cursor-composite pair once** (`92f80bdf`) The two sites had drifted — the defect class `virtual_stream`'s size invites, found live: bring-up keyed `gamescope_composite` on the compositor alone, the capture-loss retarget on `plan.gamescope_cursor`, which also folds in the `gamescope_composites_cursor()` capability. On a gamescope that paints its own pointer, bring-up planned a host composite the XFixes reader never feeds. One derivation now serves both — `settle_portal_cursor`'s discipline applied to the pair `session_plan.rs` already documents as must-agree. **5. `pf_gpu::render_node_env()` — the house DRM-node knob parses once** (`6092f80d`) Three independent readers disagreed on trim and empty-string handling; the remote-worker handshake forwarded a padded/empty value across the process boundary verbatim. The PyroWave sites keep the env-only form deliberately — their device-selection oracle rules forbid the manual-preference layer. **Verified**: `cargo fmt --all --check`; mac `cargo test -p punktfunk-core --features quic` (502 lib tests + C harness + loopback, all green — the harness now actually runs on a Mac); `punktfunk-rust-ci` docker clippy `-D warnings`, green and non-vacuous for `punktfunk-host` (default features), `pf-encode` (with and without `pyrowave`), `pf-gpu`. **Caveats for review** - `amf.rs`/`qsv.rs` are Windows-target-only and `.133` was unreachable; those two diffs are re-read-verified — this PR's CI is their compile gate. - Deliberately **not** touched: `PUNKTFUNK_FEC_PCT` still diverges with teeth — the native plane clamps ≤ 90 and trims, `gamestream/stream.rs` does neither (default 20, no clamp). Whether 90 is the right cap for the fresh WP2.1 wire-budget code is a domain call for a human. - The `virtual_stream` decomposition (2,912 lines; a 2,000-line main loop over ~60 shared mutable locals) and the cross-client settings-option schema remain **programs**, not commits — seams mapped for both.

enricobuehler added 5 commits 2026-08-27 16:32:05 +00:00

Three hand-copies of the same env knob parse once (encoder IR/LTR policy) ... 559b185247

QSV's copies had already diverged twice in ways the file itself confesses
(the dropped trim() that left LTR enabled on Intel, the IR period knob that
silently did nothing) — and PUNKTFUNK_LTR_FORCE_AT=0 still behaved
differently per vendor. enc/policy.rs now parses each knob once, rfi.rs's
sibling; tuned per-vendor defaults (QSV 1/4 s vs AMF 1/2 s LTR cadence) and
API-bound clamps deliberately stay with their backend.

The connect ladder gets a last rung that never mints another symbol (ABI v26) ... 9c164aa614

Eleven generations of punktfunk_connect_ex* each added a field by minting a
new exported symbol plus a 20-something-parameter forwarding shim (ex11 over
ex10: two fields). punktfunk_connect_opts takes every option in ONE
size-prefixed PunktfunkConnectOpts — PunktfunkConfig's struct_size guard,
with the growth direction added: an older, shorter caller gets its tail
defaulted to unspecified/auto instead of misread. Layout is locked
padding-free on both pointer widths (96/68 B) by const asserts AND by the C
harness compiling the same sizes, so an appended field can never land in
bytes an older sizeof already covered. Every ex keeps its symbol and
behaviour; the chain is closed in the embedding guide (whose ladder table
had stalled at ex7 — the maintenance tax on display).

Also: the cargo-test C harness now actually links on an Apple Silicon dev
box (-L/opt/homebrew/lib was missing) and always builds the staticlib WITH
quic — a featureless .a left by an earlier plain build was silently reused.

The control task's 31 positional arguments become a named-field Task ... c53dd15313

Five mpsc senders and six receivers in a row, two of them both carrying
bare u32s (retarget_rx / gap_rx) — one silent transposition at the spawn
site away from a runtime puzzle. control::run now takes control::Task,
destructured at the top into the same-named locals, so the 500-line body
is byte-for-byte unchanged and the spawn site reads as named fields.
Drops one of the workspace's 127 too_many_arguments allows.

Bring-up and the compositor retarget derive the cursor-composite pair once ... 92f80bdfde

The two sites had drifted — the class of defect virtual_stream's size
invites. Bring-up keyed gamescope_composite on the compositor alone; the
capture-loss retarget read plan.gamescope_cursor, which also folds in the
gamescope_composites_cursor() capability (a gamescope that paints the
pointer into its own node). On such a node, bring-up planned a host
composite the XFixes reader never feeds. composite_plan() is now THE
derivation for both, settle_portal_cursor's discipline applied to the
pair session_plan.rs already documents as must-agree.

The house DRM-node knob is parsed once, in the crate that owns GPU choice ... 6092f80df2

PUNKTFUNK_RENDER_NODE had three independent readers disagreeing on trim
and empty-string handling (pf-gpu: no trim; pyrowave: trim+filter; the
remote worker handshake: raw .ok(), so an empty or padded value crossed
the process boundary verbatim). pf_gpu::render_node_env() is now the one
parse; linux_render_node layers the console's manual GPU preference on
top, and the PyroWave sites keep reading the env-only form deliberately —
their device-selection oracle rules forbid the preference layer.

enricobuehler merged commit f4f318b8f4 into main 2026-08-27 16:42:57 +00:00

enricobuehler deleted branch worktree-refactor-sweep 2026-08-27 16:43:03 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

FernandoMX FlipperDan HitFrostbite bluelightspecial enricobuehler (Enrico Bühler) kszlim luxus nannku neoney

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unom/punktfunk#415