mirror of
https://github.com/nestriness/nestri.git
synced 2026-09-26 12:32:25 +03:00
ecebc528ae9f2d10818d231d4a5ce85e374845f7
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
e70f05e245 |
fix(nescope): make HDR reachable — start XWayland, advertise the opaque FourCCs (#314)
Three related fixes. Together they take HDR from unreachable to working
end to end on the XWayland path.
## XWayland was never started
Three lines had been commented out since the initial import: the call
that spawns XWayland, the guard that waits for it, and the `DISPLAY`
handed to the child. Every game therefore launched as a native Wayland
client. Nothing reported it -- the compositor still logged the X display
it was telling clients to point at, which is why it read as working.
That is also why HDR never fired. The colour space is signalled over a
protocol whose Vulkan layer lives in the game process and finds the
compositor through the X11 root window, so the one path able to carry it
was the one path no game was on. `ENABLE_GAMESCOPE_WSI` and `DXVK_HDR`
were already being set, which switched that layer on and then handed it
a
display it could not use.
Restoring the guard also fixes the ordering it was written for: the
launch
now happens after XWayland reports ready rather than ~40ms before it.
## Mesa was dropping every format we advertised alpha-only
Mesa tracks two flags per VkFormat -- one contributed by a format alpha
FourCC, one by its opaque FourCC -- and skips any format carrying only
one:
```c
if (!(disp_fmt->flags & WSI_WL_FMT_ALPHA) ||
!(disp_fmt->flags & WSI_WL_FMT_OPAQUE))
continue;
```
We advertised `ARGB8888` and `XRGB8888`, so `B8G8R8A8` survived and made
the list look like it was working. Everything else was alpha-only and
was
dropped in silence -- `ABGR8888` had been advertised all along while
`R8G8B8A8` never once appeared on a surface. Adding the opaque spellings
takes the surface from 6 formats to 21 and restores the three that carry
HDR.
The comment above that list claimed it was for XWayland DRI3 and that a
game swapchain format was independent of it. It was the opposite: the
list decides what a game can select, and deleting an entry removes that
format from every client.
## Verified against swapchains, not format lists
A client asking for `A2B10G10R10` + `HDR10_ST2084` now gets a swapchain
and the compositor is told colorspace `1000104008`; one asking for
`R16G16B16A16_SFLOAT` + scRGB linear gets `1000104002`. Previously both
were refused at creation -- the WSI layer re-checks the requested format
against the driver own surface list, so the colour space and the pixel
format arrive from two different places and only one was being supplied.
`apps/nescope/scripts/verify-hdr-formats.sh` asks what a client is
offered
from inside a child process, keeping the XCB and Wayland surfaces apart
since a game presents through the XCB one. The default mode guards both
halves of what the compositor controls; `--expect-layer` states the full
target and passes once a WSI layer is present. No new dependencies
(`vulkaninfo` + `python3`).
## Still open
HDR is XWayland-only, documented as a FIXME in `hdr.rs`. A WSI layer
binds
the swapchain factory on its own Wayland connection while a native
client
surface lives on the client one, and object IDs do not cross
connections.
The FIXME records the fix both reference implementations point at, and
the
trap to avoid when we take it: gating format injection on "the
compositor
supports HDR" rather than on being able to signal the surface hands a
client PQ pixels that arrive tagged as SDR, with nothing reporting an
error.
nescope ships no WSI layer of its own; the above was verified with the
stock gamescope one, which drives our protocol unmodified.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR makes HDR-capable native Wayland presentation reachable, adds
the alpha/opaque DMA-BUF FourCC pairs Mesa requires, makes XWayland an
explicit compatibility mode, and adds an HDR surface-format diagnostic.
- Starts XWayland only with `--xwayland`, waits for readiness before
launching the child, and stops cleanly if startup fails or times out.
- Routes Proton through Wayland unconditionally while retaining
`DXVK_HDR` as an HDR-specific setting.
- Advertises paired alpha and opaque FourCC variants with portable
modifiers.
- Separates XCB and Wayland probe results, selects one hardware adapter,
and distinguishes probe failures from format regressions.
- Documents the limitations of the legacy gamescope WSI path and the
external-layer dependency.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge; no outstanding correctness, security, or
repository-rule issue remains.
All previous findings are resolved in the current code, including the
XWayland failure lifecycle, removal of unsupported vendor modifiers,
corrected HDR documentation, reliable diagnostic failure handling,
per-GPU format selection, and unconditional Proton Wayland routing. The
changes since the previous review preserve diagnostic output handling
without introducing a new failure.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescope/src/main.rs | Adds opt-in XWayland lifecycle handling,
readiness timeout, conditional DISPLAY propagation, and unconditional
Proton Wayland routing; the previous launch-environment finding is
fixed. |
| apps/nescope/src/state.rs | Stops the event loop on reported XWayland
startup failure and advertises portable paired alpha/opaque DMA-BUF
formats without vendor-specific modifiers. |
| apps/nescope/src/hdr.rs | Documents the working native Wayland HDR
path and accurately distinguishes it from the external, deliberately
disabled gamescope WSI route. |
| apps/nescope/scripts/verify-hdr-formats.sh | Adds a diagnostic that
keeps GPU and surface paths separate and now preserves the intended exit
behavior when filtered Vulkan diagnostics contain no matching lines. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Launch[nescope child launch] --> Mode{--xwayland?}
Mode -->|No| Wayland[Native Wayland surface]
Mode -->|Yes| Wait[Start and await XWayland]
Wait -->|Ready| XCB[XCB / XWayland surface]
Wait -->|Error or 10s timeout| Stop[Log failure and stop]
Wayland --> Formats[Paired alpha and opaque FourCCs]
Formats --> HDR[HDR10 and scRGB formats available]
XCB --> SDR[X11 compatibility path without native HDR]
Proton[Proton child] -->|PROTON_ENABLE_WAYLAND=1| Wayland
```
<sub>Reviews (9): Last reviewed commit: ["nescope/scripts: guard the
diagnostic
pi..."](https://github.com/nestrilabs/nestri/commit/f8bdd68f87b3e29e4195f325b3c98800076a55d5)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=60368324)</sub>
**Context used:**
- Knowledge Base — [Compositor, display, and input
control](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/compositor-input.md)
- Knowledge Base — [Streaming host
runtime](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/streaming-runtime.md)
<!-- /greptile_comment -->
|
||
|
|
200bc9c75f |
fix(nescapture): stop defaulting unreadable formats to BGRA (#315)
Stacked on #313, which this depends on. Found while checking whether the 10-bit HDR path actually works now that a client can obtain an HDR swapchain (see #314). **It does** — verified end to end rather than from the format list: a client requesting `A2B10G10R10` + `HDR10_ST2084` produces ``` pix_fmt=yuv420p10le color_range=pc color_space=bt2020nc color_transfer=smpte2084 color_primaries=bt2020 ``` which is a correctly tagged HDR10 stream, and the first pixel-level HDR check here with 10-bit rather than 8-bit input. Three ways it could have gone wrong instead. ## Unrecognised formats defaulted to BGRA `vk_format_to_input_format` returned `BGRA` for anything it did not know, which reads a packed 10-bit or FP16 buffer as eight-bit channels. It now returns `None`, and the encode loop drops those frames with one log line per format. A stalled stream is a complaint. A stream at full frame rate carrying nonsense is not, and that is the failure this area keeps producing. ## Bit depth and input format had drifted apart They were two separate matches on the same `VkFormat`. `A2R10G10B10` counted as ten-bit in one and had no entry in the other, so it fell back to eight-bit BGRA — the encoder configured for ten bits while the converter read eight. Depth now derives from the input format, so that disagreement is unrepresentable. `A2R10G10B10` stays unmapped deliberately: a WSI layer offers it as one of its HDR pairs and the compositor dmabuf list advertises it, but the converter has no red-first 10-bit input, so there is nothing correct to map it to. ## The CPU fallback could not read either HDR format It read four bytes per pixel for every format and encoded eight-bit regardless, so a packed 10-bit buffer became garbage and an FP16 one was half an image of misread floats. It now refuses what it cannot read. ## Recorded, not fixed: the colour space we see is not always the one requested A FIXME at the point the value is read. A WSI layer rewrites `imageColorSpace` to `SRGB_NONLINEAR` before calling down — deliberately, since it carries the real colour space to the compositor out of band. We sit below it, so we read the rewrite. Measured, all three lines from one run: ``` [Gamescope WSI] ... colorspace: VK_COLOR_SPACE_HDR10_ST2084_EXT swapchain created — format=A2B10G10R10 colorspace=SRGB_NONLINEAR (re)init encoder: H265 Yuv420 Ten Bt709 → P010 ``` Ten-bit right, BT.709 wrong: PQ samples encoded and tagged as SDR. The same client *without* the layer gives `Ten Bt2020` and an smpte2084 stream, so this is specific to the layer path — which is the path Proton titles take. The fix cannot be local; the true colour space only exists in the compositor, which does receive it, so it needs a channel from there. Layer ordering is not a fix — we do not control it, and the non-layer path still needs the Vulkan value. Left out of this PR as a design change rather than a bug fix. ## Verification - 4 new tests, 12 total, all passing. - No new clippy warnings (diffed against the base branch). - 10-bit HDR path: unchanged, still `Ten Bt2020` / smpte2084. - SDR path: `verify-chain.sh` passes, 835 frames, 8-bit BGRA, brightness agreement 0.57. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR makes Vulkan format handling fail safely instead of interpreting unsupported swapchain buffers as BGRA. - Maps supported Vulkan formats to explicit converter inputs and derives bit depth from that mapping. - Drops unsupported GPU frames with rate-limited logging. - Rejects unsupported HDR formats in the eight-bit CPU fallback. - Documents the color-space limitation caused by rewritten WSI metadata. - Adds tests covering unsupported, eight-bit, 10-bit, and FP16 formats. <h3>Confidence Score: 5/5</h3> The PR appears safe to merge, with no new actionable issues introduced since the previous review. The changes since the previous review are empty, the sole previous finding was manually resolved after Greptile conceded it based on the stacked PR dependency, and the full PR introduces no confirmed rule violations or remaining correctness failures. <h3>Important Files Changed</h3> | Filename | Overview | |----------|----------| | apps/nescapture/src/encode.rs | Replaces unsafe BGRA fallback behavior with explicit format validation, consistent bit-depth derivation, guarded CPU fallback, and focused tests. | | apps/nescapture/src/swapchain.rs | Documents the known WSI color-space rewrite limitation at the point where swapchain metadata is recorded. | <h3>Flowchart</h3> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[Captured Vulkan frame] --> B{Known converter input?} B -->|No| C[Log format change and drop frame] B -->|Yes| D[Derive input format and bit depth] D --> E{DMA-BUF path available?} E -->|Yes| F[GPU color conversion and encoding] E -->|No| G{Eight-bit RGBA or BGRA?} G -->|Yes| H[CPU conversion and encoding] G -->|No| I[Return recoverable error] ``` <sub>Reviews (3): Last reviewed commit: ["docs(nescapture): the colour-space note ..."](https://github.com/nestrilabs/nestri/commit/7b05908e0a94739aa7ee5bd7fe5764a0a2b95071) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=60377562)</sub> **Context used:** - Knowledge Base — [Vulkan capture layer](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/capture-layer.md) <!-- /greptile_comment --> |
||
|
|
3389e6065f |
fix(nescapture): tag encoded streams full-range to match the samples written (#313)
## The bug `nescapture` sets the colour converter full-range unconditionally, but the video usability information carried pixelforge's **default limited-range flag**. A compliant decoder then expanded 16–235 out of samples that already covered 0–255 — darkening midtones and clipping both ends. pixelforge keeps two separate flags for this, one on the converter and one on the colour description, and its own documentation says they must agree. Only the first was being set. The two lines are about forty apart, each is correct on its own, and the comment above the second states the right intent while the call below it does the opposite: ```rust // GPU framebuffer captures are always full-range — use BT.709 full-range // so the decoder doesn't apply limited-range expansion. enc_cfg = enc_cfg.with_color_description(ColorDescription::bt709()); // ^ this constructor is limited-range ``` ## Evidence Measured on a Radeon RX 9060 XT, comparing the encoded result against the compositor's own readback of the same frames: | ground truth = `51` | before | after | |---|---|---| | flat background, decoded | **`38`** | `49–51` | | mean luma, capture path vs readback | **10.41 apart** | **0.55 apart** | | luma histogram intersection | **0.090** | **0.913** | | declared `color_range` | `tv` | `pc` | **The encoded luma is byte-identical before and after** — `Y = 51.00`, standard deviation `0.00` on both runs. Only the tag changed, which is what identifies this as a signalling bug rather than a conversion one, and why nothing short of a comparison against ground truth could see it: the stream was valid, the frame rate was right, the picture was recognisable, and every liveness check passed. The HDR arm (`bt2020_pq`) carried the same defect and is fixed the same way, but **has not been run** — no 10-bit verification here. ## `scripts/verify-chain.sh` Runs a Vulkan workload under `nescope` with the layer active and compares the encoded output against `nescope-shot`'s readback of the same frames. Two paths that share almost no code see the same content, so disagreement localises the fault; a single path cannot tell a correct frame from a plausible-looking wrong one. **Confirmed it fails when this change is reverted** — both the tag check and the brightness-agreement check fire. One note on its thresholds, since it is easy to get backwards: the not-blank check is a low absolute floor plus a comparison against the readback's own structure, rather than a fixed number. A fixed number was tried first and was wrong in the worst way — the **broken** build scored 20.49 on it and the **fixed** build 17.74, because the range defect stretched contrast and that reads as more detail. How much structure a correct frame carries depends on what the workload drew, so the only stable reference is ground truth measured in the same run. ## Not covered `vkcube` rather than a real workload; 720p, H.264, 8-bit; one card, one driver. XWayland, HUD detection and real swapchain formats are untouched. <!-- greptile_comment --> <h3>Greptile Summary</h3> The PR aligns encoded-stream color metadata with the full-range samples produced by nescapture and updates the CPU fallback to BT.709 full-range conversion. - Updates pixelforge and configures matching converter color space, range, and SDR reference white. - Corrects Vulkan color-space mapping and adds regression tests for SDR, HDR, and CPU fallback behavior. - Adds SDR capture-chain and HDR comparison verification scripts. <h3>Confidence Score: 5/5</h3> The PR appears safe to merge. No blocking failure remains. <h3>Important Files Changed</h3> | Filename | Overview | |----------|----------| | apps/nescapture/src/encode.rs | Aligns GPU and CPU conversion output with encoded color metadata and adds focused regression coverage. | | apps/nescapture/scripts/verify-chain.sh | Adds an end-to-end SDR verifier using a static corner patch to avoid the previously reported temporal mismatch. | | apps/nescapture/scripts/verify-hdr.sh | Adds an HDR comparison harness for inspecting conversion behavior across builds. | | apps/nescapture/Cargo.toml | Advances pixelforge to the revision providing the required color-conversion configuration. | | Cargo.lock | Records the pixelforge update and resulting transitive dependency refresh. | <sub>Reviews (5): Last reviewed commit: ["test(nescapture): check the HDR conversi..."](https://github.com/nestrilabs/nestri/commit/2f9773c4b7ded7774b9396648de26423afe926fa) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=60147076)</sub> **Context used:** - Knowledge Base — [Vulkan capture layer](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/capture-layer.md) <!-- /greptile_comment --> --------- Co-authored-by: DatCaptainHorse <DatCaptainHorse@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
270304bca5 |
feat(build): borealis-style multi-stage rootfs for the open guest components (#309)
## What Adds `build/` — a Dockerfile with `mesa-build`, `nestri-build`, `os-base`, `runtime`, `runtime_prod` and `runtime_debug` stages, plus the `etc/` overlay, `mkimage.sh` and a `Makefile` — laid out the way [borealis](https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/main/project-borealis) lays out its own `build/`. This moves the guest rootfs formula into this repo, targeting the four open guest components already here: `nescope`, `neshub`, `neswire`, `nescapture`. ## Two structural properties worth calling out - **No privileged host chroot.** A bare `chroot` into a hand-extracted rootfs needs `/proc`, `/sys`, `/dev` bind-mounted in first. `os-base` here is `FROM artixlinux/artixlinux:base-openrc` directly with `pacman -S` as plain `RUN` steps — a Docker build step already has its own `/proc`/`/sys`/`/dev`. - **No host-side ownership bug to guard against.** `COPY --from=` runs as root inside the build with no invoking-user uid in the loop. ## Scope boundary **Deliberately excludes Proton and Valve's `steamclient.so`** — both closed, and `CLAUDE.md` forbids closed content in this repo. `runtime_prod`, tagged `nestrilabs/nestri:base`, is a complete, bootable, Steam-less image — and also the shared foundation other builds start from. Whatever layers Proton/Steam on top of it is a closed build outside this repo, by design. ## Known gap Nothing starts a payload yet — `nesinit` isn't open code — so `/etc/init.d/nescope` boots it in plain-compositor mode (no command after `--`) rather than running a game. Real and testable, just not a full session yet. Details in `build/README.md`. ## Status Built and tagged locally as `nestrilabs/nestri:base` (podman, no `--no-cache` issues, greptile's three findings all fixed and verified against a real build). Not yet packed into a disk image or run inside nesbox. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> The PR adds a multi-stage Artix/OpenRC guest-rootfs build for the open Nestri components, with production and debug image flavors. - Builds patched Mesa and the Rust workspace in dedicated builder stages. - Assembles and configures the bootable guest environment and OpenRC services. - Packs a selected container image into an ext4 root filesystem while retaining rootless container storage access. <h3>Confidence Score: 5/5</h3> The PR appears safe to merge. No blocking failure remains. <h3>Important Files Changed</h3> | Filename | Overview | |----------|----------| | build/Dockerfile | Defines the complete multi-stage build, overlays repository-root-relative configuration paths, and creates production and debug runtime targets. | | build/Makefile | Provides consistent image build and packing targets using a repository-root context and matching image tags. | | build/scripts/mkimage.sh | Keeps container-runtime operations in the invoking user's storage while escalating only filesystem creation and mounting operations. | | build/etc/conf.d/nestri-user-env | Supplies the shared service environment and export function required by the OpenRC service scripts. | | build/etc/init.d/guest-net | Configures optional guest networking from kernel parameters or stable defaults. | | build/etc/init.d/neswire | Starts the audio sink after its dependencies and pins it as the PipeWire default once the graph is ready. | <h3>Flowchart</h3> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[Arch builder] --> B[Mesa build] A --> C[Nestri workspace build] D[Artix OpenRC base] --> E[Common runtime] B --> E C --> E F[build/etc overlay] --> E E --> G[runtime_prod] E --> H[runtime_debug] G --> I[Container image] H --> J[Debug container image] I --> K[mkimage.sh] J --> K K --> L[ext4 rootfs] ``` <sub>Reviews (6): Last reviewed commit: ["refactor(build): rename the published im..."](https://github.com/nestrilabs/nestri/commit/6fecc8cc31bc425db952193074d4b9b0f861de89) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=58981739)</sub> <!-- /greptile_comment --> |