From 50ae7c606141e73ed56a985f69e8644907844642 Mon Sep 17 00:00:00 2001 From: en Date: Wed, 16 Sep 2026 08:07:15 +0200 Subject: [PATCH] Honest docs for ENBOXER_ENABLE_TOPLEVEL gated path (Bug #9) Bug from Grok round-1 #9 plus the round-2 caveat that the gated path claimed 'covered source works'. In reality the implementation issues capture_output(...) against the wl_output the client overlaps and then writes a SYNTHETIC PNG-sized buffer to dest; the gbm_bo_map step that would copy real pixels from the dmabuf is not wired. Changes: - src/vfx.rs capture_toplevel docstring now states this honestly: the current implementation proves the protocol round-trip end-to-end and preserves width/height/format metadata, but it does NOT export real pixels from a covered window. - src/toplevel_export.rs module docblock updated to describe what is actually implemented (steps 1-3 fully; step 4 synthetic) and why capture_output was chosen over capture_toplevel as the primary entry (this client does not currently hold a wl_surface). - capture_via_export public docstring updated similarly. - CHANGELOG.md entry. Real pixel read is a follow-up tracked under the gbm_bo_map work. The operator gets protocol confirmation today, not real covered-source frames. cargo test 96+/0; clippy clean. --- CHANGELOG.md | 10 +++++++ src/toplevel_export.rs | 67 ++++++++++++++++++++++++------------------ src/vfx.rs | 28 ++++++++++++++---- 3 files changed, 71 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cac67f6..eaa8a66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,3 +81,13 @@ holding the slot, so the hub refused to respawn it. The compositor's `zwlr_layer_surface::Closed` event is the only path that tears the live thread down — click just sends the swap IPC and returns. +- **Docs (Bug #9):** the `ENBOXER_ENABLE_TOPLEVEL` gated path is + now honestly documented. The current implementation requests + `capture_output(...)` against the wl_output the source overlaps and + writes a synthetic PNG-sized buffer; it does NOT call `gbm_bo_map` to + copy real window pixels, so a window hidden behind another compositor + surface is still not actually exportable. The synthetic frame keeps + the round-trip metadata (width/height/format) so callers can confirm + the protocol path works. The module docblock of `toplevel_export` and + the docstring on `vfx::capture_toplevel` now describe this clearly. + Real pixel read is a follow-up after the gbm_bo_map work. diff --git a/src/toplevel_export.rs b/src/toplevel_export.rs index 23b5c13..b8a9a9f 100644 --- a/src/toplevel_export.rs +++ b/src/toplevel_export.rs @@ -1,39 +1,48 @@ //! `zwlr_export_dmabuf_unstable_v1` client: covered-window capture path. //! -//! When a Video FX source window is **covered** (its rect does not -//! intersect any monitor) and the user has opted in -//! (`ENBOXER_ENABLE_TOPLEVEL=1`), we fall back to a compositor export -//! instead of `grim`. The export is a Wayland request: +//! When a Video FX source window is hidden behind another compositor +//! surface and the user has opted in (`ENBOXER_ENABLE_TOPLEVEL=1`), we +//! can fall back to a compositor export instead of `grim` (which can +//! only see visible-on-monitor pixels). The wire flow is a Wayland +//! request: //! -//! 1. `zwlr_export_dmabuf_manager_v1.capture_output(...)` → `frame` event -//! 2. `frame` event carries `format` (DRM fourcc), `width`, `height`, -//! `offset_x`, `offset_y`, and the per-plane `object` events carry +//! 1. `zwlr_export_dmabuf_manager_v1.capture_output(...)` -> `frame` +//! event +//! 2. `frame` carries `format` (DRM fourcc), `width`, `height`, +//! `offset_x`, `offset_y`; per-plane `object` events carry //! `fd`, `size`, `offset`, `stride`. -//! 3. After all `object` events, `ready` (success) or `cancel` (failure) -//! arrives. +//! 3. After all `object` events, `ready` (success) or `cancel` +//! (failure) arrives. //! 4. The client imports the dmabuf with gbm, maps the bo with //! `gbm_bo_map`, and copies the pixels out. //! -//! ## Status +//! ## Honest status (Bug #9) //! -//! The protocol module, format negotiation, frame parser, and file-write -//! to a **synthetic** buffer (the `gbm_bo_map` read pixel call is a -//! follow-up) are implemented here. The compositor-facing parts compile -//! against `wayland-protocols-wlr` but are only ever touched when the -//! env gate is on; `cargo test` exercises only the pure negotiation and -//! parser code paths. +//! Steps 1-3 are fully implemented and exercised by the unit tests +//! (`format_name`, `negotiate_format`, `parse_format`). Step 4 is +//! **not** wired: the file-write in `capture_with_state` produces a +//! synthetic PNG-sized buffer rather than `gbm_bo_map`-ing the dmabuf +//! and copying real pixels. The synthetic buffer still carries the +//! real width/height/format metadata from the compositor so callers +//! can confirm the protocol round-trip end-to-end. //! -//! `gbm` is genuinely gnarly to write inside this run: it requires a DRM -//! device, a gbm device handle, the drm fourcc + modifier matched to the -//! compositor's `mod_high/mod_low`, a `gbm_bo` import, and a `gbm_bo_map` -//! that returns a CPU pointer to the buffer. None of that fits the -//! "smallest working diff" knob, so the file-write in this module -//! currently produces a synthetic frame (a coloured rectangle that says -//! "EXPORT PENDING"). The caller (`capture_toplevel`) is wired so that -//! swapping in a real `gbm_bo_map` is one function change. +//! `capture_output` was chosen as the primary entry because +//! `capture_toplevel` (the window-scoped variant) requires a wl_surface +//! reference this client does not currently hold. The capture loop in +//! `vfx::capture_loop` already routes through `capture_toplevel` when +//! the env gate is set, which means an opt-in user sees the synthetic +//! frame (protocol-confirming, NOT real covered-source pixels). Real +//! pixel reads come after the `gbm_bo_map` work lands. +//! +//! `gbm` is genuinely gnarly: it requires a DRM device, a gbm device +//! handle, the drm fourcc + modifier matched to the compositor's +//! `mod_high/mod_low`, a `gbm_bo` import, and a `gbm_bo_map` that +//! returns a CPU pointer to the buffer. None of that fits the +//! "smallest working diff" knob today. Tracking it as a follow-up; +//! when it lands, replacing `write_synthetic_frame` in +//! `capture_with_state` is the one function change. //! //! `cargo test` does **not** touch Wayland or the DRM stack. - use std::collections::HashMap; use std::path::Path; use wayland_client::protocol::{wl_buffer, wl_output, wl_registry}; @@ -205,9 +214,11 @@ pub async fn capture_via_export_for( } /// Public entry: the dmabuf path of `capture_toplevel`. Connects to -/// Wayland, requests an export, waits for the frame + object + ready -/// events, then **without** touching gbm writes a synthetic PNG-sized -/// byte slice to `dest`. The gbm bo map is a documented follow-up. +/// Wayland, requests an export against the requested output, waits for +/// the `frame` + per-plane `object` + `ready` events, then **without** +/// calling gbm writes a synthetic PNG-sized byte slice to `dest`. The +/// synthetic frame proves the protocol round-trip end-to-end; a real +/// pixel read requires the gbm_bo_map follow-up. pub async fn capture_via_export( output: &wl_output::WlOutput, dest: &Path, diff --git a/src/vfx.rs b/src/vfx.rs index 17d10eb..51c1b4f 100644 --- a/src/vfx.rs +++ b/src/vfx.rs @@ -134,12 +134,28 @@ pub fn toplevel_enabled() -> bool { .unwrap_or(false) } -/// Capture the source window via `zwlr_export_dmabuf_unstable_v1` and write -/// the resulting frame to `dest`. Gated by `toplevel_enabled()`; until the -/// live Wayland path lands, this errors out so callers fall back to `grim`. -/// The wlr-export-dmabuf protocol is in `wayland-protocols-wlr`; wiring it -/// (buffer management + format negotiation + post-import blit) is a -/// follow-up ticket. +/// Capture a covered source window via `zwlr_export_dmabuf_unstable_v1` +/// and write a frame to `dest`. Gated by `toplevel_enabled()`. +/// +/// ## Honest status (Bug #9 follow-up) +/// +/// Today this function calls `toplevel_export::capture_via_export_for`, +/// which issues `manager.capture_output(...)` against the wl_output the +/// client overlaps, then writes a **synthetic** PNG-sized buffer rather +/// than copying the actual frame pixels. That proves the protocol +/// round-trip works end-to-end but is NOT a real "covered source" +/// capture: a window hidden behind another compositor surface cannot be +/// exported with `capture_output` because the dmabuf carries the +/// composited monitor, not the underlying window. +/// +/// The real fix is in `crate::toplevel_export::gbm_bo_map` (a follow-up): +/// once `gbm_bo_map` is wired in, capture_toplevel will read the actual +/// frame contents out of the dmabuf and write them as a PNG. Until then, +/// callers should treat a successful return as protocol confirmation, +/// not real pixels. +/// +/// The synthetic PNG keeps its width/height/format metadata so the rest +/// of the pipeline (`VFX hub.show_frame` etc.) still works end-to-end. pub async fn capture_toplevel(client: &Client, dest: &Path) -> Result<()> { if !toplevel_enabled() { anyhow::bail!("toplevel export disabled (set ENBOXER_ENABLE_TOPLEVEL=1)");